Skip to content

Commit

Permalink
Fix remove ip rif (sonic-net#1535)
Browse files Browse the repository at this point in the history
*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>
  • Loading branch information
maksymbelei95 authored Apr 26, 2021
1 parent 41d8ddc commit c3963c5
Show file tree
Hide file tree
Showing 7 changed files with 288 additions and 1 deletion.
20 changes: 20 additions & 0 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
31 changes: 31 additions & 0 deletions tests/config_int_ip_common.py
Original file line number Diff line number Diff line change
@@ -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
"""
158 changes: 158 additions & 0 deletions tests/config_int_ip_test.py
Original file line number Diff line number Diff line change
@@ -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
32 changes: 31 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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':
Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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

Expand Down
1 change: 1 addition & 0 deletions tests/crm_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
41 changes: 41 additions & 0 deletions tests/int_ip_input/config_db.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
6 changes: 6 additions & 0 deletions tests/vlan_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="""\
+-----------+-----------------+-----------------+----------------+-----------------------+-------------+
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit c3963c5

Please sign in to comment.