Feature: watchdog-fencing: drop auto-calculation of stonith-watchdog-…

…timeout

The implementation wasn't safe in a way that it didn't prevent
a mismatch of stonith-watchdog-timeout and SBD_WATCHDOG_TIMEOUT
on certain nodes when SBD_WATCHDOG_TIMEOUT wasn't configured
to the same value on all nodes.

agents/stonith/fence_watchdog.in

@@ -14,8 +14,10 @@ import getopt
 
 AGENT_VERSION = "1.0.0"
 SHORT_DESC = "Dummy watchdog fence agent"
-LONG_DESC = """fence_watchdog just provides
-meta-data - actual fencing is done by the pacemaker internal watchdog agent."""
+LONG_DESC = """fence_watchdog just provides meta-data - actual fencing is
+done by the pacemaker internal watchdog agent.
+Use e.g. for limiting watchdog fencing to certain nodes - skipping test for
+SBD-daemon & consistent watchdog-configuration on others."""
 
 ALL_OPT = {
  "version" : {

cts/cli/regression.crm_attribute.exp

@@ -79,8 +79,8 @@ Also known as properties, these are options that affect behavior across the enti
  * Possible values (generated by Pacemaker): boolean (default: )
 
  * stonith-watchdog-timeout: How long before nodes can be assumed to be safely down when watchdog-based self-fencing via SBD is in use
- * If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. If this is set to a negative value, the cluster will use twice the local value of the `SBD_WATCHDOG_TIMEOUT` environment variable if that is positive, or otherwise treat this as 0. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active. When this is set to a negative value, `SBD_WATCHDOG_TIMEOUT` must be set to the same value on all nodes that use SBD, otherwise data corruption or loss could occur.
- * Possible values: timeout (default: )
+ * If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active.
+ * Possible values: duration (default: )
 
  * stonith-max-attempts: How many times fencing can fail before it will no longer be immediately re-attempted on a target
  * Possible values: score (default: )
@@ -297,9 +297,9 @@ Also known as properties, these are options that affect behavior across the enti
  <content type="boolean" default=""/>
  </parameter>
  <parameter name="stonith-watchdog-timeout" advanced="0" generated="0">
- <longdesc lang="en">If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. If this is set to a negative value, the cluster will use twice the local value of the `SBD_WATCHDOG_TIMEOUT` environment variable if that is positive, or otherwise treat this as 0. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active. When this is set to a negative value, `SBD_WATCHDOG_TIMEOUT` must be set to the same value on all nodes that use SBD, otherwise data corruption or loss could occur.</longdesc>
+ <longdesc lang="en">If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active.</longdesc>
  <shortdesc lang="en">How long before nodes can be assumed to be safely down when watchdog-based self-fencing via SBD is in use</shortdesc>
- <content type="timeout" default=""/>
+ <content type="duration" default=""/>
  </parameter>
  <parameter name="stonith-max-attempts" advanced="0" generated="0">
  <longdesc lang="en">How many times fencing can fail before it will no longer be immediately re-attempted on a target</longdesc>
@@ -507,8 +507,8 @@ Also known as properties, these are options that affect behavior across the enti
  * Possible values (generated by Pacemaker): boolean (default: )
 
  * stonith-watchdog-timeout: How long before nodes can be assumed to be safely down when watchdog-based self-fencing via SBD is in use
- * If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. If this is set to a negative value, the cluster will use twice the local value of the `SBD_WATCHDOG_TIMEOUT` environment variable if that is positive, or otherwise treat this as 0. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active. When this is set to a negative value, `SBD_WATCHDOG_TIMEOUT` must be set to the same value on all nodes that use SBD, otherwise data corruption or loss could occur.
- * Possible values: timeout (default: )
+ * If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active.
+ * Possible values: duration (default: )
 
  * stonith-max-attempts: How many times fencing can fail before it will no longer be immediately re-attempted on a target
  * Possible values: score (default: )
@@ -760,9 +760,9 @@ Also known as properties, these are options that affect behavior across the enti
  <content type="boolean" default=""/>
  </parameter>
  <parameter name="stonith-watchdog-timeout" advanced="0" generated="0">
- <longdesc lang="en">If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. If this is set to a negative value, the cluster will use twice the local value of the `SBD_WATCHDOG_TIMEOUT` environment variable if that is positive, or otherwise treat this as 0. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active. When this is set to a negative value, `SBD_WATCHDOG_TIMEOUT` must be set to the same value on all nodes that use SBD, otherwise data corruption or loss could occur.</longdesc>
+ <longdesc lang="en">If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active.</longdesc>
  <shortdesc lang="en">How long before nodes can be assumed to be safely down when watchdog-based self-fencing via SBD is in use</shortdesc>
- <content type="timeout" default=""/>
+ <content type="duration" default=""/>
  </parameter>
  <parameter name="stonith-max-attempts" advanced="0" generated="0">
  <longdesc lang="en">How many times fencing can fail before it will no longer be immediately re-attempted on a target</longdesc>

cts/cli/regression.daemons.exp

@@ -148,7 +148,7 @@
  </parameter>
  <parameter name="stonith-watchdog-timeout">
  <longdesc lang="en">
- If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. If this is set to a negative value, the cluster will use twice the local value of the `SBD_WATCHDOG_TIMEOUT` environment variable if that is positive, or otherwise treat this as 0. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active. When this is set to a negative value, `SBD_WATCHDOG_TIMEOUT` must be set to the same value on all nodes that use SBD, otherwise data corruption or loss could occur.
+ If this is set to a positive value, lost nodes are assumed to achieve self-fencing using watchdog-based SBD within this much time. This does not require a fencing resource to be explicitly configured, though a fence_watchdog resource can be configured, to limit use to specific nodes. If this is set to 0 (the default), the cluster will never assume watchdog-based self-fencing. WARNING: When used, this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, and Pacemaker will refuse to start on any of those nodes where this is not true for the local value or SBD is not active.
  </longdesc>
  <shortdesc lang="en">
  How long before nodes can be assumed to be safely down when watchdog-based self-fencing via SBD is in use

daemons/fenced/fenced_cib.c

@@ -169,7 +169,7 @@ fencing_topology_init(void)
 static void
 update_stonith_watchdog_timeout_ms(xmlNode *cib)
 {
- long long timeout_ms = 0;
+ guint timeout_interval = 0;
  xmlNode *stonith_watchdog_xml = NULL;
  const char *value = NULL;
 
@@ -178,15 +178,9 @@ update_stonith_watchdog_timeout_ms(xmlNode *cib)
  if (stonith_watchdog_xml) {
  value = crm_element_value(stonith_watchdog_xml, PCMK_XA_VALUE);
  }
- if (value) {
- timeout_ms = crm_get_msec(value);
- }
-
- if (timeout_ms < 0) {
- timeout_ms = pcmk__auto_stonith_watchdog_timeout();
- }
+ pcmk_parse_interval_spec(value, &timeout_interval);
 
- stonith_watchdog_timeout_ms = timeout_ms;
+ stonith_watchdog_timeout_ms = timeout_interval;
 }
 
 /*!

doc/sphinx/Pacemaker_Explained/cluster-options.rst

@@ -572,7 +572,7 @@ values, by running the ``man pacemaker-schedulerd`` and
  pair: cluster option; stonith-watchdog-timeout
 
  stonith-watchdog-timeout
- - :ref:`timeout <timeout>`
+ - :ref:`duration <duration>`
  - 0
  - If nonzero, and the cluster detects ``have-watchdog`` as ``true``, then
  watchdog-based self-fencing will be performed via SBD when fencing is
@@ -588,16 +588,10 @@ values, by running the ``man pacemaker-schedulerd`` and
  If this is set to 0 (the default), the cluster will never assume
  watchdog-based self-fencing.
 
- If this is set to a negative value, the cluster will use twice the local
- value of the ``SBD_WATCHDOG_TIMEOUT`` environment variable if that is
- positive, or otherwise treat this as 0.
-
  **Warning:** When used, this timeout must be larger than
  ``SBD_WATCHDOG_TIMEOUT`` on all nodes that use watchdog-based SBD, and
  Pacemaker will refuse to start on any of those nodes where this is not
- true for the local value or SBD is not active. When this is set to a
- negative value, ``SBD_WATCHDOG_TIMEOUT`` must be set to the same value
- on all nodes that use SBD, otherwise data corruption or loss could occur.
+ true for the local value or SBD is not active.
 
  * - .. _concurrent-fencing:
 

doc/sphinx/Pacemaker_Explained/local-options.rst

@@ -704,10 +704,11 @@ whose location varies by OS (most commonly ``/etc/sysconfig/pacemaker`` or
  SBD_WATCHDOG_TIMEOUT
  - :ref:`duration <duration>`
  -
- - If the ``stonith-watchdog-timeout`` cluster property is set to a negative
- or invalid value, use double this value as the default if positive, or
- use 0 as the default otherwise. This value must be greater than the value
- of ``stonith-watchdog-timeout`` if both are set.
+ - If the ``stonith-watchdog-timeout`` cluster property is set to a positive
+ value, it is checked to be at least as large as ``SBD_WATCHDOG_TIMEOUT``.
+ Otherwise the pacemaker node won't start or is stopped. To be on the safe
+ side it is recommended to set ``stonith-watchdog-timeout`` to double the
+ value of ``SBD_WATCHDOG_TIMEOUT`` though.
 
  * - .. _valgrind_opts:
 

include/crm/common/options_internal.h

@@ -120,9 +120,8 @@ bool pcmk__valid_percentage(const char *value);
 bool pcmk__valid_placement_strategy(const char *value);
 
 // from watchdog.c
-long pcmk__get_sbd_watchdog_timeout(void);
+guint pcmk__get_sbd_watchdog_timeout(void);
 bool pcmk__get_sbd_sync_resource_startup(void);
-long pcmk__auto_stonith_watchdog_timeout(void);
 bool pcmk__valid_stonith_watchdog_timeout(const char *value);
 
 // Constants for environment variable names

lib/common/options.c

@@ -272,18 +272,8 @@ static const pcmk__cluster_option_t cluster_options[] = {
  "SBD without requiring a fencing resource explicitly configured."),
  },
  {
- /* @COMPAT Currently, unparsable values default to -1 (auto-calculate),
- * while missing values default to 0 (disable). All values are accepted
- * (unless the controller finds that the value conflicts with the
- * SBD_WATCHDOG_TIMEOUT).
- *
- * At a compatibility break: properly validate as a timeout, let
- * either negative values or a particular string like "auto" mean auto-
- * calculate, and use 0 as the single default for when the option either
- * is unset or fails to validate.
- */
- PCMK_OPT_STONITH_WATCHDOG_TIMEOUT, NULL, PCMK_VALUE_TIMEOUT, NULL,
- "0", NULL,
+ PCMK_OPT_STONITH_WATCHDOG_TIMEOUT, NULL, PCMK_VALUE_DURATION, NULL,
+ "0", pcmk__valid_interval_spec,
  pcmk__opt_controld,
  N_("How long before nodes can be assumed to be safely down when "
  "watchdog-based self-fencing via SBD is in use"),
@@ -292,16 +282,11 @@ static const pcmk__cluster_option_t cluster_options[] = {
  "time. This does not require a fencing resource to be explicitly "
  "configured, though a fence_watchdog resource can be configured, to "
  "limit use to specific nodes. If this is set to 0 (the default), "
- "the cluster will never assume watchdog-based self-fencing. If this "
- "is set to a negative value, the cluster will use twice the local "
- "value of the `SBD_WATCHDOG_TIMEOUT` environment variable if that "
- "is positive, or otherwise treat this as 0. WARNING: When used, "
- "this timeout must be larger than `SBD_WATCHDOG_TIMEOUT` on all "
- "nodes that use watchdog-based SBD, and Pacemaker will refuse to "
- "start on any of those nodes where this is not true for the local "
- "value or SBD is not active. When this is set to a negative value, "
- "`SBD_WATCHDOG_TIMEOUT` must be set to the same value on all nodes "
- "that use SBD, otherwise data corruption or loss could occur."),
+ "the cluster will never assume watchdog-based self-fencing. "
+ "WARNING: When used, this timeout must be larger than "
+ "`SBD_WATCHDOG_TIMEOUT` on all nodes that use watchdog-based SBD, "
+ "and Pacemaker will refuse to start on any of those nodes where "
+ "this is not true for the local value or SBD is not active."),
  },
  {
  PCMK_OPT_STONITH_MAX_ATTEMPTS, NULL, PCMK_VALUE_SCORE, NULL,

lib/common/watchdog.c

@@ -227,13 +227,21 @@ pcmk__locate_sbd(void)
  return sbd_pid;
 }
 
