From 826900236e9be746fdb23a01d0acc27d73b494ff Mon Sep 17 00:00:00 2001 From: ganglyu Date: Wed, 27 Dec 2023 06:18:42 +0000 Subject: [PATCH 1/6] Run yang validation in db migrator --- scripts/config_validator.py | 41 +++++++++++++++++++ scripts/db_migrator.py | 27 ++++++++++++ setup.py | 1 + ...ross_branch_upgrade_to_4_0_3_expected.json | 6 +-- .../cross_branch_upgrade_to_4_0_3_input.json | 6 +-- .../config_db/portchannel-expected.json | 4 -- .../config_db/portchannel-input.json | 4 -- .../config_db/qos_map_table_expected.json | 10 ++++- .../config_db/qos_map_table_input.json | 10 ++++- ...-buffer-dynamic-double-pools-expected.json | 12 +++++- ...ing-buffer-dynamic-double-pools-input.json | 12 +++++- ...g-buffer-dynamic-single-pool-expected.json | 12 +++++- ...ming-buffer-dynamic-single-pool-input.json | 12 +++++- 13 files changed, 137 insertions(+), 20 deletions(-) create mode 100755 scripts/config_validator.py diff --git a/scripts/config_validator.py b/scripts/config_validator.py new file mode 100755 index 0000000000..0ebacdc651 --- /dev/null +++ b/scripts/config_validator.py @@ -0,0 +1,41 @@ +#!/usr/bin/env python3 +import json +import argparse +import sonic_yang + +from sonic_py_common import logger + +YANG_MODELS_DIR = "/usr/local/yang-models" +SYSLOG_IDENTIFIER = 'config_validator' + +# Global logger instance +log = logger.Logger(SYSLOG_IDENTIFIER) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('-c', + dest='config', + metavar='config file', + type = str, + required = True, + help = 'the config file to be validated', + default = None ) + + args = parser.parse_args() + config_file = args.config + with open(config_file) as fp: + config = json.load(fp) + # Run yang validation + yang_parser = sonic_yang.SonicYang(YANG_MODELS_DIR) + yang_parser.loadYangModel() + try: + yang_parser.loadData(configdbJson=config) + yang_parser.validate_data_tree() + except sonic_yang.SonicYangException as e: + log.log_error("Yang validation failed: " + str(e)) + raise + + +if __name__ == "__main__": + main() diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index 9667eedaff..f9a32cfdde 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -6,6 +6,7 @@ import sys import traceback import re +import subprocess from sonic_py_common import device_info, logger from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector @@ -1210,6 +1211,32 @@ def migrate(self): version = next_version # Perform common migration ops self.common_migration_ops() + # Perform yang validation + self.validate() + + def validate(self): + config = self.configDB.get_config() + # Fix table key in tuple + for table_name, table in config.items(): + new_table = {} + hit = False + for table_key, table_val in table.items(): + if isinstance(table_key, tuple): + new_key = "|".join(table_key) + new_table[new_key] = table_val + hit = True + else: + new_table[table_key] = table_val + if hit: + config[table_name] = new_table + config_file = "/tmp/validate.json" + with open(config_file, 'w') as fp: + json.dump(config, fp) + process = subprocess.Popen(["config_validator.py", "-c", config_file]) + # Check validation result for unit test + if os.environ.get("UTILITIES_UNIT_TESTING", "0") == "2": + ret = process.wait() + assert ret == 0, "Yang validation failed" def main(): try: diff --git a/setup.py b/setup.py index 196777d0e3..53bf376dba 100644 --- a/setup.py +++ b/setup.py @@ -117,6 +117,7 @@ 'scripts/buffershow', 'scripts/coredump-compress', 'scripts/configlet', + 'scripts/config_validator.py', 'scripts/db_migrator.py', 'scripts/decode-syseeprom', 'scripts/dropcheck', diff --git a/tests/db_migrator_input/config_db/cross_branch_upgrade_to_4_0_3_expected.json b/tests/db_migrator_input/config_db/cross_branch_upgrade_to_4_0_3_expected.json index 1ebfbc6afe..a64d38bc51 100644 --- a/tests/db_migrator_input/config_db/cross_branch_upgrade_to_4_0_3_expected.json +++ b/tests/db_migrator_input/config_db/cross_branch_upgrade_to_4_0_3_expected.json @@ -3,17 +3,17 @@ "VERSION": "version_4_0_3" }, "FLEX_COUNTER_TABLE|ACL": { - "FLEX_COUNTER_STATUS": "true", + "FLEX_COUNTER_STATUS": "enable", "FLEX_COUNTER_DELAY_STATUS": "true", "POLL_INTERVAL": "10000" }, "FLEX_COUNTER_TABLE|QUEUE": { - "FLEX_COUNTER_STATUS": "true", + "FLEX_COUNTER_STATUS": "enable", "FLEX_COUNTER_DELAY_STATUS": "true", "POLL_INTERVAL": "10000" }, "FLEX_COUNTER_TABLE|PG_WATERMARK": { - "FLEX_COUNTER_STATUS": "false", + "FLEX_COUNTER_STATUS": "disable", "FLEX_COUNTER_DELAY_STATUS": "true" } } diff --git a/tests/db_migrator_input/config_db/cross_branch_upgrade_to_4_0_3_input.json b/tests/db_migrator_input/config_db/cross_branch_upgrade_to_4_0_3_input.json index 07ce763683..e2d8d04588 100644 --- a/tests/db_migrator_input/config_db/cross_branch_upgrade_to_4_0_3_input.json +++ b/tests/db_migrator_input/config_db/cross_branch_upgrade_to_4_0_3_input.json @@ -3,16 +3,16 @@ "VERSION": "version_1_0_1" }, "FLEX_COUNTER_TABLE|ACL": { - "FLEX_COUNTER_STATUS": "true", + "FLEX_COUNTER_STATUS": "enable", "FLEX_COUNTER_DELAY_STATUS": "true", "POLL_INTERVAL": "10000" }, "FLEX_COUNTER_TABLE|QUEUE": { - "FLEX_COUNTER_STATUS": "true", + "FLEX_COUNTER_STATUS": "enable", "FLEX_COUNTER_DELAY_STATUS": "false", "POLL_INTERVAL": "10000" }, "FLEX_COUNTER_TABLE|PG_WATERMARK": { - "FLEX_COUNTER_STATUS": "false" + "FLEX_COUNTER_STATUS": "disable" } } diff --git a/tests/db_migrator_input/config_db/portchannel-expected.json b/tests/db_migrator_input/config_db/portchannel-expected.json index 2644e5f4e9..f380c75363 100644 --- a/tests/db_migrator_input/config_db/portchannel-expected.json +++ b/tests/db_migrator_input/config_db/portchannel-expected.json @@ -1,28 +1,24 @@ { "PORTCHANNEL|PortChannel0": { "admin_status": "up", - "members@": "Ethernet0,Ethernet4", "min_links": "2", "mtu": "9100", "lacp_key": "auto" }, "PORTCHANNEL|PortChannel1": { "admin_status": "up", - "members@": "Ethernet8,Ethernet12", "min_links": "2", "mtu": "9100", "lacp_key": "auto" }, "PORTCHANNEL|PortChannel0123": { "admin_status": "up", - "members@": "Ethernet16", "min_links": "1", "mtu": "9100", "lacp_key": "auto" }, "PORTCHANNEL|PortChannel0011": { "admin_status": "up", - "members@": "Ethernet20,Ethernet24", "min_links": "2", "mtu": "9100", "lacp_key": "auto" diff --git a/tests/db_migrator_input/config_db/portchannel-input.json b/tests/db_migrator_input/config_db/portchannel-input.json index 753a88601d..43a9fabdb5 100644 --- a/tests/db_migrator_input/config_db/portchannel-input.json +++ b/tests/db_migrator_input/config_db/portchannel-input.json @@ -1,25 +1,21 @@ { "PORTCHANNEL|PortChannel0": { "admin_status": "up", - "members@": "Ethernet0,Ethernet4", "min_links": "2", "mtu": "9100" }, "PORTCHANNEL|PortChannel1": { "admin_status": "up", - "members@": "Ethernet8,Ethernet12", "min_links": "2", "mtu": "9100" }, "PORTCHANNEL|PortChannel0123": { "admin_status": "up", - "members@": "Ethernet16", "min_links": "1", "mtu": "9100" }, "PORTCHANNEL|PortChannel0011": { "admin_status": "up", - "members@": "Ethernet20,Ethernet24", "min_links": "2", "mtu": "9100" }, diff --git a/tests/db_migrator_input/config_db/qos_map_table_expected.json b/tests/db_migrator_input/config_db/qos_map_table_expected.json index 47381ec550..f84c1a900b 100644 --- a/tests/db_migrator_input/config_db/qos_map_table_expected.json +++ b/tests/db_migrator_input/config_db/qos_map_table_expected.json @@ -29,6 +29,14 @@ "pfc_to_queue_map": "AZURE", "tc_to_pg_map": "AZURE", "tc_to_queue_map": "AZURE" - } + }, + "TC_TO_QUEUE_MAP|AZURE": {"0": "0"}, + "TC_TO_PRIORITY_GROUP_MAP|AZURE": {"0": "0"}, + "MAP_PFC_PRIORITY_TO_QUEUE|AZURE": {"0": "0"}, + "DSCP_TO_TC_MAP|AZURE": {"0": "0"}, + "PORT|Ethernet0": {"lanes": "0", "speed": "1000"}, + "PORT|Ethernet92": {"lanes": "92", "speed": "1000"}, + "PORT|Ethernet96": {"lanes": "96", "speed": "1000"}, + "PORT|Ethernet100": {"lanes": "100", "speed": "1000"} } diff --git a/tests/db_migrator_input/config_db/qos_map_table_input.json b/tests/db_migrator_input/config_db/qos_map_table_input.json index c62e293daf..3c288b9534 100644 --- a/tests/db_migrator_input/config_db/qos_map_table_input.json +++ b/tests/db_migrator_input/config_db/qos_map_table_input.json @@ -27,5 +27,13 @@ "pfc_to_queue_map": "AZURE", "tc_to_pg_map": "AZURE", "tc_to_queue_map": "AZURE" - } + }, + "TC_TO_QUEUE_MAP|AZURE": {"0": "0"}, + "TC_TO_PRIORITY_GROUP_MAP|AZURE": {"0": "0"}, + "MAP_PFC_PRIORITY_TO_QUEUE|AZURE": {"0": "0"}, + "DSCP_TO_TC_MAP|AZURE": {"0": "0"}, + "PORT|Ethernet0": {"lanes": "0", "speed": "1000"}, + "PORT|Ethernet92": {"lanes": "92", "speed": "1000"}, + "PORT|Ethernet96": {"lanes": "96", "speed": "1000"}, + "PORT|Ethernet100": {"lanes": "100", "speed": "1000"} } diff --git a/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-double-pools-expected.json b/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-double-pools-expected.json index 5181daa057..b969575c78 100644 --- a/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-double-pools-expected.json +++ b/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-double-pools-expected.json @@ -12,7 +12,7 @@ "profile": "NULL" }, "BUFFER_PG|Ethernet8|3-4": { - "profile": "customized_lossless_profile" + "profile": "customized_ingress_lossless_profile" }, "BUFFER_PG|Ethernet12|0": { "profile": "ingress_lossy_profile" @@ -103,6 +103,11 @@ "BUFFER_PORT_INGRESS_PROFILE_LIST|Ethernet24": { "profile_list": "ingress_lossless_profile,ingress_lossy_profile" }, + "BUFFER_PROFILE|customized_egress_lossless_profile": { + "dynamic_th": "7", + "pool": "egress_lossless_pool", + "size": "0" + }, "BUFFER_PROFILE|egress_lossless_profile": { "dynamic_th": "7", "pool": "egress_lossless_pool", @@ -118,6 +123,11 @@ "pool": "ingress_lossless_pool", "size": "0" }, + "BUFFER_PROFILE|customized_ingress_lossless_profile": { + "dynamic_th": "7", + "pool": "ingress_lossless_pool", + "size": "0" + }, "BUFFER_PROFILE|ingress_lossy_profile": { "dynamic_th": "3", "pool": "ingress_lossy_pool", diff --git a/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-double-pools-input.json b/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-double-pools-input.json index d8deef194f..d3337ccadb 100644 --- a/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-double-pools-input.json +++ b/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-double-pools-input.json @@ -3,7 +3,7 @@ "profile": "NULL" }, "BUFFER_PG|Ethernet8|3-4": { - "profile": "customized_lossless_profile" + "profile": "customized_ingress_lossless_profile" }, "BUFFER_PG|Ethernet12|0": { "profile": "ingress_lossy_profile" @@ -55,6 +55,11 @@ "BUFFER_PORT_INGRESS_PROFILE_LIST|Ethernet24": { "profile_list": "ingress_lossless_profile,ingress_lossy_profile" }, + "BUFFER_PROFILE|customized_egress_lossless_profile": { + "dynamic_th": "7", + "pool": "egress_lossless_pool", + "size": "0" + }, "BUFFER_PROFILE|egress_lossless_profile": { "dynamic_th": "7", "pool": "egress_lossless_pool", @@ -65,6 +70,11 @@ "pool": "egress_lossy_pool", "size": "9216" }, + "BUFFER_PROFILE|customized_ingress_lossless_profile": { + "dynamic_th": "7", + "pool": "ingress_lossless_pool", + "size": "0" + }, "BUFFER_PROFILE|ingress_lossless_profile": { "dynamic_th": "7", "pool": "ingress_lossless_pool", diff --git a/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-single-pool-expected.json b/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-single-pool-expected.json index 278a40bc0a..3572be8b69 100644 --- a/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-single-pool-expected.json +++ b/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-single-pool-expected.json @@ -12,7 +12,7 @@ "profile": "NULL" }, "BUFFER_PG|Ethernet8|3-4": { - "profile": "customized_lossless_profile" + "profile": "customized_ingress_lossless_profile" }, "BUFFER_PG|Ethernet12|0": { "profile": "ingress_lossy_profile" @@ -99,6 +99,11 @@ "BUFFER_PORT_INGRESS_PROFILE_LIST|Ethernet24": { "profile_list": "ingress_lossless_profile" }, + "BUFFER_PROFILE|customized_egress_lossless_profile": { + "dynamic_th": "7", + "pool": "egress_lossless_pool", + "size": "0" + }, "BUFFER_PROFILE|egress_lossless_profile": { "dynamic_th": "7", "pool": "egress_lossless_pool", @@ -109,6 +114,11 @@ "pool": "egress_lossy_pool", "size": "9216" }, + "BUFFER_PROFILE|customized_ingress_lossless_profile": { + "dynamic_th": "7", + "pool": "ingress_lossless_pool", + "size": "0" + }, "BUFFER_PROFILE|ingress_lossless_profile": { "dynamic_th": "7", "pool": "ingress_lossless_pool", diff --git a/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-single-pool-input.json b/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-single-pool-input.json index b3bda32f23..60f4455cad 100644 --- a/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-single-pool-input.json +++ b/tests/db_migrator_input/config_db/reclaiming-buffer-dynamic-single-pool-input.json @@ -3,7 +3,7 @@ "profile": "NULL" }, "BUFFER_PG|Ethernet8|3-4": { - "profile": "customized_lossless_profile" + "profile": "customized_ingress_lossless_profile" }, "BUFFER_PG|Ethernet12|0": { "profile": "ingress_lossy_profile" @@ -51,6 +51,11 @@ "BUFFER_PORT_INGRESS_PROFILE_LIST|Ethernet24": { "profile_list": "ingress_lossless_profile" }, + "BUFFER_PROFILE|customized_egress_lossless_profile": { + "dynamic_th": "7", + "pool": "egress_lossless_pool", + "size": "0" + }, "BUFFER_PROFILE|egress_lossless_profile": { "dynamic_th": "7", "pool": "egress_lossless_pool", @@ -61,6 +66,11 @@ "pool": "egress_lossy_pool", "size": "9216" }, + "BUFFER_PROFILE|customized_ingress_lossless_profile": { + "dynamic_th": "7", + "pool": "ingress_lossless_pool", + "size": "0" + }, "BUFFER_PROFILE|ingress_lossless_profile": { "dynamic_th": "7", "pool": "ingress_lossless_pool", From 4196bb455970c57b5717c898ea084d120aefb9ef Mon Sep 17 00:00:00 2001 From: ganglyu Date: Fri, 15 Mar 2024 13:47:08 +0800 Subject: [PATCH 2/6] Fix ut error --- tests/db_migrator_input/config_db/switchport-expected.json | 6 ++++++ tests/db_migrator_input/config_db/switchport-input.json | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/tests/db_migrator_input/config_db/switchport-expected.json b/tests/db_migrator_input/config_db/switchport-expected.json index 812abbd58f..1625bcd901 100644 --- a/tests/db_migrator_input/config_db/switchport-expected.json +++ b/tests/db_migrator_input/config_db/switchport-expected.json @@ -72,6 +72,12 @@ "VLAN|Vlan7": { "vlanid": "7" }, + "VLAN|Vlan8": { + "vlanid": "8" + }, + "VLAN|Vlan9": { + "vlanid": "9" + }, "VLAN_MEMBER|Vlan2|Ethernet0": { diff --git a/tests/db_migrator_input/config_db/switchport-input.json b/tests/db_migrator_input/config_db/switchport-input.json index c1ad306ce4..c61b17a822 100644 --- a/tests/db_migrator_input/config_db/switchport-input.json +++ b/tests/db_migrator_input/config_db/switchport-input.json @@ -68,6 +68,12 @@ "VLAN|Vlan7": { "vlanid": "7" }, + "VLAN|Vlan8": { + "vlanid": "8" + }, + "VLAN|Vlan9": { + "vlanid": "9" + }, "VLAN_MEMBER|Vlan2|Ethernet0": { "tagging_mode": "tagged" From 92fd58372f74ef44c2d932b6e69ad7de5d7972f2 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Thu, 21 Mar 2024 13:13:22 +0000 Subject: [PATCH 3/6] Improve validation logic --- scripts/db_migrator.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index 810b0e0818..f4f11cf893 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -1305,7 +1305,9 @@ def validate(self): json.dump(config, fp) process = subprocess.Popen(["config_validator.py", "-c", config_file]) # Check validation result for unit test - if os.environ.get("UTILITIES_UNIT_TESTING", "0") == "2": + # Check validation result when loganalyzer is disabled + disable_loganalyzer_mark = "/etc/sonic/disable_loganalyzer_mark" + if os.environ.get("UTILITIES_UNIT_TESTING", "0") == "2" or os.path.exists(disable_loganalyzer_mark): ret = process.wait() assert ret == 0, "Yang validation failed" From 7fd1d2c755f91ad31dc9de21f7b5b1d58367646a Mon Sep 17 00:00:00 2001 From: ganglyu Date: Mon, 25 Mar 2024 09:49:57 +0800 Subject: [PATCH 4/6] Replace mark file --- scripts/db_migrator.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index f4f11cf893..d1c7d602ea 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -1305,9 +1305,9 @@ def validate(self): json.dump(config, fp) process = subprocess.Popen(["config_validator.py", "-c", config_file]) # Check validation result for unit test - # Check validation result when loganalyzer is disabled - disable_loganalyzer_mark = "/etc/sonic/disable_loganalyzer_mark" - if os.environ.get("UTILITIES_UNIT_TESTING", "0") == "2" or os.path.exists(disable_loganalyzer_mark): + # Check validation result for end to end test + mark_file = "/etc/sonic/mgmt_test_mark" + if os.environ.get("UTILITIES_UNIT_TESTING", "0") == "2" or os.path.exists(mark_file): ret = process.wait() assert ret == 0, "Yang validation failed" From 8f7cb0245892b5614e29bbb235356230c7078249 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Tue, 2 Apr 2024 14:25:17 +0800 Subject: [PATCH 5/6] Check missing yang models --- scripts/config_validator.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/config_validator.py b/scripts/config_validator.py index 0ebacdc651..7290057adc 100755 --- a/scripts/config_validator.py +++ b/scripts/config_validator.py @@ -35,6 +35,9 @@ def main(): except sonic_yang.SonicYangException as e: log.log_error("Yang validation failed: " + str(e)) raise + if len(yang_parser.tablesWithOutYang): + log.log_error("Tables without yang models: " + str(yang_parser.tablesWithOutYang)) + raise Exception("Tables without yang models: " + str(yang_parser.tablesWithOutYang)) if __name__ == "__main__": From 22e52cdd3d4ea03c4e56780f3492b7864b4a1a77 Mon Sep 17 00:00:00 2001 From: ganglyu Date: Fri, 11 Oct 2024 10:39:25 +0800 Subject: [PATCH 6/6] Fix commit error --- scripts/config_validator.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/config_validator.py b/scripts/config_validator.py index 7290057adc..ee5789e95a 100755 --- a/scripts/config_validator.py +++ b/scripts/config_validator.py @@ -15,12 +15,12 @@ def main(): parser = argparse.ArgumentParser() parser.add_argument('-c', - dest='config', - metavar='config file', - type = str, - required = True, - help = 'the config file to be validated', - default = None ) + dest='config', + metavar='config file', + type=str, + required=True, + help='the config file to be validated', + default=None) args = parser.parse_args() config_file = args.config