Skip to content

Set key "asic0" in single-asic scenerio to keep consistent with multi-asic in function core_dump_and_config_check.#8884

Merged
wangxin merged 1 commit intosonic-net:masterfrom
yutongzhang-microsoft:yutongzhang/set_single_asic_key
Jul 10, 2023
Merged

Set key "asic0" in single-asic scenerio to keep consistent with multi-asic in function core_dump_and_config_check.#8884
wangxin merged 1 commit intosonic-net:masterfrom
yutongzhang-microsoft:yutongzhang/set_single_asic_key

Conversation

@yutongzhang-microsoft
Copy link
Contributor

@yutongzhang-microsoft yutongzhang-microsoft commented Jul 10, 2023

Description of PR

In PR (#6527), it enhanced function core_dump_and_config_check to be multi-asic aware. But in single-asic scenerio, it simply set the key None, which does not make scene. In this PR, I reset the key "asic0" in single-asic scenerio to keep consistent with the key value of multi-asic scenerio.

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

In PR (#6527), it enhanced function core_dump_and_config_check to be multi-asic aware. But in single-asic scenerio, it simply set the key None, which does not make scene. In this PR, I reset the key "asic0" in single-asic scenerio to keep consistent with the key value of multi-asic scenerio.

How did you do it?

Change the key in single-asic scenerio from None to asic0.

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@wangxin wangxin merged commit 1cf3554 into sonic-net:master Jul 10, 2023
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Jul 10, 2023
…ic. (sonic-net#8884)

What is the motivation for this PR?
In PR (sonic-net#6527), it enhanced function core_dump_and_config_check to be multi-asic aware. But in single-asic scenerio, it simply set the key None, which does not make scene. In this PR, I reset the key "asic0" in single-asic scenerio to keep consistent with the key value of multi-asic scenerio.

How did you do it?
Change the key in single-asic scenerio from None to asic0.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202205: #8885

@mssonicbld
Copy link
Collaborator

@yutongzhang-microsoft PR conflicts with 202012 branch

@yutongzhang-microsoft yutongzhang-microsoft deleted the yutongzhang/set_single_asic_key branch July 10, 2023 05:35
mssonicbld pushed a commit that referenced this pull request Jul 10, 2023
…ic. (#8884)

What is the motivation for this PR?
In PR (#6527), it enhanced function core_dump_and_config_check to be multi-asic aware. But in single-asic scenerio, it simply set the key None, which does not make scene. In this PR, I reset the key "asic0" in single-asic scenerio to keep consistent with the key value of multi-asic scenerio.

How did you do it?
Change the key in single-asic scenerio from None to asic0.
wangxin pushed a commit that referenced this pull request Jul 10, 2023
…ic. (#8886)

What is the motivation for this PR?
There is a cherry pick conflict in 202012 branch of PR (#8884).

How did you do it?

How did you verify/test it?
I tested using 202012 branch on 202012 image, and no error occurs.
@arlakshm
Copy link
Contributor

Hi @yutongzhang-microsoft, this PR is causing mulitple test failures on multi-asic platform and chassis. In this PR the config_db of host is getting overwritten with config_db of asic0. Please check and see if this can be fixed quickly

@rlhui
Copy link

rlhui commented Jul 26, 2023

tagging @wangxin and @yxieca as well, thanks.

@rlhui
Copy link

rlhui commented Jul 26, 2023

@yutongzhang-microsoft , single-asic may not mean we can use asic[0]

@wangxin
Copy link
Collaborator

wangxin commented Aug 2, 2023

@rlhui Asked @yutongzhang-microsoft to check.

@yutongzhang-microsoft
Copy link
Contributor Author

Hi, @arlakshm @rlhui I don't think these failures are caused by my PR. Variable duts_data[duthost.hostname]["pre_running_config"]["asic0"] is only used in our fixture collect_db_dump_on_duts and it is only a variable to restore the config getting from command sonic-cfggen -d --print-data.

And also, as @arlakshm said, mulitple test failures on multi-asic platform and chassis, but in my code, multi-asic scenrio will not run these codes, it will go into the code if node.is_multi_asic:, which is introduced by PR #6527.

BTW, can you revert my PR locally and test?

@yutongzhang-microsoft
Copy link
Contributor Author

Also, as (#9051) said, the issus is only found in 202205 branch. But my PR is included in master, 202012, 202205 branch. If my PR causes this issue, I think the issue will be found not only in 202205.

yutongzhang-microsoft added a commit that referenced this pull request Aug 16, 2023
yutongzhang-microsoft added a commit that referenced this pull request Aug 16, 2023
…multi-asic. (#8884)" (#9470)

Reverts #8884

In PR #8884, in multi-asic scenrio
```
node.copy(content=json.dumps(duts_data[node.hostname]["pre_running_config"]["asic0"], indent=4),
              dest='/etc/sonic/config_db.json', verbose=False)
```
This code will overwrite the global config with asic0 config, which will cause failure. So revert this PR first. And then we will set a more exactly key name.

Signed-off-by: Yutong Zhang <[email protected]>
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Aug 16, 2023
…multi-asic. (sonic-net#8884)" (sonic-net#9470)

Reverts sonic-net#8884

In PR sonic-net#8884, in multi-asic scenrio
```
node.copy(content=json.dumps(duts_data[node.hostname]["pre_running_config"]["asic0"], indent=4),
              dest='/etc/sonic/config_db.json', verbose=False)
```
This code will overwrite the global config with asic0 config, which will cause failure. So revert this PR first. And then we will set a more exactly key name.

Signed-off-by: Yutong Zhang <[email protected]>
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Aug 16, 2023
…multi-asic. (sonic-net#8884)" (sonic-net#9470)

Reverts sonic-net#8884

In PR sonic-net#8884, in multi-asic scenrio
```
node.copy(content=json.dumps(duts_data[node.hostname]["pre_running_config"]["asic0"], indent=4),
              dest='/etc/sonic/config_db.json', verbose=False)
```
This code will overwrite the global config with asic0 config, which will cause failure. So revert this PR first. And then we will set a more exactly key name.

Signed-off-by: Yutong Zhang <[email protected]>
mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Aug 16, 2023
…multi-asic. (sonic-net#8884)" (sonic-net#9470)

Reverts sonic-net#8884

In PR sonic-net#8884, in multi-asic scenrio
```
node.copy(content=json.dumps(duts_data[node.hostname]["pre_running_config"]["asic0"], indent=4),
              dest='/etc/sonic/config_db.json', verbose=False)
```
This code will overwrite the global config with asic0 config, which will cause failure. So revert this PR first. And then we will set a more exactly key name.

Signed-off-by: Yutong Zhang <[email protected]>
mssonicbld pushed a commit that referenced this pull request Aug 17, 2023
…multi-asic. (#8884)" (#9470)

Reverts #8884

In PR #8884, in multi-asic scenrio
```
node.copy(content=json.dumps(duts_data[node.hostname]["pre_running_config"]["asic0"], indent=4),
              dest='/etc/sonic/config_db.json', verbose=False)
```
This code will overwrite the global config with asic0 config, which will cause failure. So revert this PR first. And then we will set a more exactly key name.

Signed-off-by: Yutong Zhang <[email protected]>
mssonicbld pushed a commit that referenced this pull request Aug 17, 2023
…multi-asic. (#8884)" (#9470)

Reverts #8884

In PR #8884, in multi-asic scenrio
```
node.copy(content=json.dumps(duts_data[node.hostname]["pre_running_config"]["asic0"], indent=4),
              dest='/etc/sonic/config_db.json', verbose=False)
```
This code will overwrite the global config with asic0 config, which will cause failure. So revert this PR first. And then we will set a more exactly key name.

Signed-off-by: Yutong Zhang <[email protected]>
mssonicbld pushed a commit that referenced this pull request Aug 17, 2023
…multi-asic. (#8884)" (#9470)

Reverts #8884

In PR #8884, in multi-asic scenrio
```
node.copy(content=json.dumps(duts_data[node.hostname]["pre_running_config"]["asic0"], indent=4),
              dest='/etc/sonic/config_db.json', verbose=False)
```
This code will overwrite the global config with asic0 config, which will cause failure. So revert this PR first. And then we will set a more exactly key name.

Signed-off-by: Yutong Zhang <[email protected]>
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
…ic. (sonic-net#8884)

What is the motivation for this PR?
In PR (sonic-net#6527), it enhanced function core_dump_and_config_check to be multi-asic aware. But in single-asic scenerio, it simply set the key None, which does not make scene. In this PR, I reset the key "asic0" in single-asic scenerio to keep consistent with the key value of multi-asic scenerio.

How did you do it?
Change the key in single-asic scenerio from None to asic0.
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
…multi-asic. (sonic-net#8884)" (sonic-net#9470)

Reverts sonic-net#8884

In PR sonic-net#8884, in multi-asic scenrio
```
node.copy(content=json.dumps(duts_data[node.hostname]["pre_running_config"]["asic0"], indent=4),
              dest='/etc/sonic/config_db.json', verbose=False)
```
This code will overwrite the global config with asic0 config, which will cause failure. So revert this PR first. And then we will set a more exactly key name.

Signed-off-by: Yutong Zhang <[email protected]>
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.

5 participants