Skip to content

Commit

Permalink
[DHCP] Add try/exception when using psutil.process_iter (sonic-net#19537
Browse files Browse the repository at this point in the history
)

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 sonic-net#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
  • Loading branch information
yaqiangz authored Jul 16, 2024
1 parent e2b30cc commit dc3c045
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 25 deletions.
7 changes: 5 additions & 2 deletions src/sonic-dhcp-utilities/dhcp_utilities/common/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
23 changes: 13 additions & 10 deletions src/sonic-dhcp-utilities/dhcp_utilities/dhcprelayd/dhcprelayd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
7 changes: 6 additions & 1 deletion src/sonic-dhcp-utilities/tests/common_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
17 changes: 9 additions & 8 deletions src/sonic-dhcp-utilities/tests/test_dhcprelayd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions src/sonic-dhcp-utilities/tests/test_dhcpservd.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion src/sonic-dhcp-utilities/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down

0 comments on commit dc3c045

Please sign in to comment.