From 08f1fbed76ee867dccb2f5fc6ab57fe8acea8046 Mon Sep 17 00:00:00 2001 From: SuvarnaMeenakshi <50386592+SuvarnaMeenakshi@users.noreply.github.com> Date: Thu, 27 Feb 2025 19:07:11 -0800 Subject: [PATCH] Update management interface related configuration in MGMT_PORT_TABLE in STATE_DB (#21813) Currently Management interface related data is present in CONFIG_DB and 'oper_status' is present in STATE_DB MGMT_PORT_TABLE. This PR change is made to ensure that all telemetry data related to management interface is present in STATE_DB. Signed-off-by: Suvarna Meenakshi --- files/image_config/monit/mgmt_oper_status.py | 29 ++++++++++++++----- .../monit/tests/test_mgmt_oper_status.py | 26 +++++++++++++++-- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/files/image_config/monit/mgmt_oper_status.py b/files/image_config/monit/mgmt_oper_status.py index d6473ecf8dec..f5783347d789 100755 --- a/files/image_config/monit/mgmt_oper_status.py +++ b/files/image_config/monit/mgmt_oper_status.py @@ -6,7 +6,6 @@ import subprocess import syslog -from sonic_py_common import multi_asic, device_info from swsscommon.swsscommon import SonicV2Connector @@ -14,25 +13,39 @@ def main(): db = SonicV2Connector(use_unix_socket_path=True) db.connect('CONFIG_DB') db.connect('STATE_DB') - mgmt_ports_keys = db.keys(db.CONFIG_DB, 'MGMT_PORT|*' ) + mgmt_ports_keys = db.keys(db.CONFIG_DB, 'MGMT_PORT|*') if not mgmt_ports_keys: syslog.syslog(syslog.LOG_DEBUG, 'No management interface found') else: try: - mgmt_ports = [key.split('MGMT_PORT|')[-1] for key in mgmt_ports_keys] + mgmt_ports = [key.split('MGMT_PORT|')[-1] for key + in mgmt_ports_keys] for port in mgmt_ports: - state_db_mgmt_port = db.keys(db.STATE_DB, 'MGMT_PORT_TABLE|*' ) + state_db_mgmt_keys = db.keys(db.STATE_DB, 'MGMT_PORT_TABLE|*') state_db_key = "MGMT_PORT_TABLE|{}".format(port) - prev_oper_status = 'unknown' - if state_db_key in state_db_mgmt_port: - prev_oper_status = db.get(db.STATE_DB, state_db_key, 'oper_status') + config_db_key = "MGMT_PORT|{}".format(port) + config_db_mgmt = db.get_all(db.CONFIG_DB, config_db_key) + state_db_mgmt = db.get_all(db.STATE_DB, state_db_key) if state_db_key in state_db_mgmt_keys else {} + + # Sync fields from CONFIG_DB MGMT_PORT table to STATE_DB MGMT_PORT_TABLE + for field in config_db_mgmt: + if field != 'oper_status': + # Update STATE_DB if port is not present or value differs from + # CONFIG_DB + if (field in state_db_mgmt and state_db_mgmt[field] != config_db_mgmt[field]) \ + or field not in state_db_mgmt: + db.set(db.STATE_DB, state_db_key, field, config_db_mgmt[field]) + + # Update oper status if modified + prev_oper_status = state_db_mgmt.get('oper_status', 'unknown') port_operstate_path = '/sys/class/net/{}/operstate'.format(port) oper_status = subprocess.run(['cat', port_operstate_path], capture_output=True, text=True) current_oper_status = oper_status.stdout.strip() if current_oper_status != prev_oper_status: db.set(db.STATE_DB, state_db_key, 'oper_status', current_oper_status) - log_level = syslog.LOG_INFO if current_oper_status == 'up' else syslog.LOG_WARNING + log_level = syslog.LOG_INFO if current_oper_status == 'up' else syslog.LOG_WARNING syslog.syslog(log_level, "mgmt_oper_status: {}".format(current_oper_status)) + except Exception as e: syslog.syslog(syslog.LOG_ERR, "mgmt_oper_status exception : {}".format(str(e))) db.set(db.STATE_DB, state_db_key, 'oper_status', 'unknown') diff --git a/files/image_config/monit/tests/test_mgmt_oper_status.py b/files/image_config/monit/tests/test_mgmt_oper_status.py index 2b2f1fbefc42..078f2605a99b 100644 --- a/files/image_config/monit/tests/test_mgmt_oper_status.py +++ b/files/image_config/monit/tests/test_mgmt_oper_status.py @@ -29,6 +29,22 @@ def test_main_with_mgmt_ports(self, mock_syslog, mock_subprocess, mock_SonicV2Co mock_db.keys.return_value = mgmt_ports_keys mock_db.set.return_value = None + def get_all_side_effect(db_name, key): + if db_name == mock_db.CONFIG_DB: + return {'admin_status': 'up', 'alias': 'mgmt', 'speed': '1000'} + elif db_name == mock_db.STATE_DB: + return {'admin_status': 'up', 'alias': 'Management'} + return {} + mock_db.get_all.side_effect = get_all_side_effect + + def keys_side_effect(db_name, key_regex): + if db_name == mock_db.CONFIG_DB: + return ['MGMT_PORT|eth0', 'MGMT_PORT|eth1'] + elif db_name == mock_db.STATE_DB: + return ['MGMT_PORT_TABLE|eth0', 'MGMT_PORT_TABLE|eth1'] + return {} + mock_db.keys.side_effect = keys_side_effect + mock_subprocess.return_value = subprocess.CompletedProcess(args=['cat', '/sys/class/net/eth0/operstate'], returncode=0, stdout='up', stderr='') mgmt_oper_status.main() @@ -38,6 +54,12 @@ def test_main_with_mgmt_ports(self, mock_syslog, mock_subprocess, mock_SonicV2Co mock_db.set.assert_any_call(mock_db.STATE_DB, 'MGMT_PORT_TABLE|eth0', 'oper_status', 'up') mock_db.set.assert_any_call(mock_db.STATE_DB, 'MGMT_PORT_TABLE|eth1', 'oper_status', 'up') + # Assert STATE_DB was updated with field that was not present in CONFIG_DB + mock_db.set.assert_any_call(mock_db.STATE_DB, 'MGMT_PORT_TABLE|eth1', 'speed', '1000') + # Assert STATE_DB was updated with alias with updated value from CONFIG_DB + mock_db.set.assert_any_call(mock_db.STATE_DB, 'MGMT_PORT_TABLE|eth1', 'alias', 'mgmt') + # Assert STATE_DB was NOT updated with field is already present and value is not modified + assert not any(call[0] == (mock_db.STATE_DB, 'MGMT_PORT_TABLE|eth1', 'admin_status', 'up') for call in mock_db.set.call_args_list) @patch('mgmt_oper_status.SonicV2Connector') @patch('mgmt_oper_status.subprocess.run') @@ -69,8 +91,8 @@ def test_main_exception_handling(self, mock_syslog, mock_subprocess, mock_SonicV mock_db.set.return_value = None mock_subprocess.side_effect = Exception("File not found") - - mgmt_oper_status.main() + with self.assertRaises(SystemExit) as cm: + mgmt_oper_status.main() mock_syslog.assert_called_with(syslog.LOG_ERR, "mgmt_oper_status exception : File not found") mock_db.set.assert_any_call(mock_db.STATE_DB, 'MGMT_PORT_TABLE|eth0', 'oper_status', 'unknown')