diff --git a/generic_config_updater/field_operation_validators.py b/generic_config_updater/field_operation_validators.py index 01c037b740..e73c6704e9 100644 --- a/generic_config_updater/field_operation_validators.py +++ b/generic_config_updater/field_operation_validators.py @@ -4,17 +4,16 @@ import jsonpointer import subprocess from sonic_py_common import device_info -from .gu_common import GenericConfigUpdaterError, HOST_NAMESPACE +from .gu_common import GenericConfigUpdaterError from swsscommon import swsscommon from utilities_common.constants import DEFAULT_SUPPORTED_FECS_LIST -STATE_DB_NAME = 'STATE_DB' -REDIS_TIMEOUT_MSECS = 0 SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) GCU_TABLE_MOD_CONF_FILE = f"{SCRIPT_DIR}/gcu_field_operation_validators.conf.json" +GET_HWSKU_CMD = "sonic-cfggen -d -v DEVICE_METADATA.localhost.hwsku" -def get_asic_name(scope): +def get_asic_name(): asic = "unknown" if os.path.exists(GCU_TABLE_MOD_CONF_FILE): @@ -29,12 +28,7 @@ def get_asic_name(scope): if asic_type == 'cisco-8000': asic = "cisco-8000" elif asic_type == 'mellanox' or asic_type == 'vs' or asic_type == 'broadcom': - get_hwsku_cmds = [] - if scope == HOST_NAMESPACE: - get_hwsku_cmds = ["sonic-cfggen", "-d", "-v", "DEVICE_METADATA.localhost.hwsku"] - else: - get_hwsku_cmds = ["sonic-cfggen", "-d", "-n", scope, "-v", "DEVICE_METADATA.localhost.hwsku"] - proc = subprocess.Popen(get_hwsku_cmds, shell=False, universal_newlines=True, stdout=subprocess.PIPE) + proc = subprocess.Popen(GET_HWSKU_CMD, shell=True, universal_newlines=True, stdout=subprocess.PIPE) output, err = proc.communicate() hwsku = output.rstrip('\n') if asic_type == 'mellanox' or asic_type == 'vs': @@ -67,8 +61,8 @@ def get_asic_name(scope): return asic -def rdma_config_update_validator(scope, patch_element): - asic = get_asic_name(scope) +def rdma_config_update_validator(patch_element): + asic = get_asic_name() if asic == "unknown": return False version_info = device_info.get_sonic_version_info() @@ -140,17 +134,17 @@ def _get_fields_in_patch(): return True -def read_statedb_entry(scope, table, key, field): - state_db = swsscommon.DBConnector(STATE_DB_NAME, REDIS_TIMEOUT_MSECS, True, scope) +def read_statedb_entry(table, key, field): + state_db = swsscommon.DBConnector("STATE_DB", 0) tbl = swsscommon.Table(state_db, table) return tbl.hget(key, field)[1] -def port_config_update_validator(scope, patch_element): +def port_config_update_validator(patch_element): def _validate_field(field, port, value): if field == "fec": - supported_fecs_str = read_statedb_entry(scope, "PORT_TABLE", port, "supported_fecs") + supported_fecs_str = read_statedb_entry("PORT_TABLE", port, "supported_fecs") if supported_fecs_str: if supported_fecs_str != 'N/A': supported_fecs_list = [element.strip() for element in supported_fecs_str.split(',')] @@ -162,7 +156,7 @@ def _validate_field(field, port, value): return False return True if field == "speed": - supported_speeds_str = read_statedb_entry(scope, "PORT_TABLE", port, "supported_speeds") or '' + supported_speeds_str = read_statedb_entry("PORT_TABLE", port, "supported_speeds") or '' try: supported_speeds = [int(s) for s in supported_speeds_str.split(',') if s] if supported_speeds and int(value) not in supported_speeds: diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index e1a66ded9e..184566e4f5 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -191,7 +191,7 @@ def _invoke_validating_function(cmd, jsonpatch_element): raise GenericConfigUpdaterError("Attempting to call invalid method {} in module {}. Module must be generic_config_updater.field_operation_validators, and method must be a defined validator".format(method_name, module_name)) module = importlib.import_module(module_name, package=None) method_to_call = getattr(module, method_name) - return method_to_call(self.scope, jsonpatch_element) + return method_to_call(jsonpatch_element) if os.path.exists(GCU_FIELD_OP_CONF_FILE): with open(GCU_FIELD_OP_CONF_FILE, "r") as s: diff --git a/tests/generic_config_updater/field_operation_validator_test.py b/tests/generic_config_updater/field_operation_validator_test.py index a54219146b..ea8ef5f26d 100644 --- a/tests/generic_config_updater/field_operation_validator_test.py +++ b/tests/generic_config_updater/field_operation_validator_test.py @@ -1,5 +1,5 @@ -import mock import unittest +import mock import generic_config_updater import generic_config_updater.field_operation_validators as fov import generic_config_updater.gu_common as gu_common @@ -14,42 +14,32 @@ class TestValidateFieldOperation(unittest.TestCase): @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="")) def test_port_config_update_validator_valid_speed_no_state_db(self): patch_element = {"path": "/PORT/Ethernet3", "op": "add", "value": {"speed": "234"}} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="40000,30000")) def test_port_config_update_validator_invalid_speed_existing_state_db(self): patch_element = {"path": "/PORT/Ethernet3", "op": "add", "value": {"speed": "xyz"}} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is False + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is False @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) def test_port_config_update_validator_valid_speed_existing_state_db(self): patch_element = {"path": "/PORT/Ethernet3", "op": "add", "value": {"speed": "234"}} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) def test_port_config_update_validator_valid_speed_existing_state_db(self): patch_element = {"path": "/PORT/Ethernet3/speed", "op": "add", "value": "234"} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) def test_port_config_update_validator_invalid_speed_existing_state_db(self): patch_element = {"path": "/PORT/Ethernet3/speed", "op": "add", "value": "235"} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is False - + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is False + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) def test_port_config_update_validator_invalid_speed_existing_state_db_nested(self): @@ -58,9 +48,7 @@ def test_port_config_update_validator_invalid_speed_existing_state_db_nested(sel "op": "add", "value": {"Ethernet3": {"alias": "Eth0", "speed": "235"}} } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is False + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is False @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) @@ -73,10 +61,8 @@ def test_port_config_update_validator_valid_speed_existing_state_db_nested(self) "Ethernet4": {"alias": "Eth4", "speed": "234"} } } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True - + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="123,234")) def test_port_config_update_validator_invalid_speed_existing_state_db_nested_2(self): @@ -88,23 +74,17 @@ def test_port_config_update_validator_invalid_speed_existing_state_db_nested_2(s "Ethernet4": {"alias": "Eth4", "speed": "236"} } } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is False - + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is False + def test_port_config_update_validator_remove(self): patch_element = {"path": "/PORT/Ethernet3", "op": "remove"} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) def test_port_config_update_validator_invalid_fec_existing_state_db(self): patch_element = {"path": "/PORT/Ethernet3/fec", "op": "add", "value": "asf"} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is False + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is False @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) @@ -117,10 +97,8 @@ def test_port_config_update_validator_invalid_fec_existing_state_db_nested(self) "Ethernet4": {"alias": "Eth4", "fec": "fs"} } } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is False - + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is False + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) def test_port_config_update_validator_valid_fec_existing_state_db_nested(self): @@ -129,9 +107,7 @@ def test_port_config_update_validator_valid_fec_existing_state_db_nested(self): "op": "add", "value": {"Ethernet3": {"alias": "Eth0", "fec": "fc"}} } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) @@ -144,33 +120,25 @@ def test_port_config_update_validator_valid_fec_existing_state_db_nested_2(self) "Ethernet4": {"alias": "Eth4", "fec": "fc"} } } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True - + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="rs, fc")) def test_port_config_update_validator_valid_fec_existing_state_db(self): patch_element = {"path": "/PORT/Ethernet3/fec", "op": "add", "value": "rs"} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="")) def test_port_config_update_validator_valid_fec_no_state_db(self): patch_element = {"path": "/PORT/Ethernet3", "op": "add", "value": {"fec": "rs"}} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is True - + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is True + @patch("generic_config_updater.field_operation_validators.read_statedb_entry", mock.Mock(return_value="")) def test_port_config_update_validator_invalid_fec_no_state_db(self): patch_element = {"path": "/PORT/Ethernet3/fec", "op": "add", "value": "rsf"} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - port_config_update_validator(scope, patch_element) is False + assert generic_config_updater.field_operation_validators.port_config_update_validator(patch_element) is False @patch("generic_config_updater.field_operation_validators.get_asic_name", mock.Mock(return_value="unknown")) @@ -180,9 +148,7 @@ def test_rdma_config_update_validator_unknown_asic(self): "op": "replace", "value": "234234" } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - rdma_config_update_validator(scope, patch_element) is False + assert generic_config_updater.field_operation_validators.rdma_config_update_validator(patch_element) is False @patch("sonic_py_common.device_info.get_sonic_version_info", mock.Mock(return_value={"build_version": "SONiC.20220530"})) @@ -199,14 +165,11 @@ def test_rdma_config_update_validator_td3_asic_invalid_version(self): "op": "replace", "value": "234234" } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - rdma_config_update_validator(scope, patch_element) is False - + assert generic_config_updater.field_operation_validators.rdma_config_update_validator(patch_element) is False + @patch("sonic_py_common.device_info.get_sonic_version_info", mock.Mock(return_value={"build_version": "SONiC.20220530"})) - @patch("generic_config_updater.field_operation_validators.get_asic_name", - mock.Mock(return_value="spc1")) + @patch("generic_config_updater.field_operation_validators.get_asic_name", mock.Mock(return_value="spc1")) @patch("os.path.exists", mock.Mock(return_value=True)) @patch("builtins.open", mock_open(read_data='''{"tables": {"PFC_WD": {"validator_data": { "rdma_config_update_validator": {"PFCWD enable/disable": {"fields": [ @@ -214,9 +177,7 @@ def test_rdma_config_update_validator_td3_asic_invalid_version(self): ], "operations": ["remove", "replace", "add"], "platforms": {"spc1": "20181100"}}}}}}}''')) def test_rdma_config_update_validator_spc_asic_valid_version_remove(self): patch_element = {"path": "/PFC_WD/Ethernet8/detection_time", "op": "remove"} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - rdma_config_update_validator(scope, patch_element) is True + assert generic_config_updater.field_operation_validators.rdma_config_update_validator(patch_element) is True @patch("sonic_py_common.device_info.get_sonic_version_info", mock.Mock(return_value={"build_version": "SONiC.20220530"})) @@ -237,14 +198,11 @@ def test_rdma_config_update_validator_spc_asic_valid_version_add_pfcwd(self): "restoration_time": "200" } } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - rdma_config_update_validator(scope, patch_element) is True - + assert generic_config_updater.field_operation_validators.rdma_config_update_validator(patch_element) is True + @patch("sonic_py_common.device_info.get_sonic_version_info", mock.Mock(return_value={"build_version": "SONiC.20220530"})) - @patch("generic_config_updater.field_operation_validators.get_asic_name", - mock.Mock(return_value="spc1")) + @patch("generic_config_updater.field_operation_validators.get_asic_name", mock.Mock(return_value="spc1")) @patch("os.path.exists", mock.Mock(return_value=True)) @patch("builtins.open", mock_open(read_data='''{"tables": {"PFC_WD": {"validator_data": { "rdma_config_update_validator": {"PFCWD enable/disable": {"fields": [ @@ -252,10 +210,8 @@ def test_rdma_config_update_validator_spc_asic_valid_version_add_pfcwd(self): ], "operations": ["remove", "replace", "add"], "platforms": {"spc1": "20181100"}}}}}}}''')) def test_rdma_config_update_validator_spc_asic_valid_version(self): patch_element = {"path": "/PFC_WD/Ethernet8", "op": "remove"} - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - rdma_config_update_validator(scope, patch_element) is True - + assert generic_config_updater.field_operation_validators.rdma_config_update_validator(patch_element) is True + @patch("sonic_py_common.device_info.get_sonic_version_info", mock.Mock(return_value={"build_version": "SONiC.20220530"})) @patch("generic_config_updater.field_operation_validators.get_asic_name", @@ -270,10 +226,8 @@ def test_rdma_config_update_validator_spc_asic_invalid_op(self): "path": "/BUFFER_POOL/ingress_lossless_pool/xoff", "op": "remove" } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - rdma_config_update_validator(scope, patch_element) is False - + assert generic_config_updater.field_operation_validators.rdma_config_update_validator(patch_element) is False + @patch("sonic_py_common.device_info.get_sonic_version_info", mock.Mock(return_value={"build_version": "SONiC.20220530"})) @patch("generic_config_updater.field_operation_validators.get_asic_name", @@ -289,9 +243,7 @@ def test_rdma_config_update_validator_spc_asic_other_field(self): "op": "add", "value": "sample_value" } - for scope in ["localhost", "asic0"]: - assert generic_config_updater.field_operation_validators.\ - rdma_config_update_validator(scope, patch_element) is False + assert generic_config_updater.field_operation_validators.rdma_config_update_validator(patch_element) is False def test_validate_field_operation_illegal__pfcwd(self): old_config = {"PFC_WD": {"GLOBAL": {"POLL_INTERVAL": "60"}}} @@ -439,8 +391,7 @@ def test_get_asic_spc1(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'mellanox'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["Mellanox-SN2700-D48C8", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "spc1") + self.assertEqual(fov.get_asic_name(), "spc1") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') @@ -448,8 +399,7 @@ def test_get_asic_spc2(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'mellanox'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["ACS-MSN3800", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "spc2") + self.assertEqual(fov.get_asic_name(), "spc2") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') @@ -457,8 +407,7 @@ def test_get_asic_spc3(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'mellanox'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["Mellanox-SN4600C-C64", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "spc3") + self.assertEqual(fov.get_asic_name(), "spc3") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') @@ -466,8 +415,7 @@ def test_get_asic_spc4(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'mellanox'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["ACS-SN5600", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "spc4") + self.assertEqual(fov.get_asic_name(), "spc4") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') @@ -475,8 +423,7 @@ def test_get_asic_spc4(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'mellanox'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["Mellanox-SN2700-A1", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "spc1") + self.assertEqual(fov.get_asic_name(), "spc1") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') @@ -484,8 +431,7 @@ def test_get_asic_th(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'broadcom'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["Force10-S6100", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "th") + self.assertEqual(fov.get_asic_name(), "th") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') @@ -493,8 +439,7 @@ def test_get_asic_th2(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'broadcom'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["Arista-7260CX3-D108C8", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "th2") + self.assertEqual(fov.get_asic_name(), "th2") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') @@ -502,8 +447,7 @@ def test_get_asic_td2(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'broadcom'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["Force10-S6000", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "td2") + self.assertEqual(fov.get_asic_name(), "td2") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') @@ -511,12 +455,10 @@ def test_get_asic_td3(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'broadcom'} mock_popen.return_value = mock.Mock() mock_popen.return_value.communicate.return_value = ["Arista-7050CX3-32S-C32", 0] - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "td3") + self.assertEqual(fov.get_asic_name(), "td3") @patch('sonic_py_common.device_info.get_sonic_version_info') @patch('subprocess.Popen') def test_get_asic_cisco(self, mock_popen, mock_get_sonic_version_info): mock_get_sonic_version_info.return_value = {'asic_type': 'cisco-8000'} - for scope in ["localhost", "asic0"]: - self.assertEqual(fov.get_asic_name(scope), "cisco-8000") + self.assertEqual(fov.get_asic_name(), "cisco-8000")