Skip to content

fix(openbao): let a lost production server be recreated - #4653

Closed
devantler wants to merge 1 commit into
mainfrom
claude/openbao-ondelete-recovery-4391
Closed

devantler wants to merge 1 commit into
mainfrom
claude/openbao-ondelete-recovery-4391

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

The production secrets store has been running on two of its three servers since 7 October 23:52 UTC, and the third cannot come back on its own. The staged-rollout setting added last week makes Kubernetes rebuild a lost server from an outdated stored template, which the storage-placement guard correctly refuses. Until this is fixed, losing one more server stops the secrets store, production reports unhealthy, and every deploy in the merge queue is held up behind it.

What

Production goes back to the setting it used before the staged rollout: a server is replaced only when it is deleted or lost, and the replacement is built from the newest configuration. That is the same release the first upgraded server has run healthy and unsealed since 7 October. No running server is replaced by this change; the remaining older server stays as it is until the staged rollout issue decides how to replace it.

Part of #4391

👉 After merge/promotion: the deploy should bring the third server back by itself; the readback to confirm it is recorded on #4391.

Production OpenBao has run on two of three servers since openbao-0 was
lost: the partitioned rolling update makes the StatefulSet controller
build every ordinal below the partition from the stored current
revision, which predates the baseline-worker pins, and the
storage-placement policy refuses that Pod.

Return production to the chart's OnDelete strategy. A missing Pod is
then created from the newest revision, which the canary has run
unsealed, and no running server is replaced.

Part of #4391

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository YAML (base), Organization UI (inherited)
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 562645c1-2b41-4641-a285-ccae51657819
📥 Commits

Reviewing files that changed from the base of the PR and between 7db7fd8 and 41d374f.

📒 Files selected for processing (8)
  • docs/operations/arc-runners.md
  • k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml
  • k8s/providers/hetzner/infrastructure/controllers/openbao/patches/standby-oidc-canary.yaml
  • k8s/providers/hetzner/infrastructure/controllers/openbao/transport/patches/enable-arc-transport.yaml
  • scripts/tests/arc-staging/transport_test.go
  • scripts/tests/test-openbao-standby-canary.sh
  • scripts/validate-eks-ci-role-policy/approved-surface-history.md
  • scripts/validate-eks-ci-role-policy/approved-surface.txt

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-08-11T12:41:28.242Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 3082
File: k8s/bases/infrastructure/controllers/coroot/cron-job-cnpg-degraded-alert.yaml:113-120
Timestamp: 2026-08-11T12:41:28.242Z
Learning: When changing behavior in Kubernetes manifests or related documentation, review comments and documentation in YAML/YML and Markdown files for statements describing the previous behavior. Update every stale statement in the same change so the repository’s explanatory text remains consistent with the implementation.

Applied to files:

  • k8s/providers/hetzner/infrastructure/controllers/openbao/patches/standby-oidc-canary.yaml
  • k8s/providers/hetzner/infrastructure/controllers/openbao/transport/patches/enable-arc-transport.yaml
🔇 Additional comments (8)
k8s/providers/hetzner/infrastructure/controllers/openbao/patches/standby-oidc-canary.yaml (1)

1-8: LGTM!

Also applies to: 32-33

k8s/providers/hetzner/infrastructure/controllers/openbao/transport/patches/enable-arc-transport.yaml (1)

2-3: LGTM!

Also applies to: 30-31

k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml (1)

134-134: LGTM!

docs/operations/arc-runners.md (1)

55-56: LGTM!

scripts/tests/arc-staging/transport_test.go (1)

165-165: LGTM!

Also applies to: 170-171

scripts/tests/test-openbao-standby-canary.sh (1)

19-19: LGTM!

Also applies to: 103-103, 111-115, 117-117, 120-120, 129-129, 141-143, 183-183

scripts/validate-eks-ci-role-policy/approved-surface-history.md (1)

3325-3342: LGTM!

scripts/validate-eks-ci-role-policy/approved-surface.txt (1)

91-91: LGTM!


📝 Walkthrough

Walkthrough

OpenBao StatefulSet patches change from partitioned RollingUpdate to OnDelete. The operational documentation and patch comments describe replacement after a Pod is deleted or lost, without replacing a running server. Regression tests and approved rendered-surface records are updated for the new strategy.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 41d37

