Merge pull request cylc#5470 from cylc/8.1.x

8.1.x

CHANGES.md

@@ -48,6 +48,9 @@ Fixes a possible scheduler traceback observed with remote task polling.
 absence of `job name length maximum` in PBS platform settings would cause
 Cylc to crash when preparing the job script.
 
+[#5343](https://github.com/cylc/cylc-flow/pull/5343) - Fix a bug causing
+platform names to be checked as if they were hosts.
+
 [#5359](https://github.com/cylc/cylc-flow/pull/5359) - Fix bug where viewing
 a workflow's log in the GUI or using `cylc cat-log` would prevent `cylc clean`
 from working.

cylc/flow/hostuserutil.py

@@ -197,8 +197,8 @@ def is_remote_host(self, name):
 
         """
         if name not in self._remote_hosts:
-            if not name or name.split(".")[0].startswith("localhost"):
-                # e.g. localhost.localdomain
+            if not name or name.startswith("localhost"):
+                # e.g. localhost4.localdomain4
                 self._remote_hosts[name] = False
             else:
                 try:

cylc/flow/scripts/message.py

@@ -59,6 +59,18 @@
 The default message severity is INFO. The --severity=SEVERITY option can be
 used to set the default severity level for all unprefixed messages.
 
+Increased severity will make messages more visible in workflow logs, using
+colour and format changes. DEBUG messages will not be shown in logs by default.
+
+The severity levels are those of the Python Logging Library
+https://docs.python.org/3/library/logging.html#logging-levels:
+
+- CRITICAL
+- ERROR
+- WARNING
+- INFO
+- DEBUG
+
 Note:
   To abort a job script with a custom error message, use cylc__job_abort:
     cylc__job_abort 'message...'

cylc/flow/task_job_mgr.py

@@ -58,8 +58,6 @@
 )
 from cylc.flow.pathutil import get_remote_workflow_run_job_dir
 from cylc.flow.platforms import (
-    HOST_REC_COMMAND,
-    PLATFORM_REC_COMMAND,
     get_host_from_platform,
     get_install_target_from_platform,
     get_localhost_install_target,
@@ -1120,12 +1118,12 @@ def _prep_submit_task_job(
         host_n, platform_name = None, None
         try:
             if rtconfig['remote']['host'] is not None:
-                host_n = self.task_remote_mgr.subshell_eval(
-                    rtconfig['remote']['host'], HOST_REC_COMMAND
+                host_n = self.task_remote_mgr.eval_host(
+                    rtconfig['remote']['host']
                 )
             else:
-                platform_name = self.task_remote_mgr.subshell_eval(
-                    rtconfig['platform'], PLATFORM_REC_COMMAND
+                platform_name = self.task_remote_mgr.eval_platform(
+                    rtconfig['platform']
                 )
         except PlatformError as exc:
             itask.waiting_on_job_prep = False

cylc/flow/task_remote_mgr.py

@@ -32,12 +32,14 @@
 from subprocess import Popen, PIPE, DEVNULL
 import tarfile
 from time import sleep, time
-from typing import Any, Deque, Dict, TYPE_CHECKING, List, NamedTuple, Tuple
+from typing import (
+    Any, Deque, Dict, TYPE_CHECKING, List,
+    NamedTuple, Optional, Tuple
+)
 
 from cylc.flow import LOG
 from cylc.flow.exceptions import PlatformError
 import cylc.flow.flags
-from cylc.flow.hostuserutil import is_remote_host
 from cylc.flow.network.client_factory import CommsMeth
 from cylc.flow.pathutil import (
     get_dirs_to_symlink,
@@ -46,6 +48,8 @@
     get_workflow_run_dir,
 )
 from cylc.flow.platforms import (
+    HOST_REC_COMMAND,
+    PLATFORM_REC_COMMAND,
     NoHostsError,
     PlatformLookupError,
     get_host_from_platform,
@@ -67,6 +71,7 @@
 )
 
 from cylc.flow.loggingutil import get_next_log_number, get_sorted_logs_by_time
+from cylc.flow.hostuserutil import is_remote_host
 
 if TYPE_CHECKING:
     from zmq.auth.thread import ThreadAuthenticator
@@ -106,74 +111,83 @@ def __init__(self, workflow, proc_pool, bad_hosts):
         self.is_reload = False
         self.is_restart = False
 
-    def subshell_eval(self, command, command_pattern, host_check=True):
-        """Evaluate a task platform from a subshell string.
-
-        At Cylc 7, from a host string.
+    def _subshell_eval(
+        self, eval_str: str, command_pattern: re.Pattern
+    ) -> Optional[str]:
+        """Evaluate a platform or host from a possible subshell string.
 
         Arguments:
-            command (str):
-                An explicit host name, a command in back-tick or $(command)
-                format, or an environment variable holding a hostname.
-            command_pattern (re.Pattern):
+            eval_str:
+                An explicit host/platform name, a command, or an environment
+                variable holding a host/patform name.
+            command_pattern:
                 A compiled regex pattern designed to match subshell strings.
-            host_check (bool):
-                A flag to enable remote testing. If True, and if the command
-                is running locally, then it will return 'localhost'.
 
-        Return (str):
+        Return:
             - None if evaluation of command is still taking place.
-            - If command is not defined or the evaluated name is equivalent
-              to 'localhost', _and_ host_check is set to True then
-              'localhost'
-            - Otherwise, return the evaluated host name on success.
+            - 'localhost' if string is empty/not defined.
+            - Otherwise, return the evaluated host/platform name on success.
 
         Raise PlatformError on error.
 
         """
-        # BACK COMPAT: references to "host"
-        # remove at:
-        #     Cylc8.x
-        if not command:
+        if not eval_str:
             return 'localhost'
 
         # Host selection command: $(command) or `command`
-        match = command_pattern.match(command)
+        match = command_pattern.match(eval_str)
         if match:
             cmd_str = match.groups()[1]
             if cmd_str in self.remote_command_map:
                 # Command recently launched
                 value = self.remote_command_map[cmd_str]
                 if isinstance(value, PlatformError):
                     raise value  # command failed
-                elif value is None:
-                    return  # command not yet ready
-                else:
-                    command = value  # command succeeded
+                if value is None:
+                    return None  # command not yet ready
+                eval_str = value  # command succeeded
             else:
                 # Command not launched (or already reset)
                 self.proc_pool.put_command(
                     SubProcContext(
                         'remote-host-select',
                         ['bash', '-c', cmd_str],
-                        env=dict(os.environ)),
+                        env=dict(os.environ)
+                    ),
                     callback=self._subshell_eval_callback,
                     callback_args=[cmd_str]
                 )
                 self.remote_command_map[cmd_str] = None
-                return self.remote_command_map[cmd_str]
+                return None
 
         # Environment variable substitution
-        command = os.path.expandvars(command)
-        # Remote?
-        # TODO - Remove at Cylc 8.x as this only makes sense with host logic
-        if host_check is True:
-            if is_remote_host(command):
-                return command
-            else:
-                return 'localhost'
-        else:
-            return command
+        return os.path.expandvars(eval_str)
+
+    # BACK COMPAT: references to "host"
+        # remove at:
+        #     Cylc8.x
+    def eval_host(self, host_str: str) -> Optional[str]:
+        """Evaluate a host from a possible subshell string.
+
+        Args:
+            host_str: An explicit host name, a command in back-tick or
+                $(command) format, or an environment variable holding
+                a hostname.
+
+        Returns 'localhost' if evaluated name is equivalent
+        (e.g. localhost4.localdomain4).
+        """
+        host = self._subshell_eval(host_str, HOST_REC_COMMAND)
+        return host if is_remote_host(host) else 'localhost'
+
+    def eval_platform(self, platform_str: str) -> Optional[str]:
+        """Evaluate a platform from a possible subshell string.
+
+        Args:
+            platform_str: An explicit platform name, a command in $(command)
+                format, or an environment variable holding a platform name.
+        """
+        return self._subshell_eval(platform_str, PLATFORM_REC_COMMAND)
 
     def subshell_eval_reset(self):
         """Reset remote eval subshell results.

tests/functional/platforms/10-do-not-host-check-platforms.t

@@ -0,0 +1,54 @@
+#!/usr/bin/env bash
+# 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/>.
+#-------------------------------------------------------------------------------
+# Check that platform names are not treated as host names. E.g. a platform
+# name starting with "localhost" should not be treated as localhost.
+# https://github.com/cylc/cylc-flow/issues/5342
+. "$(dirname "$0")/test_header"
+
+set_test_number 2
+
+# shellcheck disable=SC2016
+create_test_global_config '' '
+[platforms]
+    [[localhost_spice]]
+        hosts = unreachable
+'
+
+make_rnd_workflow
+
+cat > "${RND_WORKFLOW_SOURCE}/flow.cylc" <<__HEREDOC__
+[scheduler]
+    [[events]]
+        stall timeout = PT0S
+[scheduling]
+    [[graph]]
+        R1 = foo
+[runtime]
+    [[foo]]
+        platform = localhost_spice
+__HEREDOC__
+
+ERR_STR='Unable to find valid host for localhost_spice'
+
+TEST_NAME="${TEST_NAME_BASE}-vip-workflow"
+run_fail "${TEST_NAME}" cylc vip "${RND_WORKFLOW_SOURCE}" --no-detach
+grep_ok "${ERR_STR}" \
+    "${TEST_NAME}.stderr" -F
+
+purge_rnd_workflow
+exit

tests/unit/test_hostuserutil.py

@@ -35,10 +35,11 @@ def test_is_remote_user_on_current_user():
     assert not is_remote_user(os.getenv('USER'))
 
 
-def test_is_remote_host_on_localhost():
+def test_is_remote_host_on_localhost(monkeypatch):
     """is_remote_host with localhost."""
     assert not is_remote_host(None)
     assert not is_remote_host('localhost')
+    assert not is_remote_host('localhost4.localhost42')
     assert not is_remote_host(os.getenv('HOSTNAME'))
     assert not is_remote_host(get_host())