486 lines
19 KiB
Diff
486 lines
19 KiB
Diff
From 5bed788246ac19c866a60ab3773d94fa4ca28c37 Mon Sep 17 00:00:00 2001
|
|
From: Miroslav Lisik <mlisik@redhat.com>
|
|
Date: Thu, 5 Jan 2023 16:21:44 +0100
|
|
Subject: [PATCH 5/5] Fix stonith-watchdog-timeout validation
|
|
|
|
---
|
|
pcs/lib/cluster_property.py | 25 ++++-
|
|
pcs/lib/sbd.py | 15 ++-
|
|
.../lib/commands/test_cluster_property.py | 50 ++++++++--
|
|
pcs_test/tier0/lib/test_cluster_property.py | 98 ++++++++++++++-----
|
|
pcs_test/tier1/test_cluster_property.py | 14 ++-
|
|
5 files changed, 157 insertions(+), 45 deletions(-)
|
|
|
|
diff --git a/pcs/lib/cluster_property.py b/pcs/lib/cluster_property.py
|
|
index 9ccacd74..b622bdaf 100644
|
|
--- a/pcs/lib/cluster_property.py
|
|
+++ b/pcs/lib/cluster_property.py
|
|
@@ -8,6 +8,7 @@ from lxml.etree import _Element
|
|
|
|
from pcs.common import reports
|
|
from pcs.common.services.interfaces import ServiceManagerInterface
|
|
+from pcs.common.tools import timeout_to_seconds
|
|
from pcs.common.types import StringSequence
|
|
from pcs.lib import (
|
|
sbd,
|
|
@@ -38,8 +39,21 @@ def _validate_stonith_watchdog_timeout_property(
|
|
force: bool = False,
|
|
) -> reports.ReportItemList:
|
|
report_list: reports.ReportItemList = []
|
|
+ original_value = value
|
|
+ # if value is not empty, try to convert time interval string
|
|
+ if value:
|
|
+ seconds = timeout_to_seconds(value)
|
|
+ if seconds is None:
|
|
+ # returns empty list because this should be reported by
|
|
+ # ValueTimeInterval validator
|
|
+ return report_list
|
|
+ value = str(seconds)
|
|
if sbd.is_sbd_enabled(service_manager):
|
|
- report_list.extend(sbd.validate_stonith_watchdog_timeout(value, force))
|
|
+ report_list.extend(
|
|
+ sbd.validate_stonith_watchdog_timeout(
|
|
+ validate.ValuePair(original_value, value), force
|
|
+ )
|
|
+ )
|
|
else:
|
|
if value not in ["", "0"]:
|
|
report_list.append(
|
|
@@ -124,9 +138,6 @@ def validate_set_cluster_properties(
|
|
# unknow properties are reported by NamesIn validator
|
|
continue
|
|
property_metadata = possible_properties_dict[property_name]
|
|
- if property_metadata.name == "stonith-watchdog-timeout":
|
|
- # needs extra validation
|
|
- continue
|
|
if property_metadata.type == "boolean":
|
|
validators.append(
|
|
validate.ValuePcmkBoolean(
|
|
@@ -154,9 +165,13 @@ def validate_set_cluster_properties(
|
|
)
|
|
)
|
|
elif property_metadata.type == "time":
|
|
+ # make stonith-watchdog-timeout value not forcable
|
|
validators.append(
|
|
validate.ValueTimeInterval(
|
|
- property_metadata.name, severity=severity
|
|
+ property_metadata.name,
|
|
+ severity=severity
|
|
+ if property_metadata.name != "stonith-watchdog-timeout"
|
|
+ else reports.ReportItemSeverity.error(),
|
|
)
|
|
)
|
|
report_list.extend(
|
|
diff --git a/pcs/lib/sbd.py b/pcs/lib/sbd.py
|
|
index 1e3cfb37..38cd8767 100644
|
|
--- a/pcs/lib/sbd.py
|
|
+++ b/pcs/lib/sbd.py
|
|
@@ -1,6 +1,9 @@
|
|
import re
|
|
from os import path
|
|
-from typing import Optional
|
|
+from typing import (
|
|
+ Optional,
|
|
+ Union,
|
|
+)
|
|
|
|
from pcs import settings
|
|
from pcs.common import reports
|
|
@@ -392,7 +395,10 @@ def _get_local_sbd_watchdog_timeout() -> int:
|
|
|
|
|
|
def validate_stonith_watchdog_timeout(
|
|
- stonith_watchdog_timeout: str, force: bool = False
|
|
+ stonith_watchdog_timeout: Union[
|
|
+ validate.TypeOptionValue, validate.ValuePair
|
|
+ ],
|
|
+ force: bool = False,
|
|
) -> reports.ReportItemList:
|
|
"""
|
|
Check sbd status and config when user is setting stonith-watchdog-timeout
|
|
@@ -401,6 +407,7 @@ def validate_stonith_watchdog_timeout(
|
|
|
|
stonith_watchdog_timeout -- value to be validated
|
|
"""
|
|
+ stonith_watchdog_timeout = validate.ValuePair.get(stonith_watchdog_timeout)
|
|
severity = reports.get_severity(reports.codes.FORCE, force)
|
|
if _is_device_set_local():
|
|
return (
|
|
@@ -412,11 +419,11 @@ def validate_stonith_watchdog_timeout(
|
|
),
|
|
)
|
|
]
|
|
- if stonith_watchdog_timeout not in ["", "0"]
|
|
+ if stonith_watchdog_timeout.normalized not in ["", "0"]
|
|
else []
|
|
)
|
|
|
|
- if stonith_watchdog_timeout in ["", "0"]:
|
|
+ if stonith_watchdog_timeout.normalized in ["", "0"]:
|
|
return [
|
|
reports.ReportItem(
|
|
severity,
|
|
diff --git a/pcs_test/tier0/lib/commands/test_cluster_property.py b/pcs_test/tier0/lib/commands/test_cluster_property.py
|
|
index 319d1df6..fd124843 100644
|
|
--- a/pcs_test/tier0/lib/commands/test_cluster_property.py
|
|
+++ b/pcs_test/tier0/lib/commands/test_cluster_property.py
|
|
@@ -120,6 +120,34 @@ class StonithWatchdogTimeoutMixin(LoadMetadataMixin):
|
|
)
|
|
self.env_assist.assert_reports([])
|
|
|
|
+ def _set_invalid_value(self, forced=False):
|
|
+ self.config.remove("services.is_enabled")
|
|
+ self.env_assist.assert_raise_library_error(
|
|
+ lambda: cluster_property.set_properties(
|
|
+ self.env_assist.get_env(),
|
|
+ {"stonith-watchdog-timeout": "15x"},
|
|
+ [] if not forced else [reports.codes.FORCE],
|
|
+ )
|
|
+ )
|
|
+ self.env_assist.assert_reports(
|
|
+ [
|
|
+ fixture.error(
|
|
+ reports.codes.INVALID_OPTION_VALUE,
|
|
+ option_name="stonith-watchdog-timeout",
|
|
+ option_value="15x",
|
|
+ allowed_values="time interval (e.g. 1, 2s, 3m, 4h, ...)",
|
|
+ cannot_be_empty=False,
|
|
+ forbidden_characters=None,
|
|
+ ),
|
|
+ ]
|
|
+ )
|
|
+
|
|
+ def test_set_invalid_value(self):
|
|
+ self._set_invalid_value(forced=False)
|
|
+
|
|
+ def test_set_invalid_value_forced(self):
|
|
+ self._set_invalid_value(forced=True)
|
|
+
|
|
|
|
class TestSetStonithWatchdogTimeoutSBDIsDisabled(
|
|
StonithWatchdogTimeoutMixin, TestCase
|
|
@@ -132,6 +160,9 @@ class TestSetStonithWatchdogTimeoutSBDIsDisabled(
|
|
def test_set_zero(self):
|
|
self._set_success({"stonith-watchdog-timeout": "0"})
|
|
|
|
+ def test_set_zero_time_suffix(self):
|
|
+ self._set_success({"stonith-watchdog-timeout": "0s"})
|
|
+
|
|
def test_set_not_zero_or_empty(self):
|
|
self.env_assist.assert_raise_library_error(
|
|
lambda: cluster_property.set_properties(
|
|
@@ -231,12 +262,12 @@ class TestSetStonithWatchdogTimeoutSBDIsEnabledWatchdogOnly(
|
|
def test_set_zero_forced(self):
|
|
self.config.env.push_cib(
|
|
crm_config=fixture_crm_config_properties(
|
|
- [("cib-bootstrap-options", {"stonith-watchdog-timeout": "0"})]
|
|
+ [("cib-bootstrap-options", {"stonith-watchdog-timeout": "0s"})]
|
|
)
|
|
)
|
|
cluster_property.set_properties(
|
|
self.env_assist.get_env(),
|
|
- {"stonith-watchdog-timeout": "0"},
|
|
+ {"stonith-watchdog-timeout": "0s"},
|
|
[reports.codes.FORCE],
|
|
)
|
|
self.env_assist.assert_reports(
|
|
@@ -271,7 +302,7 @@ class TestSetStonithWatchdogTimeoutSBDIsEnabledWatchdogOnly(
|
|
self.env_assist.assert_raise_library_error(
|
|
lambda: cluster_property.set_properties(
|
|
self.env_assist.get_env(),
|
|
- {"stonith-watchdog-timeout": "9"},
|
|
+ {"stonith-watchdog-timeout": "9s"},
|
|
[],
|
|
)
|
|
)
|
|
@@ -281,7 +312,7 @@ class TestSetStonithWatchdogTimeoutSBDIsEnabledWatchdogOnly(
|
|
reports.codes.STONITH_WATCHDOG_TIMEOUT_TOO_SMALL,
|
|
force_code=reports.codes.FORCE,
|
|
cluster_sbd_watchdog_timeout=10,
|
|
- entered_watchdog_timeout="9",
|
|
+ entered_watchdog_timeout="9s",
|
|
)
|
|
]
|
|
)
|
|
@@ -289,12 +320,12 @@ class TestSetStonithWatchdogTimeoutSBDIsEnabledWatchdogOnly(
|
|
def test_too_small_forced(self):
|
|
self.config.env.push_cib(
|
|
crm_config=fixture_crm_config_properties(
|
|
- [("cib-bootstrap-options", {"stonith-watchdog-timeout": "9"})]
|
|
+ [("cib-bootstrap-options", {"stonith-watchdog-timeout": "9s"})]
|
|
)
|
|
)
|
|
cluster_property.set_properties(
|
|
self.env_assist.get_env(),
|
|
- {"stonith-watchdog-timeout": "9"},
|
|
+ {"stonith-watchdog-timeout": "9s"},
|
|
[reports.codes.FORCE],
|
|
)
|
|
self.env_assist.assert_reports(
|
|
@@ -302,13 +333,13 @@ class TestSetStonithWatchdogTimeoutSBDIsEnabledWatchdogOnly(
|
|
fixture.warn(
|
|
reports.codes.STONITH_WATCHDOG_TIMEOUT_TOO_SMALL,
|
|
cluster_sbd_watchdog_timeout=10,
|
|
- entered_watchdog_timeout="9",
|
|
+ entered_watchdog_timeout="9s",
|
|
)
|
|
]
|
|
)
|
|
|
|
def test_more_than_timeout(self):
|
|
- self._set_success({"stonith-watchdog-timeout": "11"})
|
|
+ self._set_success({"stonith-watchdog-timeout": "11s"})
|
|
|
|
|
|
@mock.patch("pcs.lib.sbd.get_local_sbd_device_list", lambda: ["dev1", "dev2"])
|
|
@@ -323,6 +354,9 @@ class TestSetStonithWatchdogTimeoutSBDIsEnabledSharedDevices(
|
|
def test_set_to_zero(self):
|
|
self._set_success({"stonith-watchdog-timeout": "0"})
|
|
|
|
+ def test_set_to_zero_time_suffix(self):
|
|
+ self._set_success({"stonith-watchdog-timeout": "0min"})
|
|
+
|
|
def test_set_not_zero_or_empty(self):
|
|
self.env_assist.assert_raise_library_error(
|
|
lambda: cluster_property.set_properties(
|
|
diff --git a/pcs_test/tier0/lib/test_cluster_property.py b/pcs_test/tier0/lib/test_cluster_property.py
|
|
index 2feb728d..8d6f90b1 100644
|
|
--- a/pcs_test/tier0/lib/test_cluster_property.py
|
|
+++ b/pcs_test/tier0/lib/test_cluster_property.py
|
|
@@ -83,6 +83,7 @@ FIXTURE_VALID_OPTIONS_DICT = {
|
|
"integer_param": "10",
|
|
"percentage_param": "20%",
|
|
"select_param": "s3",
|
|
+ "stonith-watchdog-timeout": "0",
|
|
"time_param": "5min",
|
|
}
|
|
|
|
@@ -96,6 +97,8 @@ FIXTURE_INVALID_OPTIONS_DICT = {
|
|
"have-watchdog": "100",
|
|
}
|
|
|
|
+STONITH_WATCHDOG_TIMEOUT_UNSET_VALUES = ["", "0", "0s"]
|
|
+
|
|
|
|
def _fixture_parameter(name, param_type, default, enum_values):
|
|
return ResourceAgentParameter(
|
|
@@ -239,6 +242,7 @@ class TestValidateSetClusterProperties(TestCase):
|
|
sbd_enabled=False,
|
|
sbd_devices=False,
|
|
force=False,
|
|
+ valid_value=True,
|
|
):
|
|
self.mock_is_sbd_enabled.return_value = sbd_enabled
|
|
self.mock_sbd_devices.return_value = ["devices"] if sbd_devices else []
|
|
@@ -254,9 +258,13 @@ class TestValidateSetClusterProperties(TestCase):
|
|
),
|
|
expected_report_list,
|
|
)
|
|
- if "stonith-watchdog-timeout" in new_properties and (
|
|
- new_properties["stonith-watchdog-timeout"]
|
|
- or "stonith-watchdog-timeout" in configured_properties
|
|
+ if (
|
|
+ "stonith-watchdog-timeout" in new_properties
|
|
+ and (
|
|
+ new_properties["stonith-watchdog-timeout"]
|
|
+ or "stonith-watchdog-timeout" in configured_properties
|
|
+ )
|
|
+ and valid_value
|
|
):
|
|
self.mock_is_sbd_enabled.assert_called_once_with(
|
|
self.mock_service_manager
|
|
@@ -266,7 +274,10 @@ class TestValidateSetClusterProperties(TestCase):
|
|
if sbd_devices:
|
|
self.mock_sbd_timeout.assert_not_called()
|
|
else:
|
|
- if new_properties["stonith-watchdog-timeout"] in ["", "0"]:
|
|
+ if (
|
|
+ new_properties["stonith-watchdog-timeout"]
|
|
+ in STONITH_WATCHDOG_TIMEOUT_UNSET_VALUES
|
|
+ ):
|
|
self.mock_sbd_timeout.assert_not_called()
|
|
else:
|
|
self.mock_sbd_timeout.assert_called_once_with()
|
|
@@ -280,6 +291,8 @@ class TestValidateSetClusterProperties(TestCase):
|
|
self.mock_sbd_timeout.assert_not_called()
|
|
|
|
self.mock_is_sbd_enabled.reset_mock()
|
|
+ self.mock_sbd_devices.reset_mock()
|
|
+ self.mock_sbd_timeout.reset_mock()
|
|
|
|
def test_no_properties_to_set_or_unset(self):
|
|
self.assert_validate_set(
|
|
@@ -328,7 +341,7 @@ class TestValidateSetClusterProperties(TestCase):
|
|
)
|
|
|
|
def test_unset_stonith_watchdog_timeout_sbd_disabled(self):
|
|
- for value in ["0", ""]:
|
|
+ for value in STONITH_WATCHDOG_TIMEOUT_UNSET_VALUES:
|
|
with self.subTest(value=value):
|
|
self.assert_validate_set(
|
|
["stonith-watchdog-timeout"],
|
|
@@ -349,22 +362,27 @@ class TestValidateSetClusterProperties(TestCase):
|
|
)
|
|
|
|
def test_set_ok_stonith_watchdog_timeout_sbd_enabled_without_devices(self):
|
|
- self.assert_validate_set(
|
|
- [], {"stonith-watchdog-timeout": "15"}, [], sbd_enabled=True
|
|
- )
|
|
+ for value in ["15", "15s"]:
|
|
+ with self.subTest(value=value):
|
|
+ self.assert_validate_set(
|
|
+ [],
|
|
+ {"stonith-watchdog-timeout": value},
|
|
+ [],
|
|
+ sbd_enabled=True,
|
|
+ )
|
|
|
|
def test_set_small_stonith_watchdog_timeout_sbd_enabled_without_devices(
|
|
self,
|
|
):
|
|
self.assert_validate_set(
|
|
[],
|
|
- {"stonith-watchdog-timeout": "9"},
|
|
+ {"stonith-watchdog-timeout": "9s"},
|
|
[
|
|
fixture.error(
|
|
reports.codes.STONITH_WATCHDOG_TIMEOUT_TOO_SMALL,
|
|
force_code=reports.codes.FORCE,
|
|
cluster_sbd_watchdog_timeout=10,
|
|
- entered_watchdog_timeout="9",
|
|
+ entered_watchdog_timeout="9s",
|
|
)
|
|
],
|
|
sbd_enabled=True,
|
|
@@ -387,28 +405,54 @@ class TestValidateSetClusterProperties(TestCase):
|
|
force=True,
|
|
)
|
|
|
|
- def test_set_not_a_number_stonith_watchdog_timeout_sbd_enabled_without_devices(
|
|
+ def _set_invalid_value_stonith_watchdog_timeout(
|
|
+ self, sbd_enabled=False, sbd_devices=False
|
|
+ ):
|
|
+ for value in ["invalid", "10x"]:
|
|
+ with self.subTest(value=value):
|
|
+ self.assert_validate_set(
|
|
+ [],
|
|
+ {"stonith-watchdog-timeout": value},
|
|
+ [
|
|
+ fixture.error(
|
|
+ reports.codes.INVALID_OPTION_VALUE,
|
|
+ option_name="stonith-watchdog-timeout",
|
|
+ option_value=value,
|
|
+ allowed_values="time interval (e.g. 1, 2s, 3m, 4h, ...)",
|
|
+ cannot_be_empty=False,
|
|
+ forbidden_characters=None,
|
|
+ )
|
|
+ ],
|
|
+ sbd_enabled=sbd_enabled,
|
|
+ sbd_devices=sbd_devices,
|
|
+ valid_value=False,
|
|
+ )
|
|
+
|
|
+ def test_set_invalid_value_stonith_watchdog_timeout_sbd_enabled_without_devices(
|
|
self,
|
|
):
|
|
+ self._set_invalid_value_stonith_watchdog_timeout(
|
|
+ sbd_enabled=True, sbd_devices=False
|
|
+ )
|
|
|
|
- self.assert_validate_set(
|
|
- [],
|
|
- {"stonith-watchdog-timeout": "invalid"},
|
|
- [
|
|
- fixture.error(
|
|
- reports.codes.STONITH_WATCHDOG_TIMEOUT_TOO_SMALL,
|
|
- force_code=reports.codes.FORCE,
|
|
- cluster_sbd_watchdog_timeout=10,
|
|
- entered_watchdog_timeout="invalid",
|
|
- )
|
|
- ],
|
|
- sbd_enabled=True,
|
|
+ def test_set_invalid_value_stonith_watchdog_timeout_sbd_enabled_with_devices(
|
|
+ self,
|
|
+ ):
|
|
+ self._set_invalid_value_stonith_watchdog_timeout(
|
|
+ sbd_enabled=True, sbd_devices=True
|
|
+ )
|
|
+
|
|
+ def test_set_invalid_value_stonith_watchdog_timeout_sbd_disabled(
|
|
+ self,
|
|
+ ):
|
|
+ self._set_invalid_value_stonith_watchdog_timeout(
|
|
+ sbd_enabled=False, sbd_devices=False
|
|
)
|
|
|
|
def test_unset_stonith_watchdog_timeout_sbd_enabled_without_devices(
|
|
self,
|
|
):
|
|
- for value in ["0", ""]:
|
|
+ for value in STONITH_WATCHDOG_TIMEOUT_UNSET_VALUES:
|
|
with self.subTest(value=value):
|
|
self.assert_validate_set(
|
|
["stonith-watchdog-timeout"],
|
|
@@ -426,7 +470,7 @@ class TestValidateSetClusterProperties(TestCase):
|
|
def test_unset_stonith_watchdog_timeout_sbd_enabled_without_devices_forced(
|
|
self,
|
|
):
|
|
- for value in ["0", ""]:
|
|
+ for value in STONITH_WATCHDOG_TIMEOUT_UNSET_VALUES:
|
|
with self.subTest(value=value):
|
|
self.assert_validate_set(
|
|
["stonith-watchdog-timeout"],
|
|
@@ -459,7 +503,7 @@ class TestValidateSetClusterProperties(TestCase):
|
|
def test_set_stonith_watchdog_timeout_sbd_enabled_with_devices_forced(self):
|
|
self.assert_validate_set(
|
|
[],
|
|
- {"stonith-watchdog-timeout": 15},
|
|
+ {"stonith-watchdog-timeout": "15s"},
|
|
[
|
|
fixture.warn(
|
|
reports.codes.STONITH_WATCHDOG_TIMEOUT_CANNOT_BE_SET,
|
|
@@ -472,7 +516,7 @@ class TestValidateSetClusterProperties(TestCase):
|
|
)
|
|
|
|
def test_unset_stonith_watchdog_timeout_sbd_enabled_with_devices(self):
|
|
- for value in ["0", ""]:
|
|
+ for value in STONITH_WATCHDOG_TIMEOUT_UNSET_VALUES:
|
|
with self.subTest(value=value):
|
|
self.assert_validate_set(
|
|
["stonith-watchdog-timeout"],
|
|
diff --git a/pcs_test/tier1/test_cluster_property.py b/pcs_test/tier1/test_cluster_property.py
|
|
index ff1f9cfb..51e25efc 100644
|
|
--- a/pcs_test/tier1/test_cluster_property.py
|
|
+++ b/pcs_test/tier1/test_cluster_property.py
|
|
@@ -169,7 +169,7 @@ class TestPropertySet(PropertyMixin, TestCase):
|
|
|
|
def test_set_stonith_watchdog_timeout(self):
|
|
self.assert_pcs_fail(
|
|
- "property set stonith-watchdog-timeout=5".split(),
|
|
+ "property set stonith-watchdog-timeout=5s".split(),
|
|
stdout_full=(
|
|
"Error: stonith-watchdog-timeout can only be unset or set to 0 "
|
|
"while SBD is disabled\n"
|
|
@@ -179,6 +179,18 @@ class TestPropertySet(PropertyMixin, TestCase):
|
|
)
|
|
self.assert_resources_xml_in_cib(UNCHANGED_CRM_CONFIG)
|
|
|
|
+ def test_set_stonith_watchdog_timeout_invalid_value(self):
|
|
+ self.assert_pcs_fail(
|
|
+ "property set stonith-watchdog-timeout=5x".split(),
|
|
+ stdout_full=(
|
|
+ "Error: '5x' is not a valid stonith-watchdog-timeout value, use"
|
|
+ " time interval (e.g. 1, 2s, 3m, 4h, ...)\n"
|
|
+ "Error: Errors have occurred, therefore pcs is unable to "
|
|
+ "continue\n"
|
|
+ ),
|
|
+ )
|
|
+ self.assert_resources_xml_in_cib(UNCHANGED_CRM_CONFIG)
|
|
+
|
|
|
|
class TestPropertyUnset(PropertyMixin, TestCase):
|
|
def test_success(self):
|
|
--
|
|
2.39.0
|
|
|