taking it a few commits back to resolve errors

Author: kanza-latif

scripts/hostcfgd

@@ -1694,44 +1694,83 @@ 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:
+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):
-        self.enabled = False
-        self.retention_time = 0
-        self.sampling_interval = 0
+        self.cache = {}
+        self.memory_statistics_defaults = {
+            "enabled": "false",
+            "retention_time": "15 days",
+            "sampling_interval": "5 minutes"
+        }
 
-    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_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
+    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)
+
+    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"])
         else:
-            raise ValueError(f"Unknown configuration key: {key}")
+            self.run_cmd(["sonic-memory_statistics-config", "--disable"])
 
-        self.apply_config()
+        # 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])
 
-    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}")
+    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)
 
+    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'))
 class HostConfigDaemon:
     def __init__(self):
         self.state_db_conn = DBConnector(STATE_DB, 0)

tests/hostcfgd/hostcfgd_test.py

@@ -295,21 +295,11 @@ def test_dns_events(self):
             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')}
@@ -322,15 +312,14 @@ def test_memory_statistics_event(self):
             except TimeoutError:
                 pass
 
-            expected_calls = [
+            expected = [
                 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'])
+                mock.call(['sonic-memory_statistics-config', '--retention_time', '15']),
+                mock.call(['sonic-memory_statistics-config', '--sampling_interval', '5'])
             ]
-
-            mocked_subprocess.check_call.assert_has_calls(expected_calls, any_order=True)
-
-
+            
+            mocked_subprocess.check_call.assert_has_calls(expected, any_order=True)
+            
 class TestDnsHandler:
 
     @mock.patch('hostcfgd.run_cmd')