modify the detection function of ISO8601 time

pcs/lib/cluster_property.py

@@ -41,7 +41,7 @@ def _validate_stonith_watchdog_timeout_property(
     report_list: reports.ReportItemList = []
     original_value = value
     # if value is not empty, try to convert time interval string
-    if value and not value.startswith('P'):
+    if value:
         seconds = timeout_to_seconds(value)
         if seconds is None:
             # returns empty list because this should be reported by
@@ -167,15 +167,19 @@ def validate_set_cluster_properties(
             )
         elif property_metadata.type == "time":
             # make stonith-watchdog-timeout value not forcable
-            validators.append(
-                validate.ValueTimeIntervalOrDuration(
-                    property_metadata.name,
-                    runner=runner,
-                    severity=severity
-                    if property_metadata.name != "stonith-watchdog-timeout"
-                    else reports.ReportItemSeverity.error(),
+            if property_metadata.name == "stonith-watchdog-timeout":
+                validators.append(validate.ValueTimeInterval(
+                        property_metadata.name,
+                        severity=reports.ReportItemSeverity.error()
+                    )
+                )
+            else:
+                validators.append(validate.ValueTimeIntervalOrDuration(
+                        property_metadata.name,
+                        runner=runner,
+                        severity=severity
+                    )
                 )
-            )
     report_list.extend(
         validate.ValidatorAll(validators).validate(to_be_set_properties)
     )

pcs/lib/commands/cluster_property.py

@@ -134,6 +134,7 @@ def set_properties(
     force_flags -- list of flags codes
     """
     cib = env.get_cib()
+    runner = env.cmd_runner()
     id_provider = IdProvider(cib)
     force = reports.codes.FORCE in force_flags
     cluster_property_set_el = (
@@ -145,7 +146,7 @@ def set_properties(
 
     property_facade_list = _get_property_facade_list(
         env.report_processor,
-        ResourceAgentFacadeFactory(env.cmd_runner(), env.report_processor),
+        ResourceAgentFacadeFactory(runner, env.report_processor),
     )
 
     configured_properties = [
@@ -158,7 +159,7 @@ def set_properties(
 
     env.report_processor.report_list(
         cluster_property.validate_set_cluster_properties(
-            env.cmd_runner(),
+            runner,
             property_facade_list,
             set_id,
             configured_properties,

pcs/lib/pacemaker/values.py

@@ -73,10 +73,8 @@ def is_duration(runner: CommandRunner, value: str) -> bool:
         "--duration",
         value
     ]
-    stdout, stderr, retval = runner.run(cmd)
-    if retval != 0:
-        return False
-    return True
+    _, _, retval = runner.run(cmd)
+    return retval == 0
 
 
 def get_valid_timeout_seconds(

pcs/lib/validate.py

@@ -62,14 +62,14 @@
     is_port_number,
 )
 from pcs.lib.cib.tools import IdProvider
-from pcs.lib.external import CommandRunner
 from pcs.lib.corosync import constants as corosync_constants
+from pcs.lib.external import CommandRunner
 from pcs.lib.pacemaker.values import (
     BOOLEAN_VALUES,
     SCORE_INFINITY,
     is_boolean,
-    is_score,
     is_duration,
+    is_score,
     validate_id,
 )
 
@@ -548,7 +548,6 @@ def __init__(
         option_name: TypeOptionName,
         option_name_for_report: Optional[str] = None,
         severity: Optional[ReportItemSeverity] = None,
-        runner: CommandRunner = None,
     ):
         """
         severity -- severity of produced reports, defaults to error
@@ -559,7 +558,6 @@ def __init__(
         self._severity = (
             ReportItemSeverity.error() if severity is None else severity
         )
-        self.runner = runner
         self._value_cannot_be_empty = False
         self._forbidden_characters = None
 
@@ -997,12 +995,23 @@ class ValueTimeIntervalOrDuration(ValuePredicateBase):
     """
     Time interval in number+units or ISO8601 duration (e.g. 1, 2s, 3m, 4h, PT1H2M3S, ...)
     """
+    def __init__(
+        self,
+        option_name: TypeOptionName,
+        option_name_for_report: Optional[str] = None,
+        severity: Optional[ReportItemSeverity] = None,
+        runner: CommandRunner = None,
+    ):
+        super().__init__(
+            option_name,
+            option_name_for_report=option_name_for_report,
+            severity=severity,
+        )
+        self.runner = runner
+
     def _is_valid(self, value: TypeOptionValue) -> bool:
         if value.startswith('P'):
-            if is_duration(self.runner, value):
-                return value
-            else:
-                return None
+            return is_duration(self.runner, value)
         else:
             return timeout_to_seconds(value) is not None
 

pcs_test/tier0/lib/commands/test_cluster_property.py

@@ -157,7 +157,7 @@ def _set_invalid_value(self, forced=False):
                     reports.codes.INVALID_OPTION_VALUE,
                     option_name="stonith-watchdog-timeout",
                     option_value="15x",
-                    allowed_values="time interval (e.g. 1, 2s, 3m, 4h, PT1H2M3S, ...)",
+                    allowed_values="time interval (e.g. 1, 2s, 3m, 4h, ...)",
                     cannot_be_empty=False,
                     forbidden_characters=None,
                 ),

pcs_test/tier0/lib/pacemaker/test_live.py

@@ -32,6 +32,7 @@
     TmpFileCall,
     TmpFileMock,
 )
+from pcs_test.tools.custom_mock import get_runner_mock as get_runner
 from pcs_test.tools.misc import get_test_resource as rc
 from pcs_test.tools.xml import (
     XmlManipulation,

pcs_test/tier0/lib/pacemaker/test_values.py

@@ -5,6 +5,7 @@
 from pcs.common.reports import codes as report_codes
 
 from pcs_test.tools.assertions import assert_raise_library_error
+from pcs_test.tools.custom_mock import get_runner_mock
 
 # pylint: disable=no-self-use
 
@@ -245,3 +246,10 @@ def test_returns_false_for_nonumber_noinfinity(self):
 
     def test_returns_false_for_multiple_operators(self):
         self.assertFalse(lib.is_score("++INFINITY"))
+
+class IsDurationValueTest(TestCase):
+    def test_return_true_for_iso8601_time(self):
+        self.assertTrue(lib.is_duration(get_runner_mock(),"P600S"))
+
+    def test_return_false_for_number(self):
+        self.assertTrue(lib.is_duration(get_runner_mock(),"600"))

pcs_test/tier0/lib/test_cluster_property.py

@@ -15,13 +15,13 @@
     ResourceAgentParameter,
 )
 from pcs.lib.xml_tools import etree_to_str
-from pcs.lib.external import CommandRunner
 
 from pcs_test.tools import fixture
 from pcs_test.tools.assertions import (
     assert_report_item_list_equal,
     assert_xml_equal,
 )
+from pcs_test.tools.custom_mock import get_runner_mock
 
 TEMPLATE_CRM_CONFIG = """
 <cib validate-with="pacemaker-3.8">
@@ -182,11 +182,6 @@ def _fixture_parameter(name, param_type, default, enum_values):
     ),
 ]
 
-def get_runner(stdout="", stderr="", returncode=0, env_vars=None):
-    runner = mock.MagicMock(spec_set=CommandRunner)
-    runner.run.return_value = (stdout, stderr, returncode)
-    runner.env_vars = env_vars if env_vars else {}
-    return runner
 
 def warning_reports(report_list):
     warning_report_list = []
@@ -255,7 +250,7 @@ def assert_validate_set(
         self.mock_sbd_timeout.return_value = 10
         assert_report_item_list_equal(
             lib_cluster_property.validate_set_cluster_properties(
-                get_runner(),
+                get_runner_mock(),
                 self.mock_facade_list,
                 "property-set-id",
                 configured_properties,
@@ -425,7 +420,7 @@ def _set_invalid_value_stonith_watchdog_timeout(
                             reports.codes.INVALID_OPTION_VALUE,
                             option_name="stonith-watchdog-timeout",
                             option_value=value,
-                            allowed_values="time interval (e.g. 1, 2s, 3m, 4h, PT1H2M3S, ...)",
+                            allowed_values="time interval (e.g. 1, 2s, 3m, 4h, ...)",
                             cannot_be_empty=False,
                             forbidden_characters=None,
                         )

pcs_test/tier0/lib/test_validate.py

@@ -1,8 +1,5 @@
 import re
-from unittest import (
-    TestCase,
-    mock,
-)
+from unittest import TestCase
 
 from lxml import etree
 
@@ -13,10 +10,10 @@
 )
 from pcs.lib import validate
 from pcs.lib.cib.tools import IdProvider
-from pcs.lib.external import CommandRunner
 
 from pcs_test.tools import fixture
 from pcs_test.tools.assertions import assert_report_item_list_equal
+from pcs_test.tools.custom_mock import get_runner_mock
 
 # pylint: disable=no-self-use
 # pylint: disable=too-many-lines
@@ -1753,23 +1750,17 @@ def test_reports_about_invalid_interval(self):
 
 
 class ValueTimeIntervalOrDuration(TestCase):
-    def get_runner(stdout="", stderr="", returncode=0, env_vars=None):
-        runner = mock.MagicMock(spec_set=CommandRunner)
-        runner.run.return_value = (stdout, stderr, returncode)
-        runner.env_vars = env_vars if env_vars else {}
-        return runner
-
     def test_no_reports_for_valid_time_interval(self):
         for interval in ["0", "1s", "2sec", "3m", "4min", "5h", "PT1H2M3S", "6hr"]:
             with self.subTest(value=interval):
                 assert_report_item_list_equal(
-                    validate.ValueTimeIntervalOrDuration("a", runner=self.get_runner()).validate({"a": interval}),
+                    validate.ValueTimeIntervalOrDuration("a", runner=get_runner_mock()).validate({"a": interval}),
                     [],
                 )
 
     def test_reports_about_invalid_interval(self):
         assert_report_item_list_equal(
-            validate.ValueTimeIntervalOrDuration("a", runner=self.get_runner()).validate({"a": "invalid_value"}),
+            validate.ValueTimeIntervalOrDuration("a", runner=get_runner_mock()).validate({"a": "invalid_value"}),
             [
                 fixture.error(
                     reports.codes.INVALID_OPTION_VALUE,

pcs_test/tools/custom_mock.py

@@ -17,6 +17,7 @@
 )
 from pcs.common.types import CibRuleInEffectStatus
 from pcs.lib.cib.rule.in_effect import RuleInEffectEval
+from pcs.lib.external import CommandRunner
 
 from pcs_test.tools.assertions import assert_report_item_list_equal
 
@@ -30,6 +31,13 @@ def socket_getaddrinfo(host, port, family=0, type=0, proto=0, flags=0):
     return socket_getaddrinfo
 
 
+def get_runner_mock(stdout="", stderr="", returncode=0, env_vars=None):
+    runner = mock.MagicMock(spec_set=CommandRunner)
+    runner.run.return_value = (stdout, stderr, returncode)
+    runner.env_vars = env_vars if env_vars else {}
+    return runner
+
+
 def patch_getaddrinfo(test_case, addr_list):
     """
     class MyTest(TestCase):