Fix syntax and semantic errors in kdump remote feature (#4114)

Author: arista-hpandya

scripts/sonic-kdump-config

@@ -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():
@@ -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:
@@ -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.")
 
@@ -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():
 
@@ -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')
@@ -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:

tests/sonic_kdump_config_test.py

@@ -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")
@@ -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")
@@ -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:
@@ -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)
@@ -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):