From c3963c5673cf944aec2ff89868f4a864261f51d3 Mon Sep 17 00:00:00 2001 From: maksymbelei95 <75987222+maksymbelei95@users.noreply.github.com> Date: Mon, 26 Apr 2021 19:40:06 +0300 Subject: [PATCH] Fix remove ip rif (#1535) *Added checking of static routes, related to the interface, before deleting of the last IP entry to prevent deleting the RIF if a static route is present in the system. Signed-off-by: Maksym Belei --- config/main.py | 20 ++++ tests/config_int_ip_common.py | 31 ++++++ tests/config_int_ip_test.py | 158 ++++++++++++++++++++++++++++++ tests/conftest.py | 32 +++++- tests/crm_test.py | 1 + tests/int_ip_input/config_db.json | 41 ++++++++ tests/vlan_test.py | 6 ++ 7 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 tests/config_int_ip_common.py create mode 100644 tests/config_int_ip_test.py create mode 100644 tests/int_ip_input/config_db.json diff --git a/config/main.py b/config/main.py index 6fad33f9c103..e9bab3172d70 100644 --- a/config/main.py +++ b/config/main.py @@ -22,6 +22,7 @@ from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, SonicDBConfig from utilities_common.db import Db from utilities_common.intf_filter import parse_interface_in_filter +from utilities_common import bgp_util import utilities_common.cli as clicommon from .utils import log @@ -2787,6 +2788,25 @@ def remove(ctx, interface_name, ip_addr): table_name = get_interface_table_name(interface_name) if table_name == "": ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") + interface_dependent = interface_ipaddr_dependent_on_interface(config_db, interface_name) + # If we deleting the last IP entry of the interface, check whether a static route present for the RIF + # before deleting the entry and also the RIF. + if len(interface_dependent) == 1 and interface_dependent[0][1] == ip_addr: + # Check both IPv4 and IPv6 routes. + ip_versions = [ "ip", "ipv6"] + for ip_ver in ip_versions: + # Compete the command and ask Zebra to return the routes. + # Scopes of all VRFs will be checked. + cmd = "show {} route vrf all static".format(ip_ver) + if multi_asic.is_multi_asic(): + output = bgp_util.run_bgp_command(cmd, ctx.obj['namespace']) + else: + output = bgp_util.run_bgp_command(cmd) + # If there is output data, check is there a static route, + # bound to the interface. + if output != "": + if any(interface_name in output_line for output_line in output.splitlines()): + ctx.fail("Cannot remove the last IP entry of interface {}. A static {} route is still bound to the RIF.".format(interface_name, ip_ver)) config_db.set_entry(table_name, (interface_name, ip_addr), None) interface_dependent = interface_ipaddr_dependent_on_interface(config_db, interface_name) if len(interface_dependent) == 0 and is_interface_bind_to_vrf(config_db, interface_name) is False: diff --git a/tests/config_int_ip_common.py b/tests/config_int_ip_common.py new file mode 100644 index 000000000000..7cebfdb8ba6a --- /dev/null +++ b/tests/config_int_ip_common.py @@ -0,0 +1,31 @@ +show_ip_route_with_static_expected_output = """\ +Codes: K - kernel route, C - connected, S - static, R - RIP, + O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP, + T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP, + F - PBR, f - OpenFabric, + > - selected route, * - FIB route, q - queued, r - rejected, b - backup + +VRF Vrf11: +S>* 20.0.0.1/32 [1/0] is directly connected, Ethernet2, weight 1, 00:40:18 + +VRF default: +S>* 0.0.0.0/0 [200/0] via 192.168.111.3, eth0, weight 1, 19:51:57 +S>* 20.0.0.1/32 [1/0] is directly connected, Ethernet4 (vrf Vrf11), weight 1, 00:38:52 +S>* 20.0.0.4/32 [1/0] is directly connected, PortChannel2, weight 1, 00:38:52 +S>* 20.0.0.8/32 [1/0] is directly connected, Vlan2, weight 1, 00:38:52 +""" + +show_ipv6_route_with_static_expected_output = """\ +Codes: K - kernel route, C - connected, S - static, R - RIPng, + O - OSPFv3, I - IS-IS, B - BGP, N - NHRP, T - Table, + v - VNC, V - VNC-Direct, A - Babel, D - SHARP, F - PBR, + f - OpenFabric, + > - selected route, * - FIB route, q - queued, r - rejected, b - backup + +VRF Vrf11: +S>* fe80::/24 [1/0] is directly connected, Vlan4, weight 1, 00:00:04 + +VRF default: +S>* 20c0:a800:0:21::/64 [20/0] is directly connected, PortChannel4, 2d22h02m +S>* fe80::/32 [1/0] is directly connected, Ethernet8 (vrf Vrf11), weight 1, 00:00:04 +""" \ No newline at end of file diff --git a/tests/config_int_ip_test.py b/tests/config_int_ip_test.py new file mode 100644 index 000000000000..6968fcbe4509 --- /dev/null +++ b/tests/config_int_ip_test.py @@ -0,0 +1,158 @@ +import os +import sys +import pytest +import mock +from importlib import reload + +from click.testing import CliRunner + +from utilities_common.db import Db + +modules_path = os.path.join(os.path.dirname(__file__), "..") +test_path = os.path.join(modules_path, "tests") +sys.path.insert(0, modules_path) +sys.path.insert(0, test_path) +mock_db_path = os.path.join(test_path, "int_ip_input") + + +class TestIntIp(object): + @pytest.fixture(scope="class", autouse=True) + def setup_class(cls): + print("SETUP") + os.environ['UTILITIES_UNIT_TESTING'] = "1" + import config.main as config + reload(config) + yield + print("TEARDOWN") + os.environ["UTILITIES_UNIT_TESTING"] = "0" + from .mock_tables import dbconnector + dbconnector.dedicated_dbs = {} + + @pytest.mark.parametrize('setup_single_bgp_instance', + ['ip_route_for_int_ip'], indirect=['setup_single_bgp_instance']) + def test_config_int_ip_rem( + self, + get_cmd_module, + setup_single_bgp_instance): + (config, show) = get_cmd_module + jsonfile_config = os.path.join(mock_db_path, "config_db.json") + from .mock_tables import dbconnector + dbconnector.dedicated_dbs['CONFIG_DB'] = jsonfile_config + + runner = CliRunner() + db = Db() + obj = {'config_db': db.cfgdb} + + # remove vlan IP`s + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Ethernet16", "192.168.10.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + + @pytest.mark.parametrize('setup_single_bgp_instance', + ['ip_route_for_int_ip'], indirect=['setup_single_bgp_instance']) + def test_config_int_ip_rem_static( + self, + get_cmd_module, + setup_single_bgp_instance): + (config, show) = get_cmd_module + jsonfile_config = os.path.join(mock_db_path, "config_db") + from .mock_tables import dbconnector + dbconnector.dedicated_dbs['CONFIG_DB'] = jsonfile_config + + runner = CliRunner() + db = Db() + obj = {'config_db': db.cfgdb} + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Ethernet2", "192.168.0.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: Cannot remove the last IP entry of interface Ethernet2. A static ip route is still bound to the RIF." in result.output + assert mock_run_command.call_count == 0 + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Ethernet8", "192.168.3.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: Cannot remove the last IP entry of interface Ethernet8. A static ipv6 route is still bound to the RIF." in result.output + assert mock_run_command.call_count == 0 + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Vlan2", "192.168.1.1/21"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: Cannot remove the last IP entry of interface Vlan2. A static ip route is still bound to the RIF." in result.output + assert mock_run_command.call_count == 0 + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["PortChannel2", "10.0.0.56/31"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: Cannot remove the last IP entry of interface PortChannel2. A static ip route is still bound to the RIF." in result.output + assert mock_run_command.call_count == 0 + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Ethernet4", "192.168.4.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert mock_run_command.call_count == 1 + +class TestIntIpMultiasic(object): + @pytest.fixture(scope="class", autouse=True) + def setup_class(cls): + print("SETUP") + os.environ['UTILITIES_UNIT_TESTING'] = "1" + os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "multi_asic" + from .mock_tables import dbconnector + from .mock_tables import mock_multi_asic + reload(mock_multi_asic) + dbconnector.load_namespace_config() + yield + print("TEARDOWN") + os.environ["UTILITIES_UNIT_TESTING"] = "0" + os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "" + # change back to single asic config + from .mock_tables import dbconnector + from .mock_tables import mock_single_asic + reload(mock_single_asic) + dbconnector.dedicated_dbs = {} + dbconnector.load_namespace_config() + + @pytest.mark.parametrize('setup_multi_asic_bgp_instance', + ['ip_route_for_int_ip'], indirect=['setup_multi_asic_bgp_instance']) + def test_config_int_ip_rem_static_multiasic( + self, + get_cmd_module, + setup_multi_asic_bgp_instance): + (config, show) = get_cmd_module + jsonfile_config = os.path.join(mock_db_path, "config_db") + from .mock_tables import dbconnector + dbconnector.dedicated_dbs['CONFIG_DB'] = jsonfile_config + + runner = CliRunner() + db = Db() + obj = {'config_db': db.cfgdb, 'namespace': 'test_ns'} + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Ethernet2", "192.168.0.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: Cannot remove the last IP entry of interface Ethernet2. A static ip route is still bound to the RIF." in result.output + assert mock_run_command.call_count == 0 + + with mock.patch('utilities_common.cli.run_command') as mock_run_command: + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], + ["Ethernet8", "192.168.3.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: Cannot remove the last IP entry of interface Ethernet8. A static ipv6 route is still bound to the RIF." in result.output + assert mock_run_command.call_count == 0 \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index 16c018bb641c..4ff1a002bd68 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,6 +9,7 @@ from .mock_tables import dbconnector from . import show_ip_route_common +from . import config_int_ip_common test_path = os.path.dirname(os.path.abspath(__file__)) modules_path = os.path.dirname(test_path) @@ -124,6 +125,14 @@ def mock_run_bgp_command(vtysh_cmd, bgp_namespace): mock_frr_data = json_data.read() return mock_frr_data return "" + + def mock_run_bgp_command_for_static(vtysh_cmd, bgp_namespace=""): + if vtysh_cmd == "show ip route vrf all static": + return config_int_ip_common.show_ip_route_with_static_expected_output + elif vtysh_cmd == "show ipv6 route vrf all static": + return config_int_ip_common.show_ipv6_route_with_static_expected_output + else: + return "" def mock_run_show_ip_route_commands(request): if request.param == 'ipv6_route_err': @@ -147,10 +156,18 @@ def mock_run_show_ip_route_commands(request): request.param == 'ipv6_route', request.param == 'ipv6_specific_route']): bgp_util.run_bgp_command = mock.MagicMock( return_value=mock_run_show_ip_route_commands(request)) + elif request.param == 'ip_route_for_int_ip': + _old_run_bgp_command = bgp_util.run_bgp_command + bgp_util.run_bgp_command = mock_run_bgp_command_for_static else: bgp_util.run_bgp_command = mock.MagicMock( return_value=mock_run_bgp_command("", "")) + yield + + if request.param == 'ip_route_for_int_ip': + bgp_util.run_bgp_command = _old_run_bgp_command + @pytest.fixture def setup_multi_asic_bgp_instance(request): @@ -178,6 +195,16 @@ def setup_multi_asic_bgp_instance(request): m_asic_json_file = os.path.join( test_path, 'mock_tables', 'dummy.json') + def mock_run_bgp_command_for_static(vtysh_cmd, bgp_namespace=""): + if bgp_namespace != 'test_ns': + return "" + if vtysh_cmd == "show ip route vrf all static": + return config_int_ip_common.show_ip_route_with_static_expected_output + elif vtysh_cmd == "show ipv6 route vrf all static": + return config_int_ip_common.show_ipv6_route_with_static_expected_output + else: + return "" + def mock_run_bgp_command(vtysh_cmd, bgp_namespace): bgp_mocked_json = os.path.join( test_path, 'mock_tables', bgp_namespace, m_asic_json_file) @@ -189,7 +216,10 @@ def mock_run_bgp_command(vtysh_cmd, bgp_namespace): return "" _old_run_bgp_command = bgp_util.run_bgp_command - bgp_util.run_bgp_command = mock_run_bgp_command + if request.param == 'ip_route_for_int_ip': + bgp_util.run_bgp_command = mock_run_bgp_command_for_static + else: + bgp_util.run_bgp_command = mock_run_bgp_command yield diff --git a/tests/crm_test.py b/tests/crm_test.py index 369d9a51ab88..d99402e05732 100644 --- a/tests/crm_test.py +++ b/tests/crm_test.py @@ -1216,6 +1216,7 @@ def setup_class(cls): os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "multi_asic" from .mock_tables import dbconnector from .mock_tables import mock_multi_asic + importlib.reload(mock_multi_asic) dbconnector.load_namespace_config() def test_crm_show_summary(self): diff --git a/tests/int_ip_input/config_db.json b/tests/int_ip_input/config_db.json new file mode 100644 index 000000000000..3f2d6e5beb69 --- /dev/null +++ b/tests/int_ip_input/config_db.json @@ -0,0 +1,41 @@ +{ + "INTERFACE|Ethernet16": { + "NULL": "NULL" + }, + "INTERFACE|Ethernet16|192.168.10.1/24": { + "NULL": "NULL" + }, + "INTERFACE|Ethernet2": { + "NULL": "NULL" + }, + "INTERFACE|Ethernet2|192.168.0.1/24": { + "NULL": "NULL" + }, + "INTERFACE|Ethernet4": { + "NULL": "NULL" + }, + "INTERFACE|Ethernet4|192.168.4.1/24": { + "NULL": "NULL" + }, + "INTERFACE|Ethernet4|192.168.100.1/24": { + "NULL": "NULL" + }, + "INTERFACE|Ethernet8": { + "NULL": "NULL" + }, + "INTERFACE|Ethernet8|192.168.3.1/24": { + "NULL": "NULL" + }, + "PORTCHANNEL_INTERFACE|PortChannel2": { + "NULL": "NULL" + }, + "PORTCHANNEL_INTERFACE|PortChannel2|10.0.0.56/31": { + "NULL": "NULL" + }, + "VLAN_INTERFACE|Vlan2": { + "proxy_arp": "enabled" + }, + "VLAN_INTERFACE|Vlan2|192.168.1.1/21": { + "NULL": "NULL" + } +} \ No newline at end of file diff --git a/tests/vlan_test.py b/tests/vlan_test.py index d4832dc2cfd0..ad3ff9fbb432 100644 --- a/tests/vlan_test.py +++ b/tests/vlan_test.py @@ -7,6 +7,7 @@ import config.main as config import show.main as show from utilities_common.db import Db +from importlib import reload show_vlan_brief_output="""\ +-----------+-----------------+-----------------+----------------+-----------------------+-------------+ @@ -188,6 +189,11 @@ class TestVlan(object): @classmethod def setup_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "1" + # ensure that we are working with single asic config + from .mock_tables import dbconnector + from .mock_tables import mock_single_asic + reload(mock_single_asic) + dbconnector.load_namespace_config() print("SETUP") def test_show_vlan(self):