Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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