Merge branch 'iso8601'

CHANGELOG.md

@@ -6,6 +6,10 @@
 - Make use of filters when extracting tarballs to enhance security if provided
   by Python (`pcs config restore` command) ([rhbz#2219407])
 
+### Added
+- Support ISO 8601 duration specifications for pacemaker "time" properties
+  ([rhbz#2112268], [ghpull#712])
+
 ### Fixed
 - Exporting constraints with rules in form of pcs commands now escapes `#` and
   fixes spaces in dates to make the commands valid ([rhbz#2163953])
@@ -22,6 +26,8 @@
 - Using spaces in dates in location constraint rules (using spaces in dates in
   rules in other parts of configuration was never allowed) ([rhbz#2163953])
 
+[ghpull#712]: https://github.com/ClusterLabs/pcs/pull/712
+[rhbz#2112268]: https://bugzilla.redhat.com/show_bug.cgi?id=2112268
 [rhbz#2163953]: https://bugzilla.redhat.com/show_bug.cgi?id=2163953
 [rhbz#2217850]: https://bugzilla.redhat.com/show_bug.cgi?id=2217850
 [rhbz#2219407]: https://bugzilla.redhat.com/show_bug.cgi?id=2219407

pcs/lib/cluster_property.py

@@ -20,6 +20,7 @@
     get_pacemaker_version_by_which_cib_was_validated,
 )
 from pcs.lib.errors import LibraryError
+from pcs.lib.external import CommandRunner
 from pcs.lib.resource_agent import ResourceAgentFacade
 
 READONLY_CLUSTER_PROPERTY_LIST = [
@@ -66,6 +67,7 @@ def _validate_stonith_watchdog_timeout_property(
 
 
 def validate_set_cluster_properties(
+    runner: CommandRunner,
     cluster_property_facade_list: Iterable[ResourceAgentFacade],
     properties_set_id: str,
     configured_properties: StringSequence,
@@ -83,6 +85,7 @@ def validate_set_cluster_properties(
     service_manager -- manager for system daemon services
     force -- if True, produce warnings instead of errors
     """
+    # pylint: disable=too-many-branches
     # pylint: disable=too-many-locals
     possible_properties_dict = {
         parameter.name: parameter
@@ -165,14 +168,19 @@ 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
-                    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(
+                        runner, property_metadata.name, 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,6 +159,7 @@ def set_properties(
 
     env.report_processor.report_list(
         cluster_property.validate_set_cluster_properties(
+            runner,
             property_facade_list,
             set_id,
             configured_properties,

pcs/lib/pacemaker/values.py

@@ -1,17 +1,20 @@
+import os
 import re
 from typing import (
     List,
     Optional,
     Union,
 )
 
+from pcs import settings
 from pcs.common import reports
 from pcs.common.reports.item import (
     ReportItem,
     ReportItemList,
 )
 from pcs.common.tools import timeout_to_seconds
 from pcs.lib.errors import LibraryError
+from pcs.lib.external import CommandRunner
 
 _BOOLEAN_TRUE = frozenset(["true", "on", "yes", "y", "1"])
 _BOOLEAN_FALSE = frozenset(["false", "off", "no", "n", "0"])
@@ -21,6 +24,10 @@
 SCORE_INFINITY = "INFINITY"
 
 
+def __exec(name):
+    return os.path.join(settings.pacemaker_binaries, name)
+
+
 def is_boolean(val: str) -> bool:
     """
     Does pacemaker consider a value to be a boolean?
@@ -61,6 +68,12 @@ def is_score(value: str) -> bool:
     return unsigned_value == SCORE_INFINITY or unsigned_value.isdigit()
 
 
+def is_duration(runner: CommandRunner, value: str) -> bool:
+    cmd = [__exec("iso8601"), "--duration", value]
+    _, _, retval = runner.run(cmd)
+    return retval == 0
+
+
 def get_valid_timeout_seconds(
     timeout_candidate: Union[str, int, None],
 ) -> Optional[int]:

pcs/lib/validate.py

@@ -63,10 +63,12 @@
 )
 from pcs.lib.cib.tools import IdProvider
 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_duration,
     is_score,
     validate_id,
 )
@@ -990,6 +992,35 @@ def _validate_value(self, value: ValuePair) -> ReportItemList:
         return report_list
 
 
+class ValueTimeIntervalOrDuration(ValuePredicateBase):
+    """
+    Time interval in number+units or ISO8601 duration (e.g. 1, 2s, 3m, 4h,
+    PT1H2M3S, ...)
+    """
+
+    def __init__(
+        self,
+        runner: CommandRunner,
+        option_name: TypeOptionName,
+        option_name_for_report: Optional[str] = None,
+        severity: Optional[ReportItemSeverity] = 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"):
+            return is_duration(self.runner, value)
+        return timeout_to_seconds(value) is not None
+
+    def _get_allowed_values(self) -> Any:
+        return "time interval (e.g. 1, 2s, 3m, 4h, PT1H2M3S, ...)"
+
+
 class ValueTimeInterval(ValuePredicateBase):
     """
     Report INVALID_OPTION_VALUE when the value is not a time interval

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,
@@ -41,13 +42,6 @@
 _EXITCODE_NOT_CONNECTED = 102
 
 
-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 path(name):
     return os.path.join(settings.pacemaker_binaries, name)
 

pcs_test/tier0/lib/pacemaker/test_values.py

@@ -1,10 +1,13 @@
+import os
 from unittest import TestCase
 
 import pcs.lib.pacemaker.values as lib
+from pcs import settings
 from pcs.common.reports import ReportItemSeverity as severity
 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 +248,23 @@ 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 assert_is_duration(self, return_value):
+        duration = "P"
+        mock_runner = get_runner_mock(returncode=0 if return_value else 1)
+        self.assertEqual(lib.is_duration(mock_runner, duration), return_value)
+        mock_runner.run.assert_called_once_with(
+            [
+                os.path.join(settings.pacemaker_binaries, "iso8601"),
+                "--duration",
+                duration,
+            ]
+        )
+
+    def test_duration_valid(self):
+        self.assert_is_duration(True)
+
+    def test_duration_invalid(self):
+        self.assert_is_duration(False)

pcs_test/tier0/lib/test_cluster_property.py

@@ -21,6 +21,7 @@
     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">
@@ -175,7 +176,7 @@ def _fixture_parameter(name, param_type, default, enum_values):
         force_code=reports.codes.FORCE,
         option_name="time_param",
         option_value="10x",
-        allowed_values="time interval (e.g. 1, 2s, 3m, 4h, ...)",
+        allowed_values="time interval (e.g. 1, 2s, 3m, 4h, PT1H2M3S, ...)",
         cannot_be_empty=False,
         forbidden_characters=None,
     ),
@@ -249,6 +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_mock(),
                 self.mock_facade_list,
                 "property-set-id",
                 configured_properties,

pcs_test/tier0/lib/test_validate.py

@@ -1,8 +1,10 @@
+import os
 import re
 from unittest import TestCase
 
 from lxml import etree
 
+from pcs import settings
 from pcs.common import reports
 from pcs.common.reports.const import (
     ADD_REMOVE_CONTAINER_TYPE_STONITH_RESOURCE,
@@ -13,6 +15,7 @@
 
 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
@@ -1748,6 +1751,64 @@ def test_reports_about_invalid_interval(self):
         )
 
 
+class ValueTimeIntervalOrDuration(TestCase):
+    duration = "PT1H2M3S"
+
+    def assert_runner(self, mock_runner, interval):
+        if interval == self.duration:
+            mock_runner.run.assert_called_once_with(
+                [
+                    os.path.join(settings.pacemaker_binaries, "iso8601"),
+                    "--duration",
+                    self.duration,
+                ]
+            )
+        else:
+            mock_runner.run.assert_not_called()
+
+    def test_no_reports_for_valid_time_interval(self):
+        for interval in [
+            "0",
+            "1s",
+            "2sec",
+            "3m",
+            "4min",
+            "5h",
+            self.duration,
+            "6hr",
+        ]:
+            with self.subTest(value=interval):
+                mock_runner = get_runner_mock()
+                assert_report_item_list_equal(
+                    validate.ValueTimeIntervalOrDuration(
+                        mock_runner, "a"
+                    ).validate({"a": interval}),
+                    [],
+                )
+                self.assert_runner(mock_runner, interval)
+
+    def test_reports_about_invalid_interval(self):
+        for interval in ["invalid_value", self.duration]:
+            with self.subTest(value=interval):
+                mock_runner = get_runner_mock(returncode=1)
+                assert_report_item_list_equal(
+                    validate.ValueTimeIntervalOrDuration(
+                        mock_runner, "a"
+                    ).validate({"a": interval}),
+                    [
+                        fixture.error(
+                            reports.codes.INVALID_OPTION_VALUE,
+                            option_name="a",
+                            option_value=interval,
+                            allowed_values="time interval (e.g. 1, 2s, 3m, 4h, PT1H2M3S, ...)",
+                            cannot_be_empty=False,
+                            forbidden_characters=None,
+                        ),
+                    ],
+                )
+                self.assert_runner(mock_runner, interval)
+
+
 class ValueVersion(TestCase):
     def test_no_reports_for_valid_time_interval(self):
         for version in ["123", "123.456", "123.456.789", "1.2.3.4"]:

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):

pcsd/capabilities.xml.in

@@ -1109,6 +1109,15 @@
         daemon urls: update_cluster_settings
       </description>
     </capability>
+    <capability id="pcmk.properties.cluster.set_properties.iso8601duration" in-pcs="1" in-pcsd="1">
+      <description>
+        Support ISO 8601 duration specifications for pacemaker "time"
+        properties.
+
+        pcs commands: property set
+        daemon urls: update_cluster_settings
+      </description>
+    </capability>
     <capability id="pcmk.properties.cluster.config.output-formats" in-pcs="1" in-pcsd="0">
       <description>
         Display cluster properties configuration in various formats.