Rsync configurable (#4736)

* Configurable rsync for file installation Test config rsync * Update Documentation * Fix remote slow file install test * Review Feedback Co-authored-by: Oliver Sanders <[email protected]> Co-authored-by: Oliver Sanders <[email protected]>
cylc · Mar 22, 2022 · 67178a9 · 67178a9
1 parent 8ce3f8d
commit 67178a9
Show file tree

Hide file tree

Showing 10 changed files with 106 additions and 57 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -33,6 +33,11 @@ ones in. -->
 
 Second Release Candidate for Cylc 8 suitable for acceptance testing.
 
+### Enhancements
+
+[#4736](https://github.com/cylc/cylc-flow/pull/4736) - rsync command used for
+remote file installation is now configurable.
+
 ### Fixes
 
 [#4703](https://github.com/cylc/cylc-flow/pull/4703) - Fix `ImportError` when

diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py
@@ -1037,6 +1037,13 @@
                 with some initial options or a command that implements a
                 similar interface to ``ssh``.
             ''')
+            Conf('rsync command',
+                 VDR.V_STRING,
+                 'rsync',
+                 desc='''
+                Command used for remote file installation. This supports POSIX
+                compliant rsync implementation e.g. GNU or BSD.
+            ''')
             Conf('use login shell', VDR.V_BOOLEAN, True, desc='''
                 Whether to use a login shell or not for remote command
                 invocation.

diff --git a/cylc/flow/remote.py b/cylc/flow/remote.py
@@ -195,16 +195,17 @@ def construct_rsync_over_ssh_cmd(
     """
     dst_host = get_host_from_platform(platform, bad_hosts=bad_hosts)
     ssh_cmd = platform['ssh command']
-    rsync_cmd = [
-        "rsync",
+    command = platform['rsync command']
+    rsync_cmd = shlex.split(command)
+    rsync_options = [
         "--delete",
         "--rsh=" + ssh_cmd,
         "--include=/.service/",
         "--include=/.service/server.key"
     ] + DEFAULT_RSYNC_OPTS
     # 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.
-
+    rsync_cmd.extend(rsync_options)
     for exclude in ['log', 'share', 'work']:
         rsync_cmd.append(f"--exclude={exclude}")
     default_includes = [

diff --git a/cylc/flow/subprocpool.py b/cylc/flow/subprocpool.py
@@ -554,8 +554,14 @@ def rsync_255_fail(ctx, platform=None) -> bool:
         the failures may be caused by other problems.
         """
         rsync_255_fail = False
+        platform_rsync_cmd = (
+            platform['rsync command']
+            if platform is not None
+            else 'rsync'
+        )
+        rsync_cmd = shlex.split(platform_rsync_cmd)
         if (
-            ctx.cmd[0] == 'rsync'
+            ctx.cmd[0] == rsync_cmd[0]
             and ctx.ret_code not in [0, 255]
             and is_remote_host(ctx.host)
         ):
@@ -575,7 +581,7 @@ def rsync_255_fail(ctx, platform=None) -> bool:
             if ssh_test.returncode == 255:
                 rsync_255_fail = True
         elif (
-            ctx.cmd[0] == 'rsync'
+            ctx.cmd[0] == rsync_cmd[0]
             and ctx.ret_code == 255
         ):
             rsync_255_fail = True

diff --git a/tests/functional/remote/01-file-install.t b/tests/functional/remote/01-file-install.t
@@ -19,7 +19,7 @@
 
 export REQUIRE_PLATFORM='loc:remote comms:?(tcp|ssh)'
 . "$(dirname "$0")/test_header"
-set_test_number 8
+set_test_number 6
 
 create_files () {
     # dump some files into the run dir
@@ -105,52 +105,5 @@ ${RUN_DIR_REL}/file2
 ${RUN_DIR_REL}/lib/moo
 __OUT__
 
-purge
-# -----------------------------------------------------------------------------
-
-if ! command -v xfs_mkfile; then
-    skip 2 "xfs_mkfile not installed"
-    exit
-fi
-
-# Test file install completes before dependent tasks are executed
-TEST_NAME="${TEST_NAME_BASE}-installation-timing"
-init_workflow "${TEST_NAME}" <<__FLOW_CONFIG__
-[scheduler]
-    install = dir1/, dir2/
-    [[events]]
-        abort on stall timeout = true
-        stall timeout = PT0S
-        abort on inactivity timeout = true
-
-[scheduling]
-    [[graph]]
-        R1 = olaf => sven
-
-[runtime]
-    [[olaf]]
-        # task dependent on file install already being complete
-        script = cat \${CYLC_WORKFLOW_RUN_DIR}/dir1/moo
-        platform = $CYLC_TEST_PLATFORM
-
-    [[sven]]
-        # task dependent on file install already being complete
-        script = rm -r \${CYLC_WORKFLOW_RUN_DIR}/dir1 \${CYLC_WORKFLOW_RUN_DIR}/dir2
-        platform = $CYLC_TEST_PLATFORM
-
-__FLOW_CONFIG__
-
-# This generates a large file, ready for the file install. The aim is
-# to slow rsync and ensure tasks do not start until file install has
-# completed.
-for DIR in "dir1" "dir2"; do
-    mkdir -p "${WORKFLOW_RUN_DIR}/${DIR}"
-    xfs_mkfile 1024m "${WORKFLOW_RUN_DIR}/${DIR}/moo"
-done
-
-run_ok "${TEST_NAME}-validate" cylc validate "${WORKFLOW_NAME}"
-workflow_run_ok "${TEST_NAME}-run" \
-    cylc play --debug --no-detach "${WORKFLOW_NAME}"
-
 purge
 exit
diff --git a/tests/functional/remote/07-slow-file-install.t b/tests/functional/remote/07-slow-file-install.t
@@ -0,0 +1,45 @@
+#!/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/>.
+#-------------------------------------------------------------------------------
+# Test file install completes before dependent tasks are executed
+
+export REQUIRE_PLATFORM='loc:remote comms:?(tcp|ssh)'
+. "$(dirname "$0")/test_header"
+set_test_number 2
+create_test_global_config "" "
+[platforms]
+    [[${CYLC_TEST_PLATFORM}]] 
+        rsync command = my-rsync.sh
+"
+TEST_NAME="${TEST_NAME_BASE}-installation-timing"
+install_workflow "${TEST_NAME}" "${TEST_NAME_BASE}"
+
+for DIR in "dir1" "dir2"; do
+    mkdir -p "${WORKFLOW_RUN_DIR}/${DIR}"
+    echo "hello" > "${WORKFLOW_RUN_DIR}/${DIR}/moo"
+    done
+
+run_ok "${TEST_NAME}-validate" cylc validate "${WORKFLOW_NAME}"
+export PATH="${WORKFLOW_RUN_DIR}/bin:$PATH"
+# shellcheck disable=SC2029
+ssh -n "${CYLC_TEST_HOST}" "mkdir -p 'cylc-run/${WORKFLOW_NAME}/'"
+rsync -a 'bin' "${CYLC_TEST_HOST}:cylc-run/${WORKFLOW_NAME}/"
+workflow_run_ok "${TEST_NAME}-run" \
+    cylc play --debug --no-detach "${WORKFLOW_NAME}"
+
+purge
+exit
diff --git a/tests/functional/remote/07-slow-file-install/bin/my-rsync.sh b/tests/functional/remote/07-slow-file-install/bin/my-rsync.sh
@@ -0,0 +1,3 @@
+#!/usr/bin/env bash
+sleep 30
+exec rsync "$@"
diff --git a/tests/functional/remote/07-slow-file-install/flow.cylc b/tests/functional/remote/07-slow-file-install/flow.cylc
@@ -0,0 +1,25 @@
+[scheduler]
+    install = dir1/, dir2/
+    [[events]]
+        abort on stall timeout = true
+        stall timeout = PT0S
+        abort on inactivity timeout = true
+
+[scheduling]
+    [[graph]]
+        R1 = olaf => sven
+
+[runtime]
+    [[olaf]]
+        # task dependent on file install already being complete
+        script = """
+            cat ${CYLC_WORKFLOW_RUN_DIR}/dir1/moo
+        """
+        platform = $CYLC_TEST_PLATFORM
+
+    [[sven]]
+        # task dependent on file install already being complete
+        script = """
+            rm -r ${CYLC_WORKFLOW_RUN_DIR}/dir1 ${CYLC_WORKFLOW_RUN_DIR}/dir2
+        """
+        platform = $CYLC_TEST_PLATFORM
diff --git a/tests/unit/test_remote.py b/tests/unit/test_remote.py
@@ -55,6 +55,7 @@ def test_construct_rsync_over_ssh_cmd():
         '/foo',
         '/bar',
         {
+            'rsync command': 'rsync command',
             'hosts': ['miklegard'],
             'ssh command': 'strange_ssh',
             'selection': {'method': 'definition order'},
@@ -63,7 +64,7 @@ def test_construct_rsync_over_ssh_cmd():
     )
     assert host == 'miklegard'
     assert ' '.join(cmd) == (
-        'rsync --delete --rsh=strange_ssh --include=/.service/ '
+        'rsync command --delete --rsh=strange_ssh --include=/.service/ '
         '--include=/.service/server.key -a --checksum '
         '--out-format=%o %n%L --no-t --exclude=log --exclude=share '
         '--exclude=work --include=/app/*** --include=/bin/*** '

diff --git a/tests/unit/test_subprocpool.py b/tests/unit/test_subprocpool.py
@@ -309,11 +309,12 @@ def test__run_command_exit_rsync_fails(mock_ctx):
         bad_hosts=badhosts,
         callback=print,
         callback_args=[
-            'Welcome to Magrathea',
             {
                 'name': 'Magrathea',
                 'ssh command': 'ssh',
-            }
+                'rsync command': 'rsync command'
+            },
+            'Welcome to Magrathea',
         ]
     )
     assert badhosts == {'foo', 'bar', 'mouse'}
@@ -348,6 +349,8 @@ def test_rsync_255_fail(mock_ctx, expect, ctx_kwargs):
     """
     output = SubProcPool.rsync_255_fail(
         mock_ctx(**ctx_kwargs),
-        {'ssh command': 'ssh'}
+        {'ssh command': 'ssh',
+        'rsync command': 'rsync command'
+        }
     )
     assert output == expect