QOS fieldvalue refernce ABNF format to string (#1626)

Qos tables in config db and app db used ABNF format i.e "[TABLE_NAME|name] to refer fieldvalue other qos tables.

Example:
Config DB:
"Ethernet92|3": {
"scheduler": "[SCHEDULER|scheduler.1]",
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]"
},
"Ethernet0|0": {
"profile": "[BUFFER_PROFILE|ingress_lossy_profile]"
},
"Ethernet0": {
"dscp_to_tc_map": "[DSCP_TO_TC_MAP|AZURE]",
"pfc_enable": "3,4",
"pfc_to_queue_map": "[MAP_PFC_PRIORITY_TO_QUEUE|AZURE]",
"tc_to_pg_map": "[TC_TO_PRIORITY_GROUP_MAP|AZURE]",
"tc_to_queue_map": "[TC_TO_QUEUE_MAP|AZURE]"
},

AppDB:
"BUFFER_QUEUE_TABLE:Ethernet88:3-4": {
"profile": "[BUFFER_PROFILE_TABLE:egress_lossless_profile]"
},

1#This format is not consistent with other DB schema followed in sonic.
2# Added db_migrator.py case to  change from old format in config_db and appl_db  to new format. 
3#Modified the test case 

Dependent pull requests: 
sonic-net/sonic-buildimage#7752  - To modify platfrom files 
sonic-net/sonic-buildimage#7281 - Yang model 
sonic-net/sonic-swss#1754    - swss change to remove ABNF format

config/main.py

@@ -3786,8 +3786,7 @@ def update_pg(ctx, interface_name, pg_map, override_profile, add = True):
             ctx.fail("Profile {} doesn't exist".format(override_profile))
         if not 'xoff' in profile_dict.keys() and 'size' in profile_dict.keys():
             ctx.fail("Profile {} doesn't exist or isn't a lossless profile".format(override_profile))
-        profile_full_name = "[BUFFER_PROFILE|{}]".format(override_profile)
-        config_db.set_entry("BUFFER_PG", (interface_name, pg_map), {"profile": profile_full_name})
+        config_db.set_entry("BUFFER_PG", (interface_name, pg_map), {"profile": override_profile})
     else:
         config_db.set_entry("BUFFER_PG", (interface_name, pg_map), {"profile": "NULL"})
     adjust_pfc_enable(ctx, interface_name, pg_map, True)
@@ -3809,7 +3808,7 @@ def remove_pg_on_port(ctx, interface_name, pg_map):
         if port == interface_name and (not pg_map or pg_map == existing_pg):
             need_to_remove = False
             referenced_profile = v.get('profile')
-            if referenced_profile and referenced_profile == '[BUFFER_PROFILE|ingress_lossy_profile]':
+            if referenced_profile and referenced_profile == 'ingress_lossy_profile':
                 if pg_map:
                     ctx.fail("Lossy PG {} can't be removed".format(pg_map))
                 else:
