[sonic-cfggen] Allow cfggen to work on system without ports #7999
Conversation
Signed-off-by: liora <[email protected]>
This reverts commit 75ff366.
liorghub
force-pushed
the
zero_ports_2
branch
3 times, most recently
from
237a6ec to
38e39ac
Compare
Signed-off-by: liora <[email protected]>
| @@ -317,9 +317,6 @@ def main(): | |||
| if args.port_config is None: | |||
| args.port_config = device_info.get_path_to_port_config_file(hwsku) | |||
| (ports, _, _) = get_port_config(hwsku, platform, args.port_config, asic_id) | |||
| if not ports: | |||
There was a problem hiding this comment.
Still applicable. Could you help add a positive testcase?
There was a problem hiding this comment.
@qiluo-msft
While I implemented the unittest I noticed a bug.
I handled the flow of reading the ports from port_config.ini but not the flow of reading the ports from platrfom.json+hwsku.json. Now it is fixed and I added unittest as well.
Appreciate your review.
| @@ -317,9 +317,6 @@ def main(): | |||
| if args.port_config is None: | |||
| args.port_config = device_info.get_path_to_port_config_file(hwsku) | |||
| (ports, _, _) = get_port_config(hwsku, platform, args.port_config, asic_id) | |||
| if not ports: | |||
| print('Failed to get port config', file=sys.stderr) | |||
| sys.exit(1) | |||
| deep_update(data, {'PORT': ports}) | |||
There was a problem hiding this comment.
@qiluo-msft
Thanks, I changed the code according to your comment, I will explain.
The code I deleted, verified both conditions:
- ports is not None.
- ports is not empty
Since now (in modular chassis without linecards), ports dictionary can be empty, we just need to verify it is not None.
Signed-off-by: liora <[email protected]>
Signed-off-by: liora <[email protected]>
Signed-off-by: liora <[email protected]>
|
The compilation is failing since test "test_buffers_multi_asic_template " is failing with the error below. Failed test log: Passing test log from my env: test_multinpu_cfggen.py::TestMultiNpuCfgGen::test_buffers_multi_asic_template PASSED [100%] ================================================================================================= 1 passed, 35 deselected in 0.18 seconds ================================================================================================= |
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/AzurePipelines run |
|
Commenter does not have sufficient privileges for PR 7999 in repo Azure/sonic-buildimage |
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@qiluo-msft can you please approve this commit? |
| @@ -85,3 +91,10 @@ def test_platform_json_all_ethernet_interfaces(self): | |||
| output_dict = ast.literal_eval(output.strip()) | |||
| expected = ast.literal_eval(json.dumps(fh_data)) | |||
| self.assertDictEqual(output_dict, expected) | |||
|
|
|||
| @mock.patch('portconfig.readJson', mock.MagicMock(return_value={INTF_KEY:{}})) | |||
There was a problem hiding this comment.
I am not sure what this file is? Can you please elaborate? Additionally, can you confirm if the format readJson is correct?
Signed-off-by: liora [email protected]
Why I did it
Allow cfggen to work on system without ports in platform.json or in port_config.ini
How I did it
Add json write of PORT section only if the dictionary that contains the ports is not empty.
How to verify it
sonic-cfggen -k ACS-MSN3700 -H -j /etc/sonic/init_cfg.json --print-data
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)