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

Include DPU module to sonic-chassis-module.yang #20445

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

rameshraghupathy
Copy link
Contributor

Why I did it

The modules have been extended to DPUx to support SmartSwitch. As part of the SmartSwitch development module "DPUx" needs to be supported in every section of the code that referring to modules.

Work item tracking
  • Microsoft ADO (number only):

How I did it

Updated "src/sonic-yang-models/yang-models/sonic-chassis-module.yang" file with "|DPU[0-9]" dpu module name.

How to verify it

  1. Update "src/sonic-yang-models/tests/yang_model_tests/tests_config/chassis_module.json" to include DPUx modules and update /src/sonic-yang-models/tests/yang_model_tests/tests/chassis_module.json to verify DPU modules
  2. Update "src/sonic-yang-models/tests/files/sample_config_db.json" to include DPUx modules and verify using src/sonic-yang-mgmt/tests/libyang-python-tests/test_sonic_yang.py

Which release branch to backport (provide reason below if selected)

No need to brackport

Tested branch (Please provide the tested image version)

master

Description for the changelog

Added DPU modules to sonic-chassis-module.yang

Link to config_db schema for YANG module changes

Updated https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/doc/Configuration.md

A picture of a cute animal (not mandatory but encouraged)

prgeor
prgeor previously approved these changes Oct 9, 2024
Copy link
Contributor

@prgeor prgeor left a comment

Choose a reason for hiding this comment

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

@rameshraghupathy why PR is in draft?

@prgeor
Copy link
Contributor

prgeor commented Nov 25, 2024

@rameshraghupathy can you fix the build error

@rameshraghupathy rameshraghupathy marked this pull request as ready for review November 25, 2024 15:59
@rameshraghupathy
Copy link
Contributor Author

@rameshraghupathy why PR is in drat?
@prgeor Not sure, may be it is not synced! Synced and changed state.

@rameshraghupathy
Copy link
Contributor Author

@rameshraghupathy can you fix the build error

@prgeor It was passing before. May be some new conflict has arrived? Synced again and let us see.

@@ -54,6 +54,7 @@ $(DOCKER_PLATFORM_MONITOR)_CONTAINER_NAME = pmon
$(DOCKER_PLATFORM_MONITOR)_RUN_OPT += --privileged -t
$(DOCKER_PLATFORM_MONITOR)_RUN_OPT += -v /etc/sonic:/etc/sonic:ro
$(DOCKER_PLATFORM_MONITOR)_RUN_OPT += -v /etc/timezone:/etc/timezone:ro
$(DOCKER_PLATFORM_MONITOR)_RUN_OPT += -v /host/reboot-cause:/host/reboot-cause:rw
Copy link
Contributor

Choose a reason for hiding this comment

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

@rameshraghupathy why PMON need access to reboot-cause dir?

Copy link
Contributor

Choose a reason for hiding this comment

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

@rameshraghupathy this change is not related to YANG model change. Can you remove?

Copy link
Contributor Author

@rameshraghupathy rameshraghupathy Nov 25, 2024

Choose a reason for hiding this comment

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

@prgeor
PMON needs to persist dpu module's reboot-cause history on the host file system.
Created #20916 just for this

YANG
Now that we have #20916 for reboot-cause the yang model PR #20445 can be just by itself.
#20445 this is not affecting any PMON operation, can look at the failure later.

@vvolam
Copy link
Contributor

vvolam commented Nov 26, 2024

@rameshraghupathy could you check build errors?

rameshraghupathy added a commit to rameshraghupathy/sonic-buildimage that referenced this pull request Nov 27, 2024
{
"name": "DPU1",
"admin_status": "down"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

duplicated configuration

Copy link

@hdwhdw hdwhdw left a comment

Choose a reason for hiding this comment

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

LGTM. Just one nit.

It currently allows user to administratively bring down a line-card or fabric-card
CHASSIS_MODULE table holds the list and configuration of linecard, DPU modules and fabric modules in a SONiC chassis.
It currently allows user to administratively bring down a line-card or DPU or fabric-card
A smartswitch chassis will not have LINE-CARD and FABRIC-CARD. It will only have DPUs as shown
Copy link

Choose a reason for hiding this comment

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

Nits: use the code formatted LINE-CARD instead of plain LINE-CARD so readers know you are referring to a json field. Same for the others.

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.

5 participants