Run yang validation in db migrator (#3102)

What I did Add unit test/runtime alert for db_migrator.py/load_minigraph: ConfigDB passing yang validation How I did it Use subprocess to run yang validation, and then warm-reboot performance will not change. Check /etc/sonic/mgmt_test_mark, this mark_file means this device is under end to end test. For unit test, db_migrator will wait for validation result and raise exception. For end to end test, db_migrator will wait for validation result and raise exception. How to verify it Run unit test and run end to end test
sonic-net · Nov 13, 2024 · 9677447 · 9677447
1 parent 9708f52
commit 9677447
Show file tree

Hide file tree

Showing 13 changed files with 142 additions and 20 deletions.
diff --git a/scripts/config_validator.py b/scripts/config_validator.py
@@ -0,0 +1,44 @@
+#!/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 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__":
+    main()
diff --git 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, SonicDBConfig
@@ -1304,6 +1305,34 @@ 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
+        # 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"
 
 def main():
     try:

diff --git a/setup.py b/setup.py
@@ -116,6 +116,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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",