-long
+guint
 pcmk__get_sbd_watchdog_timeout(void)
 {
- static long sbd_timeout = -2;
+ static guint sbd_timeout = 0;
+ static bool have_timeout = false;
 
- if (sbd_timeout == -2) {
- sbd_timeout = crm_get_msec(getenv("SBD_WATCHDOG_TIMEOUT"));
+ if (have_timeout == false) {
+ char *env = getenv("SBD_WATCHDOG_TIMEOUT");
+
+ if (((env != NULL) && (env[0] == 'P')) ||
+ (pcmk_parse_interval_spec(env, &sbd_timeout) != pcmk_rc_ok)) {
+ crm_warn("SBD-daemon doesn't allow '%s' for "
+ "SBD_WATCHDOG_TIMEOUT", env?env:"");
+ }
+ have_timeout = true;
  }
  return sbd_timeout;
 }
@@ -261,29 +269,12 @@ pcmk__get_sbd_sync_resource_startup(void)
  return sync_resource_startup != 0;
 }
 
-long
-pcmk__auto_stonith_watchdog_timeout(void)
-{
- long sbd_timeout = pcmk__get_sbd_watchdog_timeout();
-
- return (sbd_timeout <= 0)? 0 : (2 * sbd_timeout);
-}
-
 bool
 pcmk__valid_stonith_watchdog_timeout(const char *value)
 {
- /* @COMPAT At a compatibility break, accept either negative values or a
- * specific string like "auto" (but not both) to mean "auto-calculate the
- * timeout." Reject other values that aren't parsable as timeouts.
- */
- long st_timeout = value? crm_get_msec(value) : 0;
-
- if (st_timeout < 0) {
- st_timeout = pcmk__auto_stonith_watchdog_timeout();
- crm_debug("Using calculated value %ld for "
- PCMK_OPT_STONITH_WATCHDOG_TIMEOUT " (%s)",
- st_timeout, value);
- }
+ guint st_timeout = 0;
+
+ pcmk_parse_interval_spec(value, &st_timeout);
 
  if (st_timeout == 0) {
  crm_debug("Watchdog may be enabled but "
@@ -298,8 +289,14 @@ pcmk__valid_stonith_watchdog_timeout(const char *value)
  return false;
 
  } else {
- long sbd_timeout = pcmk__get_sbd_watchdog_timeout();
+ guint sbd_timeout = pcmk__get_sbd_watchdog_timeout();
 
+ if (sbd_timeout == 0) {
+ crm_emerg("Shutting down: Can't get valid watchdog setting of "
+ "SBD-daemon");
+ crm_exit(CRM_EX_FATAL);
+ return false;
+ }
  if (st_timeout < sbd_timeout) {
  crm_emerg("Shutting down: " PCMK_OPT_STONITH_WATCHDOG_TIMEOUT
  " (%s) too short (must be >%ldms)",