Skip to content

feat(ha): wire opt-in production runtime - #875

Open
ankitgoswami wants to merge 3 commits into
ankitg/ha-fail-commands-on-takeoverfrom
ankitg/ha-production-wiring
Open

feat(ha): wire opt-in production runtime#875
ankitgoswami wants to merge 3 commits into
ankitg/ha-fail-commands-on-takeoverfrom
ankitg/ha-production-wiring

Conversation

@ankitgoswami

@ankitgoswami ankitgoswami commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Reviewable diff: +265/-17 across 4 files (excludes generated, test, and story files).

Summary

Fleet operators can opt a new deployment into the active/passive runtime through FLEET_HA_* configuration. HA remains off by default, while enabled deployments now fail startup before connecting or migrating if lease timing, etcd addressing, database failover routing, or PostgreSQL server authentication is unsafe.

Stack

This diff is relative to #874. The parent provides lease ownership, command restart recovery, fatal active-process exit, and active request admission. This PR selects and constructs that runtime from production configuration. Installer automation, rolling-upgrade migration coordination, and live three-host qualification remain outside this PR.

How it works

  1. With HA disabled, Fleet creates the standalone runtime without reading HA secrets or contacting etcd and Patroni.
  2. With HA enabled, Fleet validates lease intervals, requires IP-addressed HTTPS etcd endpoints, and requires an explicit multi-host PostgreSQL DSN with read-write writer selection and hostname-verifying TLS backed by a trust root.
  3. These checks run before Fleet opens the database or applies migrations, so unsupported HA deployments fail without changing database state.
  4. Fleet reads the observer password and service CA, creates authenticated etcd and Patroni clients, and prepares the SQL used by the writer observer and lease store.
  5. The observer compares Patroni's etcd-published primary with the PostgreSQL writer Fleet reached. The coordinator opens active work only after that proof and the Fleet lease agree.
  6. Normal process shutdown stops runtime jobs gracefully. Ownership loss aborts active work and skips the later graceful job-stop defer so the supervisor can restart Fleet as a passive candidate without stale cleanup running beside the replacement active.
flowchart TD
    START["fleetd starts"] --> ENABLED{"FLEET_HA_ENABLED?"}
    ENABLED -->|"No"| STANDALONE["Standalone runtime"]
    ENABLED -->|"Yes"| VALIDATE["Validate timing, IP endpoints, and secure multi-host DB routing"]
    VALIDATE --> DB["Connect and migrate PostgreSQL"]
    DB --> CLIENTS["Create etcd and Patroni clients plus prepared HA queries"]
    CLIENTS --> PROOF["Compare DCS primary with connected PostgreSQL writer"]
    PROOF --> LEASE["Acquire Fleet active lease"]
    LEASE --> ACTIVE["Start active jobs and open admission"]
    ACTIVE --> LOSS{"Ownership lost?"}
    LOSS -->|"No, normal shutdown"| STOP["Gracefully stop jobs"]
    LOSS -->|"Yes"| ABORT["Abort jobs and exit process"]
Loading

Areas of the code involved

Area / package / file What changed Why it matters for review
server/cmd/fleetd/config.go, main.go Exposes HA settings, validates the HA database contract before connection, selects the configured runtime, and preserves abort-only ownership-loss exit Review startup ordering and shutdown behavior
server/internal/ha/config.go Validates HA timing and IP endpoints, constructs authenticated clients and prepared HA queries, and owns cleanup Review disabled-mode isolation, TLS boundaries, and resource ownership
server/internal/infrastructure/db/config.go Adds the enabled-HA database contract for multi-host writer routing and authenticated TLS on every fallback Review PostgreSQL failover and transport guarantees
Focused tests Cover environment parsing and the important HA timing, endpoint, routing, and TLS rejection cases Review the operator-facing configuration contract

Key technical decisions & trade-offs

  • HA remains explicit opt-in, so standalone installations keep their existing database and runtime behavior.
  • The supported HA profile requires literal-IP etcd endpoints because its generated certificates contain IP SANs.
  • HA requires an explicit multi-host DB_DSN, target_session_attrs=read-write, and authenticated TLS instead of accepting legacy single-host fields or opportunistic encryption.
  • The observer and lease store share the repository's prepared/retrying query path and close it with the HA clients.
  • Ownership loss is crash-only for active jobs; normal shutdown remains graceful.
  • Fleet still applies migrations before runtime ownership. Coordinating migrations across rolling application upgrades is intentionally outside the clean-deployment scope of this PR.

Testing & validation

  • Focused internal/ha, database configuration, and cmd/fleetd tests passed.
  • The complete server build passed.
  • Hermit-backed server lint passed with zero issues.
  • The DB-backed HA integration suite was not run locally because the available test database credentials were invalid; live three-host qualification remains follow-up work.

Compound Engineering

@ankitgoswami
ankitgoswami requested a review from a team as a code owner August 3, 2026 23:50
@github-actions github-actions Bot added the server label Aug 3, 2026
@github-actions github-actions Bot added the review-policy: needs-review Managed by the Review Policy workflow. label Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 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 (0911bf99bfb81197339cd0a1e90929775528e2d8...1b1f7fc9d780e5e5715b1e169cacad1a1b7c4ac6, 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

The HA configuration, authenticated database routing, ownership-loss cleanup, and runtime-job abort paths were reviewed. Targeted Go tests could not run because the read-only environment prevented creation of the module cache.


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

Copilot AI lite review requested due to automatic review settings August 4, 2026 04:49
@mcharles-square
mcharles-square force-pushed the ankitg/ha-production-wiring branch from e8c7bb1 to 3aa622e Compare August 4, 2026 04:49

This comment was marked as outdated.

chatgpt-codex-connector[bot]

This comment was marked as outdated.

@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from 3aa622e to e39383b Compare August 4, 2026 16:06
chatgpt-codex-connector[bot]

This comment was marked as outdated.

@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from e39383b to 73acc35 Compare August 4, 2026 21:24
chatgpt-codex-connector[bot]

This comment was marked as outdated.

@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from 73acc35 to 4fded94 Compare August 4, 2026 21:35

@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: 4fded945d8

ℹ️ 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/fleetd/config.go
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from 4fded94 to ad1e068 Compare August 4, 2026 22:30
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from ad1e068 to 6079acd Compare August 4, 2026 22:37

@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: 6079acdede

ℹ️ 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/ha/config.go
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from 6079acd to 8aeb834 Compare August 4, 2026 23:00
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch 3 times, most recently from 62344dd to 1e938ec Compare August 5, 2026 07:32
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from 1e938ec to 09c7f00 Compare August 5, 2026 08:22

@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: 09c7f0080b

ℹ️ 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/fleetd/main.go
Comment thread server/cmd/fleetd/main.go
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from 09c7f00 to 55d1429 Compare August 5, 2026 16:02
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from 55d1429 to 344b6a4 Compare August 5, 2026 16:04
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from 344b6a4 to 12a5765 Compare August 5, 2026 16:22

@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: 12a5765c94

ℹ️ 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/ha/config.go Outdated
ankitgoswami added a commit that referenced this pull request Aug 5, 2026
- validate HA timing and database routing at startup
- require IP-addressed etcd endpoints and prepared HA queries
- preserve abort-only shutdown after ownership loss
@ankitgoswami

Copy link
Copy Markdown
Contributor Author

🤖

HA mode permits unauthenticated PostgreSQL transport.

Addressed in d8a939b023. Before Fleet opens PostgreSQL or runs migrations, enabled HA now requires an explicit multi-host DB_DSN, target_session_attrs=read-write, and hostname-verifying TLS with a configured trust root on every primary and fallback endpoint. Standalone database configuration remains unchanged.

@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: d8a939b023

ℹ️ 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/ha/config.go
- validate HA timing and database routing at startup
- require IP-addressed etcd endpoints and prepared HA queries
- preserve abort-only shutdown after ownership loss
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-production-wiring branch from d8a939b to 5aaf0ee Compare August 5, 2026 18:27

@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: 5aaf0eee77

ℹ️ 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/fleetd/main.go Outdated
- validate persisted lease precision
- hard-cancel and await fatal HA aborts
@ankitgoswami

Copy link
Copy Markdown
Contributor Author

🤖

Ownership loss leaves detached controller work running.

Addressed differently: fatal HA shutdown now hard-cancels command execution, schedule processing, and curtailment reconciliation before bounded stop/wait cleanup. Schedule and curtailment now implement runtimejobs.Aborter, so their detached work contexts are canceled immediately rather than drained after ownership loss.

@ankitgoswami

Copy link
Copy Markdown
Contributor Author

🤖

HA config validation is currently incomplete (interval invariants aren’t validated)

Addressed: enabled HA now validates positive renew/retry intervals, requires renewal before lease expiry, and requires lease duration to use the same whole-millisecond precision persisted in PostgreSQL.

@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: 1b1f7fc9d7

ℹ️ 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".

g.endActivation()
g.setGroupState(StateStopping)
abortJobs(g.jobs)
return g.finishStop(ctx)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid graceful Stop during HA aborts

Fresh evidence in this commit is that Group.Abort now calls finishStop, which invokes every job's Stop after abortJobs. During HA ownership loss or lease expiry, Runtime.abortGroup uses this path, but jobs that don't implement Aborter still run their graceful shutdown; for example TelemetryService has no Abort and its Stop waits for finishActivation, which flushes/requeues pending telemetry (server/internal/domain/telemetry/activation.go:153-168). That lets the old active process keep writing stale telemetry after a replacement has acquired ownership, so keep HA aborts to hard-cancel/wait-only cleanup or add abort semantics for these jobs.

Useful? React with 👍 / 👎.

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

Labels

review-policy: needs-review Managed by the Review Policy workflow. server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants