<p>Holger Freyther <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/11597">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Jenkins Builder: Verified
  Pau Espin Pedrol: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ms_driver: Switch to process.Process from subprocess<br><br>Use a single tmp/rundir for all these instances. In the next step<br>we can create separate directories.<br><br>Change-Id: Icf4d4e161ac4283a63ed4e0745b375e7e6a25004<br>---<br>M src/osmo_gsm_tester/ms_driver.py<br>M src/osmo_ms_driver/__main__.py<br>M src/osmo_ms_driver/location_update_test.py<br>M src/osmo_ms_driver/starter.py<br>4 files changed, 27 insertions(+), 28 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo_gsm_tester/ms_driver.py b/src/osmo_gsm_tester/ms_driver.py</span><br><span>index 86186b7..36da921 100644</span><br><span>--- a/src/osmo_gsm_tester/ms_driver.py</span><br><span>+++ b/src/osmo_gsm_tester/ms_driver.py</span><br><span>@@ -63,7 +63,6 @@</span><br><span>         self._test_duration = timedelta(seconds=120)</span><br><span>         self._cdf = cdfs["ease_in_out"](self._time_start, self._time_step)</span><br><span>         self._loop = SimpleLoop()</span><br><span style="color: hsl(0, 100%, 40%);">-        self._suite_run.remember_to_stop(self)</span><br><span>         self._test_case = None</span><br><span>         self.event_server_sk_tmp_dir = None</span><br><span> </span><br><span>@@ -86,7 +85,8 @@</span><br><span>         self._ev_server.listen(self._loop)</span><br><span>         options = BinaryOptions("virtphy", "mobile", None)</span><br><span>         self._test_case = MassUpdateLocationTest("mass", options, self._num_ms, self._cdf,</span><br><span style="color: hsl(0, 100%, 40%);">-                                                 self._ev_server, self.event_server_sk_tmp_dir)</span><br><span style="color: hsl(120, 100%, 40%);">+                                                 self._ev_server, self.event_server_sk_tmp_dir,</span><br><span style="color: hsl(120, 100%, 40%);">+                                                 suite_run=self._suite_run)</span><br><span> </span><br><span>         # TODO: We should pass subscribers down to the test and not get it from</span><br><span>         # there.</span><br><span>@@ -120,16 +120,11 @@</span><br><span> </span><br><span>     def cleanup(self):</span><br><span>         """</span><br><span style="color: hsl(0, 100%, 40%);">-        Stops the testcase and all launched processes. Called by the</span><br><span style="color: hsl(0, 100%, 40%);">-        suite.</span><br><span style="color: hsl(120, 100%, 40%);">+        Cleans up the driver (e.g. AF_UNIX files).</span><br><span>         """</span><br><span> </span><br><span>         # Clean-up the temporary directory.</span><br><span>         if self.event_server_sk_tmp_dir:</span><br><span>             shutil.rmtree(path=self.event_server_sk_tmp_dir)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        if not self._test_case:</span><br><span style="color: hsl(0, 100%, 40%);">-            return</span><br><span style="color: hsl(0, 100%, 40%);">-        self._test_case.stop_all()</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> # vim: expandtab tabstop=4 shiftwidth=4</span><br><span>diff --git a/src/osmo_ms_driver/__main__.py b/src/osmo_ms_driver/__main__.py</span><br><span>index 265ce47..0e3611b 100644</span><br><span>--- a/src/osmo_ms_driver/__main__.py</span><br><span>+++ b/src/osmo_ms_driver/__main__.py</span><br><span>@@ -83,7 +83,7 @@</span><br><span>     ev_server.listen(loop)</span><br><span> </span><br><span>     # Just a single test for now.</span><br><span style="color: hsl(0, 100%, 40%);">-    options = BinaryOptions("virtphy", "mobile", None)</span><br><span style="color: hsl(120, 100%, 40%);">+    options = BinaryOptions("virtphy", "mobile", os.environ)</span><br><span>     test = MassUpdateLocationTest("lu_test", options, args.num_ms, cdf, ev_server, tmp_dir)</span><br><span>     atexit.register(test.stop_all)</span><br><span> </span><br><span>diff --git a/src/osmo_ms_driver/location_update_test.py b/src/osmo_ms_driver/location_update_test.py</span><br><span>index dee99fe..90e405e 100644</span><br><span>--- a/src/osmo_ms_driver/location_update_test.py</span><br><span>+++ b/src/osmo_ms_driver/location_update_test.py</span><br><span>@@ -56,12 +56,14 @@</span><br><span>     TEMPLATE_LUA = "osmo-mobile-lu.lua"</span><br><span>     TEMPLATE_CFG = "osmo-mobile.cfg"</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    def __init__(self, name, options, number_of_ms, cdf_function, event_server, tmp_dir):</span><br><span style="color: hsl(120, 100%, 40%);">+    def __init__(self, name, options, number_of_ms, cdf_function,</span><br><span style="color: hsl(120, 100%, 40%);">+                 event_server, tmp_dir, suite_run=None):</span><br><span>         super().__init__(log.C_RUN, name)</span><br><span>         self._binary_options = options</span><br><span>         self._number_of_ms = number_of_ms</span><br><span>         self._cdf = cdf_function</span><br><span>         self._cdf.set_target(number_of_ms)</span><br><span style="color: hsl(120, 100%, 40%);">+        self._suite_run = suite_run</span><br><span>         self._unstarted = []</span><br><span>         self._mobiles = []</span><br><span>         self._phys = []</span><br><span>@@ -100,7 +102,7 @@</span><br><span>         """</span><br><span>         self.log("Pre-launching all virtphy's")</span><br><span>         for phy in self._phys:</span><br><span style="color: hsl(0, 100%, 40%);">-            phy.start(loop)</span><br><span style="color: hsl(120, 100%, 40%);">+            phy.start(loop, self._suite_run)</span><br><span> </span><br><span>         self.log("Checking if sockets are in the filesystem")</span><br><span>         for phy in self._phys:</span><br><span>@@ -132,7 +134,7 @@</span><br><span>         # start pending MS</span><br><span>         while len(self._started) < self._cdf.current_scaled_value() and len(self._unstarted) > 0:</span><br><span>             ms = self._unstarted.pop(0)</span><br><span style="color: hsl(0, 100%, 40%);">-            ms.start(loop)</span><br><span style="color: hsl(120, 100%, 40%);">+            ms.start(loop, self._suite_run)</span><br><span>             launch_time = time.clock_gettime(time.CLOCK_MONOTONIC)</span><br><span>             self._results[ms.name_number()].set_launch_time(launch_time)</span><br><span>             self._started.append(ms)</span><br><span>@@ -170,7 +172,7 @@</span><br><span> </span><br><span>     def stop_all(self):</span><br><span>         for launcher in self._started:</span><br><span style="color: hsl(0, 100%, 40%);">-            launcher.kill()</span><br><span style="color: hsl(120, 100%, 40%);">+            launcher.terminate()</span><br><span> </span><br><span>     def handle_msg(self, _data, addr, time):</span><br><span>         import json</span><br><span>diff --git a/src/osmo_ms_driver/starter.py b/src/osmo_ms_driver/starter.py</span><br><span>index 2ffb33a..66027c6 100644</span><br><span>--- a/src/osmo_ms_driver/starter.py</span><br><span>+++ b/src/osmo_ms_driver/starter.py</span><br><span>@@ -16,17 +16,13 @@</span><br><span> # You should have received a copy of the GNU General Public License</span><br><span> # along with this program.  If not, see <http://www.gnu.org/licenses/>.</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-from osmo_gsm_tester import log, template</span><br><span style="color: hsl(120, 100%, 40%);">+from osmo_gsm_tester import log, process, template</span><br><span> </span><br><span> import collections</span><br><span> import os</span><br><span> import os.path</span><br><span style="color: hsl(0, 100%, 40%);">-import subprocess</span><br><span> import time</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-_devnull = open(os.devnull, 'w')</span><br><span style="color: hsl(0, 100%, 40%);">-#_devnull = open('/dev/stdout', 'w')</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> BinaryOptions = collections.namedtuple("BinaryOptions", ["virtphy", "mobile", "env"])</span><br><span> </span><br><span> class Launcher(log.Origin):</span><br><span>@@ -49,14 +45,17 @@</span><br><span>     def phy_filename(self):</span><br><span>         return self._phy_filename</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    def start(self, loop):</span><br><span style="color: hsl(120, 100%, 40%);">+    def start(self, loop, suite_run=None):</span><br><span>         if len(self._phy_filename.encode()) > 107:</span><br><span>             raise log.Error('Path for unix socket is longer than max allowed len for unix socket path (107):', self._phy_filename)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        self.log("Starting virtphy process")</span><br><span style="color: hsl(120, 100%, 40%);">+        self.log("Starting virtphy")</span><br><span>         args = [self._binary, "--l1ctl-sock=" + self._phy_filename]</span><br><span style="color: hsl(0, 100%, 40%);">-        self.log(' '.join(args))</span><br><span style="color: hsl(0, 100%, 40%);">-        self._vphy_proc = subprocess.Popen(args, stderr=_devnull, stdout=_devnull, env=self._env)</span><br><span style="color: hsl(120, 100%, 40%);">+        self._vphy_proc = process.Process(self.name(), self._tmp_dir,</span><br><span style="color: hsl(120, 100%, 40%);">+                                          args, env=self._env)</span><br><span style="color: hsl(120, 100%, 40%);">+        if suite_run:</span><br><span style="color: hsl(120, 100%, 40%);">+            suite_run.remember_to_stop(self._vphy_proc)</span><br><span style="color: hsl(120, 100%, 40%);">+        self._vphy_proc.launch()</span><br><span> </span><br><span>     def verify_ready(self):</span><br><span>         while True:</span><br><span>@@ -64,7 +63,7 @@</span><br><span>                 return</span><br><span>             time.sleep(0.2)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    def kill(self):</span><br><span style="color: hsl(120, 100%, 40%);">+    def terminate(self):</span><br><span>         """Clean up things."""</span><br><span>         if self._vphy_proc:</span><br><span>             self._vphy_proc.terminate()</span><br><span>@@ -116,17 +115,20 @@</span><br><span>             w.write(mob_vty)</span><br><span>         return mob_cfg_file</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    def start(self, loop):</span><br><span style="color: hsl(120, 100%, 40%);">+    def start(self, loop, suite_run=None):</span><br><span>         lua_filename = self.write_lua_cfg()</span><br><span>         mob_filename = self.write_mob_cfg(lua_filename, self._phy_filename)</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        self.log("Starting process")</span><br><span style="color: hsl(120, 100%, 40%);">+        self.log("Starting mobile")</span><br><span>         # Let the kernel pick an unused port for the VTY.</span><br><span>         args = [self._binary, "-c", mob_filename, "--vty-port=0"]</span><br><span style="color: hsl(0, 100%, 40%);">-        self.log(' '.join(args))</span><br><span style="color: hsl(0, 100%, 40%);">-        self._omob_proc = subprocess.Popen(args, stderr=_devnull, stdout=_devnull, env=self._env)</span><br><span style="color: hsl(120, 100%, 40%);">+        self._omob_proc = process.Process(self.name(), self._tmp_dir,</span><br><span style="color: hsl(120, 100%, 40%);">+                                          args, env=self._env)</span><br><span style="color: hsl(120, 100%, 40%);">+        if suite_run:</span><br><span style="color: hsl(120, 100%, 40%);">+            suite_run.remember_to_stop(self._omob_proc)</span><br><span style="color: hsl(120, 100%, 40%);">+        self._omob_proc.launch()</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    def kill(self):</span><br><span style="color: hsl(120, 100%, 40%);">+    def terminate(self):</span><br><span>         """Clean up things."""</span><br><span>         if self._omob_proc:</span><br><span>             self._omob_proc.terminate()</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11597">change 11597</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/11597"/><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-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Icf4d4e161ac4283a63ed4e0745b375e7e6a25004 </div>
<div style="display:none"> Gerrit-Change-Number: 11597 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Holger Freyther <holger@freyther.de> </div>
<div style="display:none"> Gerrit-Reviewer: Holger Freyther <holger@freyther.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>