From 28566d6832274c59f27bb7b2f1f54420a3f3d822 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Thu, 9 May 2019 20:26:08 -0500

Subject: [PATCH 01/13] Refactor: libpe_status: functionize unfencing digest

 code more

... for readability, reusability, and avoiding unnecessary function calls or

memory allocation.

---

 lib/pengine/utils.c | 159 ++++++++++++++++++++++++++++++++++++++--------------

 1 file changed, 118 insertions(+), 41 deletions(-)

diff --git a/lib/pengine/utils.c b/lib/pengine/utils.c

index 2f4dc1e..f80f8d4 100644

--- a/lib/pengine/utils.c

+++ b/lib/pengine/utils.c

@@ -2080,57 +2080,134 @@ rsc_action_digest_cmp(resource_t * rsc, xmlNode * xml_op, node_t * node,

     return data;

 }

+/*!

+ * \internal

+ * \brief Create an unfencing summary for use in special node attribute

+ *

+ * Create a string combining a fence device's resource ID, agent type, and

+ * parameter digest (whether for all parameters or just non-private parameters).

+ * This can be stored in a special node attribute, allowing us to detect changes

+ * in either the agent type or parameters, to know whether unfencing must be

+ * redone or can be safely skipped when the device's history is cleaned.

+ *

+ * \param[in] rsc_id        Fence device resource ID

+ * \param[in] agent_type    Fence device agent

+ * \param[in] param_digest  Fence device parameter digest

+ *

+ * \return Newly allocated string with unfencing digest

+ * \note The caller is responsible for freeing the result.

+ */

+static inline char *

+create_unfencing_summary(const char *rsc_id, const char *agent_type,

+                         const char *param_digest)

+{

+    return crm_strdup_printf("%s:%s:%s", rsc_id, agent_type, param_digest);

+}

+

+/*!

+ * \internal

+ * \brief Check whether a node can skip unfencing

+ *

+ * Check whether a fence device's current definition matches a node's

+ * stored summary of when it was last unfenced by the device.

+ *

+ * \param[in] rsc_id        Fence device's resource ID

+ * \param[in] agent         Fence device's agent type

+ * \param[in] digest_calc   Fence device's current parameter digest

+ * \param[in] node_summary  Value of node's special unfencing node attribute

+ *                          (a comma-separated list of unfencing summaries for

+ *                          all devices that have unfenced this node)

+ *

+ * \return TRUE if digest matches, FALSE otherwise

+ */

+static bool

+unfencing_digest_matches(const char *rsc_id, const char *agent,

+                         const char *digest_calc, const char *node_summary)

+{

+    bool matches = FALSE;

+

+    if (rsc_id && agent && digest_calc && node_summary) {

+        char *search_secure = create_unfencing_summary(rsc_id, agent,

+                                                       digest_calc);

+

+        /* The digest was calculated including the device ID and agent,

+         * so there is no risk of collision using strstr().

+         */

+        matches = (strstr(node_summary, search_secure) != NULL);

+        crm_trace("Calculated unfencing digest '%s' %sfound in '%s'",

+                  search_secure, matches? "" : "not ", node_summary);

+        free(search_secure);

+    }

+    return matches;

+}

+

+/* Magic string to use as action name for digest cache entries used for

+ * unfencing checks. This is not a real action name (i.e. "on"), so

+ * check_action_definition() won't confuse these entries with real actions.

+ */

 #define STONITH_DIGEST_TASK "stonith-on"

+/*!

+ * \internal

+ * \brief Calculate fence device digests and digest comparison result

+ *

+ * \param[in] rsc       Fence device resource

+ * \param[in] agent     Fence device's agent type

+ * \param[in] node      Node with digest cache to use

+ * \param[in] data_set  Cluster working set

+ *

+ * \return Node's digest cache entry

+ */

 static op_digest_cache_t *

-fencing_action_digest_cmp(resource_t * rsc, node_t * node, pe_working_set_t * data_set)

+fencing_action_digest_cmp(pe_resource_t *rsc, const char *agent,

+                          pe_node_t *node, pe_working_set_t *data_set)

 {

-    char *key = generate_op_key(rsc->id, STONITH_DIGEST_TASK, 0);

-    op_digest_cache_t *data = rsc_action_digest(rsc, STONITH_DIGEST_TASK, key, node, NULL, data_set);

+    const char *node_summary = NULL;

-    const char *digest_all = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_ALL);

-    const char *digest_secure = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_SECURE);

+    // Calculate device's current parameter digests

+    char *key = generate_op_key(rsc->id, STONITH_DIGEST_TASK, 0);

+    op_digest_cache_t *data = rsc_action_digest(rsc, STONITH_DIGEST_TASK, key,

+                                                node, NULL, data_set);

-    /* No 'reloads' for fencing device changes

-     *

-     * We use the resource id + agent + digest so that we can detect

-     * changes to the agent and/or the parameters used

-     */

-    char *search_all = crm_strdup_printf("%s:%s:%s", rsc->id, (const char*)g_hash_table_lookup(rsc->meta, XML_ATTR_TYPE), data->digest_all_calc);

-    char *search_secure = crm_strdup_printf("%s:%s:%s", rsc->id, (const char*)g_hash_table_lookup(rsc->meta, XML_ATTR_TYPE), data->digest_secure_calc);

+    free(key);

-    data->rc = RSC_DIGEST_ALL;

-    if (digest_all == NULL) {

-        /* it is unknown what the previous op digest was */

+    // Check whether node has special unfencing summary node attribute

+    node_summary = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_ALL);

+    if (node_summary == NULL) {

         data->rc = RSC_DIGEST_UNKNOWN;

+        return data;

+    }

-    } else if (strstr(digest_all, search_all)) {

+    // Check whether full parameter digest matches

+    if (unfencing_digest_matches(rsc->id, agent, data->digest_all_calc,

+                                 node_summary)) {

         data->rc = RSC_DIGEST_MATCH;

+        return data;

+    }

-    } else if(digest_secure && data->digest_secure_calc) {

-        if(strstr(digest_secure, search_secure)) {

-            if (is_set(data_set->flags, pe_flag_stdout)) {

-                printf("Only 'private' parameters to %s for unfencing %s changed\n",

-                       rsc->id, node->details->uname);

-            }

-            data->rc = RSC_DIGEST_MATCH;

+    // Check whether secure parameter digest matches

+    node_summary = pe_node_attribute_raw(node, CRM_ATTR_DIGESTS_SECURE);

+    if (unfencing_digest_matches(rsc->id, agent, data->digest_secure_calc,

+                                 node_summary)) {

+        data->rc = RSC_DIGEST_MATCH;

+        if (is_set(data_set->flags, pe_flag_stdout)) {

+            printf("Only 'private' parameters to %s for unfencing %s changed\n",

+                   rsc->id, node->details->uname);

         }

+        return data;

     }

-    if (is_set(data_set->flags, pe_flag_sanitized)

-        && is_set(data_set->flags, pe_flag_stdout)

-        && (data->rc == RSC_DIGEST_ALL)

+    // Parameters don't match

+    data->rc = RSC_DIGEST_ALL;

+    if (is_set(data_set->flags, (pe_flag_sanitized|pe_flag_stdout))

         && data->digest_secure_calc) {

-        printf("Parameters to %s for unfencing %s changed, try '%s:%s:%s'\n",

-               rsc->id, node->details->uname, rsc->id,

-               (const char *) g_hash_table_lookup(rsc->meta, XML_ATTR_TYPE),

-               data->digest_secure_calc);

-    }

-

-    free(key);

-    free(search_all);

-    free(search_secure);

+        char *digest = create_unfencing_summary(rsc->id, agent,

+                                                data->digest_secure_calc);

+        printf("Parameters to %s for unfencing %s changed, try '%s'\n",

+               rsc->id, node->details->uname, digest);

+        free(digest);

+    }

     return data;

 }

@@ -2218,9 +2295,6 @@ pe_fence_op(node_t * node, const char *op, bool optional, const char *reason, pe

              *

              * We may do this for all nodes in the future, but for now

              * the check_action_definition() based stuff works fine.

-             *

-             * Use "stonith-on" to avoid creating cache entries for

-             * operations check_action_definition() would look for.

              */

             long max = 1024;

             long digests_all_offset = 0;

@@ -2232,8 +2306,11 @@ pe_fence_op(node_t * node, const char *op, bool optional, const char *reason, pe

             for (GListPtr gIter = matches; gIter != NULL; gIter = gIter->next) {

                 resource_t *match = gIter->data;

-                op_digest_cache_t *data = fencing_action_digest_cmp(match, node, data_set);

+                const char *agent = g_hash_table_lookup(match->meta,

+                                                        XML_ATTR_TYPE);

+                op_digest_cache_t *data = NULL;

+                data = fencing_action_digest_cmp(match, agent, node, data_set);

                 if(data->rc == RSC_DIGEST_ALL) {

                     optional = FALSE;

                     crm_notice("Unfencing %s (remote): because the definition of %s changed", node->details->uname, match->id);

@@ -2244,11 +2321,11 @@ pe_fence_op(node_t * node, const char *op, bool optional, const char *reason, pe

                 digests_all_offset += snprintf(

                     digests_all+digests_all_offset, max-digests_all_offset,

-                    "%s:%s:%s,", match->id, (const char*)g_hash_table_lookup(match->meta, XML_ATTR_TYPE), data->digest_all_calc);

+                    "%s:%s:%s,", match->id, agent, data->digest_all_calc);

                 digests_secure_offset += snprintf(

                     digests_secure+digests_secure_offset, max-digests_secure_offset,

-                    "%s:%s:%s,", match->id, (const char*)g_hash_table_lookup(match->meta, XML_ATTR_TYPE), data->digest_secure_calc);

+                    "%s:%s:%s,", match->id, agent, data->digest_secure_calc);

             }

             g_hash_table_insert(stonith_op->meta,

                                 strdup(XML_OP_ATTR_DIGESTS_ALL),

-- 

1.8.3.1

From fd6e06ff419c95f4423202163d2d4dca3f03a4c5 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Fri, 10 May 2019 11:57:31 -0500

Subject: [PATCH 02/13] Fix: libpe_status: calculate secure digests for

 unfencing ops

The calculation of digests for detection of when unfencing is needed reused

rsc_action_digest(). However that would only add secure digests when the

pe_flag_sanitized flag was set, which is only set by crm_simulate, so secure

digests would never be added in normal cluster operation. This led to

node attributes like name="#digests-secure"

value="stonith-fence_compute-fence-nova:fence_compute:(null),".

Now, rsc_action_digest() takes a new argument to select whether secure digests

are added, which is always set to TRUE when calculating unfencing digests.

---

 lib/pengine/utils.c | 27 ++++++++++++++++++++++-----

 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/lib/pengine/utils.c b/lib/pengine/utils.c

index f80f8d4..5b893f7 100644

--- a/lib/pengine/utils.c

+++ b/lib/pengine/utils.c

@@ -1936,9 +1936,24 @@ append_versioned_params(xmlNode *versioned_params, const char *ra_version, xmlNo

 }

 #endif

+/*!

+ * \internal

+ * \brief Calculate action digests and store in node's digest cache

+ *

+ * \param[in] rsc          Resource that action was for

+ * \param[in] task         Name of action performed

+ * \param[in] key          Action's task key

+ * \param[in] node         Node action was performed on

+ * \param[in] xml_op       XML of operation in CIB status (if available)

+ * \param[in] calc_secure  Whether to calculate secure digest

+ * \param[in] data_set     Cluster working set

+ *

+ * \return Pointer to node's digest cache entry

+ */

 static op_digest_cache_t *

-rsc_action_digest(resource_t * rsc, const char *task, const char *key,

-                  node_t * node, xmlNode * xml_op, pe_working_set_t * data_set) 

+rsc_action_digest(pe_resource_t *rsc, const char *task, const char *key,

+                  pe_node_t *node, xmlNode *xml_op, bool calc_secure,

+                  pe_working_set_t *data_set)

 {

     op_digest_cache_t *data = NULL;

@@ -2007,7 +2022,7 @@ rsc_action_digest(resource_t * rsc, const char *task, const char *key,

         data->digest_all_calc = calculate_operation_digest(data->params_all, op_version);

-        if (is_set(data_set->flags, pe_flag_sanitized)) {

+        if (calc_secure) {

             data->params_secure = copy_xml(data->params_all);

             if(secure_list) {

                 filter_parameters(data->params_secure, secure_list, FALSE);

@@ -2053,7 +2068,9 @@ rsc_action_digest_cmp(resource_t * rsc, xmlNode * xml_op, node_t * node,

     interval_ms = crm_parse_ms(interval_ms_s);

     key = generate_op_key(rsc->id, task, interval_ms);

-    data = rsc_action_digest(rsc, task, key, node, xml_op, data_set);

+    data = rsc_action_digest(rsc, task, key, node, xml_op,

+                             is_set(data_set->flags, pe_flag_sanitized),

+                             data_set);

     data->rc = RSC_DIGEST_MATCH;

     if (digest_restart && data->digest_restart_calc && strcmp(data->digest_restart_calc, digest_restart) != 0) {

@@ -2167,7 +2184,7 @@ fencing_action_digest_cmp(pe_resource_t *rsc, const char *agent,

     // Calculate device's current parameter digests

     char *key = generate_op_key(rsc->id, STONITH_DIGEST_TASK, 0);

     op_digest_cache_t *data = rsc_action_digest(rsc, STONITH_DIGEST_TASK, key,

-                                                node, NULL, data_set);

+                                                node, NULL, TRUE, data_set);

     free(key);

-- 

1.8.3.1

From 7886c8ec4dd209078cdc76274ed9d2804ea09b6a Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Wed, 5 Jun 2019 12:54:34 -0500

Subject: [PATCH 03/13] Refactor: controller: pass desired op status when

 synthesizing failure

so we can use new status codes later

---

 daemons/controld/controld_execd.c | 27 +++++++++++++++++++++------

 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c

index 8e89216..fed9419 100644

--- a/daemons/controld/controld_execd.c

+++ b/daemons/controld/controld_execd.c

@@ -1424,8 +1424,22 @@ force_reprobe(lrm_state_t *lrm_state, const char *from_sys,

     update_attrd(lrm_state->node_name, CRM_OP_PROBED, NULL, user_name, is_remote_node);

 }

+/*!

+ * \internal

+ * \brief Fail a requested action without actually executing it

+ *

+ * For an action that can't be executed, process it similarly to an actual

+ * execution result, with specified error status (except for notify actions,

+ * which will always be treated as successful).

+ *

+ * \param[in] lrm_state  Executor connection that action is for

+ * \param[in] action     Action XML from request

+ * \param[in] rc         Desired return code to use

+ * \param[in] op_status  Desired operation status to use

+ */

 static void

-synthesize_lrmd_failure(lrm_state_t *lrm_state, xmlNode *action, int rc) 

+synthesize_lrmd_failure(lrm_state_t *lrm_state, xmlNode *action,

+                        int op_status, enum ocf_exitcode rc)

 {

     lrmd_event_data_t *op = NULL;

     const char *operation = crm_element_value(action, XML_LRM_ATTR_TASK);

@@ -1451,7 +1465,7 @@ synthesize_lrmd_failure(lrm_state_t *lrm_state, xmlNode *action, int rc)

     if (safe_str_eq(operation, RSC_NOTIFY)) { // Notifications can't fail

         fake_op_status(lrm_state, op, PCMK_LRM_OP_DONE, PCMK_OCF_OK);

     } else {

-        fake_op_status(lrm_state, op, PCMK_LRM_OP_ERROR, rc);

+        fake_op_status(lrm_state, op, op_status, rc);

     }

     crm_info("Faking " CRM_OP_FMT " result (%d) on %s",

@@ -1744,7 +1758,8 @@ do_lrm_invoke(long long action,

     if ((lrm_state == NULL) && is_remote_node) {

         crm_err("Failing action because local node has never had connection to remote node %s",

                 target_node);

-        synthesize_lrmd_failure(NULL, input->xml, PCMK_OCF_CONNECTION_DIED);

+        synthesize_lrmd_failure(NULL, input->xml, PCMK_LRM_OP_ERROR,

+                                PCMK_OCF_CONNECTION_DIED);

         return;

     }

     CRM_ASSERT(lrm_state != NULL);

@@ -1800,7 +1815,7 @@ do_lrm_invoke(long long action,

         rc = get_lrm_resource(lrm_state, xml_rsc, create_rsc, &rsc);

         if (rc == -ENOTCONN) {

-            synthesize_lrmd_failure(lrm_state, input->xml,

+            synthesize_lrmd_failure(lrm_state, input->xml, PCMK_LRM_OP_ERROR,

                                     PCMK_OCF_CONNECTION_DIED);

             return;

@@ -1822,7 +1837,7 @@ do_lrm_invoke(long long action,

             // Resource operation on malformed resource

             crm_err("Invalid resource definition for %s", ID(xml_rsc));

             crm_log_xml_warn(input->msg, "invalid resource");

-            synthesize_lrmd_failure(lrm_state, input->xml,

+            synthesize_lrmd_failure(lrm_state, input->xml, PCMK_LRM_OP_ERROR,

                                     PCMK_OCF_NOT_CONFIGURED); // fatal error

             return;

@@ -1832,7 +1847,7 @@ do_lrm_invoke(long long action,

                     CRM_XS " rc=%d",

                     ID(xml_rsc), pcmk_strerror(rc), rc);

             crm_log_xml_warn(input->msg, "failed registration");

-            synthesize_lrmd_failure(lrm_state, input->xml,

+            synthesize_lrmd_failure(lrm_state, input->xml, PCMK_LRM_OP_ERROR,

                                     PCMK_OCF_INVALID_PARAM); // hard error

             return;

         }

-- 

1.8.3.1

From ddc3942d7131db9c9874031ca4b3b4a531221573 Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Wed, 5 Jun 2019 13:08:15 -0500

Subject: [PATCH 04/13] Fix: controller: use op status, not rc, for executor

 disconnection

Previously, if an action were requested for an executor (local or remote) that

the controller does not have a connection to, the action's rc would be set to

PCMK_OCF_CONNECTION_DIED and its op status to PCMK_LRM_OP_ERROR.

This was undesirable for a couple reasons: PCMK_OCF_CONNECTION_DIED is a

nonstandard extension to the OCF return codes, which can confuse users

trying to look up the meaning or interpret cluster status output; and it really

is an operation execution status and not an operation result.

This changes the result to PCMK_OCF_UNKNOWN_ERROR with a new op status

PCMK_LRM_OP_NOT_CONNECTED. The new codes are mapped to the old ones for older

DCs that don't understand them.

---

 cts/CTStests.py                         |  2 +-

 daemons/controld/controld_execd.c       | 21 +++++++++++++++++----

 daemons/controld/controld_execd_state.c |  6 ++++--

 include/crm/services.h                  |  4 +++-

 lib/common/operations.c                 |  1 +

 lib/pengine/unpack.c                    |  3 ++-

 6 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/cts/CTStests.py b/cts/CTStests.py

index 32945cb..be7fd7f 100644

--- a/cts/CTStests.py

+++ b/cts/CTStests.py

@@ -3068,7 +3068,7 @@ class RemoteStonithd(RemoteDriver):

             r"schedulerd.*:\s+Recover remote-.*\s*\(.*\)",

             r"Calculated [Tt]ransition .*pe-error",

             r"error.*: Resource .*ocf::.* is active on 2 nodes attempting recovery",

-            r"error: Result of monitor operation for .* on remote-.*: Error",

+            r"error: Result of monitor operation for .* on remote-.*: No executor connection",

         ]

         ignore_pats.extend(RemoteDriver.errorstoignore(self))

diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c

index fed9419..ac215b6 100644

--- a/daemons/controld/controld_execd.c

+++ b/daemons/controld/controld_execd.c

@@ -1758,8 +1758,8 @@ do_lrm_invoke(long long action,

     if ((lrm_state == NULL) && is_remote_node) {

         crm_err("Failing action because local node has never had connection to remote node %s",

                 target_node);

-        synthesize_lrmd_failure(NULL, input->xml, PCMK_LRM_OP_ERROR,

-                                PCMK_OCF_CONNECTION_DIED);

+        synthesize_lrmd_failure(NULL, input->xml, PCMK_LRM_OP_NOT_CONNECTED,

+                                PCMK_OCF_UNKNOWN_ERROR);

         return;

     }

     CRM_ASSERT(lrm_state != NULL);

@@ -1815,8 +1815,9 @@ do_lrm_invoke(long long action,

         rc = get_lrm_resource(lrm_state, xml_rsc, create_rsc, &rsc);

         if (rc == -ENOTCONN) {

-            synthesize_lrmd_failure(lrm_state, input->xml, PCMK_LRM_OP_ERROR,

-                                    PCMK_OCF_CONNECTION_DIED);

+            synthesize_lrmd_failure(lrm_state, input->xml,

+                                    PCMK_LRM_OP_NOT_CONNECTED,

+                                    PCMK_OCF_UNKNOWN_ERROR);

             return;

         } else if ((rc < 0) && !create_rsc) {

@@ -2532,6 +2533,18 @@ process_lrm_event(lrm_state_t *lrm_state, lrmd_event_data_t *op,

     CRM_CHECK(op != NULL, return);

     CRM_CHECK(op->rsc_id != NULL, return);

+    // Remap new status codes for older DCs

+    if (compare_version(fsa_our_dc_version, "3.2.0") < 0) {

+        switch (op->op_status) {

+            case PCMK_LRM_OP_NOT_CONNECTED:

+                op->op_status = PCMK_LRM_OP_ERROR;

+                op->rc = PCMK_OCF_CONNECTION_DIED;

+                break;

+            default:

+                break;

+        }

+    }

+

     op_id = make_stop_id(op->rsc_id, op->call_id);

     op_key = generate_op_key(op->rsc_id, op->op_type, op->interval_ms);

diff --git a/daemons/controld/controld_execd_state.c b/daemons/controld/controld_execd_state.c

index 4e9f096..63e6b33 100644

--- a/daemons/controld/controld_execd_state.c

+++ b/daemons/controld/controld_execd_state.c

@@ -1,6 +1,8 @@

 /*

  * Copyright 2012-2019 the Pacemaker project contributors

  *

+ * The version control history for this file may have further details.

+ *

  * This source code is licensed under the GNU General Public License version 2

  * or later (GPLv2+) WITHOUT ANY WARRANTY.

  */

@@ -76,8 +78,8 @@ fail_pending_op(gpointer key, gpointer value, gpointer user_data)

     event.user_data = op->user_data;

     event.timeout = 0;

     event.interval_ms = op->interval_ms;

-    event.rc = PCMK_OCF_CONNECTION_DIED;

-    event.op_status = PCMK_LRM_OP_ERROR;

+    event.rc = PCMK_OCF_UNKNOWN_ERROR;

+    event.op_status = PCMK_LRM_OP_NOT_CONNECTED;

     event.t_run = op->start_time;

     event.t_rcchange = op->start_time;

diff --git a/include/crm/services.h b/include/crm/services.h

index 4bdd21a..ca9470b 100644

--- a/include/crm/services.h

+++ b/include/crm/services.h

@@ -100,7 +100,7 @@ enum ocf_exitcode {

     /* 150-199	reserved for application use */

-    PCMK_OCF_CONNECTION_DIED = 189, /* Operation failure implied by disconnection of the LRM API to a local or remote node */

+    PCMK_OCF_CONNECTION_DIED = 189, // Deprecated (see PCMK_LRM_OP_NOT_CONNECTED)

     PCMK_OCF_DEGRADED        = 190, /* Active resource that is no longer 100% functional */

     PCMK_OCF_DEGRADED_MASTER = 191, /* Promoted resource that is no longer 100% functional */

@@ -126,6 +126,7 @@ enum op_status {

     PCMK_LRM_OP_ERROR_HARD,

     PCMK_LRM_OP_ERROR_FATAL,

     PCMK_LRM_OP_NOT_INSTALLED,

+    PCMK_LRM_OP_NOT_CONNECTED,

 };

 enum nagios_exitcode {

@@ -337,6 +338,7 @@ gboolean services_alert_async(svc_action_t *action,

                 case PCMK_LRM_OP_NOTSUPPORTED:return "NOT SUPPORTED";

                 case PCMK_LRM_OP_ERROR:return "Error";

                 case PCMK_LRM_OP_NOT_INSTALLED:return "Not installed";

+                case PCMK_LRM_OP_NOT_CONNECTED:return "No executor connection";

                 default:return "UNKNOWN!";

         }

     }

diff --git a/lib/common/operations.c b/lib/common/operations.c

index 2144cc6..c6b16cb 100644

--- a/lib/common/operations.c

+++ b/lib/common/operations.c

@@ -395,6 +395,7 @@ did_rsc_op_fail(lrmd_event_data_t * op, int target_rc)

         case PCMK_LRM_OP_NOTSUPPORTED:

         case PCMK_LRM_OP_TIMEOUT:

         case PCMK_LRM_OP_ERROR:

+        case PCMK_LRM_OP_NOT_CONNECTED:

             return TRUE;

             break;

diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c

index 0e8177b..671f0c4 100644

--- a/lib/pengine/unpack.c

+++ b/lib/pengine/unpack.c

@@ -3163,7 +3163,7 @@ unpack_rsc_op(resource_t * rsc, node_t * node, xmlNode * xml_op, xmlNode ** last

     crm_element_value_ms(xml_op, XML_LRM_ATTR_INTERVAL_MS, &interval_ms);

     CRM_CHECK(task != NULL, return FALSE);

-    CRM_CHECK(status <= PCMK_LRM_OP_NOT_INSTALLED, return FALSE);

+    CRM_CHECK(status <= PCMK_LRM_OP_NOT_CONNECTED, return FALSE);

     CRM_CHECK(status >= PCMK_LRM_OP_PENDING, return FALSE);

     if (safe_str_eq(task, CRMD_ACTION_NOTIFY) ||

@@ -3304,6 +3304,7 @@ unpack_rsc_op(resource_t * rsc, node_t * node, xmlNode * xml_op, xmlNode ** last

         case PCMK_LRM_OP_ERROR_FATAL:

         case PCMK_LRM_OP_TIMEOUT:

         case PCMK_LRM_OP_NOTSUPPORTED:

+        case PCMK_LRM_OP_NOT_CONNECTED:

             failure_strategy = get_action_on_fail(rsc, task_key, task, data_set);

             if ((failure_strategy == action_fail_ignore)

-- 

1.8.3.1

From fc135cb441fb7c66a44fbffe74dcae26c112be3f Mon Sep 17 00:00:00 2001

From: Ken Gaillot <kgaillot@redhat.com>

Date: Wed, 5 Jun 2019 13:43:08 -0500

Subject: [PATCH 05/13] Fix: controller: use op status, not rc, for execution

 in invalid state

Previously, if an action were requested while the controller cannot execute actions

(i.e. shutdown), the action's rc would be set to CRM_DIRECT_NACK_RC and its op

status to PCMK_LRM_OP_ERROR.

This was undesirable for a couple reasons: rc should only be OCF return codes,

and it really is an operation execution status and not an operation result.

This changes the result to PCMK_OCF_UNKNOWN_ERROR with a new op status

PCMK_LRM_OP_INVALID. The new codes are mapped to the old ones for older

DCs that don't understand them.

---

 daemons/controld/controld_execd.c     |  8 ++++++--

 daemons/controld/controld_fsa.h       |  6 +-----

 daemons/controld/controld_te_events.c | 13 ++++++-------

 include/crm/services.h                |  2 ++

 lib/common/operations.c               |  1 +

 lib/pengine/unpack.c                  |  3 ++-

 6 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/daemons/controld/controld_execd.c b/daemons/controld/controld_execd.c

index ac215b6..a20f96a 100644

--- a/daemons/controld/controld_execd.c

+++ b/daemons/controld/controld_execd.c

@@ -2254,8 +2254,8 @@ do_lrm_rsc_op(lrm_state_t * lrm_state, lrmd_rsc_info_t * rsc, const char *operat

                    operation, rsc->id, fsa_state2string(fsa_state),

                    is_set(fsa_input_register, R_SHUTDOWN)?"true":"false");

-        op->rc = CRM_DIRECT_NACK_RC;

-        op->op_status = PCMK_LRM_OP_ERROR;

+        op->rc = PCMK_OCF_UNKNOWN_ERROR;

+        op->op_status = PCMK_LRM_OP_INVALID;

         send_direct_ack(NULL, NULL, rsc, op, rsc->id);

         lrmd_free_event(op);

         free(op_id);

@@ -2540,6 +2540,10 @@ process_lrm_event(lrm_state_t *lrm_state, lrmd_event_data_t *op,

                 op->op_status = PCMK_LRM_OP_ERROR;

                 op->rc = PCMK_OCF_CONNECTION_DIED;

                 break;

+            case PCMK_LRM_OP_INVALID:

+                op->op_status = PCMK_LRM_OP_ERROR;

+                op->rc = CRM_DIRECT_NACK_RC;

+                break;

             default:

                 break;

         }

diff --git a/daemons/controld/controld_fsa.h b/daemons/controld/controld_fsa.h

index 397a9cd..7527ed9 100644

--- a/daemons/controld/controld_fsa.h

+++ b/daemons/controld/controld_fsa.h

@@ -426,11 +426,7 @@ enum crmd_fsa_input {

 #  define R_IN_RECOVERY     0x80000000ULL

-/*

- * Magic RC used within the controller to indicate direct nacks

- * (operation is invalid in current state)

- */

-#define CRM_DIRECT_NACK_RC (99)

+#define CRM_DIRECT_NACK_RC (99) // Deprecated (see PCMK_LRM_OP_INVALID)

 enum crmd_fsa_cause {

     C_UNKNOWN = 0,

diff --git a/daemons/controld/controld_te_events.c b/daemons/controld/controld_te_events.c

index b7b48a4..d297241 100644

--- a/daemons/controld/controld_te_events.c

+++ b/daemons/controld/controld_te_events.c

@@ -123,10 +123,8 @@ update_failcount(xmlNode * event, const char *event_node_uuid, int rc,

     const char *on_uname = crm_peer_uname(event_node_uuid);

     const char *origin = crm_element_value(event, XML_ATTR_ORIGIN);

-    /* Nothing needs to be done for success, lrm status refresh,

-     * or direct nack (internal code for "busy, try again")

-     */

-    if ((rc == CRM_DIRECT_NACK_RC) || (rc == target_rc)) {

+    // Nothing needs to be done for success or status refresh

+    if (rc == target_rc) {

         return FALSE;

     } else if (safe_str_eq(origin, "build_active_RAs")) {

         crm_debug("No update for %s (rc=%d) on %s: Old failure from lrm status refresh",

@@ -225,7 +223,7 @@ status_from_rc(crm_action_t * action, int orig_status, int rc, int target_rc)

         return PCMK_LRM_OP_DONE;

     }

-    if (rc != CRM_DIRECT_NACK_RC) {

+    if (orig_status != PCMK_LRM_OP_INVALID) {

         const char *task = crm_element_value(action->xml, XML_LRM_ATTR_TASK_KEY);

         const char *uname = crm_element_value(action->xml, XML_LRM_ATTR_TARGET);

@@ -541,8 +539,9 @@ process_graph_event(xmlNode *event, const char *event_node)

     if (action && (rc == target_rc)) {

         crm_trace("Processed update to %s: %s", id, magic);

     } else {

-        if (update_failcount(event, event_node, rc, target_rc,

-                             (transition_num == -1), ignore_failures)) {

+        if ((status != PCMK_LRM_OP_INVALID)

+            && update_failcount(event, event_node, rc, target_rc,

+                               (transition_num == -1), ignore_failures)) {

             desc = "failed";

         }

         crm_info("Detected action (%d.%d) %s.%d=%s: %s", transition_num,

diff --git a/include/crm/services.h b/include/crm/services.h

index ca9470b..0771241 100644

--- a/include/crm/services.h

+++ b/include/crm/services.h

@@ -127,6 +127,7 @@ enum op_status {

     PCMK_LRM_OP_ERROR_FATAL,

     PCMK_LRM_OP_NOT_INSTALLED,

     PCMK_LRM_OP_NOT_CONNECTED,

+    PCMK_LRM_OP_INVALID,

 };

 enum nagios_exitcode {

@@ -339,6 +340,7 @@ gboolean services_alert_async(svc_action_t *action,

                 case PCMK_LRM_OP_ERROR:return "Error";

                 case PCMK_LRM_OP_NOT_INSTALLED:return "Not installed";

                 case PCMK_LRM_OP_NOT_CONNECTED:return "No executor connection";

+                case PCMK_LRM_OP_INVALID:return "Cannot execute now";

                 default:return "UNKNOWN!";

         }

     }

diff --git a/lib/common/operations.c b/lib/common/operations.c

index c6b16cb..480bddc 100644

--- a/lib/common/operations.c

+++ b/lib/common/operations.c

@@ -396,6 +396,7 @@ did_rsc_op_fail(lrmd_event_data_t * op, int target_rc)

         case PCMK_LRM_OP_TIMEOUT:

         case PCMK_LRM_OP_ERROR:

         case PCMK_LRM_OP_NOT_CONNECTED:

+        case PCMK_LRM_OP_INVALID:

             return TRUE;

             break;

diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c

index 671f0c4..fb1ab60 100644

--- a/lib/pengine/unpack.c

+++ b/lib/pengine/unpack.c

@@ -3163,7 +3163,7 @@ unpack_rsc_op(resource_t * rsc, node_t * node, xmlNode * xml_op, xmlNode ** last

     crm_element_value_ms(xml_op, XML_LRM_ATTR_INTERVAL_MS, &interval_ms);