<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/18452">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Replace RemoteProcessFixIgnoreSIGHUP with RemoteProcessSafeExit<br><br>The API was doing far more stuff than its name indicated. Even more<br>important stuff, like making sure the process is killed at the end with<br>-9 after ssh connection is dropped.<br><br>Change-Id: If043ecab509b34b0922824d73db916196274ec64<br>---<br>M src/osmo_gsm_tester/core/remote.py<br>M src/osmo_gsm_tester/obj/bts_osmotrx.py<br>M src/osmo_gsm_tester/obj/epc_amarisoft.py<br>M src/osmo_gsm_tester/obj/ms_srs.py<br>4 files changed, 28 insertions(+), 26 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-gsm-tester refs/changes/52/18452/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo_gsm_tester/core/remote.py b/src/osmo_gsm_tester/core/remote.py</span><br><span>index 95b8967..29eea30 100644</span><br><span>--- a/src/osmo_gsm_tester/core/remote.py</span><br><span>+++ b/src/osmo_gsm_tester/core/remote.py</span><br><span>@@ -55,7 +55,7 @@</span><br><span> run_dir = self.run_dir.new_dir(name)</span><br><span> return process.RemoteProcess(name, run_dir, self.user(), self.host(), self.cwd(), popen_args, remote_env=remote_env, **popen_kwargs)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- def generate_wrapper_script(self):</span><br><span style="color: hsl(120, 100%, 40%);">+ def generate_wrapper_script(self, wait_time_sec):</span><br><span> wrapper_script = self.run_dir.new_file(RemoteHost.WRAPPER_SCRIPT)</span><br><span> with open(wrapper_script, 'w') as f:</span><br><span> r = """#!/bin/bash</span><br><span>@@ -66,8 +66,8 @@</span><br><span> if ! kill -0 $mypid; then</span><br><span> return</span><br><span> fi</span><br><span style="color: hsl(0, 100%, 40%);">- echo "sleeping some time waiting for child to die..." >>$LOGFILE</span><br><span style="color: hsl(0, 100%, 40%);">- sleep 5</span><br><span style="color: hsl(120, 100%, 40%);">+ echo "sleeping %d seconds waiting for child to die..." >>$LOGFILE</span><br><span style="color: hsl(120, 100%, 40%);">+ sleep %d</span><br><span> if ! kill -0 $mypid; then</span><br><span> return</span><br><span> fi</span><br><span>@@ -102,20 +102,18 @@</span><br><span> prep_sighandler</span><br><span> $@ &</span><br><span> wait_sighandler</span><br><span style="color: hsl(0, 100%, 40%);">- """</span><br><span style="color: hsl(120, 100%, 40%);">+ """ % (wait_time_sec, wait_time_sec)</span><br><span> f.write(r)</span><br><span> st = os.stat(wrapper_script)</span><br><span> os.chmod(wrapper_script, st.st_mode | stat.S_IEXEC)</span><br><span> return wrapper_script</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- def RemoteProcessFixIgnoreSIGHUP(self, name, remote_dir, popen_args, remote_env={}, **popen_kwargs):</span><br><span style="color: hsl(0, 100%, 40%);">- # Run remotely through ssh. We need to run binary under a wrapper</span><br><span style="color: hsl(0, 100%, 40%);">- # script since osmo-trx ignores SIGHUP and will keep running after</span><br><span style="color: hsl(0, 100%, 40%);">- # we close local ssh session. The wrapper script catches SIGHUP and</span><br><span style="color: hsl(0, 100%, 40%);">- # sends SIGINT to it.</span><br><span style="color: hsl(120, 100%, 40%);">+ def RemoteProcessSafeExit(self, name, remote_dir, popen_args, remote_env={}, wait_time_sec=5, **popen_kwargs):</span><br><span style="color: hsl(120, 100%, 40%);">+ """Run binary under a wrapper which will make sure process is killed -9</span><br><span style="color: hsl(120, 100%, 40%);">+ a few seconds after SIGHUP from SSH is received."""</span><br><span> self.create_remote_dir(remote_dir)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- wrapper_script = self.generate_wrapper_script()</span><br><span style="color: hsl(120, 100%, 40%);">+ wrapper_script = self.generate_wrapper_script(wait_time_sec)</span><br><span> remote_wrapper_script = remote_dir.child(RemoteHost.WRAPPER_SCRIPT)</span><br><span> self.scp('scp-wrapper-to-remote', wrapper_script, remote_wrapper_script)</span><br><span> </span><br><span>diff --git a/src/osmo_gsm_tester/obj/bts_osmotrx.py b/src/osmo_gsm_tester/obj/bts_osmotrx.py</span><br><span>index 1456802..0eeef49 100644</span><br><span>--- a/src/osmo_gsm_tester/obj/bts_osmotrx.py</span><br><span>+++ b/src/osmo_gsm_tester/obj/bts_osmotrx.py</span><br><span>@@ -310,7 +310,11 @@</span><br><span> remote_env = {}</span><br><span> remote_binary = self.binary_name()</span><br><span> args = (remote_binary, '-C', remote_config_file)</span><br><span style="color: hsl(0, 100%, 40%);">- self.proc_trx = rem_host.RemoteProcessFixIgnoreSIGHUP(self.binary_name(), remote_run_dir, args, remote_env=remote_env)</span><br><span style="color: hsl(120, 100%, 40%);">+ # Run remotely through ssh. We need to run binary under a wrapper</span><br><span style="color: hsl(120, 100%, 40%);">+ # script since osmo-trx ignores SIGHUP and will keep running after</span><br><span style="color: hsl(120, 100%, 40%);">+ # we close local ssh session. The wrapper script catches SIGHUP and</span><br><span style="color: hsl(120, 100%, 40%);">+ # sends SIGINT to it.</span><br><span style="color: hsl(120, 100%, 40%);">+ self.proc_trx = rem_host.RemoteProcessSafeExit(self.binary_name(), remote_run_dir, args, remote_env=remote_env)</span><br><span> self.testenv.remember_to_stop(self.proc_trx, keepalive)</span><br><span> self.proc_trx.launch()</span><br><span> </span><br><span>diff --git a/src/osmo_gsm_tester/obj/epc_amarisoft.py b/src/osmo_gsm_tester/obj/epc_amarisoft.py</span><br><span>index e9f1e82..d6cad81 100644</span><br><span>--- a/src/osmo_gsm_tester/obj/epc_amarisoft.py</span><br><span>+++ b/src/osmo_gsm_tester/obj/epc_amarisoft.py</span><br><span>@@ -46,6 +46,7 @@</span><br><span> self.ifup_file = None</span><br><span> self.process = None</span><br><span> self.rem_host = None</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_run_dir = None</span><br><span> self.remote_inst = None</span><br><span> self.remote_config_file = None</span><br><span> self.remote_log_file = None</span><br><span>@@ -92,8 +93,7 @@</span><br><span> </span><br><span> args = (remote_binary, self.remote_config_file)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- self.process = self.rem_host.RemoteProcess(AmarisoftEPC.BINFILE, args)</span><br><span style="color: hsl(0, 100%, 40%);">- #self.process = self.rem_host.RemoteProcessFixIgnoreSIGHUP(AmarisoftEPC.BINFILE, remote_run_dir, args)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.process = self.rem_host.RemoteProcessSafeExit(AmarisoftEPC.BINFILE, self.remote_run_dir, args)</span><br><span> self.testenv.remember_to_stop(self.process)</span><br><span> self.process.launch()</span><br><span> </span><br><span>@@ -137,11 +137,11 @@</span><br><span> self.rem_host = remote.RemoteHost(self.run_dir, self._run_node.ssh_user(), self._run_node.ssh_addr())</span><br><span> remote_prefix_dir = util.Dir(AmarisoftEPC.REMOTE_DIR)</span><br><span> self.remote_inst = util.Dir(remote_prefix_dir.child(os.path.basename(str(self.inst))))</span><br><span style="color: hsl(0, 100%, 40%);">- remote_run_dir = util.Dir(remote_prefix_dir.child(AmarisoftEPC.BINFILE))</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_run_dir = util.Dir(remote_prefix_dir.child(AmarisoftEPC.BINFILE))</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- self.remote_config_file = remote_run_dir.child(AmarisoftEPC.CFGFILE)</span><br><span style="color: hsl(0, 100%, 40%);">- self.remote_log_file = remote_run_dir.child(AmarisoftEPC.LOGFILE)</span><br><span style="color: hsl(0, 100%, 40%);">- self.remote_ifup_file = remote_run_dir.child(AmarisoftEPC.IFUPFILE)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_config_file = self.remote_run_dir.child(AmarisoftEPC.CFGFILE)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_log_file = self.remote_run_dir.child(AmarisoftEPC.LOGFILE)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_ifup_file = self.remote_run_dir.child(AmarisoftEPC.IFUPFILE)</span><br><span> </span><br><span> values = super().configure(['amarisoft', 'amarisoftepc'])</span><br><span> </span><br><span>@@ -162,7 +162,7 @@</span><br><span> if not self._run_node.is_local():</span><br><span> self.rem_host.recreate_remote_dir(self.remote_inst)</span><br><span> self.rem_host.scp('scp-inst-to-remote', str(self.inst), remote_prefix_dir)</span><br><span style="color: hsl(0, 100%, 40%);">- self.rem_host.recreate_remote_dir(remote_run_dir)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.rem_host.recreate_remote_dir(self.remote_run_dir)</span><br><span> self.rem_host.scp('scp-cfg-to-remote', self.config_file, self.remote_config_file)</span><br><span> self.rem_host.scp('scp-ifup-to-remote', self.ifup_file, self.remote_ifup_file)</span><br><span> </span><br><span>diff --git a/src/osmo_gsm_tester/obj/ms_srs.py b/src/osmo_gsm_tester/obj/ms_srs.py</span><br><span>index b494c54..3fa282e 100644</span><br><span>--- a/src/osmo_gsm_tester/obj/ms_srs.py</span><br><span>+++ b/src/osmo_gsm_tester/obj/ms_srs.py</span><br><span>@@ -91,6 +91,7 @@</span><br><span> self.process = None</span><br><span> self.rem_host = None</span><br><span> self.remote_inst = None</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_run_dir = None</span><br><span> self.remote_config_file = None</span><br><span> self.remote_log_file = None</span><br><span> self.remote_pcap_file = None</span><br><span>@@ -168,8 +169,7 @@</span><br><span> args = (remote_binary, self.remote_config_file, '--gw.netns=' + self.netns())</span><br><span> args += tuple(self._additional_args)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- self.process = self.rem_host.RemoteProcess(srsUE.BINFILE, args)</span><br><span style="color: hsl(0, 100%, 40%);">- #self.process = self.rem_host.RemoteProcessFixIgnoreSIGHUP(srsUE.BINFILE, remote_run_dir, args, remote_lib)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.process = self.rem_host.RemoteProcessSafeExit(srsUE.BINFILE, self.remote_run_dir, args)</span><br><span> self.testenv.remember_to_stop(self.process)</span><br><span> self.process.launch()</span><br><span> </span><br><span>@@ -213,11 +213,11 @@</span><br><span> self.rem_host = remote.RemoteHost(self.run_dir, self.remote_user, self._addr)</span><br><span> remote_prefix_dir = util.Dir(srsUE.REMOTE_DIR)</span><br><span> self.remote_inst = util.Dir(remote_prefix_dir.child(os.path.basename(str(self.inst))))</span><br><span style="color: hsl(0, 100%, 40%);">- remote_run_dir = util.Dir(remote_prefix_dir.child(srsUE.BINFILE))</span><br><span style="color: hsl(0, 100%, 40%);">- self.remote_config_file = remote_run_dir.child(srsUE.CFGFILE)</span><br><span style="color: hsl(0, 100%, 40%);">- self.remote_log_file = remote_run_dir.child(srsUE.LOGFILE)</span><br><span style="color: hsl(0, 100%, 40%);">- self.remote_pcap_file = remote_run_dir.child(srsUE.PCAPFILE)</span><br><span style="color: hsl(0, 100%, 40%);">- self.remote_metrics_file = remote_run_dir.child(srsUE.METRICSFILE)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_run_dir = util.Dir(remote_prefix_dir.child(srsUE.BINFILE))</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_config_file = self.remote_run_dir.child(srsUE.CFGFILE)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_log_file = self.remote_run_dir.child(srsUE.LOGFILE)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_pcap_file = self.remote_run_dir.child(srsUE.PCAPFILE)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.remote_metrics_file = self.remote_run_dir.child(srsUE.METRICSFILE)</span><br><span> </span><br><span> values = dict(ue=config.get_defaults('srsue'))</span><br><span> config.overlay(values, dict(ue=self.testenv.suite().config().get('modem', {})))</span><br><span>@@ -297,7 +297,7 @@</span><br><span> if not self.setup_runs_locally():</span><br><span> self.rem_host.recreate_remote_dir(self.remote_inst)</span><br><span> self.rem_host.scp('scp-inst-to-remote', str(self.inst), remote_prefix_dir)</span><br><span style="color: hsl(0, 100%, 40%);">- self.rem_host.recreate_remote_dir(remote_run_dir)</span><br><span style="color: hsl(120, 100%, 40%);">+ self.rem_host.recreate_remote_dir(self.remote_run_dir)</span><br><span> self.rem_host.scp('scp-cfg-to-remote', self.config_file, self.remote_config_file)</span><br><span> </span><br><span> def is_connected(self, mcc_mnc=None):</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/18452">change 18452</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/18452"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-gsm-tester </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: If043ecab509b34b0922824d73db916196274ec64 </div>
<div style="display:none"> Gerrit-Change-Number: 18452 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>