diff --git a/CHANGES.md b/CHANGES.md index 6232b61efd2..cf9346d61b0 100644 --- a/CHANGES.md +++ b/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. diff --git a/cylc/flow/hostuserutil.py b/cylc/flow/hostuserutil.py index 782a193746a..34e033b2d7b 100644 --- a/cylc/flow/hostuserutil.py +++ b/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: diff --git a/cylc/flow/scripts/message.py b/cylc/flow/scripts/message.py index 9e9e1f258ef..b2fb01f431c 100755 --- a/cylc/flow/scripts/message.py +++ b/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...' diff --git a/cylc/flow/task_job_mgr.py b/cylc/flow/task_job_mgr.py index e8985e48da0..e68bada23b9 100644 --- a/cylc/flow/task_job_mgr.py +++ b/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 diff --git a/cylc/flow/task_remote_mgr.py b/cylc/flow/task_remote_mgr.py index 59b6e04cf42..c38c2ce1e4c 100644 --- a/cylc/flow/task_remote_mgr.py +++ b/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,39 +111,31 @@ 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: @@ -146,34 +143,51 @@ def subshell_eval(self, command, command_pattern, host_check=True): 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. diff --git a/tests/functional/platforms/10-do-not-host-check-platforms.t b/tests/functional/platforms/10-do-not-host-check-platforms.t new file mode 100755 index 00000000000..c52ef7e8383 --- /dev/null +++ b/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 . +#------------------------------------------------------------------------------- +# 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 diff --git a/tests/unit/test_hostuserutil.py b/tests/unit/test_hostuserutil.py index f9d9d745262..aa21632530a 100644 --- a/tests/unit/test_hostuserutil.py +++ b/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())