Skip to content

Complete HA updates through bounded failover - #891

Open
ankitgoswami wants to merge 47 commits into
ankitg/ha-passive-updatefrom
ankitg/ha-complete-update
Open

Complete HA updates through bounded failover#891
ankitgoswami wants to merge 47 commits into
ankitg/ha-passive-updatefrom
ankitg/ha-complete-update

Conversation

@ankitgoswami

@ankitgoswami ankitgoswami commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Reviewable diff: +524/-111 across 8 files (excludes generated, test, and story files).

Summary

Completes a passive-first HA application update by moving the VIP to the already-updated peer before replacing the old active host. The flow accepts one bounded lease-expiry and VIP-takeover interruption rather than adding graceful ownership handoff.

Stack: #887 -> #888 -> #889 -> #890 -> #891 -> #892. This diff is relative to passive-update PR #890, which already owns passive restart recovery and the bounded self-update handoff. This PR adds active completion and completion-specific recovery; released-artifact qualification lands in #892.

How it works

fleet-ha update VERSION --complete confirms the local node is active and the peer is already healthy, passive, and running the target. It fully stages and preflights the release, then rechecks both the local active role and the target-version peer immediately before persisting recovery intent. The stop command rechecks the active role while Fleet still serves, then gives Compose a one-second stop timeout inside the 5-second active-shutdown deadline. Normal lease expiry withdraws the VIP and lets the updated peer acquire ownership. A fresh 35-second takeover deadline covers lease expiry, acquisition, and VIP convergence. If either bounded step fails, it retries transient Compose conflicts while restarting the old release under an independent recovery context without swapping. Packaged Compose files use literal version image tags, so running fallback from the unchanged old deployment root selects the retained old images. Root-only qualification sentinels can pause before stop, after a successful stop but before takeover waiting, and between the two deployment renames so the exact released binary can exercise each crash window deterministically; normal hosts never create them. Healthy hardware qualification still requires the normal handoff below 15 seconds. After successful takeover, it atomically swaps the deployment, starts the target locally, and requires both passive role and full failover readiness before reporting completion.

On reboot, startup repair takes the updater process lock before touching self-update state, then reconciles the updater handoff and deployment layout before HA starts. PR #890 keeps rollback authority until a replacement binds its production socket and restores a partially stopped passive application after HA is available. This PR extends that same post-HA recovery for completion operations: pending recovery reuses app-start VERSION any to reconcile both Fleet application containers. Recovery first returns if the requested version and role are already healthy; otherwise Compose starts stopped services from the reconciled deployment without forcing healthy containers to be recreated. An interrupted nonterminal fallback persists pending recovery and blocks another update from replacing its context. If the explicit old-release restart fails after stop or takeover failure, the terminal operation retains that same pending recovery state; the updater retries it after HA is available and keeps new updates blocked until recovery succeeds. The same pending state covers a failed restart after activation-layout recovery. Startup persists the reconciled recovery command and pending flag before clearing the activation marker, so a failed state write remains retryable on the next boot.

sequenceDiagram
  participant A as Old active N
  participant P as Updated passive N+1
  participant V as VIP
  A->>A: Stage and preflight N+1
  A->>A: Persist recovery
  A->>A: Stop Fleet within 5s
  A->>A: Start 35s takeover deadline
  A-->>P: Lease expires
  P->>V: Acquire ownership and VIP
  A->>V: Require health version N+1 before 35s fallback
  A->>A: Swap, start N+1, verify passive
Loading

Areas of the code involved

Area / package / file What changed Why it matters for review
server/cmd/fleet-ha/ Adds update VERSION --complete and lifecycle helpers Check role and argument enforcement
server/internal/ha/deployment/update.go and server/internal/ha/update_timing.go Add active validation, peer proof, Fleet stop/start, and shared stop/takeover deadlines Check takeover and fallback ordering
server/internal/updater/ and server/internal/updaterapi/ Persist completion mode with the operation identity, recovery intent, and interrupted activation state Check idempotent retries and crash points
HA README Documents the bounded interruption Check operator expectations

Key technical decisions & trade-offs

  • Uses normal lease expiry and VIP takeover instead of graceful handoff state.
  • Swaps only after the target serves the VIP, allowing the old release to restart after failed takeover.
  • Release packaging pins application images to literal version tags; pre-swap fallback runs the unchanged old Compose definition rather than selecting images from a version argument.
  • Gives active shutdown a 5-second bound with a one-second Compose stop timeout, then starts a fresh 35-second takeover ceiling; hardware must still satisfy the below-15-second qualification target.
  • Binds each idempotency key to target version and passive-vs-completion mode so cross-mode retries fail.
  • Reuses the role-aware application start command for recovery instead of adding a second lifecycle protocol.
  • Uses absent-by-default root-owned sentinels only to make exact-artifact timeout and rename-window qualification deterministic; they add no production lifecycle state.
  • Recovery checks health before Compose, preserves a fully healthy application, and reconciles stopped services only when startup is still needed.
  • Requires serialized pre-HA layout repair; recovery intent is durable before the activation marker clears, and application recovery runs only after HA is available.

