From 47203266119874167ab494dcb380e5408c8400d8 Mon Sep 17 00:00:00 2001 From: Scott Wales Date: Mon, 28 Aug 2023 16:47:11 +1000 Subject: [PATCH 1/7] Forward arbitrary environment variables over SSH Allow users to specify environment variables that get forwarded from the submit host to the scheduler and task submission. Define variables to forward in `global.cylc` like: ``` [platforms] [[localhost]] ssh forward environment variables = PROJECT, LUSTRE_DISK ``` This will add `PROJECT` and `LUSTRE_DISK` to the list of variables exported in SSH commands to launch the scheduler on remote hosts (if they have been set in the current environment). Once they are available on the scheduler they can further be forwarded to other platforms, where they may interact with the scheduler to set a default project code or be used to set storage paths: ``` [platforms] [[mtu]] ssh forward environment variables = PROJECT, LUSTRE_DISK install target = mtu [install] [[symlink dirs]] [[[mtu]]] share = $LUSTRE_DISK/$PROJECT/$USER ``` --- CONTRIBUTING.md | 2 +- cylc/flow/cfgspec/globalcfg.py | 7 +++++++ cylc/flow/remote.py | 3 ++- tests/unit/test_remote.py | 29 ++++++++++++++++++++++++++++- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6fbc4283968..911dff536ba 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,7 +64,7 @@ requests_). - Prasanna Challuri - David Matthews - Tim Whitcomb - - (Scott Wales) + - Scott Wales - Tomek Trzeciak - Thomas Coleman - Bruno Kinoshita diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index 3c01ff52729..b3a25252c09 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -1416,6 +1416,13 @@ def default_for( {REPLACES}``global.rc[hosts][]copyable environment variables``. ''') + Conf('ssh forward environment variables', VDR.V_STRING_LIST, '', + desc=f''' + A list containing the names of the environment variables to + forward with SSH connections to the server and run hosts + + .. versionchanged:: 8.3.0 + ''') Conf('retrieve job logs', VDR.V_BOOLEAN, desc=f''' {LOG_RETR_SETTINGS['retrieve job logs']} diff --git a/cylc/flow/remote.py b/cylc/flow/remote.py index 62cd77fd9af..8a72d56b6d6 100644 --- a/cylc/flow/remote.py +++ b/cylc/flow/remote.py @@ -298,7 +298,8 @@ def construct_ssh_cmd( 'CYLC_CONF_PATH', 'CYLC_COVERAGE', 'CLIENT_COMMS_METH', - 'CYLC_ENV_NAME' + 'CYLC_ENV_NAME', + *platform['ssh forward environment variables'], ]: if envvar in os.environ: command.append( diff --git a/tests/unit/test_remote.py b/tests/unit/test_remote.py index 7be01de2e20..6e86ac1a410 100644 --- a/tests/unit/test_remote.py +++ b/tests/unit/test_remote.py @@ -15,7 +15,9 @@ # along with this program. If not, see . """Test the cylc.flow.remote module.""" -from cylc.flow.remote import run_cmd, construct_rsync_over_ssh_cmd +from cylc.flow.remote import run_cmd, construct_rsync_over_ssh_cmd, construct_ssh_cmd +from unittest import mock +import cylc.flow def test_run_cmd_stdin_str(): @@ -86,3 +88,28 @@ def test_construct_rsync_over_ssh_cmd(): '/foo/', 'miklegard:/bar/', ] + + +def test_construct_ssh_cmd_forward_env(): + """ Test for 'ssh forward environment variables' + """ + import os + + host = 'example.com' + config = { + 'ssh command': 'ssh', + 'use login shell': None, + 'cylc path': None, + 'ssh forward environment variables': ['FOO'], + } + + # Variable isn't set, no change to command + expect = ['ssh', host, 'env', f'CYLC_VERSION={cylc.flow.__version__}', 'cylc', 'play'] + cmd = construct_ssh_cmd(['play'], config, host) + assert cmd == expect + + # Variable is set, appears in `env` list + with mock.patch.dict(os.environ, {'FOO': 'BAR'}): + expect = ['ssh', host, 'env', f'CYLC_VERSION={cylc.flow.__version__}', 'FOO=BAR', 'cylc', 'play'] + cmd = construct_ssh_cmd(['play'], config, host) + assert cmd == expect From 833f89c5411a60734044b2d948603c0dcdbce2a0 Mon Sep 17 00:00:00 2001 From: Scott Wales Date: Tue, 29 Aug 2023 10:29:43 +1000 Subject: [PATCH 2/7] Flake8 fix --- cylc/flow/cfgspec/globalcfg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index b3a25252c09..2182ee32edb 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -1417,7 +1417,7 @@ def default_for( environment variables``. ''') Conf('ssh forward environment variables', VDR.V_STRING_LIST, '', - desc=f''' + desc=''' A list containing the names of the environment variables to forward with SSH connections to the server and run hosts From 253674559a458dc3764b826432f4f55f25282cfb Mon Sep 17 00:00:00 2001 From: Scott Wales Date: Tue, 29 Aug 2023 11:03:40 +1000 Subject: [PATCH 3/7] Add changelog --- changes.d/5709.feat.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes.d/5709.feat.md diff --git a/changes.d/5709.feat.md b/changes.d/5709.feat.md new file mode 100644 index 00000000000..11aeabcf81d --- /dev/null +++ b/changes.d/5709.feat.md @@ -0,0 +1 @@ +Forward arbitrary environment variables over SSH connections From 9707f0a77e79da6798026241af4932b1d86ddfe8 Mon Sep 17 00:00:00 2001 From: Scott Wales Date: Wed, 13 Sep 2023 16:37:31 +1000 Subject: [PATCH 4/7] Move config option to under 'run hosts' --- cylc/flow/cfgspec/globalcfg.py | 15 ++++++++------- cylc/flow/remote.py | 5 ++++- tests/unit/test_remote.py | 12 ++++++++++-- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index 2182ee32edb..1b97e8258c0 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -784,6 +784,14 @@ def default_for( {REPLACES}``[suite servers][run host select]rank``. ''') + Conf('ssh forward environment variables', VDR.V_STRING_LIST, '', + desc=''' + A list containing the names of the environment variables to + forward with SSH connections to the run and job hosts from + the host running 'cylc play' + + .. versionchanged:: 8.3.0 + ''') with Conf('host self-identification', desc=f''' How Cylc determines and shares the identity of the workflow host. @@ -1416,13 +1424,6 @@ def default_for( {REPLACES}``global.rc[hosts][]copyable environment variables``. ''') - Conf('ssh forward environment variables', VDR.V_STRING_LIST, '', - desc=''' - A list containing the names of the environment variables to - forward with SSH connections to the server and run hosts - - .. versionchanged:: 8.3.0 - ''') Conf('retrieve job logs', VDR.V_BOOLEAN, desc=f''' {LOG_RETR_SETTINGS['retrieve job logs']} diff --git a/cylc/flow/remote.py b/cylc/flow/remote.py index 8a72d56b6d6..5d9ed7c3937 100644 --- a/cylc/flow/remote.py +++ b/cylc/flow/remote.py @@ -35,6 +35,7 @@ from cylc.flow.option_parsers import verbosity_to_opts from cylc.flow.platforms import get_platform, get_host_from_platform from cylc.flow.util import format_cmd +from cylc.flow.cfgspec.glbl_cfg import glbl_cfg def get_proc_ancestors(): @@ -299,7 +300,9 @@ def construct_ssh_cmd( 'CYLC_COVERAGE', 'CLIENT_COMMS_METH', 'CYLC_ENV_NAME', - *platform['ssh forward environment variables'], + *(glbl_cfg().get(['scheduler']) + ['run hosts'] + ['ssh forward environment variables']), ]: if envvar in os.environ: command.append( diff --git a/tests/unit/test_remote.py b/tests/unit/test_remote.py index 6e86ac1a410..c3c0c1ca9ff 100644 --- a/tests/unit/test_remote.py +++ b/tests/unit/test_remote.py @@ -90,17 +90,25 @@ def test_construct_rsync_over_ssh_cmd(): ] -def test_construct_ssh_cmd_forward_env(): +def test_construct_ssh_cmd_forward_env(mock_glbl_cfg): """ Test for 'ssh forward environment variables' """ import os + mock_glbl_cfg( + 'cylc.flow.remote.glbl_cfg', + ''' + [scheduler] + [[run hosts]] + ssh forward environment variables = FOO, BAR + ''' + ) + host = 'example.com' config = { 'ssh command': 'ssh', 'use login shell': None, 'cylc path': None, - 'ssh forward environment variables': ['FOO'], } # Variable isn't set, no change to command From 8ff3082d7641afe822ce7ae1fc9e962bb26cdf91 Mon Sep 17 00:00:00 2001 From: Scott Wales Date: Wed, 27 Sep 2023 14:28:31 +1000 Subject: [PATCH 5/7] Remove reference to job host --- cylc/flow/cfgspec/globalcfg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index 1b97e8258c0..8038d374988 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -787,7 +787,7 @@ def default_for( Conf('ssh forward environment variables', VDR.V_STRING_LIST, '', desc=''' A list containing the names of the environment variables to - forward with SSH connections to the run and job hosts from + forward with SSH connections to the workflow host from the host running 'cylc play' .. versionchanged:: 8.3.0 From 8053ead4dfb3e06db49338083176c8d0444ef4fa Mon Sep 17 00:00:00 2001 From: Scott Wales Date: Thu, 28 Sep 2023 13:49:05 +1000 Subject: [PATCH 6/7] Fix metadata --- cylc/flow/cfgspec/globalcfg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index 8038d374988..d42415d2a9c 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -790,7 +790,7 @@ def default_for( forward with SSH connections to the workflow host from the host running 'cylc play' - .. versionchanged:: 8.3.0 + .. versionadded:: 8.3.0 ''') with Conf('host self-identification', desc=f''' From ac071c6fb976f8e059ca196df9dad092e5eecf71 Mon Sep 17 00:00:00 2001 From: Scott Wales Date: Fri, 27 Oct 2023 14:22:43 +1100 Subject: [PATCH 7/7] Set forward variables by platform --- cylc/flow/cfgspec/globalcfg.py | 16 ++++++++-------- cylc/flow/remote.py | 5 +---- tests/unit/cfgspec/test_globalcfg.py | 10 ++++++++++ tests/unit/test_remote.py | 12 ++---------- 4 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cylc/flow/cfgspec/globalcfg.py b/cylc/flow/cfgspec/globalcfg.py index d42415d2a9c..e07db4c140b 100644 --- a/cylc/flow/cfgspec/globalcfg.py +++ b/cylc/flow/cfgspec/globalcfg.py @@ -784,14 +784,6 @@ def default_for( {REPLACES}``[suite servers][run host select]rank``. ''') - Conf('ssh forward environment variables', VDR.V_STRING_LIST, '', - desc=''' - A list containing the names of the environment variables to - forward with SSH connections to the workflow host from - the host running 'cylc play' - - .. versionadded:: 8.3.0 - ''') with Conf('host self-identification', desc=f''' How Cylc determines and shares the identity of the workflow host. @@ -1640,6 +1632,14 @@ def default_for( .. versionadded:: 8.0.0 ''') + Conf('ssh forward environment variables', VDR.V_STRING_LIST, '', + desc=''' + A list containing the names of the environment variables to + forward with SSH connections to the workflow host from + the host running 'cylc play' + + .. versionadded:: 8.3.0 + ''') with Conf('selection', desc=''' How to select a host from the list of platform hosts. diff --git a/cylc/flow/remote.py b/cylc/flow/remote.py index 5d9ed7c3937..8a72d56b6d6 100644 --- a/cylc/flow/remote.py +++ b/cylc/flow/remote.py @@ -35,7 +35,6 @@ from cylc.flow.option_parsers import verbosity_to_opts from cylc.flow.platforms import get_platform, get_host_from_platform from cylc.flow.util import format_cmd -from cylc.flow.cfgspec.glbl_cfg import glbl_cfg def get_proc_ancestors(): @@ -300,9 +299,7 @@ def construct_ssh_cmd( 'CYLC_COVERAGE', 'CLIENT_COMMS_METH', 'CYLC_ENV_NAME', - *(glbl_cfg().get(['scheduler']) - ['run hosts'] - ['ssh forward environment variables']), + *platform['ssh forward environment variables'], ]: if envvar in os.environ: command.append( diff --git a/tests/unit/cfgspec/test_globalcfg.py b/tests/unit/cfgspec/test_globalcfg.py index 2becda1caad..6db8d76dcf8 100644 --- a/tests/unit/cfgspec/test_globalcfg.py +++ b/tests/unit/cfgspec/test_globalcfg.py @@ -148,3 +148,13 @@ def test_source_dir_validation( assert "must be an absolute path" in str(excinfo.value) else: glblcfg.load() + +def test_platform_ssh_forward_variables(mock_global_config): + + glblcfg: GlobalConfig = mock_global_config(''' + [platforms] + [[foo]] + ssh forward environment variables = "FOO", "BAR" + ''') + + assert glblcfg.get(['platforms','foo','ssh forward environment variables']) == ["FOO", "BAR"] diff --git a/tests/unit/test_remote.py b/tests/unit/test_remote.py index c3c0c1ca9ff..8d24fc0bb3a 100644 --- a/tests/unit/test_remote.py +++ b/tests/unit/test_remote.py @@ -90,25 +90,17 @@ def test_construct_rsync_over_ssh_cmd(): ] -def test_construct_ssh_cmd_forward_env(mock_glbl_cfg): +def test_construct_ssh_cmd_forward_env(): """ Test for 'ssh forward environment variables' """ import os - mock_glbl_cfg( - 'cylc.flow.remote.glbl_cfg', - ''' - [scheduler] - [[run hosts]] - ssh forward environment variables = FOO, BAR - ''' - ) - host = 'example.com' config = { 'ssh command': 'ssh', 'use login shell': None, 'cylc path': None, + 'ssh forward environment variables': ['FOO', 'BAZ'], } # Variable isn't set, no change to command