45cd8f
From 0d15568a538349ac41028db6b506d13dd23e8732 Mon Sep 17 00:00:00 2001
45cd8f
From: Chris Lumens <clumens@redhat.com>
45cd8f
Date: Tue, 14 Feb 2023 14:00:37 -0500
45cd8f
Subject: [PATCH] High: libcrmcommon: Fix handling node=NULL in
45cd8f
 pcmk__attrd_api_query.
45cd8f
45cd8f
According to the header file, if node is NULL, pcmk__attrd_api_query
45cd8f
should query the value of the given attribute on all cluster nodes.
45cd8f
This is also what the server expects and how attrd_updater is supposed
45cd8f
to work.
45cd8f
45cd8f
However, pcmk__attrd_api_query has no way of letting callers decide
45cd8f
whether they want to query all nodes or whether they want to use the
45cd8f
local node.  We were passing NULL for the node name, which it took to
45cd8f
mean it should look up the local node name.  This calls
45cd8f
pcmk__node_attr_target, which probes the local cluster name and returns
45cd8f
that to pcmk__attrd_api_query.  If it returns non-NULL, that value will
45cd8f
then be put into the XML IPC call which means the server will only
45cd8f
return the value for that node.
45cd8f
45cd8f
In testing this was usually fine.  However, in pratice, the methods
45cd8f
pcmk__node_attr_target uses to figure out the local cluster node name
45cd8f
involves checking the OCF_RESKEY_CRM_meta_on_node environment variable
45cd8f
among others.
45cd8f
45cd8f
This variable was never set in testing, but can be set in the real
45cd8f
world.  This leads to circumstances where the user did "attrd_updater -QA"
45cd8f
expecting to get the values on all nodes, but instead only got the value
45cd8f
on the local cluster node.
45cd8f
45cd8f
In pacemaker-2.1.4 and prior, pcmk__node_attr_target was simply never
45cd8f
called if the node was NULL but was called otherwise.
45cd8f
45cd8f
The fix is to modify pcmk__attrd_api_query to take an option for
45cd8f
querying all nodes.  If that's present, we'll query all nodes.  If it's
45cd8f
not present, we'll look at the given node name - NULL means look it up,
45cd8f
anything else means just that node.
45cd8f
45cd8f
Regression in 2.1.5 introduced by eb20a65577
45cd8f
---
45cd8f
 include/crm/common/attrd_internal.h     |  6 +++++-
45cd8f
 include/crm/common/ipc_attrd_internal.h |  7 +++++--
45cd8f
 lib/common/ipc_attrd.c                  | 12 ++++++++----
45cd8f
 tools/attrd_updater.c                   |  5 +++--
45cd8f
 4 files changed, 21 insertions(+), 9 deletions(-)
45cd8f
45cd8f
diff --git a/include/crm/common/attrd_internal.h b/include/crm/common/attrd_internal.h
45cd8f
index 389be48..7337c38 100644
45cd8f
--- a/include/crm/common/attrd_internal.h
45cd8f
+++ b/include/crm/common/attrd_internal.h
45cd8f
@@ -1,5 +1,5 @@
45cd8f
 /*
45cd8f
- * Copyright 2004-2022 the Pacemaker project contributors
45cd8f
+ * Copyright 2004-2023 the Pacemaker project contributors
45cd8f
  *
45cd8f
  * The version control history for this file may have further details.
45cd8f
  *
45cd8f
@@ -25,6 +25,10 @@ enum pcmk__node_attr_opts {
45cd8f
     pcmk__node_attr_perm           = (1 << 5),
45cd8f
     pcmk__node_attr_sync_local     = (1 << 6),
45cd8f
     pcmk__node_attr_sync_cluster   = (1 << 7),
45cd8f
+    // pcmk__node_attr_utilization is 8, but that has not been backported.
45cd8f
+    // I'm leaving the gap here in case we backport that in the future and
45cd8f
+    // also to avoid problems on mixed-version clusters.
45cd8f
+    pcmk__node_attr_query_all      = (1 << 9),
45cd8f
 };
45cd8f
 
45cd8f
 #define pcmk__set_node_attr_flags(node_attr_flags, flags_to_set) do {   \
45cd8f
diff --git a/include/crm/common/ipc_attrd_internal.h b/include/crm/common/ipc_attrd_internal.h
45cd8f
index 2c6713f..b1b7584 100644
45cd8f
--- a/include/crm/common/ipc_attrd_internal.h
45cd8f
+++ b/include/crm/common/ipc_attrd_internal.h
45cd8f
@@ -1,5 +1,5 @@
45cd8f
 /*
45cd8f
- * Copyright 2022 the Pacemaker project contributors
45cd8f
+ * Copyright 2022-2023 the Pacemaker project contributors
45cd8f
  *
45cd8f
  * The version control history for this file may have further details.
45cd8f
  *
45cd8f
@@ -110,10 +110,13 @@ int pcmk__attrd_api_purge(pcmk_ipc_api_t *api, const char *node);
45cd8f
  *
45cd8f
  * \param[in,out] api           Connection to pacemaker-attrd
45cd8f
  * \param[in]     node          Look up the attribute for this node
45cd8f
- *                              (or NULL for all nodes)
45cd8f
+ *                              (or NULL for the local node)
45cd8f
  * \param[in]     name          Attribute name
45cd8f
  * \param[in]     options       Bitmask of pcmk__node_attr_opts
45cd8f
  *
45cd8f
+ * \note Passing pcmk__node_attr_query_all will cause the function to query
45cd8f
+ *       the value of \p name on all nodes, regardless of the value of \p node.
45cd8f
+ *
45cd8f
  * \return Standard Pacemaker return code
45cd8f
  */
