errors

Author: kanza-latif

scripts/hostcfgd

@@ -1694,88 +1694,43 @@ class FipsCfg(object):
         syslog.syslog(syslog.LOG_INFO, f'FipsCfg: update the FIPS enforce option {self.enforce}.')
         loader.set_fips(image, self.enforce)
 
-class Memory_StatisticsCfg(object):
-    """
-    Memory Stats Config Daemon
-    Handles changes in MEMORY_STATS table.
-    1) Handle enabling or disabling the feature
-    2) Handle change of retention period
-    3) Handle change of sampling interval
-    """
-
-    def __init__(self, config_db):
-        self.config_db = config_db
-        self.cache = {}
-        self.memory_statistics_defaults = {
-            "enabled": "false",
-            "retention_time": "15 days",
-            "sampling_interval": "5 minutes"
-        }
-
-    def load(self, memory_stats_config: dict):
-        """
-        Load memory statistics configuration when the daemon starts.
-        Args:
-            memory_stats_config: Configured memory statistics settings.
-        """
-        syslog.syslog(syslog.LOG_INFO, "Memory_StatisticsCfg init ...")
-        memory_statistics_conf = memory_stats_config.get("config", {})
-
-        # Apply default configurations if not present
-        for row, value in self.memory_statistics_defaults.items():
-            if not memory_statistics_conf.get(row):
-                self.config_db.mod_entry("MEMORY_STATISTICS", "config", {row: value})
-
-        # Apply configurations to ensure they are set correctly on startup
-        self.apply_configuration(memory_statistics_conf)
+class Memory_StatisticsCfg:
+    def __init__(self):
+        self.enabled = False
+        self.retention_time = 0
+        self.sampling_interval = 0
 
-    def apply_configuration(self, config):
-        """
-        Apply the memory statistics configuration settings.
-        Args:
-            config: Configuration data for memory statistics.
-        """
-        # Determine if the feature is enabled or disabled
-        enabled = config.get("enabled", self.memory_statistics_defaults["enabled"]).lower() == "true"
-        retention_time = config.get("retention_time", self.memory_statistics_defaults["retention_time"])
-        sampling_interval = config.get("sampling_interval", self.memory_statistics_defaults["sampling_interval"])
-
-        # Enable or disable memory statistics
-        if enabled:
-            self.run_cmd(["sonic-memory_statistics-config", "--enable"])
+    def load_config(self, config_data):
+        try:
+            config = json.loads(config_data)
+            self.enabled = config.get("enabled", self.enabled)
+            self.retention_time = config.get("retention_time", self.retention_time)
+            self.sampling_interval = config.get("sampling_interval", self.sampling_interval)
+        except json.JSONDecodeError:
+            raise ValueError("Invalid JSON format for configuration data")
+
+    def apply_config(self):
+        if self.enabled:
+            self._run_shell_command(f"set_memory_statistics --enable --retention-time {self.retention_time} --sampling-interval {self.sampling_interval}")
         else:
-            self.run_cmd(["sonic-memory_statistics-config", "--disable"])
-
-        # Set retention time and sampling interval
-        self.run_cmd(["sonic-memory_statistics-config", "--retention-time", retention_time])
-        self.run_cmd(["sonic-memory_statistics-config", "--sampling-interval", sampling_interval])
+            self._run_shell_command("set_memory_statistics --disable")
+
+    def update_config(self, key, value):
+        if key == "enabled":
+            self.enabled = value
+        elif key == "retention_time":
+            self.retention_time = value
+        elif key == "sampling_interval":
+            self.sampling_interval = value
+        else:
+            raise ValueError(f"Unknown configuration key: {key}")
 
-    def memory_statistics_update(self, key, data):
-        """
-        Handle updates to the memory statistics configuration.
-        Args:
-            key: Key identifying the config type.
-            data: Updated configuration data.
-        """
-        syslog.syslog(syslog.LOG_INFO, "Memory_Statistics global configuration update")
-        if key == "config":
-            self.apply_configuration(data)
+        self.apply_config()
 
-    def run_cmd(self, cmd):
-        """
-        Execute a shell command and return the output.
-        Args:
-            cmd: List of command arguments.
-        """
-        try:
-            output = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
-            syslog.syslog(syslog.LOG_INFO, output.decode('utf-8'))
-        except subprocess.CalledProcessError as e:
-            syslog.syslog(syslog.LOG_ERR, e.output.decode('utf-8'))
-        except FileNotFoundError:
-            syslog.syslog(syslog.LOG_ERR, f"Command not found: {' '.join(cmd)}")
-        except Exception as e:
-            syslog.syslog(syslog.LOG_ERR, f"An unexpected error occurred: {str(e)}")
+    def _run_shell_command(self, command):
+        result = subprocess.run(command, shell=True, text=True, capture_output=True)
+        if result.returncode != 0:
+            raise RuntimeError(f"Shell command failed with error: {result.stderr}")
 
 class HostConfigDaemon:
     def __init__(self):

tests/hostcfgd/hostcfgd_test.py

@@ -294,41 +294,63 @@ def test_dns_events(self):
                 pass
             mocked_run_cmd.assert_has_calls([call(['systemctl', 'restart', 'resolv-config'], True, False)])
 
-    def test_memory_statistics_event(self):
-        HOSTCFG_DAEMON_CFG_DB = {
-            'MEMORY_STATISTICS': {
-                'config': {
-                    'enabled': 'true',
-                    'retention_time': '15 days',
-                    'sampling_interval': '5 minutes'
-                }
-            }
-        }
-
-        MockConfigDb.set_config_db(HOSTCFG_DAEMON_CFG_DB)
-        daemon = hostcfgd.HostConfigDaemon()
-        daemon.register_callbacks()
-        MockConfigDb.event_queue = [('MEMORY_STATISTICS', 'config')]
-
-        with mock.patch('hostcfgd.subprocess') as mocked_subprocess:
-            popen_mock = mock.Mock()
-            attrs = {'communicate.return_value': ('output', 'error')}
-            popen_mock.configure_mock(**attrs)
-            mocked_subprocess.Popen.return_value = popen_mock
-            mocked_subprocess.check_call = mock.Mock()
-
-            try:
-                daemon.start()
-            except TimeoutError:
-                pass
-
-            expected_calls = [
-                mock.call(['sonic-memory_statistics-config', '--enable']),
-                mock.call(['sonic-memory_statistics-config', '--retention_time', '15 days']),
-                mock.call(['sonic-memory_statistics-config', '--sampling_interval', '5 minutes'])
-            ]
-
-            mocked_subprocess.check_call.assert_has_calls(expected_calls, any_order=True)
+   class TestMemoryStatisticsCfg(unittest.TestCase):
+    def setUp(self):
+        self.cfg = Memory_StatisticsCfg()
+
+    @patch('hostcfgd.subprocess.run')
+    def test_load_config(self, mock_run):
+        config_data = json.dumps({
+            "enabled": True,
+            "retention_time": 3600,
+            "sampling_interval": 60
+        })
+        self.cfg.load_config(config_data)
+        self.assertTrue(self.cfg.enabled)
+        self.assertEqual(self.cfg.retention_time, 3600)
+        self.assertEqual(self.cfg.sampling_interval, 60)
+
+    @patch('hostcfgd.subprocess.run')
+    def test_apply_config_enable(self, mock_run):
+        self.cfg.enabled = True
+        self.cfg.retention_time = 3600
+        self.cfg.sampling_interval = 60
+        self.cfg.apply_config()
+        mock_run.assert_called_once_with(
+            'set_memory_statistics --enable --retention-time 3600 --sampling-interval 60',
+            shell=True, text=True, capture_output=True
+        )
+
+    @patch('hostcfgd.subprocess.run')
+    def test_apply_config_disable(self, mock_run):
+        self.cfg.enabled = False
+        self.cfg.apply_config()
+        mock_run.assert_called_once_with(
+            'set_memory_statistics --disable',
+            shell=True, text=True, capture_output=True
+        )
+
+    @patch('hostcfgd.subprocess.run')
+    def test_update_config(self, mock_run):
+        self.cfg.update_config('retention_time', 7200)
+        self.assertEqual(self.cfg.retention_time, 7200)
+        mock_run.assert_called_once_with(
+            'set_memory_statistics --enable --retention-time 7200 --sampling-interval 0',
+            shell=True, text=True, capture_output=True
+        )
+
+    @patch('hostcfgd.subprocess.run')
+    def test_update_config_invalid_key(self, mock_run):
+        with self.assertRaises(ValueError) as context:
+            self.cfg.update_config('invalid_key', 1234)
+        self.assertEqual(str(context.exception), "Unknown configuration key: invalid_key")
+
+    @patch('hostcfgd.subprocess.run')
+    def test_load_config_invalid_json(self, mock_run):
+        invalid_config_data = "{invalid_json}"
+        with self.assertRaises(ValueError) as context:
+            self.cfg.load_config(invalid_config_data)
+        self.assertEqual(str(context.exception), "Invalid JSON format for configuration data")
 
 
 class TestDnsHandler: