Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[subinterface]Added additional checks in portchannel and subinterface commands #2345

Merged
merged 1 commit into from
Sep 13, 2022
Merged

[subinterface]Added additional checks in portchannel and subinterface commands #2345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2073,6 +2073,14 @@ def add_portchannel_member(ctx, portchannel_name, port_name):
ctx.fail(" {} has ip address configured".format(port_name))
return

for key in db.get_keys('VLAN_SUB_INTERFACE'):
if type(key) == tuple:
continue
intf = key.split(VLAN_SUB_INTERFACE_SEPARATOR)[0]
parent_intf = get_intf_longname(intf)
if parent_intf == port_name:
ctx.fail(" {} has subinterfaces configured".format(port_name))

# Dont allow a port to be member of port channel if it is configured as a VLAN member
for k,v in db.get_table('VLAN_MEMBER'):
if v == port_name:
Expand Down Expand Up @@ -6722,23 +6730,24 @@ def add_subinterface(ctx, subinterface_name, vid):

config_db = ctx.obj['db']
port_dict = config_db.get_table(intf_table_name)
parent_intf = get_intf_longname(interface_alias)
if interface_alias is not None:
if not port_dict:
ctx.fail("{} parent interface not found. {} table none".format(interface_alias, intf_table_name))
if get_intf_longname(interface_alias) not in port_dict.keys():
if parent_intf not in port_dict.keys():
ctx.fail("{} parent interface not found".format(subinterface_name))

# Validate if parent is portchannel member
portchannel_member_table = config_db.get_table('PORTCHANNEL_MEMBER')
if interface_is_in_portchannel(portchannel_member_table, interface_alias):
if interface_is_in_portchannel(portchannel_member_table, parent_intf):
ctx.fail("{} is configured as a member of portchannel. Cannot configure subinterface"
.format(interface_alias))
.format(parent_intf))

# Validate if parent is vlan member
vlan_member_table = config_db.get_table('VLAN_MEMBER')
if interface_is_in_vlan(vlan_member_table, interface_alias):
if interface_is_in_vlan(vlan_member_table, parent_intf):
ctx.fail("{} is configured as a member of vlan. Cannot configure subinterface"
.format(interface_alias))
.format(parent_intf))

sub_intfs = [k for k,v in config_db.get_table('VLAN_SUB_INTERFACE').items() if type(k) != tuple]
if subinterface_name in sub_intfs:
Expand Down
12 changes: 6 additions & 6 deletions tests/intfutil_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def test_subintf_status(self):
expected_output = (
"Sub port interface Speed MTU Vlan Admin Type\n"
"-------------------- ------- ----- ------ ------- --------------------\n"
" Eth32.10 40G 9100 100 up 802.1q-encapsulation\n"
" Eth36.10 10M 9100 100 up 802.1q-encapsulation\n"
" Ethernet0.10 25G 9100 10 up 802.1q-encapsulation\n"
" Po0001.10 40G 9100 100 up 802.1q-encapsulation"
)
Expand Down Expand Up @@ -248,10 +248,10 @@ def test_single_subintf_status(self):
expected_output = (
"Sub port interface Speed MTU Vlan Admin Type\n"
"-------------------- ------- ----- ------ ------- --------------------\n"
" Eth32.10 40G 9100 100 up 802.1q-encapsulation"
" Eth36.10 10M 9100 100 up 802.1q-encapsulation"
)
# Test 'intfutil status Eth32.10'
output = subprocess.check_output('intfutil -c status -i Eth32.10', stderr=subprocess.STDOUT, shell=True, text=True)
# Test 'intfutil status Eth36.10'
output = subprocess.check_output('intfutil -c status -i Eth36.10', stderr=subprocess.STDOUT, shell=True, text=True)
print(output, file=sys.stderr)
self.assertEqual(output.strip(), expected_output)

Expand All @@ -272,9 +272,9 @@ def test_single_subintf_status_verbose(self):
expected_output = "Command: intfutil -c status -i Ethernet0.10"
self.assertEqual(result.output.split('\n')[0], expected_output)

result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Eth32.10", "--verbose"])
result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Eth36.10", "--verbose"])
print(result.output, file=sys.stderr)
expected_output = "Command: intfutil -c status -i Eth32.10"
expected_output = "Command: intfutil -c status -i Eth36.10"
self.assertEqual(result.output.split('\n')[0], expected_output)

