Fixed: Kdump Remote Patch (sonic-net#3835)

Link to all PRs of the KDUMP = KDUMP SONiC HLD PRs

Related sonic-utilities: Fixing remote feature build failure

Issue: There was a build failure for the remote variable in kdump-sonic-utilities PR

Following were to be addressed, which are ADDRESSED in this PR

```
    E 2025 Apr 1 04:56:08.321576 vlab-02 ERR hostcfgd: ['sonic-kdump-config', '--disable'] - failed: return code - 1, output:#012None
    E 2025 Apr 1 04:56:09.638838 vlab-02 ERR hostcfgd: ['sonic-kdump-config', '--memory', '0M-2G:256M,2G-4G:320M,4G-8G:384M,8G-:448M'] - failed: return code - 1, output:#012None
    E 2025 Apr 1 04:56:11.199498 vlab-02 ERR hostcfgd: ['sonic-kdump-config', '--num_dumps', '3'] - failed: return code - 1, output:#012None
    E 2025 Apr 1 04:56:13.969595 vlab-02 ERR hostcfgd: ['sonic-kdump-config', '--remote', 'false'] - failed: return code - 2, output:#012None
    E 2025 Apr 1 04:56:16.373762 vlab-02 ERR hostcfgd: ['sonic-kdump-config', '--ssh_string', 'user@localhost'] - failed: return code - 1, output:#012None
    E 2025 Apr 1 04:56:17.579657 vlab-02 ERR hostcfgd: ['sonic-kdump-config', '--ssh_path', '/a/b/c'] - failed: return code - 1, output:#012None

Author: muhammadalihussnain

scripts/sonic-kdump-config

@@ -168,6 +168,22 @@ def cmd_dump_db():
             else:
                 print("empty")
 
+def get_kdump_remote():
+    """Get the value of the remote feature from the KDUMP configuration."""
+    config_db = ConfigDBConnector(use_unix_socket_path=True)
+    if config_db is not None:
+        config_db.connect()
+        table_data = config_db.get_table('KDUMP')
+        if table_data is not None:
+            config_data = table_data.get('config')
+            if config_data is not None:
+                remote_value = config_data.get('remote')
+                if remote_value is not None:
+                    # Return True if the remote value is 'true', otherwise False
+                    return remote_value.lower() == 'true'  # Explicitly return boolean
+
+    return False  # Default return value if no remote setting is found
+
 def cmd_dump_config_json():
     kdump_enabled = get_kdump_administrative_mode()
     kdump_memory = get_kdump_memory()
@@ -320,22 +336,6 @@ def get_kdump_num_dumps():
                     num_dumps = num
     return num_dumps
 
-def get_kdump_remote():
-    """Get the value of the remote feature from the KDUMP configuration."""
-    config_db = ConfigDBConnector(use_unix_socket_path=True)
-    if config_db is not None:
-        config_db.connect()
-        table_data = config_db.get_table('KDUMP')
-        if table_data is not None:
-            config_data = table_data.get('config')
-            if config_data is not None:
-                remote_value = config_data.get('remote')
-                if remote_value is not None:
-                    # Return True if the remote value is 'true', otherwise False
-                    return remote_value.lower() == 'true'  # Explicitly return boolean
-
-    return False  # Default return value if no remote setting is found
-
 def get_kdump_ssh_string():
     """Get the SSH string from the KDUMP configuration."""
     config_db = ConfigDBConnector(use_unix_socket_path=True)
@@ -433,7 +433,7 @@ def write_num_dumps(num_dumps):
 def write_kdump_remote():
     # Assuming there's a function that retrieves the remote value from the config database
     remote = get_kdump_remote()  # This function needs to be implemented
-    
+
     kdump_cfg = '/etc/default/kdump-tools'
 
     if remote:
@@ -641,7 +641,7 @@ def cmd_kdump_config_next(verbose):
 
 
 def kdump_disable(verbose, image, booter_config_file_path):
-    """Unloads Kdump kernel and remove parameter `crashkernel=*` from configuration file of 
+    """Unloads Kdump kernel and remove parameter `crashkernel=*` from configuration file of
     kernel boot loader.
 
     Args:
@@ -826,11 +826,15 @@ def main():
     parser.add_argument('--num_dumps', nargs='?', type=int, action='store', default=False,
         help='Maximum number of kernel dump files stored')
 
+    parser.add_argument('--remote', action='store_true', default=False,
+                help='Enable remote kdump via SSH')
+    
     parser.add_argument('--ssh_string', nargs='?', type=str, action='store', default=False,
             help='ssh_string for remote kdump')
 
     parser.add_argument('--ssh_path', nargs='?', type=str, action='store',default=False,
             help='ssh_path for remote kdump')
+    
     # Memory allocated for capture kernel on Current Image
     parser.add_argument('--memory', nargs='?', type=str, action='store', default=False,
         help='Amount of memory reserved for the capture kernel')

tests/sonic_kdump_config_test.py

@@ -4,8 +4,8 @@
 import unittest
 from unittest.mock import patch, mock_open, Mock
 from utilities_common.general import load_module_from_source
-
 from sonic_installer.common import IMAGE_PREFIX
+import argparse
 
 TESTS_DIR_PATH = os.path.dirname(os.path.abspath(__file__))
 UTILITY_DIR_PATH = os.path.dirname(TESTS_DIR_PATH)
@@ -23,6 +23,32 @@
 sonic_kdump_config = load_module_from_source("sonic_kdump_config", sonic_kdump_config_path)
 
 
+class TestRemoteFlag(unittest.TestCase):
+    def setUp(self):
+        # Create a new ArgumentParser for each test
+        self.parser = argparse.ArgumentParser(description="kdump configuration and status tool")
+        self.parser.add_argument('--remote', action='store_true', default=False,
+                                 help='Enable the Kdump remote SSH mechanism')
+
+    def test_remote_flag_provided(self):
+        """Test that the --remote flag sets the remote attribute to True."""
+        with patch.object(sys, 'argv', ['script.py', '--remote']):
+            args = self.parser.parse_args()
+            self.assertTrue(args.remote)
+
+    def test_remote_flag_not_provided(self):
+        """Test that the --remote flag defaults to False when not provided."""
+        with patch.object(sys, 'argv', ['script.py']):
+            args = self.parser.parse_args()
+            self.assertFalse(args.remote)
+
+    def test_remote_flag_with_value(self):
+        """Test that providing a value to the --remote flag raises an error."""
+        with patch.object(sys, 'argv', ['script.py', '--remote', 'some_value']):
+            with self.assertRaises(SystemExit):
+                self.parser.parse_args()
+
+
 class TestSonicKdumpConfig(unittest.TestCase):
     @classmethod
     def setup_class(cls):
@@ -246,7 +272,7 @@ def test_cmd_kdump_ssh_string_none(self, mock_print, mock_read, mock_run):
     def test_cmd_kdump_ssh_string_update(self, mock_print, mock_write, mock_read, mock_run):
         # Mock read_ssh_string to return the current SSH string
         mock_read.return_value = 'old_ssh_string'
-        # Call the function with a new SSH string
+        # Call the function with a new SSH string to configure
         sonic_kdump_config.cmd_kdump_ssh_string(verbose=True, ssh_string='new_ssh_string')
 
         # Check that write_ssh_string was called with the new SSH string
@@ -302,6 +328,63 @@ def test_cmd_kdump_disable(self, mock_path_exist, mock_num_dumps, mock_memory,
             return_result = sonic_kdump_config.cmd_kdump_disable(True)
             assert return_result == False
 
+    @patch("sonic_kdump_config.read_num_dumps")
+    @patch("sonic_kdump_config.run_command")
+    def test_write_num_dumps(self, mock_run_cmd, mock_read_num_dumps):
+        # Success case: correct write and verification
+        mock_run_cmd.side_effect = [(0, [], None)]
+        mock_read_num_dumps.return_value = 5
+        sonic_kdump_config.write_num_dumps(5)
+
+        # Case where run_command returns wrong type
+        mock_run_cmd.side_effect = [(0, (), None)]
+        mock_read_num_dumps.return_value = 5
+        with self.assertRaises(SystemExit) as sys_exit:
+            sonic_kdump_config.write_num_dumps(5)
+        self.assertEqual(sys_exit.exception.code, 1)
+
+        # Case where line is non-empty
+        mock_run_cmd.side_effect = [(0, ["Some output"], None)]
+        mock_read_num_dumps.return_value = 5
+        with self.assertRaises(SystemExit) as sys_exit:
+            sonic_kdump_config.write_num_dumps(5)
+        self.assertEqual(sys_exit.exception.code, 1)
+
+        # Case where read_num_dumps does not match input
+        mock_run_cmd.side_effect = [(0, [], None)]
+        mock_read_num_dumps.return_value = 4
+        with self.assertRaises(SystemExit) as sys_exit:
+            sonic_kdump_config.write_num_dumps(5)
+        self.assertEqual(sys_exit.exception.code, 1)
+
+        # Case where run_command fails
+        mock_run_cmd.side_effect = [(1, [], "Error")]
+        mock_read_num_dumps.return_value = 5
+        with self.assertRaises(SystemExit) as sys_exit:
+            sonic_kdump_config.write_num_dumps(5)
+        self.assertEqual(sys_exit.exception.code, 1)
+
+        # Case where lines contain non-integer
+        mock_run_cmd.side_effect = [(0, ["NotInteger"], None)]
+        mock_read_num_dumps.return_value = 5
+        with self.assertRaises(SystemExit) as sys_exit:
+            sonic_kdump_config.write_num_dumps(5)
+        self.assertEqual(sys_exit.exception.code, 1)
+
+        # Edge case: empty string in output but matches value
+        mock_run_cmd.side_effect = [(0, [""], None)]
+        mock_read_num_dumps.return_value = 5
+        with self.assertRaises(SystemExit) as sys_exit:
+            sonic_kdump_config.write_num_dumps(5)
+        self.assertEqual(sys_exit.exception.code, 1)
+
+        # Edge case: read_num_dumps returns matching value but run_command fails
+        mock_run_cmd.side_effect = [(2, [], None)]
+        mock_read_num_dumps.return_value = 5
+        with self.assertRaises(SystemExit) as sys_exit:
+            sonic_kdump_config.write_num_dumps(5)
+        self.assertEqual(sys_exit.exception.code, 1)
+
     @patch("sonic_kdump_config.get_bootloader")
     def test_get_image(self, mock_get_bootloader):
         """Tests the function `get_current_image() and get_next_image()` in script `sonic-kdump-config.py`.