rpms/pcs
7
0
Fork 0
Code Issues Pull Requests Releases Activity
4a74bda874
pcs/SOURCES/bz2158804-01-fix-stonith-watchdog-timeout-validation.patch
CentOS Sources 2a5856847e import pcs-0.10.15-3.el8
2023-01-19 08:10:48 +00:00

486 lines
19 KiB
Diff
Raw Blame History

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
Reference in New Issue View Git Blame Copy Permalink