diff --git a/config/dhcp_relay.py b/config/dhcp_relay.py index 86d11bc82a..6fe740603f 100644 --- a/config/dhcp_relay.py +++ b/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() diff --git a/config/vlan.py b/config/vlan.py index 163e1b02cf..543708b5d6 100644 --- a/config/vlan.py +++ b/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() diff --git a/tests/config_dhcp_relay_test.py b/tests/config_dhcp_relay_test.py index b759d91162..24ea893734 100644 --- a/tests/config_dhcp_relay_test.py +++ b/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 diff --git a/tests/vlan_test.py b/tests/vlan_test.py index 9539bab827..b83f0a8ab5 100644 --- a/tests/vlan_test.py +++ b/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)