result = self.runner.invoke(show.cli.commands["subinterfaces"].commands["status"], ["Po0001.10", "--verbose"])
Expand Down
20 changes: 10 additions & 10 deletions tests/ip_config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ def test_add_del_interface_valid_ipv4(self):
assert result.exit_code == 0
assert ('Ethernet0.10', '10.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

# config int ip add Eth32.10 32.11.10.1/24
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth32.10", "32.11.10.1/24"], obj=obj)
# config int ip add Eth36.10 32.11.10.1/24
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth36.10", "32.11.10.1/24"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('Eth32.10', '32.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
assert ('Eth36.10', '32.11.10.1/24') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

# config int ip remove Ethernet64 10.10.10.1/24
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet64", "10.10.10.1/24"], obj=obj)
Expand All @@ -72,11 +72,11 @@ def test_add_del_interface_valid_ipv4(self):
assert result.exit_code != 0
assert ('Ethernet0.10', '10.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

# config int ip remove Eth32.10 32.11.10.1/24
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth32.10", "32.11.10.1/24"], obj=obj)
# config int ip remove Eth36.10 32.11.10.1/24
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth36.10", "32.11.10.1/24"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code != 0
assert ('Eth32.10', '32.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
assert ('Eth36.10', '32.11.10.1/24') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

def test_add_interface_invalid_ipv4(self):
db = Db()
Expand Down Expand Up @@ -129,10 +129,10 @@ def test_add_del_interface_valid_ipv6(self):
assert result.exit_code == 0
assert ('Ethernet0.10', '1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth32.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Eth36.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('Eth32.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
assert ('Eth36.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

# config int ip remove Ethernet72 2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
Expand All @@ -145,10 +145,10 @@ def test_add_del_interface_valid_ipv6(self):
assert result.exit_code != 0
assert ('Ethernet0.10', '1010:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth32.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Eth36.10", "3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code != 0
assert ('Eth32.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
assert ('Eth36.10', '3210:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

def test_del_interface_case_sensitive_ipv6(self):
db = Db()
Expand Down
2 changes: 1 addition & 1 deletion tests/loopback_action_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
show_ip_interfaces_loopback_action_output="""\
Interface Action
--------------- --------
Eth32.10 drop
Eth36.10 drop
Ethernet0 forward
PortChannel0001 drop
Vlan3000 forward
Expand Down
6 changes: 3 additions & 3 deletions tests/mock_tables/appl_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
"admin_status": "up",
"vlan": "10"
},
"INTF_TABLE:Eth32.10": {
"INTF_TABLE:Eth36.10": {
"admin_status": "up",
"vrf_name": "Vrf1",
"vlan": "100"
Expand All @@ -202,15 +202,15 @@
"family": "IPv4",
"scope": "global"
},
"INTF_TABLE:Eth32.10|32.10.11.12/24": {
"INTF_TABLE:Eth36.10|32.10.11.12/24": {
"family": "IPv4",
"scope": "global"
},
"INTF_TABLE:Po0001.10|10.10.11.12/24": {
"family": "IPv4",
"scope": "global"
},
"INTF_TABLE:Eth32.10|3210::12/126": {
"INTF_TABLE:Eth36.10|3210::12/126": {
"family": "IPv6",
"scope": "global"
},
Expand Down
6 changes: 3 additions & 3 deletions tests/mock_tables/config_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,16 +376,16 @@
"VLAN_SUB_INTERFACE|Ethernet0.10|10.11.12.13/24": {
"NULL" : "NULL"
},
"VLAN_SUB_INTERFACE|Eth32.10": {
"VLAN_SUB_INTERFACE|Eth36.10": {
"admin_status": "up",
"loopback_action": "drop",
"vrf_name": "Vrf1",
"vlan": "100"
},
"VLAN_SUB_INTERFACE|Eth32.10|32.10.11.12/24": {
"VLAN_SUB_INTERFACE|Eth36.10|32.10.11.12/24": {
"NULL" : "NULL"
},
"VLAN_SUB_INTERFACE|Eth32.10|3210::12/126": {
"VLAN_SUB_INTERFACE|Eth36.10|3210::12/126": {
"NULL" : "NULL"
},
"VLAN_SUB_INTERFACE|Po0001.10": {
Expand Down
13 changes: 13 additions & 0 deletions tests/portchannel_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,19 @@ def test_add_portchannel_member_which_has_ipaddress(self):
assert result.exit_code != 0
assert "Error: Ethernet0 has ip address configured" in result.output

def test_add_portchannel_member_which_has_subintf(self):
runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

# add a portchannel member with port which has ip-address
result = runner.invoke(config.config.commands["portchannel"].commands["member"].commands["add"], ["PortChannel1001", "Ethernet36"], obj=obj)
print(result.exit_code)
print(result.output)
assert result.exit_code != 0
print(result.output)
assert "Error: Ethernet36 has subinterfaces configured" in result.output

def test_add_portchannel_member_which_is_member_of_vlan(self):
runner = CliRunner()
db = Db()
Expand Down
10 changes: 5 additions & 5 deletions tests/show_vrf_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def test_vrf_show(self):
Vrf101 Ethernet0.10
Vrf102 PortChannel0002
Vlan40
Eth32.10
Eth36.10
Vrf103 Ethernet4
Loopback0
Po0002.101
Expand All @@ -53,7 +53,7 @@ def test_vrf_bind_unbind(self):
Vrf101 Ethernet0.10
Vrf102 PortChannel0002
Vlan40
Eth32.10
Eth36.10
Vrf103 Ethernet4
Loopback0
Po0002.101
Expand Down Expand Up @@ -86,10 +86,10 @@ def test_vrf_bind_unbind(self):
assert result.exit_code == 0
assert 'PortChannel002' not in db.cfgdb.get_table('PORTCHANNEL_INTERFACE')

result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Eth32.10"], obj=obj)
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Eth36.10"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('vrf_name', 'Vrf102') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Eth32.10']
assert ('vrf_name', 'Vrf102') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Eth36.10']

result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Ethernet0.10"], obj=obj)
print(result.exit_code, result.output)
Expand All @@ -114,7 +114,7 @@ def test_vrf_bind_unbind(self):
Vrf101 Ethernet0.10
Vrf102 PortChannel0002
Vlan40
Eth32.10
Eth36.10
Vrf103 Ethernet4
Loopback0
Po0002.101
Expand Down
10 changes: 5 additions & 5 deletions tests/static_routes_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,16 +403,16 @@ def test_static_route_nexthop_subinterface(self):
print(result.exit_code, result.output)
assert not ('2.2.3.5/32') in db.cfgdb.get_table('STATIC_ROUTE')

# config route add prefix 2.2.3.5/32 nexthop dev Eth32.10
# config route add prefix 2.2.3.5/32 nexthop dev Eth36.10
result = runner.invoke(config.config.commands["route"].commands["add"], \
["prefix", "2.2.3.5/32", "nexthop", "dev", "Eth32.10"], obj=obj)
["prefix", "2.2.3.5/32", "nexthop", "dev", "Eth36.10"], obj=obj)
print(result.exit_code, result.output)
assert ('2.2.3.5/32') in db.cfgdb.get_table('STATIC_ROUTE')
assert db.cfgdb.get_entry('STATIC_ROUTE', '2.2.3.5/32') == {'nexthop': '', 'blackhole': 'false', 'distance': '0', 'ifname': 'Eth32.10', 'nexthop-vrf': ''}
assert db.cfgdb.get_entry('STATIC_ROUTE', '2.2.3.5/32') == {'nexthop': '', 'blackhole': 'false', 'distance': '0', 'ifname': 'Eth36.10', 'nexthop-vrf': ''}

# config route del prefix 2.2.3.5/32 nexthop dev Eth32.10
# config route del prefix 2.2.3.5/32 nexthop dev Eth36.10
result = runner.invoke(config.config.commands["route"].commands["del"], \
["prefix", "2.2.3.5/32", "nexthop", "dev", "Eth32.10"], obj=obj)
["prefix", "2.2.3.5/32", "nexthop", "dev", "Eth36.10"], obj=obj)
print(result.exit_code, result.output)
assert not ('2.2.3.5/32') in db.cfgdb.get_table('STATIC_ROUTE')

Expand Down
21 changes: 21 additions & 0 deletions tests/subintf_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
import show.main as show
from utilities_common.db import Db

SUB_INTF_ON_LAG_MEMBER_ERR="""\
Usage: add [OPTIONS] <subinterface_name> <vid>
Try "add --help" for help.

Error: Ethernet32 is configured as a member of portchannel. Cannot configure subinterface
"""

class TestSubinterface(object):
@classmethod
Expand Down Expand Up @@ -141,6 +147,21 @@ def test_invalid_subintf_creation(self):
print(result.exit_code, result.output)
assert result.exit_code != 0

def test_subintf_creation_on_lag_member(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline is it possible to add test to check creation of subintf on vlan members?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a different functionality check not related to this issue. I recommend you to raise it if it is missed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Lets track it separately for negative scenario of subintf creation on vlan member.

runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

result = runner.invoke(config.config.commands["subinterface"].commands["add"], ["Ethernet32.10"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code != 0
assert(result.output == SUB_INTF_ON_LAG_MEMBER_ERR)

result = runner.invoke(config.config.commands["subinterface"].commands["add"], ["Eth32.20"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code != 0
assert(result.output == SUB_INTF_ON_LAG_MEMBER_ERR)

def test_subintf_vrf_bind_unbind(self):
runner = CliRunner()
db = Db()
Expand Down
2 changes: 1 addition & 1 deletion tests/vrf_input/config_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"vrf_name": "Vrf101",
"admin_status": "up"
},
"VLAN_SUB_INTERFACE|Eth32.10": {
"VLAN_SUB_INTERFACE|Eth36.10": {
"vrf_name": "Vrf102",
"admin_status": "up",
"vlan": "100"
Expand Down