From 4fde0f315e149e3d5c73e5cfae6fbb714f70a3de Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Mon, 8 Apr 2024 14:14:18 -0700 Subject: [PATCH] [System Ready]Handle template value for feature table state field (#18411) * [System Ready]Handle template value for feature table state field --- .../health_checker/sysmonitor.py | 28 ++++++++++++++++++- src/system-health/tests/test_system_health.py | 15 +++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/src/system-health/health_checker/sysmonitor.py b/src/system-health/health_checker/sysmonitor.py index 3a2840e11a63..92777b256c94 100755 --- a/src/system-health/health_checker/sysmonitor.py +++ b/src/system-health/health_checker/sysmonitor.py @@ -10,8 +10,10 @@ from sonic_py_common.logger import Logger from . import utils from sonic_py_common.task_base import ProcessTaskBase +from sonic_py_common import device_info from .config import Config import signal +import jinja2 SYSLOG_IDENTIFIER = "system#monitor" REDIS_TIMEOUT_MS = 0 @@ -159,6 +161,26 @@ def get_all_service_list(self): dir_list.sort() return dir_list + def get_render_value_for_field(self, configuration, device_config, expected_values): + """ Returns the target value by rendering the configuration as J2 template. + + Args: + configuration (str): Table value from CONFIG_DB for given field + device_config (dict): DEVICE_METADATA section of CONFIG_DB and populated Device Running Metadata which is needed for rendering + expected_values (list): Expected set of values for given field + Returns: + (str): Target value for given key + """ + + if configuration is None: + return None + + template = jinja2.Template(configuration) + target_value = template.render(device_config) # nosemgrep: python.flask.security.xss.audit.direct-use-of-jinja2.direct-use-of-jinja2 + if target_value not in expected_values: + raise ValueError('Invalid value rendered for configuration {}: {}'.format(configuration, target_value)) + return target_value + def get_service_from_feature_table(self, dir_list): """Get service from CONFIG DB FEATURE table. During "config reload" command, filling FEATURE table is not an atomic operation, sonic-cfggen do it with two steps: @@ -178,12 +200,16 @@ def get_service_from_feature_table(self, dir_list): while max_retry > 0: success = True feature_table = self.config_db.get_table("FEATURE") + device_config = {} + device_config['DEVICE_METADATA'] = self.config_db.get_table('DEVICE_METADATA') + device_config.update(device_info.get_device_runtime_metadata()) for srv, fields in feature_table.items(): if 'state' not in fields: success = False logger.log_warning("FEATURE table is not fully ready: {}, retrying".format(feature_table)) break - if fields["state"] not in ["disabled", "always_disabled"]: + state = self.get_render_value_for_field(fields["state"], device_config, ['enabled', 'disabled', 'always_enabled', 'always_disabled']) + if state not in ["disabled", "always_disabled"]: srvext = srv + ".service" if srvext not in dir_list: dir_list.append(srvext) diff --git a/src/system-health/tests/test_system_health.py b/src/system-health/tests/test_system_health.py index 67f819ecc5ff..cd9a54e8799f 100644 --- a/src/system-health/tests/test_system_health.py +++ b/src/system-health/tests/test_system_health.py @@ -47,6 +47,7 @@ """ device_info.get_platform = MagicMock(return_value='unittest') +device_runtime_metadata = {"DEVICE_RUNTIME_METADATA": {"ETHERNET_PORTS_PRESENT":True}} def setup(): if os.path.exists(ServiceChecker.CRITICAL_PROCESS_CACHE): @@ -583,6 +584,7 @@ def test_utils(): @patch('docker.DockerClient') @patch('health_checker.utils.run_command') @patch('swsscommon.swsscommon.ConfigDBConnector') +@patch('sonic_py_common.device_info.get_device_runtime_metadata', MagicMock(return_value=device_runtime_metadata)) def test_get_all_service_list(mock_config_db, mock_run, mock_docker_client): mock_db_data = MagicMock() mock_get_table = MagicMock() @@ -841,6 +843,7 @@ def test_system_service(): sysmon.task_stop() +@patch('sonic_py_common.device_info.get_device_runtime_metadata', MagicMock(return_value=device_runtime_metadata)) def test_get_service_from_feature_table(): sysmon = Sysmonitor() sysmon.config_db = MagicMock() @@ -851,8 +854,18 @@ def test_get_service_from_feature_table(): 'swss': {} }, { - 'bgp': {'state': 'enabled'}, + 'localhost': { + 'type': 'ToRRouter' + } + }, + { + 'bgp': {'state': "{% if not (DEVICE_METADATA is defined and DEVICE_METADATA['localhost'] is defined and DEVICE_METADATA['localhost']['type'] is defined and DEVICE_METADATA['localhost']['type'] is not in ['ToRRouter', 'EPMS', 'MgmtTsToR', 'MgmtToRRouter', 'BmcMgmtToRRouter']) %}enabled{% else %}disabled{% endif %}"}, 'swss': {'state': 'disabled'} + }, + { + 'localhost': { + 'type': 'ToRRouter' + } } ] dir_list = []