Skip to content

Commit

Permalink
Fix sudo config load_mgmt_config fails with error "File /var/run/dh…
Browse files Browse the repository at this point in the history 
…client.eth0.pid does not exist" (sonic-net#3149)

* Fix load_mgmt_config not exit when dhclient.eth0.pid not exists

Signed-off-by: Mai Bui <[email protected]>

* add UT

Signed-off-by: Mai Bui <[email protected]>

---------

Signed-off-by: Mai Bui <[email protected]>
  • Loading branch information
@maipbui @mssonicbld
maipbui authored and mssonicbld committed Feb 6, 2024
1 parent c5e30e3 commit c5f5342
Show file tree
Hide file tree
Showing 2 changed files with 175 additions and 16 deletions.
20 changes: 9 additions & 11 deletions config/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1679,17 +1679,15 @@ def load_mgmt_config(filename):
clicommon.run_command(command, display_cmd=True, ignore_error=True)
if len(config_data['MGMT_INTERFACE'].keys()) > 0:
filepath = '/var/run/dhclient.eth0.pid'
if not os.path.isfile(filepath):
sys.exit('File {} does not exist'.format(filepath))

out0, rc0 = clicommon.run_command(['cat', filepath], display_cmd=True, return_cmd=True)
if rc0 != 0:
sys.exit('Exit: {}. Command: cat {} failed.'.format(rc0, filepath))

out1, rc1 = clicommon.run_command(['kill', str(out0).strip('\n')], return_cmd=True)
if rc1 != 0:
sys.exit('Exit: {}. Command: kill {} failed.'.format(rc1, out0))
clicommon.run_command(['rm', '-f', filepath], display_cmd=True, return_cmd=True)
if os.path.isfile(filepath):
out0, rc0 = clicommon.run_command(['cat', filepath], display_cmd=True, return_cmd=True)
if rc0 != 0:
sys.exit('Exit: {}. Command: cat {} failed.'.format(rc0, filepath))

out1, rc1 = clicommon.run_command(['kill', str(out0).strip('\n')], display_cmd=True, return_cmd=True)
if rc1 != 0:
sys.exit('Exit: {}. Command: kill {} failed.'.format(rc1, out0))
clicommon.run_command(['rm', '-f', filepath], display_cmd=True, return_cmd=True)
click.echo("Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.")

@config.command("load_minigraph")
Expand Down
171 changes: 166 additions & 5 deletions tests/config_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@
Running command: ifconfig eth0 10.0.0.100 netmask 255.255.255.0
Running command: ip route add default via 10.0.0.1 dev eth0 table default
Running command: ip rule add from 10.0.0.100 table default
Running command: kill `cat /var/run/dhclient.eth0.pid`
Running command: cat /var/run/dhclient.eth0.pid
Running command: kill 101
Running command: rm -f /var/run/dhclient.eth0.pid
Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.
"""
Expand All @@ -82,7 +83,8 @@
Running command: ifconfig eth0 add fc00:1::32/64
Running command: ip -6 route add default via fc00:1::1 dev eth0 table default
Running command: ip -6 rule add from fc00:1::32 table default
Running command: kill `cat /var/run/dhclient.eth0.pid`
Running command: cat /var/run/dhclient.eth0.pid
Running command: kill 101
Running command: rm -f /var/run/dhclient.eth0.pid
Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.
"""
Expand All @@ -97,11 +99,41 @@
Running command: ifconfig eth0 add fc00:1::32/64
Running command: ip -6 route add default via fc00:1::1 dev eth0 table default
Running command: ip -6 rule add from fc00:1::32 table default
Running command: kill `cat /var/run/dhclient.eth0.pid`
Running command: cat /var/run/dhclient.eth0.pid
Running command: kill 101
Running command: rm -f /var/run/dhclient.eth0.pid
Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.
"""

load_mgmt_config_command_ipv4_ipv6_cat_failed_output="""\
Running command: /usr/local/bin/sonic-cfggen -M device_desc.xml --write-to-db
parse dummy device_desc.xml
change hostname to dummy
Running command: ifconfig eth0 10.0.0.100 netmask 255.255.255.0
Running command: ip route add default via 10.0.0.1 dev eth0 table default
Running command: ip rule add from 10.0.0.100 table default
Running command: ifconfig eth0 add fc00:1::32/64
Running command: ip -6 route add default via fc00:1::1 dev eth0 table default
Running command: ip -6 rule add from fc00:1::32 table default
Running command: cat /var/run/dhclient.eth0.pid
Exit: 2. Command: cat /var/run/dhclient.eth0.pid failed.
"""

load_mgmt_config_command_ipv4_ipv6_kill_failed_output="""\
Running command: /usr/local/bin/sonic-cfggen -M device_desc.xml --write-to-db
parse dummy device_desc.xml
change hostname to dummy
Running command: ifconfig eth0 10.0.0.100 netmask 255.255.255.0
Running command: ip route add default via 10.0.0.1 dev eth0 table default
Running command: ip rule add from 10.0.0.100 table default
Running command: ifconfig eth0 add fc00:1::32/64
Running command: ip -6 route add default via fc00:1::1 dev eth0 table default
Running command: ip -6 rule add from fc00:1::32 table default
Running command: cat /var/run/dhclient.eth0.pid
Running command: kill 104
Exit: 4. Command: kill 104 failed.
"""

RELOAD_CONFIG_DB_OUTPUT = """\
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json --write-to-db
Expand Down Expand Up @@ -143,8 +175,6 @@ def mock_run_command_side_effect(*args, **kwargs):
command = ' '.join(command)

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

