]> git.ipfire.org Git - thirdparty/libcgroup.git/commitdiff
ftests: Cleanup pid logic in functional tests
authorTom Hromatka <tom.hromatka@oracle.com>
Fri, 17 Feb 2023 22:16:34 +0000 (15:16 -0700)
committerTom Hromatka <tom.hromatka@oracle.com>
Wed, 1 Mar 2023 15:10:39 +0000 (08:10 -0700)
PIDs were inconsistently being managed in the functional tests.  Some
functions treated them as `int` and in others they were `str`.  Modify
all functions that deal with PIDs to treat them as `int` or a list of
`int`s.

Reviewed-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
(cherry picked from commit 8483426ea7dcfe1e513ecbfc287a3a51f1097335)

15 files changed:
tests/ftests/034-cgexec-basic_cgexec.py
tests/ftests/036-cgset-multi_thread.py
tests/ftests/046-cgexec-empty_controller.py
tests/ftests/049-sudo-systemd_create_scope.py
tests/ftests/050-sudo-systemd_create_scope_w_pid.py
tests/ftests/052-sudo-cgroup_attach_task.py
tests/ftests/053-sudo-cgroup_attach_task_pid.py
tests/ftests/058-sudo-systemd_create_scope2.py
tests/ftests/060-sudo-cgconfigparser-systemd.py
tests/ftests/065-sudo-systemd_cgclassify-v1.py
tests/ftests/066-sudo-systemd_cgclassify-v2.py
tests/ftests/067-sudo-systemd_cgexec-v1.py
tests/ftests/068-sudo-systemd_cgexec-v2.py
tests/ftests/cgroup.py
tests/ftests/process.py

index 79255a86f9fd4c5439fb33e5970182e208344c6a..8a099aadda3f3f70c078ba28e0f325b3b7899a7a 100755 (executable)
@@ -7,8 +7,8 @@
 # Author: Tom Hromatka <tom.hromatka@oracle.com>
 #
 
+from process import Process
 from cgroup import Cgroup
-from run import Run
 import consts
 import ftests
 import sys
@@ -55,12 +55,7 @@ def test(config):
 
 def teardown(config):
     pids = Cgroup.get_pids_in_cgroup(config, CGNAME, CONTROLLER)
-    if pids:
-        for p in pids.splitlines():
-            if config.args.container:
-                config.container.run(['kill', '-9', p])
-            else:
-                Run.run(['sudo', 'kill', '-9', p])
+    Process.kill(config, pids)
 
     Cgroup.delete(config, CONTROLLER, CGNAME)
 
index b2c38b9cb7368b1798c9972a7b13fa9901a345bd..6f76f4371ca49ca2c9cc68f69350cc921d1ef234 100755 (executable)
@@ -8,7 +8,7 @@
 #
 
 from cgroup import Cgroup, CgroupVersion
-from run import Run
+from process import Process
 import consts
 import ftests
 import sys
@@ -69,9 +69,7 @@ def test(config):
 def teardown(config):
     # destroy the child processes
     pids = Cgroup.get_pids_in_cgroup(config, PARENT_CGNAME, CONTROLLER)
-    if pids:
-        for p in pids.splitlines():
-            Run.run(['sudo', 'kill', '-9', p])
+    Process.kill(config, pids)
 
     Cgroup.delete(config, CONTROLLER, PARENT_CGNAME, recursive=True)
 
index ba8716dbdf8851b66981d03a5e59a2c7d2c81ccf..981048a3b83cad1328f1c68bb2f2079a23b5423a 100755 (executable)
@@ -7,7 +7,7 @@
 #
 
 from cgroup import Cgroup, CgroupVersion
-from run import Run
+from process import Process
 import consts
 import ftests
 import sys
@@ -52,12 +52,7 @@ def test(config):
 
 def teardown(config):
     pids = Cgroup.get_pids_in_cgroup(config, CGNAME, CONTROLLER)
-    if pids:
-        for p in pids.splitlines():
-            if config.args.container:
-                config.container.run(['kill', '-9', p])
-            else:
-                Run.run(['sudo', 'kill', '-9', p])
+    Process.kill(config, pids)
 
     Cgroup.delete(config, None, CGNAME)
 
index a8e9a7c27997960a1081a7781be8a75e531369a0..887f88a5b0016a207f40e174a3a02f69c6d9c509 100755 (executable)
@@ -9,9 +9,10 @@
 
 from cgroup import Cgroup as CgroupCli
 from cgroup import CgroupVersion
-from run import Run, RunError
 from libcgroup import Cgroup
 from systemd import Systemd
+from process import Process
+from run import RunError
 import ftests
 import consts
 import sys
@@ -62,8 +63,7 @@ def test(config):
 def teardown(config, result):
     pid = CgroupCli.get(config, cgname=os.path.join(SLICE, SCOPE), setting='cgroup.procs',
                         print_headers=False, values_only=True)
-    if pid is not None:
-        Run.run(['kill', '-9', str(pid)], shell_bool=True)
+    Process.kill(config, pid)
 
     if result != consts.TEST_PASSED:
         # Something went wrong.  Let's force the removal of the cgroups just to be safe.
index 66b7ad34752e4292fb2256a1ef00fabaf6e9b9e0..f1b5217e2ea243e342ce7ee69a045885a86cd059 100755 (executable)
@@ -12,6 +12,7 @@ from cgroup import CgroupVersion
 from run import Run, RunError
 from libcgroup import Cgroup
 from systemd import Systemd
+from process import Process
 import ftests
 import consts
 import sys
@@ -66,8 +67,7 @@ def test(config):
 def teardown(config, result):
     global pid
 
-    if pid is not None:
-        Run.run(['kill', '-9', str(pid)], shell_bool=True)
+    Process.kill(config, pid)
 
     if result != consts.TEST_PASSED:
         # Something went wrong.  Let's force the removal of the cgroups just to be safe.
index c73d78e70ba951ab60f63bb8882c7bb184ed0b65..aeb26b1294f529155718532336c0cb8d0104c4f5 100755 (executable)
@@ -55,8 +55,8 @@ def test(config):
 
     found = False
     pids = CgroupCli.get_pids_in_cgroup(config, CGNAME, CONTROLLER)
-    for pid in pids.splitlines():
-        if int(pid) == os.getpid():
+    for pid in pids:
+        if pid == os.getpid():
             # our process was successfully added to the cgroup
             found = True
 
@@ -71,8 +71,8 @@ def test(config):
 
     found = False
     pids = CgroupCli.get_pids_in_cgroup(config, CGNAME, CONTROLLER)
-    for pid in pids.splitlines():
-        if int(pid) == os.getpid():
+    for pid in pids:
+        if pid == os.getpid():
             # our process was successfully added to the cgroup
             found = True
 
index 36f184ba3a62d83bfb19bb241207a293ded4435e..0e5010d698ae486d5bb3d81e420e2ca3818f7417 100755 (executable)
@@ -53,11 +53,11 @@ def test(config):
 
     cg = Cgroup(CGNAME, Version.CGROUP_V2)
     cg.get()
-    cg.attach(int(child_pid))
+    cg.attach(child_pid)
 
     found = False
     pids = CgroupCli.get_pids_in_cgroup(config, CGNAME, CONTROLLER)
-    for pid in pids.splitlines():
+    for pid in pids:
         if pid == child_pid:
             # our process was successfully added to the cgroup
             found = True
@@ -69,11 +69,11 @@ def test(config):
 
     # now let's attach the child process to the root cgroup and ensure we no longer
     # are in CGNAME
-    cg.attach(pid=int(child_pid), root_cgroup=True)
+    cg.attach(pid=child_pid, root_cgroup=True)
 
     found = False
     pids = CgroupCli.get_pids_in_cgroup(config, CGNAME, CONTROLLER)
-    for pid in pids.splitlines():
+    for pid in pids:
         if pid == child_pid:
             # our process was successfully added to the cgroup
             found = True
index 1eee76d1c456e9e6f36fa29583ec71164a2d81cf..74282770159013b30f679108aae78c07900c36ea 100755 (executable)
@@ -10,8 +10,9 @@
 from cgroup import CgroupVersion as CgroupCliVersion
 from cgroup import Cgroup as CgroupCli
 from libcgroup import Cgroup, Version
-from run import Run, RunError
 from systemd import Systemd
+from process import Process
+from run import RunError
 import ftests
 import consts
 import utils
@@ -66,7 +67,7 @@ def test(config):
     result = consts.TEST_PASSED
     cause = None
 
-    pid = int(config.process.create_process(config))
+    pid = config.process.create_process(config)
 
     cg = Cgroup(CGNAME, Version.CGROUP_V2)
 
@@ -122,8 +123,7 @@ def test(config):
 def teardown(config, result):
     global pid
 
-    if pid is not None:
-        Run.run(['kill', '-9', str(pid)], shell_bool=True)
+    Process.kill(config, pid)
 
     if result != consts.TEST_PASSED:
         # Something went wrong.  Let's force the removal of the cgroups just to be safe.
index 5d2e5db04fc6970014b85018b030e863789da3a9..4f55c21f0d3980fad1f38a78931278fc3a106d8c 100755 (executable)
@@ -6,9 +6,10 @@
 # Copyright (c) 2023 Oracle and/or its affiliates.
 # Author: Kamalesh Babulal <kamalesh.babulal@oracle.com>
 
-from run import Run, RunError
 from systemd import Systemd
+from process import Process
 from cgroup import Cgroup
+from run import RunError
 import consts
 import ftests
 import time
@@ -147,8 +148,8 @@ def test(config):
         result = consts.TEST_FAILED
         cause = 'Creation of systemd default slice/scope erroneously succeeded'
 
-    # killing the pid, should remove the scope cgroup too.
-    Run.run(['sudo', 'kill', '-9', pid])
+    # killing the pid should remove the scope cgroup too.
+    Process.kill(config, pid)
 
     # Let's pause and wait for the systemd to remove the scope.
     time.sleep(1)
index ef4376d384b894b8c605f729194db4d4de1d69da..850c1362ae48d7298f4aab25f3da6f0ed7abd98c 100755 (executable)
@@ -8,8 +8,9 @@
 #
 
 from cgroup import Cgroup, CgroupVersion
-from run import Run, RunError
 from systemd import Systemd
+from process import Process
+from run import RunError
 import consts
 import ftests
 import time
@@ -91,12 +92,6 @@ def create_process_get_pid(config, CGNAME, SLICENAME='', ignore_systemd=False):
     return pids, result, cause
 
 
-def terminate_process(config, pids):
-    if pids:
-        for p in pids.splitlines():
-            Run.run(['sudo', 'kill', '-9', p])
-
-
 def test(config):
     global SYSTEMD_PIDS, OTHER_PIDS
 
@@ -126,12 +121,12 @@ def test(config):
     try:
         Cgroup.classify(config, CONTROLLER, SYSTEMD_CGNAME, OTHER_PIDS, ignore_systemd=True)
     except RunError as re:
-        err_str = 'Error changing group of pid {}: Cgroup does not exist'.format(OTHER_PIDS)
+        err_str = 'Error changing group of pid {}: Cgroup does not exist'.format(OTHER_PIDS[0])
         if re.stderr != err_str:
             raise re
     else:
         result = consts.TEST_FAILED
-        cause = 'Changing group of pid {} erroneously succeeded'.format(OTHER_PIDS)
+        cause = 'Changing group of pid {} erroneously succeeded'.format(OTHER_PIDS[0])
 
     # classify a task from the systemd scope cgroup (SYSTEMD_CGNAME) to
     # non-systemd scope cgroup (OTHER_CGNAME).  Migration should fail due
@@ -140,12 +135,12 @@ def test(config):
     try:
         Cgroup.classify(config, CONTROLLER, OTHER_CGNAME, SYSTEMD_PIDS)
     except RunError as re:
-        err_str = 'Error changing group of pid {}: Cgroup does not exist'.format(SYSTEMD_PIDS)
+        err_str = 'Error changing group of pid {}: Cgroup does not exist'.format(SYSTEMD_PIDS[0])
         if re.stderr != err_str:
             raise re
     else:
         result = consts.TEST_FAILED
-        tmp_cause = 'Changing group of pid {} erroneously succeeded'.format(SYSTEMD_PIDS)
+        tmp_cause = 'Changing group of pid {} erroneously succeeded'.format(SYSTEMD_PIDS[0])
         cause = '\n'.join(filter(None, [cause, tmp_cause]))
 
     # classify the task from the non-systemd scope cgroup to systemd scope cgroup.
@@ -157,8 +152,8 @@ def test(config):
 def teardown(config):
     global SYSTEMD_PIDS, OTHER_PIDS
 
-    terminate_process(config, SYSTEMD_PIDS)
-    terminate_process(config, OTHER_PIDS)
+    Process.kill(config, SYSTEMD_PIDS)
+    Process.kill(config, OTHER_PIDS)
 
     # We need a pause, so that cgroup.procs gets updated.
     time.sleep(1)
index 7004ea6d71f36d4ac2acfd736882f727a4878dce..d610cc1351d034a59a7c67744e100d5d5a0fc351 100755 (executable)
@@ -9,7 +9,8 @@
 
 from cgroup import Cgroup, CgroupVersion
 from systemd import Systemd
-from run import Run, RunError
+from process import Process
+from run import RunError
 import consts
 import ftests
 import time
@@ -103,12 +104,6 @@ def create_process_get_pid(config, CGNAME, SLICENAME='', ignore_systemd=False):
     return pids, result, cause
 
 
-def terminate_process(config, pids):
-    if pids:
-        for p in pids.splitlines():
-            Run.run(['sudo', 'kill', '-9', p])
-
-
 def test(config):
     global SYSTEMD_PIDS, OTHER_PIDS
 
@@ -131,11 +126,6 @@ def test(config):
     if result == consts.TEST_FAILED:
         return result, cause
 
-    # SYSTEMD_CGNAME already has the pid of the task, that scope was created
-    # with and killing it will remove the scope, be careful and pick the newly
-    # spawned task
-    SYSTEMD_PIDS = SYSTEMD_PIDS.split('\n')[1]
-
     # classify a task from the non-systemd scope cgroup (OTHER_CGNAME) to
     # systemd scope cgroup (SYSTEMD_CGNAME).  Migration should fail due to
     # the incorrect destination cgroup path that gets constructed, without
@@ -143,26 +133,27 @@ def test(config):
     try:
         Cgroup.classify(config, CONTROLLER, SYSTEMD_CGNAME, OTHER_PIDS, ignore_systemd=True)
     except RunError as re:
-        err_str = 'Error changing group of pid {}: No such file or directory'.format(OTHER_PIDS)
+        err_str = 'Error changing group of pid {}: No such file or directory'.format(OTHER_PIDS[0])
         if re.stderr != err_str:
             raise re
     else:
         result = consts.TEST_FAILED
-        cause = 'Changing group of pid {} erroneously succeeded'.format(OTHER_PIDS)
+        cause = 'Changing group of pid {} erroneously succeeded'.format(OTHER_PIDS[0])
 
     # classify a task from the systemd scope cgroup (SYSTEMD_CGNAME) to
     # non-systemd scope cgroup (OTHER_CGNAME).  Migration should fail due
     # to the incorrect destination cgroup path that gets constructed, with
     # the systemd slice/scope when ignore_systemd=False)
     try:
-        Cgroup.classify(config, CONTROLLER, OTHER_CGNAME, SYSTEMD_PIDS)
+        Cgroup.classify(config, CONTROLLER, OTHER_CGNAME, SYSTEMD_PIDS[1])
     except RunError as re:
-        err_str = 'Error changing group of pid {}: No such file or directory'.format(SYSTEMD_PIDS)
+        err_str = 'Error changing group of pid {}: No such file or directory'.format(
+                  SYSTEMD_PIDS[1])
         if re.stderr != err_str:
             raise re
     else:
         result = consts.TEST_FAILED
-        tmp_cause = 'Changing group of pid {} erroneously succeeded'.format(SYSTEMD_PIDS)
+        tmp_cause = 'Changing group of pid {} erroneously succeeded'.format(SYSTEMD_PIDS[1])
         cause = '\n'.join(filter(None, [cause, tmp_cause]))
 
     # classify the task from the non-systemd scope cgroup to systemd scope cgroup.
