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

Prevent shutdown/startup commands on invalid interface names "fixes (SONiC/issues/268)" #424

Merged
merged 6 commits into from
Jan 22, 2019

Conversation

kirankella
Copy link
Contributor

@kirankella kirankella commented Jan 4, 2019

- What I did
Invalid interface names are pushed into the CONFIG DB though they don't have a valid alias name, due to a missing check in shutdown/startup functions.
Later show commands that check for alias names on the interfaces fail.

- How I did it
In the main.py utilities script, check if the given interface name exists in the PORT_TABLE in config db. If not, fail the shutdown/startup functions there throwing an error to the user prompting for a valid interface name.

- How to verify it
Issue shutdown or startup commands on invalid interface names, followed by show interfaces status.

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

root@sonic-testing:/home/admin# config interface Ethernet99 shutdown
root@sonic-testing:/home/admin# show interface status
Traceback (most recent call last):
File "/usr/bin/show", line 9, in
load_entry_point('sonic-utilities==1.2', 'console_scripts', 'show')()
File "/usr/lib/python2.7/dist-packages/pkg_resources/init.py", line 561, in load_entry_point
return get_distribution(dist).load_entry_point(group, name)
File "/usr/lib/python2.7/dist-packages/pkg_resources/init.py", line 2631, in load_entry_point
return ep.load()
File "/usr/lib/python2.7/dist-packages/pkg_resources/init.py", line 2291, in load
return self.resolve()
File "/usr/lib/python2.7/dist-packages/pkg_resources/init.py", line 2297, in resolve
module = import(self.module_name, fromlist=['name'], level=0)
File "/usr/lib/python2.7/dist-packages/show/main.py", line 193, in
iface_alias_converter = InterfaceAliasConverter()
File "/usr/lib/python2.7/dist-packages/show/main.py", line 58, in init
self.port_dict[port_name]['alias']):
KeyError: 'alias'

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

root@sonic:/home/admin# config interface Ethernet99 shutdown
Usage: config interface shutdown [OPTIONS]

Error: Enter valid interface name!!

root@sonic:/home/admin# show interface status
Interface Lanes Speed MTU Alias Oper Admin Type


Ethernet0 13 N/A 9100 tenGigE0 down up SFP
Ethernet1 14 N/A 9100 tenGigE1 down up N/A
Ethernet2 15 N/A 9100 tenGigE2 down up SFP
....
-->

…terface status

A bug in intfutil script that gets the keys object from the app-db for a specific interface

Signed-off-by: kiran.kella@broadcom.com
@msftclas
Copy link

msftclas commented Jan 7, 2019

CLA assistant check
All CLA requirements met.

config/main.py Outdated Show resolved Hide resolved
scripts/intfutil Outdated Show resolved Hide resolved
…stead of the optional alias attribute

* Addressed review comment for sonic-net#424
* Undone the change in intfutil file to push that fix in a seperate PR.

Signed-off-by: kiran.kella@broadcom.com
config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
config/main.py Outdated Show resolved Hide resolved
@jleveque jleveque merged commit 9e49466 into sonic-net:master Jan 22, 2019
@leoli-nps
Copy link

Hi kirankella, with your changes, I have another problem. When I use the command “sudo config interface PortChannel0001 shutdown”, the terminal prompts that the interface name is invalid, as shown below:
admin@sonic:~$ sudo config interface PortChannel0001 shutdown
Usage: config interface shutdown [OPTIONS]

Error: Interface name is invalid. Please enter a valid interface name!!
admin@sonic:~$ show interfaces portchannel
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected
No. Team Dev Protocol Ports


0001 PortChannel0001 LACP(A)(Up) Ethernet108(S)
admin@sonic:~$

So, when checking the validity of the interface name, should it also include the PORTCHANNEL table, not just in the PORT table?
@kirankella

@kirankella
Copy link
Contributor Author

kirankella commented Feb 28, 2019

Hi kirankella, with your changes, I have another problem. When I use the command “sudo config interface PortChannel0001 shutdown”, the terminal prompts that the interface name is invalid, as shown below:
admin@sonic:~$ sudo config interface PortChannel0001 shutdown
Usage: config interface shutdown [OPTIONS]

Error: Interface name is invalid. Please enter a valid interface name!!
admin@sonic:~$ show interfaces portchannel
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected
No. Team Dev Protocol Ports

0001 PortChannel0001 LACP(A)(Up) Ethernet108(S)
admin@sonic:~$

So, when checking the validity of the interface name, should it also include the PORTCHANNEL table, not just in the PORT table?
@kirankella

@leoli-nps You are right. We should be checking the PORTCHANNEL table as well. Raised PR #474 to fix the same.

qiluo-msft pushed a commit that referenced this pull request Mar 30, 2019
…erface names (#474)

* Avoid shutdown/startup commands on invalid interface names
* sonic-utilities: Fix bug in the show command to display a specific interface status
* sonic-utilities: Check for the presence of interface in port table instead of the optional alias attribute
* Addressed review comment for #424
* Undone the change in intfutil file to push that fix in a seperate PR.
* Corrected the error message string for 'config interface <invalid-interface-name> tartup/shutdown'.
* [sonic-utilities] Fix to shutdown and startup on valid PortChannel interface names
* [sonic-utilities] Allow shutdown/startup commands to be done using the alias names of PORTs.
yxieca pushed a commit that referenced this pull request Mar 30, 2019
…erface names (#474)

* Avoid shutdown/startup commands on invalid interface names
* sonic-utilities: Fix bug in the show command to display a specific interface status
* sonic-utilities: Check for the presence of interface in port table instead of the optional alias attribute
* Addressed review comment for #424
* Undone the change in intfutil file to push that fix in a seperate PR.
* Corrected the error message string for 'config interface <invalid-interface-name> tartup/shutdown'.
* [sonic-utilities] Fix to shutdown and startup on valid PortChannel interface names
* [sonic-utilities] Allow shutdown/startup commands to be done using the alias names of PORTs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants