[Mellanox] SKU creator Tool#1163
Conversation
|
This pull request introduces 14 alerts when merging 5c64dc43c2dfee31207959c11b6c7f5b791ee9e2 into 561d133 - view on LGTM.com new alerts:
|
…reate a new HWSKU based on XML file. Also added file sku_create_test.py pyTest unit test script to test the same
763db24 to
58e9708
Compare
|
This pull request introduces 1 alert when merging 58e9708 into 3a7457c - view on LGTM.com new alerts:
|
|
@lguohan can someone from MSFT review this SKU creator tool PR? |
|
retest |
|
retest this please |
1 similar comment
|
retest this please |
|
@qiluo-msft This is the PR for SKU creator tool. Can you please review this? |
scripts/sonic_sku_create.py
Outdated
|
|
||
| optional arguments: | ||
| -h, --help Show this help message and exit | ||
| -v, --version show program's version number and exit |
There was a problem hiding this comment.
show [](start = 24, length = 4)
Capitalize #Closed
There was a problem hiding this comment.
addressed in next commit
scripts/sonic_sku_create.py
Outdated
| @@ -0,0 +1,805 @@ | |||
| #! /usr/bin/env python | |||
There was a problem hiding this comment.
python [](start = 16, length = 6)
Please make sure the same script could be run by python3 #Closed
There was a problem hiding this comment.
tabulate module used in the script does not work with python3. It says module not found.
There was a problem hiding this comment.
So if this is true, please explicitly use python2 in shebang line.
In reply to: 508805018 [](ancestors = 508805018)
There was a problem hiding this comment.
Addressed in next commit
scripts/sonic_sku_create.py
Outdated
| minigraph_ns1 = "http://schemas.datacontract.org/2004/07/Microsoft.Search.Autopilot.Evolution" | ||
| INTERFACE_KEY = "Ethernet" | ||
|
|
||
| test_envir = None |
There was a problem hiding this comment.
Could you remove test related code out of this script? #Closed
There was a problem hiding this comment.
Yes, I will remove test related code of the script. So, I may have to remove the pytest for testing the SKU creator tool also. Is that okay?
| ### port_config.ini header | ||
| PORTCONFIG_HEADER = ["# name", "lanes", "alias", "index", "speed"] | ||
| platform_4 = ['x86_64-mlnx_lssn2700-r0','x86_64-mlnx_msn2010-r0','x86_64-mlnx_msn2100-r0','x86_64-mlnx_msn2410-r0','x86_64-mlnx_msn2700-r0','x86_64-mlnx_msn2740-r0','x86_64-mlnx_msn3700c-r0','x86_64-mlnx_msn3700-r0','x86_64-mlnx_msn3800-r0'] | ||
| platform_8 = ['x86_64-mlnx_msn4600c-r0','x86_64-mlnx_msn4700-r0'] |
There was a problem hiding this comment.
Why we have mlnx platforms? I thought this script is general to any platform. #Closed
There was a problem hiding this comment.
This script as of now is for creating SKU for Mellanox Platforms. To make SKUs for other platforms, we may have to bring in the details of other platforms and process specific to that platform, because, port name, lane number and other restrictions may change according to respective platform.
There was a problem hiding this comment.
Parsing of xml file or minigraph file for SKU definition does not have any Mellanox related definitions.
scripts/sonic_sku_create.py
Outdated
| idx +=1 | ||
|
|
||
| if match is None : | ||
| raise ValueError("Couldn't find a SKU ",hwsku, "in minigraph file") |
There was a problem hiding this comment.
, [](start = 51, length = 1)
Add one blank char after , #Closed
scripts/sonic_sku_create.py
Outdated
| self.parse_deviceinfo(child,hwsku) | ||
|
|
||
| def check_json_lanes_with_bko(self, data, port_idx): | ||
| #Function to find matching entry in bko_dict that matches Port details from config_db.json file |
There was a problem hiding this comment.
[](start = 8, length = 1)
Add one blank char after # #Closed
There was a problem hiding this comment.
Addressed in next commit
scripts/sonic_sku_create.py
Outdated
| if "speed" in curr_port_dict: | ||
| curr_speed = curr_port_dict.get("speed") | ||
| else: | ||
| print(curr_port_str, "does not contain speed key, Exiting...") |
There was a problem hiding this comment.
print [](start = 20, length = 5)
print error messages to stderr #Closed
scripts/sonic_sku_create.py
Outdated
| group.add_argument('-f', '--file', action='store', nargs=1, help='SKU definition from xml file. -f OR -m or -j must be provided when creating a new SKU', default=None) | ||
| group.add_argument('-m', '--minigraph_file', action='store', nargs='?', help='SKU definition from minigraph file. -f OR -m or -j must be provided when creating a new SKU', const="/etc/sonic/minigraph.xml") | ||
| group.add_argument('-j', '--json_file', action='store', nargs=1, help='SKU definition from config_db.json file. -f OR -m OR -j must be provided when creating a new SKU', default=None) | ||
| group.add_argument('-pp', '--port_split', action='store', nargs=2, help='port name and split', default=None) |
There was a problem hiding this comment.
-pp [](start = 24, length = 3)
Linux command convention is using single char in short option name. In this case, it's ok to have only long option name.
And one more below. #Closed
There was a problem hiding this comment.
Addressed in next commit.
| return True | ||
|
|
||
| def test_no_param(self): | ||
| my_command = sku_create_script + " -f " + sku_def_file + " -d " + input_path + " -pl " + "x86_64-mlnx_msn2700-r0" |
There was a problem hiding this comment.
-pl [](start = 91, length = 3)
Could you also test without -pl ? Which should be more common use case. #Closed
There was a problem hiding this comment.
Addressed in next commit.
|
This pull request introduces 1 alert when merging 9eb7121 into acfa824 - view on LGTM.com new alerts:
|
|
@madhanmellanox: We renamed the @liat-grozovik, @qiluo-msft for visibility. |
|
@jleveque can this SKU creator tool PR be cherry picked to 201911? Or I have to create a new PR for 201911? |
|
@madhanmellanox: This PR might cherry-pick cleanly, since the directory is still called |
I added a new script file sonic_sku_create.py to generate a new HWSKU based on XML file or Minigraph. But, focus is on creating HWSKU based on XML file and not on the minigraph and l2 mode. I also added a unit test pyTest script sku_create_test.py in sonic-utilities-test folder to test the script in Unit Testing mode. Motivation: To create SKU for Mellanox platforms based on XML file with Port related inputs and also through Minigraph file. This tool also allows to split a port or unsplit ports based on configuration which modifies the port related information in port_config.ini and config_db.json usage: sonic_sku_create.py [-h] [-v] (-f FILE | -m [MINIGRAPH_FILE] | -j JSON_FILE | -pp PORT_SPLIT PORT_SPLIT) [-b BASE] [-r] [-k HWSKU] [-p] [-vv] Create a new SKU optional arguments: -h, --help show this help message and exit -v, --version show program's version number and exit -f FILE, --file FILE SKU definition from xml file. -f OR -m or -j must be provided when creating a new SKU -m [MINIGRAPH_FILE], --minigraph_file [MINIGRAPH_FILE] SKU definition from minigraph file. -f OR -m or -j must be provided when creating a new SKU -j JSON_FILE, --json_file JSON_FILE SKU definition from config_db.json file. -f OR -m OR -j must be provided when creating a new SKU -pp PORT_SPLIT PORT_SPLIT, --port_split PORT_SPLIT PORT_SPLIT port name and split -b BASE, --base BASE SKU base definition -r, --remove Remove SKU folder -k HWSKU, --hwsku HWSKU SKU name to be used when creating a new SKU or for L2 configuration mode -p, --print Print port_config.ini without creating a new SKU -vv, --verbose Verbose output
**- What I did** I added a new script file sonic_sku_create.py to generate a new HWSKU based on XML file or Minigraph. But, focus is on creating HWSKU based on XML file and not on the minigraph and l2 mode. I also added a unit test pyTest script sku_create_test.py in sonic-utilities-test folder to test the script in Unit Testing mode. Motivation: To create SKU for Mellanox platforms based on XML file with Port related inputs and also through Minigraph file. This tool also allows to split a port or unsplit ports based on configuration which modifies the port related information in port_config.ini and config_db.json usage: sonic_sku_create.py [-h] [-v] (-f FILE | -m [MINIGRAPH_FILE] | -j JSON_FILE | -pp PORT_SPLIT PORT_SPLIT) [-b BASE] [-r] [-k HWSKU] [-p] [-vv] Create a new SKU optional arguments: -h, --help show this help message and exit -v, --version show program's version number and exit -f FILE, --file FILE SKU definition from xml file. -f OR -m or -j must be provided when creating a new SKU -m [MINIGRAPH_FILE], --minigraph_file [MINIGRAPH_FILE] SKU definition from minigraph file. -f OR -m or -j must be provided when creating a new SKU -j JSON_FILE, --json_file JSON_FILE SKU definition from config_db.json file. -f OR -m OR -j must be provided when creating a new SKU -pp PORT_SPLIT PORT_SPLIT, --port_split PORT_SPLIT PORT_SPLIT port name and split -b BASE, --base BASE SKU base definition -r, --remove Remove SKU folder -k HWSKU, --hwsku HWSKU SKU name to be used when creating a new SKU or for L2 configuration mode -p, --print Print port_config.ini without creating a new SKU -vv, --verbose Verbose output **- How I did it** I did it by adding these files which are required for new SKU generation in the forthcoming releases. **- How to verify it** I verified all the options of sonic_sku_create.py file and tested them thoroughly as well as created a test case in sku_create_test.py file to test the functionality of sonic_sku_create.py in Unit Test mode.
ccb5245 (HEAD -> 201911, origin/201911) [fast-reboot] Fix fast-reboot when NDP entries are present (sonic-net#1295) d09667b Multi-ASIC support for show ip(v6) route (201911 branch) (sonic-net#1283) 28399bf [201911-Mellanox] SKU creator Tool (sonic-net#1163) (sonic-net#1250) Signed-off-by: Abhishek Dosi <[email protected]>
I added a new script file sonic_sku_create.py to generate a new HWSKU based on XML file or Minigraph. But, focus is on creating HWSKU based on XML file and not on the minigraph and l2 mode. I also added a unit test pyTest script sku_create_test.py in sonic-utilities-test folder to test the script in Unit Testing mode.
Motivation: To create SKU for Mellanox platforms based on XML file with Port related inputs and also through Minigraph file.
This tool also allows to split a port or unsplit ports based on configuration which modifies the port related information in port_config.ini and config_db.json
usage: sonic_sku_create.py [-h] [-v]
(-f FILE | -m [MINIGRAPH_FILE] | -j JSON_FILE | -pp PORT_SPLIT PORT_SPLIT)
[-b BASE] [-r] [-k HWSKU] [-p] [-vv]
Create a new SKU
optional arguments:
-h, --help show this help message and exit
-v, --version show program's version number and exit
-f FILE, --file FILE SKU definition from xml file. -f OR -m or -j must be provided when creating a new SKU
-m [MINIGRAPH_FILE], --minigraph_file [MINIGRAPH_FILE]
SKU definition from minigraph file. -f OR -m or -j must be provided when creating a new SKU
-j JSON_FILE, --json_file JSON_FILE
SKU definition from config_db.json file. -f OR -m OR -j must be provided when creating a new SKU
-pp PORT_SPLIT PORT_SPLIT, --port_split PORT_SPLIT PORT_SPLIT
port name and split
-b BASE, --base BASE SKU base definition
-r, --remove Remove SKU folder
-k HWSKU, --hwsku HWSKU
SKU name to be used when creating a new SKU or for L2 configuration mode
-p, --print Print port_config.ini without creating a new SKU
-vv, --verbose Verbose output
Output:
Creation of Port_config.ini file.
Modifying Port_config.ini and config_db.json based on port split and unsplit configuration.
How I did it
I did it by adding these files which are required for new SKU generation in the forthcoming releases.
How to verify it
I verified all the options of sonic_sku_create.py file and tested them thoroughly as well as created a test case in sku_create_test.py file to test the functionality of sonic_sku_create.py in Unit Test mode.
**- Note we only backport fixes to a release branch, not features!
Please also provide a reason for the backporting below.**
201911 - This feature will be used in 201911..
-->
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)