if kwargs.get('return_cmd'):
Expand All @@ -154,6 +184,54 @@ def mock_run_command_side_effect(*args, **kwargs):
return 'sonic.target\nswss', 0
elif command == "systemctl is-enabled snmp.timer":
return 'enabled', 0
elif command == 'cat /var/run/dhclient.eth0.pid':
return '101', 0
else:
return '', 0

def mock_run_command_cat_failed_side_effect(*args, **kwargs):
command = args[0]
if isinstance(command, str):
command = command
elif isinstance(command, list):
command = ' '.join(command)

if kwargs.get('display_cmd'):
click.echo(click.style("Running command: ", fg='cyan') + click.style(command, fg='green'))

if kwargs.get('return_cmd'):
if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
return 'snmp.timer', 0
elif command == "systemctl list-dependencies --plain sonic.target":
return 'sonic.target\nswss', 0
elif command == "systemctl is-enabled snmp.timer":
return 'enabled', 0
elif command == 'cat /var/run/dhclient.eth0.pid':
return '102', 2
else:
return '', 0

def mock_run_command_kill_failed_side_effect(*args, **kwargs):
command = args[0]
if isinstance(command, str):
command = command
elif isinstance(command, list):
command = ' '.join(command)

if kwargs.get('display_cmd'):
click.echo(click.style("Running command: ", fg='cyan') + click.style(command, fg='green'))

if kwargs.get('return_cmd'):
if command == "systemctl list-dependencies --plain sonic-delayed.target | sed '1d'":
return 'snmp.timer', 0
elif command == "systemctl list-dependencies --plain sonic.target":
return 'sonic.target\nswss', 0
elif command == "systemctl is-enabled snmp.timer":
return 'enabled', 0
elif command == 'cat /var/run/dhclient.eth0.pid':
return '104', 0
elif command == 'kill 104':
return 'Failed to kill 104', 4
else:
return '', 0

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


def test_config_load_mgmt_config_ipv4_ipv6_cat_failed(self, get_cmd_module, setup_single_broadcom_asic):
device_desc_result = {
'DEVICE_METADATA': {
'localhost': {
'hostname': 'dummy'
}
},
'MGMT_INTERFACE': {
('eth0', '10.0.0.100/24') : {
'gwaddr': ipaddress.ip_address(u'10.0.0.1')
},
('eth0', 'FC00:1::32/64') : {
'gwaddr': ipaddress.ip_address(u'fc00:1::1')
}
}
}
self.check_output_cat_failed(get_cmd_module, device_desc_result, load_mgmt_config_command_ipv4_ipv6_cat_failed_output, 8)

def check_output_cat_failed(self, get_cmd_module, parse_device_desc_xml_result, expected_output, expected_command_call_count):
def parse_device_desc_xml_side_effect(filename):
print("parse dummy device_desc.xml")
return parse_device_desc_xml_result
def change_hostname_side_effect(hostname):
print("change hostname to {}".format(hostname))
with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_cat_failed_side_effect)) as mock_run_command:
with mock.patch('os.path.isfile', mock.MagicMock(return_value=True)):
with mock.patch('config.main.parse_device_desc_xml', mock.MagicMock(side_effect=parse_device_desc_xml_side_effect)):
with mock.patch('config.main._change_hostname', mock.MagicMock(side_effect=change_hostname_side_effect)):
(config, show) = get_cmd_module
runner = CliRunner()
with runner.isolated_filesystem():
with open('device_desc.xml', 'w') as f:
f.write('dummy')
result = runner.invoke(config.config.commands["load_mgmt_config"], ["-y", "device_desc.xml"])
print(result.exit_code)
print(result.output)
traceback.print_tb(result.exc_info[2])
assert result.exit_code == 1
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) == expected_output
assert mock_run_command.call_count == expected_command_call_count

def test_config_load_mgmt_config_ipv4_ipv6_kill_failed(self, get_cmd_module, setup_single_broadcom_asic):
device_desc_result = {
'DEVICE_METADATA': {
'localhost': {
'hostname': 'dummy'
}
},
'MGMT_INTERFACE': {
('eth0', '10.0.0.100/24') : {
'gwaddr': ipaddress.ip_address(u'10.0.0.1')
},
('eth0', 'FC00:1::32/64') : {
'gwaddr': ipaddress.ip_address(u'fc00:1::1')
}
}
}
self.check_output_kill_failed(get_cmd_module, device_desc_result, load_mgmt_config_command_ipv4_ipv6_kill_failed_output, 9)

def check_output_kill_failed(self, get_cmd_module, parse_device_desc_xml_result, expected_output, expected_command_call_count):
def parse_device_desc_xml_side_effect(filename):
print("parse dummy device_desc.xml")
return parse_device_desc_xml_result
def change_hostname_side_effect(hostname):
print("change hostname to {}".format(hostname))
with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_kill_failed_side_effect)) as mock_run_command:
with mock.patch('os.path.isfile', mock.MagicMock(return_value=True)):
with mock.patch('config.main.parse_device_desc_xml', mock.MagicMock(side_effect=parse_device_desc_xml_side_effect)):
with mock.patch('config.main._change_hostname', mock.MagicMock(side_effect=change_hostname_side_effect)):
(config, show) = get_cmd_module
runner = CliRunner()
with runner.isolated_filesystem():
with open('device_desc.xml', 'w') as f:
f.write('dummy')
result = runner.invoke(config.config.commands["load_mgmt_config"], ["-y", "device_desc.xml"])
print(result.exit_code)
print(result.output)
traceback.print_tb(result.exc_info[2])
assert result.exit_code == 1
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) == expected_output
assert mock_run_command.call_count == expected_command_call_count

@classmethod
def teardown_class(cls):
print("TEARDOWN")
Expand Down

0 comments on commit c5f5342

Please sign in to comment.