@@ -174,13 +165,17 @@ def test(config):
 def teardown(config):
     global SYSTEMD_PIDS, OTHER_PIDS
 
-    terminate_process(config, SYSTEMD_PIDS)
-    terminate_process(config, OTHER_PIDS)
+    Process.kill(config, SYSTEMD_PIDS)
+    Process.kill(config, OTHER_PIDS)
 
     # We need a pause, so that cgroup.procs gets updated.
     time.sleep(1)
 
-    Systemd.remove_scope_slice_conf(config, SLICE, SCOPE, CONTROLLER, CONFIG_FILE_NAME)
+    try:
+        Cgroup.delete(config, CONTROLLER, cgname=SLICE, ignore_systemd=True)
+    except RunError as re:
+        if 'No such file or directory' not in re.stderr:
+            raise re
 
     # Incase the error occurs before the creation of OTHER_CGNAME,
     # let's ignore the exception
index c1d049073e63f610d4147bd2ac159261b71b8171..3a452f656115c81e485e735456cb38f966b09b13 100755 (executable)
@@ -9,7 +9,8 @@
 
 from cgroup import Cgroup, CgroupVersion
 from systemd import Systemd
-from run import Run, RunError
+from process import Process
+from run import RunError
 import consts
 import ftests
 import time
@@ -94,12 +95,6 @@ def create_process_get_pid(config, CGNAME, SLICENAME='', ignore_systemd=False):
     return pids, result, cause
 
 
-def terminate_process(config, pids):
-    if pids:
-        for p in pids.splitlines():
-            Run.run(['sudo', 'kill', '-9', p])
-
-
 def test(config):
     global SYSTEMD_PIDS, OTHER_PIDS
 
@@ -125,8 +120,8 @@ def test(config):
 def teardown(config):
     global SYSTEMD_PIDS, OTHER_PIDS
 
-    terminate_process(config, SYSTEMD_PIDS)
-    terminate_process(config, OTHER_PIDS)
+    Process.kill(config, SYSTEMD_PIDS)
+    Process.kill(config, OTHER_PIDS)
 
     # We need a pause, so that cgroup.procs gets updated.
     time.sleep(1)
index d63e6a7e61a7f1f49822cd4284f60b7b26fe911a..4bb1814192d8269b9c030d36d035ededa53cc74c 100755 (executable)
@@ -8,8 +8,9 @@
 #
 
 from cgroup import Cgroup, CgroupVersion
-from run import Run, RunError
 from systemd import Systemd
+from process import Process
+from run import RunError
 import consts
 import ftests
 import time
@@ -26,8 +27,8 @@ SCOPE = 'test068.scope'
 
 CONFIG_FILE_NAME = os.path.join(os.getcwd(), '068cgconfig.conf')
 
-SYSTEMD_PIDS = ''
-OTHER_PIDS = ''
+SYSTEMD_PIDS = None
+OTHER_PIDS = None
 
 
 def prereqs(config):
@@ -106,12 +107,6 @@ def create_process_get_pid(config, CGNAME, SLICENAME='', ignore_systemd=False):
     return pids, result, cause
 
 
-def terminate_process(config, pids):
-    if pids:
-        for p in pids.splitlines():
-            Run.run(['kill', '-9', p])
-
-
 def test(config):
     global SYSTEMD_PIDS, OTHER_PIDS
 
@@ -134,7 +129,7 @@ def test(config):
     # SYSTEMD_CGNAME already has the pid of the task, that scope was created
     # with and killing it will remove the scope, be careful and pick the newly
     # spawned task
-    SYSTEMD_PIDS = SYSTEMD_PIDS.split('\n')[1]
+    SYSTEMD_PIDS = SYSTEMD_PIDS[1]
 
     return result, cause
 
@@ -142,8 +137,8 @@ def test(config):
 def teardown(config):
     global SYSTEMD_PIDS, OTHER_PIDS
 
