Skip to content

Commit

Permalink
[dhcp_relay] Remove exist check while adding dhcpv6 relay (#13822)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
yaqiangz authored and mssonicbld committed Feb 17, 2023
1 parent cf5ca9d commit 928aad1
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 8 deletions.
13 changes: 11 additions & 2 deletions dockers/docker-dhcp-relay/cli-plugin-tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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"]]
Expand Down Expand Up @@ -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
18 changes: 13 additions & 5 deletions dockers/docker-dhcp-relay/cli/config/plugins/dhcp_relay.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 928aad1

Please sign in to comment.