Respond to feedback from HO

cylc/flow/cfgspec/suite.py

@@ -85,7 +85,7 @@
         ''')
 
     with Conf('scheduler'):
-        Conf('includes', VDR.V_STRING_LIST, desc='''
+        Conf('install', VDR.V_STRING_LIST, desc='''
             Configure the directories and files to be included in the remote
             file installation.
 
@@ -117,7 +117,7 @@
             .. code-block:: cylc
 
                 [scheduler]
-                    includes = dir/, dir2/, file1, file2
+                    install = dir/, dir2/, file1, file2
                 ''')
 
     with Conf('cylc'):

cylc/flow/config.py

@@ -227,7 +227,7 @@ def __init__(
         self.cfg = self.pcfg.get(sparse=True)
         self.mem_log("config.py: after get(sparse=True)")
 
-        if 'scheduler' in self.cfg and 'includes' in self.cfg['scheduler']:
+        if 'scheduler' in self.cfg and 'install' in self.cfg['scheduler']:
             self.get_validated_rsync_includes()
 
         # First check for the essential scheduling section.
@@ -2340,18 +2340,15 @@ def get_expected_failed_tasks(self):
             return None
 
     def get_validated_rsync_includes(self):
-        """Validate and return items configured to be included in the file
-            installation"""
-        includes = self.cfg['scheduler']['includes']
-        if includes:
-            illegal_includes = []
-            for include in includes:
-                if include.count("/") > 1:
-                    illegal_includes.append(f"{include}")
-            if len(illegal_includes) > 0:
-                raise SuiteConfigError(
-                    "Error in [scheduler] includes. "
-                    "Directories can only be from the top level, please "
-                    "reconfigure:" + str(illegal_includes)[1:-1])
-            else:
-                return includes
+        """Validate and return items to be included in the file installation"""
+        includes = self.cfg['scheduler']['install']
+        illegal_includes = []
+        for include in includes:
+            if include.count("/") > 1:
+                illegal_includes.append(f"{include}")
+        if len(illegal_includes) > 0:
+            raise SuiteConfigError(
+                "Error in [scheduler] install. "
+                "Directories can only be from the top level, please "
+                "reconfigure:" + str(illegal_includes)[1:-1])
+        return includes

cylc/flow/hostuserutil.py

@@ -227,19 +227,6 @@ def _is_remote_platform(self, platform):
                 return True
         return False
 
-    def _is_remote_install_target(self, install_target):
-        """Determines whether install_target is remote or not.
-        Return True if install target has different IP address
-        to the current host.
-
-        Return False if install target is to be treated as localhost.
-        """
-
-        if is_remote_host(install_target) is True:
-            return True
-        else:
-            return False
-
 
 def get_host_ip_by_name(target):
     """Shorthand for HostUtil.get_inst().get_host_ip_by_name(target)."""
@@ -276,11 +263,6 @@ def is_remote_platform(platform):
     return HostUtil.get_inst()._is_remote_platform(platform)
 
 
-def is_remote_install_target(install_target):
-    """Shorthand for get_inst()._is_remote_install_target(install_target)."""
-    return HostUtil.get_inst()._is_remote_install_target(install_target)
-
-
 def is_remote_host(name):
     """Shorthand for HostUtil.get_inst().is_remote_host(name)."""
     return HostUtil.get_inst().is_remote_host(name)

cylc/flow/pathutil.py

@@ -74,7 +74,7 @@ def get_suite_run_log_name(suite):
 
 
 def get_suite_file_install_log_name(suite):
-    """Return suite run log file path."""
+    """Return suite file install log file path."""
     path = get_suite_run_dir(suite, 'log', 'suite', 'file-installation-log')
     return expandvars(path)
 

cylc/flow/platforms.py

@@ -408,6 +408,13 @@ def get_install_target_from_platform(platform):
     Returns install target."""
 
     if not platform['install target']:
-        platform['install target'] = platform.get('name')
+        platform['install target'] = platform['name']
 
     return platform.get('install target')
+
+
+def is_platform_with_target_in_list(
+        install_target, distinct_platforms_list):
+    """Determines whether install target is in the list of platforms"""
+    for distinct_platform in distinct_platforms_list:
+        return install_target == distinct_platform['install target']

cylc/flow/remote.py

@@ -156,13 +156,11 @@ def construct_platform_ssh_cmd(raw_cmd, platform, **kwargs):
 
 
 def get_includes_to_rsync(rsync_includes=None):
-    """Returns a list of directories/files, configured in flow.cylc,
-        to be included in the remote file installation.
-    """
+    """Returns list of configured dirs/files for remote file installation."""
 
     configured_includes = []
 
-    if rsync_includes:
+    if rsync_includes is not None:
         for include in rsync_includes:
             if include.endswith("/"):  # item is a directory
                 configured_includes.append("/" + include + "***")
@@ -205,8 +203,7 @@ def construct_rsync_over_ssh_cmd(
     # Note to future devs - be wary of changing the order of the following
     # rsync options, rsync is very particular about order of in/ex-cludes.
 
-    excludes = ['log', 'share', 'work']
-    for exclude in excludes:
+    for exclude in ['log', 'share', 'work']:
         rsync_cmd.append(f"--exclude={exclude}")
     default_includes = [
         '/app/***',
@@ -215,8 +212,7 @@ def construct_rsync_over_ssh_cmd(
         '/lib/***']
     for include in default_includes:
         rsync_cmd.append(f"--include={include}")
-    configured_includes = get_includes_to_rsync(rsync_includes)
-    for include in configured_includes:
+    for include in get_includes_to_rsync(rsync_includes):
         rsync_cmd.append(f"--include={include}")
     # The following excludes are required in case these are added to the
     rsync_cmd.append("--exclude=*")  # exclude everything else

cylc/flow/scheduler.py

@@ -72,6 +72,10 @@
     get_suite_test_log_name,
     make_suite_run_tree,
 )
+from cylc.flow.platforms import (
+    get_install_target_from_platform,
+    get_platform,
+    is_platform_with_target_in_list)
 from cylc.flow.profiler import Profiler
 from cylc.flow.resources import extract_resources
 from cylc.flow.subprocpool import SubProcPool
@@ -101,9 +105,6 @@
     get_time_string_from_unix_time as time2str,
     get_utc_mode)
 from cylc.flow.xtrigger_mgr import XtriggerManager
-from cylc.flow.platforms import (
-    get_install_target_from_platform,
-    get_platform)
 
 
 class SchedulerStop(CylcError):
@@ -722,14 +723,6 @@ def restart_remote_init(self):
 
         """
 
-        def is_platform_with_target_in_list(
-                install_target, distinct_platforms_list):
-            """Determines whether install target is in the list of platforms"""
-            for distinct_platform in distinct_platforms_list:
-                if(install_target
-                   == distinct_platform['install target']):
-                    return True
-
         distinct_install_target_platforms = []
 
         for itask in self.pool.get_rh_tasks():
@@ -750,7 +743,7 @@ def is_platform_with_target_in_list(
                     platform, self.curve_auth,
                     self.client_pub_key_dir) is None):
                 incomplete_init = True
-
+                break
         if incomplete_init:
             # TODO: Review whether this sleep is needed.
             sleep(1.0)

cylc/flow/task_remote_mgr.py

@@ -33,9 +33,7 @@
 from cylc.flow import LOG, RSYNC_LOG
 from cylc.flow.exceptions import TaskRemoteMgmtError
 import cylc.flow.flags
-from cylc.flow.hostuserutil import (
-    is_remote_host, is_remote_install_target, is_remote_platform
-)
+from cylc.flow.hostuserutil import (is_remote_host, is_remote_platform)
 from cylc.flow.pathutil import (
     get_remote_suite_run_dir,
     get_suite_run_dir)
@@ -160,7 +158,8 @@ def remote_init(self, platform, curve_auth,
             client_pub_key_dir (str):
                 Client public key directory, used by the ZMQ authenticator.
             platform (dict):
-                A dictionary containing information about platform.
+                A dictionary containing settings relating to platform used in
+                this remote installation.
 
         Return:
             REMOTE_INIT_NOT_REQUIRED:
@@ -178,7 +177,7 @@ def remote_init(self, platform, curve_auth,
 
         # If task is running locally we can skip the rest of this function
         if (self.single_task_mode or
-                not is_remote_install_target(self.install_target)):
+                not is_remote_host(get_host_from_platform(platform))):
             LOG.debug(f"REMOTE INIT NOT REQUIRED for {self.install_target}")
             return REMOTE_INIT_NOT_REQUIRED
 

tests/functional/cylc-ping/05-check-keys-sharedfs.t

@@ -23,7 +23,7 @@ require_remote_platform_wsfs
 export CYLC_TEST_PLATFORM="$CYLC_TEST_PLATFORM_WSFS"
 set_test_number 4
 
-init_suite "${TEST_NAME_BASE}" <<'__SUITE_RC__'
+init_suite "${TEST_NAME_BASE}" <<'__FLOW_CYLC__'
 #!jinja2
 [cylc]
 [scheduling]
@@ -35,7 +35,7 @@ init_suite "${TEST_NAME_BASE}" <<'__SUITE_RC__'
         platform = {{CYLC_TEST_PLATFORM}}
     [[held]]
         script = true
-__SUITE_RC__
+__FLOW_CYLC__
 
 run_ok "${TEST_NAME_BASE}-validate" cylc validate "${SUITE_NAME}" \
     -s "CYLC_TEST_PLATFORM=${CYLC_TEST_PLATFORM}"
@@ -44,7 +44,7 @@ suite_run_ok "${TEST_NAME_BASE}-run" cylc run "${SUITE_NAME}" \
 RRUND="cylc-run/${SUITE_NAME}"
 RSRVD="${RRUND}/.service"
 poll_grep_suite_log 'Holding all waiting or queued tasks now'
-SSH='ssh -n -oBatchMode=yes -oConnectTimeout=5'
+SSH="$(cylc get-global-config -i "[platforms][$CYLC_TEST_PLATFORM]ssh command")"
 ${SSH} "${CYLC_TEST_PLATFORM}" \
 find "${RSRVD}" -type f -name "*key*"|awk -F/ '{print $NF}'|sort >'find.out'
 cmp_ok 'find.out' <<'__OUT__'

tests/functional/remote/01-basic-file-install.t → tests/functional/remote/01-file-install.t

@@ -15,44 +15,22 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #-------------------------------------------------------------------------------
-# Checks default files (app, bin, etc, lib) are correctly installed on the
-# remote platform.
+# Checks configured files/directories along with default files/directories
+# (app, bin, etc, lib) are correctly installed on the remote platform.
 
 export CYLC_TEST_IS_GENERIC=false
 . "$(dirname "$0")/test_header"
 require_remote_platform
-set_test_number 3
-
-init_suite "${TEST_NAME_BASE}" <<'__SUITE_RC__'
-#!jinja2
-[cylc]
-[scheduling]
-    [[graph]]
-        R1 = startup => holder => held
-[runtime]
-    [[startup]]
-        script = """
-        for DIR in "bin" "app" "etc" "lib"
-    do
-        mkdir -p "${CYLC_SUITE_RUN_DIR}/${DIR}"
-        touch "${CYLC_SUITE_RUN_DIR}/${DIR}/moo"
-    done
-    """
-        platform = localhost
-    [[holder]]
-        script = """cylc hold "${CYLC_SUITE_NAME}" """
-        platform = {{CYLC_TEST_PLATFORM}}
-    [[held]]
-        script = true
-__SUITE_RC__
+set_test_number 6
+install_suite "${TEST_NAME_BASE}"
 
 run_ok "${TEST_NAME_BASE}-validate" cylc validate "${SUITE_NAME}" \
     -s "CYLC_TEST_PLATFORM=${CYLC_TEST_PLATFORM}"
-suite_run_ok "${TEST_NAME_BASE}-run" cylc run "${SUITE_NAME}" \
+suite_run_ok "${TEST_NAME_BASE}-run1" cylc run "${SUITE_NAME}" \
     -s "CYLC_TEST_PLATFORM=${CYLC_TEST_PLATFORM}"
 RRUND="cylc-run/${SUITE_NAME}"
 poll_grep_suite_log 'Holding all waiting or queued tasks now'
-SSH='ssh -n -oBatchMode=yes -oConnectTimeout=10' 
+SSH="$(cylc get-global-config -i "[platforms][$CYLC_TEST_PLATFORM]ssh command")"
 ${SSH} "${CYLC_TEST_PLATFORM}" \
 find "${RRUND}/"{app,bin,etc,lib} -type f | sort > 'find.out'
 cmp_ok 'find.out'  <<__OUT__
@@ -63,6 +41,34 @@ ${RRUND}/lib/moo
 __OUT__
 
 cylc stop --max-polls=60 --interval=1 "${SUITE_NAME}"
+
 purge_suite_platform "${CYLC_TEST_PLATFORM}" "${SUITE_NAME}"
 purge_suite "${SUITE_NAME}"
+
+install_suite "${TEST_NAME_BASE}"
+
+export SECOND_RUN="dir1/, dir2/, file1, file2"
+run_ok "${TEST_NAME_BASE}-validate" cylc validate "${SUITE_NAME}" \
+    -s "CYLC_TEST_PLATFORM=${CYLC_TEST_PLATFORM}" -s "SECOND_RUN=${SECOND_RUN}"
+suite_run_ok "${TEST_NAME_BASE}-run2" cylc run "${SUITE_NAME}" \
+    -s "CYLC_TEST_PLATFORM=${CYLC_TEST_PLATFORM}" -s "SECOND_RUN=${SECOND_RUN}"
+poll_grep_suite_log 'Holding all waiting or queued tasks now'
+${SSH} "${CYLC_TEST_PLATFORM}" \
+find "${RRUND}/"{app,bin,dir1,dir2,file1,file2,etc,lib} -type f | sort > 'find.out'
+cmp_ok 'find.out'  <<__OUT__
+${RRUND}/app/moo
+${RRUND}/bin/moo
+${RRUND}/dir1/moo
+${RRUND}/dir2/moo
+${RRUND}/etc/moo
+${RRUND}/file1
+${RRUND}/file2
+${RRUND}/lib/moo
+__OUT__
+
+cylc stop --max-polls=60 --interval=1 "${SUITE_NAME}"
+
+purge_suite_platform "${CYLC_TEST_PLATFORM}" "${SUITE_NAME}"
+purge_suite "${SUITE_NAME}"
+
 exit

tests/functional/remote/01-file-install/flow.cylc

@@ -0,0 +1,33 @@
+#!jinja2
+[cylc]
+
+{% if SECOND_RUN is defined %}
+
+[scheduler]
+    install = {{ SECOND_RUN }}
+
+{% endif %}
+
+[scheduling]
+    [[graph]]
+        R1 = startup => holder => held
+[runtime]
+    [[startup]]
+        script = """
+        for DIR in "bin" "app" "etc" "lib" "dir1" "dir2"
+    do
+        mkdir -p "${CYLC_SUITE_RUN_DIR}/${DIR}"
+        touch "${CYLC_SUITE_RUN_DIR}/${DIR}/moo"
+    done
+
+    for FILE in "file1" "file2"
+    do
+        touch "${CYLC_SUITE_RUN_DIR}/${FILE}"
+    done
+    """
+        platform = localhost
+    [[holder]]
+        script = """cylc hold "${CYLC_SUITE_NAME}" """
+        platform = {{CYLC_TEST_PLATFORM}}
+    [[held]]
+        script = true