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

[config]Support multi-asic Golden Config override #2738

Merged
merged 7 commits into from
Apr 20, 2023

Conversation

wen587
Copy link
Contributor

@wen587 wen587 commented Mar 15, 2023

What I did

Support multi-asic Golden Config Override

How I did it

Add ConfigMgmt support for ASIC validation. Modify override config cli to support multi-asic.

How to verify it

Unit test:
tests/config_override_test.py::TestConfigOverrideMultiasic::test_macsec_override PASSED [ 8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_table_rm PASSED [ 8%]

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)

@wen587 wen587 requested review from qiluo-msft and judyjoseph March 22, 2023 07:28
@wen587 wen587 marked this pull request as ready for review March 22, 2023 07:29
@judyjoseph
Copy link
Contributor

Otherwise looks ok.
We need to validate this and add some test results too in Comments above

config/main.py Outdated
generate_sysinfo(ns_config_input, ns)
else:
ns_config_input = config_input
generate_sysinfo(ns_config_input)
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate_sysinfo

Is it better to fill missing mac/platform during override_config_db() ?

If the original ConfigDB already has mac/platform, we should keep them even we override other fields. #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the generated sysinfo should have less priority than existing one?
Though I think they should be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will submit another PR for sysinfo generation about override.

config/main.py Outdated
else:
mac = device_info.get_system_mac()
else:
mac = device_info.get_system_mac()
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove code dup in next PR?

judyjoseph
judyjoseph previously approved these changes Apr 13, 2023
Copy link
Contributor

@judyjoseph judyjoseph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked on a multi-asic device running 2205. Find the config ( eg: macsec_profile) getting added in host/asic namespaces

StormLiangMS pushed a commit that referenced this pull request Apr 26, 2023
#### What I did
Support multi-asic Golden Config Override
#### How I did it
Add ConfigMgmt support for ASIC validation. Modify override config cli to support multi-asic.
#### How to verify it
Unit test:
tests/config_override_test.py::TestConfigOverrideMultiasic::test_macsec_override PASSED [  8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_table_rm PASSED [  8%]
dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 1, 2023
Update sonic-utilities submodule pointer to include the following:
* 600377f7 [DPB]Fixing typo in config breakout output ([sonic-net#2802](sonic-net/sonic-utilities#2802))
* 8ae2424a [config]Support multi-asic  Golden Config override ([sonic-net#2738](sonic-net/sonic-utilities#2738))
* 79003ab2 [chassis]: remote cli commands infra for sonic chassis ([sonic-net#2701](sonic-net/sonic-utilities#2701))
* cbc55eeb [voq][chassis][generate_dump] [BCM] Dump only the relevant BCM commands for fabric cards ([sonic-net#2606](sonic-net/sonic-utilities#2606))
* 39c94b7e [GCU] Prohibit removal of PFC_WD POLL_INTERVAL field ([sonic-net#2545](sonic-net/sonic-utilities#2545))

Signed-off-by: dprital <[email protected]>
dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 1, 2023
Update sonic-utilities submodule pointer to include the following:
* 88ffb167 [config]config reload should generate sysinfo if missing ([sonic-net#2778](sonic-net/sonic-utilities#2778))
* 7443b9e5 [sonic-package-manager] support extension with multiple YANG modules ([sonic-net#2752](sonic-net/sonic-utilities#2752))
* 522c3a9e [sonic-package-manager] add support for multiple CLI plugin files ([sonic-net#2753](sonic-net/sonic-utilities#2753))
* b38fcfd1 [show][muxcable] fix  RC ([sonic-net#2812](sonic-net/sonic-utilities#2812))
* 7e24463f [chassis]: remote cli commands infra for sonic chassis ([sonic-net#2701](sonic-net/sonic-utilities#2701))
* bee593e4 [DPB]Fixing typo in config breakout output ([sonic-net#2802](sonic-net/sonic-utilities#2802))
* ada603c5 [config]Support multi-asic  Golden Config override ([sonic-net#2738](sonic-net/sonic-utilities#2738))
* 88a7daa8 [show][barefoot] replace shell=True ([sonic-net#2699](sonic-net/sonic-utilities#2699))
* 5e99edb5 [sonic_package_manager] replace shell=True ([sonic-net#2726](sonic-net/sonic-utilities#2726))
* b547bb45 [acl-loader] Only add default deny rule when table is L3 or L3V6 ([sonic-net#2796](sonic-net/sonic-utilities#2796))

Signed-off-by: dprital <[email protected]>
@wen587
Copy link
Contributor Author

wen587 commented May 3, 2023

Remove Chassis tag because there is bug in this PR and block subodule. Will create an updated PR after the revert.

dprital added a commit to dprital/sonic-utilities that referenced this pull request May 3, 2023
liat-grozovik pushed a commit that referenced this pull request May 3, 2023
wen587 pushed a commit to wen587/sonic-utilities that referenced this pull request May 4, 2023
wen587 added a commit to wen587/sonic-utilities that referenced this pull request May 4, 2023
wen587 added a commit that referenced this pull request May 4, 2023
wen587 added a commit to wen587/sonic-utilities that referenced this pull request May 4, 2023
#### What I did
Support multi-asic Golden Config Override
#### How I did it
Add ConfigMgmt support for ASIC validation. Modify override config cli to support multi-asic.
#### How to verify it
Unit test:
tests/config_override_test.py::TestConfigOverrideMultiasic::test_macsec_override PASSED [  8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_table_rm PASSED [  8%]
qiluo-msft pushed a commit that referenced this pull request May 11, 2023
ADO: 17746282

#### What I did
Support multi-asic Golden Config Override with fix based on #2738
#### How I did it
Add ConfigMgmt support for ASIC validation. Modify override config cli to support multi-asic.
#### How to verify it
Unit test:
```
tests/config_override_test.py::TestConfigOverrideMultiasic::test_macsec_override PASSED [  8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_table_rm PASSED [  8%]
```
wen587 added a commit to wen587/sonic-utilities that referenced this pull request Jun 5, 2023
…onic-net#2825)

ADO: 17746282

Support multi-asic Golden Config Override with fix based on sonic-net#2738
Add ConfigMgmt support for ASIC validation. Modify override config cli to support multi-asic.
Unit test:
```
tests/config_override_test.py::TestConfigOverrideMultiasic::test_macsec_override PASSED [  8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_table_rm PASSED [  8%]
```
yxieca pushed a commit that referenced this pull request Jun 6, 2023
…2825) (#2862)

ADO: 17746282

Support multi-asic Golden Config Override with fix based on #2738
Add ConfigMgmt support for ASIC validation. Modify override config cli to support multi-asic.
Unit test:
```
tests/config_override_test.py::TestConfigOverrideMultiasic::test_macsec_override PASSED [  8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_table_rm PASSED [  8%]
```
pdhruv-marvell pushed a commit to pdhruv-marvell/sonic-utilities that referenced this pull request Aug 23, 2023
…#2825)

ADO: 17746282

#### What I did
Support multi-asic Golden Config Override with fix based on sonic-net#2738
#### How I did it
Add ConfigMgmt support for ASIC validation. Modify override config cli to support multi-asic.
#### How to verify it
Unit test:
```
tests/config_override_test.py::TestConfigOverrideMultiasic::test_macsec_override PASSED [  8%]
tests/config_override_test.py::TestConfigOverrideMultiasic::test_device_metadata_table_rm PASSED [  8%]
```
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.

4 participants