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

Fixes #12170: Delete subinterface and recreate the subinterface in #2513

Merged
merged 22 commits into from
Dec 16, 2022
Merged

Fixes #12170: Delete subinterface and recreate the subinterface in #2513

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
3dc4c7c
Fixes #12170: Delete subinterface and recreate the subinterface in
preetham-singh Nov 21, 2022
88c074b
Updating vrf python cli test scripts to use correct context so that CLIs
preetham-singh Dec 6, 2022
a6932d5
[psushow & psuutil] Support PSU power threshold checking (#2326)
stephenxs Nov 21, 2022
4aaa821
[VXLAN]Fixing traceback in show remotemac when mac moves during comma…
dgsudharsan Nov 22, 2022
eac82d7
Port 202012 DB migration changes to newer branches (#2515)
vaibhavhd Nov 23, 2022
b799c4a
Update vrf add, del commands for duplicate/non-existing VRFs (#2467)
mdanish-kh Nov 29, 2022
66e1fd0
[drop counters] Fix CLI script for unconfigured PGs (#2518)
liorghub Nov 29, 2022
7137509
Add db_migrator_constants.py script to setup.py (#2534)
vaibhavhd Nov 30, 2022
72ab1a3
Change show kube command default value of insecure key to True (#2517)
lixiaoyuner Nov 30, 2022
7578456
Use github code scanning instead of LGTM (#2530)
liushilongbuaa Nov 30, 2022
08ae841
[QoS] Introduce delay to the qos reload flow (#2503)
DavidZagury Dec 1, 2022
bb36744
[GCU] Add RemoveCreateOnlyDependency Validator/Generator (#2500)
wen587 Dec 2, 2022
a0e6a22
sonic-utilities: Update config reload() to verify formatting of an in…
cchoate54 Dec 2, 2022
46a9a54
Transceiver eeprom dom CLI modification to show output from TRANSCEIV…
mihirpat1 Dec 5, 2022
36dafb7
[system-health] Improve code structure of system health CLIs (#2453)
Junchao-Mellanox Dec 6, 2022
62dc883
[config] Add check in config interface ip command to block if the int…
dgsudharsan Dec 6, 2022
f145b9a
[generate_dump] [Mellanox] Fix the duplicate dfw dump collection prob…
vivekrnv Dec 6, 2022
796b2a1
Merge branch 'master' into subintf
preetham-singh Dec 6, 2022
75441d0
Updating VRF test case for none namespace
preetham-singh Dec 9, 2022
9811ba3
Updating vrf_test for DEFAULT_NAMESPACE
preetham-singh Dec 9, 2022
52c117a
Updating vrf test to increase code coverage for statedb verification as
preetham-singh Dec 13, 2022
6ab0e1c
Updating for better coverage
preetham-singh Dec 13, 2022
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
16 changes: 16 additions & 0 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5118,6 +5118,22 @@ def unbind(ctx, interface_name):
for ipaddress in interface_ipaddresses:
remove_router_interface_ip_address(config_db, interface_name, ipaddress)
if table_name == "VLAN_SUB_INTERFACE":
# First delete subinterface, once subinterface deletion successful,
# recreate same with same config on default vrf
if 'state_db' not in ctx.obj:
if ctx.obj['namespace'] is DEFAULT_NAMESPACE:
state_db = SonicV2Connector(use_unix_socket_path=True)
else:
state_db = SonicV2Connector(use_unix_socket_path=True, namespace=ctx.obj['namespace'])
state_db.connect(state_db.STATE_DB, False)
else:
state_db = ctx.obj['state_db']

config_db.set_entry(table_name, interface_name, None)
_hash = '{}{}'.format('INTERFACE_TABLE|', interface_name)
while state_db.exists(state_db.STATE_DB, _hash):
Copy link
Contributor

Choose a reason for hiding this comment

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

STATE_DB update must be done by intfmgr, why we doing this explicitly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@prsunny It is not updated bur rather checked here to confirm that None has been processed and state_db is removed. We already have such loginc here.

sonic-utilities/config/main.py

Line 5105 in ce1c444

while state_db.exists(state_db.STATE_DB, _hash):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats right Prince. This is present since VRF bind() CLI was implemented as Sudharsan pointed.
Here click cli is ensuring STATE_DB INTERFACE_TABLE for the interface is deleted to ensure RIF for corresponding interface is deleted before binding interface to new VRF. In unbind scenario for subinterfaces, subinterface has to be deleted and recreated with same subinterface attributes in default vrf.

Copy link
Contributor

Choose a reason for hiding this comment

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

my concern is what if there is an issue with updating state_db, this will be indefinitely hanging, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@prsunny , yes this will be hung indefinitely. We can handle this in intfmgrd. Can we track this as a separate issue since its applicable to all interfaces(vrf bind operation) and not specific to subinterfaces?

time.sleep(0.01)
state_db.close(state_db.STATE_DB)
config_db.set_entry(table_name, interface_name, subintf_entry)
else:
config_db.set_entry(table_name, interface_name, None)
Expand Down
49 changes: 39 additions & 10 deletions tests/vrf_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@

import config.main as config
import show.main as show
import threading

DEFAULT_NAMESPACE = ''
test_path = os.path.dirname(os.path.abspath(__file__))
mock_db_path = os.path.join(test_path, "vrf_input")

Expand All @@ -16,6 +18,11 @@ def setup_class(cls):
print("SETUP")
os.environ["UTILITIES_UNIT_TESTING"] = "1"

def update_statedb(self, db, db_name, key):
import time
time.sleep(0.5)
db.delete(db_name, key)

def test_vrf_show(self):
from .mock_tables import dbconnector
jsonfile_config = os.path.join(mock_db_path, "config_db")
Expand Down Expand Up @@ -64,59 +71,81 @@ def test_vrf_bind_unbind(self):
assert result.exit_code == 0
assert result.output == expected_output

obj = {'config_db':db.cfgdb}

vrf_obj = {'config_db':db.cfgdb, 'namespace':db.db.namespace}

expected_output_unbind = "Interface Ethernet4 IP disabled and address(es) removed due to unbinding VRF.\n"
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Ethernet4"], obj=obj)
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Ethernet4"], obj=vrf_obj)

print(result.exit_code, result.output)
assert result.exit_code == 0
assert 'Ethernet4' not in db.cfgdb.get_table('INTERFACE')
assert result.output == expected_output_unbind

expected_output_unbind = "Interface Loopback0 IP disabled and address(es) removed due to unbinding VRF.\n"
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Loopback0"], obj=obj)

result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Loopback0"], obj=vrf_obj)

print(result.exit_code, result.output)
assert result.exit_code == 0
assert 'Loopback0' not in db.cfgdb.get_table('LOOPBACK_INTERFACE')
assert result.output == expected_output_unbind

expected_output_unbind = "Interface Vlan40 IP disabled and address(es) removed due to unbinding VRF.\n"
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Vlan40"], obj=obj)

result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Vlan40"], obj=vrf_obj)

print(result.exit_code, result.output)
assert result.exit_code == 0
assert 'Vlan40' not in db.cfgdb.get_table('VLAN_INTERFACE')
assert result.output == expected_output_unbind

expected_output_unbind = "Interface PortChannel0002 IP disabled and address(es) removed due to unbinding VRF.\n"
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["PortChannel0002"], obj=obj)

result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["PortChannel0002"], obj=vrf_obj)

print(result.exit_code, result.output)
assert result.exit_code == 0
assert 'PortChannel002' not in db.cfgdb.get_table('PORTCHANNEL_INTERFACE')
assert result.output == expected_output_unbind

vrf_obj = {'config_db':db.cfgdb, 'namespace':DEFAULT_NAMESPACE}
state_db = SonicV2Connector(use_unix_socket_path=True, namespace='')
state_db.connect(state_db.STATE_DB, False)
_hash = "INTERFACE_TABLE|Eth36.10"
state_db.set(db.db.STATE_DB, _hash, "state", "ok")
vrf_obj['state_db'] = state_db

expected_output_unbind = "Interface Eth36.10 IP disabled and address(es) removed due to unbinding VRF.\n"
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Eth36.10"], obj=obj)
T1 = threading.Thread( target = self.update_statedb, args = (state_db, db.db.STATE_DB, _hash))
T1.start()
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Eth36.10"], obj=vrf_obj)
T1.join()
print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('vrf_name', 'Vrf102') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Eth36.10']
assert result.output == expected_output_unbind

vrf_obj = {'config_db':db.cfgdb, 'namespace':DEFAULT_NAMESPACE}

expected_output_unbind = "Interface Ethernet0.10 IP disabled and address(es) removed due to unbinding VRF.\n"
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Ethernet0.10"], obj=obj)

result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Ethernet0.10"], obj=vrf_obj)

print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('vrf_name', 'Vrf101') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Ethernet0.10']
assert result.output == expected_output_unbind

expected_output_unbind = "Interface Po0002.101 IP disabled and address(es) removed due to unbinding VRF.\n"
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Po0002.101"], obj=obj)

result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Po0002.101"], obj=vrf_obj)

print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('vrf_name', 'Vrf103') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Po0002.101']
assert result.output == expected_output_unbind

vrf_obj = {'config_db':db.cfgdb, 'namespace':db.db.namespace}

expected_output_bind = "Interface Ethernet0 IP disabled and address(es) removed due to binding VRF Vrf1.\n"
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["bind"], ["Ethernet0", "Vrf1"], obj=vrf_obj)
assert result.exit_code == 0
Expand Down