Skip to content

Commit

Permalink
Revert "Revert "[DHCPv6] [202012] Update the dhcpv6_relay config/show…
Browse files Browse the repository at this point in the history
… cli (sonic-net#2271)" (sonic-net#2336)"

This reverts commit c9aa65c.
  • Loading branch information
vivekrnv committed Oct 26, 2022
1 parent 94a3436 commit c4c3991
Show file tree
Hide file tree
Showing 5 changed files with 164 additions and 17 deletions.
39 changes: 29 additions & 10 deletions config/vlan.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,13 +209,21 @@ def add_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip):
if len(vlan) == 0:
ctx.fail("{} doesn't exist".format(vlan_name))

dhcp_relay_dests = vlan.get('dhcp_servers', [])
if dhcp_relay_destination_ip in dhcp_relay_dests:
# Verify all ip addresses are valid and not exist in DB
dhcp_servers = vlan.get('dhcp_servers', [])
dhcpv6_servers = vlan.get('dhcpv6_servers', [])

if dhcp_relay_destination_ip in dhcp_servers + dhcpv6_servers:
click.echo("{} is already a DHCP relay destination for {}".format(dhcp_relay_destination_ip, vlan_name))
return

dhcp_relay_dests.append(dhcp_relay_destination_ip)
vlan['dhcp_servers'] = dhcp_relay_dests
if clicommon.ipaddress_type(dhcp_relay_destination_ip) == 6:
dhcpv6_servers.append(dhcp_relay_destination_ip)
vlan['dhcpv6_servers'] = dhcpv6_servers
else:
dhcp_servers.append(dhcp_relay_destination_ip)
vlan['dhcp_servers'] = dhcp_servers

db.cfgdb.set_entry('VLAN', vlan_name, vlan)
click.echo("Added DHCP relay destination address {} to {}".format(dhcp_relay_destination_ip, vlan_name))
try:
Expand Down Expand Up @@ -243,15 +251,26 @@ def del_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip):
if len(vlan) == 0:
ctx.fail("{} doesn't exist".format(vlan_name))

dhcp_relay_dests = vlan.get('dhcp_servers', [])
if not dhcp_relay_destination_ip in dhcp_relay_dests:
# Remove dhcp servers if they exist in the DB
dhcp_servers = vlan.get('dhcp_servers', [])
dhcpv6_servers = vlan.get('dhcpv6_servers', [])

if not dhcp_relay_destination_ip in dhcp_servers + dhcpv6_servers:
ctx.fail("{} is not a DHCP relay destination for {}".format(dhcp_relay_destination_ip, vlan_name))

dhcp_relay_dests.remove(dhcp_relay_destination_ip)
if len(dhcp_relay_dests) == 0:
del vlan['dhcp_servers']
if clicommon.ipaddress_type(dhcp_relay_destination_ip) == 6:
dhcpv6_servers.remove(dhcp_relay_destination_ip)
if len(dhcpv6_servers) == 0:
del vlan['dhcpv6_servers']
else:
vlan['dhcpv6_servers'] = dhcpv6_servers
else:
vlan['dhcp_servers'] = dhcp_relay_dests
dhcp_servers.remove(dhcp_relay_destination_ip)
if len(dhcp_servers) == 0:
del vlan['dhcp_servers']
else:
vlan['dhcp_servers'] = dhcp_servers

db.cfgdb.set_entry('VLAN', vlan_name, vlan)
click.echo("Removed DHCP relay destination address {} from {}".format(dhcp_relay_destination_ip, vlan_name))
try:
Expand Down
9 changes: 4 additions & 5 deletions show/vlan.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ def brief(db, verbose):

# Parsing DHCP Helpers info
for key in natsorted(list(vlan_dhcp_helper_data.keys())):
try:
if vlan_dhcp_helper_data[key]['dhcp_servers']:
vlan_dhcp_helper_dict[key.strip('Vlan')] = vlan_dhcp_helper_data[key]['dhcp_servers']
except KeyError:
vlan_dhcp_helper_dict[key.strip('Vlan')] = " "
dhcp_helpers = vlan_dhcp_helper_data.get(key, {}).get("dhcp_servers", [])
dhcpv6_helpers = vlan_dhcp_helper_data.get(key, {}).get("dhcpv6_servers", [])
all_helpers = dhcp_helpers + dhcpv6_helpers
vlan_dhcp_helper_dict[key.strip('Vlan')] = all_helpers