@@ -4963,7 +4962,7 @@ def update_profile(ctx, config_db, profile_name, xon, xoff, size, dynamic_th, po
 
     if not pool:
         pool = 'ingress_lossless_pool'
-    params['pool'] = '[BUFFER_POOL|' + pool + ']'
+    params['pool'] = pool
     if not config_db.get_entry('BUFFER_POOL', pool):
         ctx.fail("Pool {} doesn't exist".format(pool))
 
@@ -5036,12 +5035,11 @@ def remove_profile(db, profile):
     config_db = db.cfgdb
     ctx = click.get_current_context()
 
-    full_profile_name = '[BUFFER_PROFILE|{}]'.format(profile)
     existing_pgs = config_db.get_table("BUFFER_PG")
     for k, v in existing_pgs.items():
         port, pg = k
         referenced_profile = v.get('profile')
-        if referenced_profile and referenced_profile == full_profile_name:
+        if referenced_profile and referenced_profile == profile:
             ctx.fail("Profile {} is referenced by {}|{} and can't be removed".format(profile, port, pg))
 
     entry = config_db.get_entry("BUFFER_PROFILE", profile)

scripts/db_migrator.py

@@ -44,7 +44,7 @@ def __init__(self, namespace, socket=None):
                      none-zero values.
               build: sequentially increase within a minor version domain.
         """
-        self.CURRENT_VERSION = 'version_2_0_2'
+        self.CURRENT_VERSION = 'version_2_0_3'
 
         self.TABLE_NAME      = 'VERSIONS'
         self.TABLE_KEY       = 'DATABASE'
@@ -60,9 +60,11 @@ def __init__(self, namespace, socket=None):
             self.configDB = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace, **db_kwargs)
         self.configDB.db_connect('CONFIG_DB')
 
-        self.appDB = SonicV2Connector(host='127.0.0.1')
-        if self.appDB is not None:
-            self.appDB.connect(self.appDB.APPL_DB)
+        if namespace is None:
+            self.appDB = ConfigDBConnector(**db_kwargs)
+        else:
+            self.appDB = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace, **db_kwargs)
+        self.appDB.db_connect('APPL_DB')
 
         self.stateDB = SonicV2Connector(host='127.0.0.1')
         if self.stateDB is not None:
@@ -382,6 +384,69 @@ def migrate_config_db_port_table_for_auto_neg(self):
                 elif value['autoneg'] == '0':
                     self.configDB.set(self.configDB.CONFIG_DB, '{}|{}'.format(table_name, key), 'autoneg', 'off')
 
+    def migrate_qos_db_fieldval_reference_remove(self, table_list, db, db_num, db_delimeter):
+        for pair in table_list:
+            table_name, fields_list = pair
+            qos_table = db.get_table(table_name)
+            for key, value in qos_table.items():
+                if type(key) is tuple:
+                    db_key = table_name + db_delimeter + db_delimeter.join(key)
+                else:
+                    db_key = table_name + db_delimeter + key
+
+                for field in fields_list:
+                    if field in value:
+                        fieldVal = value.get(field)
+                        if not fieldVal or fieldVal == "NULL":
+                            continue
+                        newFiledVal = ""
+                        # Check for ABNF format presence and convert ABNF to string
+                        if "[" in fieldVal and db_delimeter in fieldVal and "]" in fieldVal:
+                            log.log_info("Found ABNF format field value in table {} key {} field {} val {}".format(table_name, db_key, field, fieldVal))
+                            value_list = fieldVal.split(",")
+                            for item in value_list:
+                                if "[" != item[0] or db_delimeter not in item or "]" != item[-1]:
+                                    continue
+                                newFiledVal = newFiledVal + item[1:-1].split(db_delimeter)[1] + ','
+                            newFiledVal = newFiledVal[:-1]
+                            db.set(db_num, db_key, field, newFiledVal)
+                            log.log_info("Modified ABNF format field value to string in table {} key {} field {} val {}".format(table_name, db_key, field, newFiledVal))
+        return True
+
+    def migrate_qos_fieldval_reference_format(self):
+        '''
+        This is to change for first time to remove field refernces of ABNF format
+        in APPL DB for warm boot.
+        i.e "[Tabale_name:name]" to string in APPL_DB. Reasons for doing this
+         - To consistent with all other SoNIC CONFIG_DB/APPL_DB tables and fields
+         - References in DB is not required, this will be taken care by YANG model leafref.
+        '''
+        qos_app_table_list = [
+            ('BUFFER_PG_TABLE', ['profile']),
+            ('BUFFER_QUEUE_TABLE', ['profile']),
+            ('BUFFER_PROFILE_TABLE', ['pool']),
+            ('BUFFER_PORT_INGRESS_PROFILE_LIST_TABLE', ['profile_list']),
+            ('BUFFER_PORT_EGRESS_PROFILE_LIST_TABLE', ['profile_list'])
+        ]
+
+        log.log_info("Remove APPL_DB QOS tables field reference ABNF format")
+        self.migrate_qos_db_fieldval_reference_remove(qos_app_table_list, self.appDB, self.appDB.APPL_DB, ':')
+
+        qos_table_list = [
+            ('QUEUE', ['scheduler', 'wred_profile']),
+            ('PORT_QOS_MAP', ['dscp_to_tc_map', 'dot1p_to_tc_map',
+                              'pfc_to_queue_map', 'tc_to_pg_map',
+                              'tc_to_queue_map', 'pfc_to_pg_map']),
+            ('BUFFER_PG', ['profile']),
+            ('BUFFER_QUEUE', ['profile']),
+            ('BUFFER_PROFILE', ['pool']),
+            ('BUFFER_PORT_INGRESS_PROFILE_LIST', ['profile_list']),
+            ('BUFFER_PORT_EGRESS_PROFILE_LIST', ['profile_list'])
+        ]
+        log.log_info("Remove CONFIG_DB QOS tables field reference ABNF format")
+        self.migrate_qos_db_fieldval_reference_remove(qos_table_list, self.configDB, self.configDB.CONFIG_DB, '|')
+        return True
+
     def version_unknown(self):
         """
         version_unknown tracks all SONiC versions that doesn't have a version
@@ -542,9 +607,19 @@ def version_2_0_1(self):
 
     def version_2_0_2(self):
         """
-        Current latest version. Nothing to do here.
+        Version 2_0_2.
         """
         log.log_info('Handling version_2_0_2')
+        self.migrate_qos_fieldval_reference_format()
+        self.set_version('version_2_0_3')
+        return 'version_2_0_3'
+
+
+    def version_2_0_3(self):
+        """
+        Current latest version. Nothing to do here.
+        """
+        log.log_info('Handling version_2_0_3')
         return None
 
     def get_version(self):

tests/buffer_input/buffer_test_vectors.py

@@ -31,27 +31,27 @@
 ----  -------
 
 Profile: ingress_lossy_profile
-----------  --------------------------------
+----------  ------------------
 dynamic_th  3
-pool        [BUFFER_POOL|ingress_lossy_pool]
+pool        ingress_lossy_pool
 size        0
-----------  --------------------------------
+----------  ------------------
 
 Profile: headroom_profile
-----------  -----------------------------------
+----------  ---------------------
 dynamic_th  0
-pool        [BUFFER_POOL|ingress_lossless_pool]
+pool        ingress_lossless_pool
 xon         18432
 xoff        32768
 size        51200
-----------  -----------------------------------
+----------  ---------------------
 
 Profile: alpha_profile
--------------  -----------------------------------
+-------------  ---------------------
 dynamic_th     0
-pool           [BUFFER_POOL|ingress_lossless_pool]
+pool           ingress_lossless_pool
 headroom_type  dynamic
--------------  -----------------------------------
+-------------  ---------------------
 
 """
 
@@ -85,20 +85,20 @@
 ----  -------
 
 Profile: ingress_lossy_profile
-----------  --------------------------------------
+----------  ------------------
 dynamic_th  3
-pool        [BUFFER_POOL_TABLE|ingress_lossy_pool]
+pool        ingress_lossy_pool
 size        0
-----------  --------------------------------------
+----------  ------------------
 
 Profile: headroom_profile
-----------  -----------------------------------------
+----------  ---------------------
 dynamic_th  0
-pool        [BUFFER_POOL_TABLE|ingress_lossless_pool]
+pool        ingress_lossless_pool
 xon         18432
 xoff        32768
 size        51200
-----------  -----------------------------------------
+----------  ---------------------
 
 """
 

tests/buffer_test.py

@@ -34,7 +34,7 @@ def test_config_buffer_profile_headroom(self):
         print(result.output)
         assert result.exit_code == 0
         profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'testprofile')