45cd8f
 int pcmk__attrd_api_query(pcmk_ipc_api_t *api, const char *node, const char *name,
45cd8f
diff --git a/lib/common/ipc_attrd.c b/lib/common/ipc_attrd.c
45cd8f
index 4606509..dece49b 100644
45cd8f
--- a/lib/common/ipc_attrd.c
45cd8f
+++ b/lib/common/ipc_attrd.c
45cd8f
@@ -1,5 +1,5 @@
45cd8f
 /*
45cd8f
- * Copyright 2011-2022 the Pacemaker project contributors
45cd8f
+ * Copyright 2011-2023 the Pacemaker project contributors
45cd8f
  *
45cd8f
  * The version control history for this file may have further details.
45cd8f
  *
45cd8f
@@ -332,10 +332,14 @@ pcmk__attrd_api_query(pcmk_ipc_api_t *api, const char *node, const char *name,
45cd8f
         return EINVAL;
45cd8f
     }
45cd8f
 
45cd8f
-    target = pcmk__node_attr_target(node);
45cd8f
+    if (pcmk_is_set(options, pcmk__node_attr_query_all)) {
45cd8f
+        node = NULL;
45cd8f
+    } else {
45cd8f
+        target = pcmk__node_attr_target(node);
45cd8f
 
45cd8f
-    if (target != NULL) {
45cd8f
-        node = target;
45cd8f
+        if (target != NULL) {
45cd8f
+            node = target;
45cd8f
+        }
45cd8f
     }
45cd8f
 
45cd8f
     request = create_attrd_op(NULL);
45cd8f
diff --git a/tools/attrd_updater.c b/tools/attrd_updater.c
45cd8f
index 3cd766d..cbd341d 100644
45cd8f
--- a/tools/attrd_updater.c
45cd8f
+++ b/tools/attrd_updater.c
45cd8f
@@ -376,6 +376,7 @@ attrd_event_cb(pcmk_ipc_api_t *attrd_api, enum pcmk_ipc_event event_type,
45cd8f
 static int
45cd8f
 send_attrd_query(pcmk__output_t *out, const char *attr_name, const char *attr_node, gboolean query_all)
45cd8f
 {
45cd8f
+    uint32_t options = pcmk__node_attr_none;
45cd8f
     pcmk_ipc_api_t *attrd_api = NULL;
45cd8f
     int rc = pcmk_rc_ok;
45cd8f
 
45cd8f
@@ -400,10 +401,10 @@ send_attrd_query(pcmk__output_t *out, const char *attr_name, const char *attr_no
45cd8f
 
45cd8f
     /* Decide which node(s) to query */
45cd8f
     if (query_all == TRUE) {
45cd8f
-        attr_node = NULL;
45cd8f
+        options |= pcmk__node_attr_query_all;
45cd8f
     }
45cd8f
 
45cd8f
-    rc = pcmk__attrd_api_query(attrd_api, attr_node, attr_name, 0);
45cd8f
+    rc = pcmk__attrd_api_query(attrd_api, attr_node, attr_name, options);
45cd8f
 
45cd8f
     if (rc != pcmk_rc_ok) {
45cd8f
         g_set_error(&error, PCMK__RC_ERROR, rc, "Could not query value of %s: %s (%d)",
45cd8f
-- 
45cd8f
2.31.1
45cd8f