Skip to content

[Multi-asic] Enhanced Feature Table configuration for multi-asic platforms#1152

Merged
abdosi merged 11 commits intosonic-net:masterfrom
abdosi:feature_fix
Dec 17, 2020
Merged

[Multi-asic] Enhanced Feature Table configuration for multi-asic platforms#1152
abdosi merged 11 commits intosonic-net:masterfrom
abdosi:feature_fix

Conversation

@abdosi
Copy link
Contributor

@abdosi abdosi commented Oct 9, 2020 •
edited
Loading

- What I did
Enhanced Feature Table configuration for multi-asic platforms
to programmed for all config db's.

- How I did

Following changes were done:-

  • Feature Table config command check all the DB's values are same else it will fail

  • Feature Table show command always display from Host/Global Config DB.

  • Also made sure some of other test cases (crm/bgp route) that import mock_*_asic to do using reload otherwise we can see failure.

- How to verify it

  • Unit test added specific for Multiasic.
  • Test case verfiy if DB's content are not same config command fails
  • Test case verify if if DB's content are same config and show command passes.

@abdosi abdosi requested a review from jleveque October 9, 2020 00:02
@jleveque
Copy link
Contributor

jleveque commented Oct 9, 2020

Retest this please

@lguohan
Copy link
Contributor

lguohan commented Oct 11, 2020 •
edited by abdosi
Loading

can you enhance the unit test to cover multi asic ones?

@lguohan Updated the PR with unit test cases.

@abdosi abdosi changed the base branch from 201911 to master December 15, 2020 18:55
@abdosi
Copy link
Contributor Author

abdosi commented Dec 15, 2020

@jleveque can you please review. I have updated PR with new set of changes.

@abdosi
Copy link
Contributor Author

abdosi commented Dec 15, 2020

FYI @judyjoseph and @gechiang
Please see the changes done in crm and bgp test for using reload() the module

jleveque
jleveque suggested changes Dec 15, 2020
View reviewed changes
@abdosi
Address Review Comments
5725794 
Signed-off-by: Abhishek Dosi <[email protected]>
jleveque
jleveque previously approved these changes Dec 15, 2020
View reviewed changes
judyjoseph
judyjoseph reviewed Dec 16, 2020
View reviewed changes
utilities_common/multi_asic.py Outdated
help='Display interfaces for specific namespace')
return parser

class MultiAsicDb(object):
Copy link
Contributor

@judyjoseph judyjoseph Dec 16, 2020

Choose a reason for hiding this comment

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

@abdosi do we really need this new class, I see that the Db object itself is enhanced to supports multi-asic ? https://github.com/Azure/sonic-utilities/blob/master/utilities_common/db.py#L7

Copy link
Contributor Author

@abdosi abdosi Dec 16, 2020

Choose a reason for hiding this comment

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

@judyjoseph as part of next PR i will be removing that file and will be using MultiAsicDb so that code remains consistent in multiasic.py

Copy link
Contributor Author

@abdosi abdosi Dec 16, 2020

Choose a reason for hiding this comment

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

@judyjoseph On second thought I will close my PR##1312 as I feel that will create conflict for other outstanding PR's and use existing Db class and update this PR accordingly.

Thanks for pointing out.

@abdosi
Copy link
Contributor Author

abdosi commented Dec 16, 2020

retest this please

judyjoseph
judyjoseph previously approved these changes Dec 16, 2020
View reviewed changes
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.

LGTM

jleveque
jleveque suggested changes Dec 16, 2020
View reviewed changes
@abdosi
Address Review Comments
a780a90 
Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi
Fix
322629d 
Signed-off-by: Abhishek Dosi <[email protected]>
@abdosi
Copy link
Contributor Author

abdosi commented Dec 17, 2020

retest this please

1 similar comment
@abdosi
Copy link
Contributor Author

abdosi commented Dec 17, 2020

retest this please

@abdosi abdosi merged commit 9419627 into sonic-net:master Dec 17, 2020
@abdosi abdosi deleted the feature_fix branch December 17, 2020 20:43
abdosi added a commit that referenced this pull request Dec 21, 2020
@abdosi
[Multi-asic] Enhanced Feature Table configuration for multi-asic plat…
fd3e017 
…forms (#1152)

Enhanced Feature Table configuration for multi-asic platforms
to programmed for all config db's.
anand-kumar-subramanian pushed a commit to anand-kumar-subramanian/sonic-utilities that referenced this pull request Mar 2, 2021
@abdosi @anand-kumar-subramanian
[Multi-asic] Enhanced Feature Table configuration for multi-asic plat…
fe0bd80 
…forms (sonic-net#1152)

Enhanced Feature Table configuration for multi-asic platforms
to programmed for all config db's.
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 28, 2022
@abdosi
[submodule update] sonic-utilities
c70b4cd 
fd3e017 (HEAD -> 201911, origin/201911) [Multi-asic] Enhanced Feature Table configuration for multi-asic platforms (sonic-net#1152)
12f03b1 Add FW dump with new SAI implementation (sonic-net#1298)

Signed-off-by: Abhishek Dosi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@judyjoseph judyjoseph judyjoseph left review comments

+1 more reviewer

@jleveque jleveque jleveque approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@abdosi @jleveque @lguohan @judyjoseph