-        assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '32768', 'size': '18432'}
+        assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '32768', 'size': '18432'}
 
     def test_config_buffer_profile_dynamic_th(self):
         runner = CliRunner()
@@ -45,7 +45,7 @@ def test_config_buffer_profile_dynamic_th(self):
         print(result.output)
         assert result.exit_code == 0
         profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'testprofile')
-        assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'headroom_type': 'dynamic'}
+        assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'headroom_type': 'dynamic'}
 
     def test_config_buffer_profile_add_existing(self):
         runner = CliRunner()
@@ -164,7 +164,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
         print(result.output)
         assert result.exit_code == 0
         profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test1')
-        assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '32768', 'size': '51200'}
+        assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '32768', 'size': '51200'}
 
         # Xoff should equal size - xon
         result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"],
@@ -173,7 +173,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
         print(result.output)
         assert result.exit_code == 0
         profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test2')
-        assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '14336', 'size': '32768'}
+        assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '14336', 'size': '32768'}
 
         # Neither xon nor size is provided
         result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"],
@@ -198,7 +198,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
         print(result.output)
         assert result.exit_code == 0
         profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test2')
-        assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '14336', 'size': '65536'}
+        assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '14336', 'size': '65536'}
 
         # Set xon
         result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["set"],
@@ -207,7 +207,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
         print(result.output)
         assert result.exit_code == 0
         profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test2')
-        assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '19456', 'xoff': '14336', 'size': '65536'}
+        assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '19456', 'xoff': '14336', 'size': '65536'}
 
         # Set xoff
         result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["set"],
@@ -216,7 +216,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
         print(result.output)
         assert result.exit_code == 0
         profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test2')
-        assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '19456', 'xoff': '18432', 'size': '65536'}
+        assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '19456', 'xoff': '18432', 'size': '65536'}
 
         # Enable SHP by setting size
         result = runner.invoke(config.config.commands["buffer"].commands["shared-headroom-pool"].commands["size"],
@@ -232,7 +232,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
         print(result.output)
         assert result.exit_code == 0
         profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'testprofile3')
-        assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '32768', 'size': '18432'}
+        assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '32768', 'size': '18432'}
 
         # Negative test: xoff not provided
         result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"],