Skip to content

Reconcile a deferred reboot already performed out-of-band - #69

Merged
luigilink merged 3 commits into
mainfrom
feat/reconcile-outofband-reboot-68
Oct 7, 2026
Merged

luigilink merged 3 commits into
mainfrom
feat/reconcile-outofband-reboot-68

Conversation

@luigilink

Copy link
Copy Markdown
Owner

Pull Request (PR) description

Fixes an unnecessary double reboot found during the v5.1.0-preview.5 live test (WFE1/WFE2).

When an automatic reboot is deferred (a CU install returned "reboot required" outside the Reboot.Schedule window), SPSUpdate writes a .pending marker and performs the restart on the next in-window run. If the server was rebooted out-of-band in the meantime — a manual restart, or a Windows Update reboot — the pending request was still honoured and the server was restarted again.

Fix

Before acting on a deferred pending request ($hasPending and no fresh -RebootRequired from an install this run), compare the server's LastBootUpTime with the .pending marker timestamp. If the server has booted after the request was recorded, the required restart already happened out-of-band: write the .done guard, clear the .pending marker, set the Reboot phase to Done ("Reboot already completed out-of-band"), log an informational event, and skip the restart. A reboot required by an install performed this run is unaffected (new binaries still need a restart).

This Pull Request (PR) fixes the following issues

Task list

  • Added a CHANGELOG entry (Unreleased / Added).
  • PSScriptAnalyzer 0 issues on src; Pester 296 passed / 0 failed / 1 skipped. The reboot path is Windows-only (uses Get-CimInstance Win32_OperatingSystem) and not unit-tested, consistent with the rest of that action.

Note: targets the 5.x line (main); to be cherry-picked to support/4.x.

When a reboot was deferred (install returned 17022 outside the Reboot.Schedule
window), a .pending marker is written and honoured on the next in-window run. If
the server was rebooted out-of-band in the meantime (manual restart or Windows
Update), SPSUpdate restarted it again unnecessarily.

Before acting on a deferred pending request (no fresh -RebootRequired this run),
compare the server's LastBootUpTime with the .pending marker timestamp: if the
server booted after the request was recorded, mark the Reboot phase Done
('Reboot already completed out-of-band'), clear the pending marker and skip the
restart. A reboot required by an install performed this run is unaffected.

Refs #68
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Pester Test Results

  1 files   46 suites   12s ⏱️
186 tests 186 ✅ 0 💤 0 ❌
297 runs  297 ✅ 0 💤 0 ❌

Results for commit 4702bcc.

♻️ This comment has been updated with latest results.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Reconciliation can suppress a fresh forced reboot after a successful installation returns exit code 0.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Addresses #68 by reconciling deferred automatic reboots already completed outside SPSUpdate, avoiding unnecessary restarts.

Changes:

  • Compares pending-request timestamps with the server’s last boot time.
  • Records satisfied requests as done, clears pending markers, and updates status and logging.
  • Documents the behavior in the changelog.
File Description
src/​SPSUpdate.ps1 Adds out-of-band reboot reconciliation.
CHANGELOG.md Documents the deferred-reboot behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/SPSUpdate.ps1 Outdated
Out-of-band reconciliation only applies to a stale deferred request. If this run
performed an install and Reboot.Force is enabled (exit 0, no 17022), the freshly
installed binaries still need a restart; excluding (Force -and InstallPerformed)
lets that case fall through to the normal reboot/schedule decision instead of
being marked Done by a stale, already-satisfied pending marker.

Refs #68

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The timestamp comparison can miss a completed reboot across daylight-saving fall-back and trigger another restart.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Compare timestamps in UTC to handle DST fall-back correctly

src/​SPSUpdate.ps1:642

Compare these timestamps in UTC. DateTime ordering compares clock ticks rather than normalizing time zones. Across a daylight-saving fall-back, a pending request at 01:50 -04:00 followed by a boot at 01:10 -05:00 can fail this check even though the reboot happened 20 minutes later. The pending request then remains eligible for another restart. Normalize both values to UTC before comparing.

The .pending marker carries an offset (Get-Date -Format o) while CIM
LastBootUpTime is local. Comparing local wall-clock ticks can miss a completed
reboot during the DST fall-back hour (an earlier local tick for a physically
later boot). Normalize the pending timestamp (DateTimeOffset.UtcDateTime) and
LastBootUpTime (ToUniversalTime) to UTC before comparing.

Refs #68
@luigilink
luigilink requested a balanced review from Copilot October 7, 2026 10:17
@luigilink

Copy link
Copy Markdown
Owner Author

Addressed the DST fall-back finding in 4702bcc: both timestamps are now compared in UTC — the .pending marker is parsed via [datetimeoffset]::Parse(...).UtcDateTime and LastBootUpTime is normalized with .ToUniversalTime(), removing the local wall-clock ambiguity during the DST fall-back hour.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The untested Windows-only reboot reconciliation path needs target-server validation before approval.

Review effort: Balanced
Findings: None

@luigilink luigilink left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

LGTM

@luigilink
luigilink merged commit 4e57342 into main Oct 7, 2026
4 checks passed
@luigilink
luigilink deleted the feat/reconcile-outofband-reboot-68 branch October 7, 2026 11:29
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.

Reconcile a deferred reboot already performed out-of-band (manual / Windows Update)

2 participants