From: Tom Hromatka Date: Fri, 17 Feb 2023 22:16:34 +0000 (-0700) Subject: ftests: Cleanup pid logic in functional tests X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=dd7c706be9fecf3ea66efef2b472baf1acdd85c1;p=thirdparty%2Flibcgroup.git ftests: Cleanup pid logic in functional tests 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 Signed-off-by: Tom Hromatka (cherry picked from commit 8483426ea7dcfe1e513ecbfc287a3a51f1097335) --- diff --git a/tests/ftests/034-cgexec-basic_cgexec.py b/tests/ftests/034-cgexec-basic_cgexec.py index 79255a86..8a099aad 100755 --- a/tests/ftests/034-cgexec-basic_cgexec.py +++ b/tests/ftests/034-cgexec-basic_cgexec.py @@ -7,8 +7,8 @@ # Author: Tom Hromatka # +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) diff --git a/tests/ftests/036-cgset-multi_thread.py b/tests/ftests/036-cgset-multi_thread.py index b2c38b9c..6f76f437 100755 --- a/tests/ftests/036-cgset-multi_thread.py +++ b/tests/ftests/036-cgset-multi_thread.py @@ -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) diff --git a/tests/ftests/046-cgexec-empty_controller.py b/tests/ftests/046-cgexec-empty_controller.py index ba8716db..981048a3 100755 --- a/tests/ftests/046-cgexec-empty_controller.py +++ b/tests/ftests/046-cgexec-empty_controller.py @@ -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) diff --git a/tests/ftests/049-sudo-systemd_create_scope.py b/tests/ftests/049-sudo-systemd_create_scope.py index a8e9a7c2..887f88a5 100755 --- a/tests/ftests/049-sudo-systemd_create_scope.py +++ b/tests/ftests/049-sudo-systemd_create_scope.py @@ -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. diff --git a/tests/ftests/050-sudo-systemd_create_scope_w_pid.py b/tests/ftests/050-sudo-systemd_create_scope_w_pid.py index 66b7ad34..f1b5217e 100755 --- a/tests/ftests/050-sudo-systemd_create_scope_w_pid.py +++ b/tests/ftests/050-sudo-systemd_create_scope_w_pid.py @@ -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. diff --git a/tests/ftests/052-sudo-cgroup_attach_task.py b/tests/ftests/052-sudo-cgroup_attach_task.py index c73d78e7..aeb26b12 100755 --- a/tests/ftests/052-sudo-cgroup_attach_task.py +++ b/tests/ftests/052-sudo-cgroup_attach_task.py @@ -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 diff --git a/tests/ftests/053-sudo-cgroup_attach_task_pid.py b/tests/ftests/053-sudo-cgroup_attach_task_pid.py index 36f184ba..0e5010d6 100755 --- a/tests/ftests/053-sudo-cgroup_attach_task_pid.py +++ b/tests/ftests/053-sudo-cgroup_attach_task_pid.py @@ -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 diff --git a/tests/ftests/058-sudo-systemd_create_scope2.py b/tests/ftests/058-sudo-systemd_create_scope2.py index 1eee76d1..74282770 100755 --- a/tests/ftests/058-sudo-systemd_create_scope2.py +++ b/tests/ftests/058-sudo-systemd_create_scope2.py @@ -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. diff --git a/tests/ftests/060-sudo-cgconfigparser-systemd.py b/tests/ftests/060-sudo-cgconfigparser-systemd.py index 5d2e5db0..4f55c21f 100755 --- a/tests/ftests/060-sudo-cgconfigparser-systemd.py +++ b/tests/ftests/060-sudo-cgconfigparser-systemd.py @@ -6,9 +6,10 @@ # Copyright (c) 2023 Oracle and/or its affiliates. # Author: Kamalesh Babulal -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) diff --git a/tests/ftests/065-sudo-systemd_cgclassify-v1.py b/tests/ftests/065-sudo-systemd_cgclassify-v1.py index ef4376d3..850c1362 100755 --- a/tests/ftests/065-sudo-systemd_cgclassify-v1.py +++ b/tests/ftests/065-sudo-systemd_cgclassify-v1.py @@ -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) diff --git a/tests/ftests/066-sudo-systemd_cgclassify-v2.py b/tests/ftests/066-sudo-systemd_cgclassify-v2.py index 7004ea6d..d610cc13 100755 --- a/tests/ftests/066-sudo-systemd_cgclassify-v2.py +++ b/tests/ftests/066-sudo-systemd_cgclassify-v2.py @@ -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 diff --git a/tests/ftests/067-sudo-systemd_cgexec-v1.py b/tests/ftests/067-sudo-systemd_cgexec-v1.py index c1d04907..3a452f65 100755 --- a/tests/ftests/067-sudo-systemd_cgexec-v1.py +++ b/tests/ftests/067-sudo-systemd_cgexec-v1.py @@ -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) diff --git a/tests/ftests/068-sudo-systemd_cgexec-v2.py b/tests/ftests/068-sudo-systemd_cgexec-v2.py index d63e6a7e..4bb18141 100755 --- a/tests/ftests/068-sudo-systemd_cgexec-v2.py +++ b/tests/ftests/068-sudo-systemd_cgexec-v2.py @@ -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) diff --git a/tests/ftests/cgroup.py b/tests/ftests/cgroup.py index cf634c6f..948eedbb 100644 --- a/tests/ftests/cgroup.py +++ b/tests/ftests/cgroup.py @@ -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 diff --git a/tests/ftests/process.py b/tests/ftests/process.py index a1015e62..97aa1734 100644 --- a/tests/ftests/process.py +++ b/tests/ftests/process.py @@ -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: