Feature: any delays from pcmk_delay_base/max are added to priority-fencing-delay

cts/cts-fencing.in

@@ -1126,22 +1126,23 @@ class Tests(object):
             test.add_stonith_neg_log_pattern("does not advertise support for 'reboot', performing 'off'")
             test.add_stonith_log_pattern("with device 'true1' returned: 0 (OK)")
 
-        # make sure enforced fencing delay is applied only for the first device in the first level
+        # make sure requested fencing delay is applied only for the first device in the first level
+        # make sure static delay from pcmk_delay_base is added
         for test_type in test_types:
             if test_type["use_cpg"] == 0:
                 continue
 
             test = self.new_test("%s_topology_delay" % test_type["prefix"],
-                                 "Verify enforced fencing delay is applied only for the first device in the first level.",
+                                 "Verify requested fencing delay is applied only for the first device in the first level and pcmk_delay_base is added.",
                                  test_type["use_cpg"])
             test.add_cmd("stonith_admin",
-                         "--output-as=xml -R true1 -a fence_dummy -o \"mode=pass\" -o \"pcmk_host_list=node1 node2 node3\"")
+                         "--output-as=xml -R true1 -a fence_dummy -o \"mode=pass\" -o \"pcmk_host_list=node1 node2 node3\" -o \"pcmk_delay_base=1\"")
             test.add_cmd("stonith_admin",
-                         "--output-as=xml -R false1  -a fence_dummy -o \"mode=fail\" -o \"pcmk_host_list=node1 node2 node3\"")
+                         "--output-as=xml -R false1 -a fence_dummy -o \"mode=fail\" -o \"pcmk_host_list=node1 node2 node3\" -o \"pcmk_delay_base=1\"")
             test.add_cmd("stonith_admin",
-                         "--output-as=xml -R true2  -a fence_dummy -o \"mode=pass\" -o \"pcmk_host_list=node1 node2 node3\"")
+                         "--output-as=xml -R true2 -a fence_dummy -o \"mode=pass\" -o \"pcmk_host_list=node1 node2 node3\"")
             test.add_cmd("stonith_admin",
-                         "--output-as=xml -R true3  -a fence_dummy -o \"mode=pass\" -o \"pcmk_host_list=node1 node2 node3\"")
+                         "--output-as=xml -R true3 -a fence_dummy -o \"mode=pass\" -o \"pcmk_host_list=node1 node2 node3\"")
 
             test.add_cmd("stonith_admin", "--output-as=xml -r node3 -i 1 -v true1")
             test.add_cmd("stonith_admin", "--output-as=xml -r node3 -i 1 -v false1")
@@ -1150,8 +1151,8 @@ class Tests(object):
 
             test.add_cmd("stonith_admin", "--output-as=xml -F node3 --delay 1")
 
-            test.add_stonith_log_pattern("Delaying 'off' action targeting node3 on true1 for enforced 1s")
-            test.add_stonith_neg_log_pattern("Delaying 'off' action targeting node3 on false1")
+            test.add_stonith_log_pattern("Delaying 'off' action targeting node3 on true1 for 2s (timeout=120s, requested_delay=1s, base=1s, max=1s)")
+            test.add_stonith_log_pattern("Delaying 'off' action targeting node3 on false1 for 1s (timeout=120s, requested_delay=0s, base=1s, max=1s)")
             test.add_stonith_neg_log_pattern("Delaying 'off' action targeting node3 on true2")
             test.add_stonith_neg_log_pattern("Delaying 'off' action targeting node3 on true3")
 

daemons/controld/controld_fencing.c

@@ -848,7 +848,7 @@ te_fence_node(crm_graph_t *graph, crm_action_t *action)
 
     rc = stonith_api->cmds->fence_with_delay(stonith_api, options, target, type,
                                              (int) (transition_graph->stonith_timeout / 1000),
-                                             0, crm_atoi(priority_delay, "-1"));
+                                             0, crm_atoi(priority_delay, "0"));
 
     transition_key = pcmk__transition_key(transition_graph->id, action->id, 0,
                                           te_uuid),

