Repository navigation
[bmcctld]: Add graceful switch-host shutdown and restart - #923
william8545 wants to merge 1 commit into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
1 similar comment
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
@judyjoseph kindly reminder to review |
#### Description Implement BMC-side graceful shutdown and restart for the switch host. - Send authenticated gNOI HALT and match RebootStatus to the running request before power removal. - Serialize power actions: an identical action joins the in-flight request before any priority comparison, a higher-priority conflicting action preempts it, and other conflicting requests get BUSY responses. - Record confirmed outcomes without replaying interrupted operations. A repeated shutdown is skipped only after a recorded power-off, an operation cancelled by daemon stop is recorded as ABANDONED, and the startup power-on is dropped when a power-removing request was admitted first. - Read optional CA, client-certificate and private-key paths from CONFIG_DB `BMC_GNOI|certs`. - Default the graceful timeout to 120 seconds while preserving configured values, including 0 for forced-only operation. STATE_DB `HOST_STATE|switch-host` gains `op_request_id`, `op_trigger`, `op_result` and `op_reason`. Rack-manager command rows gain `request_id` for correlation. #### Motivation and Context Allow the host to stop services before BMC power removal while keeping critical-event handling and forced power operations available. #### How Has This Been Tested? - `git diff --check` passed. - Added or updated unit coverage for gNOI failures, request correlation, timeouts, certificate paths, preemption, duplicate actions, power failures and daemon cleanup. - The unit suite runs in Azure CI on this PR. A live-patch run of this daemon on a BMC and switch-host pair exercised graceful shutdown end to end, a critical event joining an in-flight graceful shutdown, startup with a pending power-off command and daemon stop during the handshake. No image build was run. #### Additional Information (Optional) This PR contains only the daemon-side changes. End-to-end graceful completion also needs [the switch-host `reboot -p` changes](sonic-net/sonic-utilities#4906), [the host-services HALT changes](sonic-net/sonic-host-services#449) and deployment-provisioned mTLS credentials. The YANG model for the optional `BMC_GNOI|certs` table is in [sonic-buildimage#30103](sonic-net/sonic-buildimage#30103). Each operation runs on its own worker thread, with at most one worker alive at a time, so the daemon keeps processing events and critical actions during the graceful wait; daemon stop joins the worker for at most 5 seconds. Each thread uses its own database connections. The rack-manager telemetry handler added in sonic-net#880 runs on the event thread's subscription list. A request that conflicts with the in-flight operation is refused as BUSY and is not queued or retried. The refusal is reported in the rack-manager command row and the event log, while `HOST_STATE|switch-host` keeps the record of the in-flight operation. CLI admin-status writes are deduplicated per value, so an `admin_status=up` written while a shutdown is in flight is not acted on: it is a no-op while the host is still online and is refused as BUSY once it is off. To power on afterwards, run `config chassis modules shutdown SWITCH-HOST` and then `startup` again. Signed-off-by: William Tsai <willtsai@nvidia.com>
1e86891 to
2b37a5c
Compare
#### What I did Add switch-host support to `reboot -p` and show BMC power-operation results. #### How I did it - Preserve platform pre-check and next-image verification failure exit codes on every path that runs the pre-checks. A `verify-next-image` failure previously went unnoticed because a `local` assignment masked its status and the reboot proceeded; `reboot` now exits 21 on a broken next image as originally intended. - Gate stricter teardown checks on switch-host identity: a failed `syncd` shutdown request, a still-running `pmon`, or a failed `sync` make `reboot -p` fail so the BMC falls back to forced power-off. - Skip the platform `pre_reboot_hook` and the platform firmware auto-update handler on the switch-host pre-shutdown path. The hook prepares the device for the coming reboot, for example by programming pending firmware, and firmware programming must not start when BMC power removal follows. Ordinary reboot and the DPU path run both unchanged. - Arm the watchdog explicitly for 180 seconds and verify the readback. A platform that cannot arm at least 180 seconds fails pre-shutdown and the BMC falls back to forced power-off. - Show `Result` and `Request-Id` from host operation state and document the shutdown-timeout policy. - Add unit coverage for failure paths, identity selection, CLI output, and unchanged DPU behavior. This is the host/CLI portion of BMC graceful shutdown/restart. It complements [the BMC daemon changes](sonic-net/sonic-platform-daemons#923) and [the host-services changes](sonic-net/sonic-host-services#449). Deployment must provision the gNOI management credentials; this PR does not provision them. #### How to verify it Run `pytest tests/reboot_pre_shutdown_test.py tests/reboot_smartswitch_helper_test.py tests/chassis_modules_test.py tests/python_isolated_mode_test.py`. `git diff --check` passed. The listed unit suites pass locally; Azure CI runs them on this PR. A live-patch run on a BMC and switch-host pair exercised the switch-host pre-shutdown path end to end, including the watchdog recovery after the BMC daemon stopped mid-handshake. No image build was run. #### Previous command output (if the output of a command-line utility has changed) `show chassis modules status` does not include `Result` or `Request-Id`. #### New command output (if the output of a command-line utility has changed) On the BMC, `show chassis modules status` appends `Result` and `Request-Id`. Missing switch-host values display `-`; other module rows display `N/A`. Signed-off-by: William Tsai <willtsai@nvidia.com>
Preserve request messages in terminal reboot status so a BMC can correlate a HALT result with its request without changing the gNOI response schema. Select the HALT completion timeout by device identity. On switch-hosts, require both the reboot process and PMON to stop, as before, and report failure if completion cannot be verified. Keep legacy DPU completion behavior. Persist the graceful BMC shutdown cause before reporting success and retain it when the next boot reports a BMC power-down cause. Add unit coverage for status correlation, timeout selection, completion errors, and reboot-cause handling. Notes for review: - On a switch-host the HALT completion check is tri-state. Completion is confirmed only from the real `pmon` container state and reboot-process state. A Docker or process-table error during the check is reported as `Halt completion check could not be answered` and the HALT fails, instead of an errored check reading as "stopped" and publishing success. DPU behavior is unchanged. - `switch_host_halt_services_timeout` in platform.json must be a JSON integer greater than 0; any other value falls back to 60 seconds. The legacy DPU reader keeps its value handling. - The reboot cause is written only when the HALT message carries exactly one `[bmc-req:<uuid>]` tag, matching the BMC's own acceptance rule, so a cause is never attributed to a request the BMC would not recognize. This is the host-service portion of BMC graceful shutdown/restart, complementing [the BMC daemon changes](sonic-net/sonic-platform-daemons#923) and [the switch-host `reboot -p` changes](sonic-net/sonic-utilities#4906). It does not provision management credentials or modify the D-Bus interface. Verification: `git diff --check` passed. The listed unit suites pass locally; Azure CI runs them on this PR. Image builds and hardware tests were not run. Run `pytest tests/host_modules/reboot_test.py tests/determine-reboot-cause_test.py` for the affected unit suites. Signed-off-by: William Tsai <willtsai@nvidia.com>
#### Why I did it The BMC daemon in [sonic-platform-daemons#923](sonic-net/sonic-platform-daemons#923) performs BMC-controlled graceful shutdown and restart of the switch host ([HLD](sonic-net/SONiC#2500)). It reaches the host over gNOI with mTLS and reads its client credential paths from the table `BMC_GNOI|certs` in the BMC's CONFIG_DB. That table has no YANG model yet, so it cannot be validated like the existing `GNMI|certs` server paths. ##### Work item tracking - Microsoft ADO **(number only)**: N/A #### How I did it - Add `sonic-bmc-gnoi.yang`: one optional singleton row `BMC_GNOI|certs` with the leaves `ca_crt`, `client_crt` and `client_key`. Each is an absolute path, visible inside the BMC's PMON container. The model defines no defaults, so an absent field does not materialize a row; the daemon falls back to `/etc/sonic/bmc-link/{ca.crt,client.crt,client.key}` for absent fields only. - Register the model in `setup.py` and add a `BMC_GNOI` entry to `sample_config_db.json` so the model is covered by the existing sample-config validation. - Add `yang_model_pytests/test_bmc_gnoi.py`: accepted path combinations, rejection of empty, bare-root, relative, templated and whitespace-containing paths, rejection of unknown rows or fields, and no leaf default. - Document the table in `Configuration.md`. The image does not seed this table. Deployment installs the files and configures any non-default paths. #### How to verify it ``` cd src/sonic-yang-models pytest tests/yang_model_pytests/test_bmc_gnoi.py tests/yang_model_pytests/test_configdb_layout.py ``` The pytests need the `python3-libyang` this repository builds (libyang-python 3.1.0 with the `src/libyang3-py3` patches), for example inside the sonic-slave container. The sonic-yang-mgmt validation test (`test_validate_yang_models`) loads every model together with `sample_config_db.json` and runs as part of the sonic-yang-mgmt build. #### Which release branch to backport (provide reason below if selected) - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [ ] 202511 - [ ] 202512 - [ ] 202605 - [ ] 202608 Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): N/A Failure type: other (feature) #### Tested branch - [x] master - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [ ] 202511 - [ ] 202512 - [ ] 202605 - [ ] 202608 - [ ] N/A #### Test result - master: `tests/yang_model_pytests/` passed (1015 tests) and the sonic-yang-mgmt `test_validate_yang_models` and `test_xlate_rev_xlate` passed with this branch's models and sample config, run in a Debian bookworm container with libyang 3.12.2 and the libyang-python 3.1.0 built from `src/libyang3-py3`. `setup.py build_py` YANG completeness validation passed and `pyang -Vf tree` over all models reports nothing for the new model. No image build or hardware test was run for this YANG-only change; Azure CI runs the model tests in the sonic-yang-models and sonic-yang-mgmt builds. #### Description for the changelog Add the `BMC_GNOI` YANG model for the BMC daemon's gNOI client certificate paths. #### Link to config_db schema for YANG module changes https://github.com/william8545/sonic-buildimage/blob/bmc-graceful-shutdown-restart/src/sonic-yang-models/doc/Configuration.md#bmc_gnoi Signed-off-by: William Tsai <willtsai@nvidia.com>
## Summary Align the BMC graceful shutdown/restart HLD merged in sonic-net#2500 with the implementation. Only the existing HLD is changed; its section structure and main flows are retained. ## Differences from the merged HLD | Area | Updated design | | --- | --- | | Eligibility | Use existing BMC/switch-host identity. Remove the `bmc_pairing` opt-in and old/mixed-image compatibility promise. | | Provisioning | Deployment owns credentials, gNMI authorization and CACL. BMC client paths use optional `BMC_GNOI\|certs`; host server paths use existing `GNMI\|certs`. No image-side DB seeding. | | Timing | Default BMC timeout 120 s, explicit 0 preserved, no CLI upper bound. Host timeout defaults to 60 s without DPU fallback; watchdog 180 s; restart pause 10 s. | | Host safeguards | Document watchdog readback, the skipped platform pre-reboot hook and firmware auto-update handler, the checked host-partition flush, and immediate propagation of pre-check failures, including the narrow shared-path fixes. | | Operation handling | Align same-action JOIN ahead of priority displacement, preemption ownership, the startup power-on yielding to an admitted power removal, the repeated-shutdown skip after a recorded power-off, ABANDONED on daemon stop and no automatic power-call retry. Distinguish platform `ONLINE` from OS/management readiness. | The tagged HALT/RebootStatus protocol, vendor teardown branches and deployment qualification requirements remain. This PR does not claim production security approval or introduce runtime changes. ## Validation - Compared with the implementation proposed in sonic-net/sonic-platform-daemons#923, sonic-net/sonic-host-services#449, sonic-net/sonic-utilities#4906 and sonic-net/sonic-buildimage#30103. - Checked document structure, link targets and whitespace. - Documentation only: no build or device test. Signed-off-by: William Tsai <willtsai@nvidia.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
This PR has backport request label(s) for branch(es): msft-202608, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202608: <test result>) in the Test result section as well in your PR description. ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
8 similar comments
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
|
Hi @sonic-net/sonic-platform-daemons-maintainer, this approved PR has workflow run(s) waiting for approval. Please help review. Thanks! ---Powered by SONiC BuildBot
|
Description
Implement BMC-side graceful shutdown and restart for the switch host.
BMC_GNOI|certs.STATE_DB
HOST_STATE|switch-hostgainsop_request_id,op_trigger,op_resultandop_reason. Rack-manager command rows gainrequest_idfor correlation.Motivation and Context
Allow the host to stop services before BMC power removal while keeping critical-event handling and forced power operations available.
How Has This Been Tested?
git diff --checkpassed.Additional Information (Optional)
This PR contains only the daemon-side changes. End-to-end graceful completion also needs the switch-host
reboot -pchanges, the host-services HALT changes and deployment-provisioned mTLS credentials. The YANG model for the optionalBMC_GNOI|certstable is in sonic-buildimage#30103.Each operation runs on its own worker thread, with at most one worker alive at a time, so the daemon keeps processing events and critical actions during the graceful wait; daemon stop joins the worker for at most 5 seconds. Each thread uses its own database connections. The rack-manager telemetry handler added in #880 runs on the event thread's subscription list.
A request that conflicts with the in-flight operation is refused as BUSY and is not queued or retried. The refusal is reported in the rack-manager command row and the event log, while
HOST_STATE|switch-hostkeeps the record of the in-flight operation. CLI admin-status writes are deduplicated per value, so anadmin_status=upwritten while a shutdown is in flight is not acted on: it is a no-op while the host is still online and is refused as BUSY once it is off. To power on afterwards, runconfig chassis modules shutdown SWITCH-HOSTand thenstartupagain.