# Parsing VLAN Gateway info
for key in vlan_ip_data:
Expand Down
1 change: 1 addition & 0 deletions tests/mock_tables/config_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@
},
"VLAN|Vlan1000": {
"dhcp_servers@": "192.0.0.1,192.0.0.2,192.0.0.3,192.0.0.4",
"dhcpv6_servers@": "fc02:2000::1,fc02:2000::2",
"vlanid": "1000"
},
"VLAN|Vlan2000": {
Expand Down
119 changes: 118 additions & 1 deletion tests/vlan_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
| | fc02:1000::1/64 | Ethernet8 | untagged | 192.0.0.2 | |
| | | Ethernet12 | untagged | 192.0.0.3 | |
| | | Ethernet16 | untagged | 192.0.0.4 | |
| | | | | fc02:2000::1 | |
| | | | | fc02:2000::2 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 2000 | 192.168.0.10/21 | Ethernet24 | untagged | 192.0.0.1 | enabled |
| | fc02:1011::1/64 | Ethernet28 | untagged | 192.0.0.2 | |
Expand All @@ -36,6 +38,8 @@
| | fc02:1000::1/64 | etp3 | untagged | 192.0.0.2 | |
| | | etp4 | untagged | 192.0.0.3 | |
| | | etp5 | untagged | 192.0.0.4 | |
| | | | | fc02:2000::1 | |
| | | | | fc02:2000::2 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 2000 | 192.168.0.10/21 | etp7 | untagged | 192.0.0.1 | enabled |
| | fc02:1011::1/64 | etp8 | untagged | 192.0.0.2 | |
Expand Down Expand Up @@ -71,7 +75,8 @@
| | fc02:1000::1/64 | Ethernet8 | untagged | 192.0.0.2 | |
| | | Ethernet12 | untagged | 192.0.0.3 | |
| | | Ethernet16 | untagged | 192.0.0.4 | |
| | | PortChannel1001 | untagged | | |
| | | PortChannel1001 | untagged | fc02:2000::1 | |
| | | | | fc02:2000::2 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 2000 | 192.168.0.10/21 | Ethernet24 | untagged | 192.0.0.1 | enabled |
| | fc02:1011::1/64 | Ethernet28 | untagged | 192.0.0.2 | |
Expand Down Expand Up @@ -120,6 +125,16 @@
Restarting DHCP relay service...
"""

config_vlan_add_dhcp_relayv6_output="""\
Added DHCP relay destination address fc02:2000::3 to Vlan1000
Restarting DHCP relay service...
"""

config_vlan_del_dhcp_relayv6_output="""\
Removed DHCP relay destination address fc02:2000::3 from Vlan1000
Restarting DHCP relay service...
"""

show_vlan_brief_output_with_new_dhcp_relay_address="""\
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| VLAN ID | IP Address | Ports | Port Tagging | DHCP Helper Address | Proxy ARP |
Expand All @@ -129,6 +144,31 @@
| | | Ethernet12 | untagged | 192.0.0.3 | |
| | | Ethernet16 | untagged | 192.0.0.4 | |
| | | | | 192.0.0.100 | |
| | | | | fc02:2000::1 | |
| | | | | fc02:2000::2 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 2000 | 192.168.0.10/21 | Ethernet24 | untagged | 192.0.0.1 | enabled |
| | fc02:1011::1/64 | Ethernet28 | untagged | 192.0.0.2 | |
| | | | | 192.0.0.3 | |
| | | | | 192.0.0.4 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 3000 | | | | | disabled |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 4000 | | PortChannel1001 | tagged | | disabled |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
"""

show_vlan_brief_output_with_new_dhcp_relayv6_address="""\
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| VLAN ID | IP Address | Ports | Port Tagging | DHCP Helper Address | Proxy ARP |
+===========+=================+=================+================+=======================+=============+
| 1000 | 192.168.0.1/21 | Ethernet4 | untagged | 192.0.0.1 | disabled |
| | fc02:1000::1/64 | Ethernet8 | untagged | 192.0.0.2 | |
| | | Ethernet12 | untagged | 192.0.0.3 | |
| | | Ethernet16 | untagged | 192.0.0.4 | |
| | | | | fc02:2000::1 | |
| | | | | fc02:2000::2 | |
| | | | | fc02:2000::3 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 2000 | 192.168.0.10/21 | Ethernet24 | untagged | 192.0.0.1 | enabled |
| | fc02:1011::1/64 | Ethernet28 | untagged | 192.0.0.2 | |
Expand All @@ -149,6 +189,8 @@
| | fc02:1000::1/64 | Ethernet8 | untagged | 192.0.0.2 | |
| | | Ethernet12 | untagged | 192.0.0.3 | |
| | | Ethernet16 | untagged | 192.0.0.4 | |
| | | | | fc02:2000::1 | |
| | | | | fc02:2000::2 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 1001 | | Ethernet20 | untagged | | disabled |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
Expand All @@ -171,6 +213,8 @@
| | fc02:1000::1/64 | etp3 | untagged | 192.0.0.2 | |
| | | etp4 | untagged | 192.0.0.3 | |
| | | etp5 | untagged | 192.0.0.4 | |
| | | | | fc02:2000::1 | |
| | | | | fc02:2000::2 | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 1001 | | etp6 | untagged | | disabled |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
Expand Down Expand Up @@ -535,6 +579,19 @@ def test_config_vlan_add_dhcp_relay_with_invalid_ip(self):
assert result.exit_code != 0
assert "Error: 192.0.0.1000 is invalid IP address" in result.output
assert mock_run_command.call_count == 0

def test_config_vlan_add_dhcp_relay_with_invalid_ipv6(self):
runner = CliRunner()

with mock.patch('utilities_common.cli.run_command') as mock_run_command:
result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
["1000", "fe80:2030:31:24"])
print(result.exit_code)
print(result.output)
# traceback.print_tb(result.exc_info[2])
assert result.exit_code != 0
assert "Error: fe80:2030:31:24 is invalid IP address" in result.output
assert mock_run_command.call_count == 0

def test_config_vlan_add_dhcp_relay_with_invalid_ip_2(self):
runner = CliRunner()
Expand All @@ -561,6 +618,19 @@ def test_config_vlan_add_dhcp_relay_with_exist_ip(self):
assert result.exit_code == 0
assert "192.0.0.1 is already a DHCP relay destination for Vlan1000" in result.output
assert mock_run_command.call_count == 0

def test_config_vlan_add_dhcp_relay_with_exist_ipv6(self):
runner = CliRunner()

with mock.patch('utilities_common.cli.run_command') as mock_run_command:
result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
["1000", "fc02:2000::1"])
print(result.exit_code)
print(result.output)
# traceback.print_tb(result.exc_info[2])
assert result.exit_code == 0
assert "fc02:2000::1 is already a DHCP relay destination for Vlan1000" in result.output
assert mock_run_command.call_count == 0

def test_config_vlan_add_del_dhcp_relay_dest(self):
runner = CliRunner()
Expand Down Expand Up @@ -596,6 +666,40 @@ def test_config_vlan_add_del_dhcp_relay_dest(self):
print(result.output)
assert result.output == show_vlan_brief_output

def test_config_vlan_add_del_dhcp_relayv6_dest(self):
runner = CliRunner()
db = Db()

# add new relay dest
with mock.patch("utilities_common.cli.run_command") as mock_run_command:
result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["add"],
["1000", "fc02:2000::3"], obj=db)
print(result.exit_code)
print(result.output)
assert result.exit_code == 0
assert result.output == config_vlan_add_dhcp_relayv6_output
assert mock_run_command.call_count == 3

# show output
result = runner.invoke(show.cli.commands["vlan"].commands["brief"], [], obj=db)
print(result.output)
assert result.output == show_vlan_brief_output_with_new_dhcp_relayv6_address

# del relay dest
with mock.patch("utilities_common.cli.run_command") as mock_run_command:
result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"],
["1000", "fc02:2000::3"], obj=db)
print(result.exit_code)
print(result.output)
assert result.exit_code == 0
assert result.output == config_vlan_del_dhcp_relayv6_output
assert mock_run_command.call_count == 3

# show output
result = runner.invoke(show.cli.commands["vlan"].commands["brief"], [], obj=db)
print(result.output)
assert result.output == show_vlan_brief_output

def test_config_vlan_remove_nonexist_dhcp_relay_dest(self):
runner = CliRunner()

Expand All @@ -608,6 +712,19 @@ def test_config_vlan_remove_nonexist_dhcp_relay_dest(self):
assert result.exit_code != 0
assert "Error: 192.0.0.100 is not a DHCP relay destination for Vlan1000" in result.output
assert mock_run_command.call_count == 0

def test_config_vlan_remove_nonexist_dhcp_relayv6_dest(self):
runner = CliRunner()

with mock.patch('utilities_common.cli.run_command') as mock_run_command:
result = runner.invoke(config.config.commands["vlan"].commands["dhcp_relay"].commands["del"],
["1000", "fc02:2000::3"])
print(result.exit_code)
print(result.output)
# traceback.print_tb(result.exc_info[2])
assert result.exit_code != 0
assert "Error: fc02:2000::3 is not a DHCP relay destination for Vlan1000" in result.output
assert mock_run_command.call_count == 0

def test_config_vlan_remove_dhcp_relay_dest_with_nonexist_vlanid(self):
runner = CliRunner()
Expand Down
13 changes: 12 additions & 1 deletion utilities_common/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import click
import json
import netaddr

from natsort import natsorted
from sonic_py_common import multi_asic
Expand Down Expand Up @@ -190,7 +191,6 @@ def get_interface_naming_mode():

def is_ipaddress(val):
""" Validate if an entry is a valid IP """
import netaddr
if not val:
return False
try:
Expand All @@ -199,6 +199,17 @@ def is_ipaddress(val):
return False
return True

def ipaddress_type(val):
""" Return the IP address type """
if not val:
return None

try:
ip_version = netaddr.IPAddress(str(val))
except netaddr.core.AddrFormatError:
return None

return ip_version.version

def is_ip_prefix_in_key(key):
'''
Expand Down

0 comments on commit c4c3991

Please sign in to comment.