From c4c39910bf12481793ff6df43652d004875d5f07 Mon Sep 17 00:00:00 2001 From: Vivek Reddy Date: Wed, 26 Oct 2022 16:43:45 +0000 Subject: [PATCH] Revert "Revert "[DHCPv6] [202012] Update the dhcpv6_relay config/show cli (#2271)" (#2336)" This reverts commit c9aa65c072a5e74dc3b919f671f4b799a7a66272. --- config/vlan.py | 39 +++++++--- show/vlan.py | 9 ++- tests/mock_tables/config_db.json | 1 + tests/vlan_test.py | 119 ++++++++++++++++++++++++++++++- utilities_common/cli.py | 13 +++- 5 files changed, 164 insertions(+), 17 deletions(-) diff --git a/config/vlan.py b/config/vlan.py index 001893b52f..671eada8ca 100644 --- a/config/vlan.py +++ b/config/vlan.py @@ -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: @@ -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: diff --git a/show/vlan.py b/show/vlan.py index df4149fca9..156868917c 100644 --- a/show/vlan.py +++ b/show/vlan.py @@ -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: diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 3f2ce59499..0ef3e75d94 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -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": { diff --git a/tests/vlan_test.py b/tests/vlan_test.py index c771fda8d2..cfd1f53db4 100644 --- a/tests/vlan_test.py +++ b/tests/vlan_test.py @@ -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 | | @@ -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 | | @@ -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 | | @@ -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 | @@ -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 | | @@ -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 | +-----------+-----------------+-----------------+----------------+-----------------------+-------------+ @@ -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 | +-----------+-----------------+-----------------+----------------+-----------------------+-------------+ @@ -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() @@ -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() @@ -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() @@ -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() diff --git a/utilities_common/cli.py b/utilities_common/cli.py index b121e15c3e..50315f321f 100644 --- a/utilities_common/cli.py +++ b/utilities_common/cli.py @@ -6,6 +6,7 @@ import click import json +import netaddr from natsort import natsorted from sonic_py_common import multi_asic @@ -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: @@ -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): '''