Change Details

This affects `attrd_updater` and `crm_attribute`. See https://github.com/ClusterLabs/pacemaker/pull/3277#issuecomment-1846224118. `pcmk__node_attr_target()` looks up environment variables that don't exist: * `CRM_meta_container_attribute_target` instead of `OCF_RESKEY_CRM_meta_container_attribute_target` * `CRM_meta_physical_host` instead of `OCF_RESKEY_CRM_meta_physical_host` OpenStack RAs work because they call `ocf_attribute_target()`. This avoids calling an attribute-update command with no node set. ``` ocf_attribute_target() { if [ x$1 = x ]; then if [ x$OCF_RESKEY_CRM_meta_container_attribute_target = xhost -a x$OCF_RESKEY_CRM_meta_physical_host != x ]; then echo $OCF_RESKEY_CRM_meta_physical_host else if [ x$OCF_RESKEY_CRM_meta_on_node != x ]; then echo $OCF_RESKEY_CRM_meta_on_node else ocf_local_nodename fi fi return ... ``` That was added 6 years ago by https://github.com/ClusterLabs/resource-agents/commit/708e11c1. That was very shortly after the equivalent of `pcmk__node_attr_target()` was added to Pacemaker: * https://github.com/ClusterLabs/pacemaker/commit/cf34f4c95 * https://github.com/ClusterLabs/pacemaker/commit/ccbdb2a2 To me it seems likely that `pcmk__node_attr_target()` has never worked for `container-attribute-target="host"`. It might not have been tested properly when those commits were added, and then `ocf_attribute_target()` masked the issue in resource agents. We can either 1. Fix `pcmk__node_attr_target()` so that it uses the correct environment variables (as a failsafe in case an RA author forgets to use `ocf_attribute_target()`), or 2. We can simply drop all or part of `pcmk__node_attr_target()`. * This part has probably never worked. * `ocf_attribute_target()`'s logic is identical to what *the entirety of* `pcmk__node_attr_target()` is supposed to do, and more (`OCF_RESKEY_CRM_meta_notify_all_uname`). * The fix would have no effect on existing container images, since the call to `pcmk__node_attr_target()` happens on the container. It would only affect container images built with a new version of Pacemaker. So RAs will have to continue using `ocf_attribute_target()` for the foreseeable future, as a container might be running an older Pacemaker version. * Under normal circumstances, those environment variables should never be set anyway outside of an RA execution by Pacemaker. So they don't play a role in CLI execution of `attrd_updater`/`crm_attribute` on a container outside of an RA. * It's unusual for Pacemaker's C code to check `OCF_RESKEY` variables. Those are typically consumed only by OCF resource agents. Here, Pacemaker sets them for RAs and then checks them when RAs call back into Pacemaker. (There is one other case, when `crm_node` checks `OCF_RESKEY_CRM_meta_on_node` -- although that looks like it's used only as a small optimization and could be dropped if desired.) In short, `pcmk__node_attr_target()` is basically redundant code at this point. It's only relevant within an RA on a container, and `ocf_attribute_target()` handles that case.

This affects `attrd_updater` and `crm_attribute`. See https://github.com/ClusterLabs/pacemaker/pull/3277#issuecomment-1846224118. `pcmk__node_attr_target()` looks up environment variables that don't exist: * `CRM_meta_container_attribute_target` instead of `OCF_RESKEY_CRM_meta_container_attribute_target` * `CRM_meta_physical_host` instead of `OCF_RESKEY_CRM_meta_physical_host` OpenStack RAs work because they call `ocf_attribute_target()`. This avoids calling an attribute-update command with no node set. ``` ocf_attribute_target() { if [ x$1 = x ]; then if [ x$OCF_RESKEY_CRM_meta_container_attribute_target = xhost -a x$OCF_RESKEY_CRM_meta_physical_host != x ]; then echo $OCF_RESKEY_CRM_meta_physical_host else if [ x$OCF_RESKEY_CRM_meta_on_node != x ]; then echo $OCF_RESKEY_CRM_meta_on_node else ocf_local_nodename fi fi return ... ``` That was added 6 years ago by https://github.com/ClusterLabs/resource-agents/commit/708e11c1. That was very shortly after the equivalent of `pcmk__node_attr_target()` was added to Pacemaker: * https://github.com/ClusterLabs/pacemaker/commit/cf34f4c95 * https://github.com/ClusterLabs/pacemaker/commit/ccbdb2a2 To me it seems likely that `pcmk__node_attr_target()` has never worked for `container-attribute-target="host"`. It might not have been tested properly when those commits were added, and then `ocf_attribute_target()` masked the issue in resource agents. We can either 1. Fix `pcmk__node_attr_target()` so that it uses the correct environment variables (as a failsafe in case an RA author forgets to use `ocf_attribute_target()`), or 2. Simply drop all or part of `pcmk__node_attr_target()`. * This part has probably never worked. * `ocf_attribute_target()`'s logic is identical to what *the entirety of* `pcmk__node_attr_target()` is supposed to do, and more (`OCF_RESKEY_CRM_meta_notify_all_uname`). * The fix would have no effect on existing container images, since the call to `pcmk__node_attr_target()` happens on the container. It would only affect container images built with a new version of Pacemaker. So RAs will have to continue using `ocf_attribute_target()` for the foreseeable future, as a container might be running an older Pacemaker version. * Under normal circumstances, those environment variables should never be set anyway outside of an RA execution by Pacemaker. So they don't play a role in CLI execution of `attrd_updater`/`crm_attribute` on a container outside of an RA. * It's unusual for Pacemaker's C code to check `OCF_RESKEY` variables. Those are typically consumed only by OCF resource agents. Here, Pacemaker sets them for RAs and then checks them when RAs call back into Pacemaker. (There is one other case, when `crm_node` checks `OCF_RESKEY_CRM_meta_on_node` -- although that looks like it's used only as a small optimization and could be dropped if desired.) In short, `pcmk__node_attr_target()` is basically redundant code at this point. It's only relevant within an RA on a container, and `ocf_attribute_target()` handles that case.

This affects `attrd_updater` and `crm_attribute`. See https://github.com/ClusterLabs/pacemaker/pull/3277#issuecomment-1846224118. `pcmk__node_attr_target()` looks up environment variables that don't exist: * `CRM_meta_container_attribute_target` instead of `OCF_RESKEY_CRM_meta_container_attribute_target` * `CRM_meta_physical_host` instead of `OCF_RESKEY_CRM_meta_physical_host` OpenStack RAs work because they call `ocf_attribute_target()`. This avoids calling an attribute-update command with no node set. ``` ocf_attribute_target() { if [ x$1 = x ]; then if [ x$OCF_RESKEY_CRM_meta_container_attribute_target = xhost -a x$OCF_RESKEY_CRM_meta_physical_host != x ]; then echo $OCF_RESKEY_CRM_meta_physical_host else if [ x$OCF_RESKEY_CRM_meta_on_node != x ]; then echo $OCF_RESKEY_CRM_meta_on_node else ocf_local_nodename fi fi return ... ``` That was added 6 years ago by https://github.com/ClusterLabs/resource-agents/commit/708e11c1. That was very shortly after the equivalent of `pcmk__node_attr_target()` was added to Pacemaker: * https://github.com/ClusterLabs/pacemaker/commit/cf34f4c95 * https://github.com/ClusterLabs/pacemaker/commit/ccbdb2a2 To me it seems likely that `pcmk__node_attr_target()` has never worked for `container-attribute-target="host"`. It might not have been tested properly when those commits were added, and then `ocf_attribute_target()` masked the issue in resource agents. We can either 1. Fix `pcmk__node_attr_target()` so that it uses the correct environment variables (as a failsafe in case an RA author forgets to use `ocf_attribute_target()`), or 2. We can sSimply drop all or part of `pcmk__node_attr_target()`. * This part has probably never worked. * `ocf_attribute_target()`'s logic is identical to what *the entirety of* `pcmk__node_attr_target()` is supposed to do, and more (`OCF_RESKEY_CRM_meta_notify_all_uname`). * The fix would have no effect on existing container images, since the call to `pcmk__node_attr_target()` happens on the container. It would only affect container images built with a new version of Pacemaker. So RAs will have to continue using `ocf_attribute_target()` for the foreseeable future, as a container might be running an older Pacemaker version. * Under normal circumstances, those environment variables should never be set anyway outside of an RA execution by Pacemaker. So they don't play a role in CLI execution of `attrd_updater`/`crm_attribute` on a container outside of an RA. * It's unusual for Pacemaker's C code to check `OCF_RESKEY` variables. Those are typically consumed only by OCF resource agents. Here, Pacemaker sets them for RAs and then checks them when RAs call back into Pacemaker. (There is one other case, when `crm_node` checks `OCF_RESKEY_CRM_meta_on_node` -- although that looks like it's used only as a small optimization and could be dropped if desired.) In short, `pcmk__node_attr_target()` is basically redundant code at this point. It's only relevant within an RA on a container, and `ocf_attribute_target()` handles that case.