Wait for a reply from various controller commands

- Resolves: rhbz2221084
- Related: rhbz2189301

006-controller-reply.patch

@@ -0,0 +1,109 @@
+From 3e31da0016795397bfeacb2f3d76ecfe35cc1f67 Mon Sep 17 00:00:00 2001
+From: Ken Gaillot <kgaillot@redhat.com>
+Date: Mon, 17 Jul 2023 14:52:42 -0500
+Subject: [PATCH] Fix: libcrmcommon: wait for reply from appropriate controller
+ commands
+
+ipc_controld.c:reply_expected() wrongly omitted PCMK__CONTROLD_CMD_NODES (which
+hasn't been a problem because crm_node uses a mainloop instead of sync dispatch
+for that) and CRM_OP_RM_NODE_CACHE (which can be sent via
+ipc_client.c:pcmk_ipc_purge_node()).
+
+Because CRM_OP_RM_NODE_CACHE gets only an ack and no further replies, we now
+have to be careful not to return true from the controller's dispatch()
+function, otherwise crm_node -R would wait forever for more data. That means
+we have to check for whether any replies are expected, which means we have to
+increment expected replies *before* sending a request (in case it's sync).
+
+Regression introduced in 2.0.5 by ae14fa4a
+
+Fixes T681
+---
+ lib/common/ipc_controld.c | 49 ++++++++++++++-------------------------
+ 1 file changed, 17 insertions(+), 32 deletions(-)
+
+diff --git a/lib/common/ipc_controld.c b/lib/common/ipc_controld.c
+index 3c3a98964..405fd0518 100644
+--- a/lib/common/ipc_controld.c
++++ b/lib/common/ipc_controld.c
+@@ -177,18 +177,16 @@ set_nodes_data(pcmk_controld_api_reply_t *data, xmlNode *msg_data)
+ static bool
+ reply_expected(pcmk_ipc_api_t *api, xmlNode *request)
+ {
+-    const char *command = crm_element_value(request, F_CRM_TASK);
+-
+-    if (command == NULL) {
+-        return false;
+-    }
+-
+-    // We only need to handle commands that functions in this file can send
+-    return !strcmp(command, CRM_OP_REPROBE)
+-           || !strcmp(command, CRM_OP_NODE_INFO)
+-           || !strcmp(command, CRM_OP_PING)
+-           || !strcmp(command, CRM_OP_LRM_FAIL)
+-           || !strcmp(command, CRM_OP_LRM_DELETE);
++    // We only need to handle commands that API functions can send
++    return pcmk__str_any_of(crm_element_value(request, F_CRM_TASK),
++                            PCMK__CONTROLD_CMD_NODES,
++                            CRM_OP_LRM_DELETE,
++                            CRM_OP_LRM_FAIL,
++                            CRM_OP_NODE_INFO,
++                            CRM_OP_PING,
++                            CRM_OP_REPROBE,
++                            CRM_OP_RM_NODE_CACHE,
++                            NULL);
+ }
+ 
+ static bool
+@@ -202,22 +200,12 @@ dispatch(pcmk_ipc_api_t *api, xmlNode *reply)
+         pcmk_controld_reply_unknown, NULL, NULL,
+     };
+ 
+-    /* If we got an ACK, return true so the caller knows to expect more responses
+-     * from the IPC server.  We do this before decrementing replies_expected because
+-     * ACKs are not going to be included in that value.
+-     *
+-     * Note that we cannot do the same kind of status checking here that we do in
+-     * ipc_pacemakerd.c.  The ACK message we receive does not necessarily contain
+-     * a status attribute.  That is, we may receive this:
+-     *
+-     * <ack function="crmd_remote_proxy_cb" line="556"/>
+-     *
+-     * Instead of this:
+-     *
+-     * <ack function="dispatch_controller_ipc" line="391" status="112"/>
+-     */
+     if (pcmk__str_eq(crm_element_name(reply), "ack", pcmk__str_none)) {
+-        return true; // More replies needed
++        /* ACKs are trivial responses that do not count toward expected replies,
++         * and do not have all the fields that validation requires, so skip that
++         * processing.
++         */
++        return private->replies_expected > 0;
+     }
+ 
+     if (private->replies_expected > 0) {
+@@ -344,18 +332,15 @@ static int
+ send_controller_request(pcmk_ipc_api_t *api, xmlNode *request,
+                         bool reply_is_expected)
+ {
+-    int rc;
+-
+     if (crm_element_value(request, XML_ATTR_REFERENCE) == NULL) {
+         return EINVAL;
+     }
+-    rc = pcmk__send_ipc_request(api, request);
+-    if ((rc == pcmk_rc_ok) && reply_is_expected) {
++    if (reply_is_expected) {
+         struct controld_api_private_s *private = api->api_data;
+ 
+         private->replies_expected++;
+     }
+-    return rc;
++    return pcmk__send_ipc_request(api, request);
+ }
+ 
+ static xmlNode *
+-- 
+2.41.0
+

pacemaker.spec

@@ -36,7 +36,7 @@
 ## can be incremented to build packages reliably considered "newer"
 ## than previously built packages with the same pcmkversion)
 %global pcmkversion 2.1.6
-%global specversion 6
+%global specversion 7
 
 ## Upstream commit (full commit ID, abbreviated commit ID, or tag) to build
 %global commit 6fdc9deea294bbad629b003c6ae036aaed8e3ee0
@@ -253,6 +253,7 @@ Patch002:      002-group-colocation-constraint.patch
 Patch003:      003-clone-shuffle.patch
 Patch004:      004-clone-rsc-display.patch
 Patch005:      005-attrd-dampen.patch
+Patch006:      006-controller-reply.patch
 
 Requires:      resource-agents
 Requires:      %{pkgname_pcmk_libs}%{?_isa} = %{version}-%{release}
@@ -902,6 +903,11 @@ exit 0
 %license %{nagios_name}-%{nagios_hash}/COPYING
 
 %changelog
+* Thu Jul 27 2023 Chris Lumens <clumens@redhat.com> - 2.1.6-7
+- Wait for a reply from various controller commands
+- Resolves: rhbz2221084
+- Related: rhbz2189301
+
 * Mon Jul 24 2023 Chris Lumens <clumens@redhat.com> - 2.1.6-6
 - Apply dampening when creating attributes with attrd_updater -U
 - Resolves: rhbz2224051