Validate platform settings (background job runner) (#4938)

Validate platform settings.

CHANGES.md

@@ -35,8 +35,8 @@ Fourth Release Candidate for Cylc 8 suitable for acceptance testing.
 
 ### Enhancements
 
-[#4936](https://github.com/cylc/cylc-flow/pull/4936) - Fix incorrect
-error messages when workflow CLI commands fail.
+[#4938](https://github.com/cylc/cylc-flow/pull/4938) - Detect bad Platforms
+config: background and at job runners should have a single host.
 
 [#4877](https://github.com/cylc/cylc-flow/pull/4877) - Upgrade the version of
 Jinja2 used by Cylc from 2.11 to 3.0.
@@ -49,6 +49,9 @@ in `global.cylc[install]source dirs`.
 
 ### Fixes
 
+[#4936](https://github.com/cylc/cylc-flow/pull/4936) - Fix incorrect
+error messages when workflow CLI commands fail.
+
 [#4941](https://github.com/cylc/cylc-flow/pull/4941) - Fix job state for
 platform submit-failures.
 

cylc/flow/cfgspec/globalcfg.py

@@ -26,6 +26,7 @@
 
 from cylc.flow import LOG
 from cylc.flow import __version__ as CYLC_VERSION
+from cylc.flow.platforms import validate_platforms
 from cylc.flow.exceptions import GlobalConfigError
 from cylc.flow.hostuserutil import get_user_home
 from cylc.flow.network.client_factory import CommsMeth
@@ -1679,6 +1680,13 @@ def default_for(
                 the name of this platform group.
 
                 .. versionadded:: 8.0.0
+
+                .. note::
+
+                   Some job runners ("background", "at") require a single-host
+                   platform, because the job ID is only valid on the submission
+                   host.
+
             ''')
             with Conf('selection'):
                 Conf(
@@ -1858,6 +1866,8 @@ def load(self) -> None:
 
         self._set_default_editors()
         self._no_platform_group_name_overlap()
+        with suppress(KeyError):
+            validate_platforms(self.sparse['platforms'])
 
     def _validate_source_dirs(self) -> None:
         """Check source dirs are absolute paths."""

cylc/flow/platforms.py

@@ -26,11 +26,11 @@
 from cylc.flow import LOG
 
 from cylc.flow.exceptions import (
+    GlobalConfigError,
     PlatformLookupError, CylcError, NoHostsError, NoPlatformsError)
 from cylc.flow.cfgspec.glbl_cfg import glbl_cfg
 from cylc.flow.hostuserutil import is_remote_host
 
-
 UNKNOWN_TASK = 'unknown task'
 
 FORBIDDEN_WITH_PLATFORM: Tuple[Tuple[str, str, List[Optional[str]]], ...] = (
@@ -39,6 +39,9 @@
     ('job', 'batch submit command template', [None])
 )
 
+DEFAULT_JOB_RUNNER = 'background'
+SINGLE_HOST_JOB_RUNNERS = ['background', 'at']
+
 # Regex to check whether a string is a command
 HOST_REC_COMMAND = re.compile(r'(`|\$\()\s*(.*)\s*([`)])$')
 PLATFORM_REC_COMMAND = re.compile(r'(\$\()\s*(.*)\s*([)])$')
@@ -658,3 +661,36 @@ def get_localhost_install_target() -> str:
     """Returns the install target of localhost platform"""
     localhost = get_platform()
     return get_install_target_from_platform(localhost)
+
+
+def _validate_single_host(platforms_cfg) -> None:
+    """Check that single-host platforms only specify a single host.
+
+    Some job runners don't work across multiple hosts; the job ID is only valid
+    on the specific submission host.
+    """
+    bad_platforms = []
+    runners = set()
+    for name, config in platforms_cfg.items():
+        runner = config.get('job runner', DEFAULT_JOB_RUNNER)
+        hosts = config.get('hosts', [])
+        if runner in SINGLE_HOST_JOB_RUNNERS and len(hosts) > 1:
+            bad_platforms.append((name, runner, hosts))
+            runners.add(runner)
+    if bad_platforms:
+        if len(runners) > 1:
+            grammar = ["are", "s"]
+        else:
+            grammar = ["is a", ""]
+        msg = (
+            f"{', '.join(runners)} {grammar[0]} single-host"
+            f" job runner{grammar[1]}:"
+        )
+        for name, runner, hosts in bad_platforms:
+            msg += f'\n * Platform {name} ({runner}) hosts: {", ".join(hosts)}'
+        raise GlobalConfigError(msg)
+
+
+def validate_platforms(platforms_cfg: Dict[str, Any]) -> None:
+    """Check for invalid or inconsistent platforms config."""
+    _validate_single_host(platforms_cfg)

tests/functional/cylc-config/09-platforms.t

@@ -23,8 +23,10 @@ set_test_number 5
 cat > "global.cylc" <<__HEREDOC__
 [platforms]
     [[foo]]
+        job runner = slurm
         hosts = of_melkor, of_valar
     [[bar]]
+        job runner = slurm
         hosts = of_orcs, of_gondor
 [platform groups]
     [[FOO]]
@@ -58,7 +60,7 @@ Platform Groups
 __HEREDOC__
 
 TEST_NAME="${TEST_NAME_BASE}-s"
-head -n 8 > just_platforms < global.cylc
+head -n 10 > just_platforms < global.cylc
 run_ok "${TEST_NAME}" cylc config --platforms
 cmp_ok "${TEST_NAME}.stdout" "just_platforms"
 

tests/functional/cylc-config/10-platform-expansion.t

@@ -27,6 +27,7 @@ cat > "global.cylc" <<__HEREDOC__
         baz\d\d, qux\S\S \
     ]]
         hosts = of_melkor, of_valar
+        job runner = slurm
 __HEREDOC__
 
 export CYLC_CONF_PATH="${PWD}"

tests/functional/intelligent-host-selection/00-mixedhost.t

@@ -24,6 +24,8 @@ export REQUIRE_PLATFORM='loc:remote fs:indep comms:tcp'
 #-------------------------------------------------------------------------------
 set_test_number 4
 
+# Uses a fake background job runner to get around the single host restriction.
+
 create_test_global_config "" "
 [platforms]
     [[goodhostplatform]]
@@ -32,6 +34,7 @@ create_test_global_config "" "
         retrieve job logs = True
 
     [[mixedhostplatform]]
+        job runner = my_background
         hosts = unreachable_host, ${CYLC_TEST_HOST}
         install target = ${CYLC_TEST_INSTALL_TARGET}
         retrieve job logs = True
@@ -42,6 +45,9 @@ create_test_global_config "" "
 
 install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
 
+# Install the fake background job runner.
+cp -r "${TEST_SOURCE_DIR}/lib" "${WORKFLOW_RUN_DIR}"
+
 run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}"
 
 workflow_run_ok "${TEST_NAME_BASE}-run" \

tests/functional/intelligent-host-selection/01-periodic-clear-badhosts.t

@@ -27,6 +27,9 @@ set_test_number 3
 
 install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
 
+# Install the fake background job runner.
+cp -r "${TEST_SOURCE_DIR}/lib" "${WORKFLOW_RUN_DIR}"
+
 create_test_global_config '' "
     [scheduler]
         [[main loop]]

tests/functional/intelligent-host-selection/02-badhosts.t

@@ -25,9 +25,12 @@ export REQUIRE_PLATFORM='loc:remote fs:indep comms:tcp'
 #-------------------------------------------------------------------------------
 set_test_number 6
 
+# Uses a fake background job runner to get around the single host restriction.
+
 create_test_global_config "" "
 [platforms]
     [[badhostplatform]]
+        job runner = my_background
         hosts = e9755ca30f5, 3c0b4799402
         install target = ${CYLC_TEST_INSTALL_TARGET}
         [[[selection]]]
@@ -39,6 +42,7 @@ create_test_global_config "" "
         retrieve job logs = True
 
     [[mixedhostplatform]]
+        job runner = my_background
         hosts = unreachable_host, ${CYLC_TEST_HOST}
         install target = ${CYLC_TEST_INSTALL_TARGET}
         retrieve job logs = True
@@ -51,6 +55,9 @@ create_test_global_config "" "
 
 install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
 
+# Install the fake background job runner.
+cp -r "${TEST_SOURCE_DIR}/lib" "${WORKFLOW_RUN_DIR}"
+
 run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}"
 
 workflow_run_ok "${TEST_NAME_BASE}-run" \

tests/functional/intelligent-host-selection/03-polling.t

@@ -30,6 +30,8 @@ export REQUIRE_PLATFORM='loc:remote fs:indep'
 #-------------------------------------------------------------------------------
 set_test_number 4
 
+# Uses a fake background job runner to get around the single host restriction.
+
 create_test_global_config "" "
 [scheduler]
     [[main loop]]
@@ -46,6 +48,7 @@ create_test_global_config "" "
         submission polling intervals = PT0S, 5*PT2S
 
     [[mixedhostplatform]]
+        job runner = my_background
         hosts = unreachable_host, ${CYLC_TEST_HOST}
         install target = ${CYLC_TEST_INSTALL_TARGET}
         retrieve job logs = True
@@ -58,6 +61,9 @@ create_test_global_config "" "
 #-------------------------------------------------------------------------------
 install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
 
+# Install the fake background job runner.
+cp -r "${TEST_SOURCE_DIR}/lib" "${WORKFLOW_RUN_DIR}"
+
 run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}"
 
 workflow_run_ok "${TEST_NAME_BASE}-run" \

tests/functional/intelligent-host-selection/04-kill.t

@@ -24,6 +24,8 @@ export REQUIRE_PLATFORM='loc:remote fs:indep comms:tcp'
 #-------------------------------------------------------------------------------
 set_test_number 4
 
+# Uses a fake background job runner to get around the single host restriction.
+
 create_test_global_config "" "
 [scheduler]
     [[main loop]]
@@ -36,6 +38,7 @@ create_test_global_config "" "
         install target = ${CYLC_TEST_INSTALL_TARGET}
 
     [[mixedhostplatform]]
+        job runner = my_background
         hosts = unreachable_host, ${CYLC_TEST_HOST}
         install target = ${CYLC_TEST_INSTALL_TARGET}
         [[[selection]]]
@@ -45,6 +48,9 @@ create_test_global_config "" "
 
 install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
 
+# Install the fake background job runner.
+cp -r "${TEST_SOURCE_DIR}/lib" "${WORKFLOW_RUN_DIR}"
+
 run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}"
 
 workflow_run_ok "${TEST_NAME_BASE}-run" \

tests/functional/intelligent-host-selection/05-from-platform-group.t

@@ -25,14 +25,18 @@ export REQUIRE_PLATFORM='loc:remote fs:indep comms:tcp'
 #-------------------------------------------------------------------------------
 set_test_number 11
 
+# Uses a fake background job runner to get around the single host restriction.
+
 create_test_global_config "" "
 [platforms]
     [[${CYLC_TEST_PLATFORM}]]
         # mixed host platform
+        job runner = my_background
         hosts = unreachable_host, ${CYLC_TEST_HOST}
         [[[selection]]]
             method = 'definition order'
     [[badhostplatform]]
+        job runner = my_background
         hosts = bad_host1, bad_host2
         [[[selection]]]
             method = 'definition order'
@@ -54,6 +58,9 @@ create_test_global_config "" "
 
 install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
 
+# Install the fake background job runner.
+cp -r "${TEST_SOURCE_DIR}/lib" "${WORKFLOW_RUN_DIR}"
+
 run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}"
 
 workflow_run_ok "${TEST_NAME_BASE}-run" \

tests/functional/intelligent-host-selection/06-from-platform-group-fails.t

@@ -22,11 +22,16 @@
 . "$(dirname "$0")/test_header"
 set_test_number 12
 #-------------------------------------------------------------------------------
+
+# Uses a fake background job runner to get around the single host restriction.
+
 create_test_global_config "" "
 [platforms]
     [[badhostplatform1]]
+        job runner = my_background
         hosts = bad_host1, bad_host2
     [[badhostplatform2]]
+        job runner = my_background
         hosts = bad_host3, bad_host4
 
 [platform groups]
@@ -36,6 +41,9 @@ create_test_global_config "" "
 
 install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
 
+# Install the fake background job runner.
+cp -r "${TEST_SOURCE_DIR}/lib" "${WORKFLOW_RUN_DIR}"
+
 run_ok "${TEST_NAME_BASE}-validate" cylc validate "${WORKFLOW_NAME}"
 
 workflow_run_fail "${TEST_NAME_BASE}-run" \

tests/functional/intelligent-host-selection/lib/python/my_background.py

@@ -0,0 +1,26 @@
+#!/usr/bin/env python3
+
+# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE.
+# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from cylc.flow.job_runner_handlers.background import BgCommandHandler
+
+
+class MyBgCommandHandler(BgCommandHandler):
+    pass
+
+
+JOB_RUNNER_HANDLER = MyBgCommandHandler()

tests/unit/test_platforms.py

@@ -27,9 +27,13 @@
     platform_from_name, platform_from_job_info,
     get_install_target_from_platform,
     get_install_target_to_platforms_map,
-    generic_items_match
+    generic_items_match,
+    _validate_single_host
+)
+from cylc.flow.exceptions import (
+    PlatformLookupError,
+    GlobalConfigError
 )
-from cylc.flow.exceptions import PlatformLookupError
 
 PLATFORMS = {
     'desktop[0-9]{2}|laptop[0-9]{2}': {
@@ -108,6 +112,20 @@
 }
 
 
+PLATFORMS_INVALID = {
+    'enterprise': {
+        'hosts': ['kirk', 'picard'],
+        'install target': 'picard',
+        'job runner': 'background'  # requires one host
+    },
+    'voyager': {
+        'hosts': ['janeway', 'seven-of-nine'],
+        'install target': 'janeway',
+        'job runner': 'at'  # requires one host
+    }
+}
+
+
 # ----------------------------------------------------------------------------
 # Tests of platform_from_name
 # ----------------------------------------------------------------------------
@@ -171,6 +189,17 @@ def test_platform_not_there():
         platform_from_name('moooo', PLATFORMS)
 
 
+@pytest.mark.parametrize(
+    'platform',
+    [
+        {i: j} for i, j in PLATFORMS_INVALID.items()
+    ]
+)
+def test_invalid_platforms(platform):
+    with pytest.raises(GlobalConfigError):
+        _validate_single_host(platform)
+
+
 def test_similar_but_not_exact_match():
     with pytest.raises(PlatformLookupError):
         platform_from_name('vld1', PLATFORMS_WITH_RE)