Skip to content

Commit

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

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

# 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:
dhcp_relay_dests = vlan.get('dhcp_servers', [])
if dhcp_relay_destination_ip in dhcp_relay_dests:
click.echo("{} is already a DHCP relay destination for {}".format(dhcp_relay_destination_ip, vlan_name))
return

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

dhcp_relay_dests.append(dhcp_relay_destination_ip)
vlan['dhcp_servers'] = dhcp_relay_dests
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 @@ -251,26 +243,15 @@ def del_vlan_dhcp_relay_destination(db, vid, dhcp_relay_destination_ip):
if len(vlan) == 0:
ctx.fail("{} doesn't exist".format(vlan_name))

# 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:
dhcp_relay_dests = vlan.get('dhcp_servers', [])
if not dhcp_relay_destination_ip in dhcp_relay_dests:
ctx.fail("{} is not a DHCP relay destination for {}".format(dhcp_relay_destination_ip, vlan_name))

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
dhcp_relay_dests.remove(dhcp_relay_destination_ip)
if len(dhcp_relay_dests) == 0:
del vlan['dhcp_servers']
else:
dhcp_servers.remove(dhcp_relay_destination_ip)
if len(dhcp_servers) == 0:
del vlan['dhcp_servers']
else:
vlan['dhcp_servers'] = dhcp_servers

vlan['dhcp_servers'] = dhcp_relay_dests
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: 5 additions & 4 deletions show/vlan.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ def brief(db, verbose):

# Parsing DHCP Helpers info
for key in natsorted(list(vlan_dhcp_helper_data.keys())):
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
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')] = " "

# Parsing VLAN Gateway info
for key in vlan_ip_data:
Expand Down
1 change: 0 additions & 1 deletion tests/mock_tables/config_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,6 @@
},
"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: 1 addition & 118 deletions tests/vlan_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
| | 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 @@ -38,8 +36,6 @@
| | 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 @@ -75,8 +71,7 @@
| | fc02:1000::1/64 | Ethernet8 | untagged | 192.0.0.2 | |
| | | Ethernet12 | untagged | 192.0.0.3 | |
| | | Ethernet16 | untagged | 192.0.0.4 | |
| | | PortChannel1001 | untagged | fc02:2000::1 | |
| | | | | fc02:2000::2 | |
| | | PortChannel1001 | untagged | | |
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
| 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 @@ -125,16 +120,6 @@
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 @@ -144,31 +129,6 @@
| | | 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 @@ -189,8 +149,6 @@
| | 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 @@ -213,8 +171,6 @@
| | 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 @@ -579,19 +535,6 @@ 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 @@ -618,19 +561,6 @@ 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 @@ -666,40 +596,6 @@ 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 @@ -712,19 +608,6 @@ 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: 1 addition & 12 deletions utilities_common/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import click
import json
import netaddr

from natsort import natsorted
from sonic_py_common import multi_asic
Expand Down Expand Up @@ -191,6 +190,7 @@ 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,17 +199,6 @@ 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 c9aa65c

Please sign in to comment.