Skip to content

[reboot]: Support BMC graceful shutdown status and cause - #449

Open
william8545 wants to merge 2 commits into
sonic-net:masterfrom
william8545:bmc-graceful-shutdown-restart
Open

william8545 wants to merge 2 commits into
sonic-net:masterfrom
william8545:bmc-graceful-shutdown-restart

Conversation

@william8545

@william8545 william8545 commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

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 and the switch-host reboot -p changes. 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.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@liat-grozovik

Copy link
Copy Markdown
Collaborator

@judyjoseph kindly reminder to review

william8545 added a commit to william8545/sonic-platform-daemons that referenced this pull request Oct 15, 2026
#### 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>
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>
william8545 added a commit to william8545/sonic-utilities that referenced this pull request Oct 15, 2026
#### 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>
@william8545
william8545 force-pushed the bmc-graceful-shutdown-restart branch from 50875b8 to d73ec93 Compare October 15, 2026 22:37
william8545 added a commit to william8545/SONiC that referenced this pull request Oct 15, 2026
## 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>
@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Contributor

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

@judyjoseph judyjoseph left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One comment on how this timeout relates to the BMC timeout.

Comment thread host_modules/reboot.py
"platform.json data is unavailable or not a dictionary"
)

value = data.get("switch_host_halt_services_timeout")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This value bounds the residual completion check after reboot -p exits. It is not independent of the BMC side: the BMC graceful_shutdown_timeout (default 120 s, changeable by CLI on the BMC) has to exceed the pre-shutdown time plus this value, and this value plus the BMC power-off verification has to fit inside the 180 s watchdog armed by scripts/reboot. Nothing here, or on the BMC, checks that relationship at run time. A BMC timeout set too low makes every shutdown end forced with reason deadline even when the host succeeded.

Suggestion: add a comment here stating the constraint, and consider logging the effective value when the check starts so a deadline on the BMC side can be correlated with it. Also, since this is only settable through platform.json, please confirm who is expected to size it per platform (the HLD currently leaves that open).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. The docstring states the relationship: this value bounds only the check after reboot -p returns, and the BMC timeout has to outlast request, pre-shutdown, this value and polling for a failed check to keep its reason. The check logs the effective value when it starts, and a rejected platform.json value is logged instead of silently replaced. Nothing checks the pair at run time.

Address a review comment on switch_host_halt_services_timeout: state in
the reader how the value relates to the BMC graceful_shutdown_timeout and
the watchdog, log the effective value when the completion check starts,
and log a platform.json value that is rejected instead of ignoring it
silently. Add unit coverage for both log paths.

Signed-off-by: William Tsai <willtsai@nvidia.com>
@mssonicbld

Copy link
Copy Markdown
Contributor

Removed label(s):

  • Request for msft-202608 Branch: The matching Tested for msft-202608 Branch label is missing. This requirement also applies to manually added request labels.

This PR requests backports for branch(es): 202608, but is missing required test information.

Update the PR description, not a PR comment:

  1. List each requested branch under Backport.
  2. After testing on each branch, tick its checkbox (- [x]) under Tested branch. An unchecked box (- [ ]) does not count.
  3. Under Test result, start a separate entry with each branch number, followed by its test evidence on the same line or the following lines, such as a test-run link or a summary with logs. A branch number alone, placeholders, and results for another branch do not count.

Example format for the branches above (replace every placeholder with your actual test evidence; only check branches you have tested):

### Backport
202608

### Tested branch
- [x] 202608

### Test result
202608: <link to test results or summary with logs>

Keep this information outside HTML comments (<!-- ... -->) so the app can read it.
After you save the PR description, the app checks it again and adds the matching Tested for <branch> Branch labels first, then the Request for <branch> Branch labels. Request labels remain blocked until their matching tested labels are present.

---Powered by SONiC BuildBot

@mssonicbld

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants