Fix: fencer: Refresh CIB devices on change to nodes section

This fixes a regression introduced by bf7ffcd. As of that commit, the
fake local node is created after all resources have been unpacked. So it
doesn't get added to resources' allowed_nodes tables.

This prevents registration of fencing devices when the fencer receives a
CIB diff that removes the local node. For example, the user may have
replaced the CIB with a boilerplate configuration that has an empty
nodes section. Registering a fencing device requires that the local node
be in the resource's allowed nodes table.

One option would be to add the fake local node to all resources' allowed
nodes tables immediately after creating it. However, it shouldn't
necessarily be an allowed node for all resources. For example, if
symmetric-cluster=false, then a node should not be placed in a
resource's allowed nodes table by default.

It's difficult to ensure correctness of the allowed nodes tables when a
fake local node is involved. It may involve repeated code or a fragile
and confusing dependence on the order of unpacking.

Since the fake local node is a hack in the first place, it seems better
to avoid using it where possible. Currently the only code that even sets
the local_node_name member of scheduler->priv is in:
* the fencer
* crm_mon when showing the "times" section

This commit works as follows. If the fencer receives a CIB diff
notification that contains the nodes section, it triggers a full refresh
of fencing device registrations. In our example above, where a user has
replaced the CIB with a configuration that has an empty nodes section,
this means all fencing device registrations will be removed.

However, the controller also has a CIB diff notification callback:
do_cib_updated(). The controller's callback repopulates the nodes
section with up-to-date information from the cluster layer (or its node
cache) if it finds that an untrusted client like cibadmin has sent a
modified the nodes section. Then it updates the CIB accordingly.

The fencer will receive this updated CIB and refresh fencing device
registrations again, re-registering the fencing devices that were just
removed.

Note that in the common case, we're not doing all this wasted work. The
"remove and then re-register" sequence should happen only when a user
has modified the CIB in a sloppy way (for example, by deleting nodes
from the CIB's nodes section that have not been removed from the
cluster).

In short, it seems better to rely on the controller's maintenance of the
CIB's node list, than to rely on a "fake local node" hack in the
scheduler.

See the following pull requests from Hideo Yamauchi and their
discussions:
#3849
#3852

Thanks to Hideo for the report and finding the cause.

Signed-off-by: Reid Wahl <[email protected]>

Author: nrwahl2

daemons/fenced/fenced_cib.c

@@ -294,7 +294,23 @@ update_cib_stonith_devices(const xmlNode *patchset)
             }
         }
 
-        if (strstr(xpath, "/" PCMK_XE_RESOURCES)
+        /* @FIXME Suppose a node gets deleted in one CIB diff and then re-added
+         * in another. For example, this can happen if a client deletes a node
+         * from the CIB when it's still part of the cluster, and then the
+         * controller's diff callback repopulates the nodes section. If
+         * responding to the deletion here causes a stonith device to get
+         * unregistered, then the next monitor will fail. This is because when
+         * the device gets re-registered after nodes are repopulated, the
+         * api_registered flag doesn't get set. Only the cib_registered flag
+         * gets set.
+         *
+         * See commit message in eaee0fc for some more info.
+         *
+         * The fencer should not refuse to run a monitor action for a resource
+         * that the controller knows is started.
+         */
+        if (strstr(xpath, "/" PCMK_XE_NODES)
+            || strstr(xpath, "/" PCMK_XE_RESOURCES)
             || strstr(xpath, "/" PCMK_XE_CONSTRAINTS)
             || strstr(xpath, "/" PCMK_XE_RSC_DEFAULTS)) {
 

daemons/fenced/fenced_scheduler.c

@@ -52,35 +52,6 @@ fenced_scheduler_init(void)
     return pcmk_rc_ok;
 }
 
-/*!
- * \internal
- * \brief Set the local node name for scheduling purposes
- *
- * \param[in] node_name  Name to set as local node name
- */
-void
-fenced_set_local_node(const char *node_name)
-{
-    pcmk__assert(scheduler != NULL);
-
-    scheduler->priv->local_node_name = pcmk__str_copy(node_name);
-}
-
-/*!
- * \internal
- * \brief Get the local node name
- *
- * \return Local node name
- */
-const char *
-fenced_get_local_node(void)
-{
-    if (scheduler == NULL) {
-        return NULL;
-    }
-    return scheduler->priv->local_node_name;
-}
-
 /*!
  * \internal
  * \brief Free all scheduler-related resources
@@ -112,14 +83,15 @@ fenced_scheduler_cleanup(void)
 static pcmk_node_t *
 local_node_allowed_for(const pcmk_resource_t *rsc)
 {
-    if ((rsc != NULL) && (scheduler->priv->local_node_name != NULL)) {
+    const char *local_node = fenced_get_local_node();
+
+    if ((rsc != NULL) && (local_node != NULL)) {
         GHashTableIter iter;
         pcmk_node_t *node = NULL;
 
         g_hash_table_iter_init(&iter, rsc->priv->allowed_nodes);
         while (g_hash_table_iter_next(&iter, NULL, (void **) &node)) {
-            if (pcmk__str_eq(node->priv->name, scheduler->priv->local_node_name,
-                             pcmk__str_casei)) {
+            if (pcmk__str_eq(node->priv->name, local_node, pcmk__str_casei)) {
                 return node;
             }
         }

daemons/fenced/pacemaker-fenced.c

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2024 the Pacemaker project contributors
+ * Copyright 2009-2025 the Pacemaker project contributors
  *
  * The version control history for this file may have further details.
  *
@@ -40,6 +40,9 @@
 
 #define SUMMARY "daemon for executing fencing devices in a Pacemaker cluster"
 
+//! Local node's name in cluster
+static char *local_node_name = NULL;
+
 // @TODO This should be guint
 long long stonith_watchdog_timeout_ms = 0;
 
@@ -68,6 +71,18 @@ crm_exit_t exit_code = CRM_EX_OK;
 
 static void stonith_cleanup(void);
 
+/*!
+ * \internal
+ * \brief Get the local node name
+ *
+ * \return Local node name
+ */
+const char *
+fenced_get_local_node(void)
+{
+    return local_node_name;
+}
+
 static int32_t
 st_ipc_accept(qb_ipcs_connection_t * c, uid_t uid, gid_t gid)
 {
@@ -628,7 +643,7 @@ main(int argc, char **argv)
         crm_crit("Cannot sign in to the cluster... terminating");
         goto done;
     }
-    fenced_set_local_node(cluster->priv->node_name);
+    local_node_name = pcmk__str_copy(cluster->priv->node_name);
 
     if (!options.stand_alone) {
         setup_cib();
@@ -654,6 +669,8 @@ main(int argc, char **argv)
     pcmk_cluster_free(cluster);
     fenced_scheduler_cleanup();
 
+    free(local_node_name);
+
     pcmk__output_and_clear_error(&error, out);
 
     if (out != NULL) {

daemons/fenced/pacemaker-fenced.h

@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2024 the Pacemaker project contributors
+ * Copyright 2009-2025 the Pacemaker project contributors
  *
  * This source code is licensed under the GNU General Public License version 2
  * or later (GPLv2+) WITHOUT ANY WARRANTY.
@@ -13,6 +13,8 @@
 #include <crm/stonith-ng.h>
 #include <crm/fencing/internal.h>
 
+const char *fenced_get_local_node(void);
+
 /*!
  * \internal
  * \brief Check whether target has already been fenced recently
@@ -296,8 +298,6 @@ void setup_cib(void);
 void fenced_cib_cleanup(void);
 
 int fenced_scheduler_init(void);
-void fenced_set_local_node(const char *node_name);
-const char *fenced_get_local_node(void);
 void fenced_scheduler_cleanup(void);
 void fenced_scheduler_run(xmlNode *cib);