Skip to content

Commit 9df0e3f

Browse files
committed
ssh: move logic about background processes to VM.start_background_process
We likely never need to launch a background process that way on a windows machine that is not a VM. Encapsulating the behaviour at this place will let us stop having commands.ssh return type depend on `target_os`, which cannot be properly expressed with type hints because of its non-literal nature. Signed-off-by: Yann Dirson <[email protected]>
1 parent da3bf33 commit 9df0e3f

File tree

2 files changed

+10
-10
lines changed

2 files changed

+10
-10
lines changed

lib/commands.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,11 @@ def _ssh(hostname_or_ip, cmd, check, simple_output, suppress_fingerprint_warning
7676
command = cmd
7777
else:
7878
command = " ".join(cmd)
79-
if background and target_os != "windows":
80-
# https://stackoverflow.com/questions/29142/getting-ssh-to-execute-a-command-in-the-background-on-target-machine
81-
# ... and run the command through a bash shell so that output redirection both works on Linux and FreeBSD.
82-
# Bash being available on VMs is a documented requirement.
83-
command = "nohup bash -c \"%s &>/dev/null &\"" % command
8479

8580
ssh_cmd = f"ssh root@{hostname_or_ip} {' '.join(opts)} {shlex.quote(command)}"
8681

87-
windows_background = background and target_os == "windows"
8882
# Fetch banner and remove it to avoid stdout/stderr pollution.
89-
if config.ignore_ssh_banner and not windows_background:
83+
if config.ignore_ssh_banner:
9084
banner_res = subprocess.run(
9185
"ssh root@%s %s '%s'" % (hostname_or_ip, ' '.join(opts), '\n'),
9286
shell=True,
@@ -102,7 +96,7 @@ def _ssh(hostname_or_ip, cmd, check, simple_output, suppress_fingerprint_warning
10296
stdout=subprocess.PIPE,
10397
stderr=subprocess.STDOUT
10498
)
105-
if windows_background:
99+
if background:
106100
return True, process
107101

108102
stdout = []

lib/vm.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,15 @@ def start_background_process(self, cmd):
271271
self.sftp_put(f.name, script.replace("/tmp/", "/Users/root/AppData/Local/Temp/"))
272272
else:
273273
self.scp(f.name, script)
274-
# Use bash to run the script, to avoid being hit by differences between shells, for example on FreeBSD
274+
275+
# https://stackoverflow.com/questions/29142/getting-ssh-to-execute-a-command-in-the-background-on-target-machine
276+
# ... and run the command through a bash shell so that output redirection both works on Linux and FreeBSD.
275277
# It is a documented requirement that bash is present on all test VMs.
276-
self.ssh(['bash', script], background=True)
278+
remote_cmd = f"bash {script}"
279+
if not self.is_windows:
280+
remote_cmd = f'nohup bash -c "{remote_cmd} &>/dev/null &"'
281+
self.ssh(remote_cmd, background=True)
282+
277283
wait_for(lambda: self.ssh_with_result(['test', '-f', pidfile]),
278284
"wait for pid file %s to exist" % pidfile)
279285
pid = self.ssh(['cat', pidfile])

0 commit comments

Comments
 (0)