daemons/fenced/fenced_commands.c

@@ -241,8 +241,7 @@ create_async_command(xmlNode * msg)
     crm_element_value_int(msg, F_STONITH_CALLOPTS, &(cmd->options));
     crm_element_value_int(msg, F_STONITH_TIMEOUT, &(cmd->default_timeout));
     cmd->timeout = cmd->default_timeout;
-    // Default value -1 means no enforced fencing delay
-    cmd->start_delay = -1;
+    // Value -1 means disable any static/random fencing delays
     crm_element_value_int(msg, F_STONITH_DELAY, &(cmd->start_delay));
 
     cmd->origin = crm_element_value_copy(msg, F_ORIG);
@@ -465,7 +464,7 @@ schedule_stonith_command(async_command_t * cmd, stonith_device_t * device)
 {
     int delay_max = 0;
     int delay_base = 0;
-    bool delay_enforced = (cmd->start_delay >= 0);
+    int requested_delay = cmd->start_delay;
 
     CRM_CHECK(cmd != NULL, return);
     CRM_CHECK(device != NULL, return);
@@ -498,35 +497,36 @@ schedule_stonith_command(async_command_t * cmd, stonith_device_t * device)
     device->pending_ops = g_list_append(device->pending_ops, cmd);
     mainloop_set_trigger(device->work);
 
-    // No enforced fencing delay
-    if (delay_enforced == FALSE) {
-        delay_max = get_action_delay_max(device, cmd->action);
-        delay_base = get_action_delay_base(device, cmd->action);
-        if (delay_max == 0) {
-            delay_max = delay_base;
-        }
-        if (delay_max < delay_base) {
-            crm_warn("Base-delay (%ds) is larger than max-delay (%ds) "
-                     "for %s on %s - limiting to max-delay",
-                     delay_base, delay_max, cmd->action, device->id);
-            delay_base = delay_max;
-        }
-        if (delay_max > 0) {
-            // coverity[dont_call] We're not using rand() for security
-            cmd->start_delay =
-                ((delay_max != delay_base)?(rand() % (delay_max - delay_base)):0)
-                + delay_base;
-        }
+    // Value -1 means disable any static/random fencing delays
+    if (requested_delay < 0) {
+        return;
+    }
+
+    delay_max = get_action_delay_max(device, cmd->action);
+    delay_base = get_action_delay_base(device, cmd->action);
+    if (delay_max == 0) {
+        delay_max = delay_base;
+    }
+    if (delay_max < delay_base) {
+        crm_warn("Base-delay (%ds) is larger than max-delay (%ds) "
+                 "for %s on %s - limiting to max-delay",
+                 delay_base, delay_max, cmd->action, device->id);
+        delay_base = delay_max;
+    }
+    if (delay_max > 0) {
+        // coverity[dont_call] We're not using rand() for security
+        cmd->start_delay +=
+            ((delay_max != delay_base)?(rand() % (delay_max - delay_base)):0)
+            + delay_base;
     }
 
     if (cmd->start_delay > 0) {
-        crm_notice("Delaying '%s' action%s%s on %s for %s%ds (timeout=%ds, base=%ds, "
-                   "max=%ds)",
+        crm_notice("Delaying '%s' action%s%s on %s for %ds (timeout=%ds, "
+                   "requested_delay=%ds, base=%ds, max=%ds)",
                    cmd->action,
                    cmd->victim ? " targeting " : "", cmd->victim ? cmd->victim : "",
-                   device->id, delay_enforced ? "enforced " : "",
-                   cmd->start_delay, cmd->timeout,
-                   delay_base, delay_max);
+                   device->id, cmd->start_delay, cmd->timeout,
+                   requested_delay, delay_base, delay_max);
         cmd->delay_id =
             g_timeout_add_seconds(cmd->start_delay, start_delay_helper, cmd);
     }

daemons/fenced/fenced_remote.c

@@ -842,7 +842,7 @@ stonith_topology_next(remote_fencing_op_t * op)
                   op->client_name, op->originator, op->id);
         set_op_device_list(op, tp->levels[op->level]);
 
-        // The enforced delay has been applied for the first fencing level
+        // The requested delay has been applied for the first fencing level
         if (op->level > 1 && op->delay > 0) {
             op->delay = 0;
         }
@@ -1004,9 +1004,7 @@ create_remote_stonith_op(const char *client, xmlNode * request, gboolean peer)
     op = calloc(1, sizeof(remote_fencing_op_t));
 
     crm_element_value_int(request, F_STONITH_TIMEOUT, &(op->base_timeout));
-
-    // Default value -1 means no enforced fencing delay
-    op->delay = -1;
+    // Value -1 means disable any static/random fencing delays
     crm_element_value_int(request, F_STONITH_DELAY, &(op->delay));
 
     if (peer && dev) {
@@ -1458,7 +1456,7 @@ advance_op_topology(remote_fencing_op_t *op, const char *device, xmlNode *msg,
         crm_trace("Next targeting %s on behalf of %s@%s (rc was %d)",
                   op->target, op->originator, op->client_name, rc);
 
-        // The enforced delay has been applied for the first device
+        // The requested delay has been applied for the first device
         if (op->delay > 0) {
             op->delay = 0;
         }
@@ -1517,10 +1515,7 @@ call_remote_stonith(remote_fencing_op_t * op, st_query_result_t * peer)
         crm_xml_add(remote_op, F_STONITH_CLIENTNAME, op->client_name);
         crm_xml_add_int(remote_op, F_STONITH_TIMEOUT, timeout);
         crm_xml_add_int(remote_op, F_STONITH_CALLOPTS, op->call_options);
-
-        if (op->delay >= 0) {
-            crm_xml_add_int(remote_op, F_STONITH_DELAY, op->delay);
-        }
+        crm_xml_add_int(remote_op, F_STONITH_DELAY, op->delay);
 
         if (device) {
             timeout_one = TIMEOUT_MULTIPLY_FACTOR *

daemons/fenced/pacemaker-fenced.h

@@ -113,8 +113,8 @@ typedef struct remote_fencing_op_s {
      * values associated with the devices this fencing operation may call */
     gint total_timeout;
 
-    /*! Enforced fencing delay.
-     * Default value -1 means no enforced fencing delay. */
+    /*! Requested fencing delay.
+     * Value -1 means disable any static/random fencing delays. */
     int delay;
 
     /*! Delegate is the node being asked to perform a fencing action

doc/Pacemaker_Explained/en-US/Ch-Options.txt

@@ -292,21 +292,21 @@ are +stop+ to attempt to immediately stop pacemaker and stay stopped, or
 on failure. The default is likely to be changed to +panic+ in a future release.
 '(since 2.0.3)'
 
-| priority-fencing-delay  | |
+| priority-fencing-delay  | 0 |
 indexterm:[priority-fencing-delay,Cluster Option]
 indexterm:[Cluster,Option,priority-fencing-delay]
-Enforce specified delay for the fencings that are targeting the lost
+Apply specified delay for the fencings that are targeting the lost
 nodes with the highest total resource priority in case we don't
 have the majority of the nodes in our cluster partition, so that
 the more significant nodes potentially win any fencing match,
 which is especially meaningful under split-brain of 2-node
 cluster. A promoted resource instance takes the base priority + 1
-on calculation if the base priority is not 0. If all the nodes
-have equal priority, then any pcmk_delay_base/max configured for
-the corresponding fencing resources will be applied. Otherwise as
-long as it's set, even if to 0, it takes precedence over any
-configured pcmk_delay_base/max. By default, priority fencing
-delay is disabled. '(since 2.0.4)'
+on calculation if the base priority is not 0. Any static/random
+delays that are introduced by `pcmk_delay_base/max` configured
+for the corresponding fencing resources will be added to this
+delay. This delay should be significantly greater than, safely
+twice, the maximum `pcmk_delay_base/max`. By default, priority
+fencing delay is disabled. '(since 2.0.4)'
 
 | cluster-delay | 60s |
 indexterm:[cluster-delay,Cluster Option]

include/crm/pengine/pe_types.h

@@ -176,7 +176,7 @@ struct pe_working_set_s {
     time_t recheck_by;  // Hint to controller to re-run scheduler by this time
     int ninstances;     // Total number of resource instances
     guint shutdown_lock;// How long (seconds) to lock resources to shutdown node
-    int priority_fencing_delay; // Enforced priority fencing delay
+    int priority_fencing_delay; // Priority fencing delay
 };
 
 enum pe_check_parameters {

include/crm/stonith-ng.h

@@ -393,7 +393,7 @@ typedef struct stonith_api_operations_s
                     char **error_output);
 
     /*!
-     * \brief Issue a fencing action against a node with enforced fencing delay.
+     * \brief Issue a fencing action against a node with requested fencing delay.
      *
      * \note Possible actions are, 'on', 'off', and 'reboot'.
      *
@@ -403,7 +403,8 @@ typedef struct stonith_api_operations_s
      * \param action, The fencing action to take
      * \param timeout, The default per device timeout to use with each device
      *                 capable of fencing the target.
-     * \param delay, Any enforced fencing delay. -1 to disable
+     * \param delay, Apply a fencing delay. Value -1 means disable also any
+     *               static/random fencing delays from pcmk_delay_base/max
      *
      * \retval 0 success
      * \retval negative error code on failure.

include/pcmki/pcmki_fence.h

@@ -26,7 +26,8 @@
  * \param[in] tolerance If a successful action for \p target happened within
  *                      this many ms, return 0 without performing the action
  *                      again.
- * \param[in] delay     Enforce a fencing delay. Value -1 means disabled.
+ * \param[in] delay     Apply a fencing delay. Value -1 means disable also any
+ *                      static/random fencing delays from pcmk_delay_base/max
  *
  * \return Standard Pacemaker return code
  */

lib/fencing/st_client.c

@@ -1080,10 +1080,7 @@ stonith_api_fence_with_delay(stonith_t * stonith, int call_options, const char *
     crm_xml_add(data, F_STONITH_ACTION, action);
     crm_xml_add_int(data, F_STONITH_TIMEOUT, timeout);
     crm_xml_add_int(data, F_STONITH_TOLERANCE, tolerance);
-
-    if (delay >= 0) {
-        crm_xml_add_int(data, F_STONITH_DELAY, delay);
-    }
+    crm_xml_add_int(data, F_STONITH_DELAY, delay);
 
     rc = stonith_send_command(stonith, STONITH_OP_FENCE, data, NULL, call_options, timeout);
     free_xml(data);
@@ -1096,7 +1093,7 @@ stonith_api_fence(stonith_t * stonith, int call_options, const char *node, const
                   int timeout, int tolerance)
 {
     return stonith_api_fence_with_delay(stonith, call_options, node, action,
-                                        timeout, tolerance, -1);
+                                        timeout, tolerance, 0);
 }
 
 static int

lib/pengine/common.c

@@ -160,20 +160,20 @@ static pcmk__cluster_option_t pe_opts[] = {
     },
     {
         XML_CONFIG_ATTR_PRIORITY_FENCING_DELAY, NULL, "time", NULL,
-        NULL, pcmk__valid_interval_spec,
-        "Enforced fencing delay targeting the lost nodes with the highest total resource priority",
-        "Enforce specified delay for the fencings that are targeting the lost "
+        "0", pcmk__valid_interval_spec,
+        "Apply fencing delay targeting the lost nodes with the highest total resource priority",
+        "Apply specified delay for the fencings that are targeting the lost "
             "nodes with the highest total resource priority in case we don't "
             "have the majority of the nodes in our cluster partition, so that "
             "the more significant nodes potentially win any fencing match, "
             "which is especially meaningful under split-brain of 2-node "
             "cluster. A promoted resource instance takes the base priority + 1 "
-            "on calculation if the base priority is not 0. If all the nodes "
-            "have equal priority, then any pcmk_delay_base/max configured for "
-            "the corresponding fencing resources will be applied. Otherwise as "
-            "long as it's set, even if to 0, it takes precedence over any "
-            "configured pcmk_delay_base/max. By default, priority fencing "
-            "delay is disabled."
+            "on calculation if the base priority is not 0. Any static/random "
+            "delays that are introduced by `pcmk_delay_base/max` configured "
+            "for the corresponding fencing resources will be added to this "
+            "delay. This delay should be significantly greater than, safely "
+            "twice, the maximum `pcmk_delay_base/max`. By default, priority "
+            "fencing delay is disabled."
     },
 
     {

lib/pengine/unpack.c

@@ -233,8 +233,6 @@ unpack_config(xmlNode * config, pe_working_set_t * data_set)
     crm_debug("Concurrent fencing is %s",
               is_set(data_set->flags, pe_flag_concurrent_fencing) ? "enabled" : "disabled");
 
-    // Default value -1 means `priority-fencing-delay` is disabled
-    data_set->priority_fencing_delay = -1;
     value = pe_pref(data_set->config_hash,
                     XML_CONFIG_ATTR_PRIORITY_FENCING_DELAY);
     if (value) {

lib/pengine/utils.c

@@ -2337,17 +2337,17 @@ node_priority_fencing_delay(pe_node_t * node, pe_working_set_t * data_set)
     GListPtr gIter = NULL;
 
     // `priority-fencing-delay` is disabled
-    if (data_set->priority_fencing_delay < 0) {
-        return -1;
+    if (data_set->priority_fencing_delay <= 0) {
+        return 0;
     }
 
-    /* No need to delay fencing if the fencing target is not a normal cluster
+    /* No need to request a delay if the fencing target is not a normal cluster
      * member, for example if it's a remote node or a guest node. */
     if (node->details->type != node_member) {
         return 0;
     }
 
-    // No need to delay fencing if the fencing target is in our partition
+    // No need to request a delay if the fencing target is in our partition
     if (node->details->online) {
         return 0;
     }
@@ -2384,7 +2384,7 @@ node_priority_fencing_delay(pe_node_t * node, pe_working_set_t * data_set)
     /* All the nodes have equal priority.
      * Any configured corresponding `pcmk_delay_base/max` will be applied. */
     if (lowest_priority == top_priority) {
-        return -1;
+        return 0;
     }
 
     if (node->details->priority < top_priority) {
@@ -2468,7 +2468,7 @@ pe_fence_op(pe_node_t * node, const char *op, bool optional, const char *reason,
         free(op_key);
     }
 
-    if (data_set->priority_fencing_delay >= 0
+    if (data_set->priority_fencing_delay > 0
 
             /* It's a suitable case where `priority-fencing-delay` applies.
              * At least add `priority-fencing-delay` field as an indicator. */

tools/stonith_admin.c

@@ -71,7 +71,7 @@ struct {
     char *unregister_level;
 } options = {
     .timeout = 120,
-    .delay = -1
+    .delay = 0
 };
 
 gboolean add_env_params(const gchar *option_name, const gchar *optarg, gpointer data, GError **error);
@@ -208,8 +208,10 @@ static GOptionEntry addl_entries[] = {
       INDENT "used with most commands).",
       "SECONDS" },
     { "delay", 'y', 0, G_OPTION_ARG_INT, &options.delay,
-      "Enforced fencing delay in seconds (default -1 (disabled);\n"
-      INDENT "with --fence, --reboot, --unfence).",
+      "Apply a fencing delay in seconds. Any static/random delays from\n"
+      INDENT "pcmk_delay_base/max will be added, otherwise all\n"
+      INDENT "disabled with the value -1\n"
+      INDENT "(default 0; with --fence, --reboot, --unfence).",
       "SECONDS" },
     { "as-node-id", 'n', 0, G_OPTION_ARG_NONE, &options.as_nodeid,
       "(Advanced) The supplied node is the corosync node ID\n"