A lost server should be recreated without replacing running servers, and the configured placement and startup paths align with that goal. End-to-end rejoining remains unproven in checked-in tests, so retain the tracked readback as follow-up; no concrete merge blocker is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (6 skipped: 6 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: allowing a lost production OpenBao server to be recreated.
Description check ✅ Passed The description explains why production must return to OnDelete and how the change recreates lost servers without replacing running servers.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. (6 skipped: 6 unsupported.)

  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Evaluation at 41d374f2e4b9d101ffd2eadd568c5059c900a20a

  • Rendered effect: the regression pulls the pinned chart, applies the release's actual post-renderer, and the resulting StatefulSet carries updateStrategy: {type: OnDelete} with nothing else changed against the chart output apart from the two reviewed fields (token injection, the TLS identity label). With the old partitioned strategy restored in the patch, the same regression fails — so it tests this change, not just the file.
  • Why this brings the server back (read-only, production, 01:05–01:15 UTC): the StatefulSet reports its stored current revision as the one from May and its newest revision as the one openbao-2 runs. The May revision has no baseline-worker pin; the newest has it, and openbao-2 was admitted with it. The released Kubernetes 1.36.4 controller builds a missing Pod from the newest revision under OnDelete, and from the stored current revision for any ordinal below a partition.
  • What it cannot do: OnDelete never deletes a Pod, so openbao-1 and openbao-2 keep running as they are.
  • Authorization ledger: exactly one entry moves (the OpenBao release), computed with the approved renderer versions; the ledger tests pass.
  • Not proven before deployment: that the recreated server unseals and rejoins. Its start-up hook is the same one openbao-2 used. If it does not, the cluster stays on two servers as it is now. I will read that back after the deploy and record it on fix(openbao): deliver the released standby OIDC repair through a staged rollout #4391.

Review: CodeRabbit completed at this commit with no actionable comments.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Cross-lane dependency readback: the failed #4197 deployment was followed by an unsuccessful Heal Prod run. Recovery stopped before publication because the Coroot baseline-context guard requires every operator-owned Coroot template to be fully ready. Fresh OIDC reads still find an incomplete Coroot server rollout; #4652 tracks its connection-exhaustion repair.

The same before-publication guard applies to the standard deployment path for this OpenBao repair. This is a separate live blocker from OpenBao’s stale revision and must be resolved without bypassing the guard. I am leaving this actively owned PR unchanged and continuing the Coroot work.

@devantler
devantler marked this pull request as ready for review October 8, 2026 01:48
@devantler
devantler added this pull request to the merge queue Oct 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 8, 2026
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Why this left the merge queue (02:53Z): its merge-group run 37719268317 failed in 🚀 Deploy to Prod at the pre-publish Coroot baseline check, before anything was published. The cause is unrelated to this change: one Coroot server is unready because the Coroot metadata database has no free connection slots (platform#4652). Every merge-group run since has failed at the same step, so re-queuing now would only evict it again.

Head 41d374f2e4b9d101ffd2eadd568c5059c900a20a is unchanged: CI green, review green, no open threads. Production still runs OpenBao on 2 of 3 servers. I re-queue this as soon as the Coroot check passes again and then read back the third server.

Blocker: #4652 | authority | last-verified 2026-10-08: Coroot server unready, deploys refused | asked slack 2026-10-08

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Why this left the merge queue at 02:53 UTC, and why it is not re-queued yet. Its merge-group deploy stopped before publishing at the Coroot readiness check, not at anything in this change. The same check has refused every deploy since 01:52 UTC (#4652, #4658). Re-queuing now would fail the same way, so this waits on the Coroot restart asked of the maintainer; it goes back in first once Coroot is fully ready. Head is unchanged at 41d374f2, review and checks still green.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Reviewed commit: 41d374f2e4b9d101ffd2eadd568c5059c900a20a

The Docstring Coverage advisory is not a missing public API contract. The Go change renames one Test... function and updates its assertion; no production Go function or exported API is introduced. The shell changes adjust the existing rendered-manifest predicate and mutation controls. Their behavior and the reason for OnDelete are documented next to the assertions. A docstring coverage percentage for these test functions does not identify an undocumented product interface. I am refuting that advisory on the current diff, not disabling the evaluator or reducing any test requirement.

body_findings=0-resolved@41d374f2e4b9d101ffd2eadd568c5059c900a20a for that unchanged advisory only. Any new or changed finding remains actionable. I am restarting the review loop at this same head.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Reviewed commit: 41d374f2e4b9d101ffd2eadd568c5059c900a20a

The deployment blocker has cleared. The authorized recovery restored the secret service and observability service to their desired replica counts. The database reports healthy, with both standbys streaming and participating in synchronous replication. Existing database storage and backups were preserved.

The unchanged pre-publication Coroot guard passes. Its post-reconciliation guard also passes: all six templates have the required baseline fields, are ready, and remain stable across three separated samples. The platform live-health check returns PLATFORM-HEALTH=OK.

The reviewed OnDelete behavior was exercised during recovery: the missing server was recreated while the two running servers were retained. CodeRabbit's fresh full review is green at this head. Its identical test-docstring advisory retains the specific same-head refutation in the resolution record; there are no new findings or unresolved threads. Required checks are present and green, and no other local session holds this PR.

I am clearing the stale blocker record and returning this unchanged head to the protected merge queue. Queue admission is not completion; I will verify deployment and the merged head afterward.

@devantler
devantler removed this pull request from the merge queue due to a manual request Oct 9, 2026
@devantler

devantler commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Blocker: #4673 | upstream | last-verified 2026-10-10: still open. Unchanged head 41d374f is clean with checks green, and the fault it repairs is still live: read at 10:46Z, the third secrets-store server is still refused at admission and the configuration job still waits for it, so infrastructure and apps do not reconcile. It cannot be delivered yet: a deploy from main still fails at the credential probe (run 38042707843, 09:48Z), and the deploy that carried the probe repair (#4666) stopped at the database drain. Order once the database is back and no recovery run is active: #4666 first, then this, then read back that the server is recreated and unsealed and record it on #4391.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

The follow-up protected deployment confirmed that the two existing fixes must
arrive together: the credential probe stops before publication, while the
missing secret-store member independently prevents reconciliation from
completing. The production queue builds one candidate at a time.

#4710 carries the exact intended source changes from this PR and its companion,
with their original heads recorded, against fresh main. It has fresh tests and
is going through new current-head CI and review. This PR remains open until the
combined fix has merged and its production outcome has been verified.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Closing as superseded by #4710, which merged at 21:17 UTC on 2026-10-10 and carries every file this pull request changes together with the two transition fixes its first deploy attempt showed were also needed. Read back at 21:20 UTC: the platform health check reports OK, all three secrets-store servers are running, and the configuration job completed. Nothing in this pull request remains to deliver.

@devantler devantler closed this Oct 10, 2026
@devantler
devantler deleted the claude/openbao-ondelete-recovery-4391 branch October 11, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant