[multi-asic] show ip interface changes for multi asic #1396
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
arlakshm
commented
Feb 2, 2021
This comment has been minimized.
This comment has been minimized.
jleveque
suggested changes
Feb 5, 2021
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
jleveque
suggested changes
Feb 8, 2021
tests/show_ip_int_test.py
Outdated
| 'sudo ip link del Vlan100', | ||
| 'sudo sysctl -w net.ipv6.conf.default.addr_gen_mode=1', | ||
| 'sudo sysctl -w net.ipv6.conf.all.disable_ipv6=1' | ||
|
|
There was a problem hiding this comment.
looks like we have an issue with using sysctl in docker, @jleveque ?
There was a problem hiding this comment.
Yes. Good catch. Discussion is here: #1406. We need to figure out a solution.
There was a problem hiding this comment.
@jleveque , @prsunny
I am able to use the sysctl commands in the slave docker.
arlakshm@a0b00d89505a:/sonic/src/sonic-utilities$ sudo sysctl net.ipv6.conf.all.disable_ipv6
net.ipv6.conf.all.disable_ipv6 = 1
arlakshm@a0b00d89505a:/sonic/src/sonic-utilities$ sudo sysctl -w net.ipv6.conf.all.disable_ipv6=0
net.ipv6.conf.all.disable_ipv6 = 0
arlakshm@a0b00d89505a:/sonic/src/sonic-utilities$ sudo sysctl net.ipv6.conf.all.disable_ipv6
net.ipv6.conf.all.disable_ipv6 = 0
arlakshm@a0b00d89505a:/sonic/src/sonic-utilities$ sudo sysctl -w net.ipv6.conf.all.disable_ipv6=1
net.ipv6.conf.all.disable_ipv6 = 1
arlakshm@a0b00d89505a:/sonic/src/sonic-utilities$ sudo sysctl net.ipv6.conf.all.disable_ipv6
net.ipv6.conf.all.disable_ipv6 = 1
arlakshm@a0b00d89505a:/sonic/src/sonic-utilities$
We do similar configuration for the ptf docker in sonic-mgmt
There was a problem hiding this comment.
I wanted to use 'netifaces' apis in the tests instead of mocking the data. If this might cause as issue, let me know I can rewrite the tests
There was a problem hiding this comment.
@arlakshm: I have confirmed that "privileged" mode is the difference. However, please see the comments here. Tests which rely on interacting with the system should be reserved for sonic-mgmt tests. For unit testing, you should simply mock any system calls and return contrived output that you can test against.
There was a problem hiding this comment.
@jleveque, update the unit tests to use mock data in the latest commit.
|
This pull request introduces 3 alerts when merging 67de06f into 794cdd3 - view on LGTM.com new alerts:
|
jleveque
suggested changes
Feb 8, 2021
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
Fixed the LGTMs warnings |
jleveque
suggested changes
Feb 8, 2021
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
jleveque
approved these changes
Feb 9, 2021
3 tasks
arlakshm
added a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Feb 23, 2021
show ip interfaces is enhanced recently to support multi ASIC platforms in this PR- sonic-net/sonic-utilities#1396 . The ipintutil script as to run as sudo user, to get the ip interface from each namespace. Add this script to the sudoer file so that show ip interface command is available for user with read-only permissions Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
anand-kumar-subramanian
pushed a commit
to anand-kumar-subramanian/sonic-utilities
that referenced
this pull request
Mar 2, 2021
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
- What I did
The PR has changes to support command show ip interface for multi asic platform.
- How I did it
The following changes are done in this PR
The code for show ip interface has been moved from show/main.py to scripts\ipintutil
- Modify the code to support both single and multi asic platform
- To do this the library pyroute2 is used to get interface information in each namespace
- Add the following options for multi asic
[-n, --namespace] to allow user to display the information for given namespaces
If this option is not present the information from all the namespaces will be displayed
[-d, --display] to allow user to display information related both internal and external interfaces
If this option is not present only external interfaces/neighbors will be display
- Add unit tests for single and multi asic
qiluo-msft
reviewed
Mar 4, 2021
| from tabulate import tabulate | ||
|
|
||
| from sonic_py_common import multi_asic | ||
| from swsssdk import ConfigDBConnector, SonicDBConfig |
There was a problem hiding this comment.
swsssdk is deprecated largely in sonic-utilities. Could you adapt the new ConfigDBConnector as in latest code?
3 tasks
lguohan
pushed a commit
to sonic-net/sonic-buildimage
that referenced
this pull request
Mar 14, 2021
show ip interfaces is enhanced recently to support multi ASIC platforms in this PR- sonic-net/sonic-utilities#1396 . The ipintutil script as to run as sudo user, to get the ip interface from each namespace. Add this script to the sudoer file so that show ip interface command is available for user with read-only permissions Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
carl-nokia
pushed a commit
to carl-nokia/sonic-buildimage
that referenced
this pull request
Aug 7, 2021
show ip interfaces is enhanced recently to support multi ASIC platforms in this PR- sonic-net/sonic-utilities#1396 . The ipintutil script as to run as sudo user, to get the ip interface from each namespace. Add this script to the sudoer file so that show ip interface command is available for user with read-only permissions Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
|
This PR could not be cleanly cherry-pick to 202012. Please submit another PR. |
judyjoseph
added a commit
to judyjoseph/sonic-utilities
that referenced
this pull request
Oct 7, 2021
judyjoseph
added a commit
that referenced
this pull request
Jan 26, 2022
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan arlakshm@microsoft.com
- What I did
The PR has changes to support command
show ip interfacefor multi asic platform.- How I did it
The following changes are done in this PR
show ip interfacehas been moved fromshow/main.pytoscripts\ipintutil[-n, --namespace] to allow user to display the information for given namespaces
If this option is not present the information from all the namespaces will be displayed
[-d, --display] to allow user to display information related both internal and external interfaces
If this option is not present only external interfaces/neighbors will be display
- How to verify it
Verify on single and multi asic
Code Coverage
- Previous command output (if the output of a command-line utility has changed)
- New command output (if the output of a command-line utility has changed)