Handling deletion of Port Channel before deletion of its members #1062

madhanmellanox · 2020-08-21T02:14:44Z

What I did
Handled configuration code to block deletion of Port channel when user tries to delete a Port channel before deleting its members.
How I did it
In the sonic-utilities/config/main.py, where the removal of Port channel configuration is handled, I added a check whether the Port channel to be deleted has any members. If members exist, I will block the configuration of deleting the port channel.
How to verify it
Create a Portchannel. Add 2 switch ports to it as members. Then try to delete the port channel. Now, the deletion of port channel should be blocked by the config since it has port channel members still existing.
Previous command output (if the output of a command-line utility has changed)

admin@r-qa-sw-eth-53:/tmp$ sudo config portchannel add portchannel100
admin@r-qa-sw-eth-53:/tmp$ sudo config portchannel member add portchannel100 Ethernet4
admin@r-qa-sw-eth-53:/tmp$ sudo config portchannel member add portchannel100 Ethernet8
admin@r-qa-sw-eth-53:/tmp$ show interfaces portchannel
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available,
       S - selected, D - deselected, * - not synced
  No.  Team Dev        Protocol     Ports
-----  --------------  -----------  -------------------------
  100  PortChannel100  LACP(A)(Dw)  Ethernet8(D) Ethernet4(D)
admin@r-qa-sw-eth-53:/tmp$ sudo config portchannel del portchannel100
admin@r-qa-sw-eth-53:/tmp$ show interfaces portchannel
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available,
       S - selected, D - deselected, * - not synced
No.    Team Dev    Protocol    Ports
-----  ----------  ----------  -------
admin@r-qa-sw-eth-53:/tmp$

New command output (if the output of a command-line utility has changed)

admin@r-qa-sw-eth-53:~$ sudo config portchannel add portchannel100
admin@r-qa-sw-eth-53:~$ sudo config portchannel member add portchannel100 Ethernet4
admin@r-qa-sw-eth-53:~$ sudo config portchannel member add portchannel100 Ethernet8
admin@r-qa-sw-eth-53:~$ show interfaces portchannel
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available,
       S - selected, D - deselected, * - not synced
  No.  Team Dev        Protocol     Ports
-----  --------------  -----------  -------------------------
  100  PortChannel100  LACP(A)(Dw)  Ethernet8(D) Ethernet4(D)
admin@r-qa-sw-eth-53:~$ sudo config portchannel del portchannel100
Error: Portchannel portchannel100 contains members. Remove members before deleting Portchannel!
admin@r-qa-sw-eth-53:~$ show interfaces portchannel
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available,
       S - selected, D - deselected, * - not synced
  No.  Team Dev        Protocol     Ports
-----  --------------  -----------  -------------------------
  100  PortChannel100  LACP(A)(Dw)  Ethernet8(D) Ethernet4(D)
admin@r-qa-sw-eth-53:~$ sudo config portchannel member del portchannel100 Ethernet4
admin@r-qa-sw-eth-53:~$ sudo config portchannel member del portchannel100 Ethernet8
admin@r-qa-sw-eth-53:~$ sudo config portchannel del portchannel100
admin@r-qa-sw-eth-53:~$ show interfaces portchannel
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available,
       S - selected, D - deselected, * - not synced
No.    Team Dev    Protocol    Ports
-----  ----------  ----------  -------
admin@r-qa-sw-eth-53:~$

liat-grozovik · 2020-08-31T16:43:05Z

@madhanmellanox why do we get the usage when there is a specific error message due to the new checker?
are there any other UI which need to have this logic? should we add it also on orchagent while handling the port channel delete flow just in case it derives from config change ?

liat-grozovik · 2020-08-31T16:44:01Z

config/main.py

@@ -1230,7 +1230,10 @@ def add_portchannel(ctx, portchannel_name, min_links, fallback):
 def remove_portchannel(ctx, portchannel_name):
    """Remove port channel"""
    db = ctx.obj['db']
-    db.set_entry('PORTCHANNEL', portchannel_name, None)
+    if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0:
+        ctx.fail("Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name))


replace 'Portchannel' with 'Port channel'.

liat-grozovik · 2020-08-31T16:45:30Z

doc/Command-Reference.md

@@ -4838,6 +4838,8 @@ Command takes two optional arguements given below.
 1) min-links  - minimum number of links required to bring up the portchannel
 2) fallback - true/false. LACP fallback feature can be enabled / disabled.  When it is set to true, only one member port will be selected as active per portchannel during fallback mode. Refer https://github.com/Azure/SONiC/blob/master/doc/lag/LACP%20Fallback%20Feature%20for%20SONiC_v0.5.md for more details about fallback feature.

+A portchannel can be deleted only if it does not have any members or the members are already deleted. When a user tries to delete a portchannel and the portchannel still has one or more members that exist, the deletion of portchannel is blocked. 


replace 'Portchannel' with 'Port channel'.
IMO portchannel as CLI command, this is what we have as we dont have spaces. but for comments or CLI output messages i believe we should use it with space.

Addressed in the next commit.

…rror. Also, references of portchannel is replaced with port channel in the command reference guide

madhanmellanox · 2020-09-01T02:35:24Z

@madhanmellanox why do we get the usage when there is a specific error message due to the new checker?
are there any other UI which need to have this logic? should we add it also on orchagent while handling the port channel delete flow just in case it derives from config change ?

Addressed in the next commit.

madhanmellanox · 2020-09-08T19:01:43Z

@yxieca @lguohan can someone from MSFT review this PR? I am waiting for approval from reviewers with write access.

prsunny · 2020-09-08T19:07:37Z

@madhanmellanox , Liat has "requested changes". Can you ask Liat for approval?

madhanmellanox · 2020-09-08T19:10:14Z

@liat-grozovik can you please approve this PR?

madhanmellanox · 2020-09-15T16:39:16Z

@prsunny can this PR be cherry picked to 201911?

madhanmellanox · 2020-09-16T20:39:17Z

@yxieca can this PR be cherry picked to 201911?

* Handling deletion of Portchannel before deletion of its members * removed a extra space in the condifitional statement * added Note about portchannel deletion in Command-Reference guide * removed ctx.fail() and replaced it with click.echo() to display the error. Also, references of portchannel is replaced with port channel in the command reference guide Co-authored-by: Madhan Babu <madhan@arc-build-server.mtr.labs.mlnx>

Revert "Revert " [201911]show interface counters for multi ASIC devices (sonic-net#1104)"" Revert "Revert "Pfcstat (sonic-net#1097)"" [show] Fix 'show int neighbor expected' (sonic-net#1106) Update argument for docker exec it->i (sonic-net#1118) Update to make config load/reload backward compatible. (sonic-net#1115) Handling deletion of Port Channel before deletion of its members (sonic-net#1062) Skip default route present in ASIC-DB but not in APP-DB. (sonic-net#1107) [CLI][PFCWD][Multi-ASIC] Added multi ASIC support to 'pfcwd' CLI (sonic-net#1102) [201911] Multi asic platform config interface portchannel, show transceiver (sonic-net#1087) [drop counters] Fix configuration for counters with lowercase names (sonic-net#1103) Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

Madhan Babu added 3 commits August 12, 2020 02:26

Handling deletion of Portchannel before deletion of its members

219d78b

removed a extra space in the condifitional statement

828719e

added Note about portchannel deletion in Command-Reference guide

b36079f

madhanmellanox changed the title ~~Pcdelsumadhan~~ Handling deletion of Port Channel before deletion of its members Aug 21, 2020

madhanmellanox mentioned this pull request Aug 21, 2020

Interfaces are displayed as Portchannel members even after deletion of the Portchannel sonic-net/sonic-buildimage#5051

Closed

liat-grozovik requested changes Aug 31, 2020

View reviewed changes

removed ctx.fail() and replaced it with click.echo() to display the e…

b716b97

…rror. Also, references of portchannel is replaced with port channel in the command reference guide

liat-grozovik approved these changes Sep 14, 2020

View reviewed changes

liat-grozovik merged commit aa27dd9 into sonic-net:master Sep 14, 2020

yxieca added the Request for 201911 Branch label Sep 16, 2020

abdosi added the Included in 201911 Branch label Sep 19, 2020

This was referenced Sep 30, 2020

show interfaces status Corresponding portchannel is not deleted when config delete the portchannel sonic-net/sonic-buildimage#5457

Closed

Failed to delete LAG when it is a member of a vlan sonic-net/sonic-buildimage#5456

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handling deletion of Port Channel before deletion of its members #1062

Handling deletion of Port Channel before deletion of its members #1062

madhanmellanox commented Aug 21, 2020 •

edited

Loading

liat-grozovik commented Aug 31, 2020 •

edited

Loading

liat-grozovik Aug 31, 2020

liat-grozovik Aug 31, 2020

madhanmellanox Sep 1, 2020

madhanmellanox commented Sep 1, 2020

madhanmellanox commented Sep 8, 2020

prsunny commented Sep 8, 2020

madhanmellanox commented Sep 8, 2020

madhanmellanox commented Sep 15, 2020

madhanmellanox commented Sep 16, 2020

Handling deletion of Port Channel before deletion of its members #1062

Handling deletion of Port Channel before deletion of its members #1062

Conversation

madhanmellanox commented Aug 21, 2020 • edited Loading

liat-grozovik commented Aug 31, 2020 • edited Loading

liat-grozovik Aug 31, 2020

Choose a reason for hiding this comment

liat-grozovik Aug 31, 2020

Choose a reason for hiding this comment

madhanmellanox Sep 1, 2020

Choose a reason for hiding this comment

madhanmellanox commented Sep 1, 2020

madhanmellanox commented Sep 8, 2020

prsunny commented Sep 8, 2020

madhanmellanox commented Sep 8, 2020

madhanmellanox commented Sep 15, 2020

madhanmellanox commented Sep 16, 2020

madhanmellanox commented Aug 21, 2020 •

edited

Loading

liat-grozovik commented Aug 31, 2020 •

edited

Loading