Fix platform selection from subshell not re-evaluating each time

changes.d/6836.fix.md

@@ -0,0 +1 @@
+Fix a bug causing the results of `platform = $(subshell commands)` to be cached, and preventing re-evaluation for each task with the same config.

cylc/flow/platforms.py

@@ -83,7 +83,8 @@ def log_platform_event(
 def get_platform(
     task_conf: Optional[str] = None,
     task_name: str = UNKNOWN_TASK,
-    bad_hosts: Optional[Set[str]] = None
+    bad_hosts: Optional[Set[str]] = None,
+    evaluated_host: Optional[str] = None,
 ) -> Dict[str, Any]:
     ...
 
@@ -92,7 +93,8 @@ def get_platform(
 def get_platform(
     task_conf: Union[dict, 'OrderedDictWithDefaults'],
     task_name: str = UNKNOWN_TASK,
-    bad_hosts: Optional[Set[str]] = None
+    bad_hosts: Optional[Set[str]] = None,
+    evaluated_host: Optional[str] = None,
 ) -> Optional[Dict[str, Any]]:
     ...
 
@@ -108,7 +110,8 @@ def get_platform(
 def get_platform(
     task_conf: Union[str, dict, 'OrderedDictWithDefaults', None] = None,
     task_name: str = UNKNOWN_TASK,
-    bad_hosts: Optional[Set[str]] = None
+    bad_hosts: Optional[Set[str]] = None,
+    evaluated_host: Optional[str] = None,
 ) -> Optional[Dict[str, Any]]:
     """Get a platform.
 
@@ -121,6 +124,7 @@ def get_platform(
         task_name: Help produce more helpful error messages.
         bad_hosts: A set of hosts known to be unreachable (had an ssh 255
             error)
+        evaluated_host: Host name evaluated from platform subshell.
 
     Returns:
         platform: A platform definition dictionary. Uses either
@@ -169,7 +173,8 @@ def get_platform(
                 platform_name_from_job_info(
                     glbl_cfg().get(['platforms']),
                     task_job_section,
-                    task_remote_section
+                    task_remote_section,
+                    evaluated_host
                 ),
                 bad_hosts=bad_hosts
             )
@@ -331,7 +336,8 @@ def get_platform_from_group(
 def platform_name_from_job_info(
     platforms: Union[dict, 'OrderedDictWithDefaults'],
     job: Dict[str, Any],
-    remote: Dict[str, Any]
+    remote: Dict[str, Any],
+    evaluated_host: Optional[str] = None,
 ) -> str:
     """
     Find out which job platform to use given a list of possible platforms
@@ -386,6 +392,7 @@ def platform_name_from_job_info(
         job: Workflow config [runtime][TASK][job] section.
         remote: Workflow config [runtime][TASK][remote] section.
         platforms: Dictionary containing platform definitions.
+        evaluated_host: Host is the result of evaluating a subshell.
 
     Returns:
         platform: string representing a platform from the global config.
@@ -423,7 +430,9 @@ def platform_name_from_job_info(
 
     # NOTE: Do NOT use .get() on OrderedDictWithDefaults -
     # https://github.com/cylc/cylc-flow/pull/4975
-    if 'host' in remote and remote['host']:
+    if evaluated_host:
+        task_host = evaluated_host
+    elif 'host' in remote and remote['host']:
         task_host = remote['host']
     else:
         task_host = 'localhost'

cylc/flow/task_job_mgr.py

@@ -1236,10 +1236,10 @@ def _prep_submit_task_job(
                 f"\"{itask.identity}\" the following are not compatible:\n"
             )
 
-        host_n, platform_name = None, None
+        host_name, platform_name = [None] * 2
         try:
             if rtconfig['remote']['host'] is not None:
-                host_n = self.task_remote_mgr.eval_host(
+                host_name = self.task_remote_mgr.eval_host(
                     rtconfig['remote']['host']
                 )
             else:
@@ -1258,31 +1258,34 @@ def _prep_submit_task_job(
             return False
         else:
             # host/platform select not ready
-            if host_n is None and platform_name is None:
+            if host_name is None and platform_name is None:
                 return None
             elif (
-                host_n is None
+                host_name is None
                 and rtconfig['platform']
                 and rtconfig['platform'] != platform_name
             ):
                 LOG.debug(
                     f"for task {itask.identity}: platform = "
                     f"{rtconfig['platform']} evaluated as {platform_name}"
                 )
-                rtconfig['platform'] = platform_name
+                # orig_platform_name = rtconfig['platform']
+                # rtconfig['platform'] = platform_name
             elif (
                 platform_name is None
-                and rtconfig['remote']['host'] != host_n
+                and rtconfig['remote']['host'] != host_name
             ):
                 LOG.debug(
                     f"[{itask}] host = "
-                    f"{rtconfig['remote']['host']} evaluated as {host_n}"
+                    f"{rtconfig['remote']['host']} evaluated as {host_name}"
                 )
-                rtconfig['remote']['host'] = host_n
 
             try:
                 platform = get_platform(
-                    rtconfig, itask.tdef.name, bad_hosts=self.bad_hosts
+                    platform_name or rtconfig,
+                    itask.tdef.name,
+                    bad_hosts=self.bad_hosts,
+                    evaluated_host=host_name,
                 )
             except PlatformLookupError as exc:
                 itask.waiting_on_job_prep = False

tests/functional/platforms/12-ping-pong.t

@@ -0,0 +1,41 @@
+#!/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/>.
+#-----------------------------------------------------------------------------
+
+# If a task has a platform set using a subshell this should be evaluated
+# every time the task is run.
+# https://github.com/cylc/cylc-flow/issues/6808
+export REQUIRE_PLATFORM='loc:remote'
+
+. "$(dirname "$0")/test_header"
+
+set_test_number 3
+
+install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
+
+workflow_run_ok "${TEST_NAME_BASE}-run" \
+    cylc play "${WORKFLOW_NAME}" --debug --no-detach
+
+named_grep_ok "1/remote_task submits to ${CYLC_TEST_PLATFORM}" \
+    "\[1/remote_task/01:preparing\] submitted to ${CYLC_TEST_PLATFORM}" \
+    "${WORKFLOW_RUN_DIR}/log/scheduler/log"
+
+named_grep_ok "2/remote_task submits to localhost" \
+    "\[2/remote_task/01:preparing\] submitted to localhost" \
+    "${WORKFLOW_RUN_DIR}/log/scheduler/log"
+
+purge

tests/functional/platforms/12-ping-pong/flow.cylc

@@ -0,0 +1,31 @@
+#!jinja2
+[scheduler]
+    [[events]]
+        stall timeout = PT0S
+
+[scheduling]
+    cycling mode = integer
+    final cycle point = 2
+    [[graph]]
+        P1 = remote_task[-P1] => toggler => remote_task
+
+[runtime]
+    [[remote_task]]
+        platform = $(cat ${CYLC_WORKFLOW_RUN_DIR}/pretend-hall-info)
+
+    [[toggler]]
+        script = """
+            # Toggle the platform between localhost and the remote host
+            # using the content of a file, ${CYLC_WORKFLOW_RUN_DIR}/pretend-hall-info.
+            # between localhost and the remote.
+
+            if (( $CYLC_TASK_CYCLE_POINT % 2 == 1 )); then
+                echo ${REMOTE_PLATFORM} > ${CYLC_WORKFLOW_RUN_DIR}/pretend-hall-info
+                cylc message -- "changing platform to ${REMOTE_PLATFORM}"
+            else
+                echo "localhost" > ${CYLC_WORKFLOW_RUN_DIR}/pretend-hall-info
+                cylc message -- "changing platform to localhost"
+            fi
+        """
+        [[[environment]]]
+            REMOTE_PLATFORM = {{ CYLC_TEST_PLATFORM }}

tests/functional/platforms/13-ping-pong-host.t

@@ -0,0 +1,41 @@
+#!/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/>.
+#-----------------------------------------------------------------------------
+
+# If a task has a platform set using a subshell this should be evaluated
+# every time the task is run.
+# https://github.com/cylc/cylc-flow/issues/6808
+export REQUIRE_PLATFORM='loc:remote'
+
+. "$(dirname "$0")/test_header"
+
+set_test_number 3
+
+install_workflow "${TEST_NAME_BASE}" "${TEST_NAME_BASE}"
+
+workflow_run_ok "${TEST_NAME_BASE}-run" \
+    cylc play "${WORKFLOW_NAME}" --debug --no-detach
+
+named_grep_ok "1/remote_task submits to ${CYLC_TEST_PLATFORM}" \
+    "\[1/remote_task/01:preparing\] submitted to ${CYLC_TEST_HOST}" \
+    "${WORKFLOW_RUN_DIR}/log/scheduler/log"
+
+named_grep_ok "2/remote_task submits to localhost" \
+    "\[2/remote_task/01:preparing\] submitted to localhost" \
+    "${WORKFLOW_RUN_DIR}/log/scheduler/log"
+
+purge

tests/functional/platforms/13-ping-pong-host/flow.cylc

@@ -0,0 +1,32 @@
+#!jinja2
+[scheduler]
+    [[events]]
+        stall timeout = PT0S
+
+[scheduling]
+    cycling mode = integer
+    final cycle point = 2
+    [[graph]]
+        P1 = remote_task[-P1] => toggler => remote_task
+
+[runtime]
+    [[remote_task]]
+        [[[remote]]]
+            host = $(cat ${CYLC_WORKFLOW_RUN_DIR}/pretend-hall-info)
+
+    [[toggler]]
+        script = """
+            # Toggle the platform between localhost and the remote host
+            # using the content of a file, ${CYLC_WORKFLOW_RUN_DIR}/pretend-hall-info.
+            # between localhost and the remote.
+
+            if (( $CYLC_TASK_CYCLE_POINT % 2 == 1 )); then
+                echo ${REMOTE_HOST} > ${CYLC_WORKFLOW_RUN_DIR}/pretend-hall-info
+                cylc message -- "changing platform to ${REMOTE_HOST}"
+            else
+                echo "localhost" > ${CYLC_WORKFLOW_RUN_DIR}/pretend-hall-info
+                cylc message -- "changing platform to localhost"
+            fi
+        """
+        [[[environment]]]
+            REMOTE_HOST = {{ CYLC_TEST_HOST }}

tests/unit/test_platforms.py

@@ -273,6 +273,16 @@ def test_platform_name_from_job_info_basic(job, remote, returns):
     assert platform_name_from_job_info(PLATFORMS, job, remote) == returns
 
 
+def test_platform_name_from_job_info_evaluated_hostname():
+    result = platform_name_from_job_info(
+        PLATFORMS,
+        {'batch system': 'background'},
+        {'host': '$(cat tiddles)'},
+        evaluated_host='hpc2',
+    )
+    assert result == 'hpc2-bg'
+
+
 def test_platform_name_from_job_info_ordered_dict_comparison():
     """Check that we are only comparing set items in OrderedDictWithDefaults.
     """