Subinterface vrf bind issue fix (#2211)

* Add support to configure routed subinterface in short name and long name format
* Update "show subniterface status" to reflect subinterface in user configured long name and short name format.

config/main.py

@@ -4831,6 +4831,11 @@ def bind(ctx, interface_name, vrf_name):
     interface_addresses = get_interface_ipaddresses(config_db, interface_name)
     for ipaddress in interface_addresses:
         remove_router_interface_ip_address(config_db, interface_name, ipaddress)
+    if table_name == "VLAN_SUB_INTERFACE":
+        subintf_entry = config_db.get_entry(table_name, interface_name)
+        if 'vrf_name' in subintf_entry:
+            subintf_entry.pop('vrf_name')
+
     config_db.set_entry(table_name, interface_name, None)
     # When config_db del entry and then add entry with same key, the DEL will lost.
     if ctx.obj['namespace'] is DEFAULT_NAMESPACE:
@@ -4842,7 +4847,11 @@ def bind(ctx, interface_name, vrf_name):
     while state_db.exists(state_db.STATE_DB, _hash):
         time.sleep(0.01)
     state_db.close(state_db.STATE_DB)
-    config_db.set_entry(table_name, interface_name, {"vrf_name": vrf_name})
+    if table_name == "VLAN_SUB_INTERFACE":
+        subintf_entry['vrf_name'] = vrf_name
+        config_db.set_entry(table_name, interface_name, subintf_entry)
+    else:
+        config_db.set_entry(table_name, interface_name, {"vrf_name": vrf_name})
 
 #
 # 'unbind' subcommand
@@ -4864,12 +4873,21 @@ def unbind(ctx, interface_name):
     table_name = get_interface_table_name(interface_name)
     if table_name == "":
         ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]")
+
     if is_interface_bind_to_vrf(config_db, interface_name) is False:
         return
+    if table_name == "VLAN_SUB_INTERFACE":
+        subintf_entry = config_db.get_entry(table_name, interface_name)
+        if 'vrf_name' in subintf_entry:
+            subintf_entry.pop('vrf_name')
+
     interface_ipaddresses = get_interface_ipaddresses(config_db, interface_name)
     for ipaddress in interface_ipaddresses:
         remove_router_interface_ip_address(config_db, interface_name, ipaddress)
-    config_db.set_entry(table_name, interface_name, None)
+    if table_name == "VLAN_SUB_INTERFACE":
+        config_db.set_entry(table_name, interface_name, subintf_entry)
+    else:
+        config_db.set_entry(table_name, interface_name, None)
 
 #
 # 'ipv6' subgroup ('config interface ipv6 ...')
@@ -6601,6 +6619,13 @@ def subintf_vlan_check(config_db, parent_intf, vlan):
                     return True
     return False
 
+def is_subintf_shortname(intf):
+    if VLAN_SUB_INTERFACE_SEPARATOR in intf:
+        if intf.startswith("Ethernet") or intf.startswith("PortChannel"):
+            return False
+        return True
+    return False
+
 @subinterface.command('add')
 @click.argument('subinterface_name', metavar='<subinterface_name>', required=True)
 @click.argument('vid', metavar='<vid>', required=False, type=click.IntRange(1,4094))
@@ -6646,6 +6671,8 @@ def add_subinterface(ctx, subinterface_name, vid):
     subintf_dict = {}
     if vid is not None:
         subintf_dict.update({"vlan" : vid})
+    elif is_subintf_shortname(subinterface_name):
+        ctx.fail("{} Encap vlan is mandatory for short name subinterfaces".format(subinterface_name))
 
     if subintf_vlan_check(config_db, get_intf_longname(interface_alias), vid) is True:
         ctx.fail("Vlan {} encap already configured on other subinterface on {}".format(vid, interface_alias))

tests/intfutil_test.py

@@ -210,7 +210,8 @@ def test_subintf_status(self):
             "Sub port interface    Speed    MTU    Vlan    Admin                  Type\n"
           "--------------------  -------  -----  ------  -------  --------------------\n"
           "            Eth32.10      40G   9100     100       up  802.1q-encapsulation\n"
-          "        Ethernet0.10      25G   9100      10       up  802.1q-encapsulation"
+          "        Ethernet0.10      25G   9100      10       up  802.1q-encapsulation\n"
+          "           Po0001.10      40G   9100     100       up  802.1q-encapsulation"
         )
         self.assertEqual(result.output.strip(), expected_output)
 
@@ -254,6 +255,16 @@ def test_single_subintf_status(self):
         print(output, file=sys.stderr)
         self.assertEqual(output.strip(), expected_output)
 
+        expected_output = (
+            "Sub port interface    Speed    MTU    Vlan    Admin                  Type\n"
+          "--------------------  -------  -----  ------  -------  --------------------\n"
+          "           Po0001.10      40G   9100     100       up  802.1q-encapsulation"
+        )
+        # Test 'intfutil status Po0001.10'
+        output = subprocess.check_output('intfutil -c status -i Po0001.10', stderr=subprocess.STDOUT, shell=True, text=True)
+        print(output, file=sys.stderr)
+        self.assertEqual(output.strip(), expected_output)
+
     # Test '--verbose' status of single sub interface
     def test_single_subintf_status_verbose(self):
         result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Ethernet0.10", "--verbose"])
@@ -266,6 +277,11 @@ def test_single_subintf_status_verbose(self):
         expected_output = "Command: intfutil -c status -i Eth32.10"
         self.assertEqual(result.output.split('\n')[0], expected_output)
 
+        result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Po0001.10", "--verbose"])
+        print(result.output, file=sys.stderr)
+        expected_output = "Command: intfutil -c status -i Po0001.10"
+        self.assertEqual(result.output.split('\n')[0], expected_output)
+
     # Test status of single sub interface in alias naming mode
     def test_single_subintf_status_alias_mode(self):
         os.environ["SONIC_CLI_IFACE_MODE"] = "alias"

tests/ip_config_test.py

@@ -34,12 +34,36 @@ def test_add_del_interface_valid_ipv4(self):
         assert result.exit_code == 0
         assert ('Ethernet64', '10.10.10.1/24') in db.cfgdb.get_table('INTERFACE')
 
+        # config int ip add Ethernet0.10 10.11.10.1/24
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet0.10", "10.11.10.1/24"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert ('Ethernet0.10', '10.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+
+        # config int ip add Eth32.10 32.11.10.1/24
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth32.10", "32.11.10.1/24"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert ('Eth32.10', '32.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+
         # config int ip remove Ethernet64 10.10.10.1/24
         result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet64", "10.10.10.1/24"], obj=obj)
         print(result.exit_code, result.output)
         assert result.exit_code != 0
         assert ('Ethernet64', '10.10.10.1/24') not in db.cfgdb.get_table('INTERFACE')
 
