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 <Maksym_Belei@jabil.com>

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:

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
+"""

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

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
 

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):

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"
+    }
+}

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):