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

Change to use asic_instance() API in all test cases #3118

Merged
merged 10 commits into from
Mar 10, 2021

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Mar 9, 2021

Initially we had two set of API doing same thing get_asic() and asic_instance().
As part of https://github.com/Azure/sonic-mgmt/pull/3070/files#diff-ac7113ac4afb15af99807b123d079536d87e66389eab665362b294e2a760fa24L2382
change was made get_asic() to return sonichost if asic Index is none. (this is needed if we want to run something on host for multi-asic platfroms). This change cause some of platform test to fail on single-asic platfroms since they were calling API's present in Asic host but not in Sonic host.

With this PR made following changes:

  • Use asic_instance() to always return ASIC object

  • Rename get_asic() to get_asic_or_sonic_host() which will return sonic host if asic_index is None else return asic host object. This API is useful if we want to execute/run something on host as well on asic in multi-asic platforms

  • Updated all the testcases to use correct set of API's.

Fix the issue
#3103

How I verify:
On Single Asic no exception is found.

@abdosi abdosi requested a review from a team as a code owner March 9, 2021 17:29
@abdosi abdosi requested a review from judyjoseph March 9, 2021 17:29
@abdosi abdosi linked an issue Mar 9, 2021 that may be closed by this pull request
Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi
Copy link
Contributor Author

abdosi commented Mar 10, 2021

/Azurepipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@abdosi
Copy link
Contributor Author

abdosi commented Mar 10, 2021

/Azurepipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi abdosi requested a review from jleveque as a code owner March 10, 2021 18:08
Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi abdosi changed the title Added get_docker_cmd API for sonichost Change to use asic_instance() API in all test cases Mar 10, 2021
abdosi added 2 commits March 10, 2021 11:56
Signed-off-by: Abhishek Dosi <[email protected]>
Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi abdosi requested review from smaheshm, wangxin and arlakshm March 10, 2021 20:00
Copy link
Contributor

@smaheshm smaheshm left a comment

Choose a reason for hiding this comment

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

looks good

@abdosi abdosi merged commit 57b0b83 into sonic-net:master Mar 10, 2021
@abdosi abdosi deleted the asichost_fix branch March 10, 2021 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

function get_asic() returns wrong object
4 participants