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.

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

[config] Fix issues on config remove_portchannel #1206

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ljm625
Copy link

@ljm625 ljm625 commented Nov 2, 2020

- What I did
Fixed the behaviour of config portchannel del command

In current remove_portchannel code, it didn't check whether the portchannel is configured with an ip address before delete, as a result, it messed up the redis config db.

Also in current code, it didn't remove the PortChannel interface under PORTCHANNEL_INTERFACE when deleting, so also leaving junk when deleting PortChannel

- How I did it

In addition to previous implemented portchannel member check code, now it also checks if the ip is configured on portchannel.

If the IP is configured the delete process is aborted.

While deleting now it will also delete the interface under PORTCHANNEL_INTERFACE.

- How to verify it

root@sonic:~# config portchannel add PortChannel0
root@sonic:~# 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
-----  ------------  -----------  -------
    0  PortChannel0  LACP(A)(Dw)  N/A
root@sonic:~# config interface ip add PortChannel0 1.1.1.1/32
root@sonic:~# show run all | grep -i port
        "PORT": {
    "PORT": {
    "PORTCHANNEL": {
        "PortChannel0": {
    "PORTCHANNEL_INTERFACE": {
        "PortChannel0": {},
        "PortChannel0|1.1.1.1/32": {}
            "port": "8080"
# Now proceed to delete
root@sonic:~# config portchannel del PortChannel0
root@sonic:~# show run all | grep -i port
        "PORT": {
    "PORT": {
    "PORTCHANNEL_INTERFACE": {
        "PortChannel0": {},
        "PortChannel0|1.1.1.1/32": {}
            "port": "8080"

As You can see the PortChannel's IP address and Portchannel under PORTCHANNEL_INTERFACE still exists.

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

root@sonic:~# config portchannel add PortChannel0
root@sonic:~# 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
-----  ------------  -----------  -------
    0  PortChannel0  LACP(A)(Dw)  N/A
root@sonic:~# config interface ip add PortChannel0 1.1.1.1/32
root@sonic:~# show run all | grep -i port
        "PORT": {
    "PORT": {
    "PORTCHANNEL": {
        "PortChannel0": {
    "PORTCHANNEL_INTERFACE": {
        "PortChannel0": {},
        "PortChannel0|1.1.1.1/32": {}
            "port": "8080"
# Now proceed to delete
root@sonic:~# config portchannel del PortChannel0
root@sonic:~# show run all | grep -i port
        "PORT": {
    "PORT": {
    "PORTCHANNEL_INTERFACE": {
        "PortChannel0": {},
        "PortChannel0|1.1.1.1/32": {}
            "port": "8080"
root@sonic:~# config interface ip remove PortChannel0 1.1.1.1/32
Cannot find device "PortChannel0"

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

root@sonic:~# config portchannel add PortChannel0
root@sonic:~# 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
-----  ------------  -----------  -------
    0  PortChannel0  LACP(A)(Dw)  N/A
root@sonic:~# config interface ip add PortChannel0 1.1.1.1/32
root@sonic:~# show run all | grep -i port
        "PORT": {
    "PORT": {
    "PORTCHANNEL": {
        "PortChannel0": {
    "PORTCHANNEL_INTERFACE": {
        "PortChannel0": {},
        "PortChannel0|1.1.1.1/32": {}
            "port": "8080"
root@sonic:~# config portchannel del PortChannel0
Error: Portchannel PortChannel0 contains ips. Remove ip before deleting Portchannel!
root@sonic:~# config interface ip remove PortChannel0 1.1.1.1/32
root@sonic:~# config portchannel del PortChannel0
root@sonic:~# show run all | grep -i port
        "PORT": {
    "PORT": {
            "port": "8080"

1. Add checks on portchannel interface ip before removing
2. Remove junks in PORTCHANNEL_INTERFACE after removing

Signed-off-by: Jiaming Li <jiaminli@cisco.com>
@ljm625 ljm625 changed the title Fix issues on config remove_portchannel [config] Fix issues on config remove_portchannel Nov 2, 2020
@prsunny
Copy link
Contributor

prsunny commented Nov 4, 2020

retest this please

1 similar comment
@prsunny
Copy link
Contributor

prsunny commented Nov 4, 2020

retest this please

@ljm625 ljm625 closed this Nov 5, 2020
@ljm625 ljm625 reopened this Nov 5, 2020
@lguohan
Copy link
Contributor

lguohan commented Nov 5, 2020

can you add unit test?

@ljm625
Copy link
Author

ljm625 commented Nov 5, 2020

well I can add unit test of this part, but actually since this code is just adding a new check while configuring, it should be fine with current test.

Also I didn't see any current test code/files relating to portchannel config actions, the only one I saw was "test_config_vlan_add_nonexist_portchannel_member", so not sure where to start also

@ljm625 ljm625 closed this Nov 16, 2020
@ljm625 ljm625 reopened this Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants