Skip to content

Fix syntax and semantic errors in kdump remote feature #4114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 22, 2025
Merged

Fix syntax and semantic errors in kdump remote feature #4114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 9 additions & 15 deletions scripts/sonic-kdump-config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,13 +447,11 @@ def write_kdump_remote():

if remote:
# Uncomment SSH and SSH_KEY in the /etc/default/kdump-tools file
run_command("/bin/sed -i 's/#SSH/SSH/' %s" % kdump_cfg, use_shell=False)
run_command("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' %s" % kdump_cfg, use_shell=False)
run_command("/bin/sed -i 's/^#SSH/SSH/' %s" % kdump_cfg, use_shell=False)
print("SSH and SSH_KEY uncommented for remote configuration.")
else:
# Comment out SSH and SSH_KEY in the /etc/default/kdump-tools file
run_command("/bin/sed -i 's/SSH/#SSH/' %s" % kdump_cfg, use_shell=False)
run_command("/bin/sed -i 's/SSH_KEY/#SSH_KEY/' %s" % kdump_cfg, use_shell=False)
run_command("/bin/sed -i 's/^SSH/#SSH/' %s" % kdump_cfg, use_shell=False)
print("SSH and SSH_KEY commented out for local configuration.")

def read_ssh_string():
Expand Down Expand Up @@ -513,7 +511,7 @@ def read_ssh_path():
sys.exit(1)

def write_ssh_path(ssh_path):
cmd = "/bin/sed -i -e 's/#*SSH_KEY=.*/SSH_KEY=\"%s\"/' %s" % (ssh_path, kdump_cfg)
cmd = "/bin/sed -i -e 's|#*SSH_KEY=.*|SSH_KEY=\"%s\"|' %s" % (ssh_path, kdump_cfg)

(rc, lines, err_str) = run_command(cmd, use_shell=False) # Make sure to set use_shell=True
if rc == 0 and type(lines) == list and len(lines) == 0:
Expand Down Expand Up @@ -772,14 +770,12 @@ def cmd_kdump_remote(verbose):

if remote:
# Uncomment SSH and SSH_KEY in the /etc/default/kdump-tools file
run_command("/bin/sed -i 's/#SSH/SSH/' /etc/default/kdump-tools", use_shell=False)
run_command("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' /etc/default/kdump-tools", use_shell=False)
run_command("/bin/sed -i 's/^#SSH/SSH/' /etc/default/kdump-tools", use_shell=False)
if verbose:
print("SSH and SSH_KEY uncommented for remote configuration.")
else:
# Comment out SSH and SSH_KEY in the /etc/default/kdump-tools file
run_command("/bin/sed -i 's/SSH/#SSH/' /etc/default/kdump-tools", use_shell=False)
run_command("/bin/sed -i 's/SSH_KEY/#SSH_KEY/' /etc/default/kdump-tools", use_shell=False)
run_command("/bin/sed -i 's/^SSH/#SSH/' /etc/default/kdump-tools", use_shell=False)
if verbose:
print("SSH and SSH_KEY commented out for local configuration.")

Expand All @@ -798,10 +794,8 @@ def cmd_kdump_ssh_path(verbose, ssh_path):
(rc, lines, err_str) = run_command("show kdump ssh_path", use_shell=False)
print('\n'.join(lines))
else:
current_ssh_path = read_ssh_path()
if current_ssh_path != ssh_path:
write_ssh_path(ssh_path)
print("SSH path updated. Changes will take effect after reboot.")
write_ssh_path(ssh_path)
print("SSH path updated. Changes will take effect after reboot.")

def main():

Expand Down Expand Up @@ -846,7 +840,7 @@ def main():
help='Maximum number of kernel dump files stored')

parser.add_argument('--remote', action='store_true', default=False,
help='Enable remote kdump via SSH')
help='Update remote kdump via SSH based on CONFIG_DB')

parser.add_argument('--ssh_string', nargs='?', type=str, action='store', default=False,
help='ssh_string for remote kdump')
Expand Down Expand Up @@ -892,7 +886,7 @@ def main():
elif options.ssh_string:
cmd_kdump_ssh_string(options.verbose, options.ssh_string)
elif options.ssh_path:
cmd_kdump_ssh_path(options.verbos, options.ssh_path)
cmd_kdump_ssh_path(options.verbose, options.ssh_path)
elif options.num_dumps != False:
cmd_kdump_num_dumps(options.verbose, options.num_dumps)
elif options.dump_db:
Expand Down
35 changes: 7 additions & 28 deletions tests/sonic_kdump_config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,8 @@ def test_write_kdump_remote_true(self, mock_read_remote, mock_run_command):
sonic_kdump_config.write_kdump_remote() # Call the function

# Ensure the correct commands were run to uncomment SSH and SSH_KEY
mock_run_command.assert_any_call("/bin/sed -i 's/#SSH/SSH/' /etc/default/kdump-tools", use_shell=False)
mock_run_command.assert_any_call("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' /etc/default/kdump-tools", use_shell=False)
self.assertEqual(mock_run_command.call_count, 2) # Ensure both commands were called
mock_run_command.assert_any_call("/bin/sed -i 's/^#SSH/SSH/' /etc/default/kdump-tools", use_shell=False)
self.assertEqual(mock_run_command.call_count, 1) # Ensure the sed command was called once

@patch("sonic_kdump_config.run_command")
@patch("sonic_kdump_config.get_kdump_remote")
Expand All @@ -408,9 +407,8 @@ def test_write_kdump_remote_false(self, mock_read_remote, mock_run_command):
sonic_kdump_config.write_kdump_remote() # Call the function

# Ensure the correct commands were run to comment SSH and SSH_KEY
mock_run_command.assert_any_call("/bin/sed -i 's/SSH/#SSH/' /etc/default/kdump-tools", use_shell=False)
mock_run_command.assert_any_call("/bin/sed -i 's/SSH_KEY/#SSH_KEY/' /etc/default/kdump-tools", use_shell=False)
self.assertEqual(mock_run_command.call_count, 2)
mock_run_command.assert_any_call("/bin/sed -i 's/^SSH/#SSH/' /etc/default/kdump-tools", use_shell=False)
self.assertEqual(mock_run_command.call_count, 1)

@patch("sonic_kdump_config.get_kdump_remote")
@patch("sonic_kdump_config.run_command")
Expand All @@ -422,16 +420,14 @@ def test_cmd_kdump_remote(self, mock_run_command, mock_read_remote):
sonic_kdump_config.cmd_kdump_remote(verbose=True)

# Ensure the correct commands are being run
mock_run_command.assert_any_call("/bin/sed -i 's/#SSH/SSH/' /etc/default/kdump-tools", use_shell=False)
mock_run_command.assert_any_call("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' /etc/default/kdump-tools", use_shell=False)
mock_run_command.assert_any_call("/bin/sed -i 's/^#SSH/SSH/' /etc/default/kdump-tools", use_shell=False)

# Test case: Remote is False
mock_read_remote.return_value = False
sonic_kdump_config.cmd_kdump_remote(verbose=True)

# Ensure the correct commands are being run
mock_run_command.assert_any_call("/bin/sed -i 's/SSH/#SSH/' /etc/default/kdump-tools", use_shell=False)
mock_run_command.assert_any_call("/bin/sed -i 's/SSH_KEY/#SSH_KEY/' /etc/default/kdump-tools", use_shell=False)
mock_run_command.assert_any_call("/bin/sed -i 's/^SSH/#SSH/' /etc/default/kdump-tools", use_shell=False)

# Test case: Checking output messages
with patch("builtins.print") as mock_print:
Expand Down Expand Up @@ -538,7 +534,7 @@ def test_write_ssh_path(self, mock_read_ssh_path, mock_run_cmd):
sonic_kdump_config.write_ssh_path('/path/to/keys') # Call function with valid path
# Ensure the correct command is being run
expected_cmd = (
"/bin/sed -i -e 's/#*SSH_KEY=.*/SSH_KEY=\"/path/to/keys\"/' %s"
"/bin/sed -i -e 's|#*SSH_KEY=.*|SSH_KEY=\"/path/to/keys\"|' %s"
% sonic_kdump_config.kdump_cfg
)
mock_run_cmd.assert_called_once_with(expected_cmd, use_shell=False)
Expand Down Expand Up @@ -670,23 +666,6 @@ def test_cmd_kdump_ssh_path_update(self, mock_print, mock_write, mock_read, mock
# Verify that the correct message is printed
mock_print.assert_called_once_with("SSH path updated. Changes will take effect after reboot.")

@patch('sonic_kdump_config.run_command')
@patch('sonic_kdump_config.read_ssh_path')
@patch('sonic_kdump_config.write_ssh_path')
@patch('builtins.print') # Mock print to capture printed output
def test_cmd_kdump_ssh_path_no_update(self, mock_print, mock_write, mock_read, mock_run):
# Mock read_ssh_path to return the same SSH path provided
mock_read.return_value = '/same/path/to/keys'

# Call the function with the same SSH path
sonic_kdump_config.cmd_kdump_ssh_path(verbose=True, ssh_path='/same/path/to/keys')

# Check that write_ssh_path was not called
mock_write.assert_not_called()

# Check that no message is printed for update
mock_print.assert_not_called()

@patch("sonic_kdump_config.write_use_kdump")
@patch("os.path.exists")
def test_kdump_disable(self, mock_path_exist, mock_write_kdump):
Expand Down
Loading