Testing & validation

  • Tests cover successful completion, wrong peer version, takeover timeout, the qualification barriers, deployment restoration between renames, bounded active shutdown with retrying old-release restart, cross-mode idempotency rejection, interrupted swaps, failed stop, takeover, terminal activation-layout restart persistence, failed recovery-state writes across restart, durable startup recovery, and repair exclusion while the updater lock is held.
  • Targeted Go tests, Hermit lint, and the static HA profile check pass.
  • Hardware timing evidence remains pending in Define adjacent-release HA update qualification #892.

@github-actions github-actions Bot added documentation Improvements or additions to documentation server labels Aug 7, 2026
@github-actions github-actions Bot added the review-policy: needs-review Managed by the Review Policy workflow. label Aug 7, 2026
@ankitgoswami ankitgoswami changed the title feat(ha): complete updates through failover Complete HA updates through bounded failover Aug 7, 2026
@ankitgoswami
ankitgoswami marked this pull request as ready for review August 7, 2026 18:31
Copilot AI lite review requested due to automatic review settings August 7, 2026 18:31
@ankitgoswami
ankitgoswami requested a review from a team as a code owner August 7, 2026 18:31

Copilot AI 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.

Pull request overview

This PR completes the HA “passive-first” application update workflow by adding an explicit completion mode that intentionally triggers a bounded failover (VIP moves to the already-updated peer) before swapping the old active host to the target release and rejoining as passive.

Changes:

  • Add a completion trigger path (--complete / Complete: true) end-to-end (CLI → updater API → manager).
  • Add HA completion orchestration in the updater manager (require-active, stop active app, wait for VIP target-version, then swap + start as passive; restart old release on takeover failure).
  • Add VIP takeover probing via the service CA and 15s timeout, plus startup recovery that restarts the currently-installed HA application after an interrupted operation.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
server/internal/updaterapi/types.go Add Complete flag to the trigger request payload.
server/internal/updaterapi/client.go Add TriggerComplete convenience method and shared trigger helper.
server/internal/updater/server.go Route completion requests to TriggerCompleteWithID.
server/internal/updater/manager.go Implement completion flow behavior, add HA app recovery on manager startup, and thread complete through the run loop.
server/internal/updater/manager_test.go Extend HA updater tests to cover completion path and restart recovery behavior.
server/internal/ha/deployment/update.go Add active-role enforcement, role-aware app stop/start, and VIP target-version probe with 15s bound.
server/internal/ha/deployment/update_test.go Unit test for VIP version mismatch behavior.
server/internal/domain/updates/executor_test.go Verify unix executor client can send Complete: true trigger payloads.
server/cmd/fleet-ha/main.go Add fleet-ha update VERSION --complete and new hidden lifecycle helpers (require-active, wait-takeover, role args).
server/cmd/fleet-ha/main_test.go Add coverage for completion CLI path selecting active precheck + completion trigger.
deployment-files/ha/README.md Document completion procedure and explicitly call out bounded interruption and 15s takeover expectation.

Comment thread server/internal/ha/deployment/update.go
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

  • Reviewed pull request diff only (6ddb8fe5c0a2c90b61bcc09c0c7e5bede6d24fa5...778b1f28b263f1e67e90c09785f1c8e0d3dda973, exact PR three-dot diff)
  • Model: gpt-5.6-sol

💡 Click "edited" above to see previous reviews for this PR.


Review Summary

Overall Risk: NONE

Findings

No concrete security, correctness, or reliability issues were identified in the changed hunks.

Notes

Review was limited to the authoritative PR diff. Targeted Go tests could not run because the review environment is read-only; diff validation passed.


Generated by Codex Security Review |
Triggered by: @ankitgoswami |
Review workflow run

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b440c119b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/cmd/fleet-ha/main.go Outdated
Comment thread deployment-files/ha/README.md
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-complete-update branch from 6b440c1 to 5d225ac Compare August 7, 2026 18:44
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-complete-update branch from 5d225ac to f9f03e7 Compare August 7, 2026 18:53
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-complete-update branch 2 times, most recently from 11776cf to 159afd9 Compare August 7, 2026 19:11
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-complete-update branch from 159afd9 to 7a681e5 Compare August 7, 2026 19:16

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a681e538b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/updater/manager.go
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-complete-update branch from 7a681e5 to 389e59b Compare August 7, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation review-policy: needs-review Managed by the Review Policy workflow. server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants