From 17629e82b5002aabb2351a0121a2d4ae83ed2f0d Mon Sep 17 00:00:00 2001 From: Sergio Maffioletti Date: Thu, 24 Aug 2017 15:40:47 +0200 Subject: [PATCH] Two fixes Two separate fixes lumped together in one commit: * Fix behavior of `task.free()`: it should invoke action on the attached controller, like `task.kill()` etc. do. This also fixes `gsession abort` and similar which were not cleaning up working directories. * Ensure that PIDs in the "job info" structures in `ShellcmdLrms` are always stored as strings. (Do not mix string and int types as was happening before.) Closes #630 --- gc3libs/__init__.py | 8 ++++++- gc3libs/backends/shellcmd.py | 31 ++++++++++-------------- gc3libs/backends/tests/test_machine.py | 16 ++++++------- gc3libs/backends/tests/test_shellcmd.py | 32 +++++++++++++++++++++++++ gc3utils/commands.py | 5 ++-- 5 files changed, 62 insertions(+), 30 deletions(-) diff --git a/gc3libs/__init__.py b/gc3libs/__init__.py index 7a39151c..641652b6 100755 --- a/gc3libs/__init__.py +++ b/gc3libs/__init__.py @@ -484,7 +484,13 @@ def free(self, **extra_args): See :meth:`gc3libs.Core.free` for a full explanation. """ - return + assert self._attached, ("Task.free() called on detached task %s." % + self) + assert hasattr(self._controller, 'free'), \ + ("Invalid `_controller` object '%s' in Task %s" % + (self._controller, self)) + self._controller.free(self, **extra_args) + # convenience methods, do not really add any functionality over # what's above diff --git a/gc3libs/backends/shellcmd.py b/gc3libs/backends/shellcmd.py index 68f686bc..ceeddf38 100755 --- a/gc3libs/backends/shellcmd.py +++ b/gc3libs/backends/shellcmd.py @@ -229,7 +229,7 @@ def get_process_state(self, pid): Raise ``LookupError`` if no process is identified by the given PID. """ - cmd = 'ps -p {0:d} -o state='.format(pid) + cmd = 'ps -p {0} -o state='.format(pid) rc, stdout, stderr = self.transport.execute_command(cmd) if rc == 1: # FIXME: same return code on MacOSX? raise LookupError('No process with PID {0}'.format(pid)) @@ -245,7 +245,7 @@ def get_process_running_time(self, pid): Raise ``LookupError`` if no process is identified by the given PID. """ - cmd = 'ps -p {0:d} -o etime='.format(pid) + cmd = 'ps -p {0} -o etime='.format(pid) rc, stdout, stderr = self.transport.execute_command(cmd) if rc == 1: # FIXME: same return code on MacOSX? raise LookupError('No process with PID {0}'.format(pid)) @@ -297,7 +297,7 @@ def _get_total_memory_impl(self): """Machine-specific part of `get_total_memory`.""" pass - def list_process_tree(self, root_pid=1): + def list_process_tree(self, root_pid="1"): """ Return list of PIDs of children of the given process. @@ -316,7 +316,7 @@ def list_process_tree(self, root_pid=1): if not line: continue pid, ppid = line.split() - children[int(ppid)].append(int(pid)) + children[str(ppid)].append(str(pid)) if root_pid not in children: return [] @@ -903,7 +903,7 @@ def _get_persisted_job_info(self): for pid in pidfiles: job = self._read_job_info_file(pid) if job: - job_infos[pid] = job + job_infos[str(pid)] = job else: # Process not found, ignore it continue @@ -915,7 +915,7 @@ def _read_job_info_file(self, pid): exists. Returns None if it does not exist. """ self.transport.connect() - log.debug("Reading job info file for pid %s", pid) + log.debug("Reading job info file for pid %r", pid) jobinfo = None path = posixpath.join(self.resource_dir, str(pid)) with self.transport.open(path, 'rb') as fp: @@ -944,7 +944,7 @@ def _delete_job_info_file(self, pid): """ self.transport.connect() log.debug("Deleting job info file for pid %s ...", pid) - pidfile = posixpath.join(self.resource_dir, str(pid)) + pidfile = posixpath.join(self.resource_dir, pid) try: self.transport.remove(pidfile) except Exception as err: @@ -981,7 +981,7 @@ def cancel_job(self, app): stored (by `submit_job`:meth:) as `app.execution.lrms_jobid`. """ try: - root_pid = int(app.execution.lrms_jobid) + root_pid = app.execution.lrms_jobid except ValueError: raise gc3libs.exceptions.InvalidArgument( "Invalid field `lrms_jobid` in Task '{0}':" @@ -1540,16 +1540,11 @@ def _read_app_process_id(self, pidfile_path): try: with self.transport.open(pidfile_path, 'r') as pidfile: pid = pidfile.read().strip() - return int(pid) - except ValueError: - # it happens that the PID file exists, but `.read()` - # returns the empty string... just wait and retry. - if pid == '': - continue - else: - raise gc3libs.exceptions.LRMSSubmitError( - "Invalid PID `{0}` in pidfile '{1}': {2}." - .format(pid, pidfile_path, err)) + # it happens that the PID file exists, but `.read()` + # returns the empty string... just wait and retry. + if pid == '': + continue + return pid except gc3libs.exceptions.TransportError as ex: if '[Errno 2]' in str(ex): # no such file or directory time.sleep(retry) diff --git a/gc3libs/backends/tests/test_machine.py b/gc3libs/backends/tests/test_machine.py index ae15a4f1..28960450 100755 --- a/gc3libs/backends/tests/test_machine.py +++ b/gc3libs/backends/tests/test_machine.py @@ -85,16 +85,16 @@ def test_list_process_tree_full(transport): '', ) - pids = mach.list_process_tree(2361) + pids = mach.list_process_tree("2361") assert pids == [ # PID PPID CMD - 2361, # 2361 1466 /sbin/upstart --user - 2431, # 2431 2361 \_ upstart-udev-bridge --daemon --user - 2663, # 2663 2361 \_ gpg-agent --homedir /home/rmurri/.gnupg --use-standard-socket --daemon - 2679, # 2679 2361 \_ /usr/lib/at-spi2-core/at-spi-bus-launcher - 2725, # 2684 2679 | \_ /usr/bin/dbus-daemon --config-file=/etc/at-spi2/accessibility.conf --nofork --print-address 3 - 2684, # 2725 2361 \_ /usr/bin/lxsession -s Lubuntu -e LXDE - 2736, # 2736 2725 | \_ lxpanel --profile Lubuntu + "2361", # 2361 1466 /sbin/upstart --user + "2431", # 2431 2361 \_ upstart-udev-bridge --daemon --user + "2663", # 2663 2361 \_ gpg-agent --homedir /home/rmurri/.gnupg --use-standard-socket --daemon + "2679", # 2679 2361 \_ /usr/lib/at-spi2-core/at-spi-bus-launcher + "2725", # 2684 2679 | \_ /usr/bin/dbus-daemon --config-file=/etc/at-spi2/accessibility.conf --nofork --print-address 3 + "2684", # 2725 2361 \_ /usr/bin/lxsession -s Lubuntu -e LXDE + "2736", # 2736 2725 | \_ lxpanel --profile Lubuntu ] diff --git a/gc3libs/backends/tests/test_shellcmd.py b/gc3libs/backends/tests/test_shellcmd.py index 45c32671..369ee33c 100755 --- a/gc3libs/backends/tests/test_shellcmd.py +++ b/gc3libs/backends/tests/test_shellcmd.py @@ -415,5 +415,37 @@ def test_resource_sharing_w_multiple_backends(self): core1.free(app) + def test_pid_list_is_string(self): + tmpdir = tempfile.mkdtemp(prefix=__name__, suffix='.d') + (fd, cfgfile) = tempfile.mkstemp() + f = os.fdopen(fd, 'w+') + f.write(TestBackendShellcmdCFG.CONF % ("False", cfgfile + '.d')) + f.close() + self.files_to_remove = [cfgfile, cfgfile + '.d'] + + self.cfg = gc3libs.config.Configuration() + self.cfg.merge_file(cfgfile) + + self.core = gc3libs.core.Core(self.cfg) + self.backend = self.core.get_backend('localhost_test') + # Update resource status + + app = gc3libs.Application( + arguments=['/bin/echo', 'Hello', 'World'], + inputs=[], + outputs=[], + output_dir=tmpdir, + requested_cores=1, + requested_memory=10 * Memory.MiB, ) + + try: + self.core.submit(app) + for pid in self.backend._job_infos.keys(): + assert isinstance(pid,str) + finally: + self.core.kill(app) + self.core.free(app) + + if __name__ == "__main__": pytest.main(["-v", __file__]) diff --git a/gc3utils/commands.py b/gc3utils/commands.py index 8c19bd71..61b47f38 100755 --- a/gc3utils/commands.py +++ b/gc3utils/commands.py @@ -1155,13 +1155,12 @@ def abort_session(self): continue try: task.kill() + task.free() + rc -= 1 except gc3libs.exceptions.Error, err: gc3libs.log.error( "Could not abort task '%s': %s: %s", task, err.__class__.__name__, err) - finally: - task.free() - rc -= 1 if rc: gc3libs.log.error(