+        # config int ip remove Ethernet0.10 10.11.10.1/24
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet0.10", "10.11.10.1/24"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code != 0
+        assert ('Ethernet0.10', '10.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+
+        # config int ip remove Eth32.10 32.11.10.1/24
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth32.10", "32.11.10.1/24"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code != 0
+        assert ('Eth32.10', '32.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+
     def test_add_interface_invalid_ipv4(self):
         db = Db()
         runner = CliRunner()
@@ -86,12 +110,32 @@ def test_add_del_interface_valid_ipv6(self):
         assert result.exit_code == 0
         assert ('Ethernet72', '2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('INTERFACE')
 
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet0.10", "1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert ('Ethernet0.10', '1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth32.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert ('Eth32.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+
         # config int ip remove Ethernet72 2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34
         result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
         print(result.exit_code, result.output)
         assert result.exit_code != 0
         assert ('Ethernet72', '2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('INTERFACE')
 
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet0.10", "1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code != 0
+        assert ('Ethernet0.10', '1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+
+        result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth32.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code != 0
+        assert ('Eth32.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
+
     def test_del_interface_case_sensitive_ipv6(self):
         db = Db()
         runner = CliRunner()

tests/mock_tables/appl_db.json

@@ -190,8 +190,34 @@
     },
     "INTF_TABLE:Eth32.10": {
         "admin_status": "up",
+        "vrf_name": "Vrf1",
         "vlan": "100"
     },
+    "INTF_TABLE:Po0001.10": {
+        "admin_status": "up",
+        "vrf_name": "Vrf1",
+        "vlan": "100"
+    },
+    "INTF_TABLE:Ethernet0.10|10.11.12.13/24": {
+        "family": "IPv4",
+        "scope": "global"
+    },
+    "INTF_TABLE:Eth32.10|32.10.11.12/24": {
+        "family": "IPv4",
+        "scope": "global"
+    },
+    "INTF_TABLE:Po0001.10|10.10.11.12/24": {
+        "family": "IPv4",
+        "scope": "global"
+    },
+    "INTF_TABLE:Eth32.10|3210::12/126": {
+        "family": "IPv6",
+        "scope": "global"
+    },
+    "INTF_TABLE:Po0001.10|1010::12/126": {
+        "family": "IPv6",
+        "scope": "global"
+    },
     "_GEARBOX_TABLE:phy:1": {
         "name": "sesto-1",
         "phy_id": "1",

tests/mock_tables/config_db.json

@@ -379,11 +379,26 @@
     "VLAN_SUB_INTERFACE|Eth32.10": {
         "admin_status": "up",
         "loopback_action": "drop",
+        "vrf_name": "Vrf1",
         "vlan": "100"
     },
     "VLAN_SUB_INTERFACE|Eth32.10|32.10.11.12/24": {
         "NULL" : "NULL"
     },
+    "VLAN_SUB_INTERFACE|Eth32.10|3210::12/126": {
+        "NULL" : "NULL"
+    },
+    "VLAN_SUB_INTERFACE|Po0001.10": {
+        "admin_status": "up",
+        "vrf_name": "Vrf1",
+        "vlan": "100"
+    },
+    "VLAN_SUB_INTERFACE|Po0001.10|10.10.11.12/24": {
+        "NULL" : "NULL"
+    },
+    "VLAN_SUB_INTERFACE|Po0001.10|1010::12/126": {
+        "NULL" : "NULL"
+    },
     "ACL_RULE|NULL_ROUTE_V4|DEFAULT_RULE": {
         "PACKET_ACTION": "DROP",
         "PRIORITY": "1"

tests/show_vrf_test.py

@@ -4,6 +4,7 @@
 from swsscommon.swsscommon import SonicV2Connector
 from utilities_common.db import Db
 
+import config.main as config
 import show.main as show
 
 test_path = os.path.dirname(os.path.abspath(__file__))
@@ -31,6 +32,92 @@ def test_vrf_show(self):
         Eth32.10
 Vrf103  Ethernet4
         Loopback0
+        Po0002.101
+"""
+
+        result = runner.invoke(show.cli.commands['vrf'], [], obj=db)
+        dbconnector.dedicated_dbs = {}
+        assert result.exit_code == 0
+        assert result.output == expected_output
+
+    def test_vrf_bind_unbind(self):
+        from .mock_tables import dbconnector
+        jsonfile_config = os.path.join(mock_db_path, "config_db")
+        dbconnector.dedicated_dbs['CONFIG_DB'] = jsonfile_config
+        runner = CliRunner()
+        db = Db()
+        expected_output = """\
+VRF     Interfaces
+------  ---------------
+Vrf1
+Vrf101  Ethernet0.10
+Vrf102  PortChannel0002
+        Vlan40
+        Eth32.10
+Vrf103  Ethernet4
+        Loopback0
+        Po0002.101
+"""
+
+        result = runner.invoke(show.cli.commands['vrf'], [], obj=db)
+        dbconnector.dedicated_dbs = {}
+        assert result.exit_code == 0
+        assert result.output == expected_output
+
+        obj = {'config_db':db.cfgdb}
+
+        result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Ethernet4"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert 'Ethernet4' not in db.cfgdb.get_table('INTERFACE')
+
+        result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Loopback0"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert 'Loopback0' not in db.cfgdb.get_table('LOOPBACK_INTERFACE')
+
+        result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Vlan40"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert 'Vlan40' not in db.cfgdb.get_table('VLAN_INTERFACE')
+
+        result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["PortChannel0002"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert 'PortChannel002' not in db.cfgdb.get_table('PORTCHANNEL_INTERFACE')
+
+        result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Eth32.10"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert ('vrf_name', 'Vrf102') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Eth32.10']
+
+        result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Ethernet0.10"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert ('vrf_name', 'Vrf101') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Ethernet0.10']
+
+        result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Po0002.101"], obj=obj)
+        print(result.exit_code, result.output)
+        assert result.exit_code == 0
+        assert ('vrf_name', 'Vrf103') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Po0002.101']
+
+
+        #Bind click CLI cannot be tested as it tries to connecte to statedb
+        #for verification of all IP address delete before applying new vrf configuration
+        jsonfile_config = os.path.join(mock_db_path, "config_db")
+        dbconnector.dedicated_dbs['CONFIG_DB'] = jsonfile_config
+
+        expected_output = """\
+VRF     Interfaces
+------  ---------------
+Vrf1
+Vrf101  Ethernet0.10
+Vrf102  PortChannel0002
+        Vlan40
+        Eth32.10
+Vrf103  Ethernet4
+        Loopback0
+        Po0002.101
 """
 
         result = runner.invoke(show.cli.commands['vrf'], [], obj=db)