From dc3c0451b1f8853bfd4a41b02ad6800a7080f0fa Mon Sep 17 00:00:00 2001 From: Yaqiang Zhu Date: Wed, 17 Jul 2024 04:55:19 +0800 Subject: [PATCH] [DHCP] Add try/exception when using psutil.process_iter (#19537) Why I did it After got running process list by psutil, if some of the processes exited, but invoke name() / cmdline() function of them, it will raise NoSuchProcess exception Fix issue #19507 How I did it Add try/exception in process execution Change func get_target_process_cmds to get_target_process for reuse How to verify it UT passed --- .../dhcp_utilities/common/utils.py | 7 ++++-- .../dhcp_utilities/dhcprelayd/dhcprelayd.py | 23 +++++++++++-------- .../dhcp_utilities/dhcpservd/dhcpservd.py | 9 +++++--- .../tests/common_utils.py | 7 +++++- .../tests/test_dhcprelayd.py | 17 +++++++------- .../tests/test_dhcpservd.py | 1 + src/sonic-dhcp-utilities/tests/test_utils.py | 4 +++- 7 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/sonic-dhcp-utilities/dhcp_utilities/common/utils.py b/src/sonic-dhcp-utilities/dhcp_utilities/common/utils.py index f6975c452cdd..325553f536af 100644 --- a/src/sonic-dhcp-utilities/dhcp_utilities/common/utils.py +++ b/src/sonic-dhcp-utilities/dhcp_utilities/common/utils.py @@ -160,8 +160,11 @@ def get_target_process_cmds(process_name): """ res = [] for proc in psutil.process_iter(): - if proc.name() == process_name: - res.append(proc.cmdline()) + try: + if proc.name() == process_name: + res.append(proc.cmdline()) + except psutil.NoSuchProcess: + continue return res diff --git a/src/sonic-dhcp-utilities/dhcp_utilities/dhcprelayd/dhcprelayd.py b/src/sonic-dhcp-utilities/dhcp_utilities/dhcprelayd/dhcprelayd.py index 66139500d0a5..01e6d5499cf5 100644 --- a/src/sonic-dhcp-utilities/dhcp_utilities/dhcprelayd/dhcprelayd.py +++ b/src/sonic-dhcp-utilities/dhcp_utilities/dhcprelayd/dhcprelayd.py @@ -319,16 +319,19 @@ def _kill_exist_relay_releated_process(self, new_dhcp_interfaces, process_name, # Get old dhcrelay process and get old dhcp interfaces for proc in psutil.process_iter(): - if proc.name() == process_name: - cmds = proc.cmdline() - index = 0 - target_procs.append(proc) - while index < len(cmds): - if cmds[index] == "-id": - old_dhcp_interfaces.add(cmds[index + 1]) - index += 2 - else: - index += 1 + try: + if proc.name() == process_name: + cmds = proc.cmdline() + index = 0 + target_procs.append(proc) + while index < len(cmds): + if cmds[index] == "-id": + old_dhcp_interfaces.add(cmds[index + 1]) + index += 2 + else: + index += 1 + except psutil.NoSuchProcess: + continue if len(target_procs) == 0: return NOT_FOUND_PROC diff --git a/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcpservd.py b/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcpservd.py index e8341e436b7c..73a928eb08c9 100644 --- a/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcpservd.py +++ b/src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcpservd.py @@ -39,9 +39,12 @@ def _notify_kea_dhcp4_proc(self): Send SIGHUP signal to kea-dhcp4 process """ for proc in psutil.process_iter(): - if KEA_DHCP4_PROC_NAME in proc.name(): - proc.send_signal(signal.SIGHUP) - break + try: + if KEA_DHCP4_PROC_NAME in proc.name(): + proc.send_signal(signal.SIGHUP) + break + except psutil.NoSuchProcess: + continue def dump_dhcp4_config(self): """ diff --git a/src/sonic-dhcp-utilities/tests/common_utils.py b/src/sonic-dhcp-utilities/tests/common_utils.py index 6f3c07a174f8..f5eb4e018f86 100644 --- a/src/sonic-dhcp-utilities/tests/common_utils.py +++ b/src/sonic-dhcp-utilities/tests/common_utils.py @@ -65,17 +65,22 @@ def mock_get_config_db_table(table_name): class MockProc(object): - def __init__(self, name, pid=None, status=psutil.STATUS_RUNNING): + def __init__(self, name, pid=1, exited=False): self.proc_name = name self.pid = pid + self.exited = exited def name(self): + if self.exited: + raise psutil.NoSuchProcess(self.pid) return self.proc_name def send_signal(self, sig_num): pass def cmdline(self): + if self.exited: + raise psutil.NoSuchProcess(self.pid) if self.proc_name == "dhcrelay": return ["/usr/sbin/dhcrelay", "-d", "-m", "discard", "-a", "%h:%p", "%P", "--name-alias-map-file", "/tmp/port-name-alias-map.txt", "-id", "Vlan1000", "-iu", "docker0", "240.127.1.2"] diff --git a/src/sonic-dhcp-utilities/tests/test_dhcprelayd.py b/src/sonic-dhcp-utilities/tests/test_dhcprelayd.py index 0cc95ad6a21c..4914ff020d63 100644 --- a/src/sonic-dhcp-utilities/tests/test_dhcprelayd.py +++ b/src/sonic-dhcp-utilities/tests/test_dhcprelayd.py @@ -111,6 +111,7 @@ def test_kill_exist_relay_releated_process(mock_swsscommon_dbconnector_init, new process_iter_ret = [] for running_proc in running_procs: process_iter_ret.append(MockProc(running_proc)) + process_iter_ret.append(MockProc("exited_proc", exited=True)) with patch.object(psutil, "process_iter", return_value=process_iter_ret), \ patch.object(ConfigDbEventChecker, "enable"): dhcp_db_connector = DhcpDbConnector() @@ -194,23 +195,23 @@ def test_execute_supervisor_dhcp_relay_process(mock_swsscommon_dbconnector_init, mock_run.assert_called_once_with(["supervisorctl", op, "dhcpmon-Vlan1000"], check=True) -@pytest.mark.parametrize("target_cmds", [[["/usr/bin/dhcrelay"]], [["/usr/bin/dhcpmon"]]]) -def test_check_dhcp_relay_process(mock_swsscommon_dbconnector_init, mock_swsscommon_table_init, target_cmds): - exp_config = {"isc-dhcpv4-relay-Vlan1000": ["/usr/bin/dhcrelay"]} - with patch("dhcp_utilities.dhcprelayd.dhcprelayd.get_target_process_cmds", return_value=target_cmds), \ +@pytest.mark.parametrize("target_procs_cmds", [[["dhcrelay", "-d"]], [["dhcpmon"]]]) +def test_check_dhcp_relay_process(mock_swsscommon_dbconnector_init, mock_swsscommon_table_init, target_procs_cmds): + exp_config = { + "isc-dhcpv4-relay-Vlan1000": ["dhcrelay", "-d"] + } + with patch("dhcp_utilities.dhcprelayd.dhcprelayd.get_target_process_cmds", return_value=target_procs_cmds), \ patch.object(DhcpRelayd, "dhcp_relay_supervisor_config", return_value=exp_config, new_callable=PropertyMock), \ patch.object(sys, "exit", mock_exit_func): dhcp_db_connector = DhcpDbConnector() dhcprelayd = DhcpRelayd(dhcp_db_connector, None) - exp_cmds = [value for key, value in exp_config.items() if "isc-dhcpv4-relay" in key] - exp_cmds.sort() try: dhcprelayd._check_dhcp_relay_processes() except SystemExit: - assert exp_cmds != target_cmds + assert target_procs_cmds[0] != exp_config["isc-dhcpv4-relay-Vlan1000"] else: - assert exp_cmds == target_cmds + assert target_procs_cmds[0] == exp_config["isc-dhcpv4-relay-Vlan1000"] def test_get_dhcp_relay_config(mock_swsscommon_dbconnector_init, mock_swsscommon_table_init): diff --git a/src/sonic-dhcp-utilities/tests/test_dhcpservd.py b/src/sonic-dhcp-utilities/tests/test_dhcpservd.py index 4c8175b7e0bc..b55703aeb416 100644 --- a/src/sonic-dhcp-utilities/tests/test_dhcpservd.py +++ b/src/sonic-dhcp-utilities/tests/test_dhcpservd.py @@ -66,6 +66,7 @@ def test_dump_dhcp4_config(mock_swsscommon_dbconnector_init, enabled_checker): def test_notify_kea_dhcp4_proc(process_list, mock_swsscommon_dbconnector_init, mock_get_render_template, mock_parse_port_map_alias): proc_list = [MockProc(process_name) for process_name in process_list] + proc_list.append(MockProc("exited_proc", exited=True)) with patch.object(psutil, "process_iter", return_value=proc_list), \ patch.object(MockProc, "send_signal", MagicMock()) as mock_send_signal: dhcp_db_connector = DhcpDbConnector() diff --git a/src/sonic-dhcp-utilities/tests/test_utils.py b/src/sonic-dhcp-utilities/tests/test_utils.py index cf69c2f93377..4a41049fe06e 100644 --- a/src/sonic-dhcp-utilities/tests/test_utils.py +++ b/src/sonic-dhcp-utilities/tests/test_utils.py @@ -142,7 +142,9 @@ def test_validate_ttr_type(test_data): def test_get_target_process_cmds(): - with patch.object(psutil, "process_iter", return_value=[MockProc("dhcrelay", 1), MockProc("dhcpmon", 2)], + with patch.object(psutil, "process_iter", return_value=[MockProc("dhcrelay", 1), + MockProc("dhcrelay", 1, exited=True), + MockProc("dhcpmon", 2)], new_callable=PropertyMock): res = utils.get_target_process_cmds("dhcrelay") expected_res = [