Skip to content

Commit 31a6584

Browse files
maipbuimssonicbld
maipbui
authored and
mssonicbld
committed
Fix sudo config load_mgmt_config fails with error "File /var/run/dhclient.eth0.pid does not exist" (#3149)
* Fix load_mgmt_config not exit when dhclient.eth0.pid not exists Signed-off-by: Mai Bui <maibui@microsoft.com> * add UT Signed-off-by: Mai Bui <maibui@microsoft.com> --------- Signed-off-by: Mai Bui <maibui@microsoft.com>
1 parent 2046e66 commit 31a6584

File tree

2 files changed

+175
-16
lines changed

2 files changed

+175
-16
lines changed

‎config/main.py

+9-11
Original file line numberDiff line numberDiff line change
@@ -1683,17 +1683,15 @@ def load_mgmt_config(filename):
16831683
clicommon.run_command(command, display_cmd=True, ignore_error=True)
16841684
if len(config_data['MGMT_INTERFACE'].keys()) > 0:
16851685
filepath = '/var/run/dhclient.eth0.pid'
1686-
if not os.path.isfile(filepath):
1687-
sys.exit('File {} does not exist'.format(filepath))
1688-
1689-
out0, rc0 = clicommon.run_command(['cat', filepath], display_cmd=True, return_cmd=True)
1690-
if rc0 != 0:
1691-
sys.exit('Exit: {}. Command: cat {} failed.'.format(rc0, filepath))
1692-
1693-
out1, rc1 = clicommon.run_command(['kill', str(out0).strip('\n')], return_cmd=True)
1694-
if rc1 != 0:
1695-
sys.exit('Exit: {}. Command: kill {} failed.'.format(rc1, out0))
1696-
clicommon.run_command(['rm', '-f', filepath], display_cmd=True, return_cmd=True)
1686+
if os.path.isfile(filepath):
1687+
out0, rc0 = clicommon.run_command(['cat', filepath], display_cmd=True, return_cmd=True)
1688+
if rc0 != 0:
1689+
sys.exit('Exit: {}. Command: cat {} failed.'.format(rc0, filepath))
1690+
1691+
out1, rc1 = clicommon.run_command(['kill', str(out0).strip('\n')], display_cmd=True, return_cmd=True)
1692+
if rc1 != 0:
1693+
sys.exit('Exit: {}. Command: kill {} failed.'.format(rc1, out0))
1694+
clicommon.run_command(['rm', '-f', filepath], display_cmd=True, return_cmd=True)
16971695
click.echo("Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.")
16981696

16991697
@config.command("load_minigraph")

‎tests/config_test.py

+166-5
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@
7070
Running command: ifconfig eth0 10.0.0.100 netmask 255.255.255.0
7171
Running command: ip route add default via 10.0.0.1 dev eth0 table default
7272
Running command: ip rule add from 10.0.0.100 table default
73-
Running command: kill `cat /var/run/dhclient.eth0.pid`
73+
Running command: cat /var/run/dhclient.eth0.pid
74+
Running command: kill 101
7475
Running command: rm -f /var/run/dhclient.eth0.pid
7576
Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.
7677
"""
@@ -82,7 +83,8 @@
8283
Running command: ifconfig eth0 add fc00:1::32/64
8384
Running command: ip -6 route add default via fc00:1::1 dev eth0 table default
8485
Running command: ip -6 rule add from fc00:1::32 table default
85-
Running command: kill `cat /var/run/dhclient.eth0.pid`
86+
Running command: cat /var/run/dhclient.eth0.pid
87+
Running command: kill 101
8688
Running command: rm -f /var/run/dhclient.eth0.pid
8789
Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.
8890
"""
@@ -97,11 +99,41 @@
9799
Running command: ifconfig eth0 add fc00:1::32/64
98100
Running command: ip -6 route add default via fc00:1::1 dev eth0 table default
99101
Running command: ip -6 rule add from fc00:1::32 table default
100-
Running command: kill `cat /var/run/dhclient.eth0.pid`
102+
Running command: cat /var/run/dhclient.eth0.pid
103+
Running command: kill 101
101104
Running command: rm -f /var/run/dhclient.eth0.pid
102105
Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.
103106
"""
104107

108+
load_mgmt_config_command_ipv4_ipv6_cat_failed_output="""\
109+
Running command: /usr/local/bin/sonic-cfggen -M device_desc.xml --write-to-db
110+
parse dummy device_desc.xml
111+
change hostname to dummy
112+
Running command: ifconfig eth0 10.0.0.100 netmask 255.255.255.0
113+
Running command: ip route add default via 10.0.0.1 dev eth0 table default
114+
Running command: ip rule add from 10.0.0.100 table default
115+
Running command: ifconfig eth0 add fc00:1::32/64
116+
Running command: ip -6 route add default via fc00:1::1 dev eth0 table default
117+
Running command: ip -6 rule add from fc00:1::32 table default
118+
Running command: cat /var/run/dhclient.eth0.pid
119+
Exit: 2. Command: cat /var/run/dhclient.eth0.pid failed.
120+
"""
121+
122+
load_mgmt_config_command_ipv4_ipv6_kill_failed_output="""\
123+
Running command: /usr/local/bin/sonic-cfggen -M device_desc.xml --write-to-db
124+
parse dummy device_desc.xml
125+
change hostname to dummy
126+
Running command: ifconfig eth0 10.0.0.100 netmask 255.255.255.0
127+
Running command: ip route add default via 10.0.0.1 dev eth0 table default
128+
Running command: ip rule add from 10.0.0.100 table default
129+
Running command: ifconfig eth0 add fc00:1::32/64
130+
Running command: ip -6 route add default via fc00:1::1 dev eth0 table default
131+
Running command: ip -6 rule add from fc00:1::32 table default
132+
Running command: cat /var/run/dhclient.eth0.pid
133+
Running command: kill 104
134+
Exit: 4. Command: kill 104 failed.
135+
"""
136+
105137
RELOAD_CONFIG_DB_OUTPUT = """\
106138
Stopping SONiC target ...
107139
Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json --write-to-db
@@ -143,8 +175,6 @@ def mock_run_command_side_effect(*args, **kwargs):
143175
command = ' '.join(command)
144176

145177
if kwargs.get('display_cmd'):
146-
if 'cat /var/run/dhclient.eth0.pid' in command:
147-
command = 'kill `cat /var/run/dhclient.eth0.pid`'
148178
click.echo(click.style("Running command: ", fg='cyan') + click.style(command, fg='green'))
149179

150180
if kwargs.get('return_cmd'):
@@ -154,6 +184,54 @@ def mock_run_command_side_effect(*args, **kwargs):
154184
return 'sonic.target\nswss', 0
155185
elif command == "systemctl is-enabled snmp.timer":
156186
return 'enabled', 0
187+
elif command == 'cat /var/run/dhclient.eth0.pid':
188+
return '101', 0
189+
else:
190+
return '', 0
191+
192+
def mock_run_command_cat_failed_side_effect(*args, **kwargs):
193+
command = args[0]
194+
if isinstance(command, str):
195+
command = command
196+
elif isinstance(command, list):
197+
command = ' '.join(command)
198+
199+
if kwargs.get('display_cmd'):
200+
click.echo(click.style("Running command: ", fg='cyan') + click.style(command, fg='green'))
201+
202+
if kwargs.get('return_cmd'):
203+
if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
204+
return 'snmp.timer', 0
205+
elif command == "systemctl list-dependencies --plain sonic.target":
206+
return 'sonic.target\nswss', 0
207+
elif command == "systemctl is-enabled snmp.timer":
208+
return 'enabled', 0
209+
elif command == 'cat /var/run/dhclient.eth0.pid':
210+
return '102', 2
211+
else:
212+
return '', 0
213+
214+
def mock_run_command_kill_failed_side_effect(*args, **kwargs):
215+
command = args[0]
216+
if isinstance(command, str):
217+
command = command
218+
elif isinstance(command, list):
219+
command = ' '.join(command)
220+
221+
if kwargs.get('display_cmd'):
222+
click.echo(click.style("Running command: ", fg='cyan') + click.style(command, fg='green'))
223+
224+
if kwargs.get('return_cmd'):
225+
if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
226+
return 'snmp.timer', 0
227+
elif command == "systemctl list-dependencies --plain sonic.target":
228+
return 'sonic.target\nswss', 0
229+
elif command == "systemctl is-enabled snmp.timer":
230+
return 'enabled', 0
231+
elif command == 'cat /var/run/dhclient.eth0.pid':
232+
return '104', 0
233+
elif command == 'kill 104':
234+
return 'Failed to kill 104', 4
157235
else:
158236
return '', 0
159237

@@ -1663,6 +1741,89 @@ def change_hostname_side_effect(hostname):
16631741
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) == expected_output
16641742
assert mock_run_command.call_count == expected_command_call_count
16651743

1744+
1745+
def test_config_load_mgmt_config_ipv4_ipv6_cat_failed(self, get_cmd_module, setup_single_broadcom_asic):
1746+
device_desc_result = {
1747+
'DEVICE_METADATA': {
1748+
'localhost': {
1749+
'hostname': 'dummy'
1750+
}
1751+
},
1752+
'MGMT_INTERFACE': {
1753+
('eth0', '10.0.0.100/24') : {
1754+
'gwaddr': ipaddress.ip_address(u'10.0.0.1')
1755+
},
1756+
('eth0', 'FC00:1::32/64') : {
1757+
'gwaddr': ipaddress.ip_address(u'fc00:1::1')
1758+
}
1759+
}
1760+
}
1761+
self.check_output_cat_failed(get_cmd_module, device_desc_result, load_mgmt_config_command_ipv4_ipv6_cat_failed_output, 8)
1762+
1763+
def check_output_cat_failed(self, get_cmd_module, parse_device_desc_xml_result, expected_output, expected_command_call_count):
1764+
def parse_device_desc_xml_side_effect(filename):
1765+
print("parse dummy device_desc.xml")
1766+
return parse_device_desc_xml_result
1767+
def change_hostname_side_effect(hostname):
1768+
print("change hostname to {}".format(hostname))
1769+
with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_cat_failed_side_effect)) as mock_run_command:
1770+
with mock.patch('os.path.isfile', mock.MagicMock(return_value=True)):
1771+
with mock.patch('config.main.parse_device_desc_xml', mock.MagicMock(side_effect=parse_device_desc_xml_side_effect)):
1772+
with mock.patch('config.main._change_hostname', mock.MagicMock(side_effect=change_hostname_side_effect)):
1773+
(config, show) = get_cmd_module
1774+
runner = CliRunner()
1775+
with runner.isolated_filesystem():
1776+
with open('device_desc.xml', 'w') as f:
1777+
f.write('dummy')
1778+
result = runner.invoke(config.config.commands["load_mgmt_config"], ["-y", "device_desc.xml"])
1779+
print(result.exit_code)
1780+
print(result.output)
1781+
traceback.print_tb(result.exc_info[2])
1782+
assert result.exit_code == 1
1783+
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) == expected_output
1784+
assert mock_run_command.call_count == expected_command_call_count
1785+
1786+
def test_config_load_mgmt_config_ipv4_ipv6_kill_failed(self, get_cmd_module, setup_single_broadcom_asic):
1787+
device_desc_result = {
1788+
'DEVICE_METADATA': {
1789+
'localhost': {
1790+
'hostname': 'dummy'
1791+
}
1792+
},
1793+
'MGMT_INTERFACE': {
1794+
('eth0', '10.0.0.100/24') : {
1795+
'gwaddr': ipaddress.ip_address(u'10.0.0.1')
1796+
},
1797+
('eth0', 'FC00:1::32/64') : {
1798+
'gwaddr': ipaddress.ip_address(u'fc00:1::1')
1799+
}
1800+
}
1801+
}
1802+
self.check_output_kill_failed(get_cmd_module, device_desc_result, load_mgmt_config_command_ipv4_ipv6_kill_failed_output, 9)
1803+
1804+
def check_output_kill_failed(self, get_cmd_module, parse_device_desc_xml_result, expected_output, expected_command_call_count):
1805+
def parse_device_desc_xml_side_effect(filename):
1806+
print("parse dummy device_desc.xml")
1807+
return parse_device_desc_xml_result
1808+
def change_hostname_side_effect(hostname):
1809+
print("change hostname to {}".format(hostname))
1810+
with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_kill_failed_side_effect)) as mock_run_command:
1811+
with mock.patch('os.path.isfile', mock.MagicMock(return_value=True)):
1812+
with mock.patch('config.main.parse_device_desc_xml', mock.MagicMock(side_effect=parse_device_desc_xml_side_effect)):
1813+
with mock.patch('config.main._change_hostname', mock.MagicMock(side_effect=change_hostname_side_effect)):
1814+
(config, show) = get_cmd_module
1815+
runner = CliRunner()
1816+
with runner.isolated_filesystem():
1817+
with open('device_desc.xml', 'w') as f:
1818+
f.write('dummy')
1819+
result = runner.invoke(config.config.commands["load_mgmt_config"], ["-y", "device_desc.xml"])
1820+
print(result.exit_code)
1821+
print(result.output)
1822+
traceback.print_tb(result.exc_info[2])
1823+
assert result.exit_code == 1
1824+
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) == expected_output
1825+
assert mock_run_command.call_count == expected_command_call_count
1826+
16661827
@classmethod
16671828
def teardown_class(cls):
16681829
print("TEARDOWN")

0 commit comments

Comments
 (0)