[202012][vlan] Remove add field of vlanid to DHCP_RELAY table while a…

…dding vlan (#2681)

What I did
Remove add field of vlanid to DHCP_RELAY table while add vlan which would cause conflict with yang model.

How I did it
Remove add field of vlanid to DHCP_RELAY table while add vlan

Signed-off-by: Yaqiang Zhu <[email protected]>

config/dhcp_relay.py

@@ -24,11 +24,13 @@ def validate_ips(ctx, ips, ip_version):
             ctx.fail("{} is not IPv{} address".format(ip, ip_version))
 
 
-def get_dhcp_servers(db, vlan_name, ctx, table_name, dhcp_servers_str):
-    table = db.cfgdb.get_entry(table_name, vlan_name)
-    if len(table.keys()) == 0:
-        ctx.fail("{} doesn't exist".format(vlan_name))
+def get_dhcp_servers(db, vlan_name, ctx, table_name, dhcp_servers_str, check_is_exist=True):
+    if check_is_exist:
+        keys = db.cfgdb.get_keys(table_name)
+        if vlan_name not in keys:
+            ctx.fail("{} doesn't exist".format(vlan_name))
 
+    table = db.cfgdb.get_entry(table_name, vlan_name)
     dhcp_servers = table.get(dhcp_servers_str, [])
 
     return dhcp_servers, table
@@ -46,7 +48,9 @@ def add_dhcp_relay(vid, dhcp_relay_ips, db, ip_version):
     ctx = click.get_current_context()
     # Verify ip addresses are valid
     validate_ips(ctx, dhcp_relay_ips, ip_version)
-    dhcp_servers, table = get_dhcp_servers(db, vlan_name, ctx, table_name, dhcp_servers_str)
+    # It's unnecessary for DHCPv6 Relay to verify entry exist
+    check_config_exist = True if ip_version == 4 else False
+    dhcp_servers, table = get_dhcp_servers(db, vlan_name, ctx, table_name, dhcp_servers_str, check_config_exist)
     added_ips = []
 
     for dhcp_relay_ip in dhcp_relay_ips:
@@ -93,6 +97,9 @@ def del_dhcp_relay(vid, dhcp_relay_ips, db, ip_version):
     else:
         table[dhcp_servers_str] = dhcp_servers
 
+    if ip_version == 6 and len(table.keys()) == 0:
+        table = None
+
     db.cfgdb.set_entry(table_name, vlan_name, table)
     click.echo("Removed DHCP relay address [{}] from {}".format(",".join(dhcp_relay_ips), vlan_name))
     dhcp_relay_util.handle_restart_dhcp_relay_service()

config/vlan.py

@@ -40,7 +40,7 @@ def add_vlan(db, vid):
     set_dhcp_relay_table('VLAN', db.cfgdb, vlan, {'vlanid': str(vid)})
 
     # set dhcpv6_relay table
-    set_dhcp_relay_table('DHCP_RELAY', db.cfgdb, vlan, {'vlanid': str(vid)})
+    set_dhcp_relay_table('DHCP_RELAY', db.cfgdb, vlan, None)
     # We need to restart dhcp_relay service after dhcpv6_relay config change
     dhcp_relay_util.handle_restart_dhcp_relay_service()
 

tests/config_dhcp_relay_test.py

@@ -121,9 +121,25 @@ def setup_class(cls):
         os.environ['UTILITIES_UNIT_TESTING'] = "1"
         print("SETUP")
 
-    def test_config_dhcp_relay_add_del_with_nonexist_vlanid(self, ip_version, op):
+    def test_config_dhcp_relay_add_del_with_nonexist_vlanid_ipv4(self, op):
         runner = CliRunner()
 
+        ip_version = "ipv4"
+        with mock.patch("utilities_common.cli.run_command") as mock_run_command:
+            result = runner.invoke(config.config.commands["dhcp_relay"].commands[ip_version]
+                                   .commands[IP_VER_TEST_PARAM_MAP[ip_version]["command"]]
+                                   .commands[op], ["1001", IP_VER_TEST_PARAM_MAP[ip_version]["ips"][0]])
+            print(result.exit_code)
+            print(result.stdout)
+            assert result.exit_code != 0
+            assert "Error: Vlan1001 doesn't exist" in result.output
+            assert mock_run_command.call_count == 0
+
+    def test_config_dhcp_relay_del_with_nonexist_vlanid_ipv6(self):
+        runner = CliRunner()
+
+        op = "del"
+        ip_version = "ipv6"
         with mock.patch("utilities_common.cli.run_command") as mock_run_command:
             result = runner.invoke(config.config.commands["dhcp_relay"].commands[ip_version]
                                    .commands[IP_VER_TEST_PARAM_MAP[ip_version]["command"]]
@@ -257,3 +273,24 @@ def test_config_add_del_duplicate_dhcp_relay(self, ip_version, op):
             assert result.exit_code != 0
             assert "Error: Find duplicate DHCP relay ip {} in {} list".format(test_ip, op) in result.output
             assert mock_run_command.call_count == 0
+
+    def test_config_add_dhcp_relay_ipv6_with_non_entry(self):
+        op = "add"
+        ip_version = "ipv6"
+        test_ip = IP_VER_TEST_PARAM_MAP[ip_version]["ips"][0]
+        runner = CliRunner()
+        db = Db()
+        table = IP_VER_TEST_PARAM_MAP[ip_version]["table"]
+        db.cfgdb.set_entry(table, "Vlan1000", None)
+        assert db.cfgdb.get_entry(table, "Vlan1000") == {}
+        assert len(db.cfgdb.get_keys(table)) == 0
+
+        with mock.patch("utilities_common.cli.run_command") as mock_run_command:
+            result = runner.invoke(config.config.commands["dhcp_relay"].commands[ip_version]
+                                   .commands[IP_VER_TEST_PARAM_MAP[ip_version]["command"]]
+                                   .commands[op], ["1000", test_ip], obj=db)
+            print(result.exit_code)
+            print(result.output)
+            assert result.exit_code == 0
+            assert db.cfgdb.get_entry(table, "Vlan1000") == {"dhcpv6_servers": [test_ip]}
+            assert mock_run_command.call_count == 3

tests/vlan_test.py

@@ -857,7 +857,8 @@ def test_config_add_del_vlan_dhcp_relay(self, ip_version, mock_restart_dhcp_rela
         print(result.output)
         assert result.exit_code == 0
 
-        assert db.cfgdb.get_entry(IP_VERSION_PARAMS_MAP[ip_version]["table"], "Vlan1001") == DHCP_RELAY_TABLE_ENTRY
+        exp_output = {"vlanid": "1001"} if ip_version == "ipv4" else {}
+        assert db.cfgdb.get_entry(IP_VERSION_PARAMS_MAP[ip_version]["table"], "Vlan1001") == exp_output
 
         # del vlan 1001
         result = runner.invoke(config.config.commands["vlan"].commands["del"], ["1001"], obj=db)