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

[MPLS][CLI][Show] Fixed associated bugs with invalid interfaces in MPLS #1770

Merged
merged 6 commits into from
Aug 20, 2021

Conversation

developfast
Copy link
Contributor

What I did

Added bug fixes for MPLS CLI commands where mpls show and config commands would allow invalid interfaces to be passed into the command, without displaying an error. No longer require the "-d all" flag to show internal interfaces too, when a specific interface is asked to "show". Also added respective unit tests to check out the errors, and test the expected output.

How I did it

Modified the config/main.py to add logic to detect invalid interfaces by comparing against PORTCHANNEL_INTERFACES, INTERFACE, and VLAN_INTERFACE. Similarly added logic in show/interfaces in init.py to detect invalid interfaces by comparing interfaces across all ASICs.

How to verify it

Type the following commands into the CLI, and see the output:

"show interfaces mpls $VALIDPORTCHANNEL" -> should output a table format of whether that interface has MPLS enabled or not. Notice the "d-all" flag is not required here, an internal interface will also be displayed when called.

"show interfaces mpls $INVALID_PORTCHANNEL" -> produces an error as invalid interface

"sudo config interface -n $QASIC# mpls add $INVALID_PORTCHANNEL" -> produces an error as invalid interface

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

For configuring an invalid interface in an ASIC:
"""
admin@vlab-07:~$ sudo config interface -n asic2 mpls add PortChannel4011
"""

No error is produced, which it should produce.

For showing an invalid interface in an ASIC:
"""
admin@vlab-07: show interfaces mpls PortChannel4001
Interface MPLS State


"""

An error should be produced, which it is not.

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

For configuring an invalid interface in an ASIC:
"""
admin@vlab-07:~$ sudo config interface -n asic2 mpls add PortChannel4011
Usage: config interface mpls add [OPTIONS] <interface_name>
Try "config interface mpls add -h" for help.

Error: interface PortChannel4011 doesn`t exist
"""

For showing an invalid interface in an ASIC:
"""
admin@vlab-07:~$ show interfaces mpls PortChannel3007
Error: Invalid interface. Interface not found.
"""

Valid show mpls interface:
"""
admin@vlab-07: show interfaces mpls PortChannel4001
Interface MPLS State


PortChannel4001 enable
"""

Notice the "-d all" flag is not required. Internal interfaces are also displayed when trying to show a specific interface.

…s, and added appropriate unit tests

Signed-off-by: Dev Ojha <[email protected]>
(cherry picked from commit 19bb318)
@lgtm-com
Copy link

lgtm-com bot commented Aug 17, 2021

This pull request introduces 1 alert when merging 950490e into 5002745 - view on LGTM.com

new alerts:

  • 1 for Unreachable code

@lgtm-com
Copy link

lgtm-com bot commented Aug 18, 2021

This pull request introduces 1 alert when merging e9a984d into 5002745 - view on LGTM.com

new alerts:

  • 1 for Unreachable code

@smaheshm smaheshm self-requested a review August 18, 2021 18:06
utilities_common/cli.py Outdated Show resolved Hide resolved
utilities_common/cli.py Outdated Show resolved Hide resolved
tests/mpls_test.py Outdated Show resolved Hide resolved
tests/mpls_test.py Outdated Show resolved Hide resolved
tests/mpls_test.py Outdated Show resolved Hide resolved
tests/mpls_test.py Outdated Show resolved Hide resolved
tests/mpls_test.py Outdated Show resolved Hide resolved
@smaheshm
Copy link
Contributor

rest looks good. Just update as per the latest comments.

Copy link
Contributor

@smaheshm smaheshm left a comment

Choose a reason for hiding this comment

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

Looks good.

@smaheshm smaheshm merged commit 103de86 into sonic-net:master Aug 20, 2021
@yozhao101
Copy link
Contributor

@devadityaojha Please remove ... and update the title of this PR.

@developfast developfast changed the title Fixed show and config mpls cli bug where invalid interfaces would pas… [MPLS][CLI] Fixed associated bugs with invalid interfaces Aug 22, 2021
@developfast developfast changed the title [MPLS][CLI] Fixed associated bugs with invalid interfaces [MPLS][CLI] Fixed associated bugs with invalid interfaces in MPLS Aug 22, 2021
@developfast developfast changed the title [MPLS][CLI] Fixed associated bugs with invalid interfaces in MPLS [MPLS][CLI][Show] Fixed associated bugs with invalid interfaces in MPLS Aug 22, 2021
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