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