[dhcp_relay] Remove exist check while adding dhcpv6 relay (#13822)

Why I did it
DHCPv6 relay config entry is not useful while del dhcpv6 relay config.

How I did it
Remove dhcpv6_relay entry if it is empty and not check entry exist while adding dhcpv6 relay

Author: yaqiangz

dockers/docker-dhcp-relay/cli-plugin-tests/conftest.py

@@ -19,14 +19,23 @@ def mock_cfgdb():
     }
 
     def get_entry(table, key):
+        if table not in CONFIG or key not in CONFIG[table]:
+            return {}
         return CONFIG[table][key]
 
     def set_entry(table, key, data):
         CONFIG[table].setdefault(key, {})
-        CONFIG[table][key] = data
+
+        if data is None:
+            CONFIG[table].pop(key)
+        else:
+            CONFIG[table][key] = data
+
+    def get_keys(table):
+        return CONFIG[table].keys()
 
     cfgdb.get_entry = mock.Mock(side_effect=get_entry)
     cfgdb.set_entry = mock.Mock(side_effect=set_entry)
+    cfgdb.get_keys = mock.Mock(side_effect=get_keys)
 
     yield cfgdb
-

dockers/docker-dhcp-relay/cli-plugin-tests/test_config_dhcp_relay.py

@@ -111,9 +111,25 @@ def test_plugin_registration(self):
         cli = mock.MagicMock()
         dhcp_relay.register(cli)
 
-    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(dhcp_relay.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(dhcp_relay.dhcp_relay.commands[ip_version]
                                    .commands[IP_VER_TEST_PARAM_MAP[ip_version]["command"]]
@@ -262,3 +278,25 @@ def test_config_add_del_duplicate_dhcp_relay(self, mock_cfgdb, 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, mock_cfgdb):
+        op = "add"
+        ip_version = "ipv6"
+        test_ip = IP_VER_TEST_PARAM_MAP[ip_version]["ips"][0]
+        runner = CliRunner()
+        db = Db()
+        db.cfgdb = mock_cfgdb
+        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(dhcp_relay.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

dockers/docker-dhcp-relay/cli/config/plugins/dhcp_relay.py

@@ -22,11 +22,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
@@ -49,7 +51,10 @@ 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:
@@ -100,6 +105,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))
     try: