Skip to content

Add local HA operator status - #887

Open
ankitgoswami wants to merge 11 commits into
mainfrom
ankitg/ha-operator-status
Open

Add local HA operator status#887
ankitgoswami wants to merge 11 commits into
mainfrom
ankitg/ha-operator-status

Conversation

@ankitgoswami

@ankitgoswami ankitgoswami commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Reviewable diff: +842/-90 across 16 files (excludes generated, test, and story files).

Summary

Adds a local, redacted HA status surface so operators can distinguish active, passive, initializing, and degraded hosts without exposing cluster diagnostics through the VIP. It reports control availability separately from failover redundancy and adds the running release to the public health response header.

Stack: This is PR 1 of 6: #887 -> #888 -> #889 -> #890 -> #891 -> #892. Later PRs install, qualify, and update the HA profile; this PR only establishes operator visibility.

How it works

The HA runtime combines coordinator observation, command-gate state, and VIP/interface health into a redacted snapshot. fleet-ha status reads the local /health/ha endpoint, renders human or JSON output, and with --check probes etcd quorum, Patroni roles, writable PostgreSQL identity, both Fleet hosts, and the VIP. Normal lease contention is healthy passive state; stale observations, duplicate VIP ownership, or missing redundancy fail readiness. Nginx explicitly returns 404 for /health/ha, while public /health exposes only X-Proto-Fleet-Version.

flowchart LR
  R["HA runtime"] --> S["Redacted local status"]
  S --> C["fleet-ha status"]
  C --> P["Optional dependency probes"]
  V["VIP nginx"] --> N["404 for /health/ha"]
Loading

Areas of the code involved

Area / package / file What changed Why it matters for review
server/internal/ha/ Builds runtime status and checks endpoint ownership Check role, freshness, and wrong-interface behavior
server/internal/ha/deployment/status.go Aggregates local and dependency checks Check redaction and readiness semantics
server/cmd/fleet-ha/ Adds status [--json] [--check] Check output and exit behavior
health handlers and nginx Adds version metadata and isolates /health/ha Check the exposure boundary
SQL and generated sqlc Reads connected writer identity Generated output is derived from server/sqlc/queries/ha.sql

Key technical decisions & trade-offs

  • Local host or SSH access is the authentication boundary; no remote diagnostics token is added.
  • control_ready and failover_ready remain separate so loss of redundancy does not misreport the active service as down.
  • failover_ready proves current observable redundancy; clean-install qualification exercises actual activation because a status probe cannot prove that future job startup will succeed.
  • Status fields are redacted; topology details stay inside local checks.

Testing & validation

  • Tests cover active, passive, initializing, stale observation, stray or wrong-interface VIP ownership, redaction, CLI output, and nginx isolation.
  • Targeted Go tests, Hermit lint, and the static HA profile check pass.
  • Three-host behavior is qualified in Define clean-install HA qualification #889.

@github-actions github-actions Bot added documentation Improvements or additions to documentation automation client server review-policy: needs-review Managed by the Review Policy workflow. labels Aug 7, 2026
@ankitgoswami ankitgoswami changed the title ankitg/ha operator status Add local HA operator status Aug 7, 2026
@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 (dff6538cd00678458c5b6135852e83aed9248781...3392048c7ba154dafaf562d39cab25a4aa22d012, 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 security, correctness, or reliability findings were identified in the authoritative diff.

Notes

The diff, formatting, and shell syntax checks passed. Focused Go tests could not run because the read-only environment prevented creation of the Go module cache.


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

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 introduces a local-only, redacted HA operator status surface for Proto Fleet, adds a fleet-ha status CLI to read it (with optional deeper dependency checks), and tightens the VIP/nginx boundary so /health/ha diagnostics are not exposed publicly. It also adds the running release version to the public /health response via the X-Proto-Fleet-Version header.

Changes:

  • Add /health/ha loopback handler returning a redacted HA runtime contract, and include X-Proto-Fleet-Version on /health.
  • Add fleet-ha status [node.env] [--json] [--check] which reads the local HA status and optionally checks etcd/Patroni/DB writer/VIP readiness.
  • Update HA runtime/coordinator snapshot semantics and deployment/nginx/RFC docs to match the local-only diagnostic boundary.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
server/internal/handlers/health/handler.go Add version header on /health; add /health/ha JSON handler for redacted HA status.
server/internal/handlers/health/handler_test.go Add tests for /health/ha redaction and /health version header.
server/internal/ha/status.go Define redacted HA status contract and implement Runtime.Status().
server/internal/ha/status_test.go Add unit tests covering status transitions and redaction semantics.
server/internal/ha/runtime.go Extend runtime owner interface to expose a Snapshot() for status reporting.
server/internal/ha/runtime_test.go Update runtime owner test fakes to satisfy Snapshot() interface.
server/internal/ha/deployment/status.go Implement fleet-ha status runtime read + optional control-path dependency probes.
server/internal/ha/deployment/preflight.go Refactor Fleet env parsing into loadFleetEnvironment and strengthen required key validation.
server/internal/ha/coordinator.go Rework snapshot fields to support freshness/availability without leaking error details.
server/internal/ha/config.go Export LoadServiceTLS for shared use between runtime and host tooling.
server/cmd/fleetd/main.go Wire version header handler and mount /health/ha only when HA is enabled.
server/cmd/fleetd/main_test.go Add test ensuring HA requires loopback-only HTTP listen address.
server/cmd/fleetd/config.go Add validateHAHTTPAddress enforcing loopback listen address when HA enabled.
server/cmd/fleet-ha/main.go Add status subcommand with human/JSON output and --check failover readiness gate.
server/cmd/fleet-ha/main_test.go Add CLI tests for JSON output and --check failing when not failover-ready.
docs/rfcs/0002-active-passive-fleet-ha.md Update RFC to reflect loopback-only /health/ha and fleet-ha status tooling.
deployment-files/ha/tests/test-profile.sh Add assertions that nginx blocks HA diagnostics through the VIP.
deployment-files/client/nginx.https.conf Add nginx rule intended to block /api-proxy/health/ha through VIP.
deployment-files/client/nginx.http.conf Add nginx rule intended to block /api-proxy/health/ha through VIP.
Suppressed comments (1)

server/internal/ha/deployment/status.go:208

  • The goroutine in this range closes over the loop variable probe, so concurrent checks may all reference the same probe (typically the last one). That would break primary/replica counting and can misreport database readiness.
	for _, probe := range probes {
		go func() {
			if endpointReadyWithClient(ctx, client, probe.endpoint) {
				results <- probe.role
				return
			}
			results <- ""
		}()

Comment thread server/internal/ha/deployment/status.go Outdated
Comment thread deployment-files/client/nginx.http.conf
Comment thread deployment-files/client/nginx.https.conf
Comment thread deployment-files/ha/tests/test-profile.sh

@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: 372ade4542

ℹ️ 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/deployment/status.go Outdated
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-operator-status branch from 372ade4 to 147dd0d Compare August 7, 2026 18:44

@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: 147dd0d5f5

ℹ️ 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/deployment/status.go Outdated
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-operator-status branch 4 times, most recently from d19be5a to 64424fd Compare August 7, 2026 19:47
@ankitgoswami
ankitgoswami force-pushed the ankitg/ha-operator-status branch from 64424fd to c284fb4 Compare August 7, 2026 20:11
- block all HA diagnostic path suffixes through nginx
- open the writer probe through prepared database helpers
- use the host CA path for the writer probe
- execute one sqlc query on one pinned connection

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

ℹ️ 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/deployment/status.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation client 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