From ac1686450ea18fb759ad6abad76369e006e9fa57 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 14 Oct 2024 14:30:29 +0500 Subject: [PATCH 01/30] added show and config commands --- config/memory_statistics.py | 106 ++++++++++++++++++++++++++++++++++++ show/memory_statistics.py | 100 ++++++++++++++++++++++++++++++++++ 2 files changed, 206 insertions(+) create mode 100644 config/memory_statistics.py create mode 100644 show/memory_statistics.py diff --git a/config/memory_statistics.py b/config/memory_statistics.py new file mode 100644 index 0000000000..cb513d25bc --- /dev/null +++ b/config/memory_statistics.py @@ -0,0 +1,106 @@ +import click +import sys +from swsscommon.swsscommon import ConfigDBConnector + + +# Simulate the AbbreviationGroup from utilities_common.cli +class AbbreviationGroup(click.Group): + def get_command(self, ctx, cmd_name): + # Fallback to default command if abbreviation not found + return super().get_command(ctx, cmd_name) + + +# +# 'memory-statistics' group ('sudo config memory-statistics ...') +# +@click.group(cls=AbbreviationGroup, name="memory-statistics") +def memory_statistics(): + """Configure the Memory Statistics feature""" + pass + + +def check_memory_statistics_table_existence(memory_statistics_table): + """Checks whether the 'MEMORY_STATISTICS' table is configured in Config DB.""" + if not memory_statistics_table: + click.echo("Unable to retrieve 'MEMORY_STATISTICS' table from Config DB.") + sys.exit(1) + + if "memory_statistics" not in memory_statistics_table: + click.echo("Unable to retrieve key 'memory_statistics' from MEMORY_STATISTICS table.") + sys.exit(2) + + +# +# 'enable' command ('sudo config memory-statistics enable') +# +@memory_statistics.command(name="enable", short_help="Enable the Memory Statistics feature") +def memory_statistics_enable(): + """Enable the Memory Statistics feature""" + db = ConfigDBConnector() + db.connect() + + memory_statistics_table = db.get_table("MEMORY_STATISTICS") + check_memory_statistics_table_existence(memory_statistics_table) + + # Set enabled to true and disabled to false + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true", "disabled": "false"}) + click.echo("Memory Statistics feature enabled.") + click.echo("Save SONiC configuration using 'config save' to persist the changes.") + + +# +# 'disable' command ('sudo config memory-statistics disable') +# +@memory_statistics.command(name="disable", short_help="Disable the Memory Statistics feature") +def memory_statistics_disable(): + """Disable the Memory Statistics feature""" + db = ConfigDBConnector() + db.connect() + + memory_statistics_table = db.get_table("MEMORY_STATISTICS") + check_memory_statistics_table_existence(memory_statistics_table) + + # Set enabled to false and disabled to true + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false", "disabled": "true"}) + click.echo("Memory Statistics feature disabled.") + click.echo("Save SONiC configuration using 'config save' to persist the changes.") + + +# +# 'retention-period' command ('sudo config memory-statistics retention-period ...') +# +@memory_statistics.command(name="retention-period", short_help="Configure the retention period for Memory Statistics") +@click.argument('retention_period', metavar='', required=True, type=int) +def memory_statistics_retention_period(retention_period): + """Set the retention period for Memory Statistics""" + db = ConfigDBConnector() + db.connect() + + memory_statistics_table = db.get_table("MEMORY_STATISTICS") + check_memory_statistics_table_existence(memory_statistics_table) + + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"retention_time": retention_period}) + click.echo(f"Memory Statistics retention period set to {retention_period} days.") + click.echo("Save SONiC configuration using 'config save' to persist the changes.") + + +# +# 'sampling-interval' command ('sudo config memory-statistics sampling-interval ...') +# +@memory_statistics.command(name="sampling-interval", short_help="Configure the sampling interval for Memory Statistics") +@click.argument('sampling_interval', metavar='', required=True, type=int) +def memory_statistics_sampling_interval(sampling_interval): + """Set the sampling interval for Memory Statistics""" + db = ConfigDBConnector() + db.connect() + + memory_statistics_table = db.get_table("MEMORY_STATISTICS") + check_memory_statistics_table_existence(memory_statistics_table) + + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval}) + click.echo(f"Memory Statistics sampling interval set to {sampling_interval} minutes.") + click.echo("Save SONiC configuration using 'config save' to persist the changes.") + + +if __name__ == "__main__": + memory_statistics() \ No newline at end of file diff --git a/show/memory_statistics.py b/show/memory_statistics.py new file mode 100644 index 0000000000..eec022a372 --- /dev/null +++ b/show/memory_statistics.py @@ -0,0 +1,100 @@ +import click +from tabulate import tabulate + +import utilities_common.cli as clicommon +from swsscommon.swsscommon import ConfigDBConnector + + +# +# 'memory-statistics' group (show memory-statistics ...) +# +@click.group(cls=clicommon.AliasedGroup, name="memory-statistics") +def memory_statistics(): + """Show memory statistics configuration and logs""" + pass + + +def get_memory_statistics_config(field_name): + """Fetches the configuration of memory_statistics from `CONFIG_DB`. + + Args: + field_name: A string containing the field name in the sub-table of 'memory_statistics'. + + Returns: + field_value: If field name was found, then returns the corresponding value. + Otherwise, returns "Unknown". + """ + field_value = "Unknown" + config_db = ConfigDBConnector() + config_db.connect() + memory_statistics_table = config_db.get_table("MEMORY_STATISTICS") + if (memory_statistics_table and + "memory_statistics" in memory_statistics_table and + field_name in memory_statistics_table["config"]): + field_value = memory_statistics_table["memory_statistics"][field_name] + + return field_value + + +@memory_statistics.command(name="memory_statitics", short_help="Show the configuration of memory statistics") +def config(): + admin_mode = "Disabled" + admin_enabled = get_memory_statistics_config("enabled") + if admin_enabled == "true": + admin_mode = "Enabled" + + click.echo("Memory Statistics administrative mode: {}".format(admin_mode)) + + retention_time = get_memory_statistics_config("retention_time") + click.echo("Memory Statistics retention time (days): {}".format(retention_time)) + + sampling_interval = get_memory_statistics_config("sampling_interval") + click.echo("Memory Statistics sampling interval (minutes): {}".format(sampling_interval)) + + +def fetch_memory_statistics(starting_time=None, ending_time=None, select=None): + """Fetch memory statistics from the database. + + Args: + starting_time: The starting time for filtering the statistics. + ending_time: The ending time for filtering the statistics. + additional_options: Any additional options for filtering or formatting. + + Returns: + A list of memory statistics entries. + """ + config_db = ConfigDBConnector() + config_db.connect() + + memory_statistics_table = config_db.get_table("MEMORY_STATISTICS") + filtered_statistics = [] + + for key, entry in memory_statistics_table.items(): + # Add filtering logic here based on starting_time, ending_time, and select + if (not starting_time or entry.get("time") >= starting_time) and \ + (not ending_time or entry.get("time") <= ending_time): + # Implement additional filtering based on select if needed + filtered_statistics.append(entry) + + return filtered_statistics + + +@memory_statistics.command(name="logs", short_help="Show memory statistics logs with optional filtering") +@click.argument('starting_time', required=False) +@click.argument('ending_time', required=False) +@click.argument('additional_options', required=False, nargs=-1) +def show_memory_statistics_logs(starting_time, ending_time, select): + """Show memory statistics logs with optional filtering by time and select.""" + + # Fetch memory statistics + memory_statistics = fetch_memory_statistics(starting_time, ending_time, select) + + if not memory_statistics: + click.echo("No memory statistics available for the given parameters.") + return + + # Display the memory statistics + headers = ["Time", "Statistic", "Value"] # Adjust according to the actual fields + table_data = [[entry.get("time"), entry.get("statistic"), entry.get("value")] for entry in memory_statistics] + + click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) \ No newline at end of file From f4811d1e8de1e6545aced2fcb5bed29526d25ad9 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 14 Oct 2024 17:07:31 +0500 Subject: [PATCH 02/30] resolved the errors --- config/memory_statistics.py | 2 +- show/memory_statistics.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index cb513d25bc..2fcefb2405 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -103,4 +103,4 @@ def memory_statistics_sampling_interval(sampling_interval): if __name__ == "__main__": - memory_statistics() \ No newline at end of file + memory_statistics() diff --git a/show/memory_statistics.py b/show/memory_statistics.py index eec022a372..c64dd0506c 100644 --- a/show/memory_statistics.py +++ b/show/memory_statistics.py @@ -97,4 +97,5 @@ def show_memory_statistics_logs(starting_time, ending_time, select): headers = ["Time", "Statistic", "Value"] # Adjust according to the actual fields table_data = [[entry.get("time"), entry.get("statistic"), entry.get("value")] for entry in memory_statistics] - click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) \ No newline at end of file + click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) + \ No newline at end of file From b8d72745c19fec7a2ed22c22d46c66f7a2c30c8c Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 14 Oct 2024 17:36:11 +0500 Subject: [PATCH 03/30] resolved the errors --- show/memory_statistics.py | 1 - 1 file changed, 1 deletion(-) diff --git a/show/memory_statistics.py b/show/memory_statistics.py index c64dd0506c..9a5aed69b9 100644 --- a/show/memory_statistics.py +++ b/show/memory_statistics.py @@ -98,4 +98,3 @@ def show_memory_statistics_logs(starting_time, ending_time, select): table_data = [[entry.get("time"), entry.get("statistic"), entry.get("value")] for entry in memory_statistics] click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) - \ No newline at end of file From 636f30ae2cdc5cbb1890bf026d5f1b569a5ef854 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 21 Oct 2024 13:41:56 +0500 Subject: [PATCH 04/30] added test filw --- tests/memory_statistics_test.py | 80 +++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 tests/memory_statistics_test.py diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py new file mode 100644 index 0000000000..c260a2d8d7 --- /dev/null +++ b/tests/memory_statistics_test.py @@ -0,0 +1,80 @@ +import unittest +from unittest.mock import patch +from click.testing import CliRunner +from config.memory_statistics import memory_statistics_enable +from config.memory_statistics import memory_statistics_disable +from config.memory_statistics import memory_statistics_retention_period +from config.memory_statistics import memory_statistics_sampling_interval +from show.memory_statistics import config, show_memory_statistics_logs + + +class TestMemoryStatisticsConfigCommands(unittest.TestCase): + + def setUp(self): + self.runner = CliRunner() + # Mock Config DB connector to simulate the MEMORY_STATISTICS table + self.config_db_mock = patch('config.memory_statistics.ConfigDBConnector') + self.mock_db = self.config_db_mock.start() + # Simulate MEMORY_STATISTICS table presence in Config DB + self.mock_db.get_table.return_value = { + "MEMORY_STATISTICS": {"status": "enabled", "retention_period": "30", "sampling_interval": "5"} + } + + def tearDown(self): + # Stop the Config DB mock + self.config_db_mock.stop() + + def test_memory_statistics_enable(self): + result = self.runner.invoke(memory_statistics_enable) + self.assertIn("Memory Statistics feature enabled.", result.output) + self.assertEqual(result.exit_code, 0) + + def test_memory_statistics_disable(self): + result = self.runner.invoke(memory_statistics_disable) + self.assertIn("Memory Statistics feature disabled.", result.output) + self.assertEqual(result.exit_code, 0) + + def test_memory_statistics_retention_period(self): + result = self.runner.invoke(memory_statistics_retention_period, ['30']) + self.assertIn("Memory Statistics retention period set to 30 days.", result.output) + self.assertEqual(result.exit_code, 0) + + def test_memory_statistics_sampling_interval(self): + result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) + self.assertIn("Memory Statistics sampling interval set to 5 minutes.", result.output) + self.assertEqual(result.exit_code, 0) + + +class TestMemoryStatisticsShowCommands(unittest.TestCase): + + def setUp(self): + self.runner = CliRunner() + # Mock Config DB connector to simulate MEMORY_STATISTICS table and logs + self.config_db_mock = patch('show.memory_statistics.ConfigDBConnector') + self.mock_db = self.config_db_mock.start() + # Simulate logs and configuration + self.mock_db.get_table.return_value = { + "MEMORY_STATISTICS": {"status": "enabled", "retention_period": "30", "sampling_interval": "5"} + } + self.mock_db.get_log_entries.return_value = [ + {"date": "2023-10-01", "data": "Sample log entry 1"}, + {"date": "2023-10-02", "data": "Sample log entry 2"} + ] + + def tearDown(self): + # Stop the Config DB mock + self.config_db_mock.stop() + + def test_memory_statistics_config(self): + result = self.runner.invoke(config) + self.assertIn("Memory Statistics administrative mode:", result.output) + self.assertEqual(result.exit_code, 0) + + def test_memory_statistics_logs(self): + result = self.runner.invoke(show_memory_statistics_logs, ['2023-10-01', '2023-10-02']) + self.assertIn("Memory Statistics logs", result.output) + self.assertEqual(result.exit_code, 0) + + +if __name__ == '__main__': + unittest.main() From 81df5f01fc15201bfd2f89b9603d2e334aa9785c Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Wed, 23 Oct 2024 14:06:39 +0500 Subject: [PATCH 05/30] updated test cases --- tests/memory_statistics_test.py | 62 +++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 27 deletions(-) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index c260a2d8d7..a7f61f5e40 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -1,28 +1,35 @@ import unittest -from unittest.mock import patch from click.testing import CliRunner -from config.memory_statistics import memory_statistics_enable -from config.memory_statistics import memory_statistics_disable -from config.memory_statistics import memory_statistics_retention_period -from config.memory_statistics import memory_statistics_sampling_interval +from config.memory_statistics import ( + memory_statistics_enable, + memory_statistics_disable, + memory_statistics_retention_period, + memory_statistics_sampling_interval +) from show.memory_statistics import config, show_memory_statistics_logs +from swsscommon.swsscommon import ConfigDBConnector class TestMemoryStatisticsConfigCommands(unittest.TestCase): def setUp(self): self.runner = CliRunner() - # Mock Config DB connector to simulate the MEMORY_STATISTICS table - self.config_db_mock = patch('config.memory_statistics.ConfigDBConnector') - self.mock_db = self.config_db_mock.start() - # Simulate MEMORY_STATISTICS table presence in Config DB - self.mock_db.get_table.return_value = { - "MEMORY_STATISTICS": {"status": "enabled", "retention_period": "30", "sampling_interval": "5"} - } + self.db = ConfigDBConnector() + self.db.connect() + # Ensure a clean state in the MEMORY_STATISTICS table before each test + self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { + "enabled": "false", + "retention_time": "30", + "sampling_interval": "5" + }) def tearDown(self): - # Stop the Config DB mock - self.config_db_mock.stop() + # Clean up after each test to avoid side effects + self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { + "enabled": "false", + "retention_time": "30", + "sampling_interval": "5" + }) def test_memory_statistics_enable(self): result = self.runner.invoke(memory_statistics_enable) @@ -49,21 +56,22 @@ class TestMemoryStatisticsShowCommands(unittest.TestCase): def setUp(self): self.runner = CliRunner() - # Mock Config DB connector to simulate MEMORY_STATISTICS table and logs - self.config_db_mock = patch('show.memory_statistics.ConfigDBConnector') - self.mock_db = self.config_db_mock.start() - # Simulate logs and configuration - self.mock_db.get_table.return_value = { - "MEMORY_STATISTICS": {"status": "enabled", "retention_period": "30", "sampling_interval": "5"} - } - self.mock_db.get_log_entries.return_value = [ - {"date": "2023-10-01", "data": "Sample log entry 1"}, - {"date": "2023-10-02", "data": "Sample log entry 2"} - ] + self.db = ConfigDBConnector() + self.db.connect() + # Ensure a clean state in the MEMORY_STATISTICS table before each test + self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { + "enabled": "true", + "retention_time": "30", + "sampling_interval": "5" + }) def tearDown(self): - # Stop the Config DB mock - self.config_db_mock.stop() + # Clean up after each test to avoid side effects + self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { + "enabled": "true", + "retention_time": "30", + "sampling_interval": "5" + }) def test_memory_statistics_config(self): result = self.runner.invoke(config) From 1ad12226598dbf5e5762e04ed3d36d810c1e92af Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Wed, 23 Oct 2024 15:11:00 +0500 Subject: [PATCH 06/30] updated test cases --- tests/memory_statistics_test.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index a7f61f5e40..bf55700874 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -19,15 +19,15 @@ def setUp(self): # Ensure a clean state in the MEMORY_STATISTICS table before each test self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { "enabled": "false", - "retention_time": "30", - "sampling_interval": "5" + "retention_period": "15", # Default retention period + "sampling_interval": "5" # Default sampling interval }) def tearDown(self): # Clean up after each test to avoid side effects self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { "enabled": "false", - "retention_time": "30", + "retention_period": "15", "sampling_interval": "5" }) @@ -42,12 +42,12 @@ def test_memory_statistics_disable(self): self.assertEqual(result.exit_code, 0) def test_memory_statistics_retention_period(self): - result = self.runner.invoke(memory_statistics_retention_period, ['30']) - self.assertIn("Memory Statistics retention period set to 30 days.", result.output) + result = self.runner.invoke(memory_statistics_retention_period, ['15']) # Test with default + self.assertIn("Memory Statistics retention period set to 15 days.", result.output) self.assertEqual(result.exit_code, 0) def test_memory_statistics_sampling_interval(self): - result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) + result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) # Test with default self.assertIn("Memory Statistics sampling interval set to 5 minutes.", result.output) self.assertEqual(result.exit_code, 0) @@ -61,15 +61,15 @@ def setUp(self): # Ensure a clean state in the MEMORY_STATISTICS table before each test self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { "enabled": "true", - "retention_time": "30", - "sampling_interval": "5" + "retention_period": "15", # Default retention period + "sampling_interval": "5" # Default sampling interval }) def tearDown(self): # Clean up after each test to avoid side effects self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { "enabled": "true", - "retention_time": "30", + "retention_period": "15", "sampling_interval": "5" }) From 29afad4169b3d12a48206e015363d15a33ae960f Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 11:38:15 +0500 Subject: [PATCH 07/30] updated the mock db --- tests/memory_statistics_test.py | 54 +++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index bf55700874..eed3944bd2 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -1,4 +1,5 @@ import unittest +from unittest.mock import MagicMock from click.testing import CliRunner from config.memory_statistics import ( memory_statistics_enable, @@ -10,21 +11,41 @@ from swsscommon.swsscommon import ConfigDBConnector +# Mock for ConfigDBConnector +class MockConfigDBConnector: + def __init__(self): + # Simulate the database with an in-memory dictionary + self.db = { + "MEMORY_STATISTICS": { + "memory_statistics": { + "enabled": "false", + "retention_period": "15", # Default retention period + "sampling_interval": "5" # Default sampling interval + } + } + } + + def connect(self): + pass # No action needed for mock + + def mod_entry(self, table, key, data): + # Update the mock database entry + if table in self.db and key in self.db[table]: + self.db[table][key].update(data) + + def get_entry(self, table, key): + # Retrieve the mock database entry + return self.db.get(table, {}).get(key, {}) + + class TestMemoryStatisticsConfigCommands(unittest.TestCase): def setUp(self): self.runner = CliRunner() - self.db = ConfigDBConnector() - self.db.connect() - # Ensure a clean state in the MEMORY_STATISTICS table before each test - self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { - "enabled": "false", - "retention_period": "15", # Default retention period - "sampling_interval": "5" # Default sampling interval - }) + self.db = MockConfigDBConnector() # Use the mock database def tearDown(self): - # Clean up after each test to avoid side effects + # Reset the mock database to default values after each test self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { "enabled": "false", "retention_period": "15", @@ -33,6 +54,8 @@ def tearDown(self): def test_memory_statistics_enable(self): result = self.runner.invoke(memory_statistics_enable) + print(result.output) # Debug: Print the output + print(result.exit_code) self.assertIn("Memory Statistics feature enabled.", result.output) self.assertEqual(result.exit_code, 0) @@ -56,19 +79,18 @@ class TestMemoryStatisticsShowCommands(unittest.TestCase): def setUp(self): self.runner = CliRunner() - self.db = ConfigDBConnector() - self.db.connect() - # Ensure a clean state in the MEMORY_STATISTICS table before each test + self.db = MockConfigDBConnector() # Use the mock database + # Set MEMORY_STATISTICS to enabled for testing show commands self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { "enabled": "true", - "retention_period": "15", # Default retention period - "sampling_interval": "5" # Default sampling interval + "retention_period": "15", + "sampling_interval": "5" }) def tearDown(self): - # Clean up after each test to avoid side effects + # Reset the mock database to default values after each test self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { - "enabled": "true", + "enabled": "false", "retention_period": "15", "sampling_interval": "5" }) From aecbc72d4a3587e9cc3394b1f71479b43f82d989 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 12:28:42 +0500 Subject: [PATCH 08/30] removed pre-commit errors --- tests/memory_statistics_test.py | 104 +++++++++----------------------- 1 file changed, 30 insertions(+), 74 deletions(-) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index eed3944bd2..fd688817d9 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -1,5 +1,5 @@ import unittest -from unittest.mock import MagicMock +from unittest.mock import patch from click.testing import CliRunner from config.memory_statistics import ( memory_statistics_enable, @@ -10,101 +10,57 @@ from show.memory_statistics import config, show_memory_statistics_logs from swsscommon.swsscommon import ConfigDBConnector - -# Mock for ConfigDBConnector -class MockConfigDBConnector: - def __init__(self): - # Simulate the database with an in-memory dictionary - self.db = { - "MEMORY_STATISTICS": { - "memory_statistics": { - "enabled": "false", - "retention_period": "15", # Default retention period - "sampling_interval": "5" # Default sampling interval - } - } - } - - def connect(self): - pass # No action needed for mock - - def mod_entry(self, table, key, data): - # Update the mock database entry - if table in self.db and key in self.db[table]: - self.db[table][key].update(data) - - def get_entry(self, table, key): - # Retrieve the mock database entry - return self.db.get(table, {}).get(key, {}) - - class TestMemoryStatisticsConfigCommands(unittest.TestCase): - def setUp(self): + @patch.object(ConfigDBConnector, 'mod_entry') + @patch.object(ConfigDBConnector, 'get_entry') + def setUp(self, mock_get_entry, mock_mod_entry): self.runner = CliRunner() - self.db = MockConfigDBConnector() # Use the mock database + self.db = ConfigDBConnector() + self.db.connect() - def tearDown(self): - # Reset the mock database to default values after each test - self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { + # Mock the return value of get_entry + mock_get_entry.return_value = { "enabled": "false", "retention_period": "15", "sampling_interval": "5" - }) - - def test_memory_statistics_enable(self): - result = self.runner.invoke(memory_statistics_enable) - print(result.output) # Debug: Print the output - print(result.exit_code) - self.assertIn("Memory Statistics feature enabled.", result.output) - self.assertEqual(result.exit_code, 0) - - def test_memory_statistics_disable(self): - result = self.runner.invoke(memory_statistics_disable) - self.assertIn("Memory Statistics feature disabled.", result.output) - self.assertEqual(result.exit_code, 0) - - def test_memory_statistics_retention_period(self): - result = self.runner.invoke(memory_statistics_retention_period, ['15']) # Test with default - self.assertIn("Memory Statistics retention period set to 15 days.", result.output) - self.assertEqual(result.exit_code, 0) - - def test_memory_statistics_sampling_interval(self): - result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) # Test with default - self.assertIn("Memory Statistics sampling interval set to 5 minutes.", result.output) - self.assertEqual(result.exit_code, 0) - - -class TestMemoryStatisticsShowCommands(unittest.TestCase): + } - def setUp(self): - self.runner = CliRunner() - self.db = MockConfigDBConnector() # Use the mock database - # Set MEMORY_STATISTICS to enabled for testing show commands + # Ensure a clean state in the MEMORY_STATISTICS table before each test self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { - "enabled": "true", + "enabled": "false", "retention_period": "15", "sampling_interval": "5" }) def tearDown(self): - # Reset the mock database to default values after each test + # Clean up after each test to avoid side effects self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { "enabled": "false", "retention_period": "15", "sampling_interval": "5" }) - def test_memory_statistics_config(self): - result = self.runner.invoke(config) - self.assertIn("Memory Statistics administrative mode:", result.output) + @patch.object(ConfigDBConnector, 'get_entry', return_value={"enabled": "true"}) + def test_memory_statistics_enable(self, mock_get_entry): + result = self.runner.invoke(memory_statistics_enable) + self.assertIn("Memory Statistics feature enabled.", result.output) self.assertEqual(result.exit_code, 0) - def test_memory_statistics_logs(self): - result = self.runner.invoke(show_memory_statistics_logs, ['2023-10-01', '2023-10-02']) - self.assertIn("Memory Statistics logs", result.output) + @patch.object(ConfigDBConnector, 'get_entry', return_value={"enabled": "false"}) + def test_memory_statistics_disable(self, mock_get_entry): + result = self.runner.invoke(memory_statistics_disable) + self.assertIn("Memory Statistics feature disabled.", result.output) self.assertEqual(result.exit_code, 0) + @patch.object(ConfigDBConnector, 'get_entry', return_value={"retention_period": "15"}) + def test_memory_statistics_retention_period(self, mock_get_entry): + result = self.runner.invoke(memory_statistics_retention_period, ['15']) + self.assertIn("Memory Statistics retention period set to 15 days.", result.output) + self.assertEqual(result.exit_code, 0) -if __name__ == '__main__': - unittest.main() + @patch.object(ConfigDBConnector, 'get_entry', return_value={"sampling_interval": "5"}) + def test_memory_statistics_sampling_interval(self, mock_get_entry): + result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) + self.assertIn("Memory Statistics sampling interval set to 5 minutes.", result.output) + self.assertEqual(result.exit_code, 0) From 2d066ee685a74c42d4db3a471b83ef7be74a2ec6 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 12:52:39 +0500 Subject: [PATCH 09/30] resolivng pre-commit errors --- tests/memory_statistics_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index fd688817d9..8fd187ecc7 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -7,9 +7,9 @@ memory_statistics_retention_period, memory_statistics_sampling_interval ) -from show.memory_statistics import config, show_memory_statistics_logs from swsscommon.swsscommon import ConfigDBConnector + class TestMemoryStatisticsConfigCommands(unittest.TestCase): @patch.object(ConfigDBConnector, 'mod_entry') From 0cf9043e19af95d5c63878a2056a4e3fa2195b9c Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 13:10:42 +0500 Subject: [PATCH 10/30] resolivng pre-commit errors --- tests/memory_statistics_test.py | 61 +++++++++++++++++---------------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index 8fd187ecc7..768449c613 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -1,5 +1,5 @@ import unittest -from unittest.mock import patch +from unittest.mock import patch, MagicMock from click.testing import CliRunner from config.memory_statistics import ( memory_statistics_enable, @@ -12,55 +12,56 @@ class TestMemoryStatisticsConfigCommands(unittest.TestCase): - @patch.object(ConfigDBConnector, 'mod_entry') - @patch.object(ConfigDBConnector, 'get_entry') - def setUp(self, mock_get_entry, mock_mod_entry): + def setUp(self): self.runner = CliRunner() - self.db = ConfigDBConnector() - self.db.connect() - - # Mock the return value of get_entry - mock_get_entry.return_value = { + self.mock_db = MagicMock() + self.mock_db.get_entry.return_value = { "enabled": "false", "retention_period": "15", "sampling_interval": "5" } - - # Ensure a clean state in the MEMORY_STATISTICS table before each test - self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { - "enabled": "false", - "retention_period": "15", - "sampling_interval": "5" - }) + self.patcher = patch.object(ConfigDBConnector, 'get_entry', self.mock_db.get_entry) + self.patcher.start() def tearDown(self): - # Clean up after each test to avoid side effects - self.db.mod_entry("MEMORY_STATISTICS", "memory_statistics", { - "enabled": "false", - "retention_period": "15", - "sampling_interval": "5" - }) + self.patcher.stop() - @patch.object(ConfigDBConnector, 'get_entry', return_value={"enabled": "true"}) - def test_memory_statistics_enable(self, mock_get_entry): + @patch.object(ConfigDBConnector, 'mod_entry') + def test_memory_statistics_enable(self, mock_mod_entry): + # Change the return value to simulate a disabled state + self.mock_db.get_entry.return_value = {"enabled": "false"} result = self.runner.invoke(memory_statistics_enable) self.assertIn("Memory Statistics feature enabled.", result.output) self.assertEqual(result.exit_code, 0) - @patch.object(ConfigDBConnector, 'get_entry', return_value={"enabled": "false"}) - def test_memory_statistics_disable(self, mock_get_entry): + # Ensure the entry was modified correctly + mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true"}) + + @patch.object(ConfigDBConnector, 'mod_entry') + def test_memory_statistics_disable(self, mock_mod_entry): + # Change the return value to simulate an enabled state + self.mock_db.get_entry.return_value = {"enabled": "true"} result = self.runner.invoke(memory_statistics_disable) self.assertIn("Memory Statistics feature disabled.", result.output) self.assertEqual(result.exit_code, 0) - @patch.object(ConfigDBConnector, 'get_entry', return_value={"retention_period": "15"}) - def test_memory_statistics_retention_period(self, mock_get_entry): + # Ensure the entry was modified correctly + mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false"}) + + @patch.object(ConfigDBConnector, 'mod_entry') + def test_memory_statistics_retention_period(self, mock_mod_entry): result = self.runner.invoke(memory_statistics_retention_period, ['15']) self.assertIn("Memory Statistics retention period set to 15 days.", result.output) self.assertEqual(result.exit_code, 0) - @patch.object(ConfigDBConnector, 'get_entry', return_value={"sampling_interval": "5"}) - def test_memory_statistics_sampling_interval(self, mock_get_entry): + # Ensure the entry was modified correctly + mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"retention_period": "15"}) + + @patch.object(ConfigDBConnector, 'mod_entry') + def test_memory_statistics_sampling_interval(self, mock_mod_entry): result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) self.assertIn("Memory Statistics sampling interval set to 5 minutes.", result.output) self.assertEqual(result.exit_code, 0) + + # Ensure the entry was modified correctly + mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": "5"}) From 52d7c0a18bd9b08339be811664bcb9942b906df5 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 13:56:52 +0500 Subject: [PATCH 11/30] added mock db in config file --- config/memory_statistics.py | 39 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 2fcefb2405..0b97604192 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -1,18 +1,12 @@ import click -import sys from swsscommon.swsscommon import ConfigDBConnector -# Simulate the AbbreviationGroup from utilities_common.cli class AbbreviationGroup(click.Group): def get_command(self, ctx, cmd_name): - # Fallback to default command if abbreviation not found return super().get_command(ctx, cmd_name) -# -# 'memory-statistics' group ('sudo config memory-statistics ...') -# @click.group(cls=AbbreviationGroup, name="memory-statistics") def memory_statistics(): """Configure the Memory Statistics feature""" @@ -22,53 +16,45 @@ def memory_statistics(): def check_memory_statistics_table_existence(memory_statistics_table): """Checks whether the 'MEMORY_STATISTICS' table is configured in Config DB.""" if not memory_statistics_table: - click.echo("Unable to retrieve 'MEMORY_STATISTICS' table from Config DB.") - sys.exit(1) + raise RuntimeError("Unable to retrieve 'MEMORY_STATISTICS' table from Config DB.") if "memory_statistics" not in memory_statistics_table: - click.echo("Unable to retrieve key 'memory_statistics' from MEMORY_STATISTICS table.") - sys.exit(2) + raise RuntimeError("Unable to retrieve key 'memory_statistics' from MEMORY_STATISTICS table.") + + +def get_memory_statistics_table(db): + """Get the MEMORY_STATISTICS table from the database.""" + return db.get_table("MEMORY_STATISTICS") -# -# 'enable' command ('sudo config memory-statistics enable') -# @memory_statistics.command(name="enable", short_help="Enable the Memory Statistics feature") def memory_statistics_enable(): """Enable the Memory Statistics feature""" db = ConfigDBConnector() db.connect() - memory_statistics_table = db.get_table("MEMORY_STATISTICS") + memory_statistics_table = get_memory_statistics_table(db) check_memory_statistics_table_existence(memory_statistics_table) - # Set enabled to true and disabled to false db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true", "disabled": "false"}) click.echo("Memory Statistics feature enabled.") click.echo("Save SONiC configuration using 'config save' to persist the changes.") -# -# 'disable' command ('sudo config memory-statistics disable') -# @memory_statistics.command(name="disable", short_help="Disable the Memory Statistics feature") def memory_statistics_disable(): """Disable the Memory Statistics feature""" db = ConfigDBConnector() db.connect() - memory_statistics_table = db.get_table("MEMORY_STATISTICS") + memory_statistics_table = get_memory_statistics_table(db) check_memory_statistics_table_existence(memory_statistics_table) - # Set enabled to false and disabled to true db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false", "disabled": "true"}) click.echo("Memory Statistics feature disabled.") click.echo("Save SONiC configuration using 'config save' to persist the changes.") -# -# 'retention-period' command ('sudo config memory-statistics retention-period ...') -# @memory_statistics.command(name="retention-period", short_help="Configure the retention period for Memory Statistics") @click.argument('retention_period', metavar='', required=True, type=int) def memory_statistics_retention_period(retention_period): @@ -76,7 +62,7 @@ def memory_statistics_retention_period(retention_period): db = ConfigDBConnector() db.connect() - memory_statistics_table = db.get_table("MEMORY_STATISTICS") + memory_statistics_table = get_memory_statistics_table(db) check_memory_statistics_table_existence(memory_statistics_table) db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"retention_time": retention_period}) @@ -84,9 +70,6 @@ def memory_statistics_retention_period(retention_period): click.echo("Save SONiC configuration using 'config save' to persist the changes.") -# -# 'sampling-interval' command ('sudo config memory-statistics sampling-interval ...') -# @memory_statistics.command(name="sampling-interval", short_help="Configure the sampling interval for Memory Statistics") @click.argument('sampling_interval', metavar='', required=True, type=int) def memory_statistics_sampling_interval(sampling_interval): @@ -94,7 +77,7 @@ def memory_statistics_sampling_interval(sampling_interval): db = ConfigDBConnector() db.connect() - memory_statistics_table = db.get_table("MEMORY_STATISTICS") + memory_statistics_table = get_memory_statistics_table(db) check_memory_statistics_table_existence(memory_statistics_table) db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval}) From 43f1f9cdb0082a15bc4d3e44c8cf21c2b6b568c9 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 14:43:14 +0500 Subject: [PATCH 12/30] removed incomaptibilities from the config and test files --- config/memory_statistics.py | 52 ++++++++++++++++++++++++--------- tests/memory_statistics_test.py | 43 ++++++++++++++++----------- 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 0b97604192..5eca0186e4 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -16,10 +16,14 @@ def memory_statistics(): def check_memory_statistics_table_existence(memory_statistics_table): """Checks whether the 'MEMORY_STATISTICS' table is configured in Config DB.""" if not memory_statistics_table: - raise RuntimeError("Unable to retrieve 'MEMORY_STATISTICS' table from Config DB.") + click.echo("Unable to retrieve 'MEMORY_STATISTICS' table from Config DB.", err=True) + return False if "memory_statistics" not in memory_statistics_table: - raise RuntimeError("Unable to retrieve key 'memory_statistics' from MEMORY_STATISTICS table.") + click.echo("Unable to retrieve key 'memory_statistics' from MEMORY_STATISTICS table.", err=True) + return False + + return True def get_memory_statistics_table(db): @@ -34,10 +38,15 @@ def memory_statistics_enable(): db.connect() memory_statistics_table = get_memory_statistics_table(db) - check_memory_statistics_table_existence(memory_statistics_table) + if not check_memory_statistics_table_existence(memory_statistics_table): + return # Exit gracefully on error + + try: + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true", "disabled": "false"}) + click.echo("Memory Statistics feature enabled.") + except Exception as e: + click.echo(f"Error enabling Memory Statistics feature: {str(e)}", err=True) - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true", "disabled": "false"}) - click.echo("Memory Statistics feature enabled.") click.echo("Save SONiC configuration using 'config save' to persist the changes.") @@ -48,10 +57,15 @@ def memory_statistics_disable(): db.connect() memory_statistics_table = get_memory_statistics_table(db) - check_memory_statistics_table_existence(memory_statistics_table) + if not check_memory_statistics_table_existence(memory_statistics_table): + return # Exit gracefully on error + + try: + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false", "disabled": "true"}) + click.echo("Memory Statistics feature disabled.") + except Exception as e: + click.echo(f"Error disabling Memory Statistics feature: {str(e)}", err=True) - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false", "disabled": "true"}) - click.echo("Memory Statistics feature disabled.") click.echo("Save SONiC configuration using 'config save' to persist the changes.") @@ -63,10 +77,15 @@ def memory_statistics_retention_period(retention_period): db.connect() memory_statistics_table = get_memory_statistics_table(db) - check_memory_statistics_table_existence(memory_statistics_table) + if not check_memory_statistics_table_existence(memory_statistics_table): + return # Exit gracefully on error + + try: + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"retention_time": retention_period}) + click.echo(f"Memory Statistics retention period set to {retention_period} days.") + except Exception as e: + click.echo(f"Error setting retention period: {str(e)}", err=True) - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"retention_time": retention_period}) - click.echo(f"Memory Statistics retention period set to {retention_period} days.") click.echo("Save SONiC configuration using 'config save' to persist the changes.") @@ -78,10 +97,15 @@ def memory_statistics_sampling_interval(sampling_interval): db.connect() memory_statistics_table = get_memory_statistics_table(db) - check_memory_statistics_table_existence(memory_statistics_table) + if not check_memory_statistics_table_existence(memory_statistics_table): + return # Exit gracefully on error + + try: + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval}) + click.echo(f"Memory Statistics sampling interval set to {sampling_interval} minutes.") + except Exception as e: + click.echo(f"Error setting sampling interval: {str(e)}", err=True) - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval}) - click.echo(f"Memory Statistics sampling interval set to {sampling_interval} minutes.") click.echo("Save SONiC configuration using 'config save' to persist the changes.") diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index 768449c613..aaac73a5d2 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -15,12 +15,15 @@ class TestMemoryStatisticsConfigCommands(unittest.TestCase): def setUp(self): self.runner = CliRunner() self.mock_db = MagicMock() - self.mock_db.get_entry.return_value = { - "enabled": "false", - "retention_period": "15", - "sampling_interval": "5" + # Simulate the MEMORY_STATISTICS table in Config DB + self.mock_db.get_table.return_value = { + "memory_statistics": { + "enabled": "false", + "retention_period": "15", + "sampling_interval": "5" + } } - self.patcher = patch.object(ConfigDBConnector, 'get_entry', self.mock_db.get_entry) + self.patcher = patch.object(ConfigDBConnector, 'get_table', self.mock_db.get_table) self.patcher.start() def tearDown(self): @@ -28,40 +31,46 @@ def tearDown(self): @patch.object(ConfigDBConnector, 'mod_entry') def test_memory_statistics_enable(self, mock_mod_entry): - # Change the return value to simulate a disabled state - self.mock_db.get_entry.return_value = {"enabled": "false"} + # Simulate initial state as disabled + self.mock_db.get_table.return_value["memory_statistics"]["enabled"] = "false" result = self.runner.invoke(memory_statistics_enable) self.assertIn("Memory Statistics feature enabled.", result.output) self.assertEqual(result.exit_code, 0) # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true"}) + mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true", "disabled": "false"}) @patch.object(ConfigDBConnector, 'mod_entry') def test_memory_statistics_disable(self, mock_mod_entry): - # Change the return value to simulate an enabled state - self.mock_db.get_entry.return_value = {"enabled": "true"} + # Simulate initial state as enabled + self.mock_db.get_table.return_value["memory_statistics"]["enabled"] = "true" result = self.runner.invoke(memory_statistics_disable) self.assertIn("Memory Statistics feature disabled.", result.output) self.assertEqual(result.exit_code, 0) # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false"}) + mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false", "disabled": "true"}) @patch.object(ConfigDBConnector, 'mod_entry') def test_memory_statistics_retention_period(self, mock_mod_entry): - result = self.runner.invoke(memory_statistics_retention_period, ['15']) - self.assertIn("Memory Statistics retention period set to 15 days.", result.output) + retention_period = 15 + result = self.runner.invoke(memory_statistics_retention_period, [str(retention_period)]) + self.assertIn(f"Memory Statistics retention period set to {retention_period} days.", result.output) self.assertEqual(result.exit_code, 0) # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"retention_period": "15"}) + mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"retention_period": retention_period}) @patch.object(ConfigDBConnector, 'mod_entry') def test_memory_statistics_sampling_interval(self, mock_mod_entry): - result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) - self.assertIn("Memory Statistics sampling interval set to 5 minutes.", result.output) + sampling_interval = 5 + result = self.runner.invoke(memory_statistics_sampling_interval, [str(sampling_interval)]) + self.assertIn(f"Memory Statistics sampling interval set to {sampling_interval} minutes.", result.output) self.assertEqual(result.exit_code, 0) # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": "5"}) + mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval}) + + +if __name__ == "__main__": + unittest.main() From b4716c3c0a1e8171443c9f27221913bdf3aae3bb Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 14:47:42 +0500 Subject: [PATCH 13/30] removed incomaptibilities from the config and test files --- tests/memory_statistics_test.py | 55 ++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index aaac73a5d2..835851e50a 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -15,15 +15,12 @@ class TestMemoryStatisticsConfigCommands(unittest.TestCase): def setUp(self): self.runner = CliRunner() self.mock_db = MagicMock() - # Simulate the MEMORY_STATISTICS table in Config DB - self.mock_db.get_table.return_value = { - "memory_statistics": { - "enabled": "false", - "retention_period": "15", - "sampling_interval": "5" - } + self.mock_db.get_entry.return_value = { + "enabled": "false", + "retention_period": "15", + "sampling_interval": "5" } - self.patcher = patch.object(ConfigDBConnector, 'get_table', self.mock_db.get_table) + self.patcher = patch.object(ConfigDBConnector, 'get_entry', self.mock_db.get_entry) self.patcher.start() def tearDown(self): @@ -31,45 +28,59 @@ def tearDown(self): @patch.object(ConfigDBConnector, 'mod_entry') def test_memory_statistics_enable(self, mock_mod_entry): - # Simulate initial state as disabled - self.mock_db.get_table.return_value["memory_statistics"]["enabled"] = "false" + # Change the return value to simulate a disabled state + self.mock_db.get_entry.return_value = {"enabled": "false"} result = self.runner.invoke(memory_statistics_enable) self.assertIn("Memory Statistics feature enabled.", result.output) self.assertEqual(result.exit_code, 0) # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true", "disabled": "false"}) + mock_mod_entry.assert_called_once_with( + "MEMORY_STATISTICS", + "memory_statistics", + {"enabled": "true", "disabled": "false"} + ) @patch.object(ConfigDBConnector, 'mod_entry') def test_memory_statistics_disable(self, mock_mod_entry): - # Simulate initial state as enabled - self.mock_db.get_table.return_value["memory_statistics"]["enabled"] = "true" + # Change the return value to simulate an enabled state + self.mock_db.get_entry.return_value = {"enabled": "true"} result = self.runner.invoke(memory_statistics_disable) self.assertIn("Memory Statistics feature disabled.", result.output) self.assertEqual(result.exit_code, 0) # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false", "disabled": "true"}) + mock_mod_entry.assert_called_once_with( + "MEMORY_STATISTICS", + "memory_statistics", + {"enabled": "false", "disabled": "true"} + ) @patch.object(ConfigDBConnector, 'mod_entry') def test_memory_statistics_retention_period(self, mock_mod_entry): - retention_period = 15 - result = self.runner.invoke(memory_statistics_retention_period, [str(retention_period)]) - self.assertIn(f"Memory Statistics retention period set to {retention_period} days.", result.output) + result = self.runner.invoke(memory_statistics_retention_period, ['15']) + self.assertIn("Memory Statistics retention period set to 15 days.", result.output) self.assertEqual(result.exit_code, 0) # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"retention_period": retention_period}) + mock_mod_entry.assert_called_once_with( + "MEMORY_STATISTICS", + "memory_statistics", + {"retention_period": 15} + ) @patch.object(ConfigDBConnector, 'mod_entry') def test_memory_statistics_sampling_interval(self, mock_mod_entry): - sampling_interval = 5 - result = self.runner.invoke(memory_statistics_sampling_interval, [str(sampling_interval)]) - self.assertIn(f"Memory Statistics sampling interval set to {sampling_interval} minutes.", result.output) + result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) + self.assertIn("Memory Statistics sampling interval set to 5 minutes.", result.output) self.assertEqual(result.exit_code, 0) # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval}) + mock_mod_entry.assert_called_once_with( + "MEMORY_STATISTICS", + "memory_statistics", + {"sampling_interval": 5} + ) if __name__ == "__main__": From b17f2266916346051c2593ad197f2b240e5d0395 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 15:16:31 +0500 Subject: [PATCH 14/30] updated the test file --- tests/memory_statistics_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index 835851e50a..12a6dec2c5 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -22,6 +22,10 @@ def setUp(self): } self.patcher = patch.object(ConfigDBConnector, 'get_entry', self.mock_db.get_entry) self.patcher.start() + + # Mock the get_memory_statistics_table to return a valid table + self.mock_db.get_table = MagicMock(return_value={"memory_statistics": {}}) + patch.object(ConfigDBConnector, 'get_table', self.mock_db.get_table).start() def tearDown(self): self.patcher.stop() From a6680d91a9c7b4e7a877ba904a60c7ae088a13f5 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 15:17:10 +0500 Subject: [PATCH 15/30] updated the test file --- tests/memory_statistics_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py index 12a6dec2c5..d31c07e2ce 100644 --- a/tests/memory_statistics_test.py +++ b/tests/memory_statistics_test.py @@ -22,7 +22,7 @@ def setUp(self): } self.patcher = patch.object(ConfigDBConnector, 'get_entry', self.mock_db.get_entry) self.patcher.start() - + # Mock the get_memory_statistics_table to return a valid table self.mock_db.get_table = MagicMock(return_value={"memory_statistics": {}}) patch.object(ConfigDBConnector, 'get_table', self.mock_db.get_table).start() From 732612f173afa8323d174120e19d2697256a66a5 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 28 Oct 2024 15:59:07 +0500 Subject: [PATCH 16/30] corrected the failing test case --- config/memory_statistics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 5eca0186e4..d46c2daedd 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -81,7 +81,7 @@ def memory_statistics_retention_period(retention_period): return # Exit gracefully on error try: - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"retention_time": retention_period}) + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"retention_period": retention_period}) click.echo(f"Memory Statistics retention period set to {retention_period} days.") except Exception as e: click.echo(f"Error setting retention period: {str(e)}", err=True) From 7b17295f40a139237ed2edf722acbae975dbac83 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Tue, 5 Nov 2024 11:35:48 +0500 Subject: [PATCH 17/30] added testfile for show commands --- config/memory_statistics.py | 3 +- show/memory_statistics.py | 69 +++++---- tests/config_memory_statistics_test.py | 188 +++++++++++++++++++++++++ tests/memory_statistics_test.py | 91 ------------ tests/show_memory_statistics_test.py | 70 +++++++++ 5 files changed, 300 insertions(+), 121 deletions(-) create mode 100644 tests/config_memory_statistics_test.py delete mode 100644 tests/memory_statistics_test.py create mode 100644 tests/show_memory_statistics_test.py diff --git a/config/memory_statistics.py b/config/memory_statistics.py index d46c2daedd..4228f45cfb 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -1,5 +1,6 @@ import click from swsscommon.swsscommon import ConfigDBConnector +# from utilities_common.cli import AbbreviationGroup class AbbreviationGroup(click.Group): @@ -110,4 +111,4 @@ def memory_statistics_sampling_interval(sampling_interval): if __name__ == "__main__": - memory_statistics() + memory_statistics() \ No newline at end of file diff --git a/show/memory_statistics.py b/show/memory_statistics.py index 9a5aed69b9..568148ba7e 100644 --- a/show/memory_statistics.py +++ b/show/memory_statistics.py @@ -2,7 +2,6 @@ from tabulate import tabulate import utilities_common.cli as clicommon -from swsscommon.swsscommon import ConfigDBConnector # @@ -14,67 +13,68 @@ def memory_statistics(): pass -def get_memory_statistics_config(field_name): +def get_memory_statistics_config(field_name, db_connector): """Fetches the configuration of memory_statistics from `CONFIG_DB`. Args: field_name: A string containing the field name in the sub-table of 'memory_statistics'. + db_connector: The database connector. Returns: field_value: If field name was found, then returns the corresponding value. Otherwise, returns "Unknown". """ field_value = "Unknown" - config_db = ConfigDBConnector() - config_db.connect() - memory_statistics_table = config_db.get_table("MEMORY_STATISTICS") + memory_statistics_table = db_connector.get_table("MEMORY_STATISTICS") if (memory_statistics_table and "memory_statistics" in memory_statistics_table and - field_name in memory_statistics_table["config"]): + field_name in memory_statistics_table["memory_statistics"]): field_value = memory_statistics_table["memory_statistics"][field_name] return field_value -@memory_statistics.command(name="memory_statitics", short_help="Show the configuration of memory statistics") -def config(): +@memory_statistics.command(name="memory_statistics", short_help="Show the configuration of memory statistics") +@click.pass_context +def config(ctx): + """Show the configuration of memory statistics.""" + db_connector = ctx.obj['db_connector'] # Get the database connector from the context admin_mode = "Disabled" - admin_enabled = get_memory_statistics_config("enabled") + admin_enabled = get_memory_statistics_config("enabled", db_connector) if admin_enabled == "true": admin_mode = "Enabled" click.echo("Memory Statistics administrative mode: {}".format(admin_mode)) - retention_time = get_memory_statistics_config("retention_time") + retention_time = get_memory_statistics_config("retention_period", db_connector) click.echo("Memory Statistics retention time (days): {}".format(retention_time)) - sampling_interval = get_memory_statistics_config("sampling_interval") + sampling_interval = get_memory_statistics_config("sampling_interval", db_connector) click.echo("Memory Statistics sampling interval (minutes): {}".format(sampling_interval)) -def fetch_memory_statistics(starting_time=None, ending_time=None, select=None): +def fetch_memory_statistics(starting_time=None, ending_time=None, additional_options=None, db_connector=None): """Fetch memory statistics from the database. Args: starting_time: The starting time for filtering the statistics. ending_time: The ending time for filtering the statistics. additional_options: Any additional options for filtering or formatting. + db_connector: The database connector. Returns: A list of memory statistics entries. """ - config_db = ConfigDBConnector() - config_db.connect() - - memory_statistics_table = config_db.get_table("MEMORY_STATISTICS") + memory_statistics_table = db_connector.get_table("MEMORY_STATISTICS") filtered_statistics = [] - for key, entry in memory_statistics_table.items(): - # Add filtering logic here based on starting_time, ending_time, and select - if (not starting_time or entry.get("time") >= starting_time) and \ - (not ending_time or entry.get("time") <= ending_time): - # Implement additional filtering based on select if needed - filtered_statistics.append(entry) + # Ensure you access the right table structure + if "memory_statistics" in memory_statistics_table: + for key, entry in memory_statistics_table["memory_statistics"].items(): + # Add filtering logic here based on starting_time, ending_time, and additional options + if (not starting_time or entry.get("time", "") >= starting_time) and \ + (not ending_time or entry.get("time", "") <= ending_time): + filtered_statistics.append(entry) return filtered_statistics @@ -83,18 +83,29 @@ def fetch_memory_statistics(starting_time=None, ending_time=None, select=None): @click.argument('starting_time', required=False) @click.argument('ending_time', required=False) @click.argument('additional_options', required=False, nargs=-1) -def show_memory_statistics_logs(starting_time, ending_time, select): - """Show memory statistics logs with optional filtering by time and select.""" +@click.pass_context +def show_memory_statistics_logs(ctx, starting_time, ending_time, additional_options): + """Show memory statistics logs with optional filtering by time and additional options.""" + db_connector = ctx.obj['db_connector'] # Get the database connector from the context # Fetch memory statistics - memory_statistics = fetch_memory_statistics(starting_time, ending_time, select) - + memory_statistics = fetch_memory_statistics( + starting_time, + ending_time, + additional_options, + db_connector=db_connector + ) if not memory_statistics: click.echo("No memory statistics available for the given parameters.") return # Display the memory statistics headers = ["Time", "Statistic", "Value"] # Adjust according to the actual fields - table_data = [[entry.get("time"), entry.get("statistic"), entry.get("value")] for entry in memory_statistics] - - click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) + table_data = [ + [ + entry.get("time", "N/A"), + entry.get("statistic", "N/A"), + entry.get("value", "N/A"), + ] for entry in memory_statistics + ] + click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) \ No newline at end of file diff --git a/tests/config_memory_statistics_test.py b/tests/config_memory_statistics_test.py new file mode 100644 index 0000000000..cd6c1f96da --- /dev/null +++ b/tests/config_memory_statistics_test.py @@ -0,0 +1,188 @@ +import pytest +from unittest.mock import patch +from click.testing import CliRunner +from utilities_common.cli import AbbreviationGroup +from config.memory_statistics import ( + memory_statistics_enable, + memory_statistics_disable, + memory_statistics_retention_period, + memory_statistics_sampling_interval, + get_memory_statistics_table, + check_memory_statistics_table_existence, +) + + +@pytest.fixture +def mock_db(): + """Fixture for the mock database.""" + with patch("config.memory_statistics.ConfigDBConnector") as MockConfigDBConnector: + mock_db_instance = MockConfigDBConnector.return_value + yield mock_db_instance + + +def test_memory_statistics_enable(mock_db): + """Test enabling the Memory Statistics feature.""" + mock_db.get_table.return_value = {"memory_statistics": {"enabled": "false"}} + runner = CliRunner() + + with patch("click.echo") as mock_echo: + result = runner.invoke(memory_statistics_enable) + assert result.exit_code == 0 # Ensure the command exits without error + assert mock_echo.call_count == 2 # Check if the echo function was called twice + mock_db.mod_entry.assert_called_once_with( + "MEMORY_STATISTICS", "memory_statistics", + {"enabled": "true", "disabled": "false"} + ) + + +def test_memory_statistics_disable(mock_db): + """Test disabling the Memory Statistics feature.""" + mock_db.get_table.return_value = {"memory_statistics": {"enabled": "true"}} + runner = CliRunner() + + with patch("click.echo") as mock_echo: + result = runner.invoke(memory_statistics_disable) + assert result.exit_code == 0 + assert mock_echo.call_count == 2 + mock_db.mod_entry.assert_called_once_with( + "MEMORY_STATISTICS", "memory_statistics", + {"enabled": "false", "disabled": "true"} + ) + + +def test_memory_statistics_disable_exception(mock_db): + """Test disabling Memory Statistics feature when an exception occurs.""" + mock_db.get_table.return_value = {"memory_statistics": {"enabled": "true"}} + runner = CliRunner() + + # Mock `mod_entry` to raise an exception. + mock_db.mod_entry.side_effect = Exception("Simulated database error") + + with patch("click.echo") as mock_echo: + result = runner.invoke(memory_statistics_disable) + assert result.exit_code == 0 # Ensure the command exits without crashing. + + # Check that the error message was outputted. + mock_echo.assert_any_call("Error disabling Memory Statistics feature: Simulated database error", err=True) + + +def test_memory_statistics_retention_period(mock_db): + """Test setting the retention period for Memory Statistics.""" + mock_db.get_table.return_value = {"memory_statistics": {}} + runner = CliRunner() + retention_period_value = 30 + + with patch("click.echo") as mock_echo: + result = runner.invoke(memory_statistics_retention_period, [str(retention_period_value)]) + assert result.exit_code == 0 + assert mock_echo.call_count == 2 + mock_db.mod_entry.assert_called_once_with( + "MEMORY_STATISTICS", "memory_statistics", + {"retention_period": retention_period_value} + ) + + +def test_memory_statistics_retention_period_exception(mock_db): + """Test setting retention period for Memory Statistics when an exception occurs.""" + mock_db.get_table.return_value = {"memory_statistics": {}} + runner = CliRunner() + retention_period_value = 30 + + # Mock `mod_entry` to raise an exception. + mock_db.mod_entry.side_effect = Exception("Simulated retention period error") + + with patch("click.echo") as mock_echo: + result = runner.invoke(memory_statistics_retention_period, [str(retention_period_value)]) + assert result.exit_code == 0 # Ensure the command exits without crashing. + + # Check that the error message was outputted. + mock_echo.assert_any_call("Error setting retention period: Simulated retention period error", err=True) + + +def test_memory_statistics_sampling_interval(mock_db): + """Test setting the sampling interval for Memory Statistics.""" + mock_db.get_table.return_value = {"memory_statistics": {}} + runner = CliRunner() + sampling_interval_value = 10 + + with patch("click.echo") as mock_echo: + result = runner.invoke(memory_statistics_sampling_interval, [str(sampling_interval_value)]) + assert result.exit_code == 0 + assert mock_echo.call_count == 2 + mock_db.mod_entry.assert_called_once_with( + "MEMORY_STATISTICS", "memory_statistics", + {"sampling_interval": sampling_interval_value} + ) + + +def test_memory_statistics_sampling_interval_exception(mock_db): + """Test setting sampling interval for Memory Statistics when an exception occurs.""" + mock_db.get_table.return_value = {"memory_statistics": {}} + runner = CliRunner() + sampling_interval_value = 10 + + # Mock `mod_entry` to raise an exception. + mock_db.mod_entry.side_effect = Exception("Simulated sampling interval error") + + with patch("click.echo") as mock_echo: + result = runner.invoke(memory_statistics_sampling_interval, [str(sampling_interval_value)]) + assert result.exit_code == 0 # Ensure the command exits without crashing. + + # Check that the error message was outputted. + mock_echo.assert_any_call("Error setting sampling interval: Simulated sampling interval error", err=True) + + +def test_check_memory_statistics_table_existence(): + """Test existence check for MEMORY_STATISTICS table.""" + assert check_memory_statistics_table_existence({"memory_statistics": {}}) is True + assert check_memory_statistics_table_existence({}) is False + + +def test_get_memory_statistics_table(mock_db): + """Test getting MEMORY_STATISTICS table.""" + mock_db.get_table.return_value = {"memory_statistics": {}} + + result = get_memory_statistics_table(mock_db) + assert result == {"memory_statistics": {}} + + +def test_abbreviation_group_get_command_existing_command(): + """Test AbbreviationGroup's get_command method with an existing command.""" + # Create an instance of AbbreviationGroup with a sample command. + group = AbbreviationGroup() + + # Invoke get_command with the name of the existing command. + command = group.get_command(ctx=None, cmd_name="existing_command") + + # Check that the correct command is returned. + assert command is None + + +def test_check_memory_statistics_table_existence_missing_key(): + """Test check_memory_statistics_table_existence when 'memory_statistics' key is missing.""" + with patch("click.echo") as mock_echo: + result = check_memory_statistics_table_existence({"another_key": {}}) + + # Ensure the function returns False when 'memory_statistics' key is missing. + assert result is False + + # Check that the specific error message was outputted. + mock_echo.assert_called_once_with( + "Unable to retrieve key 'memory_statistics' from MEMORY_STATISTICS table.", err=True + ) + + +def test_memory_statistics_enable_exception(mock_db): + """Test enabling Memory Statistics feature when an exception occurs.""" + mock_db.get_table.return_value = {"memory_statistics": {"enabled": "false"}} + runner = CliRunner() + + # Mock `mod_entry` to raise an exception. + mock_db.mod_entry.side_effect = Exception("Simulated database error") + + with patch("click.echo") as mock_echo: + result = runner.invoke(memory_statistics_enable) + assert result.exit_code == 0 # Ensure the command exits without crashing. + + # Check that the error message was outputted. + mock_echo.assert_any_call("Error enabling Memory Statistics feature: Simulated database error", err=True) \ No newline at end of file diff --git a/tests/memory_statistics_test.py b/tests/memory_statistics_test.py deleted file mode 100644 index d31c07e2ce..0000000000 --- a/tests/memory_statistics_test.py +++ /dev/null @@ -1,91 +0,0 @@ -import unittest -from unittest.mock import patch, MagicMock -from click.testing import CliRunner -from config.memory_statistics import ( - memory_statistics_enable, - memory_statistics_disable, - memory_statistics_retention_period, - memory_statistics_sampling_interval -) -from swsscommon.swsscommon import ConfigDBConnector - - -class TestMemoryStatisticsConfigCommands(unittest.TestCase): - - def setUp(self): - self.runner = CliRunner() - self.mock_db = MagicMock() - self.mock_db.get_entry.return_value = { - "enabled": "false", - "retention_period": "15", - "sampling_interval": "5" - } - self.patcher = patch.object(ConfigDBConnector, 'get_entry', self.mock_db.get_entry) - self.patcher.start() - - # Mock the get_memory_statistics_table to return a valid table - self.mock_db.get_table = MagicMock(return_value={"memory_statistics": {}}) - patch.object(ConfigDBConnector, 'get_table', self.mock_db.get_table).start() - - def tearDown(self): - self.patcher.stop() - - @patch.object(ConfigDBConnector, 'mod_entry') - def test_memory_statistics_enable(self, mock_mod_entry): - # Change the return value to simulate a disabled state - self.mock_db.get_entry.return_value = {"enabled": "false"} - result = self.runner.invoke(memory_statistics_enable) - self.assertIn("Memory Statistics feature enabled.", result.output) - self.assertEqual(result.exit_code, 0) - - # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with( - "MEMORY_STATISTICS", - "memory_statistics", - {"enabled": "true", "disabled": "false"} - ) - - @patch.object(ConfigDBConnector, 'mod_entry') - def test_memory_statistics_disable(self, mock_mod_entry): - # Change the return value to simulate an enabled state - self.mock_db.get_entry.return_value = {"enabled": "true"} - result = self.runner.invoke(memory_statistics_disable) - self.assertIn("Memory Statistics feature disabled.", result.output) - self.assertEqual(result.exit_code, 0) - - # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with( - "MEMORY_STATISTICS", - "memory_statistics", - {"enabled": "false", "disabled": "true"} - ) - - @patch.object(ConfigDBConnector, 'mod_entry') - def test_memory_statistics_retention_period(self, mock_mod_entry): - result = self.runner.invoke(memory_statistics_retention_period, ['15']) - self.assertIn("Memory Statistics retention period set to 15 days.", result.output) - self.assertEqual(result.exit_code, 0) - - # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with( - "MEMORY_STATISTICS", - "memory_statistics", - {"retention_period": 15} - ) - - @patch.object(ConfigDBConnector, 'mod_entry') - def test_memory_statistics_sampling_interval(self, mock_mod_entry): - result = self.runner.invoke(memory_statistics_sampling_interval, ['5']) - self.assertIn("Memory Statistics sampling interval set to 5 minutes.", result.output) - self.assertEqual(result.exit_code, 0) - - # Ensure the entry was modified correctly - mock_mod_entry.assert_called_once_with( - "MEMORY_STATISTICS", - "memory_statistics", - {"sampling_interval": 5} - ) - - -if __name__ == "__main__": - unittest.main() diff --git a/tests/show_memory_statistics_test.py b/tests/show_memory_statistics_test.py new file mode 100644 index 0000000000..f7b02638bd --- /dev/null +++ b/tests/show_memory_statistics_test.py @@ -0,0 +1,70 @@ +# import pytest +from click.testing import CliRunner +# from unittest.mock import MagicMock + +from show.memory_statistics import memory_statistics + + +class MockConfigDBConnector: + def __init__(self, data): + self.data = data + + def get_table(self, table_name): + return self.data.get(table_name, {}) + + +def test_show_memory_statistics_logs(): + # Prepare mock data with some example log entries + mock_data = { + "MEMORY_STATISTICS": { + "memory_statistics": { + "log1": {"time": "2024-11-04 10:00:00", "statistic": "Usage", "value": "512 MB"}, + "log2": {"time": "2024-11-04 10:05:00", "statistic": "Usage", "value": "514 MB"}, + } + } + } + + # Create a mock database connector with the prepared data + mock_db = MockConfigDBConnector(data=mock_data) + + # Use CliRunner to invoke the CLI command with the mock database + runner = CliRunner() + result = runner.invoke(memory_statistics, ["logs", "2024-11-04 09:00:00", "2024-11-04 11:00:00"], + obj={"db_connector": mock_db}) + + # Print result output and exception details for debugging + if result.exit_code != 0: + print("Command failed with output:", result.output) + print("Exception:", result.exception) + + # Assertions to verify the output matches the mock data + assert result.exit_code == 0 + assert "2024-11-04 10:00:00" in result.output + assert "Usage" in result.output + assert "512 MB" in result.output + + +def test_show_memory_statistics_config(): + # Prepare mock data for configuration + mock_data = { + "MEMORY_STATISTICS": { + "memory_statistics": { + "enabled": "true", + "retention_period": "30", + "sampling_interval": "10", + } + } + } + + # Create a mock database connector with the prepared data + mock_db = MockConfigDBConnector(data=mock_data) + + # Use CliRunner to invoke the CLI command with the mock database + runner = CliRunner() + result = runner.invoke(memory_statistics, ["memory_statistics"], obj={"db_connector": mock_db}) + + # Assertions to verify the output matches the mock data + assert result.exit_code == 0 + assert "Memory Statistics administrative mode: Enabled" in result.output + assert "Memory Statistics retention time (days): 30" in result.output + assert "Memory Statistics sampling interval (minutes): 10" in result.output \ No newline at end of file From 5e72f6bf4cf19fd19ddef5c397e61d5a3722df55 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Tue, 5 Nov 2024 11:37:12 +0500 Subject: [PATCH 18/30] added testfile for show commands --- config/memory_statistics.py | 2 +- show/memory_statistics.py | 2 +- tests/config_memory_statistics_test.py | 2 +- tests/show_memory_statistics_test.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 4228f45cfb..4dbd3f3ad4 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -111,4 +111,4 @@ def memory_statistics_sampling_interval(sampling_interval): if __name__ == "__main__": - memory_statistics() \ No newline at end of file + memory_statistics() diff --git a/show/memory_statistics.py b/show/memory_statistics.py index 568148ba7e..3289e87da9 100644 --- a/show/memory_statistics.py +++ b/show/memory_statistics.py @@ -108,4 +108,4 @@ def show_memory_statistics_logs(ctx, starting_time, ending_time, additional_opti entry.get("value", "N/A"), ] for entry in memory_statistics ] - click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) \ No newline at end of file + click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) diff --git a/tests/config_memory_statistics_test.py b/tests/config_memory_statistics_test.py index cd6c1f96da..fb33703145 100644 --- a/tests/config_memory_statistics_test.py +++ b/tests/config_memory_statistics_test.py @@ -185,4 +185,4 @@ def test_memory_statistics_enable_exception(mock_db): assert result.exit_code == 0 # Ensure the command exits without crashing. # Check that the error message was outputted. - mock_echo.assert_any_call("Error enabling Memory Statistics feature: Simulated database error", err=True) \ No newline at end of file + mock_echo.assert_any_call("Error enabling Memory Statistics feature: Simulated database error", err=True) diff --git a/tests/show_memory_statistics_test.py b/tests/show_memory_statistics_test.py index f7b02638bd..2ff10d10c9 100644 --- a/tests/show_memory_statistics_test.py +++ b/tests/show_memory_statistics_test.py @@ -67,4 +67,4 @@ def test_show_memory_statistics_config(): assert result.exit_code == 0 assert "Memory Statistics administrative mode: Enabled" in result.output assert "Memory Statistics retention time (days): 30" in result.output - assert "Memory Statistics sampling interval (minutes): 10" in result.output \ No newline at end of file + assert "Memory Statistics sampling interval (minutes): 10" in result.output From a51c84dddd5147742a699843d09c02364c6a9d48 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Wed, 6 Nov 2024 23:08:06 +0500 Subject: [PATCH 19/30] removed the commented import --- config/memory_statistics.py | 1 - 1 file changed, 1 deletion(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 4dbd3f3ad4..d46c2daedd 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -1,6 +1,5 @@ import click from swsscommon.swsscommon import ConfigDBConnector -# from utilities_common.cli import AbbreviationGroup class AbbreviationGroup(click.Group): From 07fbf15b6d777bd829cfc55f4cd8008262b3640f Mon Sep 17 00:00:00 2001 From: Arham-Nasir Date: Wed, 13 Nov 2024 18:54:25 +0500 Subject: [PATCH 20/30] Add show memory-stats CLI command with Dict2Obj handling, clean output and socket communication Signed-off-by: Arham-Nasir --- show/memory_statistics.py | 336 ++++++++++++++++++++++++++------------ 1 file changed, 235 insertions(+), 101 deletions(-) diff --git a/show/memory_statistics.py b/show/memory_statistics.py index 3289e87da9..7d54aa9d17 100644 --- a/show/memory_statistics.py +++ b/show/memory_statistics.py @@ -1,111 +1,245 @@ +import sys +import socket +import json import click -from tabulate import tabulate - +import syslog +from click_default_group import DefaultGroup +from difflib import get_close_matches +from datetime import datetime, timedelta import utilities_common.cli as clicommon - -# -# 'memory-statistics' group (show memory-statistics ...) -# -@click.group(cls=clicommon.AliasedGroup, name="memory-statistics") -def memory_statistics(): - """Show memory statistics configuration and logs""" - pass - - -def get_memory_statistics_config(field_name, db_connector): - """Fetches the configuration of memory_statistics from `CONFIG_DB`. - - Args: - field_name: A string containing the field name in the sub-table of 'memory_statistics'. - db_connector: The database connector. - - Returns: - field_value: If field name was found, then returns the corresponding value. - Otherwise, returns "Unknown". +class Dict2Obj(object): + """Converts dictionaries or lists into objects with attribute-style access. + + Recursively transforms dictionaries and lists to allow accessing nested + structures as attributes. Supports nested dictionaries and lists of dictionaries. """ - field_value = "Unknown" - memory_statistics_table = db_connector.get_table("MEMORY_STATISTICS") - if (memory_statistics_table and - "memory_statistics" in memory_statistics_table and - field_name in memory_statistics_table["memory_statistics"]): - field_value = memory_statistics_table["memory_statistics"][field_name] - return field_value - - -@memory_statistics.command(name="memory_statistics", short_help="Show the configuration of memory statistics") + def __init__(self, d): + """Initializes the Dict2Obj object. + + Parameters: + d (dict or list): The dictionary or list to be converted into an object. + """ + if not isinstance(d, (dict, list)): + raise ValueError("Input should be a dictionary or a list") + + if isinstance(d, dict): + for key, value in d.items(): + if isinstance(value, (list, tuple)): + setattr(self, key, [Dict2Obj(x) if isinstance(x, dict) else x for x in value]) + else: + setattr(self, key, Dict2Obj(value) if isinstance(value, dict) else value) + elif isinstance(d, list): + self.items = [Dict2Obj(x) if isinstance(x, dict) else x for x in d] + + def to_dict(self): + """Converts the object back to a dictionary format.""" + result = {} + if hasattr(self, 'items'): + return [x.to_dict() if isinstance(x, Dict2Obj) else x for x in self.items] + + for key in self.__dict__: + value = getattr(self, key) + if isinstance(value, Dict2Obj): + result[key] = value.to_dict() + elif isinstance(value, list): + result[key] = [v.to_dict() if isinstance(v, Dict2Obj) else v for v in value] + else: + result[key] = value + return result + + def __repr__(self): + """Provides a string representation of the object for debugging.""" + return f"<{self.__class__.__name__} {self.to_dict()}>" + +syslog.openlog(ident="memory_statistics_cli", logoption=syslog.LOG_PID) + +@click.group(cls=DefaultGroup, default='show', default_if_no_args=True) @click.pass_context -def config(ctx): - """Show the configuration of memory statistics.""" - db_connector = ctx.obj['db_connector'] # Get the database connector from the context - admin_mode = "Disabled" - admin_enabled = get_memory_statistics_config("enabled", db_connector) - if admin_enabled == "true": - admin_mode = "Enabled" - - click.echo("Memory Statistics administrative mode: {}".format(admin_mode)) - - retention_time = get_memory_statistics_config("retention_period", db_connector) - click.echo("Memory Statistics retention time (days): {}".format(retention_time)) - - sampling_interval = get_memory_statistics_config("sampling_interval", db_connector) - click.echo("Memory Statistics sampling interval (minutes): {}".format(sampling_interval)) - - -def fetch_memory_statistics(starting_time=None, ending_time=None, additional_options=None, db_connector=None): - """Fetch memory statistics from the database. - - Args: - starting_time: The starting time for filtering the statistics. - ending_time: The ending time for filtering the statistics. - additional_options: Any additional options for filtering or formatting. - db_connector: The database connector. - - Returns: - A list of memory statistics entries. +def cli(ctx): + """Main entry point for the SONiC CLI. + + Parameters: + ctx (click.Context): The Click context that holds configuration data and other CLI-related information. """ - memory_statistics_table = db_connector.get_table("MEMORY_STATISTICS") - filtered_statistics = [] - - # Ensure you access the right table structure - if "memory_statistics" in memory_statistics_table: - for key, entry in memory_statistics_table["memory_statistics"].items(): - # Add filtering logic here based on starting_time, ending_time, and additional options - if (not starting_time or entry.get("time", "") >= starting_time) and \ - (not ending_time or entry.get("time", "") <= ending_time): - filtered_statistics.append(entry) + ctx.ensure_object(dict) + + if clicommon: + try: + ctx.obj['db_connector'] = clicommon.get_db_connector() + except AttributeError as e: + error_msg = "Error: 'utilities_common.cli' does not have 'get_db_connector' function." + click.echo(error_msg, err=True) + syslog.syslog(syslog.LOG_ERR, error_msg) + sys.exit(1) + else: + ctx.obj['db_connector'] = None + +def validate_command(command, valid_commands): + """Validates the user's command input against a list of valid commands. + + Parameters: + command (str): The command entered by the user. + valid_commands (list): List of valid command strings. + + Raises: + click.UsageError: If the command is invalid, with suggestions for the closest valid command. + """ + match = get_close_matches(command, valid_commands, n=1, cutoff=0.6) + if match: + error_msg = f"Error: No such command '{command}'. Did you mean '{match[0]}'?" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise click.UsageError(error_msg) + else: + error_msg = f"Error: No such command '{command}'." + syslog.syslog(syslog.LOG_ERR, error_msg) + raise click.UsageError(error_msg) + +# ------------------- Integration of show memory-stats Command ------------------- + +@cli.group() +@click.pass_context +def show(ctx): + """Displays various information about the system using the 'show' subcommand. + + Parameters: + ctx (click.Context): The Click context that holds configuration data and other CLI-related information. + """ + pass - return filtered_statistics +@show.command(name='memory-stats') +@click.argument('from_keyword', required=False) +@click.argument('from_time', required=False) +@click.argument('to_keyword', required=False) +@click.argument('to_time', required=False) +@click.argument('select_keyword', required=False) +@click.argument('select_metric', required=False) +@click.pass_context +def memory_stats(ctx, from_keyword, from_time, to_keyword, to_time, select_keyword, select_metric): + """Displays memory statistics. + + Fetches and shows memory statistics based on the provided time range and metric. + If no time range or metric is specified, defaults are used. + + Parameters: + ctx (click.Context): The Click context holding configuration and command info. + from_keyword (str): Expected keyword 'from' indicating the start of the time range. + from_time (str): The start time for the data retrieval. + to_keyword (str): Expected keyword 'to' indicating the end of the time range. + to_time (str): The end time for the data retrieval. + select_keyword (str): Expected keyword 'select' to indicate a specific metric. + select_metric (str): The specific metric to retrieve data for. + """ + request_data = { + "type": "system", + "metric_name": None, + "from": None, + "to": None + } + + if from_keyword: + if from_keyword != 'from': + raise click.UsageError("Expected 'from' keyword as the first argument.") + if to_keyword and to_keyword != 'to': + raise click.UsageError("Expected 'to' keyword before the end time.") + if select_keyword and select_keyword != 'select': + raise click.UsageError("Expected 'select' keyword before the metric name.") + + request_data["from"] = from_time.strip("'\"") + if to_time: + request_data["to"] = to_time.strip("'\"") + if select_metric: + request_data["metric_name"] = select_metric.strip("'\"") + + try: + response = send_data("memory_statistics_command_request_handler", request_data) + + if isinstance(response, Dict2Obj): + clean_and_print(response.to_dict()) + else: + error_msg = f"Error: Expected Dict2Obj, but got {type(response)}" + syslog.syslog(syslog.LOG_ERR, error_msg) + print(error_msg) + + except Exception as e: + error_msg = f"Error: {str(e)}" + syslog.syslog(syslog.LOG_ERR, error_msg) + print(error_msg) + + +def clean_and_print(data): + """Formats and prints memory statistics in a user-friendly format. + + If the data is received in a valid format, it extracts the relevant memory + statistics and prints them. Otherwise, it prints an error message. + + Parameters: + data (dict): Dictionary containing the memory statistics to display. + """ + if isinstance(data, dict): + status = data.get("status", True) + memory_stats = data.get("data", "") + + cleaned_output = memory_stats.replace("\n", "\n").strip() + print(f"Memory Statistics:\n{cleaned_output}") + else: + print("Error: Invalid data format.") + +def send_data(command, data, quiet=False): + """Sends a command and data to the memory statistics service. + + Connects to the UNIX socket, sends the JSON-encoded command and data, + and returns the response. If the service is unavailable, it handles the error. + + Parameters: + command (str): The command name to be sent to the server. + data (dict): Data payload containing parameters for the command. + quiet (bool): If True, suppresses output on exceptions. + Returns: + response (Dict2Obj): Parsed response from the server. -@memory_statistics.command(name="logs", short_help="Show memory statistics logs with optional filtering") -@click.argument('starting_time', required=False) -@click.argument('ending_time', required=False) -@click.argument('additional_options', required=False, nargs=-1) -@click.pass_context -def show_memory_statistics_logs(ctx, starting_time, ending_time, additional_options): - """Show memory statistics logs with optional filtering by time and additional options.""" - db_connector = ctx.obj['db_connector'] # Get the database connector from the context - - # Fetch memory statistics - memory_statistics = fetch_memory_statistics( - starting_time, - ending_time, - additional_options, - db_connector=db_connector - ) - if not memory_statistics: - click.echo("No memory statistics available for the given parameters.") - return - - # Display the memory statistics - headers = ["Time", "Statistic", "Value"] # Adjust according to the actual fields - table_data = [ - [ - entry.get("time", "N/A"), - entry.get("statistic", "N/A"), - entry.get("value", "N/A"), - ] for entry in memory_statistics - ] - click.echo(tabulate(table_data, headers=headers, tablefmt="grid")) + Raises: + click.Abort: If there are issues connecting to the server or receiving data. + """ + SERVER_ADDRESS = '/var/run/dbus/memstats.socket' + sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + try: + sock.connect(SERVER_ADDRESS) + except socket.error as msg: + error_msg = "Could not connect to the server. Please check if the memory stats service is running." + syslog.syslog(syslog.LOG_ERR, error_msg) + raise click.Abort(error_msg) from msg + + response = {} + try: + request = {"command": command, "data": data} + sock.sendall(json.dumps(request).encode('utf-8')) + res = sock.recv(40240).decode('utf-8') + + if res == '': + sock.close() + raise click.Abort("No response from the server. Please check the service and try again.") + + jdata = json.loads(res) + + if isinstance(jdata, dict): + response = Dict2Obj(jdata) + else: + raise Exception("Unexpected response format from server") + + if not getattr(response, 'status', True): + sock.close() + raise click.Abort(getattr(response, 'msg', 'An error occurred')) + + except Exception as e: + if quiet: + sock.close() + raise click.Abort(str(e)) + click.echo("Error: {}".format(str(e))) + sock.close() + sys.exit(1) + + sock.close() + return response From 9ddaf964e9cc0742b632f433ddd63759b61e2cf3 Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 25 Nov 2024 16:16:17 +0500 Subject: [PATCH 21/30] updated config and test files --- config/memory_statistics.py | 167 +++++++++-------- show/memory_statistics.py | 245 ------------------------- tests/config_memory_statistics_test.py | 172 +++++++---------- tests/show_memory_statistics_test.py | 70 ------- 4 files changed, 154 insertions(+), 500 deletions(-) delete mode 100644 show/memory_statistics.py delete mode 100644 tests/show_memory_statistics_test.py diff --git a/config/memory_statistics.py b/config/memory_statistics.py index d46c2daedd..a7da905789 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -1,113 +1,118 @@ import click +import syslog from swsscommon.swsscommon import ConfigDBConnector +# Default values +DEFAULT_SAMPLING_INTERVAL = 5 +DEFAULT_RETENTION_PERIOD = 15 -class AbbreviationGroup(click.Group): - def get_command(self, ctx, cmd_name): - return super().get_command(ctx, cmd_name) +def log_to_syslog(message, level=syslog.LOG_INFO): + """Log a message to syslog.""" + syslog.openlog("memory_statistics", syslog.LOG_PID | syslog.LOG_CONS, syslog.LOG_USER) + syslog.syslog(level, message) -@click.group(cls=AbbreviationGroup, name="memory-statistics") -def memory_statistics(): - """Configure the Memory Statistics feature""" - pass - -def check_memory_statistics_table_existence(memory_statistics_table): - """Checks whether the 'MEMORY_STATISTICS' table is configured in Config DB.""" - if not memory_statistics_table: - click.echo("Unable to retrieve 'MEMORY_STATISTICS' table from Config DB.", err=True) - return False - - if "memory_statistics" not in memory_statistics_table: - click.echo("Unable to retrieve key 'memory_statistics' from MEMORY_STATISTICS table.", err=True) - return False - - return True - - -def get_memory_statistics_table(db): - """Get the MEMORY_STATISTICS table from the database.""" - return db.get_table("MEMORY_STATISTICS") +def update_memory_statistics_status(status, db): + """Updates the status of the memory statistics feature in the config DB.""" + try: + db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": status}) + msg = f"Memory statistics feature {'enabled' if status == 'true' else 'disabled'} successfully." + click.echo(msg) + log_to_syslog(msg) # Log to syslog + return True, None # Success: return True and no error + except Exception as e: + error_msg = f"Error updating memory statistics status: {e}" + click.echo(error_msg, err=True) + log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog + return False, error_msg # Failure: return False and the error message -@memory_statistics.command(name="enable", short_help="Enable the Memory Statistics feature") +@click.command() def memory_statistics_enable(): - """Enable the Memory Statistics feature""" + """Enable memory statistics.""" db = ConfigDBConnector() db.connect() - - memory_statistics_table = get_memory_statistics_table(db) - if not check_memory_statistics_table_existence(memory_statistics_table): - return # Exit gracefully on error - - try: - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "true", "disabled": "false"}) - click.echo("Memory Statistics feature enabled.") - except Exception as e: - click.echo(f"Error enabling Memory Statistics feature: {str(e)}", err=True) - - click.echo("Save SONiC configuration using 'config save' to persist the changes.") - - -@memory_statistics.command(name="disable", short_help="Disable the Memory Statistics feature") + success, error = update_memory_statistics_status("true", db) + if not success: + click.echo(error, err=True) # Handle error if unsuccessful + else: + success_msg = "Memory statistics enabled successfully." + click.echo(success_msg) + log_to_syslog(success_msg) # Log to syslog + reminder_msg = "Reminder: Please run 'config save' to persist changes." + click.echo(reminder_msg) + log_to_syslog(reminder_msg) # Log to syslog + + +@click.command() def memory_statistics_disable(): - """Disable the Memory Statistics feature""" + """Disable memory statistics.""" db = ConfigDBConnector() db.connect() - - memory_statistics_table = get_memory_statistics_table(db) - if not check_memory_statistics_table_existence(memory_statistics_table): - return # Exit gracefully on error - - try: - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": "false", "disabled": "true"}) - click.echo("Memory Statistics feature disabled.") - except Exception as e: - click.echo(f"Error disabling Memory Statistics feature: {str(e)}", err=True) - - click.echo("Save SONiC configuration using 'config save' to persist the changes.") - - -@memory_statistics.command(name="retention-period", short_help="Configure the retention period for Memory Statistics") -@click.argument('retention_period', metavar='', required=True, type=int) + success, error = update_memory_statistics_status("false", db) + if not success: + click.echo(error, err=True) # Handle error if unsuccessful + else: + success_msg = "Memory statistics disabled successfully." + click.echo(success_msg) + log_to_syslog(success_msg) # Log to syslog + reminder_msg = "Reminder: Please run 'config save' to persist changes." + click.echo(reminder_msg) + log_to_syslog(reminder_msg) # Log to syslog + + +@click.command() +@click.argument("retention_period", type=int, required=False, default=DEFAULT_RETENTION_PERIOD) def memory_statistics_retention_period(retention_period): - """Set the retention period for Memory Statistics""" + """Set retention period for memory statistics.""" + if not (1 <= retention_period <= 30): + error_msg = "Error: Retention period must be between 1 and 30." + click.echo(error_msg, err=True) + log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog + return + db = ConfigDBConnector() db.connect() - - memory_statistics_table = get_memory_statistics_table(db) - if not check_memory_statistics_table_existence(memory_statistics_table): - return # Exit gracefully on error - try: db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"retention_period": retention_period}) - click.echo(f"Memory Statistics retention period set to {retention_period} days.") + success_msg = f"Retention period set to {retention_period} successfully." + click.echo(success_msg) + log_to_syslog(success_msg) # Log to syslog except Exception as e: - click.echo(f"Error setting retention period: {str(e)}", err=True) - - click.echo("Save SONiC configuration using 'config save' to persist the changes.") + error_msg = f"Error setting retention period: {e}" + click.echo(error_msg, err=True) + log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog -@memory_statistics.command(name="sampling-interval", short_help="Configure the sampling interval for Memory Statistics") -@click.argument('sampling_interval', metavar='', required=True, type=int) +@click.command() +@click.argument("sampling_interval", type=int, required=False, default=DEFAULT_SAMPLING_INTERVAL) def memory_statistics_sampling_interval(sampling_interval): - """Set the sampling interval for Memory Statistics""" + """Set sampling interval for memory statistics.""" + if not (3 <= sampling_interval <= 15): + error_msg = "Error: Sampling interval must be between 3 and 15." + click.echo(error_msg, err=True) + log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog + return + db = ConfigDBConnector() db.connect() - - memory_statistics_table = get_memory_statistics_table(db) - if not check_memory_statistics_table_existence(memory_statistics_table): - return # Exit gracefully on error - try: db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval}) - click.echo(f"Memory Statistics sampling interval set to {sampling_interval} minutes.") + success_msg = f"Sampling interval set to {sampling_interval} successfully." + click.echo(success_msg) + log_to_syslog(success_msg) # Log to syslog except Exception as e: - click.echo(f"Error setting sampling interval: {str(e)}", err=True) + error_msg = f"Error setting sampling interval: {e}" + click.echo(error_msg, err=True) + log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog + - click.echo("Save SONiC configuration using 'config save' to persist the changes.") +def get_memory_statistics_table(db): + """Retrieve MEMORY_STATISTICS table from config DB.""" + return db.get_table("MEMORY_STATISTICS") -if __name__ == "__main__": - memory_statistics() +def check_memory_statistics_table_existence(table): + """Check if MEMORY_STATISTICS table exists in the given table.""" + return "memory_statistics" in table \ No newline at end of file diff --git a/show/memory_statistics.py b/show/memory_statistics.py deleted file mode 100644 index 7d54aa9d17..0000000000 --- a/show/memory_statistics.py +++ /dev/null @@ -1,245 +0,0 @@ -import sys -import socket -import json -import click -import syslog -from click_default_group import DefaultGroup -from difflib import get_close_matches -from datetime import datetime, timedelta -import utilities_common.cli as clicommon - -class Dict2Obj(object): - """Converts dictionaries or lists into objects with attribute-style access. - - Recursively transforms dictionaries and lists to allow accessing nested - structures as attributes. Supports nested dictionaries and lists of dictionaries. - """ - - def __init__(self, d): - """Initializes the Dict2Obj object. - - Parameters: - d (dict or list): The dictionary or list to be converted into an object. - """ - if not isinstance(d, (dict, list)): - raise ValueError("Input should be a dictionary or a list") - - if isinstance(d, dict): - for key, value in d.items(): - if isinstance(value, (list, tuple)): - setattr(self, key, [Dict2Obj(x) if isinstance(x, dict) else x for x in value]) - else: - setattr(self, key, Dict2Obj(value) if isinstance(value, dict) else value) - elif isinstance(d, list): - self.items = [Dict2Obj(x) if isinstance(x, dict) else x for x in d] - - def to_dict(self): - """Converts the object back to a dictionary format.""" - result = {} - if hasattr(self, 'items'): - return [x.to_dict() if isinstance(x, Dict2Obj) else x for x in self.items] - - for key in self.__dict__: - value = getattr(self, key) - if isinstance(value, Dict2Obj): - result[key] = value.to_dict() - elif isinstance(value, list): - result[key] = [v.to_dict() if isinstance(v, Dict2Obj) else v for v in value] - else: - result[key] = value - return result - - def __repr__(self): - """Provides a string representation of the object for debugging.""" - return f"<{self.__class__.__name__} {self.to_dict()}>" - -syslog.openlog(ident="memory_statistics_cli", logoption=syslog.LOG_PID) - -@click.group(cls=DefaultGroup, default='show', default_if_no_args=True) -@click.pass_context -def cli(ctx): - """Main entry point for the SONiC CLI. - - Parameters: - ctx (click.Context): The Click context that holds configuration data and other CLI-related information. - """ - ctx.ensure_object(dict) - - if clicommon: - try: - ctx.obj['db_connector'] = clicommon.get_db_connector() - except AttributeError as e: - error_msg = "Error: 'utilities_common.cli' does not have 'get_db_connector' function." - click.echo(error_msg, err=True) - syslog.syslog(syslog.LOG_ERR, error_msg) - sys.exit(1) - else: - ctx.obj['db_connector'] = None - -def validate_command(command, valid_commands): - """Validates the user's command input against a list of valid commands. - - Parameters: - command (str): The command entered by the user. - valid_commands (list): List of valid command strings. - - Raises: - click.UsageError: If the command is invalid, with suggestions for the closest valid command. - """ - match = get_close_matches(command, valid_commands, n=1, cutoff=0.6) - if match: - error_msg = f"Error: No such command '{command}'. Did you mean '{match[0]}'?" - syslog.syslog(syslog.LOG_ERR, error_msg) - raise click.UsageError(error_msg) - else: - error_msg = f"Error: No such command '{command}'." - syslog.syslog(syslog.LOG_ERR, error_msg) - raise click.UsageError(error_msg) - -# ------------------- Integration of show memory-stats Command ------------------- - -@cli.group() -@click.pass_context -def show(ctx): - """Displays various information about the system using the 'show' subcommand. - - Parameters: - ctx (click.Context): The Click context that holds configuration data and other CLI-related information. - """ - pass - -@show.command(name='memory-stats') -@click.argument('from_keyword', required=False) -@click.argument('from_time', required=False) -@click.argument('to_keyword', required=False) -@click.argument('to_time', required=False) -@click.argument('select_keyword', required=False) -@click.argument('select_metric', required=False) -@click.pass_context -def memory_stats(ctx, from_keyword, from_time, to_keyword, to_time, select_keyword, select_metric): - """Displays memory statistics. - - Fetches and shows memory statistics based on the provided time range and metric. - If no time range or metric is specified, defaults are used. - - Parameters: - ctx (click.Context): The Click context holding configuration and command info. - from_keyword (str): Expected keyword 'from' indicating the start of the time range. - from_time (str): The start time for the data retrieval. - to_keyword (str): Expected keyword 'to' indicating the end of the time range. - to_time (str): The end time for the data retrieval. - select_keyword (str): Expected keyword 'select' to indicate a specific metric. - select_metric (str): The specific metric to retrieve data for. - """ - request_data = { - "type": "system", - "metric_name": None, - "from": None, - "to": None - } - - if from_keyword: - if from_keyword != 'from': - raise click.UsageError("Expected 'from' keyword as the first argument.") - if to_keyword and to_keyword != 'to': - raise click.UsageError("Expected 'to' keyword before the end time.") - if select_keyword and select_keyword != 'select': - raise click.UsageError("Expected 'select' keyword before the metric name.") - - request_data["from"] = from_time.strip("'\"") - if to_time: - request_data["to"] = to_time.strip("'\"") - if select_metric: - request_data["metric_name"] = select_metric.strip("'\"") - - try: - response = send_data("memory_statistics_command_request_handler", request_data) - - if isinstance(response, Dict2Obj): - clean_and_print(response.to_dict()) - else: - error_msg = f"Error: Expected Dict2Obj, but got {type(response)}" - syslog.syslog(syslog.LOG_ERR, error_msg) - print(error_msg) - - except Exception as e: - error_msg = f"Error: {str(e)}" - syslog.syslog(syslog.LOG_ERR, error_msg) - print(error_msg) - - -def clean_and_print(data): - """Formats and prints memory statistics in a user-friendly format. - - If the data is received in a valid format, it extracts the relevant memory - statistics and prints them. Otherwise, it prints an error message. - - Parameters: - data (dict): Dictionary containing the memory statistics to display. - """ - if isinstance(data, dict): - status = data.get("status", True) - memory_stats = data.get("data", "") - - cleaned_output = memory_stats.replace("\n", "\n").strip() - print(f"Memory Statistics:\n{cleaned_output}") - else: - print("Error: Invalid data format.") - -def send_data(command, data, quiet=False): - """Sends a command and data to the memory statistics service. - - Connects to the UNIX socket, sends the JSON-encoded command and data, - and returns the response. If the service is unavailable, it handles the error. - - Parameters: - command (str): The command name to be sent to the server. - data (dict): Data payload containing parameters for the command. - quiet (bool): If True, suppresses output on exceptions. - - Returns: - response (Dict2Obj): Parsed response from the server. - - Raises: - click.Abort: If there are issues connecting to the server or receiving data. - """ - SERVER_ADDRESS = '/var/run/dbus/memstats.socket' - sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) - try: - sock.connect(SERVER_ADDRESS) - except socket.error as msg: - error_msg = "Could not connect to the server. Please check if the memory stats service is running." - syslog.syslog(syslog.LOG_ERR, error_msg) - raise click.Abort(error_msg) from msg - - response = {} - try: - request = {"command": command, "data": data} - sock.sendall(json.dumps(request).encode('utf-8')) - res = sock.recv(40240).decode('utf-8') - - if res == '': - sock.close() - raise click.Abort("No response from the server. Please check the service and try again.") - - jdata = json.loads(res) - - if isinstance(jdata, dict): - response = Dict2Obj(jdata) - else: - raise Exception("Unexpected response format from server") - - if not getattr(response, 'status', True): - sock.close() - raise click.Abort(getattr(response, 'msg', 'An error occurred')) - - except Exception as e: - if quiet: - sock.close() - raise click.Abort(str(e)) - click.echo("Error: {}".format(str(e))) - sock.close() - sys.exit(1) - - sock.close() - return response diff --git a/tests/config_memory_statistics_test.py b/tests/config_memory_statistics_test.py index fb33703145..f4d3f24193 100644 --- a/tests/config_memory_statistics_test.py +++ b/tests/config_memory_statistics_test.py @@ -1,7 +1,7 @@ import pytest from unittest.mock import patch from click.testing import CliRunner -from utilities_common.cli import AbbreviationGroup +import syslog from config.memory_statistics import ( memory_statistics_enable, memory_statistics_disable, @@ -22,114 +22,120 @@ def mock_db(): def test_memory_statistics_enable(mock_db): """Test enabling the Memory Statistics feature.""" - mock_db.get_table.return_value = {"memory_statistics": {"enabled": "false"}} runner = CliRunner() - with patch("click.echo") as mock_echo: - result = runner.invoke(memory_statistics_enable) - assert result.exit_code == 0 # Ensure the command exits without error - assert mock_echo.call_count == 2 # Check if the echo function was called twice - mock_db.mod_entry.assert_called_once_with( - "MEMORY_STATISTICS", "memory_statistics", - {"enabled": "true", "disabled": "false"} - ) + with patch("config.memory_statistics.update_memory_statistics_status") as mock_update_status: + mock_update_status.return_value = (True, None) # Simulate successful update + with patch("syslog.syslog") as mock_syslog: + result = runner.invoke(memory_statistics_enable) + assert result.exit_code == 0 + mock_update_status.assert_called_once_with("true", mock_db) + mock_syslog.assert_any_call(syslog.LOG_INFO, "Memory statistics enabled successfully.") def test_memory_statistics_disable(mock_db): """Test disabling the Memory Statistics feature.""" - mock_db.get_table.return_value = {"memory_statistics": {"enabled": "true"}} - runner = CliRunner() - - with patch("click.echo") as mock_echo: - result = runner.invoke(memory_statistics_disable) - assert result.exit_code == 0 - assert mock_echo.call_count == 2 - mock_db.mod_entry.assert_called_once_with( - "MEMORY_STATISTICS", "memory_statistics", - {"enabled": "false", "disabled": "true"} - ) - - -def test_memory_statistics_disable_exception(mock_db): - """Test disabling Memory Statistics feature when an exception occurs.""" - mock_db.get_table.return_value = {"memory_statistics": {"enabled": "true"}} runner = CliRunner() - # Mock `mod_entry` to raise an exception. - mock_db.mod_entry.side_effect = Exception("Simulated database error") - - with patch("click.echo") as mock_echo: - result = runner.invoke(memory_statistics_disable) - assert result.exit_code == 0 # Ensure the command exits without crashing. - - # Check that the error message was outputted. - mock_echo.assert_any_call("Error disabling Memory Statistics feature: Simulated database error", err=True) + with patch("config.memory_statistics.update_memory_statistics_status") as mock_update_status: + mock_update_status.return_value = (True, None) # Simulate successful update + with patch("syslog.syslog") as mock_syslog: + result = runner.invoke(memory_statistics_disable) + assert result.exit_code == 0 + mock_update_status.assert_called_once_with("false", mock_db) + mock_syslog.assert_any_call(syslog.LOG_INFO, "Memory statistics disabled successfully.") def test_memory_statistics_retention_period(mock_db): """Test setting the retention period for Memory Statistics.""" - mock_db.get_table.return_value = {"memory_statistics": {}} runner = CliRunner() - retention_period_value = 30 + retention_period_value = 20 # Within valid range - with patch("click.echo") as mock_echo: + with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: result = runner.invoke(memory_statistics_retention_period, [str(retention_period_value)]) assert result.exit_code == 0 - assert mock_echo.call_count == 2 + mock_echo.assert_any_call(f"Retention period set to {retention_period_value} successfully.") mock_db.mod_entry.assert_called_once_with( "MEMORY_STATISTICS", "memory_statistics", {"retention_period": retention_period_value} ) + mock_syslog.assert_any_call(syslog.LOG_INFO, f"Retention period set to {retention_period_value} successfully.") -def test_memory_statistics_retention_period_exception(mock_db): - """Test setting retention period for Memory Statistics when an exception occurs.""" - mock_db.get_table.return_value = {"memory_statistics": {}} +def test_memory_statistics_retention_period_invalid(mock_db): + """Test setting an invalid retention period for Memory Statistics.""" runner = CliRunner() - retention_period_value = 30 - - # Mock `mod_entry` to raise an exception. - mock_db.mod_entry.side_effect = Exception("Simulated retention period error") - - with patch("click.echo") as mock_echo: - result = runner.invoke(memory_statistics_retention_period, [str(retention_period_value)]) - assert result.exit_code == 0 # Ensure the command exits without crashing. + invalid_value = 50 # Out of valid range - # Check that the error message was outputted. - mock_echo.assert_any_call("Error setting retention period: Simulated retention period error", err=True) + with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: + result = runner.invoke(memory_statistics_retention_period, [str(invalid_value)]) + assert result.exit_code == 0 + mock_echo.assert_any_call("Error: Retention period must be between 1 and 30.", err=True) + mock_syslog.assert_any_call(syslog.LOG_ERR, "Error: Retention period must be between 1 and 30.") def test_memory_statistics_sampling_interval(mock_db): """Test setting the sampling interval for Memory Statistics.""" - mock_db.get_table.return_value = {"memory_statistics": {}} runner = CliRunner() - sampling_interval_value = 10 + sampling_interval_value = 10 # Within valid range - with patch("click.echo") as mock_echo: + with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: result = runner.invoke(memory_statistics_sampling_interval, [str(sampling_interval_value)]) assert result.exit_code == 0 - assert mock_echo.call_count == 2 + mock_echo.assert_any_call(f"Sampling interval set to {sampling_interval_value} successfully.") mock_db.mod_entry.assert_called_once_with( "MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval_value} ) + mock_syslog.assert_any_call( + syslog.LOG_INFO, + f"Sampling interval set to {sampling_interval_value} successfully." + ) + + +def test_memory_statistics_sampling_interval_invalid(mock_db): + """Test setting an invalid sampling interval for Memory Statistics.""" + runner = CliRunner() + invalid_value = 20 # Out of valid range + + with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: + result = runner.invoke(memory_statistics_sampling_interval, [str(invalid_value)]) + assert result.exit_code == 0 + mock_echo.assert_any_call("Error: Sampling interval must be between 3 and 15.", err=True) + mock_syslog.assert_any_call(syslog.LOG_ERR, "Error: Sampling interval must be between 3 and 15.") + + +def test_memory_statistics_retention_period_exception(mock_db): + """Test setting retention period for Memory Statistics when an exception occurs.""" + runner = CliRunner() + retention_period_value = 30 + + # Mock `mod_entry` to raise an exception + mock_db.mod_entry.side_effect = Exception("Simulated retention period error") + + with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: + result = runner.invoke(memory_statistics_retention_period, [str(retention_period_value)]) + assert result.exit_code == 0 + mock_echo.assert_any_call("Error setting retention period: Simulated retention period error", err=True) + mock_syslog.assert_any_call(syslog.LOG_ERR, "Error setting retention period: Simulated retention period error") def test_memory_statistics_sampling_interval_exception(mock_db): """Test setting sampling interval for Memory Statistics when an exception occurs.""" - mock_db.get_table.return_value = {"memory_statistics": {}} runner = CliRunner() sampling_interval_value = 10 - # Mock `mod_entry` to raise an exception. + # Mock `mod_entry` to raise an exception mock_db.mod_entry.side_effect = Exception("Simulated sampling interval error") - with patch("click.echo") as mock_echo: + with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: result = runner.invoke(memory_statistics_sampling_interval, [str(sampling_interval_value)]) - assert result.exit_code == 0 # Ensure the command exits without crashing. - - # Check that the error message was outputted. + assert result.exit_code == 0 mock_echo.assert_any_call("Error setting sampling interval: Simulated sampling interval error", err=True) + mock_syslog.assert_any_call( + syslog.LOG_ERR, + "Error setting sampling interval: Simulated sampling interval error" + ) def test_check_memory_statistics_table_existence(): @@ -144,45 +150,3 @@ def test_get_memory_statistics_table(mock_db): result = get_memory_statistics_table(mock_db) assert result == {"memory_statistics": {}} - - -def test_abbreviation_group_get_command_existing_command(): - """Test AbbreviationGroup's get_command method with an existing command.""" - # Create an instance of AbbreviationGroup with a sample command. - group = AbbreviationGroup() - - # Invoke get_command with the name of the existing command. - command = group.get_command(ctx=None, cmd_name="existing_command") - - # Check that the correct command is returned. - assert command is None - - -def test_check_memory_statistics_table_existence_missing_key(): - """Test check_memory_statistics_table_existence when 'memory_statistics' key is missing.""" - with patch("click.echo") as mock_echo: - result = check_memory_statistics_table_existence({"another_key": {}}) - - # Ensure the function returns False when 'memory_statistics' key is missing. - assert result is False - - # Check that the specific error message was outputted. - mock_echo.assert_called_once_with( - "Unable to retrieve key 'memory_statistics' from MEMORY_STATISTICS table.", err=True - ) - - -def test_memory_statistics_enable_exception(mock_db): - """Test enabling Memory Statistics feature when an exception occurs.""" - mock_db.get_table.return_value = {"memory_statistics": {"enabled": "false"}} - runner = CliRunner() - - # Mock `mod_entry` to raise an exception. - mock_db.mod_entry.side_effect = Exception("Simulated database error") - - with patch("click.echo") as mock_echo: - result = runner.invoke(memory_statistics_enable) - assert result.exit_code == 0 # Ensure the command exits without crashing. - - # Check that the error message was outputted. - mock_echo.assert_any_call("Error enabling Memory Statistics feature: Simulated database error", err=True) diff --git a/tests/show_memory_statistics_test.py b/tests/show_memory_statistics_test.py deleted file mode 100644 index 2ff10d10c9..0000000000 --- a/tests/show_memory_statistics_test.py +++ /dev/null @@ -1,70 +0,0 @@ -# import pytest -from click.testing import CliRunner -# from unittest.mock import MagicMock - -from show.memory_statistics import memory_statistics - - -class MockConfigDBConnector: - def __init__(self, data): - self.data = data - - def get_table(self, table_name): - return self.data.get(table_name, {}) - - -def test_show_memory_statistics_logs(): - # Prepare mock data with some example log entries - mock_data = { - "MEMORY_STATISTICS": { - "memory_statistics": { - "log1": {"time": "2024-11-04 10:00:00", "statistic": "Usage", "value": "512 MB"}, - "log2": {"time": "2024-11-04 10:05:00", "statistic": "Usage", "value": "514 MB"}, - } - } - } - - # Create a mock database connector with the prepared data - mock_db = MockConfigDBConnector(data=mock_data) - - # Use CliRunner to invoke the CLI command with the mock database - runner = CliRunner() - result = runner.invoke(memory_statistics, ["logs", "2024-11-04 09:00:00", "2024-11-04 11:00:00"], - obj={"db_connector": mock_db}) - - # Print result output and exception details for debugging - if result.exit_code != 0: - print("Command failed with output:", result.output) - print("Exception:", result.exception) - - # Assertions to verify the output matches the mock data - assert result.exit_code == 0 - assert "2024-11-04 10:00:00" in result.output - assert "Usage" in result.output - assert "512 MB" in result.output - - -def test_show_memory_statistics_config(): - # Prepare mock data for configuration - mock_data = { - "MEMORY_STATISTICS": { - "memory_statistics": { - "enabled": "true", - "retention_period": "30", - "sampling_interval": "10", - } - } - } - - # Create a mock database connector with the prepared data - mock_db = MockConfigDBConnector(data=mock_data) - - # Use CliRunner to invoke the CLI command with the mock database - runner = CliRunner() - result = runner.invoke(memory_statistics, ["memory_statistics"], obj={"db_connector": mock_db}) - - # Assertions to verify the output matches the mock data - assert result.exit_code == 0 - assert "Memory Statistics administrative mode: Enabled" in result.output - assert "Memory Statistics retention time (days): 30" in result.output - assert "Memory Statistics sampling interval (minutes): 10" in result.output From 5c6b013637ed6f73786b27ed373e90c2d5bc568c Mon Sep 17 00:00:00 2001 From: "kanza.latif" Date: Mon, 25 Nov 2024 16:16:42 +0500 Subject: [PATCH 22/30] updated config and test files --- config/memory_statistics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index a7da905789..5ff1676952 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -115,4 +115,4 @@ def get_memory_statistics_table(db): def check_memory_statistics_table_existence(table): """Check if MEMORY_STATISTICS table exists in the given table.""" - return "memory_statistics" in table \ No newline at end of file + return "memory_statistics" in table From 81be860910787de3ba6a9a69f8a107ff6ae04c26 Mon Sep 17 00:00:00 2001 From: Arham-Nasir Date: Thu, 26 Dec 2024 11:45:07 +0500 Subject: [PATCH 23/30] update CLI commands for memory statistics configuration and management. Signed-off-by: Arham-Nasir --- config/memory_statistics.py | 180 ++++++++------ tests/config_memory_statistics_test.py | 319 +++++++++++++++++-------- 2 files changed, 317 insertions(+), 182 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 5ff1676952..9b64c0c044 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -1,10 +1,17 @@ -import click import syslog + +import click from swsscommon.swsscommon import ConfigDBConnector -# Default values -DEFAULT_SAMPLING_INTERVAL = 5 -DEFAULT_RETENTION_PERIOD = 15 +# Constants +MEMORY_STATISTICS_TABLE = "MEMORY_STATISTICS" +MEMORY_STATISTICS_KEY = "memory_statistics" +SAMPLING_INTERVAL_MIN = 3 +SAMPLING_INTERVAL_MAX = 15 +RETENTION_PERIOD_MIN = 1 +RETENTION_PERIOD_MAX = 30 +DEFAULT_SAMPLING_INTERVAL = 5 # minutes +DEFAULT_RETENTION_PERIOD = 15 # days def log_to_syslog(message, level=syslog.LOG_INFO): @@ -13,106 +20,127 @@ def log_to_syslog(message, level=syslog.LOG_INFO): syslog.syslog(level, message) +def get_db_connection(): + """Create and return a database connection.""" + db = ConfigDBConnector() + db.connect() + return db + + def update_memory_statistics_status(status, db): - """Updates the status of the memory statistics feature in the config DB.""" + """ + Updates the status of the memory statistics feature in the config DB. + Returns a tuple of (success, error_message). + """ try: - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"enabled": status}) + db.mod_entry(MEMORY_STATISTICS_TABLE, MEMORY_STATISTICS_KEY, {"enabled": status}) msg = f"Memory statistics feature {'enabled' if status == 'true' else 'disabled'} successfully." click.echo(msg) - log_to_syslog(msg) # Log to syslog - return True, None # Success: return True and no error + log_to_syslog(msg) + return True, None except Exception as e: error_msg = f"Error updating memory statistics status: {e}" click.echo(error_msg, err=True) - log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog - return False, error_msg # Failure: return False and the error message + log_to_syslog(error_msg, syslog.LOG_ERR) + return False, error_msg -@click.command() -def memory_statistics_enable(): - """Enable memory statistics.""" - db = ConfigDBConnector() - db.connect() +@click.group() +def cli(): + """Memory statistics configuration tool.""" + pass + + +@cli.group() +def config(): + """Configuration commands.""" + pass + + +@config.group(name='memory-stats') +def memory_stats(): + """Configure memory statistics.""" + pass + + +@memory_stats.command(name='enable') +def memory_stats_enable(): + """Enable memory statistics collection.""" + db = get_db_connection() success, error = update_memory_statistics_status("true", db) - if not success: - click.echo(error, err=True) # Handle error if unsuccessful - else: - success_msg = "Memory statistics enabled successfully." - click.echo(success_msg) - log_to_syslog(success_msg) # Log to syslog - reminder_msg = "Reminder: Please run 'config save' to persist changes." - click.echo(reminder_msg) - log_to_syslog(reminder_msg) # Log to syslog + if success: + click.echo("Reminder: Please run 'config save' to persist changes.") + log_to_syslog("Memory statistics enabled. Reminder to run 'config save'") -@click.command() -def memory_statistics_disable(): - """Disable memory statistics.""" - db = ConfigDBConnector() - db.connect() +@memory_stats.command(name='disable') +def memory_stats_disable(): + """Disable memory statistics collection.""" + db = get_db_connection() success, error = update_memory_statistics_status("false", db) - if not success: - click.echo(error, err=True) # Handle error if unsuccessful - else: - success_msg = "Memory statistics disabled successfully." - click.echo(success_msg) - log_to_syslog(success_msg) # Log to syslog - reminder_msg = "Reminder: Please run 'config save' to persist changes." - click.echo(reminder_msg) - log_to_syslog(reminder_msg) # Log to syslog - - -@click.command() -@click.argument("retention_period", type=int, required=False, default=DEFAULT_RETENTION_PERIOD) -def memory_statistics_retention_period(retention_period): - """Set retention period for memory statistics.""" - if not (1 <= retention_period <= 30): - error_msg = "Error: Retention period must be between 1 and 30." + if success: + click.echo("Reminder: Please run 'config save' to persist changes.") + log_to_syslog("Memory statistics disabled. Reminder to run 'config save'") + + +@memory_stats.command(name='sampling-interval') +@click.argument("interval", type=int) +def memory_stats_sampling_interval(interval): + """ + Set sampling interval for memory statistics. + + The sampling interval must be between 3 and 15 minutes. + This determines how frequently memory usage data is collected. + """ + if not (SAMPLING_INTERVAL_MIN <= interval <= SAMPLING_INTERVAL_MAX): + error_msg = ( + f"Error: Sampling interval must be between {SAMPLING_INTERVAL_MIN} " + f"and {SAMPLING_INTERVAL_MAX} minutes." + ) click.echo(error_msg, err=True) - log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog + log_to_syslog(error_msg, syslog.LOG_ERR) return - db = ConfigDBConnector() - db.connect() + db = get_db_connection() try: - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"retention_period": retention_period}) - success_msg = f"Retention period set to {retention_period} successfully." + db.mod_entry(MEMORY_STATISTICS_TABLE, MEMORY_STATISTICS_KEY, {"sampling_interval": str(interval)}) + success_msg = f"Sampling interval set to {interval} minutes successfully." click.echo(success_msg) - log_to_syslog(success_msg) # Log to syslog + log_to_syslog(success_msg) + click.echo("Reminder: Please run 'config save' to persist changes.") except Exception as e: - error_msg = f"Error setting retention period: {e}" + error_msg = f"Error setting sampling interval: {e}" click.echo(error_msg, err=True) - log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog + log_to_syslog(error_msg, syslog.LOG_ERR) -@click.command() -@click.argument("sampling_interval", type=int, required=False, default=DEFAULT_SAMPLING_INTERVAL) -def memory_statistics_sampling_interval(sampling_interval): - """Set sampling interval for memory statistics.""" - if not (3 <= sampling_interval <= 15): - error_msg = "Error: Sampling interval must be between 3 and 15." +@memory_stats.command(name='retention-period') +@click.argument("period", type=int) +def memory_stats_retention_period(period): + """ + Set retention period for memory statistics. + + The retention period must be between 1 and 30 days. + This determines how long memory usage data is stored before being purged. + """ + if not (RETENTION_PERIOD_MIN <= period <= RETENTION_PERIOD_MAX): + error_msg = f"Error: Retention period must be between {RETENTION_PERIOD_MIN} and {RETENTION_PERIOD_MAX} days." click.echo(error_msg, err=True) - log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog + log_to_syslog(error_msg, syslog.LOG_ERR) return - db = ConfigDBConnector() - db.connect() + db = get_db_connection() try: - db.mod_entry("MEMORY_STATISTICS", "memory_statistics", {"sampling_interval": sampling_interval}) - success_msg = f"Sampling interval set to {sampling_interval} successfully." + db.mod_entry(MEMORY_STATISTICS_TABLE, MEMORY_STATISTICS_KEY, {"retention_period": str(period)}) + success_msg = f"Retention period set to {period} days successfully." click.echo(success_msg) - log_to_syslog(success_msg) # Log to syslog + log_to_syslog(success_msg) + click.echo("Reminder: Please run 'config save' to persist changes.") except Exception as e: - error_msg = f"Error setting sampling interval: {e}" + error_msg = f"Error setting retention period: {e}" click.echo(error_msg, err=True) - log_to_syslog(error_msg, syslog.LOG_ERR) # Log error to syslog - - -def get_memory_statistics_table(db): - """Retrieve MEMORY_STATISTICS table from config DB.""" - return db.get_table("MEMORY_STATISTICS") + log_to_syslog(error_msg, syslog.LOG_ERR) -def check_memory_statistics_table_existence(table): - """Check if MEMORY_STATISTICS table exists in the given table.""" - return "memory_statistics" in table +if __name__ == "__main__": + cli() diff --git a/tests/config_memory_statistics_test.py b/tests/config_memory_statistics_test.py index f4d3f24193..43440cd934 100644 --- a/tests/config_memory_statistics_test.py +++ b/tests/config_memory_statistics_test.py @@ -1,152 +1,259 @@ +import subprocess +import syslog +from unittest.mock import Mock, patch + import pytest -from unittest.mock import patch from click.testing import CliRunner -import syslog + from config.memory_statistics import ( - memory_statistics_enable, - memory_statistics_disable, - memory_statistics_retention_period, - memory_statistics_sampling_interval, - get_memory_statistics_table, - check_memory_statistics_table_existence, + cli, + log_to_syslog, + MEMORY_STATISTICS_KEY, + MEMORY_STATISTICS_TABLE, + RETENTION_PERIOD_MAX, + RETENTION_PERIOD_MIN, + SAMPLING_INTERVAL_MAX, + SAMPLING_INTERVAL_MIN, + update_memory_statistics_status, ) @pytest.fixture def mock_db(): - """Fixture for the mock database.""" - with patch("config.memory_statistics.ConfigDBConnector") as MockConfigDBConnector: - mock_db_instance = MockConfigDBConnector.return_value + """Fixture to create a mock database connection.""" + with patch('config.memory_statistics.ConfigDBConnector') as mock_db_class: + mock_db_instance = Mock() + mock_db_class.return_value = mock_db_instance yield mock_db_instance -def test_memory_statistics_enable(mock_db): - """Test enabling the Memory Statistics feature.""" - runner = CliRunner() +@pytest.fixture +def cli_runner(): + """Fixture to create a CLI runner.""" + return CliRunner() - with patch("config.memory_statistics.update_memory_statistics_status") as mock_update_status: - mock_update_status.return_value = (True, None) # Simulate successful update - with patch("syslog.syslog") as mock_syslog: - result = runner.invoke(memory_statistics_enable) - assert result.exit_code == 0 - mock_update_status.assert_called_once_with("true", mock_db) - mock_syslog.assert_any_call(syslog.LOG_INFO, "Memory statistics enabled successfully.") +class TestUpdateMemoryStatisticsStatus: + """Direct tests for update_memory_statistics_status function""" -def test_memory_statistics_disable(mock_db): - """Test disabling the Memory Statistics feature.""" - runner = CliRunner() + def test_successful_enable(self, mock_db): + """Test successful status update to enable.""" + success, error = update_memory_statistics_status("true", mock_db) + assert success is True + assert error is None + mock_db.mod_entry.assert_called_once_with( + MEMORY_STATISTICS_TABLE, + MEMORY_STATISTICS_KEY, + {"enabled": "true"} + ) - with patch("config.memory_statistics.update_memory_statistics_status") as mock_update_status: - mock_update_status.return_value = (True, None) # Simulate successful update - with patch("syslog.syslog") as mock_syslog: - result = runner.invoke(memory_statistics_disable) - assert result.exit_code == 0 - mock_update_status.assert_called_once_with("false", mock_db) - mock_syslog.assert_any_call(syslog.LOG_INFO, "Memory statistics disabled successfully.") + def test_successful_disable(self, mock_db): + """Test successful status update to disable.""" + success, error = update_memory_statistics_status("false", mock_db) + assert success is True + assert error is None + mock_db.mod_entry.assert_called_once_with( + MEMORY_STATISTICS_TABLE, + MEMORY_STATISTICS_KEY, + {"enabled": "false"} + ) + def test_database_error(self, mock_db): + """Test handling of database errors.""" + mock_db.mod_entry.side_effect = Exception("DB Error") + success, error = update_memory_statistics_status("true", mock_db) + assert success is False + assert "Error updating memory statistics status" in error + assert "DB Error" in error -def test_memory_statistics_retention_period(mock_db): - """Test setting the retention period for Memory Statistics.""" - runner = CliRunner() - retention_period_value = 20 # Within valid range - with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: - result = runner.invoke(memory_statistics_retention_period, [str(retention_period_value)]) +class TestMemoryStatisticsEnable: + def test_enable_success(self, cli_runner, mock_db): + """Test successful enabling of memory statistics.""" + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'enable']) assert result.exit_code == 0 - mock_echo.assert_any_call(f"Retention period set to {retention_period_value} successfully.") mock_db.mod_entry.assert_called_once_with( - "MEMORY_STATISTICS", "memory_statistics", - {"retention_period": retention_period_value} + MEMORY_STATISTICS_TABLE, + MEMORY_STATISTICS_KEY, + {"enabled": "true"} ) - mock_syslog.assert_any_call(syslog.LOG_INFO, f"Retention period set to {retention_period_value} successfully.") + assert "successfully" in result.output + assert "config save" in result.output - -def test_memory_statistics_retention_period_invalid(mock_db): - """Test setting an invalid retention period for Memory Statistics.""" - runner = CliRunner() - invalid_value = 50 # Out of valid range - - with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: - result = runner.invoke(memory_statistics_retention_period, [str(invalid_value)]) + def test_enable_db_error(self, cli_runner, mock_db): + """Test handling of database error when enabling.""" + mock_db.mod_entry.side_effect = Exception("DB Error") + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'enable']) assert result.exit_code == 0 - mock_echo.assert_any_call("Error: Retention period must be between 1 and 30.", err=True) - mock_syslog.assert_any_call(syslog.LOG_ERR, "Error: Retention period must be between 1 and 30.") + assert "Error" in result.output -def test_memory_statistics_sampling_interval(mock_db): - """Test setting the sampling interval for Memory Statistics.""" - runner = CliRunner() - sampling_interval_value = 10 # Within valid range - - with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: - result = runner.invoke(memory_statistics_sampling_interval, [str(sampling_interval_value)]) +class TestMemoryStatisticsDisable: + def test_disable_success(self, cli_runner, mock_db): + """Test successful disabling of memory statistics.""" + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'disable']) assert result.exit_code == 0 - mock_echo.assert_any_call(f"Sampling interval set to {sampling_interval_value} successfully.") mock_db.mod_entry.assert_called_once_with( - "MEMORY_STATISTICS", "memory_statistics", - {"sampling_interval": sampling_interval_value} + MEMORY_STATISTICS_TABLE, + MEMORY_STATISTICS_KEY, + {"enabled": "false"} ) - mock_syslog.assert_any_call( - syslog.LOG_INFO, - f"Sampling interval set to {sampling_interval_value} successfully." + assert "successfully" in result.output + assert "config save" in result.output + + def test_disable_db_error(self, cli_runner, mock_db): + """Test handling of database error when disabling.""" + mock_db.mod_entry.side_effect = Exception("DB Error") + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'disable']) + assert result.exit_code == 0 + assert "Error" in result.output + + +class TestSamplingInterval: + @pytest.mark.parametrize("interval", [ + SAMPLING_INTERVAL_MIN, + SAMPLING_INTERVAL_MAX, + (SAMPLING_INTERVAL_MIN + SAMPLING_INTERVAL_MAX) // 2 + ]) + def test_valid_sampling_intervals(self, interval, cli_runner, mock_db): + """Test setting valid sampling intervals.""" + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', str(interval)]) + assert result.exit_code == 0 + mock_db.mod_entry.assert_called_once_with( + MEMORY_STATISTICS_TABLE, + MEMORY_STATISTICS_KEY, + {"sampling_interval": str(interval)} ) + assert f"set to {interval}" in result.output + @pytest.mark.parametrize("interval", [ + SAMPLING_INTERVAL_MIN - 1, + SAMPLING_INTERVAL_MAX + 1, + 0, + -1 + ]) + def test_invalid_sampling_intervals(self, interval, cli_runner, mock_db): + """Test handling of invalid sampling intervals.""" + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', str(interval)]) + assert "Error" in result.output + assert not mock_db.mod_entry.called -def test_memory_statistics_sampling_interval_invalid(mock_db): - """Test setting an invalid sampling interval for Memory Statistics.""" - runner = CliRunner() - invalid_value = 20 # Out of valid range + def test_sampling_interval_specific_db_error(self, cli_runner, mock_db): + """Test specific database error case when setting sampling interval.""" + mock_db.mod_entry.side_effect = Exception("Database connection lost") - with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: - result = runner.invoke(memory_statistics_sampling_interval, [str(invalid_value)]) - assert result.exit_code == 0 - mock_echo.assert_any_call("Error: Sampling interval must be between 3 and 15.", err=True) - mock_syslog.assert_any_call(syslog.LOG_ERR, "Error: Sampling interval must be between 3 and 15.") + with patch('config.memory_statistics.log_to_syslog') as mock_log: + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', '5']) + expected_error = "Error setting sampling interval: Database connection lost" + assert expected_error in result.output -def test_memory_statistics_retention_period_exception(mock_db): - """Test setting retention period for Memory Statistics when an exception occurs.""" - runner = CliRunner() - retention_period_value = 30 + mock_log.assert_called_with(expected_error, syslog.LOG_ERR) - # Mock `mod_entry` to raise an exception - mock_db.mod_entry.side_effect = Exception("Simulated retention period error") + assert result.exit_code == 0 - with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: - result = runner.invoke(memory_statistics_retention_period, [str(retention_period_value)]) + mock_db.mod_entry.assert_called_once_with( + MEMORY_STATISTICS_TABLE, + MEMORY_STATISTICS_KEY, + {"sampling_interval": "5"} + ) + + +class TestRetentionPeriod: + @pytest.mark.parametrize("period", [ + RETENTION_PERIOD_MIN, + RETENTION_PERIOD_MAX, + (RETENTION_PERIOD_MIN + RETENTION_PERIOD_MAX) // 2 + ]) + def test_valid_retention_periods(self, period, cli_runner, mock_db): + """Test setting valid retention periods.""" + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'retention-period', str(period)]) assert result.exit_code == 0 - mock_echo.assert_any_call("Error setting retention period: Simulated retention period error", err=True) - mock_syslog.assert_any_call(syslog.LOG_ERR, "Error setting retention period: Simulated retention period error") + mock_db.mod_entry.assert_called_once_with( + MEMORY_STATISTICS_TABLE, + MEMORY_STATISTICS_KEY, + {"retention_period": str(period)} + ) + assert f"set to {period}" in result.output + + @pytest.mark.parametrize("period", [ + RETENTION_PERIOD_MIN - 1, + RETENTION_PERIOD_MAX + 1, + 0, + -1 + ]) + def test_invalid_retention_periods(self, period, cli_runner, mock_db): + """Test handling of invalid retention periods.""" + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'retention-period', str(period)]) + assert "Error" in result.output + assert not mock_db.mod_entry.called + + def test_db_error(self, cli_runner, mock_db): + """Test handling of database errors.""" + mock_db.mod_entry.side_effect = Exception("DB Error") + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'retention-period', '15']) + assert "Error" in result.output + + +class TestSyslogLogging: + @pytest.mark.parametrize("log_level,expected_level", [ + ("INFO", syslog.LOG_INFO), + ("ERROR", syslog.LOG_ERR) + ]) + def test_syslog_logging(self, log_level, expected_level): + """Test syslog logging functionality.""" + with patch('syslog.syslog') as mock_syslog, \ + patch('syslog.openlog') as mock_openlog: + + log_to_syslog("Test message", expected_level) + + mock_openlog.assert_called_once_with( + "memory_statistics", + syslog.LOG_PID | syslog.LOG_CONS, + syslog.LOG_USER + ) + + mock_syslog.assert_called_once_with(expected_level, "Test message") + + def test_syslog_logging_default_level(self): + """Test syslog logging with default log level.""" + with patch('syslog.syslog') as mock_syslog, \ + patch('syslog.openlog') as _: + log_to_syslog("Test message") + mock_syslog.assert_called_once_with(syslog.LOG_INFO, "Test message") + + +def test_main_execution(): + """Test the main execution block of the script.""" + with patch('config.memory_statistics.cli') as mock_cli: + module_code = compile( + 'if __name__ == "__main__": cli()', + 'memory_statistics.py', + 'exec' + ) + namespace = {'__name__': '__main__', 'cli': mock_cli} + exec(module_code, namespace) -def test_memory_statistics_sampling_interval_exception(mock_db): - """Test setting sampling interval for Memory Statistics when an exception occurs.""" - runner = CliRunner() - sampling_interval_value = 10 + mock_cli.assert_called_once() - # Mock `mod_entry` to raise an exception - mock_db.mod_entry.side_effect = Exception("Simulated sampling interval error") - with patch("click.echo") as mock_echo, patch("syslog.syslog") as mock_syslog: - result = runner.invoke(memory_statistics_sampling_interval, [str(sampling_interval_value)]) - assert result.exit_code == 0 - mock_echo.assert_any_call("Error setting sampling interval: Simulated sampling interval error", err=True) - mock_syslog.assert_any_call( - syslog.LOG_ERR, - "Error setting sampling interval: Simulated sampling interval error" - ) +def test_main_cli_integration(): + """Test the main CLI integration with actual command.""" + runner = CliRunner() + with patch('config.memory_statistics.get_db_connection') as mock_get_db: + mock_db = Mock() + mock_get_db.return_value = mock_db -def test_check_memory_statistics_table_existence(): - """Test existence check for MEMORY_STATISTICS table.""" - assert check_memory_statistics_table_existence({"memory_statistics": {}}) is True - assert check_memory_statistics_table_existence({}) is False + result = runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', '5']) + assert result.exit_code == 0 + mock_get_db.assert_called_once() -def test_get_memory_statistics_table(mock_db): - """Test getting MEMORY_STATISTICS table.""" - mock_db.get_table.return_value = {"memory_statistics": {}} - result = get_memory_statistics_table(mock_db) - assert result == {"memory_statistics": {}} +def test_script_execution(): + """Test that the script runs successfully.""" + result = subprocess.run(["python3", + "config/memory_statistics.py"], capture_output=True) + assert result.returncode == 0 From 02848eb8f6a80dab9baeea6e4ff74f7314b36708 Mon Sep 17 00:00:00 2001 From: Arham-Nasir Date: Thu, 26 Dec 2024 12:23:49 +0500 Subject: [PATCH 24/30] Add show commands for memory statistics and configuration Signed-off-by: Arham-Nasir --- show/memory_statistics.py | 432 ++++++++++++++++ tests/show_memory_statistics_test.py | 732 +++++++++++++++++++++++++++ 2 files changed, 1164 insertions(+) create mode 100644 show/memory_statistics.py create mode 100644 tests/show_memory_statistics_test.py diff --git a/show/memory_statistics.py b/show/memory_statistics.py new file mode 100644 index 0000000000..aff83f5a58 --- /dev/null +++ b/show/memory_statistics.py @@ -0,0 +1,432 @@ +# Standard library imports +import json +import os +import signal +import socket +import sys +import syslog +import time +from typing import Any, Dict, Union + +# Third-party library imports +import click +from dataclasses import dataclass + +# Local application/library imports +from swsscommon.swsscommon import ConfigDBConnector + + +@dataclass +class Config: + SOCKET_PATH: str = '/var/run/dbus/memstats.socket' + SOCKET_TIMEOUT: int = 30 + BUFFER_SIZE: int = 8192 + MAX_RETRIES: int = 3 + RETRY_DELAY: float = 1.0 + + DEFAULT_CONFIG = { + "enabled": "false", + "sampling_interval": "5", + "retention_period": "15" + } + + +class ConnectionError(Exception): + """Custom exception for connection-related errors.""" + pass + + +class Dict2Obj: + """Converts dictionaries or lists into objects with attribute-style access.""" + def __init__(self, d: Union[Dict[str, Any], list]) -> None: + if not isinstance(d, (dict, list)): + raise ValueError("Input should be a dictionary or a list") + + if isinstance(d, dict): + for key, value in d.items(): + if isinstance(value, (list, tuple)): + setattr( + self, + key, + [Dict2Obj(x) if isinstance(x, dict) else x for x in value], + ) + else: + setattr( + self, key, Dict2Obj(value) if isinstance(value, dict) else value + ) + elif isinstance(d, list): + self.items = [Dict2Obj(x) if isinstance(x, dict) else x for x in d] + + def to_dict(self) -> Dict[str, Any]: + """Converts the object back to a dictionary format.""" + result = {} + if hasattr(self, "items"): + return [x.to_dict() if isinstance(x, Dict2Obj) else x for x in self.items] + + for key in self.__dict__: + value = getattr(self, key) + if isinstance(value, Dict2Obj): + result[key] = value.to_dict() + elif isinstance(value, list): + result[key] = [v.to_dict() if isinstance(v, Dict2Obj) else v for v in value] + else: + result[key] = value + return result + + def __repr__(self) -> str: + """Provides a string representation of the object for debugging.""" + return f"<{self.__class__.__name__} {self.to_dict()}>" + + +class SonicDBConnector: + """Handles interactions with SONiC's configuration database with improved connection handling.""" + def __init__(self) -> None: + """Initialize the database connector with retry mechanism.""" + self.config_db = ConfigDBConnector() + self.connect_with_retry() + + def connect_with_retry(self, max_retries: int = 3, retry_delay: float = 1.0) -> None: + """ + Attempts to connect to the database with a retry mechanism. + """ + retries = 0 + last_error = None + + while retries < max_retries: + try: + self.config_db.connect() + syslog.syslog(syslog.LOG_INFO, "Successfully connected to SONiC config database") + return + except Exception as e: + last_error = e + retries += 1 + if retries < max_retries: + syslog.syslog(syslog.LOG_WARNING, + f"Failed to connect to database" + f"(attempt {retries}/{max_retries}): {str(e)}") + time.sleep(retry_delay) + + error_msg = ( + f"Failed to connect to SONiC config database after {max_retries} attempts. " + f"Last error: {str(last_error)}" + ) + syslog.syslog(syslog.LOG_ERR, error_msg) + raise ConnectionError(error_msg) + + def get_memory_statistics_config(self) -> Dict[str, str]: + """ + Retrieves memory statistics configuration with error handling. + """ + try: + config = self.config_db.get_table('MEMORY_STATISTICS') + if not isinstance(config, dict) or 'memory_statistics' not in config: + return Config.DEFAULT_CONFIG.copy() + + current_config = config.get('memory_statistics', {}) + if not isinstance(current_config, dict): + return Config.DEFAULT_CONFIG.copy() + + result_config = Config.DEFAULT_CONFIG.copy() + for key, value in current_config.items(): + if value is not None and value != "": + result_config[key] = value + + return result_config + + except Exception as e: + error_msg = f"Error retrieving memory statistics configuration: {str(e)}" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise RuntimeError(error_msg) + + +class SocketManager: + """Manages Unix domain socket connections with improved reliability.""" + def __init__(self, socket_path: str = Config.SOCKET_PATH): + self.socket_path = socket_path + self.sock = None + self._validate_socket_path() + + def _validate_socket_path(self) -> None: + """Validates the socket path exists or can be created.""" + socket_dir = os.path.dirname(self.socket_path) + if not os.path.exists(socket_dir): + error_msg = f"Socket directory {socket_dir} does not exist" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise ConnectionError(error_msg) + + def connect(self) -> None: + """Establishes socket connection with improved error handling.""" + retries = 0 + last_error = None + + while retries < Config.MAX_RETRIES: + try: + if self.sock: + self.close() + + self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + self.sock.settimeout(Config.SOCKET_TIMEOUT) + self.sock.connect(self.socket_path) + syslog.syslog(syslog.LOG_INFO, "Successfully connected to memory statistics service") + return + except socket.error as e: + last_error = e + retries += 1 + if retries < Config.MAX_RETRIES: + syslog.syslog(syslog.LOG_WARNING, + f"Failed to connect to socket (attempt {retries}/{Config.MAX_RETRIES}): {str(e)}") + time.sleep(Config.RETRY_DELAY) + self.close() + + error_msg = ( + f"Failed to connect to memory statistics service after {Config.MAX_RETRIES} " + f"attempts. Last error: {str(last_error)}. " + f"Please verify that the service is running and socket file exists at {self.socket_path}" + ) + syslog.syslog(syslog.LOG_ERR, error_msg) + raise ConnectionError(error_msg) + + def receive_all(self) -> str: + """Receives all data with improved error handling.""" + if not self.sock: + raise ConnectionError("No active socket connection") + + chunks = [] + while True: + try: + chunk = self.sock.recv(Config.BUFFER_SIZE) + if not chunk: + break + chunks.append(chunk) + except socket.timeout: + error_msg = f"Socket operation timed out after {Config.SOCKET_TIMEOUT} seconds" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise ConnectionError(error_msg) + except socket.error as e: + error_msg = f"Socket error during receive: {str(e)}" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise ConnectionError(error_msg) + + return b''.join(chunks).decode('utf-8') + + def send(self, data: str) -> None: + """Sends data with improved error handling.""" + if not self.sock: + raise ConnectionError("No active socket connection") + + try: + self.sock.sendall(data.encode('utf-8')) + except socket.error as e: + error_msg = f"Failed to send data: {str(e)}" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise ConnectionError(error_msg) + + def close(self) -> None: + """Closes the socket connection safely.""" + if self.sock: + try: + self.sock.close() + except Exception as e: + syslog.syslog(syslog.LOG_WARNING, f"Error closing socket: {str(e)}") + finally: + self.sock = None + + +def send_data(command: str, data: Dict[str, Any], quiet: bool = False) -> Dict2Obj: + """ + Sends a command and data to the memory statistics service. + + Time format for statistics retrieval are given below. + + - Relative time formats: + - 'X days ago', 'X hours ago', 'X minutes ago' + - 'yesterday', 'today' + - Specific times and dates: + - 'now' + - 'July 23', 'July 23, 2024', '2 November 2024' + - '7/24', '1/2' + - Time expressions: + - '2 am', '3:15 pm' + - 'Aug 01 06:43:40', 'July 1 3:00:00' + - Named months: + - 'jan', 'feb', 'march', 'september', etc. + - Full month names: 'January', 'February', 'March', etc. + - ISO 8601 format (e.g., '2024-07-01T15:00:00') + """ + socket_manager = SocketManager() + + try: + socket_manager.connect() + request = {"command": command, "data": data} + socket_manager.sock.sendall(json.dumps(request).encode('utf-8')) + + response = socket_manager.receive_all() + if not response: + raise ConnectionError("No response received from memory statistics service") + + try: + jdata = json.loads(response) + except json.JSONDecodeError as e: + error_msg = f"Failed to parse server response: {str(e)}" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise ValueError(error_msg) + + if not isinstance(jdata, dict): + raise ValueError("Invalid response format from server") + + response_obj = Dict2Obj(jdata) + if not getattr(response_obj, 'status', True): + error_msg = getattr(response_obj, 'msg', 'Unknown error occurred') + raise RuntimeError(error_msg) + + return response_obj + + except Exception as e: + if not quiet: + click.echo(f"Error: {str(e)}", err=True) + raise + finally: + socket_manager.close() + + +def format_field_value(field_name: str, value: str) -> str: + """Formats configuration field values for display.""" + if field_name == "enabled": + return "True" if value.lower() == "true" else "False" + return value if value != "Unknown" else "Not configured" + + +def display_config(db_connector: SonicDBConnector) -> None: + """Displays memory statistics configuration.""" + try: + config = db_connector.get_memory_statistics_config() + enabled = format_field_value("enabled", config.get("enabled", "Unknown")) + retention = format_field_value("retention_period", config.get("retention_period", "Unknown")) + sampling = format_field_value("sampling_interval", config.get("sampling_interval", "Unknown")) + + click.echo(f"{'Configuration Field':<30}{'Value'}") + click.echo("-" * 50) + click.echo(f"{'Enabled':<30}{enabled}") + click.echo(f"{'Retention Time (days)':<30}{retention}") + click.echo(f"{'Sampling Interval (minutes)':<30}{sampling}") + except Exception as e: + error_msg = f"Failed to retrieve configuration: {str(e)}" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise click.ClickException(error_msg) + + +@click.group() +def cli(): + """Main entry point for the SONiC Memory Statistics CLI.""" + pass + + +@cli.group() +def show(): + """Show commands for memory statistics.""" + pass + + +@show.command(name="memory-stats") +@click.option( + '--from', 'from_time', + help='Start time for memory statistics (e.g., "15 hours ago", "7 days ago", "ISO Format")' +) +@click.option( + '--to', 'to_time', + help='End time for memory statistics (e.g., "now", "ISO Format")' +) +@click.option( + '--select', 'select_metric', + help='Show statistics for specific metric (e.g., total_memory, used_memory)' +) +def show_statistics(from_time: str, to_time: str, select_metric: str): + """Display memory statistics.""" + try: + request_data = { + "type": "system", + "metric_name": select_metric, + "from": from_time, + "to": to_time + } + + response = send_data("memory_statistics_command_request_handler", request_data) + stats_data = response.to_dict() + + if isinstance(stats_data, dict): + memory_stats = stats_data.get("data", "") + if memory_stats: + cleaned_output = memory_stats.replace("\n", "\n").strip() + click.echo(f"Memory Statistics:\n{cleaned_output}") + else: + click.echo("No memory statistics data available.") + else: + click.echo("Error: Invalid data format received") + + except Exception as e: + click.echo(f"Error: {str(e)}", err=True) + sys.exit(1) + + +@show.command(name="memory-stats-config") +def show_configuration(): + """Display memory statistics configuration.""" + try: + db_connector = SonicDBConnector() + display_config(db_connector) + except Exception as e: + click.echo(f"Error: {str(e)}", err=True) + sys.exit(1) + + +def cleanup_resources(): + """Performs cleanup of resources before shutdown.""" + try: + if hasattr(cleanup_resources, 'db_connector'): + del cleanup_resources.db_connector + + if hasattr(cleanup_resources, 'socket_manager'): + cleanup_resources.socket_manager.close() + + syslog.syslog(syslog.LOG_INFO, "Successfully cleaned up resources during shutdown") + except Exception as e: + syslog.syslog(syslog.LOG_ERR, f"Error during cleanup: {str(e)}") + + +def shutdown_handler(signum: int, frame) -> None: + """ + Signal handler for graceful shutdown. + Handles SIGTERM signal with proper resource cleanup. + + Args: + signum: Signal number + frame: Current stack frame + """ + try: + syslog.syslog(syslog.LOG_INFO, "Received SIGTERM signal, initiating graceful shutdown...") + cleanup_resources() + click.echo("\nShutting down gracefully...") + sys.exit(0) + except Exception as e: + syslog.syslog(syslog.LOG_ERR, f"Error during shutdown: {str(e)}") + sys.exit(1) + + +def main(): + """Main entry point with enhanced error handling and shutdown management.""" + try: + signal.signal(signal.SIGTERM, shutdown_handler) + + cleanup_resources.db_connector = None + cleanup_resources.socket_manager = None + + cli() + except Exception as e: + syslog.syslog(syslog.LOG_ERR, f"Fatal error in main: {str(e)}") + click.echo(f"Error: {str(e)}", err=True) + cleanup_resources() + sys.exit(1) + + +if __name__ == '__main__': + main() diff --git a/tests/show_memory_statistics_test.py b/tests/show_memory_statistics_test.py new file mode 100644 index 0000000000..621854556d --- /dev/null +++ b/tests/show_memory_statistics_test.py @@ -0,0 +1,732 @@ +import json +import os +import signal +import socket +import syslog +import unittest +from unittest.mock import MagicMock, Mock, patch + +import click +from click.testing import CliRunner +import pytest + +from show.memory_statistics import ( + Config, + ConnectionError, + Dict2Obj, + SonicDBConnector, + SocketManager, + cleanup_resources, + display_config, + format_field_value, + main, + send_data, + show_configuration, + show_statistics, + shutdown_handler, +) + + +class TestConfig(unittest.TestCase): + """Tests for Config class""" + + def test_default_config_values(self): + """Test that Config class has correct default values""" + self.assertEqual(Config.SOCKET_PATH, '/var/run/dbus/memstats.socket') + self.assertEqual(Config.SOCKET_TIMEOUT, 30) + self.assertEqual(Config.BUFFER_SIZE, 8192) + self.assertEqual(Config.MAX_RETRIES, 3) + self.assertEqual(Config.RETRY_DELAY, 1.0) + + def test_default_config_dictionary(self): + """Test the DEFAULT_CONFIG dictionary has correct values""" + expected = { + "enabled": "false", + "sampling_interval": "5", + "retention_period": "15" + } + self.assertEqual(Config.DEFAULT_CONFIG, expected) + + +class TestDict2Obj(unittest.TestCase): + """Tests for Dict2Obj class""" + + def test_dict_conversion(self): + """Test basic dictionary conversion""" + test_dict = {"name": "test", "value": 123} + obj = Dict2Obj(test_dict) + self.assertEqual(obj.name, "test") + self.assertEqual(obj.value, 123) + + def test_nested_dict_conversion(self): + """Test nested dictionary conversion""" + test_dict = { + "outer": { + "inner": "value", + "number": 42 + } + } + obj = Dict2Obj(test_dict) + self.assertEqual(obj.outer.inner, "value") + self.assertEqual(obj.outer.number, 42) + + def test_list_conversion(self): + """Test list conversion""" + test_list = [{"name": "item1"}, {"name": "item2"}] + obj = Dict2Obj(test_list) + self.assertEqual(obj.items[0].name, "item1") + self.assertEqual(obj.items[1].name, "item2") + + def test_invalid_input(self): + """Test invalid input handling""" + with self.assertRaises(ValueError): + Dict2Obj("invalid") + + def test_to_dict_conversion(self): + """Test conversion back to dictionary""" + original = {"name": "test", "nested": {"value": 123}} + obj = Dict2Obj(original) + result = obj.to_dict() + self.assertEqual(result, original) + + def test_nested_list_conversion(self): + """Test conversion of nested lists with dictionaries""" + test_dict = { + "items": [ + {"id": 1, "subitems": [{"name": "sub1"}, {"name": "sub2"}]}, + {"id": 2, "subitems": [{"name": "sub3"}, {"name": "sub4"}]} + ] + } + obj = Dict2Obj(test_dict) + self.assertEqual(obj.items[0].subitems[0].name, "sub1") + self.assertEqual(obj.items[1].subitems[1].name, "sub4") + + def test_empty_structures(self): + """Test conversion of empty structures""" + self.assertEqual(Dict2Obj({}).to_dict(), {}) + self.assertEqual(Dict2Obj([]).to_dict(), []) + + def test_complex_nested_structure(self): + """Test conversion of complex nested structures""" + test_dict = { + "level1": { + "level2": { + "level3": { + "value": 42, + "list": [1, 2, {"nested": "value"}] + } + } + } + } + obj = Dict2Obj(test_dict) + self.assertEqual(obj.level1.level2.level3.value, 42) + self.assertEqual(obj.level1.level2.level3.list[2].nested, "value") + + +class TestSonicDBConnector(unittest.TestCase): + """Tests for SonicDBConnector class""" + + @patch('show.memory_statistics.ConfigDBConnector') + def setUp(self, mock_config_db): + self.mock_config_db = mock_config_db + self.connector = SonicDBConnector() + self.mock_config_db.reset_mock() + + def test_successful_connection(self): + """Test successful database connection""" + self.mock_config_db.return_value.connect.return_value = None + self.connector.connect_with_retry() + self.mock_config_db.return_value.connect.assert_called_once() + + @patch('time.sleep') + def test_connection_retry(self, mock_sleep): + """Test connection retry mechanism""" + self.mock_config_db.return_value.connect.side_effect = [ + Exception("Connection failed"), + None + ] + self.connector.connect_with_retry(max_retries=2, retry_delay=0.1) + self.assertEqual(mock_sleep.call_count, 1) + self.assertEqual(self.mock_config_db.return_value.connect.call_count, 2) + + def test_connection_failure(self): + """Test connection failure after max retries""" + self.mock_config_db.return_value.connect.side_effect = Exception("Connection failed") + with self.assertRaises(ConnectionError): + self.connector.connect_with_retry(max_retries=1) + + def test_get_memory_statistics_config_success(self): + """Test successful config retrieval""" + expected_config = { + "memory_statistics": { + "enabled": "true", + "sampling_interval": "10", + "retention_period": "30" + } + } + self.mock_config_db.return_value.get_table.return_value = expected_config + result = self.connector.get_memory_statistics_config() + self.assertEqual(result["enabled"], "true") + self.assertEqual(result["sampling_interval"], "10") + self.assertEqual(result["retention_period"], "30") + + def test_get_memory_statistics_config_default(self): + """Test default config when table is empty""" + self.mock_config_db.return_value.get_table.return_value = {} + result = self.connector.get_memory_statistics_config() + self.assertEqual(result, Config.DEFAULT_CONFIG) + + def test_invalid_config_format(self): + """Test handling of invalid configuration format""" + self.mock_config_db.return_value.get_table.return_value = { + "memory_statistics": "invalid_string_instead_of_dict" + } + result = self.connector.get_memory_statistics_config() + self.assertEqual(result, Config.DEFAULT_CONFIG) + + def test_partial_config(self): + """Test handling of partial configuration""" + self.mock_config_db.return_value.get_table.return_value = { + "memory_statistics": { + "enabled": "true" + # missing other fields + } + } + result = self.connector.get_memory_statistics_config() + self.assertEqual(result["enabled"], "true") + self.assertEqual(result["sampling_interval"], "5") # default value + self.assertEqual(result["retention_period"], "15") # default value + + +class TestSocketManager(unittest.TestCase): + """Tests for SocketManager class""" + + def setUp(self): + self.test_socket_path = "/tmp/test_socket" + os.makedirs(os.path.dirname(self.test_socket_path), exist_ok=True) + self.socket_manager = SocketManager(self.test_socket_path) + + @patch('socket.socket') + def test_successful_connection(self, mock_socket): + """Test successful socket connection""" + mock_sock = Mock() + mock_socket.return_value = mock_sock + self.socket_manager.connect() + mock_sock.connect.assert_called_once_with(self.test_socket_path) + + @patch('socket.socket') + @patch('time.sleep') + def test_connection_retry(self, mock_sleep, mock_socket): + """Test socket connection retry mechanism""" + mock_sock = Mock() + mock_sock.connect.side_effect = [socket.error, None] + mock_socket.return_value = mock_sock + self.socket_manager.connect() + self.assertEqual(mock_sock.connect.call_count, 2) + + @patch('socket.socket') + def test_receive_all(self, mock_socket): + """Test receiving data from socket""" + mock_sock = Mock() + mock_sock.recv.side_effect = [b'test', b''] + mock_socket.return_value = mock_sock + self.socket_manager.sock = mock_sock + result = self.socket_manager.receive_all() + self.assertEqual(result, 'test') + + @patch('socket.socket') + def test_send_data(self, mock_socket): + """Test sending data through socket""" + mock_sock = Mock() + mock_socket.return_value = mock_sock + self.socket_manager.sock = mock_sock + self.socket_manager.send("test_data") + mock_sock.sendall.assert_called_once_with(b'test_data') + + def test_close_connection(self): + """Test closing socket connection""" + mock_sock = Mock() + self.socket_manager.sock = mock_sock + self.socket_manager.close() + mock_sock.close.assert_called_once() + self.assertIsNone(self.socket_manager.sock) + + def test_invalid_socket_path(self): + """Test socket creation with invalid path""" + with self.assertRaises(ConnectionError): + SocketManager("/nonexistent/path/socket") + + @patch('socket.socket') + def test_connection_max_retries_exceeded(self, mock_socket): + """Test connection failure after max retries""" + mock_sock = Mock() + mock_sock.connect.side_effect = socket.error("Connection failed") + mock_socket.return_value = mock_sock + + with self.assertRaises(ConnectionError) as ctx: + self.socket_manager.connect() + self.assertIn("Failed to connect to memory statistics service", str(ctx.exception)) + + @patch('socket.socket') + def test_receive_timeout(self, mock_socket): + """Test socket timeout during receive""" + mock_sock = Mock() + mock_sock.recv.side_effect = socket.timeout + self.socket_manager.sock = mock_sock + + with self.assertRaises(ConnectionError) as context: + self.socket_manager.receive_all() + self.assertIn("timed out", str(context.exception)) + + @patch('socket.socket') + def test_receive_with_socket_error(self, mock_socket): + """Test receive with socket error""" + mock_sock = Mock() + mock_sock.recv.side_effect = socket.error("Receive error") + self.socket_manager.sock = mock_sock + + with self.assertRaises(ConnectionError) as ctx: + self.socket_manager.receive_all() + self.assertIn("Socket error during receive", str(ctx.exception)) + + @patch('socket.socket') + def test_send_without_connection(self, mock_socket): + """Test sending data without an active connection""" + self.socket_manager.sock = None + with self.assertRaises(ConnectionError) as context: + self.socket_manager.send("test") + self.assertIn("No active socket connection", str(context.exception)) + + @patch('socket.socket') + def test_multiple_chunk_receive(self, mock_socket): + """Test receiving multiple chunks of data""" + mock_sock = Mock() + mock_sock.recv.side_effect = [b'chunk1', b'chunk2', b'chunk3', b''] + self.socket_manager.sock = mock_sock + result = self.socket_manager.receive_all() + self.assertEqual(result, 'chunk1chunk2chunk3') + + +class TestCLICommands(unittest.TestCase): + """Tests for CLI commands""" + + def setUp(self): + self.runner = CliRunner() + + @patch('show.memory_statistics.send_data') + def test_show_statistics(self, mock_send_data): + """Test show statistics command""" + mock_response = Dict2Obj({ + "status": True, + "data": "Memory Statistics Data" + }) + mock_send_data.return_value = mock_response + + result = self.runner.invoke(show_statistics, ['--from', '1h', '--to', 'now']) + self.assertEqual(result.exit_code, 0) + self.assertIn("Memory Statistics", result.output) + + @patch('show.memory_statistics.send_data') + def test_show_statistics_with_metric(self, mock_send_data): + """Test show statistics with specific metric""" + mock_response = Dict2Obj({ + "status": True, + "data": "Memory Usage: 75%" + }) + mock_send_data.return_value = mock_response + + result = self.runner.invoke(show_statistics, + ['--select', 'used_memory']) + self.assertEqual(result.exit_code, 0) + self.assertIn("Memory Usage", result.output) + + @patch('show.memory_statistics.send_data') + def test_show_statistics_error_handling(self, mock_send_data): + """Test error handling in show statistics""" + mock_send_data.side_effect = ConnectionError("Failed to connect") + + result = self.runner.invoke(show_statistics) + self.assertEqual(result.exit_code, 1) + self.assertIn("Error", result.output) + + @patch('show.memory_statistics.send_data') + def test_show_statistics_empty_data(self, mock_send): + """Test show_statistics with empty data""" + mock_send.return_value = Dict2Obj({"data": ""}) + result = self.runner.invoke(show_statistics) + self.assertIn("No memory statistics data available", result.output) + + +class TestShowConfiguration(unittest.TestCase): + """Tests for show_configuration command""" + + def setUp(self): + self.runner = CliRunner() + + @patch('show.memory_statistics.SonicDBConnector') + def test_show_config_error(self, mock_db): + """Test show_configuration error handling""" + mock_db.side_effect = Exception("DB Connection Error") + result = self.runner.invoke(show_configuration) + self.assertEqual(result.exit_code, 1) + self.assertIn("Error", result.output) + + +class TestErrorHandling(unittest.TestCase): + """Tests for error handling""" + + def test_cleanup_resources(self): + """Test resource cleanup""" + mock_db = Mock() + mock_socket = Mock() + cleanup_resources.db_connector = mock_db + cleanup_resources.socket_manager = mock_socket + cleanup_resources() + self.assertFalse(hasattr(cleanup_resources, 'db_connector')) + mock_socket.close.assert_called_once() + + @patch('sys.exit') + def test_shutdown_handler(self, mock_exit): + """Test shutdown handler""" + shutdown_handler(None, None) + mock_exit.assert_called_once_with(0) + + @patch('syslog.syslog') + def test_cleanup_with_exceptions(self, mock_syslog): + """Test cleanup with exceptions""" + mock_socket = Mock() + mock_socket.close.side_effect = Exception("Cleanup failed") + cleanup_resources.socket_manager = mock_socket + + cleanup_resources() + mock_syslog.assert_any_call(syslog.LOG_ERR, "Error during cleanup: Cleanup failed") + + @patch('syslog.syslog') + def test_cleanup_with_missing_attributes(self, mock_syslog): + """Test cleanup when attributes don't exist""" + # Ensure attributes don't exist + if hasattr(cleanup_resources, 'db_connector'): + delattr(cleanup_resources, 'db_connector') + if hasattr(cleanup_resources, 'socket_manager'): + delattr(cleanup_resources, 'socket_manager') + + cleanup_resources() + mock_syslog.assert_any_call(syslog.LOG_INFO, "Successfully cleaned up resources during shutdown") + + @patch('sys.exit') + @patch('syslog.syslog') + def test_shutdown_handler_cleanup_error(self, mock_syslog, mock_exit): + """Test shutdown handler with cleanup error""" + @patch('show.memory_statistics.cleanup_resources', side_effect=Exception("Cleanup Error")) + def test(mock_cleanup): + shutdown_handler(signal.SIGTERM, None) + mock_syslog.assert_any_call(syslog.LOG_ERR, "Error during shutdown: Cleanup Error") + mock_exit.assert_called_once_with(1) + test() + + +class TestHelperFunctions(unittest.TestCase): + """Tests for helper functions""" + + def test_format_field_value(self): + """Test field value formatting""" + self.assertEqual(format_field_value("enabled", "true"), "True") + self.assertEqual(format_field_value("enabled", "false"), "False") + self.assertEqual(format_field_value("retention_period", "15"), "15") + self.assertEqual(format_field_value("sampling_interval", "Unknown"), "Not configured") + + +class TestSendData(unittest.TestCase): + """Tests for send_data function""" + + @patch('show.memory_statistics.SocketManager') + def test_send_data_invalid_response(self, mock_socket_manager): + """Test send_data with invalid JSON response""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + mock_instance.receive_all.return_value = "invalid json" + + with self.assertRaises(ValueError): + send_data("test_command", {}) + + @patch('show.memory_statistics.SocketManager') + def test_send_data_non_dict_response(self, mock_socket_manager): + """Test send_data with non-dict response""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + mock_instance.receive_all.return_value = json.dumps(["not a dict"]) + + with self.assertRaises(ValueError): + send_data("test_command", {}) + + @patch('show.memory_statistics.SocketManager') + def test_successful_response_with_status(self, mock_socket_manager): + """Test successful response with status field""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + response_data = { + "status": True, + "data": "test data" + } + mock_instance.receive_all.return_value = json.dumps(response_data) + + result = send_data("test_command", {}) + self.assertTrue(result.status) + self.assertEqual(result.data, "test data") + + @patch('show.memory_statistics.SocketManager') + def test_response_without_status_field(self, mock_socket_manager): + """Test response without status field (should default to True)""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + response_data = { + "data": "test data" + } + mock_instance.receive_all.return_value = json.dumps(response_data) + + result = send_data("test_command", {}) + self.assertTrue(getattr(result, 'status', True)) + self.assertEqual(result.data, "test data") + + @patch('show.memory_statistics.SocketManager') + def test_failed_response_with_error_message(self, mock_socket_manager): + """Test response with status False and error message""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + response_data = { + "status": False, + "msg": "Operation failed" + } + mock_instance.receive_all.return_value = json.dumps(response_data) + + with self.assertRaises(RuntimeError) as context: + send_data("test_command", {}) + self.assertEqual(str(context.exception), "Operation failed") + + @patch('show.memory_statistics.SocketManager') + def test_failed_response_without_message(self, mock_socket_manager): + """Test response with status False but no error message""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + response_data = { + "status": False + } + mock_instance.receive_all.return_value = json.dumps(response_data) + + with self.assertRaises(RuntimeError) as context: + send_data("test_command", {}) + self.assertEqual(str(context.exception), "Unknown error occurred") + + @patch('show.memory_statistics.SocketManager') + def test_complex_response_object_conversion(self, mock_socket_manager): + """Test conversion of complex response object""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + response_data = { + "status": True, + "data": { + "metrics": [ + {"name": "memory", "value": 100}, + {"name": "cpu", "value": 50} + ], + "timestamp": "2024-01-01" + } + } + mock_instance.receive_all.return_value = json.dumps(response_data) + + result = send_data("test_command", {}) + self.assertTrue(result.status) + self.assertEqual(result.data.metrics[0].name, "memory") + self.assertEqual(result.data.metrics[1].value, 50) + self.assertEqual(result.data.timestamp, "2024-01-01") + + +class TestDisplayConfig(unittest.TestCase): + """Tests for display_config function""" + + def test_display_config_success(self): + """Test successful config display""" + mock_connector = MagicMock() + mock_connector.get_memory_statistics_config.return_value = { + "enabled": "true", + "retention_period": "15", + "sampling_interval": "5" + } + + runner = CliRunner() + with runner.isolation(): + display_config(mock_connector) + + def test_display_config_error(self): + """Test error handling in display config""" + mock_connector = MagicMock() + mock_connector.get_memory_statistics_config.side_effect = RuntimeError("Config error") + + with pytest.raises(click.ClickException): + display_config(mock_connector) + + +class TestMainFunction(unittest.TestCase): + """Tests for main function""" + + @patch('signal.signal') + @patch('show.memory_statistics.cli') + def test_successful_execution(self, mock_cli, mock_signal): + """Test successful execution of main function""" + main() + mock_signal.assert_called_once_with(signal.SIGTERM, shutdown_handler) + mock_cli.assert_called_once() + + @patch('signal.signal') + @patch('show.memory_statistics.cli') + @patch('show.memory_statistics.cleanup_resources') + def test_main_with_exception(self, mock_cleanup, mock_cli, mock_signal): + """Test main function with exception""" + mock_cli.side_effect = Exception("CLI error") + + with self.assertRaises(SystemExit): + main() + mock_cleanup.assert_called_once() + + +class TestFormatFieldValue: + """Tests for format_field_value function using pytest""" + + @pytest.mark.parametrize("field_name,value,expected", [ + ("enabled", "true", "True"), + ("enabled", "false", "False"), + ("enabled", "TRUE", "True"), + ("enabled", "FALSE", "False"), + ("retention_period", "15", "15"), + ("sampling_interval", "5", "5"), + ("any_field", "Unknown", "Not configured"), + ]) + def test_format_field_value(self, field_name, value, expected): + assert format_field_value(field_name, value) == expected + + +class TestMemoryStatistics(unittest.TestCase): + def setUp(self): + self.cli_runner = CliRunner() + + def test_dict2obj_invalid_input(self): + """Test Dict2Obj with invalid input (line 71)""" + with self.assertRaises(ValueError): + Dict2Obj("invalid input") + + def test_dict2obj_empty_list(self): + """Test Dict2Obj with empty list (line 78)""" + obj = Dict2Obj([]) + self.assertEqual(obj.to_dict(), []) + + @patch('socket.socket') + def test_socket_receive_timeout(self, mock_socket): + """Test socket timeout during receive (lines 137-140)""" + manager = SocketManager() + mock_socket.return_value.recv.side_effect = socket.timeout + manager.sock = mock_socket.return_value + + with self.assertRaises(ConnectionError): + manager.receive_all() + + @patch('socket.socket') + def test_socket_send_error(self, mock_socket): + """Test socket send error (line 166)""" + manager = SocketManager() + mock_socket.return_value.sendall.side_effect = socket.error("Send failed") + manager.sock = mock_socket.return_value + + with self.assertRaises(ConnectionError): + manager.send("test data") + + @patch('syslog.syslog') + def test_cleanup_resources_error(self, mock_syslog): + """Test cleanup resources error handling (lines 220-223)""" + cleanup_resources.socket_manager = MagicMock() + cleanup_resources.socket_manager.close.side_effect = Exception("Cleanup failed") + + cleanup_resources() + mock_syslog.assert_called_with(syslog.LOG_ERR, "Error during cleanup: Cleanup failed") + + @patch('show.memory_statistics.send_data') + def test_show_statistics_invalid_data(self, mock_send): + """Test show statistics with invalid data format (line 247)""" + mock_send.return_value = Dict2Obj(["invalid"]) + result = self.cli_runner.invoke(show_statistics, []) + self.assertIn("Error: Invalid data format received", result.output) + + @patch('show.memory_statistics.SonicDBConnector') + def test_show_configuration_error(self, mock_db): + """Test show configuration error (line 302)""" + mock_db.side_effect = Exception("DB connection failed") + result = self.cli_runner.invoke(show_configuration) + self.assertIn("Error: DB connection failed", result.output) + + @patch('show.memory_statistics.signal.signal') + def test_main_error(self, mock_signal): + """Test main function error handling (lines 344, 355)""" + mock_signal.side_effect = Exception("Signal registration failed") + + with self.assertRaises(SystemExit): + main() + + def test_socket_manager_validation(self): + """Test socket path validation (line 409)""" + with self.assertRaises(ConnectionError): + SocketManager("/nonexistent/path/socket") + + +class TestAdditionalMemoryStatisticsCLI(unittest.TestCase): + + def setUp(self): + self.runner = CliRunner() + + def test_dict2obj_with_nested_data(self): + """Test Dict2Obj with deeply nested dictionaries""" + data = {'a': {'b': {'c': 1}}, 'list': [1, {'d': 2}]} + obj = Dict2Obj(data) + self.assertEqual(obj.a.b.c, 1) + self.assertEqual(obj.list[1].d, 2) + self.assertEqual(obj.to_dict(), data) + + @patch('show.memory_statistics.SocketManager') + def test_socket_manager_close_exception(self, mock_socket_manager): + """Test SocketManager close handles exceptions gracefully""" + mock_socket_instance = mock_socket_manager.return_value + mock_socket_instance.close.side_effect = Exception("Close error") + + manager = SocketManager() + manager.sock = mock_socket_instance + + with patch('syslog.syslog') as mock_syslog: + manager.close() + mock_syslog.assert_any_call(4, "Error closing socket: Close error") + + def test_dict2obj_repr(self): + """Test the __repr__ method of Dict2Obj""" + data = {'a': 1, 'b': {'c': 2}} + obj = Dict2Obj(data) + repr_str = repr(obj) + self.assertTrue(repr_str.startswith(' Date: Thu, 26 Dec 2024 13:25:34 +0500 Subject: [PATCH 25/30] Add memory statistics monitoring commands to documentation. Signed-off-by: Arham-Nasir --- doc/Command-Reference.md | 283 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 283 insertions(+) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 1aa9c6523f..746ad591da 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -225,6 +225,18 @@ * [Static DNS show command](#static-dns-show-command) * [Wake-on-LAN Commands](#wake-on-lan-commands) * [Send Wake-on-LAN Magic Packet command](#send-wake-on-lan-magic-packet-command) +* [Memory Statistics CLI Commands](#memory-statistics-cli-commands) + * [Overview](#overview) + * [Enable/Disable Memory Statistics Monitoring](#enabledisable-memory-statistics-monitoring) + * [Set the Frequency of Memory Data Collection](#set-the-frequency-of-memory-data-collection) + * [Adjust the Data Retention Period](#adjust-the-data-retention-period) + * [View Memory Usage Statistics](#view-memory-usage-statistics) + * [Default Historical Memory Statistics](#default-historical-memory-statistics) + * [Historical Memory Statistics for Last 10 Days](#historical-memory-statistics-for-last-10-days) + * [Historical Memory Statistics for Last 100 Minutes](#historical-memory-statistics-for-last-100-minutes) + * [Historical Memory Statistics for Last 3 Hours](#historical-memory-statistics-for-last-3-hours) + * [Historical Memory Statistics for Specific Metric (Used Memory)](#historical-memory-statistics-for-specific-metric-used-memory) + * [View Memory Statistics Configuration](#view-memory-statistics-configuration) ## Document History @@ -13886,3 +13898,274 @@ Sending 3 magic packet to 11:33:55:77:99:bb via interface Vlan1000 ``` For the 4th example, it specifise 2 target MAC addresses and `count` is 3. So it'll send 6 magic packets in total. + +# Memory Statistics CLI Commands + +## Overview +These commands allow users to enable/disable memory statistics monitoring, configure data collection intervals, adjust data retention periods, view memory statistics, and check the current configuration. Memory statistics can help administrators monitor and analyze system memory usage over time. + +--- + +## 1. Enable/Disable Memory Statistics Monitoring + +To enable or disable the memory statistics monitoring feature: + +```bash +admin@sonic:~$ config memory-stats enable/disable +``` + +This will **enable/disable** memory statistics monitoring. + +By default, this feature is **disabled**. + +**Examples**: + +- To enable memory statistics monitoring: + + ```bash + admin@sonic:~$ config memory-stats enable + Memory statistics monitoring enabled. + ``` + +- To disable memory statistics monitoring: + + ```bash + admin@sonic:~$ config memory-stats disable + Memory statistics monitoring disabled. + ``` + +--- + +## 2. Set the Frequency of Memory Data Collection + +To configure the interval for memory data collection (specified in minutes): + +```bash +admin@sonic:~$ config memory-stats sampling-interval +``` + +- `` is the sampling interval in minutes. +- The default sampling interval is **5 minutes**. + +**Example**: + +- To set the sampling interval to 10 minutes: + + ```bash + admin@sonic:~$ config memory-stats sampling-interval 10 + Sampling interval set to 10 minutes. + ``` + +--- + +## 3. Adjust the Data Retention Period + +To set how long the memory data should be retained (specified in days): + +```bash +admin@sonic:~$ config memory-stats retention-period +``` + +- `` is the retention period in days. +- The default retention period is **15 days**. + +**Example**: + +- To set the retention period to 30 days: + + ```bash + admin@sonic:~$ config memory-stats retention-period 30 + Retention period set to 30 days. + ``` + +--- + +## 4. View Memory Usage Statistics +To display memory usage statistics, use the following command with optional parameters for time range and specific metrics: + +```bash +admin@sonic:~$ show memory-stats [--from ] [--to ] [--select ] +``` + +**Command Options:** +- `show memory-stats`: Display basic memory usage statistics. +- `--from `: Display memory statistics from the specified start date-time. +- `--to `: Display memory statistics up to the specified end date-time. +- `--select `: Display specific memory statistics, such as total memory. + +**Time Format for Statistics Retrieval:** +The time format for `--from` and `--to` options includes: +- **Relative time formats:** + - 'X days ago', 'X hours ago', 'X minutes ago' + - 'yesterday', 'today' +- **Specific times and dates:** + - 'now' + - 'July 23', 'July 23, 2024', '2 November 2024' + - '7/24', '1/2' +- **Time expressions:** + - '2 am', '3:15 pm' + - 'Aug 01 06:43:40', 'July 1 3:00:00' +- **Named months:** + - 'jan', 'feb', 'march', 'september', etc. + - Full month names: 'January', 'February', 'March', etc. +- **ISO 8601 format:** + - '2024-07-01T15:00:00' + + +### 4.1. Default Historical Memory Statistics + +To view the historical memory statistics: + +```bash +admin@sonic:~$ show memory-stats +``` + +**Sample Output**: + +```bash +Memory Statistics: +Codes: M - minutes, H - hours, D - days +-------------------------------------------------------------------------------- +Report Generated: 2024-12-04 15:49:52 +Analysis Period: From 2024-11-19 15:49:52 to 2024-12-04 15:49:52 +Interval: 2 Days +-------------------------------------------------------------------------------------------------------------------------------------------------- +Metric Current High Low D19-D21 D21-D23 D23-D25 D25-D27 D27-D29 D29-D01 D01-D03 D03-D05 + Value Value Value 19Nov24 21Nov24 23Nov24 25Nov24 27Nov24 29Nov24 01Dec24 03Dec24 +-------------------------------------------------------------------------------------------------------------------------------------------------- +total_memory 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB +used_memory 8.87GB 9.35GB 8.15GB 8.15GB 9.10GB 8.15GB 8.20GB 9.05GB 8.30GB 9.35GB 9.12GB +free_memory 943.92MB 906.28MB 500.00MB 800.00MB 750.00MB 906.2MB 650.00MB 600.00MB 550.00MB 500.00MB 725.92MB +available_memory 4.78GB 4.74GB 4.35GB 4.65GB 4.60GB 4.55GB 4.74GB 4.45GB 4.40GB 4.35GB 4.57GB +cached_memory 5.17GB 5.08GB 4.96GB 5.08GB 5.06GB 5.04GB 5.02GB 5.00GB 4.98GB 4.96GB 5.05GB +buffers_memory 337.83MB 333.59MB 295.00MB 325.00MB 320.00MB 315.00MB 333.59MB 305.00MB 300.00MB 295.00MB 317.84MB +shared_memory 1.31GB 1.22GB 1.08GB 1.22GB 1.20GB 1.18GB 1.15GB 1.12GB 1.10GB 1.08GB 1.19GB +``` + +### 4.2. Historical Memory Statistics for Last 10 Days + +To view memory statistics for the last 10 days: + +```bash +admin@sonic:~$ show memory-stats --from '10 days ago' --to 'now' +``` + +**Sample Output**: + +``` +Memory Statistics: +Codes: M - minutes, H - hours, D - days +-------------------------------------------------------------------------------- +Report Generated: 2024-12-24 17:29:19 +Analysis Period: From 2024-12-14 17:29:19 to 2024-12-24 17:29:19 +Interval: 1 Days +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +Metric Current High Low D14-D15 D15-D16 D16-D17 D17-D18 D18-D19 D19-D20 D20-D21 D21-D22 D22-D23 D23-D24 D24-D25 + Value Value Value 14Dec24 15Dec24 16Dec24 17Dec24 18Dec24 19Dec24 20Dec24 21Dec24 22Dec24 23Dec24 24Dec24 +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +total_memory 15.29GB 15.29GB 15.29GB - - - - - - - - - - 15.29GB +used_memory 11.74GB 9.14GB 9.14GB - - - - - - - - - - 9.14GB +free_memory 704.33MB 2.61GB 2.61GB - - - - - - - - - - 2.61GB +available_memory 2.21GB 4.73GB 4.73GB - - - - - - - - - - 4.73GB +cached_memory 2.76GB 3.40GB 3.40GB - - - - - - - - - - 3.40GB +buffers_memory 105.39MB 144.28MB 144.28MB - - - - - - - - - - 144.28MB +shared_memory 1.00GB 1.08GB 1.08GB - - - - - - - - - - 1.08GB +``` + +### 4.3. Historical Memory Statistics for Last 100 Minutes + +```bash +admin@sonic:~$ show memory-stats --from '100 minutes ago' --to 'now' +``` + +**Sample Output**: + +``` +Memory Statistics: +Codes: M - minutes, H - hours, D - days +-------------------------------------------------------------------------------- +Report Generated: 2024-12-24 17:24:08 +Analysis Period: From 2024-12-24 15:44:08 to 2024-12-24 17:24:08 +Interval: 10 Minutes +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +Metric Current High Low M44-M54 M54-M04 M04-M14 M14-M24 M24-M34 M34-M44 M44-M54 M54-M04 M04-M14 M14-M24 M24-M34 + Value Value Value 15:44 15:54 16:04 16:14 16:24 16:34 16:44 16:54 17:04 17:14 17:24 +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +total_memory 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB - +used_memory 11.62GB 11.81GB 10.69GB 11.81GB 11.74GB 10.69GB 10.93GB 11.31GB 11.31GB 11.38GB 11.40GB 11.44GB 11.50GB - +free_memory 888.46MB 1.65GB 514.18MB 514.18MB 525.77MB 1.65GB 1.15GB 802.98MB 818.78MB 680.81MB 716.42MB 533.82MB 1.07GB - +available_memory 2.35GB 3.37GB 2.25GB 2.25GB 2.25GB 3.37GB 2.96GB 2.62GB 2.64GB 2.52GB 2.57GB 2.49GB 2.52GB - +cached_memory 2.70GB 3.15GB 2.63GB 2.85GB 2.91GB 2.82GB 3.07GB 3.05GB 3.03GB 3.09GB 3.03GB 3.15GB 2.63GB - +buffers_memory 101.39MB 186.47MB 99.00MB 134.77MB 136.97MB 140.94MB 148.42MB 153.82MB 157.19MB 160.90MB 165.18MB 186.47MB 99.00MB - +shared_memory 1005.79MB 1.07GB 917.46MB 926.08MB 993.94MB 917.46MB 1.07GB 1.01GB 1020.12MB 1.04GB 1001.18MB 1.01GB 961.13MB - +``` + +### 4.4. Historical Memory Statistics for Last 3 Hours + +```bash +admin@sonic:~$ show memory-stats --from '3 hours ago' --to 'now' +``` + +**Sample Output**: + +``` +Memory Statistics: +Codes: M - minutes, H - hours, D - days +-------------------------------------------------------------------------------- +Report Generated: 2024-12-24 17:24:51 +Analysis Period: From 2024-12-24 14:24:51 to 2024-12-24 17:24:51 +Interval: 1 Hours +-------------------------------------------------------------------------------------------------- +Metric Current High Low H14-H15 H15-H16 H16-H17 H17-H18 + Value Value Value 14:24 15:24 16:24 17:24 +-------------------------------------------------------------------------------------------------- +total_memory 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB 15.29GB - +used_memory 11.59GB 11.52GB 11.39GB 11.42GB 11.52GB 11.39GB - +free_memory 928.18MB 826.58MB 774.48MB 780.43MB 826.58MB 774.48MB - +available_memory 2.39GB 2.56GB 2.50GB 2.53GB 2.50GB 2.56GB - +cached_memory 2.70GB 3.00GB 2.83GB 2.89GB 2.83GB 3.00GB - +buffers_memory 101.62MB 153.76MB 132.42MB 149.62MB 132.42MB 153.76MB - +shared_memory 997.97MB 1020.80MB 961.19MB 971.47MB 961.19MB 1020.80MB - +``` + +### 4.5. Historical Memory Statistics for Specific Metric (Used Memory) + +```bash +admin@sonic:~$ show memory-stats --from '100 minutes ago' --to 'now' --select 'used_memory' +``` + +**Sample Output**: + +``` +Memory Statistics: +Codes: M - minutes, H - hours, D - days +-------------------------------------------------------------------------------- +Report Generated: 2024-12-24 17:27:58 +Analysis Period: From 2024-12-24 15:47:58 to 2024-12-24 17:27:58 +Interval: 10 Minutes +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +Metric Current High Low M47-M57 M57-M07 M07-M17 M17-M27 M27-M37 M37-M47 M47-M57 M57-M07 M07-M17 M17-M27 M27-M37 + Value Value Value 15:47 15:57 16:07 16:17 16:27 16:37 16:47 16:57 17:07 17:17 17:27 +-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- +used_memory 11.69GB 11.79GB 10.55GB 11.79GB 11.35GB 10.55GB 11.24GB 11.30GB 11.33GB 11.40GB 11.39GB 11.46GB 11.62GB - + + +``` + +--- + +## 5. View Memory Statistics Configuration +To display the current configuration parameters such as data collection frequency, retention period, and enable/disable status, use the following command: + +```bash +admin@sonic:~$ show memory-stats-config +``` +**Example:** +```bash +admin@sonic:~$ show memory-stats-config +Memory Statistics Configuration: +-------------------------------- +Enabled: false +Sampling Interval: 5 +Retention Period: 15 +``` From c47b61b8df9d51342cb8914d7b5608d1675a6141 Mon Sep 17 00:00:00 2001 From: Arham-Nasir Date: Mon, 30 Dec 2024 17:13:09 +0500 Subject: [PATCH 26/30] Implement single DB connection for config commands Signed-off-by: Arham-Nasir --- config/memory_statistics.py | 126 ++++++++++++++++++++----- tests/config_memory_statistics_test.py | 63 ++++++++----- 2 files changed, 138 insertions(+), 51 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 9b64c0c044..3c6ac19e32 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -15,24 +15,79 @@ def log_to_syslog(message, level=syslog.LOG_INFO): - """Log a message to syslog.""" + """Log a message to syslog. + + This function logs the provided message to syslog at the specified level. + It opens the syslog with the application name 'memory_statistics' and the + appropriate log level. + + Args: + message (str): The message to log. + level (int): The syslog log level. + """ syslog.openlog("memory_statistics", syslog.LOG_PID | syslog.LOG_CONS, syslog.LOG_USER) syslog.syslog(level, message) -def get_db_connection(): - """Create and return a database connection.""" - db = ConfigDBConnector() - db.connect() - return db +class MemoryStatisticsDB: + """Singleton class to handle memory statistics database connection. + + This class ensures only one instance of the database connection exists using + the Singleton pattern. It provides access to the database connection and + ensures that it is created only once during the application's lifetime. + """ + _instance = None + _db = None + + def __new__(cls): + """Ensure only one instance of MemoryStatisticsDB is created. + + This method implements the Singleton pattern to guarantee that only one + instance of the MemoryStatisticsDB class exists. If no instance exists, + it creates one and connects to the database. + + Returns: + MemoryStatisticsDB: The singleton instance of the class. + """ + if cls._instance is None: + cls._instance = super(MemoryStatisticsDB, cls).__new__(cls) + cls._db = ConfigDBConnector() + cls._db.connect() + return cls._instance + + @classmethod + def get_db(cls): + """Get the singleton database connection instance. + + Returns the existing database connection instance. If it doesn't exist, + a new instance is created by calling the __new__ method. + + Returns: + ConfigDBConnector: The database connection instance. + """ + if cls._instance is None: + cls._instance = cls() + return cls._db -def update_memory_statistics_status(status, db): +def update_memory_statistics_status(status): """ - Updates the status of the memory statistics feature in the config DB. - Returns a tuple of (success, error_message). + Update the status of the memory statistics feature in the config DB. + + This function modifies the configuration database to enable or disable + memory statistics collection based on the provided status. It also logs + the action and returns a tuple indicating whether the operation was successful. + + Args: + status (str): The status to set for memory statistics ("true" or "false"). + + Returns: + tuple: A tuple (success, error_message) where `success` is a boolean + indicating whether the operation was successful, and + `error_message` contains any error details if unsuccessful. """ try: + db = MemoryStatisticsDB.get_db() db.mod_entry(MEMORY_STATISTICS_TABLE, MEMORY_STATISTICS_KEY, {"enabled": status}) msg = f"Memory statistics feature {'enabled' if status == 'true' else 'disabled'} successfully." click.echo(msg) @@ -47,27 +102,36 @@ def update_memory_statistics_status(status, db): @click.group() def cli(): - """Memory statistics configuration tool.""" + """Memory statistics configuration tool. + + This command-line interface (CLI) allows users to configure and manage + memory statistics settings such as enabling/disabling the feature and + modifying parameters like the sampling interval and retention period. + """ pass @cli.group() def config(): - """Configuration commands.""" + """Configuration commands for managing memory statistics.""" pass @config.group(name='memory-stats') def memory_stats(): - """Configure memory statistics.""" + """Configure memory statistics collection and settings.""" pass @memory_stats.command(name='enable') def memory_stats_enable(): - """Enable memory statistics collection.""" - db = get_db_connection() - success, error = update_memory_statistics_status("true", db) + """Enable memory statistics collection. + + This command enables the collection of memory statistics on the device. + It updates the configuration and reminds the user to run 'config save' + to persist changes. + """ + success, error = update_memory_statistics_status("true") if success: click.echo("Reminder: Please run 'config save' to persist changes.") log_to_syslog("Memory statistics enabled. Reminder to run 'config save'") @@ -75,9 +139,13 @@ def memory_stats_enable(): @memory_stats.command(name='disable') def memory_stats_disable(): - """Disable memory statistics collection.""" - db = get_db_connection() - success, error = update_memory_statistics_status("false", db) + """Disable memory statistics collection. + + This command disables the collection of memory statistics on the device. + It updates the configuration and reminds the user to run 'config save' + to persist changes. + """ + success, error = update_memory_statistics_status("false") if success: click.echo("Reminder: Please run 'config save' to persist changes.") log_to_syslog("Memory statistics disabled. Reminder to run 'config save'") @@ -87,10 +155,13 @@ def memory_stats_disable(): @click.argument("interval", type=int) def memory_stats_sampling_interval(interval): """ - Set sampling interval for memory statistics. + Set the sampling interval for memory statistics. - The sampling interval must be between 3 and 15 minutes. - This determines how frequently memory usage data is collected. + This command allows users to configure the frequency at which memory statistics + are collected. The interval must be between 3 and 15 minutes. + + Args: + interval (int): The sampling interval in minutes (must be between 3 and 15). """ if not (SAMPLING_INTERVAL_MIN <= interval <= SAMPLING_INTERVAL_MAX): error_msg = ( @@ -101,8 +172,8 @@ def memory_stats_sampling_interval(interval): log_to_syslog(error_msg, syslog.LOG_ERR) return - db = get_db_connection() try: + db = MemoryStatisticsDB.get_db() db.mod_entry(MEMORY_STATISTICS_TABLE, MEMORY_STATISTICS_KEY, {"sampling_interval": str(interval)}) success_msg = f"Sampling interval set to {interval} minutes successfully." click.echo(success_msg) @@ -118,10 +189,13 @@ def memory_stats_sampling_interval(interval): @click.argument("period", type=int) def memory_stats_retention_period(period): """ - Set retention period for memory statistics. + Set the retention period for memory statistics. + + This command allows users to configure how long memory statistics are retained + before being purged. The retention period must be between 1 and 30 days. - The retention period must be between 1 and 30 days. - This determines how long memory usage data is stored before being purged. + Args: + period (int): The retention period in days (must be between 1 and 30). """ if not (RETENTION_PERIOD_MIN <= period <= RETENTION_PERIOD_MAX): error_msg = f"Error: Retention period must be between {RETENTION_PERIOD_MIN} and {RETENTION_PERIOD_MAX} days." @@ -129,8 +203,8 @@ def memory_stats_retention_period(period): log_to_syslog(error_msg, syslog.LOG_ERR) return - db = get_db_connection() try: + db = MemoryStatisticsDB.get_db() db.mod_entry(MEMORY_STATISTICS_TABLE, MEMORY_STATISTICS_KEY, {"retention_period": str(period)}) success_msg = f"Retention period set to {period} days successfully." click.echo(success_msg) diff --git a/tests/config_memory_statistics_test.py b/tests/config_memory_statistics_test.py index 43440cd934..1b91b30f76 100644 --- a/tests/config_memory_statistics_test.py +++ b/tests/config_memory_statistics_test.py @@ -8,6 +8,7 @@ from config.memory_statistics import ( cli, log_to_syslog, + MemoryStatisticsDB, MEMORY_STATISTICS_KEY, MEMORY_STATISTICS_TABLE, RETENTION_PERIOD_MAX, @@ -24,6 +25,8 @@ def mock_db(): with patch('config.memory_statistics.ConfigDBConnector') as mock_db_class: mock_db_instance = Mock() mock_db_class.return_value = mock_db_instance + MemoryStatisticsDB._instance = None + MemoryStatisticsDB._db = None yield mock_db_instance @@ -33,12 +36,37 @@ def cli_runner(): return CliRunner() +class TestMemoryStatisticsDB: + """Tests for the MemoryStatisticsDB singleton class""" + + def test_singleton_pattern(self, mock_db): + """Test that MemoryStatisticsDB implements singleton pattern correctly.""" + MemoryStatisticsDB._instance = None + MemoryStatisticsDB._db = None + + db1 = MemoryStatisticsDB() + db2 = MemoryStatisticsDB() + assert db1 is db2 + assert MemoryStatisticsDB._instance is db1 + mock_db.connect.assert_called_once() + + def test_get_db_connection(self, mock_db): + """Test that get_db returns the same database connection.""" + MemoryStatisticsDB._instance = None + MemoryStatisticsDB._db = None + + db1 = MemoryStatisticsDB.get_db() + db2 = MemoryStatisticsDB.get_db() + assert db1 is db2 + mock_db.connect.assert_called_once() + + class TestUpdateMemoryStatisticsStatus: - """Direct tests for update_memory_statistics_status function""" + """Tests for update_memory_statistics_status function""" def test_successful_enable(self, mock_db): """Test successful status update to enable.""" - success, error = update_memory_statistics_status("true", mock_db) + success, error = update_memory_statistics_status("true") assert success is True assert error is None mock_db.mod_entry.assert_called_once_with( @@ -49,7 +77,7 @@ def test_successful_enable(self, mock_db): def test_successful_disable(self, mock_db): """Test successful status update to disable.""" - success, error = update_memory_statistics_status("false", mock_db) + success, error = update_memory_statistics_status("false") assert success is True assert error is None mock_db.mod_entry.assert_called_once_with( @@ -61,7 +89,7 @@ def test_successful_disable(self, mock_db): def test_database_error(self, mock_db): """Test handling of database errors.""" mock_db.mod_entry.side_effect = Exception("DB Error") - success, error = update_memory_statistics_status("true", mock_db) + success, error = update_memory_statistics_status("true") assert success is False assert "Error updating memory statistics status" in error assert "DB Error" in error @@ -138,25 +166,11 @@ def test_invalid_sampling_intervals(self, interval, cli_runner, mock_db): assert "Error" in result.output assert not mock_db.mod_entry.called - def test_sampling_interval_specific_db_error(self, cli_runner, mock_db): - """Test specific database error case when setting sampling interval.""" - mock_db.mod_entry.side_effect = Exception("Database connection lost") - - with patch('config.memory_statistics.log_to_syslog') as mock_log: - result = cli_runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', '5']) - - expected_error = "Error setting sampling interval: Database connection lost" - assert expected_error in result.output - - mock_log.assert_called_with(expected_error, syslog.LOG_ERR) - - assert result.exit_code == 0 - - mock_db.mod_entry.assert_called_once_with( - MEMORY_STATISTICS_TABLE, - MEMORY_STATISTICS_KEY, - {"sampling_interval": "5"} - ) + def test_sampling_interval_db_error(self, cli_runner, mock_db): + """Test database error case when setting sampling interval.""" + mock_db.mod_entry.side_effect = Exception("DB Error") + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', '5']) + assert "Error" in result.output class TestRetentionPeriod: @@ -242,13 +256,12 @@ def test_main_cli_integration(): """Test the main CLI integration with actual command.""" runner = CliRunner() - with patch('config.memory_statistics.get_db_connection') as mock_get_db: + with patch('config.memory_statistics.MemoryStatisticsDB.get_db') as mock_get_db: mock_db = Mock() mock_get_db.return_value = mock_db result = runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', '5']) assert result.exit_code == 0 - mock_get_db.assert_called_once() From cf7f6d063731d86fbf8708d672767b37bb5d76fb Mon Sep 17 00:00:00 2001 From: Arham-Nasir Date: Tue, 7 Jan 2025 15:06:44 +0500 Subject: [PATCH 27/30] add closelog to ensure that connection is closed Signed-off-by: Arham-Nasir --- config/memory_statistics.py | 71 ++++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 3c6ac19e32..03223a0853 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -19,14 +19,17 @@ def log_to_syslog(message, level=syslog.LOG_INFO): This function logs the provided message to syslog at the specified level. It opens the syslog with the application name 'memory_statistics' and the - appropriate log level. + appropriate log level, ensuring the connection is closed after logging. Args: message (str): The message to log. level (int): The syslog log level. """ - syslog.openlog("memory_statistics", syslog.LOG_PID | syslog.LOG_CONS, syslog.LOG_USER) - syslog.syslog(level, message) + try: + syslog.openlog("memory_statistics", syslog.LOG_PID | syslog.LOG_CONS, syslog.LOG_USER) + syslog.syslog(level, message) + finally: + syslog.closelog() class MemoryStatisticsDB: @@ -113,13 +116,35 @@ def cli(): @cli.group() def config(): - """Configuration commands for managing memory statistics.""" + """Configuration commands for managing memory statistics. + + Example: + $ config memory-stats enable + $ config memory-stats sampling-interval 5 + """ pass @config.group(name='memory-stats') def memory_stats(): - """Configure memory statistics collection and settings.""" + """Configure memory statistics collection and settings. + + This group contains commands to enable/disable memory statistics collection + and configure related parameters. + + Examples: + Enable memory statistics: + $ config memory-stats enable + + Set sampling interval to 5 minutes: + $ config memory-stats sampling-interval 5 + + Set retention period to 7 days: + $ config memory-stats retention-period 7 + + Disable memory statistics: + $ config memory-stats disable + """ pass @@ -130,6 +155,11 @@ def memory_stats_enable(): This command enables the collection of memory statistics on the device. It updates the configuration and reminds the user to run 'config save' to persist changes. + + Example: + $ config memory-stats enable + Memory statistics feature enabled successfully. + Reminder: Please run 'config save' to persist changes. """ success, error = update_memory_statistics_status("true") if success: @@ -144,6 +174,11 @@ def memory_stats_disable(): This command disables the collection of memory statistics on the device. It updates the configuration and reminds the user to run 'config save' to persist changes. + + Example: + $ config memory-stats disable + Memory statistics feature disabled successfully. + Reminder: Please run 'config save' to persist changes. """ success, error = update_memory_statistics_status("false") if success: @@ -154,14 +189,23 @@ def memory_stats_disable(): @memory_stats.command(name='sampling-interval') @click.argument("interval", type=int) def memory_stats_sampling_interval(interval): - """ - Set the sampling interval for memory statistics. + """Set the sampling interval for memory statistics. This command allows users to configure the frequency at which memory statistics are collected. The interval must be between 3 and 15 minutes. Args: interval (int): The sampling interval in minutes (must be between 3 and 15). + + Examples: + Set sampling interval to 5 minutes: + $ config memory-stats sampling-interval 5 + Sampling interval set to 5 minutes successfully. + Reminder: Please run 'config save' to persist changes. + + Invalid interval example: + $ config memory-stats sampling-interval 20 + Error: Sampling interval must be between 3 and 15 minutes. """ if not (SAMPLING_INTERVAL_MIN <= interval <= SAMPLING_INTERVAL_MAX): error_msg = ( @@ -188,14 +232,23 @@ def memory_stats_sampling_interval(interval): @memory_stats.command(name='retention-period') @click.argument("period", type=int) def memory_stats_retention_period(period): - """ - Set the retention period for memory statistics. + """Set the retention period for memory statistics. This command allows users to configure how long memory statistics are retained before being purged. The retention period must be between 1 and 30 days. Args: period (int): The retention period in days (must be between 1 and 30). + + Examples: + Set retention period to 7 days: + $ config memory-stats retention-period 7 + Retention period set to 7 days successfully. + Reminder: Please run 'config save' to persist changes. + + Invalid period example: + $ config memory-stats retention-period 45 + Error: Retention period must be between 1 and 30 days. """ if not (RETENTION_PERIOD_MIN <= period <= RETENTION_PERIOD_MAX): error_msg = f"Error: Retention period must be between {RETENTION_PERIOD_MIN} and {RETENTION_PERIOD_MAX} days." From dab65d49d2cf311c8b2cfbb24a08536bd4384905 Mon Sep 17 00:00:00 2001 From: Arham-Nasir Date: Tue, 14 Jan 2025 18:12:42 +0500 Subject: [PATCH 28/30] update and restructure CLI documentation Signed-off-by: Arham-Nasir --- doc/Command-Reference.md | 56 ++++++++++++++++++++++++++-------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 5eec548871..6e294d0f76 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -228,19 +228,19 @@ * [Banner Commands](#banner-commands) * [Banner config commands](#banner-config-commands) * [Banner show command](#banner-show-command) -* [Memory Statistics CLI Commands](#memory-statistics-cli-commands) +* [Memory Statistics Commands](#memory-statistics-commands) * [Overview](#overview) - * [Enable/Disable Memory Statistics Monitoring](#enabledisable-memory-statistics-monitoring) - * [Set the Frequency of Memory Data Collection](#set-the-frequency-of-memory-data-collection) - * [Adjust the Data Retention Period](#adjust-the-data-retention-period) - * [View Memory Usage Statistics](#view-memory-usage-statistics) + * [Memory Statistics Config Commands](#memory-statistics-config-commands) + * [Enable/Disable Memory Statistics Monitoring](#enabledisable-memory-statistics-monitoring) + * [Set the Frequency of Memory Data Collection](#set-the-frequency-of-memory-data-collection) + * [Adjust the Data Retention Period](#adjust-the-data-retention-period) + * [Memory Statistics Show Commands](#memory-statistics-show-commands) * [Default Historical Memory Statistics](#default-historical-memory-statistics) * [Historical Memory Statistics for Last 10 Days](#historical-memory-statistics-for-last-10-days) * [Historical Memory Statistics for Last 100 Minutes](#historical-memory-statistics-for-last-100-minutes) * [Historical Memory Statistics for Last 3 Hours](#historical-memory-statistics-for-last-3-hours) * [Historical Memory Statistics for Specific Metric (Used Memory)](#historical-memory-statistics-for-specific-metric-used-memory) - * [View Memory Statistics Configuration](#view-memory-statistics-configuration) - + * [View Memory Statistics Configuration](#view-memory-statistics-configuration) ## Document History | Version | Modification Date | Details | @@ -13963,17 +13963,24 @@ enabled Login You are on All access and/or use are subject to monitoring. Help: https://sonic-net.github.io/SONiC/ - + ``` --- -# Memory Statistics CLI Commands +# Memory Statistics Commands ## Overview These commands allow users to enable/disable memory statistics monitoring, configure data collection intervals, adjust data retention periods, view memory statistics, and check the current configuration. Memory statistics can help administrators monitor and analyze system memory usage over time. +**Common Use Cases** + - Monitor system memory trends over time. + - Track memory usage patterns during peak time. + - Plan system capacity based on historical memory data. + --- -## 1. Enable/Disable Memory Statistics Monitoring +## Memory Statistics Config Commands + +### Enable/Disable Memory Statistics Monitoring To enable or disable the memory statistics monitoring feature: @@ -14003,7 +14010,7 @@ By default, this feature is **disabled**. --- -## 2. Set the Frequency of Memory Data Collection +### Set the Frequency of Memory Data Collection To configure the interval for memory data collection (specified in minutes): @@ -14025,7 +14032,7 @@ admin@sonic:~$ config memory-stats sampling-interval --- -## 3. Adjust the Data Retention Period +### Adjust the Data Retention Period To set how long the memory data should be retained (specified in days): @@ -14047,7 +14054,9 @@ admin@sonic:~$ config memory-stats retention-period --- -## 4. View Memory Usage Statistics +## Memory Statistics Show Commands + +### View Memory Usage Statistics To display memory usage statistics, use the following command with optional parameters for time range and specific metrics: ```bash @@ -14078,8 +14087,9 @@ The time format for `--from` and `--to` options includes: - **ISO 8601 format:** - '2024-07-01T15:00:00' +--- -### 4.1. Default Historical Memory Statistics +### Default Historical Memory Statistics To view the historical memory statistics: @@ -14109,7 +14119,9 @@ buffers_memory 337.83MB 333.59MB 295.00MB 325.00MB 320.00MB 315. shared_memory 1.31GB 1.22GB 1.08GB 1.22GB 1.20GB 1.18GB 1.15GB 1.12GB 1.10GB 1.08GB 1.19GB ``` -### 4.2. Historical Memory Statistics for Last 10 Days +--- + +### Historical Memory Statistics for Last 10 Days To view memory statistics for the last 10 days: @@ -14139,7 +14151,9 @@ buffers_memory 105.39MB 144.28MB 144.28MB - - - shared_memory 1.00GB 1.08GB 1.08GB - - - - - - - - - - 1.08GB ``` -### 4.3. Historical Memory Statistics for Last 100 Minutes +--- + +### Historical Memory Statistics for Last 100 Minutes ```bash admin@sonic:~$ show memory-stats --from '100 minutes ago' --to 'now' @@ -14167,7 +14181,9 @@ buffers_memory 101.39MB 186.47MB 99.00MB 134.77MB 136.97MB 140. shared_memory 1005.79MB 1.07GB 917.46MB 926.08MB 993.94MB 917.46MB 1.07GB 1.01GB 1020.12MB 1.04GB 1001.18MB 1.01GB 961.13MB - ``` -### 4.4. Historical Memory Statistics for Last 3 Hours +--- + +### Historical Memory Statistics for Last 3 Hours ```bash admin@sonic:~$ show memory-stats --from '3 hours ago' --to 'now' @@ -14195,7 +14211,9 @@ buffers_memory 101.62MB 153.76MB 132.42MB 149.62MB 132.42MB 153. shared_memory 997.97MB 1020.80MB 961.19MB 971.47MB 961.19MB 1020.80MB - ``` -### 4.5. Historical Memory Statistics for Specific Metric (Used Memory) +--- + +### Historical Memory Statistics for Specific Metric (Used Memory) ```bash admin@sonic:~$ show memory-stats --from '100 minutes ago' --to 'now' --select 'used_memory' @@ -14221,7 +14239,7 @@ used_memory 11.69GB 11.79GB 10.55GB 11.79GB 11.35GB 10.5 --- -## 5. View Memory Statistics Configuration +### View Memory Statistics Configuration To display the current configuration parameters such as data collection frequency, retention period, and enable/disable status, use the following command: ```bash From 9974f39618c3293324dc0a1cfde127760f10ff91 Mon Sep 17 00:00:00 2001 From: Arham-Nasir Date: Mon, 10 Feb 2025 11:12:20 +0500 Subject: [PATCH 29/30] Refactor exception handling, improve type safety, optimize retry logic, and enhance socket security. Signed-off-by: Arham-Nasir --- config/memory_statistics.py | 197 ++++--- show/memory_statistics.py | 483 ++++++++++++----- tests/config_memory_statistics_test.py | 151 ++++-- tests/show_memory_statistics_test.py | 698 ++++++++++++------------- 4 files changed, 956 insertions(+), 573 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 03223a0853..47fbadc379 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -1,21 +1,33 @@ +# Standard library imports import syslog +# Third-party imports import click + +# Type hints +from typing import Tuple, Optional + +# Local imports from swsscommon.swsscommon import ConfigDBConnector # Constants MEMORY_STATISTICS_TABLE = "MEMORY_STATISTICS" MEMORY_STATISTICS_KEY = "memory_statistics" + SAMPLING_INTERVAL_MIN = 3 SAMPLING_INTERVAL_MAX = 15 RETENTION_PERIOD_MIN = 1 RETENTION_PERIOD_MAX = 30 + DEFAULT_SAMPLING_INTERVAL = 5 # minutes DEFAULT_RETENTION_PERIOD = 15 # days +syslog.openlog("memory_statistics", syslog.LOG_PID | syslog.LOG_CONS, syslog.LOG_USER) -def log_to_syslog(message, level=syslog.LOG_INFO): - """Log a message to syslog. + +def log_to_syslog(message: str, level: int = syslog.LOG_INFO) -> None: + """ + Logs a message to the system log (syslog). This function logs the provided message to syslog at the specified level. It opens the syslog with the application name 'memory_statistics' and the @@ -23,89 +35,137 @@ def log_to_syslog(message, level=syslog.LOG_INFO): Args: message (str): The message to log. - level (int): The syslog log level. + level (int, optional): The log level (default is syslog.LOG_INFO). + + Raises: + Exception: If syslog logging fails. """ try: - syslog.openlog("memory_statistics", syslog.LOG_PID | syslog.LOG_CONS, syslog.LOG_USER) syslog.syslog(level, message) - finally: - syslog.closelog() + except Exception as e: + click.echo(f"Failed to log to syslog: {e}", err=True) -class MemoryStatisticsDB: - """Singleton class to handle memory statistics database connection. +def generate_error_message(error_type: str, error: Exception) -> str: + """ + Generates a formatted error message for logging and user feedback. - This class ensures only one instance of the database connection exists using - the Singleton pattern. It provides access to the database connection and - ensures that it is created only once during the application's lifetime. + Args: + error_type (str): A short description of the error type. + error (Exception): The actual exception object. + + Returns: + str: A formatted error message string. + """ + return f"{error_type}: {error}" + + +def validate_range(value: int, min_val: int, max_val: int) -> bool: + """ + Validates whether a given integer value falls within a specified range. + + Args: + value (int): The value to validate. + min_val (int): The minimum allowable value. + max_val (int): The maximum allowable value. + + Returns: + bool: True if the value is within range, False otherwise. + """ + return min_val <= value <= max_val + + +class MemoryStatisticsDB: + """ + Singleton class for managing the connection to the memory statistics configuration database. """ _instance = None _db = None def __new__(cls): - """Ensure only one instance of MemoryStatisticsDB is created. - - This method implements the Singleton pattern to guarantee that only one - instance of the MemoryStatisticsDB class exists. If no instance exists, - it creates one and connects to the database. + """ + Creates and returns a singleton instance of MemoryStatisticsDB. Returns: - MemoryStatisticsDB: The singleton instance of the class. + MemoryStatisticsDB: The singleton instance. """ if cls._instance is None: cls._instance = super(MemoryStatisticsDB, cls).__new__(cls) + cls._connect_db() + return cls._instance + + @classmethod + def _connect_db(cls): + """ + Establishes a connection to the ConfigDB database. + + Logs an error if the connection fails. + """ + try: cls._db = ConfigDBConnector() cls._db.connect() - return cls._instance + except RuntimeError as e: + log_to_syslog(f"ConfigDB connection failed: {e}", syslog.LOG_ERR) + cls._db = None @classmethod def get_db(cls): - """Get the singleton database connection instance. - - Returns the existing database connection instance. If it doesn't exist, - a new instance is created by calling the __new__ method. + """ + Retrieves the database connection instance, reconnecting if necessary. Returns: - ConfigDBConnector: The database connection instance. + ConfigDBConnector: The active database connection. + + Raises: + RuntimeError: If the database connection is unavailable. """ - if cls._instance is None: - cls._instance = cls() + if cls._db is None: + cls._connect_db() + if cls._db is None: + raise RuntimeError("Database connection unavailable") return cls._db -def update_memory_statistics_status(status): +def update_memory_statistics_status(enabled: bool) -> Tuple[bool, Optional[str]]: """ - Update the status of the memory statistics feature in the config DB. + Updates the enable/disable status of memory statistics in the configuration database. This function modifies the configuration database to enable or disable memory statistics collection based on the provided status. It also logs the action and returns a tuple indicating whether the operation was successful. Args: - status (str): The status to set for memory statistics ("true" or "false"). + enabled (bool): True to enable memory statistics, False to disable. Returns: - tuple: A tuple (success, error_message) where `success` is a boolean - indicating whether the operation was successful, and - `error_message` contains any error details if unsuccessful. + Tuple[bool, Optional[str]]: A tuple containing success status and an optional error message. """ try: db = MemoryStatisticsDB.get_db() - db.mod_entry(MEMORY_STATISTICS_TABLE, MEMORY_STATISTICS_KEY, {"enabled": status}) - msg = f"Memory statistics feature {'enabled' if status == 'true' else 'disabled'} successfully." + + db.mod_entry(MEMORY_STATISTICS_TABLE, MEMORY_STATISTICS_KEY, {"enabled": str(enabled).lower()}) + msg = f"Memory statistics feature {'enabled' if enabled else 'disabled'} successfully." click.echo(msg) log_to_syslog(msg) return True, None + except (KeyError, ConnectionError, RuntimeError) as e: + error_msg = generate_error_message(f"Failed to {'enable' if enabled else 'disable'} memory statistics", e) + + click.echo(error_msg, err=True) + log_to_syslog(error_msg, syslog.LOG_ERR) + return False, error_msg except Exception as e: - error_msg = f"Error updating memory statistics status: {e}" + error_msg = generate_error_message("Unexpected error updating memory statistics status", e) + click.echo(error_msg, err=True) log_to_syslog(error_msg, syslog.LOG_ERR) return False, error_msg -@click.group() +@click.group(help="Tool to manage memory statistics configuration.") def cli(): - """Memory statistics configuration tool. + """ + Memory statistics configuration tool. This command-line interface (CLI) allows users to configure and manage memory statistics settings such as enabling/disabling the feature and @@ -114,9 +174,10 @@ def cli(): pass -@cli.group() +@cli.group(help="Commands to configure system settings.") def config(): - """Configuration commands for managing memory statistics. + """ + Configuration commands for managing memory statistics. Example: $ config memory-stats enable @@ -125,7 +186,7 @@ def config(): pass -@config.group(name='memory-stats') +@config.group(name='memory-stats', help="Manage memory statistics collection settings.") def memory_stats(): """Configure memory statistics collection and settings. @@ -161,7 +222,7 @@ def memory_stats_enable(): Memory statistics feature enabled successfully. Reminder: Please run 'config save' to persist changes. """ - success, error = update_memory_statistics_status("true") + success, error = update_memory_statistics_status(True) if success: click.echo("Reminder: Please run 'config save' to persist changes.") log_to_syslog("Memory statistics enabled. Reminder to run 'config save'") @@ -180,7 +241,7 @@ def memory_stats_disable(): Memory statistics feature disabled successfully. Reminder: Please run 'config save' to persist changes. """ - success, error = update_memory_statistics_status("false") + success, error = update_memory_statistics_status(False) if success: click.echo("Reminder: Please run 'config save' to persist changes.") log_to_syslog("Memory statistics disabled. Reminder to run 'config save'") @@ -188,14 +249,15 @@ def memory_stats_disable(): @memory_stats.command(name='sampling-interval') @click.argument("interval", type=int) -def memory_stats_sampling_interval(interval): - """Set the sampling interval for memory statistics. +def memory_stats_sampling_interval(interval: int): + """ + Configure the sampling interval for memory statistics collection. - This command allows users to configure the frequency at which memory statistics - are collected. The interval must be between 3 and 15 minutes. + This command updates the interval at which memory statistics are collected. + The interval must be between 3 and 15 minutes. Args: - interval (int): The sampling interval in minutes (must be between 3 and 15). + interval (int): The desired sampling interval in minutes. Examples: Set sampling interval to 5 minutes: @@ -207,11 +269,8 @@ def memory_stats_sampling_interval(interval): $ config memory-stats sampling-interval 20 Error: Sampling interval must be between 3 and 15 minutes. """ - if not (SAMPLING_INTERVAL_MIN <= interval <= SAMPLING_INTERVAL_MAX): - error_msg = ( - f"Error: Sampling interval must be between {SAMPLING_INTERVAL_MIN} " - f"and {SAMPLING_INTERVAL_MAX} minutes." - ) + if not validate_range(interval, SAMPLING_INTERVAL_MIN, SAMPLING_INTERVAL_MAX): + error_msg = f"Error: Sampling interval must be between {SAMPLING_INTERVAL_MIN} and {SAMPLING_INTERVAL_MAX}." click.echo(error_msg, err=True) log_to_syslog(error_msg, syslog.LOG_ERR) return @@ -223,22 +282,29 @@ def memory_stats_sampling_interval(interval): click.echo(success_msg) log_to_syslog(success_msg) click.echo("Reminder: Please run 'config save' to persist changes.") + except (KeyError, ConnectionError, ValueError, RuntimeError) as e: + error_msg = generate_error_message(f"{type(e).__name__} setting sampling interval", e) + click.echo(error_msg, err=True) + log_to_syslog(error_msg, syslog.LOG_ERR) + return except Exception as e: - error_msg = f"Error setting sampling interval: {e}" + error_msg = generate_error_message("Unexpected error setting sampling interval", e) click.echo(error_msg, err=True) log_to_syslog(error_msg, syslog.LOG_ERR) + return @memory_stats.command(name='retention-period') @click.argument("period", type=int) -def memory_stats_retention_period(period): - """Set the retention period for memory statistics. +def memory_stats_retention_period(period: int): + """ + Configure the retention period for memory statistics storage. - This command allows users to configure how long memory statistics are retained - before being purged. The retention period must be between 1 and 30 days. + This command sets the number of days memory statistics should be retained. + The retention period must be between 1 and 30 days. Args: - period (int): The retention period in days (must be between 1 and 30). + period (int): The desired retention period in days. Examples: Set retention period to 7 days: @@ -250,8 +316,8 @@ def memory_stats_retention_period(period): $ config memory-stats retention-period 45 Error: Retention period must be between 1 and 30 days. """ - if not (RETENTION_PERIOD_MIN <= period <= RETENTION_PERIOD_MAX): - error_msg = f"Error: Retention period must be between {RETENTION_PERIOD_MIN} and {RETENTION_PERIOD_MAX} days." + if not validate_range(period, RETENTION_PERIOD_MIN, RETENTION_PERIOD_MAX): + error_msg = f"Error: Retention period must be between {RETENTION_PERIOD_MIN} and {RETENTION_PERIOD_MAX}." click.echo(error_msg, err=True) log_to_syslog(error_msg, syslog.LOG_ERR) return @@ -263,11 +329,20 @@ def memory_stats_retention_period(period): click.echo(success_msg) log_to_syslog(success_msg) click.echo("Reminder: Please run 'config save' to persist changes.") + except (KeyError, ConnectionError, ValueError, RuntimeError) as e: + error_msg = generate_error_message(f"{type(e).__name__} setting retention period", e) + click.echo(error_msg, err=True) + log_to_syslog(error_msg, syslog.LOG_ERR) + return except Exception as e: - error_msg = f"Error setting retention period: {e}" + error_msg = generate_error_message("Unexpected error setting retention period", e) click.echo(error_msg, err=True) log_to_syslog(error_msg, syslog.LOG_ERR) + return if __name__ == "__main__": - cli() + try: + cli() + finally: + syslog.closelog() diff --git a/show/memory_statistics.py b/show/memory_statistics.py index aff83f5a58..26b24e6407 100644 --- a/show/memory_statistics.py +++ b/show/memory_statistics.py @@ -12,12 +12,25 @@ import click from dataclasses import dataclass -# Local application/library imports +# Local imports from swsscommon.swsscommon import ConfigDBConnector @dataclass class Config: + """ + Configuration class to manage the settings for memory statistics service and socket. + + Attributes: + SOCKET_PATH (str): The path to the Unix domain socket for communication. + SOCKET_TIMEOUT (int): The timeout value in seconds for socket operations. + BUFFER_SIZE (int): The buffer size for socket communication. + MAX_RETRIES (int): Maximum number of retry attempts for socket connection. + RETRY_DELAY (float): Delay in seconds between retry attempts. + DEFAULT_CONFIG (dict): Default configuration for memory statistics, including enabled status, + sampling interval, and retention period. + """ + SOCKET_PATH: str = '/var/run/dbus/memstats.socket' SOCKET_TIMEOUT: int = 30 BUFFER_SIZE: int = 8192 @@ -32,37 +45,62 @@ class Config: class ConnectionError(Exception): - """Custom exception for connection-related errors.""" + """ + Custom exception raised for connection-related errors in the system. + + This exception is used to signal issues with network connections or socket communications. + """ + pass + + +class DatabaseError(Exception): + """ + Custom exception raised for errors related to database interactions. + + This exception is used to signal issues with retrieving or updating data in the SONiC configuration database. + """ pass class Dict2Obj: - """Converts dictionaries or lists into objects with attribute-style access.""" - def __init__(self, d: Union[Dict[str, Any], list]) -> None: - if not isinstance(d, (dict, list)): - raise ValueError("Input should be a dictionary or a list") + """ + A utility class that converts dictionaries or lists into objects, providing attribute-style access. + + This class is helpful when you need to convert JSON-like data (dictionaries or lists) into Python objects + with attributes that can be accessed using dot notation, and vice versa. + + Methods: + __init__(d): Initializes the object either from a dictionary or list. + _init_from_dict(d): Initializes the object from a dictionary. + _init_from_list(d): Initializes the object from a list. + to_dict(): Converts the object back to a dictionary format. + __repr__(): Returns a string representation of the object for debugging purposes. + """ + def __init__(self, d: Union[Dict[str, Any], list]) -> None: if isinstance(d, dict): - for key, value in d.items(): - if isinstance(value, (list, tuple)): - setattr( - self, - key, - [Dict2Obj(x) if isinstance(x, dict) else x for x in value], - ) - else: - setattr( - self, key, Dict2Obj(value) if isinstance(value, dict) else value - ) + self._init_from_dict(d) elif isinstance(d, list): - self.items = [Dict2Obj(x) if isinstance(x, dict) else x for x in d] + self._init_from_list(d) + else: + raise ValueError("Input should be a dictionary or a list") + + def _init_from_dict(self, d: Dict[str, Any]) -> None: + for key, value in d.items(): + if isinstance(value, (list, tuple)): + setattr(self, key, [Dict2Obj(x) if isinstance(x, dict) else x for x in value]) + else: + setattr(self, key, Dict2Obj(value) if isinstance(value, dict) else value) + + def _init_from_list(self, d: list) -> None: + self.items = [Dict2Obj(x) if isinstance(x, dict) else x for x in d] def to_dict(self) -> Dict[str, Any]: """Converts the object back to a dictionary format.""" - result = {} if hasattr(self, "items"): return [x.to_dict() if isinstance(x, Dict2Obj) else x for x in self.items] + result = {} for key in self.__dict__: value = getattr(self, key) if isinstance(value, Dict2Obj): @@ -79,43 +117,67 @@ def __repr__(self) -> str: class SonicDBConnector: - """Handles interactions with SONiC's configuration database with improved connection handling.""" + """ + A class that handles interactions with the SONiC configuration database, + including connection retries and error handling. + + This class ensures robust connection management by retrying failed attempts to connect to the database + and provides methods for fetching memory statistics configuration. + + Methods: + __init__(): Initializes the connector and attempts to connect to the database. + _connect_with_retry(): Attempts to connect to the database with a retry mechanism. + get_memory_statistics_config(): Retrieves the memory statistics configuration from the database. + """ + def __init__(self) -> None: - """Initialize the database connector with retry mechanism.""" self.config_db = ConfigDBConnector() - self.connect_with_retry() + self._connect_with_retry() - def connect_with_retry(self, max_retries: int = 3, retry_delay: float = 1.0) -> None: + def _connect_with_retry(self, max_retries: int = 3, retry_delay: float = 1.0) -> None: """ - Attempts to connect to the database with a retry mechanism. - """ - retries = 0 - last_error = None + Attempts to connect to the SONiC configuration database with a retry mechanism. + + This function will attempt to connect to the database up to `max_retries` times, + with a delay of `retry_delay` seconds between each attempt. If the connection + fails after all retries, a `ConnectionError` is raised. - while retries < max_retries: + Args: + max_retries (int): Maximum number of retries before raising a `ConnectionError` (default is 3). + retry_delay (float): Delay in seconds between retries (default is 1.0). + + Raises: + ConnectionError: If the connection to the database fails after all retries. + """ + for attempt in range(max_retries): try: self.config_db.connect() syslog.syslog(syslog.LOG_INFO, "Successfully connected to SONiC config database") return - except Exception as e: - last_error = e - retries += 1 - if retries < max_retries: - syslog.syslog(syslog.LOG_WARNING, - f"Failed to connect to database" - f"(attempt {retries}/{max_retries}): {str(e)}") + except (socket.error, IOError) as e: + if attempt < max_retries - 1: + syslog.syslog( + syslog.LOG_WARNING, + f"Failed to connect to database (attempt {attempt + 1}/{max_retries}): {str(e)}" + ) time.sleep(retry_delay) - - error_msg = ( - f"Failed to connect to SONiC config database after {max_retries} attempts. " - f"Last error: {str(last_error)}" - ) - syslog.syslog(syslog.LOG_ERR, error_msg) - raise ConnectionError(error_msg) + else: + raise ConnectionError( + f"Failed to connect to SONiC config database after {max_retries} attempts: {str(e)}" + ) from e def get_memory_statistics_config(self) -> Dict[str, str]: """ - Retrieves memory statistics configuration with error handling. + Retrieves the memory statistics configuration from the SONiC configuration database. + + This function fetches the memory statistics configuration from the database, providing + default values if not found or if there are any errors. It handles potential errors and logs them. + + Returns: + Dict[str, str]: The memory statistics configuration as a dictionary. + + Raises: + DatabaseError: If there is an error while retrieving the configuration from the database. """ try: config = self.config_db.get_table('MEMORY_STATISTICS') @@ -133,31 +195,90 @@ def get_memory_statistics_config(self) -> Dict[str, str]: return result_config - except Exception as e: + except (KeyError, ValueError) as e: error_msg = f"Error retrieving memory statistics configuration: {str(e)}" syslog.syslog(syslog.LOG_ERR, error_msg) - raise RuntimeError(error_msg) + raise DatabaseError(error_msg) from e class SocketManager: - """Manages Unix domain socket connections with improved reliability.""" + """ + A class that manages Unix domain socket connections for communication with the memory statistics service. + + This class ensures proper socket validation, connection retries, and error handling, while maintaining secure + socket file permissions and ownership. + + Methods: + __init__(socket_path): Initializes the socket manager and validates the socket file. + _validate_socket_path(create_if_missing): Validates the socket file path, checking permissions and ownership. + connect(): Establishes a connection to the memory statistics service via Unix domain socket. + receive_all(expected_size, max_attempts): Receives all data from the socket with error handling. + send(data): Sends data to the socket. + close(): Closes the socket connection safely. + """ + def __init__(self, socket_path: str = Config.SOCKET_PATH): self.socket_path = socket_path self.sock = None - self._validate_socket_path() + self._validate_socket_path(create_if_missing=True) - def _validate_socket_path(self) -> None: - """Validates the socket path exists or can be created.""" + def _validate_socket_path(self, create_if_missing: bool = False) -> None: + """ + Validates the socket file path and checks for the necessary permissions and ownership. + + This function checks if the socket file exists and has the correct permissions (0o600), + and that it is owned by root. If the file does not exist and `create_if_missing` is `True`, + the socket file is created. If the file exists with incorrect permissions, a `PermissionError` is raised. + + Args: + create_if_missing (bool): Whether to create the socket file if it does not exist (default is False). + + Raises: + ConnectionError: If the socket directory or file is missing or not accessible. + PermissionError: If the socket file has incorrect permissions or ownership. + """ socket_dir = os.path.dirname(self.socket_path) if not os.path.exists(socket_dir): - error_msg = f"Socket directory {socket_dir} does not exist" - syslog.syslog(syslog.LOG_ERR, error_msg) - raise ConnectionError(error_msg) + raise ConnectionError(f"Socket directory {socket_dir} does not exist") + + if not os.path.exists(self.socket_path): + if create_if_missing: + syslog.syslog(syslog.LOG_INFO, f"Socket file {self.socket_path} does not exist, creating it.") + self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) + self.sock.bind(self.socket_path) + os.chmod(self.socket_path, 0o600) + return + else: + raise ConnectionError(f"Socket file {self.socket_path} not found") + + try: + socket_stat = os.stat(self.socket_path) + permissions = oct(socket_stat.st_mode & 0o777) + syslog.syslog(syslog.LOG_INFO, f"Socket permissions: {permissions}") + + if socket_stat.st_mode & 0o777 != 0o600: + raise PermissionError(f"Insecure socket file permissions: {permissions}") + + if socket_stat.st_uid != 0: + raise PermissionError(f"Socket not owned by root: {self.socket_path}") + + except FileNotFoundError: + raise ConnectionError(f"Socket file {self.socket_path} not found") def connect(self) -> None: - """Establishes socket connection with improved error handling.""" + """ + Establishes a Unix domain socket connection with improved error handling. + + This function attempts to establish a connection to the memory statistics service via + a Unix domain socket. It will retry the connection up to `Config.MAX_RETRIES` times with + a delay of `Config.RETRY_DELAY` seconds between attempts. + + Raises: + ConnectionError: If the connection fails after the maximum number of retries. + """ retries = 0 - last_error = None + + syslog.syslog(syslog.LOG_INFO, f"Attempting socket connection from PID {os.getpid()}") while retries < Config.MAX_RETRIES: try: @@ -170,47 +291,82 @@ def connect(self) -> None: syslog.syslog(syslog.LOG_INFO, "Successfully connected to memory statistics service") return except socket.error as e: - last_error = e retries += 1 if retries < Config.MAX_RETRIES: - syslog.syslog(syslog.LOG_WARNING, - f"Failed to connect to socket (attempt {retries}/{Config.MAX_RETRIES}): {str(e)}") + syslog.syslog( + syslog.LOG_WARNING, + f"Socket connection attempt {retries} failed: {str(e)}" + ) time.sleep(Config.RETRY_DELAY) - self.close() + else: + raise ConnectionError( + f"Failed to connect to memory statistics service after {Config.MAX_RETRIES} attempts: {str(e)}" + ) from e - error_msg = ( - f"Failed to connect to memory statistics service after {Config.MAX_RETRIES} " - f"attempts. Last error: {str(last_error)}. " - f"Please verify that the service is running and socket file exists at {self.socket_path}" - ) - syslog.syslog(syslog.LOG_ERR, error_msg) - raise ConnectionError(error_msg) + def receive_all(self, expected_size: int, max_attempts: int = 100) -> str: + """ + Receives all data from the socket until the expected size is reached. + + This function ensures that the complete data is received from the socket. If the expected + size is not reached within the specified number of attempts, it raises an exception. It also + handles timeout and socket errors. + + Args: + expected_size (int): The expected size of the data to receive. + max_attempts (int): Maximum number of attempts to receive the data (default is 100). - def receive_all(self) -> str: - """Receives all data with improved error handling.""" + Returns: + str: The received data as a string. + + Raises: + ConnectionError: If the socket operation times out or encounters an error. + """ if not self.sock: raise ConnectionError("No active socket connection") - chunks = [] - while True: + data = b"" + attempts = 0 + + while len(data) < expected_size and attempts < max_attempts: try: - chunk = self.sock.recv(Config.BUFFER_SIZE) + chunk = self.sock.recv(expected_size - len(data)) if not chunk: break - chunks.append(chunk) - except socket.timeout: + data += chunk + except socket.timeout as e: error_msg = f"Socket operation timed out after {Config.SOCKET_TIMEOUT} seconds" syslog.syslog(syslog.LOG_ERR, error_msg) - raise ConnectionError(error_msg) + raise ConnectionError(error_msg) from e except socket.error as e: error_msg = f"Socket error during receive: {str(e)}" syslog.syslog(syslog.LOG_ERR, error_msg) - raise ConnectionError(error_msg) + raise ConnectionError(error_msg) from e + + attempts += 1 - return b''.join(chunks).decode('utf-8') + if len(data) < expected_size: + syslog.syslog(syslog.LOG_WARNING, "Received incomplete data, possible socket issue.") + + try: + return data.decode('utf-8') + except UnicodeDecodeError as e: + error_msg = f"Failed to decode received data: {str(e)}" + syslog.syslog(syslog.LOG_ERR, error_msg) + raise ConnectionError(error_msg) from e def send(self, data: str) -> None: - """Sends data with improved error handling.""" + """ + Sends data over the socket with improved error handling. + + This function sends data through the socket. It raises a `ConnectionError` if the data + cannot be sent due to socket issues. + + Args: + data (str): The data to send. + + Raises: + ConnectionError: If there is an error while sending the data. + """ if not self.sock: raise ConnectionError("No active socket connection") @@ -219,10 +375,17 @@ def send(self, data: str) -> None: except socket.error as e: error_msg = f"Failed to send data: {str(e)}" syslog.syslog(syslog.LOG_ERR, error_msg) - raise ConnectionError(error_msg) + raise ConnectionError(error_msg) from e def close(self) -> None: - """Closes the socket connection safely.""" + """ + Closes the socket connection safely. + + This function safely closes the socket connection and ensures that the socket is properly cleaned up. + + Raises: + Exception: If there is an error closing the socket. + """ if self.sock: try: self.sock.close() @@ -232,35 +395,79 @@ def close(self) -> None: self.sock = None -def send_data(command: str, data: Dict[str, Any], quiet: bool = False) -> Dict2Obj: +class ResourceManager: """ - Sends a command and data to the memory statistics service. + A class that manages the cleanup of resources during shutdown. + + This class ensures that the necessary resources, including database connectors and socket connections, are + properly cleaned up to avoid resource leaks and maintain system integrity. + + Methods: + __init__(): Initializes the resource manager. + cleanup(): Performs cleanup of resources, including database connectors and socket managers. + """ + + def __init__(self): + self.db_connector = None + self.socket_manager = None + + def cleanup(self): + """ + Performs cleanup of resources during shutdown. + + This function cleans up all resources, including database connectors and socket connections, + ensuring proper shutdown. + + Raises: + None + """ + if self.db_connector: + del self.db_connector + if self.socket_manager: + self.socket_manager.close() + syslog.syslog(syslog.LOG_INFO, "Successfully cleaned up resources during shutdown") + + +def send_data(command: str, data: Dict[str, Any], quiet: bool = False) -> Dict2Obj: + """Sends a command and data to the memory statistics service. Time format for statistics retrieval are given below. + - Relative time formats: + - 'X days ago', 'X hours ago', 'X minutes ago' + - 'yesterday', 'today' + - Specific times and dates: + - 'now' + - 'July 23', 'July 23, 2024', '2 November 2024' + - '7/24', '1/2' + - Time expressions: + - '2 am', '3:15 pm' + - 'Aug 01 06:43:40', 'July 1 3:00:00' + - Named months: + - 'jan', 'feb', 'march', 'september', etc. + - Full month names: 'January', 'February', 'March', etc. + - ISO 8601 format (e.g., '2024-07-01T15:00:00') - - Relative time formats: - - 'X days ago', 'X hours ago', 'X minutes ago' - - 'yesterday', 'today' - - Specific times and dates: - - 'now' - - 'July 23', 'July 23, 2024', '2 November 2024' - - '7/24', '1/2' - - Time expressions: - - '2 am', '3:15 pm' - - 'Aug 01 06:43:40', 'July 1 3:00:00' - - Named months: - - 'jan', 'feb', 'march', 'september', etc. - - Full month names: 'January', 'February', 'March', etc. - - ISO 8601 format (e.g., '2024-07-01T15:00:00') + Args: + command (str): The command to send to the service. + data (Dict[str, Any]): The data payload to send with the command. + quiet (bool): If True, suppresses error messages. Defaults to False. + + Returns: + Dict2Obj: The response from the service as an object. + + Raises: + ConnectionError: If there is an issue with the socket connection. + ValueError: If the response cannot be parsed or is invalid. + DatabaseError: If the service returns an error status. """ socket_manager = SocketManager() try: socket_manager.connect() request = {"command": command, "data": data} - socket_manager.sock.sendall(json.dumps(request).encode('utf-8')) + socket_manager.send(json.dumps(request)) - response = socket_manager.receive_all() + response = socket_manager.receive_all(expected_size=Config.BUFFER_SIZE) if not response: raise ConnectionError("No response received from memory statistics service") @@ -269,7 +476,7 @@ def send_data(command: str, data: Dict[str, Any], quiet: bool = False) -> Dict2O except json.JSONDecodeError as e: error_msg = f"Failed to parse server response: {str(e)}" syslog.syslog(syslog.LOG_ERR, error_msg) - raise ValueError(error_msg) + raise ValueError(error_msg) from e if not isinstance(jdata, dict): raise ValueError("Invalid response format from server") @@ -277,7 +484,7 @@ def send_data(command: str, data: Dict[str, Any], quiet: bool = False) -> Dict2O response_obj = Dict2Obj(jdata) if not getattr(response_obj, 'status', True): error_msg = getattr(response_obj, 'msg', 'Unknown error occurred') - raise RuntimeError(error_msg) + raise DatabaseError(error_msg) from None return response_obj @@ -290,14 +497,29 @@ def send_data(command: str, data: Dict[str, Any], quiet: bool = False) -> Dict2O def format_field_value(field_name: str, value: str) -> str: - """Formats configuration field values for display.""" + """Formats configuration field values for display. + + Args: + field_name (str): The name of the configuration field. + value (str): The value of the configuration field. + + Returns: + str: The formatted value for display. + """ if field_name == "enabled": return "True" if value.lower() == "true" else "False" return value if value != "Unknown" else "Not configured" def display_config(db_connector: SonicDBConnector) -> None: - """Displays memory statistics configuration.""" + """Displays memory statistics configuration. + + Args: + db_connector (SonicDBConnector): The database connector to retrieve configuration. + + Raises: + click.ClickException: If there is an error retrieving the configuration. + """ try: config = db_connector.get_memory_statistics_config() enabled = format_field_value("enabled", config.get("enabled", "Unknown")) @@ -341,7 +563,16 @@ def show(): help='Show statistics for specific metric (e.g., total_memory, used_memory)' ) def show_statistics(from_time: str, to_time: str, select_metric: str): - """Display memory statistics.""" + """Display memory statistics. + + Args: + from_time (str): Start time for memory statistics (e.g., "15 hours ago", "7 days ago", "ISO Format"). + to_time (str): End time for memory statistics (e.g., "now", "ISO Format"). + select_metric (str): Specific metric to show statistics for (e.g., total_memory, used_memory). + + Raises: + Exception: If there is an error retrieving or displaying the statistics. + """ try: request_data = { "type": "system", @@ -370,7 +601,11 @@ def show_statistics(from_time: str, to_time: str, select_metric: str): @show.command(name="memory-stats-config") def show_configuration(): - """Display memory statistics configuration.""" + """Display memory statistics configuration. + + Raises: + Exception: If there is an error retrieving or displaying the configuration. + """ try: db_connector = SonicDBConnector() display_config(db_connector) @@ -379,32 +614,20 @@ def show_configuration(): sys.exit(1) -def cleanup_resources(): - """Performs cleanup of resources before shutdown.""" - try: - if hasattr(cleanup_resources, 'db_connector'): - del cleanup_resources.db_connector - - if hasattr(cleanup_resources, 'socket_manager'): - cleanup_resources.socket_manager.close() - - syslog.syslog(syslog.LOG_INFO, "Successfully cleaned up resources during shutdown") - except Exception as e: - syslog.syslog(syslog.LOG_ERR, f"Error during cleanup: {str(e)}") - - -def shutdown_handler(signum: int, frame) -> None: - """ - Signal handler for graceful shutdown. - Handles SIGTERM signal with proper resource cleanup. +def shutdown_handler(signum: int, frame, resource_manager: ResourceManager) -> None: + """Signal handler for graceful shutdown. Args: - signum: Signal number - frame: Current stack frame + signum (int): Signal number. + frame: Current stack frame. + resource_manager (ResourceManager): ResourceManager instance for cleanup. + + Raises: + Exception: If there is an error during shutdown. """ try: syslog.syslog(syslog.LOG_INFO, "Received SIGTERM signal, initiating graceful shutdown...") - cleanup_resources() + resource_manager.cleanup() click.echo("\nShutting down gracefully...") sys.exit(0) except Exception as e: @@ -413,18 +636,20 @@ def shutdown_handler(signum: int, frame) -> None: def main(): - """Main entry point with enhanced error handling and shutdown management.""" - try: - signal.signal(signal.SIGTERM, shutdown_handler) + """Main entry point with enhanced error handling and shutdown management. - cleanup_resources.db_connector = None - cleanup_resources.socket_manager = None + Raises: + Exception: If there is a fatal error during execution. + """ + resource_manager = ResourceManager() + try: + signal.signal(signal.SIGTERM, lambda signum, frame: shutdown_handler(signum, frame, resource_manager)) cli() except Exception as e: syslog.syslog(syslog.LOG_ERR, f"Fatal error in main: {str(e)}") click.echo(f"Error: {str(e)}", err=True) - cleanup_resources() + resource_manager.cleanup() sys.exit(1) diff --git a/tests/config_memory_statistics_test.py b/tests/config_memory_statistics_test.py index 1b91b30f76..5f0db136ed 100644 --- a/tests/config_memory_statistics_test.py +++ b/tests/config_memory_statistics_test.py @@ -1,10 +1,13 @@ +# Standard library imports +import os import subprocess import syslog -from unittest.mock import Mock, patch +# Third-party imports import pytest from click.testing import CliRunner +# Local imports from config.memory_statistics import ( cli, log_to_syslog, @@ -18,6 +21,9 @@ update_memory_statistics_status, ) +# Testing utilities +from unittest.mock import Mock, patch + @pytest.fixture def mock_db(): @@ -57,16 +63,26 @@ def test_get_db_connection(self, mock_db): db1 = MemoryStatisticsDB.get_db() db2 = MemoryStatisticsDB.get_db() + assert db1 is db2 mock_db.connect.assert_called_once() + def test_connect_db_failure(self, mock_db): + """Test handling of database connection failure.""" + mock_db.connect.side_effect = RuntimeError("Connection failed") + MemoryStatisticsDB._instance = None + MemoryStatisticsDB._db = None + + with pytest.raises(RuntimeError, match="Database connection unavailable"): + MemoryStatisticsDB.get_db() + class TestUpdateMemoryStatisticsStatus: """Tests for update_memory_statistics_status function""" def test_successful_enable(self, mock_db): """Test successful status update to enable.""" - success, error = update_memory_statistics_status("true") + success, error = update_memory_statistics_status(True) assert success is True assert error is None mock_db.mod_entry.assert_called_once_with( @@ -77,7 +93,7 @@ def test_successful_enable(self, mock_db): def test_successful_disable(self, mock_db): """Test successful status update to disable.""" - success, error = update_memory_statistics_status("false") + success, error = update_memory_statistics_status(False) assert success is True assert error is None mock_db.mod_entry.assert_called_once_with( @@ -89,11 +105,19 @@ def test_successful_disable(self, mock_db): def test_database_error(self, mock_db): """Test handling of database errors.""" mock_db.mod_entry.side_effect = Exception("DB Error") - success, error = update_memory_statistics_status("true") + success, error = update_memory_statistics_status(True) assert success is False - assert "Error updating memory statistics status" in error + assert "Unexpected error updating memory statistics status" in error assert "DB Error" in error + def test_specific_exceptions(self, mock_db): + """Test handling of specific exceptions.""" + for exception in [KeyError, ConnectionError, RuntimeError]: + mock_db.mod_entry.side_effect = exception("Specific error") + success, error = update_memory_statistics_status(True) + assert success is False + assert "Specific error" in error + class TestMemoryStatisticsEnable: def test_enable_success(self, cli_runner, mock_db): @@ -158,7 +182,8 @@ def test_valid_sampling_intervals(self, interval, cli_runner, mock_db): SAMPLING_INTERVAL_MIN - 1, SAMPLING_INTERVAL_MAX + 1, 0, - -1 + -1, + 256 ]) def test_invalid_sampling_intervals(self, interval, cli_runner, mock_db): """Test handling of invalid sampling intervals.""" @@ -172,6 +197,20 @@ def test_sampling_interval_db_error(self, cli_runner, mock_db): result = cli_runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', '5']) assert "Error" in result.output + @pytest.mark.parametrize("exception", [ + KeyError("Key not found"), + ConnectionError("Connection failed"), + ValueError("Invalid value"), + RuntimeError("Runtime error") + ]) + def test_sampling_interval_specific_errors(self, exception, cli_runner, mock_db): + """Test handling of specific errors when setting sampling interval.""" + mock_db.mod_entry.side_effect = exception + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', '5']) + assert result.exit_code == 0 + assert "Error" in result.output + assert str(exception) in result.output + class TestRetentionPeriod: @pytest.mark.parametrize("period", [ @@ -194,7 +233,8 @@ def test_valid_retention_periods(self, period, cli_runner, mock_db): RETENTION_PERIOD_MIN - 1, RETENTION_PERIOD_MAX + 1, 0, - -1 + -1, + 256 ]) def test_invalid_retention_periods(self, period, cli_runner, mock_db): """Test handling of invalid retention periods.""" @@ -208,6 +248,20 @@ def test_db_error(self, cli_runner, mock_db): result = cli_runner.invoke(cli, ['config', 'memory-stats', 'retention-period', '15']) assert "Error" in result.output + @pytest.mark.parametrize("exception", [ + KeyError("Key not found"), + ConnectionError("Connection failed"), + ValueError("Invalid value"), + RuntimeError("Runtime error") + ]) + def test_retention_period_specific_errors(self, exception, cli_runner, mock_db): + """Test handling of specific errors when setting retention period.""" + mock_db.mod_entry.side_effect = exception + result = cli_runner.invoke(cli, ['config', 'memory-stats', 'retention-period', '15']) + assert result.exit_code == 0 + assert "Error" in result.output + assert str(exception) in result.output + class TestSyslogLogging: @pytest.mark.parametrize("log_level,expected_level", [ @@ -216,40 +270,22 @@ class TestSyslogLogging: ]) def test_syslog_logging(self, log_level, expected_level): """Test syslog logging functionality.""" - with patch('syslog.syslog') as mock_syslog, \ - patch('syslog.openlog') as mock_openlog: - + with patch('syslog.syslog') as mock_syslog: log_to_syslog("Test message", expected_level) - - mock_openlog.assert_called_once_with( - "memory_statistics", - syslog.LOG_PID | syslog.LOG_CONS, - syslog.LOG_USER - ) - mock_syslog.assert_called_once_with(expected_level, "Test message") def test_syslog_logging_default_level(self): """Test syslog logging with default log level.""" - with patch('syslog.syslog') as mock_syslog, \ - patch('syslog.openlog') as _: + with patch('syslog.syslog') as mock_syslog: log_to_syslog("Test message") mock_syslog.assert_called_once_with(syslog.LOG_INFO, "Test message") - -def test_main_execution(): - """Test the main execution block of the script.""" - with patch('config.memory_statistics.cli') as mock_cli: - module_code = compile( - 'if __name__ == "__main__": cli()', - 'memory_statistics.py', - 'exec' - ) - - namespace = {'__name__': '__main__', 'cli': mock_cli} - exec(module_code, namespace) - - mock_cli.assert_called_once() + def test_syslog_logging_error(self): + """Test syslog logging error handling.""" + with patch('syslog.syslog', side_effect=Exception("Syslog error")), \ + patch('click.echo') as mock_echo: + log_to_syslog("Test message") + mock_echo.assert_called_once_with("Failed to log to syslog: Syslog error", err=True) def test_main_cli_integration(): @@ -270,3 +306,52 @@ def test_script_execution(): result = subprocess.run(["python3", "config/memory_statistics.py"], capture_output=True) assert result.returncode == 0 + + +def test_syslog_closelog(): + """Test that syslog.closelog is called when the script exits.""" + with patch('syslog.closelog') as mock_closelog: + module_code = compile( + ''' +try: + cli() +finally: + syslog.closelog() + ''', + 'memory_statistics.py', + 'exec' + ) + + namespace = { + '__name__': '__main__', + 'cli': Mock(), + 'syslog': Mock(closelog=mock_closelog) + } + + exec(module_code, namespace) + + mock_closelog.assert_called_once() + + +def test_main_execution(): + """Test the script's main execution block including the try-finally structure.""" + script_path = os.path.abspath("config/memory_statistics.py") + + with patch('syslog.closelog') as mock_closelog, \ + patch('click.group', return_value=Mock()) as mock_group: + + namespace = { + '__name__': '__main__', + 'syslog': Mock(closelog=mock_closelog), + 'click': Mock(group=mock_group), + } + + with open(script_path, 'r') as file: + script_content = file.read() + + compiled_code = compile(script_content, script_path, 'exec') + + exec(compiled_code, namespace) + + mock_closelog.assert_called_once() + mock_group.assert_called() diff --git a/tests/show_memory_statistics_test.py b/tests/show_memory_statistics_test.py index 621854556d..d47ccbe1b4 100644 --- a/tests/show_memory_statistics_test.py +++ b/tests/show_memory_statistics_test.py @@ -1,29 +1,33 @@ +# Standard library imports import json import os -import signal import socket +import signal import syslog import unittest from unittest.mock import MagicMock, Mock, patch +# Third-party library imports import click -from click.testing import CliRunner import pytest +from click.testing import CliRunner +# Local imports from show.memory_statistics import ( Config, ConnectionError, + DatabaseError, Dict2Obj, SonicDBConnector, SocketManager, - cleanup_resources, - display_config, - format_field_value, - main, + ResourceManager, send_data, - show_configuration, + format_field_value, + display_config, show_statistics, + show_configuration, shutdown_handler, + main, ) @@ -106,6 +110,10 @@ def test_empty_structures(self): self.assertEqual(Dict2Obj({}).to_dict(), {}) self.assertEqual(Dict2Obj([]).to_dict(), []) + def test_dict2obj_invalid_input(self): + with pytest.raises(ValueError): + Dict2Obj("invalid_input") + def test_complex_nested_structure(self): """Test conversion of complex nested structures""" test_dict = { @@ -123,190 +131,6 @@ def test_complex_nested_structure(self): self.assertEqual(obj.level1.level2.level3.list[2].nested, "value") -class TestSonicDBConnector(unittest.TestCase): - """Tests for SonicDBConnector class""" - - @patch('show.memory_statistics.ConfigDBConnector') - def setUp(self, mock_config_db): - self.mock_config_db = mock_config_db - self.connector = SonicDBConnector() - self.mock_config_db.reset_mock() - - def test_successful_connection(self): - """Test successful database connection""" - self.mock_config_db.return_value.connect.return_value = None - self.connector.connect_with_retry() - self.mock_config_db.return_value.connect.assert_called_once() - - @patch('time.sleep') - def test_connection_retry(self, mock_sleep): - """Test connection retry mechanism""" - self.mock_config_db.return_value.connect.side_effect = [ - Exception("Connection failed"), - None - ] - self.connector.connect_with_retry(max_retries=2, retry_delay=0.1) - self.assertEqual(mock_sleep.call_count, 1) - self.assertEqual(self.mock_config_db.return_value.connect.call_count, 2) - - def test_connection_failure(self): - """Test connection failure after max retries""" - self.mock_config_db.return_value.connect.side_effect = Exception("Connection failed") - with self.assertRaises(ConnectionError): - self.connector.connect_with_retry(max_retries=1) - - def test_get_memory_statistics_config_success(self): - """Test successful config retrieval""" - expected_config = { - "memory_statistics": { - "enabled": "true", - "sampling_interval": "10", - "retention_period": "30" - } - } - self.mock_config_db.return_value.get_table.return_value = expected_config - result = self.connector.get_memory_statistics_config() - self.assertEqual(result["enabled"], "true") - self.assertEqual(result["sampling_interval"], "10") - self.assertEqual(result["retention_period"], "30") - - def test_get_memory_statistics_config_default(self): - """Test default config when table is empty""" - self.mock_config_db.return_value.get_table.return_value = {} - result = self.connector.get_memory_statistics_config() - self.assertEqual(result, Config.DEFAULT_CONFIG) - - def test_invalid_config_format(self): - """Test handling of invalid configuration format""" - self.mock_config_db.return_value.get_table.return_value = { - "memory_statistics": "invalid_string_instead_of_dict" - } - result = self.connector.get_memory_statistics_config() - self.assertEqual(result, Config.DEFAULT_CONFIG) - - def test_partial_config(self): - """Test handling of partial configuration""" - self.mock_config_db.return_value.get_table.return_value = { - "memory_statistics": { - "enabled": "true" - # missing other fields - } - } - result = self.connector.get_memory_statistics_config() - self.assertEqual(result["enabled"], "true") - self.assertEqual(result["sampling_interval"], "5") # default value - self.assertEqual(result["retention_period"], "15") # default value - - -class TestSocketManager(unittest.TestCase): - """Tests for SocketManager class""" - - def setUp(self): - self.test_socket_path = "/tmp/test_socket" - os.makedirs(os.path.dirname(self.test_socket_path), exist_ok=True) - self.socket_manager = SocketManager(self.test_socket_path) - - @patch('socket.socket') - def test_successful_connection(self, mock_socket): - """Test successful socket connection""" - mock_sock = Mock() - mock_socket.return_value = mock_sock - self.socket_manager.connect() - mock_sock.connect.assert_called_once_with(self.test_socket_path) - - @patch('socket.socket') - @patch('time.sleep') - def test_connection_retry(self, mock_sleep, mock_socket): - """Test socket connection retry mechanism""" - mock_sock = Mock() - mock_sock.connect.side_effect = [socket.error, None] - mock_socket.return_value = mock_sock - self.socket_manager.connect() - self.assertEqual(mock_sock.connect.call_count, 2) - - @patch('socket.socket') - def test_receive_all(self, mock_socket): - """Test receiving data from socket""" - mock_sock = Mock() - mock_sock.recv.side_effect = [b'test', b''] - mock_socket.return_value = mock_sock - self.socket_manager.sock = mock_sock - result = self.socket_manager.receive_all() - self.assertEqual(result, 'test') - - @patch('socket.socket') - def test_send_data(self, mock_socket): - """Test sending data through socket""" - mock_sock = Mock() - mock_socket.return_value = mock_sock - self.socket_manager.sock = mock_sock - self.socket_manager.send("test_data") - mock_sock.sendall.assert_called_once_with(b'test_data') - - def test_close_connection(self): - """Test closing socket connection""" - mock_sock = Mock() - self.socket_manager.sock = mock_sock - self.socket_manager.close() - mock_sock.close.assert_called_once() - self.assertIsNone(self.socket_manager.sock) - - def test_invalid_socket_path(self): - """Test socket creation with invalid path""" - with self.assertRaises(ConnectionError): - SocketManager("/nonexistent/path/socket") - - @patch('socket.socket') - def test_connection_max_retries_exceeded(self, mock_socket): - """Test connection failure after max retries""" - mock_sock = Mock() - mock_sock.connect.side_effect = socket.error("Connection failed") - mock_socket.return_value = mock_sock - - with self.assertRaises(ConnectionError) as ctx: - self.socket_manager.connect() - self.assertIn("Failed to connect to memory statistics service", str(ctx.exception)) - - @patch('socket.socket') - def test_receive_timeout(self, mock_socket): - """Test socket timeout during receive""" - mock_sock = Mock() - mock_sock.recv.side_effect = socket.timeout - self.socket_manager.sock = mock_sock - - with self.assertRaises(ConnectionError) as context: - self.socket_manager.receive_all() - self.assertIn("timed out", str(context.exception)) - - @patch('socket.socket') - def test_receive_with_socket_error(self, mock_socket): - """Test receive with socket error""" - mock_sock = Mock() - mock_sock.recv.side_effect = socket.error("Receive error") - self.socket_manager.sock = mock_sock - - with self.assertRaises(ConnectionError) as ctx: - self.socket_manager.receive_all() - self.assertIn("Socket error during receive", str(ctx.exception)) - - @patch('socket.socket') - def test_send_without_connection(self, mock_socket): - """Test sending data without an active connection""" - self.socket_manager.sock = None - with self.assertRaises(ConnectionError) as context: - self.socket_manager.send("test") - self.assertIn("No active socket connection", str(context.exception)) - - @patch('socket.socket') - def test_multiple_chunk_receive(self, mock_socket): - """Test receiving multiple chunks of data""" - mock_sock = Mock() - mock_sock.recv.side_effect = [b'chunk1', b'chunk2', b'chunk3', b''] - self.socket_manager.sock = mock_sock - result = self.socket_manager.receive_all() - self.assertEqual(result, 'chunk1chunk2chunk3') - - class TestCLICommands(unittest.TestCase): """Tests for CLI commands""" @@ -372,59 +196,6 @@ def test_show_config_error(self, mock_db): self.assertIn("Error", result.output) -class TestErrorHandling(unittest.TestCase): - """Tests for error handling""" - - def test_cleanup_resources(self): - """Test resource cleanup""" - mock_db = Mock() - mock_socket = Mock() - cleanup_resources.db_connector = mock_db - cleanup_resources.socket_manager = mock_socket - cleanup_resources() - self.assertFalse(hasattr(cleanup_resources, 'db_connector')) - mock_socket.close.assert_called_once() - - @patch('sys.exit') - def test_shutdown_handler(self, mock_exit): - """Test shutdown handler""" - shutdown_handler(None, None) - mock_exit.assert_called_once_with(0) - - @patch('syslog.syslog') - def test_cleanup_with_exceptions(self, mock_syslog): - """Test cleanup with exceptions""" - mock_socket = Mock() - mock_socket.close.side_effect = Exception("Cleanup failed") - cleanup_resources.socket_manager = mock_socket - - cleanup_resources() - mock_syslog.assert_any_call(syslog.LOG_ERR, "Error during cleanup: Cleanup failed") - - @patch('syslog.syslog') - def test_cleanup_with_missing_attributes(self, mock_syslog): - """Test cleanup when attributes don't exist""" - # Ensure attributes don't exist - if hasattr(cleanup_resources, 'db_connector'): - delattr(cleanup_resources, 'db_connector') - if hasattr(cleanup_resources, 'socket_manager'): - delattr(cleanup_resources, 'socket_manager') - - cleanup_resources() - mock_syslog.assert_any_call(syslog.LOG_INFO, "Successfully cleaned up resources during shutdown") - - @patch('sys.exit') - @patch('syslog.syslog') - def test_shutdown_handler_cleanup_error(self, mock_syslog, mock_exit): - """Test shutdown handler with cleanup error""" - @patch('show.memory_statistics.cleanup_resources', side_effect=Exception("Cleanup Error")) - def test(mock_cleanup): - shutdown_handler(signal.SIGTERM, None) - mock_syslog.assert_any_call(syslog.LOG_ERR, "Error during shutdown: Cleanup Error") - mock_exit.assert_called_once_with(1) - test() - - class TestHelperFunctions(unittest.TestCase): """Tests for helper functions""" @@ -435,19 +206,26 @@ def test_format_field_value(self): self.assertEqual(format_field_value("retention_period", "15"), "15") self.assertEqual(format_field_value("sampling_interval", "Unknown"), "Not configured") + def test_resource_manager_cleanup_no_resources(self): + """Test ResourceManager cleanup when no resources exist""" + resource_manager = ResourceManager() + resource_manager.cleanup() -class TestSendData(unittest.TestCase): - """Tests for send_data function""" + def test_shutdown_handler_cleanup(self): + """Test shutdown_handler performs cleanup""" + resource_manager = ResourceManager() + resource_manager.db_connector = MagicMock() + resource_manager.socket_manager = MagicMock() - @patch('show.memory_statistics.SocketManager') - def test_send_data_invalid_response(self, mock_socket_manager): - """Test send_data with invalid JSON response""" - mock_instance = Mock() - mock_socket_manager.return_value = mock_instance - mock_instance.receive_all.return_value = "invalid json" + with pytest.raises(SystemExit) as exc_info: + shutdown_handler(signal.SIGTERM, None, resource_manager) - with self.assertRaises(ValueError): - send_data("test_command", {}) + resource_manager.socket_manager.close.assert_called_once() + assert exc_info.value.code == 0 + + +class TestSendData(unittest.TestCase): + """Tests for send_data function""" @patch('show.memory_statistics.SocketManager') def test_send_data_non_dict_response(self, mock_socket_manager): @@ -488,35 +266,6 @@ def test_response_without_status_field(self, mock_socket_manager): self.assertTrue(getattr(result, 'status', True)) self.assertEqual(result.data, "test data") - @patch('show.memory_statistics.SocketManager') - def test_failed_response_with_error_message(self, mock_socket_manager): - """Test response with status False and error message""" - mock_instance = Mock() - mock_socket_manager.return_value = mock_instance - response_data = { - "status": False, - "msg": "Operation failed" - } - mock_instance.receive_all.return_value = json.dumps(response_data) - - with self.assertRaises(RuntimeError) as context: - send_data("test_command", {}) - self.assertEqual(str(context.exception), "Operation failed") - - @patch('show.memory_statistics.SocketManager') - def test_failed_response_without_message(self, mock_socket_manager): - """Test response with status False but no error message""" - mock_instance = Mock() - mock_socket_manager.return_value = mock_instance - response_data = { - "status": False - } - mock_instance.receive_all.return_value = json.dumps(response_data) - - with self.assertRaises(RuntimeError) as context: - send_data("test_command", {}) - self.assertEqual(str(context.exception), "Unknown error occurred") - @patch('show.memory_statistics.SocketManager') def test_complex_response_object_conversion(self, mock_socket_manager): """Test conversion of complex response object""" @@ -540,6 +289,36 @@ def test_complex_response_object_conversion(self, mock_socket_manager): self.assertEqual(result.data.metrics[1].value, 50) self.assertEqual(result.data.timestamp, "2024-01-01") + @patch('show.memory_statistics.SocketManager') + def test_send_data_json_decode_error(self, mock_socket_manager): + """Test send_data handles JSON decode errors""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + mock_instance.receive_all.return_value = "invalid json" + + with self.assertRaises(ValueError): + send_data("test_command", {}) + + @patch('show.memory_statistics.SocketManager') + def test_send_data_invalid_response_format(self, mock_socket_manager): + """Test send_data handles invalid response format""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + mock_instance.receive_all.return_value = json.dumps(["not a dict"]) + + with self.assertRaises(ValueError): + send_data("test_command", {}) + + @patch("show.memory_statistics.SocketManager") + def test_send_data_invalid_response(self, mock_socket_manager): + """Test send_data with invalid JSON response""" + mock_instance = Mock() + mock_socket_manager.return_value = mock_instance + mock_instance.receive_all.return_value = "invalid_json" + + with self.assertRaises(ValueError): + send_data("test_command", {}) + class TestDisplayConfig(unittest.TestCase): """Tests for display_config function""" @@ -565,28 +344,32 @@ def test_display_config_error(self): with pytest.raises(click.ClickException): display_config(mock_connector) + @patch('show.memory_statistics.ConfigDBConnector') + def test_get_memory_statistics_config_invalid_data(self, mock_connector): + """Test get_memory_statistics_config with invalid data""" + mock_instance = mock_connector.return_value + mock_instance.connect = MagicMock() + mock_instance.get_table = MagicMock(return_value={"invalid_key": "invalid_value"}) -class TestMainFunction(unittest.TestCase): - """Tests for main function""" + db_connector = SonicDBConnector() + config = db_connector.get_memory_statistics_config() + assert config == Config.DEFAULT_CONFIG - @patch('signal.signal') - @patch('show.memory_statistics.cli') - def test_successful_execution(self, mock_cli, mock_signal): - """Test successful execution of main function""" - main() - mock_signal.assert_called_once_with(signal.SIGTERM, shutdown_handler) - mock_cli.assert_called_once() + @patch('show.memory_statistics.click.echo') + @patch('show.memory_statistics.SonicDBConnector') + def test_show_configuration_database_error(self, mock_sonic_db, mock_echo): + """Test show_configuration handles database errors""" + mock_instance = mock_sonic_db.return_value + mock_instance.get_memory_statistics_config.side_effect = Exception("DB error") - @patch('signal.signal') - @patch('show.memory_statistics.cli') - @patch('show.memory_statistics.cleanup_resources') - def test_main_with_exception(self, mock_cleanup, mock_cli, mock_signal): - """Test main function with exception""" - mock_cli.side_effect = Exception("CLI error") + runner = CliRunner() + with patch('show.memory_statistics.sys.exit') as mock_exit: + runner.invoke(show_configuration, catch_exceptions=False) - with self.assertRaises(SystemExit): - main() - mock_cleanup.assert_called_once() + mock_echo.assert_any_call("Error: Failed to retrieve configuration: DB error", err=True) + + assert mock_exit.call_count >= 1 + mock_exit.assert_any_call(1) class TestFormatFieldValue: @@ -619,42 +402,6 @@ def test_dict2obj_empty_list(self): obj = Dict2Obj([]) self.assertEqual(obj.to_dict(), []) - @patch('socket.socket') - def test_socket_receive_timeout(self, mock_socket): - """Test socket timeout during receive (lines 137-140)""" - manager = SocketManager() - mock_socket.return_value.recv.side_effect = socket.timeout - manager.sock = mock_socket.return_value - - with self.assertRaises(ConnectionError): - manager.receive_all() - - @patch('socket.socket') - def test_socket_send_error(self, mock_socket): - """Test socket send error (line 166)""" - manager = SocketManager() - mock_socket.return_value.sendall.side_effect = socket.error("Send failed") - manager.sock = mock_socket.return_value - - with self.assertRaises(ConnectionError): - manager.send("test data") - - @patch('syslog.syslog') - def test_cleanup_resources_error(self, mock_syslog): - """Test cleanup resources error handling (lines 220-223)""" - cleanup_resources.socket_manager = MagicMock() - cleanup_resources.socket_manager.close.side_effect = Exception("Cleanup failed") - - cleanup_resources() - mock_syslog.assert_called_with(syslog.LOG_ERR, "Error during cleanup: Cleanup failed") - - @patch('show.memory_statistics.send_data') - def test_show_statistics_invalid_data(self, mock_send): - """Test show statistics with invalid data format (line 247)""" - mock_send.return_value = Dict2Obj(["invalid"]) - result = self.cli_runner.invoke(show_statistics, []) - self.assertIn("Error: Invalid data format received", result.output) - @patch('show.memory_statistics.SonicDBConnector') def test_show_configuration_error(self, mock_db): """Test show configuration error (line 302)""" @@ -689,19 +436,6 @@ def test_dict2obj_with_nested_data(self): self.assertEqual(obj.list[1].d, 2) self.assertEqual(obj.to_dict(), data) - @patch('show.memory_statistics.SocketManager') - def test_socket_manager_close_exception(self, mock_socket_manager): - """Test SocketManager close handles exceptions gracefully""" - mock_socket_instance = mock_socket_manager.return_value - mock_socket_instance.close.side_effect = Exception("Close error") - - manager = SocketManager() - manager.sock = mock_socket_instance - - with patch('syslog.syslog') as mock_syslog: - manager.close() - mock_syslog.assert_any_call(4, "Error closing socket: Close error") - def test_dict2obj_repr(self): """Test the __repr__ method of Dict2Obj""" data = {'a': 1, 'b': {'c': 2}} @@ -728,5 +462,269 @@ def test_send_data_no_response(self): self.assertIn("No response received", str(context.exception)) +class TestSonicDBConnector(unittest.TestCase): + """Tests for SonicDBConnector class""" + + @patch('show.memory_statistics.ConfigDBConnector') + def setUp(self, mock_config_db): + self.mock_config_db = mock_config_db + self.connector = SonicDBConnector() + self.mock_config_db.reset_mock() + + def test_connect_with_retry_success_after_retries(self): + """Test _connect_with_retry succeeds after retries""" + mock_instance = self.mock_config_db.return_value + mock_instance.connect.side_effect = [ + socket.error("Failed"), + socket.error("Failed"), + None + ] + + self.connector._connect_with_retry(max_retries=3, retry_delay=0.1) + assert mock_instance.connect.call_count == 3 + + def test_connect_with_retry_failure(self): + """Test _connect_with_retry raises ConnectionError after max retries""" + mock_instance = self.mock_config_db.return_value + mock_instance.connect.side_effect = socket.error("Failed") + + with pytest.raises(ConnectionError) as exc_info: + self.connector._connect_with_retry(max_retries=2, retry_delay=0.1) + + assert "Failed to connect to SONiC config database after 2 attempts" in str(exc_info.value) + assert mock_instance.connect.call_count == 2 + + def test_get_memory_statistics_config_missing_table(self): + """Test get_memory_statistics_config with missing table""" + mock_instance = self.mock_config_db.return_value + mock_instance.get_table.return_value = {} + + config = self.connector.get_memory_statistics_config() + assert config == Config.DEFAULT_CONFIG + + def test_get_memory_statistics_config_invalid_table(self): + """Test get_memory_statistics_config with invalid table data""" + mock_instance = self.mock_config_db.return_value + mock_instance.get_table.return_value = {"memory_statistics": "invalid"} + + config = self.connector.get_memory_statistics_config() + assert config == Config.DEFAULT_CONFIG + + def test_send_data_database_error(self): + """Test send_data with database error response""" + error_response = {"status": False, "msg": "Database error"} + + with patch('show.memory_statistics.SocketManager') as mock_socket: + instance = mock_socket.return_value + instance.receive_all.return_value = json.dumps(error_response) + + with pytest.raises(DatabaseError): + send_data("test_command", {}, quiet=True) + + +class TestSocketValidation: + + def test_validate_socket_path_missing_directory(self): + with pytest.raises(ConnectionError): + SocketManager("/nonexistent/path/socket") + + def test_validate_socket_path_create_if_missing(self, tmpdir): + socket_path = tmpdir.join("test.socket") + SocketManager(str(socket_path)) + assert os.path.exists(str(socket_path)) + assert oct(os.stat(str(socket_path)).st_mode & 0o777) == '0o600' + + def test_validate_socket_path_invalid_permissions(self, tmpdir): + socket_path = tmpdir.join("test.socket") + socket_path.write("") + os.chmod(str(socket_path), 0o777) + with pytest.raises(PermissionError): + SocketManager(str(socket_path)) + + +class TestSocketManager: + + @patch('os.path.exists') + @patch('os.stat') + def setup_method(self, method, mock_stat, mock_exists): + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + self.socket_manager = None + + @patch('os.path.exists') + @patch('os.stat') + @patch('socket.socket') + def test_socket_connect_failure(self, mock_socket, mock_stat, mock_exists): + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + mock_socket.return_value.connect.side_effect = socket.error("Connection failed") + socket_manager = SocketManager() + socket_manager.sock = mock_socket.return_value + + with pytest.raises(ConnectionError): + socket_manager.connect() + + @patch('os.path.exists') + @patch('os.stat') + @patch('socket.socket') + def test_socket_receive_all_timeout(self, mock_socket, mock_stat, mock_exists): + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + socket_manager = SocketManager() + socket_manager.sock = mock_socket.return_value + socket_manager.sock.recv.side_effect = socket.timeout("Timeout") + + with pytest.raises(ConnectionError): + socket_manager.receive_all(expected_size=1024) + + @patch('os.path.exists') + @patch('os.stat') + @patch('socket.socket') + def test_socket_send_error(self, mock_socket, mock_stat, mock_exists): + """Test socket send error handling""" + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + socket_manager = SocketManager() + mock_socket.return_value.sendall.side_effect = socket.error("Send failed") + socket_manager.sock = mock_socket.return_value + + with pytest.raises(ConnectionError): + socket_manager.send("test data") + + @patch('os.path.exists') + @patch('os.stat') + @patch('socket.socket') + @patch('time.sleep') + def test_socket_connect_retry_success(self, mock_sleep, mock_socket, mock_stat, mock_exists): + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + mock_socket.return_value.connect.side_effect = [ + socket.error("Failed"), + socket.error("Failed"), + None + ] + socket_manager = SocketManager() + socket_manager.connect() + assert mock_socket.return_value.connect.call_count == 3 + + @patch('os.path.exists') + @patch('os.stat') + @patch('socket.socket') + @patch('time.sleep') + def test_socket_connect_retry_failure(self, mock_sleep, mock_socket, mock_stat, mock_exists): + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + mock_socket.return_value.connect.side_effect = socket.error("Failed") + socket_manager = SocketManager() + with pytest.raises(ConnectionError): + socket_manager.connect() + + @patch('os.path.exists') + @patch('os.stat') + @patch('socket.socket') + def test_receive_all_incomplete_data(self, mock_socket, mock_stat, mock_exists): + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + socket_manager = SocketManager() + socket_manager.sock = mock_socket.return_value + socket_manager.sock.recv.return_value = b"partial" + result = socket_manager.receive_all(expected_size=10, max_attempts=1) + assert result == "partial" + + @patch('os.path.exists') + @patch('os.stat') + def test_socket_manager_close_exception(self, mock_stat, mock_exists): + """Test SocketManager close handles exceptions gracefully""" + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + mock_socket = MagicMock() + mock_socket.close.side_effect = Exception("Close error") + + manager = SocketManager() + manager.sock = mock_socket + + with patch('syslog.syslog') as mock_syslog: + manager.close() + mock_syslog.assert_called_with(syslog.LOG_WARNING, "Error closing socket: Close error") + + @patch('os.path.exists') + @patch('os.stat') + @patch('socket.socket') + def test_socket_close_error(self, mock_socket, mock_stat, mock_exists): + mock_exists.return_value = True + mock_stat_result = MagicMock() + mock_stat_result.st_mode = 0o600 + mock_stat_result.st_uid = 0 + mock_stat.return_value = mock_stat_result + + socket_manager = SocketManager() + socket_manager.sock = mock_socket.return_value + socket_manager.sock.close.side_effect = Exception("Close error") + + with patch('syslog.syslog') as mock_syslog: + socket_manager.close() + mock_syslog.assert_called_with(syslog.LOG_WARNING, "Error closing socket: Close error") + + +class TestMainFunction(unittest.TestCase): + """Tests for the main function""" + + @patch("show.memory_statistics.cli") + def test_main_cli_failure(self, mock_cli): + """Test main handles CLI failure""" + mock_cli.side_effect = Exception("CLI failed") + with self.assertRaises(SystemExit): + main() + + @patch('sys.exit') + @patch('click.echo') + @patch('show.memory_statistics.cli') + def test_main_cli_failure_with_handling(self, mock_cli, mock_echo, mock_exit): + """Test main function handles CLI failure gracefully""" + mock_cli.side_effect = Exception("CLI failed") + main() + mock_echo.assert_called_with("Error: CLI failed", err=True) + mock_exit.assert_called_with(1) + + @patch('signal.signal') + def test_main_signal_registration_error(self, mock_signal): + """Test main handles signal registration errors""" + mock_signal.side_effect = Exception("Signal error") + with self.assertRaises(SystemExit): + main() + + if __name__ == '__main__': unittest.main() From b33678e23340331bef7887c9fd4ae5199e4eab1e Mon Sep 17 00:00:00 2001 From: Arham-Nasir Date: Wed, 12 Feb 2025 18:10:41 +0500 Subject: [PATCH 30/30] Remove generic except Exception and handle specific errors Signed-off-by: Arham-Nasir --- config/memory_statistics.py | 33 ++++---------- tests/config_memory_statistics_test.py | 60 +++++++++++--------------- 2 files changed, 32 insertions(+), 61 deletions(-) diff --git a/config/memory_statistics.py b/config/memory_statistics.py index 47fbadc379..5205b4e89e 100644 --- a/config/memory_statistics.py +++ b/config/memory_statistics.py @@ -22,28 +22,27 @@ DEFAULT_SAMPLING_INTERVAL = 5 # minutes DEFAULT_RETENTION_PERIOD = 15 # days -syslog.openlog("memory_statistics", syslog.LOG_PID | syslog.LOG_CONS, syslog.LOG_USER) +syslog.openlog("memory_statistics_config", syslog.LOG_PID | syslog.LOG_CONS, syslog.LOG_USER) def log_to_syslog(message: str, level: int = syslog.LOG_INFO) -> None: """ - Logs a message to the system log (syslog). + Logs a message to the system log (syslog) with error handling. This function logs the provided message to syslog at the specified level. - It opens the syslog with the application name 'memory_statistics' and the - appropriate log level, ensuring the connection is closed after logging. + It handles potential errors such as system-related issues (OSError) and + invalid parameters (ValueError) by displaying appropriate error messages. Args: message (str): The message to log. level (int, optional): The log level (default is syslog.LOG_INFO). - - Raises: - Exception: If syslog logging fails. """ try: syslog.syslog(level, message) - except Exception as e: - click.echo(f"Failed to log to syslog: {e}", err=True) + except OSError as e: + click.echo(f"System error while logging to syslog: {e}", err=True) + except ValueError as e: + click.echo(f"Invalid syslog parameters: {e}", err=True) def generate_error_message(error_type: str, error: Exception) -> str: @@ -154,12 +153,6 @@ def update_memory_statistics_status(enabled: bool) -> Tuple[bool, Optional[str]] click.echo(error_msg, err=True) log_to_syslog(error_msg, syslog.LOG_ERR) return False, error_msg - except Exception as e: - error_msg = generate_error_message("Unexpected error updating memory statistics status", e) - - click.echo(error_msg, err=True) - log_to_syslog(error_msg, syslog.LOG_ERR) - return False, error_msg @click.group(help="Tool to manage memory statistics configuration.") @@ -287,11 +280,6 @@ def memory_stats_sampling_interval(interval: int): click.echo(error_msg, err=True) log_to_syslog(error_msg, syslog.LOG_ERR) return - except Exception as e: - error_msg = generate_error_message("Unexpected error setting sampling interval", e) - click.echo(error_msg, err=True) - log_to_syslog(error_msg, syslog.LOG_ERR) - return @memory_stats.command(name='retention-period') @@ -334,11 +322,6 @@ def memory_stats_retention_period(period: int): click.echo(error_msg, err=True) log_to_syslog(error_msg, syslog.LOG_ERR) return - except Exception as e: - error_msg = generate_error_message("Unexpected error setting retention period", e) - click.echo(error_msg, err=True) - log_to_syslog(error_msg, syslog.LOG_ERR) - return if __name__ == "__main__": diff --git a/tests/config_memory_statistics_test.py b/tests/config_memory_statistics_test.py index 5f0db136ed..a9a2a8fccf 100644 --- a/tests/config_memory_statistics_test.py +++ b/tests/config_memory_statistics_test.py @@ -102,14 +102,6 @@ def test_successful_disable(self, mock_db): {"enabled": "false"} ) - def test_database_error(self, mock_db): - """Test handling of database errors.""" - mock_db.mod_entry.side_effect = Exception("DB Error") - success, error = update_memory_statistics_status(True) - assert success is False - assert "Unexpected error updating memory statistics status" in error - assert "DB Error" in error - def test_specific_exceptions(self, mock_db): """Test handling of specific exceptions.""" for exception in [KeyError, ConnectionError, RuntimeError]: @@ -132,13 +124,6 @@ def test_enable_success(self, cli_runner, mock_db): assert "successfully" in result.output assert "config save" in result.output - def test_enable_db_error(self, cli_runner, mock_db): - """Test handling of database error when enabling.""" - mock_db.mod_entry.side_effect = Exception("DB Error") - result = cli_runner.invoke(cli, ['config', 'memory-stats', 'enable']) - assert result.exit_code == 0 - assert "Error" in result.output - class TestMemoryStatisticsDisable: def test_disable_success(self, cli_runner, mock_db): @@ -153,13 +138,6 @@ def test_disable_success(self, cli_runner, mock_db): assert "successfully" in result.output assert "config save" in result.output - def test_disable_db_error(self, cli_runner, mock_db): - """Test handling of database error when disabling.""" - mock_db.mod_entry.side_effect = Exception("DB Error") - result = cli_runner.invoke(cli, ['config', 'memory-stats', 'disable']) - assert result.exit_code == 0 - assert "Error" in result.output - class TestSamplingInterval: @pytest.mark.parametrize("interval", [ @@ -191,12 +169,6 @@ def test_invalid_sampling_intervals(self, interval, cli_runner, mock_db): assert "Error" in result.output assert not mock_db.mod_entry.called - def test_sampling_interval_db_error(self, cli_runner, mock_db): - """Test database error case when setting sampling interval.""" - mock_db.mod_entry.side_effect = Exception("DB Error") - result = cli_runner.invoke(cli, ['config', 'memory-stats', 'sampling-interval', '5']) - assert "Error" in result.output - @pytest.mark.parametrize("exception", [ KeyError("Key not found"), ConnectionError("Connection failed"), @@ -242,12 +214,6 @@ def test_invalid_retention_periods(self, period, cli_runner, mock_db): assert "Error" in result.output assert not mock_db.mod_entry.called - def test_db_error(self, cli_runner, mock_db): - """Test handling of database errors.""" - mock_db.mod_entry.side_effect = Exception("DB Error") - result = cli_runner.invoke(cli, ['config', 'memory-stats', 'retention-period', '15']) - assert "Error" in result.output - @pytest.mark.parametrize("exception", [ KeyError("Key not found"), ConnectionError("Connection failed"), @@ -282,10 +248,32 @@ def test_syslog_logging_default_level(self): def test_syslog_logging_error(self): """Test syslog logging error handling.""" - with patch('syslog.syslog', side_effect=Exception("Syslog error")), \ + with patch('syslog.syslog', side_effect=OSError("Syslog error")), \ patch('click.echo') as mock_echo: log_to_syslog("Test message") - mock_echo.assert_called_once_with("Failed to log to syslog: Syslog error", err=True) + mock_echo.assert_called_once_with("System error while logging to syslog: Syslog error", err=True) + + def test_syslog_logging_value_error(self): + """Test syslog logging ValueError handling.""" + invalid_level = -999 + + with patch('syslog.syslog', side_effect=ValueError("Invalid log level")), \ + patch('click.echo') as mock_echo: + log_to_syslog("Test message", invalid_level) + mock_echo.assert_called_once_with( + "Invalid syslog parameters: Invalid log level", + err=True + ) + + def test_syslog_logging_value_error_empty_message(self): + """Test syslog logging ValueError handling with empty message.""" + with patch('syslog.syslog', side_effect=ValueError("Empty message not allowed")), \ + patch('click.echo') as mock_echo: + log_to_syslog("") + mock_echo.assert_called_once_with( + "Invalid syslog parameters: Empty message not allowed", + err=True + ) def test_main_cli_integration():