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

[sonic-platform-base] Introduce APIs for modular chassis support #124

Merged
merged 10 commits into from
Nov 10, 2020

Conversation

mprabhu-nokia
Copy link
Contributor

@mprabhu-nokia mprabhu-nokia commented Oct 1, 2020

sonic-platform-base: Changes to enhance module_base.py and chassis_base.py for modular chassis

HLD: sonic-net/SONiC#646

- What I did
Enhance ModuleBase with new APIs to repesent pluggable cards in a voq-chassis.
Enhance ChassisBase with new APIs

- How I did it
Add helper APIs in module_base.py

  • get_name()
  • get_description()
  • get_slot()
  • get_type()
  • get_status()
  • reboot()
  • set_admin_state()

Add helper APIs in chassis_base.py

  • get_module_index()
  • get_supervisor_slot()
  • get_my_slot()

- How I verified it

Copy link
Contributor Author

@mprabhu-nokia mprabhu-nokia left a comment

Choose a reason for hiding this comment

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

All comments are addressed.
get_change_event() - Will be implemented in an incremental PR. We will make a note of it.
get_name() - Standard name format as updated in the comments
get_description() - PID of the card that can be updated when the information is available.

@mprabhu-nokia
Copy link
Contributor Author

There are different variants/use-cases of reboot such as the whole-card or CPU-only based on fault(s)/error(s)/user-specification etc.
This should be facilitated in the args list passed to reboot_card() API. Could be enum list or some such

Discuss additional options for reboot options - linux, cpu complex or entire card.

Introducing CardBase class to repesent pluggable cards in
a voq-chassis.

Add helper APIs in chassis_base.py for num_cards(),
get_card_list(), get_controlcard_instance() and get_my_instance()
Addressing review-comments
Fixes for other review comments
* Add more decscriptive headers for APIs
sonic_platform_base/chassis_base.py Outdated Show resolved Hide resolved
sonic_platform_base/chassis_base.py Outdated Show resolved Hide resolved
sonic_platform_base/module_base.py Show resolved Hide resolved
sonic_platform_base/module_base.py Outdated Show resolved Hide resolved
@shyam77git
Copy link

There are different variants/use-cases of reboot such as the whole-card or CPU-only based on fault(s)/error(s)/user-specification etc.
This should be facilitated in the args list passed to reboot_card() API. Could be enum list or some such

Discuss additional options for reboot options - linux, cpu complex or entire card.

Can this be facilitated to reboot() api as part of current code/PR?

sonic_platform_base/module_base.py Outdated Show resolved Hide resolved
sonic_platform_base/chassis_base.py Outdated Show resolved Hide resolved
In module_base.py, introducing a reboot_type option in reboot() API
as per review comments.
sonic_platform_base/module_base.py Outdated Show resolved Hide resolved
@jleveque
Copy link
Contributor

@mprabhu-nokia: Can you please update the PR title and description since they no longer reflect the changes here?

@mprabhu-nokia mprabhu-nokia changed the title Introduce card_base for modular chassis Introduce platform-common APIs for modular chassis support Oct 28, 2020
@mprabhu-nokia
Copy link
Contributor Author

@mprabhu-nokia: Can you please update the PR title and description since they no longer reflect the changes here?

Agree. Fixed both to reflect latest changes.

@jleveque
Copy link
Contributor

@Staphylo: Can you please review this, also?

@Staphylo
Copy link
Contributor

LGTM

Copy link
Contributor

@Staphylo Staphylo left a comment

Choose a reason for hiding this comment

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

LGTM

sonic_platform_base/chassis_base.py Outdated Show resolved Hide resolved
sonic_platform_base/chassis_base.py Outdated Show resolved Hide resolved
sonic_platform_base/chassis_base.py Outdated Show resolved Hide resolved
sonic_platform_base/module_base.py Outdated Show resolved Hide resolved
sonic_platform_base/module_base.py Outdated Show resolved Hide resolved
sonic_platform_base/module_base.py Outdated Show resolved Hide resolved
sonic_platform_base/module_base.py Outdated Show resolved Hide resolved
Changing get_status() to get_oper_status() since the signature
is different from the device_base.get_status() which returns a bool
@jleveque
Copy link
Contributor

jleveque commented Nov 4, 2020

@mprabhu-nokia: Once this PR merges, can you please also add regression tests for these new API methods in the sonic-mgmt repo:
https://github.com/Azure/sonic-mgmt/blob/master/tests/platform_tests/api/test_module.py?

@mprabhu-nokia
Copy link
Contributor Author

@mprabhu-nokia: Once this PR merges, can you please also add regression tests for these new API methods in the sonic-mgmt repo:
https://github.com/Azure/sonic-mgmt/blob/master/tests/platform_tests/api/test_module.py?

Yes, will do.

@jleveque
Copy link
Contributor

jleveque commented Nov 5, 2020

@lguohan: Since you had outstanding comments, can you please review again before merge?

@jleveque jleveque changed the title Introduce platform-common APIs for modular chassis support [sonic-platform-base] Introduce APIs for modular chassis support Nov 10, 2020
@jleveque jleveque merged commit b37f156 into sonic-net:master Nov 10, 2020
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.

6 participants