-    terminate_process(config, SYSTEMD_PIDS)
-    terminate_process(config, OTHER_PIDS)
+    Process.kill(config, SYSTEMD_PIDS)
+    Process.kill(config, OTHER_PIDS)
 
     # We need a pause, so that cgroup.procs gets updated.
     time.sleep(1)
index cf634c6ffb0df427860d8c89ebf75bd06f690192..948eedbb1c8e2f1aad38cca221a471e22c765bed 100644 (file)
@@ -220,9 +220,9 @@ class Cgroup(object):
     def __set(config, cmd, cgname=None, setting=None, value=None,
               copy_from=None, cghelp=False, ignore_systemd=False):
         if setting is not None or value is not None:
-            if isinstance(setting, str) and isinstance(value, str):
+            if isinstance(setting, str) and (isinstance(value, str) or isinstance(value, int)):
                 cmd.append('-r')
-                cmd.append('{}={}'.format(setting, value))
+                cmd.append('{}={}'.format(setting, str(value)))
             elif isinstance(setting, list) and isinstance(value, list):
                 if len(setting) != len(value):
                     raise ValueError(
@@ -232,7 +232,7 @@ class Cgroup(object):
 
                 for idx, stg in enumerate(setting):
                     cmd.append('-r')
-                    cmd.append('{}={}'.format(stg, value[idx]))
+                    cmd.append('{}={}'.format(stg, str(value[idx])))
             else:
                 raise ValueError(
                                     'Invalid inputs to cgget:\nsetting: {}\n'
@@ -450,7 +450,7 @@ class Cgroup(object):
             cmd.append(str(pid_list))
         elif isinstance(pid_list, list):
             for pid in pid_list:
-                cmd.append(pid)
+                cmd.append(str(pid))
 
         if config.args.container:
             config.container.run(cmd)
@@ -580,6 +580,8 @@ class Cgroup(object):
             if re.ret == 0 and \
                'neither blacklisted nor whitelisted' in re.stderr:
                 res = re.stdout
+            elif re.ret == 0 and 'ERROR: can\'t get' in re.stderr:
+                res = re.stdout
             else:
                 raise(re)
 
@@ -951,7 +953,7 @@ class Cgroup(object):
             cmd.append(cmdline)
         elif isinstance(cmdline, list):
             for entry in cmdline:
-                cmd.append(entry)
+                cmd.append(str(entry))
 
         if cghelp:
             cmd.append('-h')
@@ -975,9 +977,15 @@ class Cgroup(object):
                 cmd = ['cat', proc_file]
 
                 if config.args.container:
-                    return config.container.run(cmd, shell_bool=True)
+                    pids = config.container.run(cmd, shell_bool=True)
                 else:
-                    return Run.run(cmd, shell_bool=True)
+                    pids = Run.run(cmd, shell_bool=True)
+
+                pid_list = list()
+                for pid in pids.splitlines():
+                    pid_list.append(int(pid))
+
+                return pid_list
 
         return None
 
index a1015e627eb4dcef6c4273ae8dc0d5f0da6dcf2a..97aa1734028e3b7e723a91495fcff1648ba2c290 100644 (file)
@@ -77,7 +77,7 @@ class Process(object):
             pid = Run.run(cmd, shell_bool=True)
 
             for _pid in pid.splitlines():
-                self.children_pids.append(_pid)
+                self.children_pids.append(int(_pid))
 
             if pid.find('\n') >= 0:
                 # The second pid in the list contains the actual perl process
@@ -90,7 +90,7 @@ class Process(object):
                                 ''.format(pid)
                             )
 
-        return pid
+        return int(pid)
 
     def create_process(self, config):
         # To allow for multiple processes to be created, each new process
@@ -254,4 +254,20 @@ class Process(object):
 
         raise ValueError("get_cgroup() shouldn't reach this point")
 
+    @staticmethod
+    def kill(config, pids):
+        if not pids:
+            return
+
+        if type(pids) == str:
+            pids = [int(pid) for pid in pids.splitlines()]
+        elif type(pids) == int:
+            pids = [pids]
+
+        for pid in pids:
+            if config.args.container:
+                config.container.run(['kill', '-9', str(pid)])
+            else:
+                Run.run(['kill', '-9', str(pid)])
+
 # vim: set et ts=4 sw=4: