support show interface commands for multi ASIC platforms #1006
Conversation
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
This comment has been minimized.
This comment has been minimized.
scripts/intfutil
Outdated
| from utilities_common.constants import PORT_CHANNEL_OBJ | ||
| from utilities_common.constants import PORT_OBJ | ||
| from utilities_common.multi_asic import MultiAsic | ||
| from utilities_common.multi_asic import multi_asic_args | ||
| from utilities_common.multi_asic import run_on_all_asics |
There was a problem hiding this comment.
Fixed in latest commit
scripts/intfutil
Outdated
| appl_db_sub_intf_status_get(self.db, self.config_db, self.front_panel_ports_list, self.portchannel_speed_dict, sub_intf, PORT_OPTICS_TYPE))) | ||
| return table | ||
|
|
||
| def __init__(self, intf_name, namespace_option, display_option): |
scripts/intfutil
Outdated
|
|
||
| def __init__(self, intf_name): | ||
| def __init__(self, intf_name, namespace_option, display_option): |
There was a problem hiding this comment.
Please also add/modify unit tests as necessary to add coverage for intfutil: https://github.com/Azure/sonic-utilities/blob/master/sonic-utilities-tests/intfutil_test.py
|
i think we need to add new unit test to cover the new functions. |
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
This comment has been minimized.
This comment has been minimized.
arlakshm
force-pushed
the
masic_intfutil
branch
from
7d5cded
to
9ca8716
Compare
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
This comment has been minimized.
This comment has been minimized.
|
retest this please |
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
|
This pull request fixes 3 alerts when merging da04d1a into 8768580 - view on LGTM.com fixed alerts:
|
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
|
This pull request fixes 3 alerts when merging 99f0741 into 8768580 - view on LGTM.com fixed alerts:
|
jleveque
dismissed
their stale review
Looks good to me. Wait for other reviewers.
scripts/intfutil
Outdated
| modules_path = os.path.join(os.path.dirname(__file__), "..") | ||
| tests_path = os.path.join(modules_path, "tests") | ||
| sys.path.insert(0, modules_path) | ||
| sys.path.insert(0, tests_path) | ||
| import mock_tables.dbconnector | ||
| if os.environ["UTILITIES_UNIT_TESTING"] == "3": |
There was a problem hiding this comment.
i think we need to keep this as '2': but maybe we can introduce a new environmental variable, "UTILITIES_UNIT_TESTING_TOPOLOGY"="multi-asic"
|
in the unit test, we do have backend asic v.s. frontend asic? |
| result = subprocess.check_output( | ||
| 'intfutil -c description -n asic0 -d all', stderr=subprocess.STDOUT, shell=True) | ||
| print(result) | ||
| assert result == intf_description_asic0_all |
There was a problem hiding this comment.
please assert exit_code for all cases.
There was a problem hiding this comment.
Fixed in latest commit
The cli command is implementation is same for backend or frontend asic. For any asic either the internal or all the interfaces are displayed |
| assert result == intf_description_asic0_all | ||
|
|
||
|
|
||
| def teardown_class(cls): |
There was a problem hiding this comment.
i do not see any negative unit test cases, for example, check an asic name that does not exists.
There was a problem hiding this comment.
add negative cases for invalid asic-id
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
|
This pull request fixes 3 alerts when merging ee66cc5 into 5263b54 - view on LGTM.com fixed alerts:
|
| def get_result_and_return_code(self, cmd): | ||
| return_code = 0 | ||
| try: | ||
| output = subprocess.check_output( | ||
| cmd, stderr=subprocess.STDOUT, shell=True) | ||
| except subprocess.CalledProcessError as e: | ||
| return_code = e.returncode | ||
| #store only the error, no need for the traceback | ||
| output = e.output.strip().split("\n")[-1] | ||
|
|
||
| return(return_code, output) |
There was a problem hiding this comment.
This is a good candidate to be a shared function between many tests. In the future, we can maybe consider creating a base class and adding this to it.
Support show interface commands for multi ASIC platforms
Depends on the following PRs
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan arlakshm@microsoft.com
- What I did
Support the following commands for multi ASIC
2 new options have added
[-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
One single ASIC platform, this options ignored, so the behavior remains unchanged
- How I did it
-nand-dclick options- How to verify it
Help menu
Handling for invalid namespaces
Sample output
UT results
- Previous command output (if the output of a command-line utility has changed)
NA
- New command output (if the output of a command-line utility has changed)
NA