From f2e51898735b5e9990464141fc4aea3dd83f5067 Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Thu, 4 Nov 2021 14:36:41 -0400

Subject: [PATCH 01/21] Refactor: scheduler: Use bool in unpack_rsc_op.

Previously, we were using bool but TRUE/FALSE.  Instead, use the actual

values.

---

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

 1 file changed, 2 insertions(+), 2 deletions(-)

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

index b1e84110a2..ecc7275e15 100644

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

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

@@ -3671,7 +3671,7 @@ unpack_rsc_op(pe_resource_t *rsc, pe_node_t *node, xmlNode *xml_op,

     const char *task = NULL;

     const char *task_key = NULL;

     const char *exit_reason = NULL;

-    bool expired = FALSE;

+    bool expired = false;

     pe_resource_t *parent = rsc;

     enum action_fail_response failure_strategy = action_fail_recover;

@@ -3727,7 +3727,7 @@ unpack_rsc_op(pe_resource_t *rsc, pe_node_t *node, xmlNode *xml_op,

     if ((status != PCMK_EXEC_NOT_INSTALLED)

         && check_operation_expiry(rsc, node, rc, xml_op, data_set)) {

-        expired = TRUE;

+        expired = true;

     }

     if (!strcmp(task, CRMD_ACTION_STATUS)) {

-- 

2.27.0

From 4c961b8e670d336a368c7fd1535c247e40c6b48e Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Thu, 4 Nov 2021 15:07:01 -0400

Subject: [PATCH 02/21] Refactor: scheduler: Add functions for determining if

 an op is a probe.

---

 include/crm/common/util.h                     |  3 +

 lib/common/operations.c                       | 21 +++++++

 lib/common/tests/operations/Makefile.am       |  6 +-

 .../tests/operations/pcmk_is_probe_test.c     | 37 +++++++++++++

 .../tests/operations/pcmk_xe_is_probe_test.c  | 55 +++++++++++++++++++

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

 lib/pengine/utils.c                           |  5 +-

 7 files changed, 127 insertions(+), 12 deletions(-)

 create mode 100644 lib/common/tests/operations/pcmk_is_probe_test.c

 create mode 100644 lib/common/tests/operations/pcmk_xe_is_probe_test.c

diff --git a/include/crm/common/util.h b/include/crm/common/util.h

index 2728b64492..fbea6e560c 100644

--- a/include/crm/common/util.h

+++ b/include/crm/common/util.h

@@ -72,6 +72,9 @@ xmlNode *crm_create_op_xml(xmlNode *parent, const char *prefix,

                            const char *timeout);

 #define CRM_DEFAULT_OP_TIMEOUT_S "20s"

+bool pcmk_is_probe(const char *task, guint interval);

+bool pcmk_xe_is_probe(xmlNode *xml_op);

+

 int compare_version(const char *version1, const char *version2);

 /* coverity[+kill] */

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

index 366c189702..978df79082 100644

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

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

@@ -537,3 +537,24 @@ pcmk__is_fencing_action(const char *action)

 {

     return pcmk__str_any_of(action, "off", "reboot", "poweroff", NULL);

 }

+

+bool

+pcmk_is_probe(const char *task, guint interval)

+{

+    if (task == NULL) {

+        return false;

+    }

+

+    return (interval == 0) && pcmk__str_eq(task, CRMD_ACTION_STATUS, pcmk__str_none);

+}

+

+bool

+pcmk_xe_is_probe(xmlNode *xml_op)

+{

+    const char *task = crm_element_value(xml_op, XML_LRM_ATTR_TASK);

+    const char *interval_ms_s = crm_element_value(xml_op, XML_LRM_ATTR_INTERVAL_MS);

+    int interval_ms;

+

+    pcmk__scan_min_int(interval_ms_s, &interval_ms, 0);

+    return pcmk_is_probe(task, interval_ms);

+}

diff --git a/lib/common/tests/operations/Makefile.am b/lib/common/tests/operations/Makefile.am

index c8814ff0a8..2e3d0b0679 100644

--- a/lib/common/tests/operations/Makefile.am

+++ b/lib/common/tests/operations/Makefile.am

@@ -1,5 +1,5 @@

 #

-# Copyright 2020 the Pacemaker project contributors

+# Copyright 2020-2021 the Pacemaker project contributors

 #

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

 #

@@ -12,6 +12,8 @@ LDADD = $(top_builddir)/lib/common/libcrmcommon.la -lcmocka

 include $(top_srcdir)/mk/tap.mk

 # Add "_test" to the end of all test program names to simplify .gitignore.

-check_PROGRAMS = parse_op_key_test

+check_PROGRAMS = parse_op_key_test \

+				 pcmk_is_probe_test \

+				 pcmk_xe_is_probe_test

 TESTS = $(check_PROGRAMS)

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

new file mode 100644

index 0000000000..9b449f1a70

--- /dev/null

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

@@ -0,0 +1,37 @@

+/*

+ * Copyright 2021 the Pacemaker project contributors

+ *

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

+ *

+ * This source code is licensed under the GNU Lesser General Public License

+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.

+ */

+

+#include <crm_internal.h>

+

+#include <stdarg.h>

+#include <stddef.h>

+#include <stdint.h>

+#include <stdlib.h>

+#include <setjmp.h>

+#include <cmocka.h>

+

+static void

+is_probe_test(void **state)

+{

+    assert_false(pcmk_is_probe(NULL, 0));

+    assert_false(pcmk_is_probe("", 0));

+    assert_false(pcmk_is_probe("blahblah", 0));

+    assert_false(pcmk_is_probe("monitor", 1));

+    assert_true(pcmk_is_probe("monitor", 0));

+}

+

+int main(int argc, char **argv)

+{

+    const struct CMUnitTest tests[] = {

+        cmocka_unit_test(is_probe_test),

+    };

+

+    cmocka_set_message_output(CM_OUTPUT_TAP);

+    return cmocka_run_group_tests(tests, NULL, NULL);

+}

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

new file mode 100644

index 0000000000..0283d1c145

--- /dev/null

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

@@ -0,0 +1,55 @@

+/*

+ * Copyright 2021 the Pacemaker project contributors

+ *

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

+ *

+ * This source code is licensed under the GNU Lesser General Public License

+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.

+ */

+

+#include <crm_internal.h>

+

+#include <stdarg.h>

+#include <stddef.h>

+#include <stdint.h>

+#include <stdlib.h>

+#include <setjmp.h>

+#include <cmocka.h>

+

+static void

+op_is_probe_test(void **state)

+{

+    xmlNode *node = NULL;

+

+    assert_false(pcmk_xe_is_probe(NULL));

+

+    node = string2xml("<lrm_rsc_op/>");

+    assert_false(pcmk_xe_is_probe(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation_key=\"blah\" interval=\"30s\"/>");

+    assert_false(pcmk_xe_is_probe(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"30s\"/>");

+    assert_false(pcmk_xe_is_probe(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"start\" interval=\"0\"/>");

+    assert_false(pcmk_xe_is_probe(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\"/>");

+    assert_true(pcmk_xe_is_probe(node));

+    free_xml(node);

+}

+

+int main(int argc, char **argv)

+{

+    const struct CMUnitTest tests[] = {

+        cmocka_unit_test(op_is_probe_test),

+    };

+

+    cmocka_set_message_output(CM_OUTPUT_TAP);

+    return cmocka_run_group_tests(tests, NULL, NULL);

+}

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

index ecc7275e15..7c0c66e696 100644

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

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

@@ -83,7 +83,6 @@ is_dangling_guest_node(pe_node_t *node)

     return FALSE;

 }

-

 /*!

  * \brief Schedule a fence action for a node

  *

@@ -2984,7 +2983,6 @@ static void

 unpack_rsc_op_failure(pe_resource_t * rsc, pe_node_t * node, int rc, xmlNode * xml_op, xmlNode ** last_failure,

                       enum action_fail_response * on_fail, pe_working_set_t * data_set)

 {

-    guint interval_ms = 0;

     bool is_probe = false;

     pe_action_t *action = NULL;

@@ -2998,10 +2996,7 @@ unpack_rsc_op_failure(pe_resource_t * rsc, pe_node_t * node, int rc, xmlNode * x

     *last_failure = xml_op;

-    crm_element_value_ms(xml_op, XML_LRM_ATTR_INTERVAL_MS, &interval_ms);

-    if ((interval_ms == 0) && !strcmp(task, CRMD_ACTION_STATUS)) {

-        is_probe = true;

-    }

+    is_probe = pcmk_xe_is_probe(xml_op);

     if (exit_reason == NULL) {

         exit_reason = "";

@@ -3163,8 +3158,9 @@ determine_op_status(

     }

     crm_element_value_ms(xml_op, XML_LRM_ATTR_INTERVAL_MS, &interval_ms);

-    if ((interval_ms == 0) && !strcmp(task, CRMD_ACTION_STATUS)) {

-        is_probe = true;

+    is_probe = pcmk_xe_is_probe(xml_op);

+

+    if (is_probe) {

         task = "probe";

     }

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

index c5eda3898e..07753e173a 100644

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

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

@@ -1066,8 +1066,7 @@ unpack_operation(pe_action_t * action, xmlNode * xml_obj, pe_resource_t * contai

 {

     int timeout_ms = 0;

     const char *value = NULL;

-    bool is_probe = pcmk__str_eq(action->task, RSC_STATUS, pcmk__str_casei)

-                    && (interval_ms == 0);

+    bool is_probe = false;

 #if ENABLE_VERSIONED_ATTRS

     pe_rsc_action_details_t *rsc_details = NULL;

 #endif

@@ -1094,6 +1093,8 @@ unpack_operation(pe_action_t * action, xmlNode * xml_obj, pe_resource_t * contai

     CRM_CHECK(action && action->rsc, return);

+    is_probe = pcmk_is_probe(action->task, interval_ms);

+

     // Cluster-wide <op_defaults> <meta_attributes>

     pe__unpack_dataset_nvpairs(data_set->op_defaults, XML_TAG_META_SETS, &rule_data,

                                action->meta, NULL, FALSE, data_set);

-- 

2.27.0

From 09f32df97ab5064a15ba5a1fb3970d5c64ee7b30 Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Fri, 19 Nov 2021 14:47:22 -0500

Subject: [PATCH 03/21] Refactor: scheduler: Move setting interval_ms in

 determine_op_status.

This can now happen in the only place it's being used.

---

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

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

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

index 7c0c66e696..b9986d2462 100644

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

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

@@ -3142,7 +3142,6 @@ static int

 determine_op_status(

     pe_resource_t *rsc, int rc, int target_rc, pe_node_t * node, xmlNode * xml_op, enum action_fail_response * on_fail, pe_working_set_t * data_set) 

 {

-    guint interval_ms = 0;

     bool is_probe = false;

     int result = PCMK_EXEC_DONE;

     const char *key = get_op_key(xml_op);

@@ -3157,7 +3156,6 @@ determine_op_status(

         exit_reason = "";

     }

-    crm_element_value_ms(xml_op, XML_LRM_ATTR_INTERVAL_MS, &interval_ms);

     is_probe = pcmk_xe_is_probe(xml_op);

     if (is_probe) {

@@ -3230,12 +3228,17 @@ determine_op_status(

             result = PCMK_EXEC_ERROR_FATAL;

             break;

-        case PCMK_OCF_UNIMPLEMENT_FEATURE:

+        case PCMK_OCF_UNIMPLEMENT_FEATURE: {

+            guint interval_ms = 0;

+            crm_element_value_ms(xml_op, XML_LRM_ATTR_INTERVAL_MS, &interval_ms);

+

             if (interval_ms > 0) {

                 result = PCMK_EXEC_NOT_SUPPORTED;

                 break;

             }

             // fall through

+        }

+

         case PCMK_OCF_NOT_INSTALLED:

         case PCMK_OCF_INVALID_PARAM:

         case PCMK_OCF_INSUFFICIENT_PRIV:

-- 

2.27.0

From 6c8f47453afd6c100fddc45187faff17e15f7bfe Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Fri, 19 Nov 2021 14:57:57 -0500

Subject: [PATCH 04/21] Refactor: scheduler: Add pcmk_xe_mask_failed_probe.

Given an xmlNodePtr for a resource operation, this function will

determine whether it is a failed probe operation that should not be

displayed in crm_mon (or other places, I suppose) or not.

---

 include/crm/common/util.h                     |   1 +

 lib/common/operations.c                       |  17 ++

 lib/common/tests/operations/Makefile.am       |   3 +-

 .../pcmk_xe_mask_probe_failure_test.c         | 162 ++++++++++++++++++

 4 files changed, 182 insertions(+), 1 deletion(-)

 create mode 100644 lib/common/tests/operations/pcmk_xe_mask_probe_failure_test.c

diff --git a/include/crm/common/util.h b/include/crm/common/util.h

index fbea6e560c..784069ba1b 100644

--- a/include/crm/common/util.h

+++ b/include/crm/common/util.h

@@ -74,6 +74,7 @@ xmlNode *crm_create_op_xml(xmlNode *parent, const char *prefix,

 bool pcmk_is_probe(const char *task, guint interval);

 bool pcmk_xe_is_probe(xmlNode *xml_op);

+bool pcmk_xe_mask_probe_failure(xmlNode *xml_op);

 int compare_version(const char *version1, const char *version2);

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

index 978df79082..54482b8863 100644

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

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

@@ -558,3 +558,20 @@ pcmk_xe_is_probe(xmlNode *xml_op)

     pcmk__scan_min_int(interval_ms_s, &interval_ms, 0);

     return pcmk_is_probe(task, interval_ms);

 }

+

+bool

+pcmk_xe_mask_probe_failure(xmlNode *xml_op)

+{

+    int status = PCMK_EXEC_UNKNOWN;

+    int rc = PCMK_OCF_OK;

+

+    if (!pcmk_xe_is_probe(xml_op)) {

+        return false;

+    }

+

+    crm_element_value_int(xml_op, XML_LRM_ATTR_OPSTATUS, &status);

+    crm_element_value_int(xml_op, XML_LRM_ATTR_RC, &rc);

+

+    return rc == PCMK_OCF_NOT_INSTALLED || rc == PCMK_OCF_INVALID_PARAM ||

+           status == PCMK_EXEC_NOT_INSTALLED;

+}

diff --git a/lib/common/tests/operations/Makefile.am b/lib/common/tests/operations/Makefile.am

index 2e3d0b0679..457c5f7c7a 100644

--- a/lib/common/tests/operations/Makefile.am

+++ b/lib/common/tests/operations/Makefile.am

@@ -14,6 +14,7 @@ include $(top_srcdir)/mk/tap.mk

 # Add "_test" to the end of all test program names to simplify .gitignore.

 check_PROGRAMS = parse_op_key_test \

 				 pcmk_is_probe_test \

-				 pcmk_xe_is_probe_test

+				 pcmk_xe_is_probe_test \

+				 pcmk_xe_mask_probe_failure_test

 TESTS = $(check_PROGRAMS)

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

new file mode 100644

index 0000000000..a13f6d98f4

--- /dev/null

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

@@ -0,0 +1,162 @@

+/*

+ * Copyright 2021 the Pacemaker project contributors

+ *

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

+ *

+ * This source code is licensed under the GNU Lesser General Public License

+ * version 2.1 or later (LGPLv2.1+) WITHOUT ANY WARRANTY.

+ */

+

+#include <crm_internal.h>

+

+#include <stdarg.h>

+#include <stddef.h>

+#include <stdint.h>

+#include <stdlib.h>

+#include <setjmp.h>

+#include <cmocka.h>

+

+static void

+op_is_not_probe_test(void **state) {

+    xmlNode *node = NULL;

+

+    /* Not worth testing this thoroughly since it's just a duplicate of whether

+     * pcmk_op_is_probe works or not.

+     */

+

+    node = string2xml("<lrm_rsc_op operation=\"start\" interval=\"0\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+}

+

+static void

+op_does_not_have_right_values_test(void **state) {

+    xmlNode *node = NULL;

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"0\" op-status=\"\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+}

+

+static void

+check_values_test(void **state) {

+    xmlNode *node = NULL;

+

+    /* PCMK_EXEC_NOT_SUPPORTED */

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"0\" op-status=\"3\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"5\" op-status=\"3\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    /* PCMK_EXEC_DONE */

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"0\" op-status=\"0\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"2\" op-status=\"0\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"5\" op-status=\"0\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"6\" op-status=\"0\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"7\" op-status=\"0\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    /* PCMK_EXEC_NOT_INSTALLED */

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"0\" op-status=\"7\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"5\" op-status=\"7\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    /* PCMK_EXEC_ERROR */

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"0\" op-status=\"4\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"2\" op-status=\"4\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"5\" op-status=\"4\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"6\" op-status=\"4\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"7\" op-status=\"4\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    /* PCMK_EXEC_ERROR_HARD */

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"0\" op-status=\"5\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"2\" op-status=\"5\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"5\" op-status=\"5\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"6\" op-status=\"5\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"7\" op-status=\"5\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    /* PCMK_EXEC_ERROR_FATAL */

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"0\" op-status=\"6\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"2\" op-status=\"6\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"5\" op-status=\"6\"/>");

+    assert_true(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"6\" op-status=\"6\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+

+    node = string2xml("<lrm_rsc_op operation=\"monitor\" interval=\"0\" rc-code=\"7\" op-status=\"6\"/>");

+    assert_false(pcmk_xe_mask_probe_failure(node));

+    free_xml(node);

+}

+

+int main(int argc, char **argv)

+{

+    const struct CMUnitTest tests[] = {

+        cmocka_unit_test(op_is_not_probe_test),

+        cmocka_unit_test(op_does_not_have_right_values_test),

+        cmocka_unit_test(check_values_test),

+    };

+

+    cmocka_set_message_output(CM_OUTPUT_TAP);

+    return cmocka_run_group_tests(tests, NULL, NULL);

+}

-- 

2.27.0

From c9ce1aaf93cd20bb01e80102dda0ffffb07e6472 Mon Sep 17 00:00:00 2001

From: Chris Lumens <clumens@redhat.com>

Date: Wed, 1 Dec 2021 14:26:31 -0500

Subject: [PATCH 05/21] Refactor: scheduler: Combine op status and rc remapping

 into one function.

Well, not quite.  Doing the remapping is complicated enough to where it

makes sense to have them in separate functions.  However, they can both

be called from a single new function that takes the place of the

previous two calls in unpack_rsc_op.

---

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

 1 file changed, 72 insertions(+), 85 deletions(-)

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

index b9986d2462..b659f319fb 100644

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

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

@@ -3121,36 +3121,68 @@ unpack_rsc_op_failure(pe_resource_t * rsc, pe_node_t * node, int rc, xmlNode * x

 /*!

  * \internal

- * \brief Remap operation status based on action result

+ * \brief Remap informational monitor results and operation status

  *

- * Given an action result, determine an appropriate operation status for the

- * purposes of responding to the action (the status provided by the executor is

- * not directly usable since the executor does not know what was expected).

+ * For the monitor results, certain OCF codes are for providing extended information

+ * to the user about services that aren't yet failed but not entirely healthy either.

+ * These must be treated as the "normal" result by Pacemaker.

+ *

+ * For operation status, the action result can be used to determine an appropriate

+ * status for the purposes of responding to the action.  The status provided by the

+ * executor is not directly usable since the executor does not know what was expected.

  *

+ * \param[in]     xml_op     Operation history entry XML from CIB status

  * \param[in,out] rsc        Resource that operation history entry is for

- * \param[in]     rc         Actual return code of operation

- * \param[in]     target_rc  Expected return code of operation

  * \param[in]     node       Node where operation was executed

- * \param[in]     xml_op     Operation history entry XML from CIB status

- * \param[in,out] on_fail    What should be done about the result

  * \param[in]     data_set   Current cluster working set

+ * \param[in,out] on_fail    What should be done about the result

+ * \param[in]     target_rc  Expected return code of operation

+ * \param[in,out] rc         Actual return code of operation

+ * \param[in,out] status     Operation execution status

+ *

+ * \note If the result is remapped and the node is not shutting down or failed,

+ *       the operation will be recorded in the data set's list of failed operations

+ *       to highlight it for the user.

  *

- * \return Operation status based on return code and action info

  * \note This may update the resource's current and next role.

  */

-static int

-determine_op_status(

-    pe_resource_t *rsc, int rc, int target_rc, pe_node_t * node, xmlNode * xml_op, enum action_fail_response * on_fail, pe_working_set_t * data_set) 

-{

+static void

+remap_operation(xmlNode *xml_op, pe_resource_t *rsc, pe_node_t *node,

+                pe_working_set_t *data_set, enum action_fail_response *on_fail,

+                int target_rc, int *rc, int *status) {

     bool is_probe = false;

-    int result = PCMK_EXEC_DONE;

-    const char *key = get_op_key(xml_op);

     const char *task = crm_element_value(xml_op, XML_LRM_ATTR_TASK);

+    const char *key = get_op_key(xml_op);

     const char *exit_reason = crm_element_value(xml_op,

                                                 XML_LRM_ATTR_EXIT_REASON);

+    if (pcmk__str_eq(task, CRMD_ACTION_STATUS, pcmk__str_none)) {

+        int remapped_rc = pcmk__effective_rc(*rc);

+

+        if (*rc != remapped_rc) {

+            crm_trace("Remapping monitor result %d to %d", *rc, remapped_rc);

+            if (!node->details->shutdown || node->details->online) {

+                record_failed_op(xml_op, node, rsc, data_set);

+            }

+

+            *rc = remapped_rc;

+        }

+    }

+

+    /* If the executor reported an operation status of anything but done or

+     * error, consider that final. But for done or error, we know better whether

+     * it should be treated as a failure or not, because we know the expected

+     * result.

+     */

+    if (*status != PCMK_EXEC_DONE && *status != PCMK_EXEC_ERROR) {

+        return;

+    }

+

     CRM_ASSERT(rsc);

-    CRM_CHECK(task != NULL, return PCMK_EXEC_ERROR);

+    CRM_CHECK(task != NULL,

+              *status = PCMK_EXEC_ERROR; return);

+

+    *status = PCMK_EXEC_DONE;

     if (exit_reason == NULL) {

         exit_reason = "";

@@ -3171,23 +3203,23 @@ determine_op_status(

          * those versions or processing of saved CIB files from those versions,

          * so we do not need to care much about this case.

          */

-        result = PCMK_EXEC_ERROR;

+        *status = PCMK_EXEC_ERROR;

         crm_warn("Expected result not found for %s on %s (corrupt or obsolete CIB?)",

                  key, node->details->uname);

-    } else if (target_rc != rc) {

-        result = PCMK_EXEC_ERROR;

+    } else if (target_rc != *rc) {

+        *status = PCMK_EXEC_ERROR;

         pe_rsc_debug(rsc, "%s on %s: expected %d (%s), got %d (%s%s%s)",

                      key, node->details->uname,

                      target_rc, services_ocf_exitcode_str(target_rc),

-                     rc, services_ocf_exitcode_str(rc),

+                     *rc, services_ocf_exitcode_str(*rc),

                      (*exit_reason? ": " : ""), exit_reason);

     }

-    switch (rc) {

+    switch (*rc) {

         case PCMK_OCF_OK:

             if (is_probe && (target_rc == PCMK_OCF_NOT_RUNNING)) {

-                result = PCMK_EXEC_DONE;

+                *status = PCMK_EXEC_DONE;

                 pe_rsc_info(rsc, "Probe found %s active on %s at %s",

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

                             last_change_str(xml_op));