Skip to content

fix(deploy): let a deploy through when a failed deploy left one node on a recorded image - #4667

Closed
devantler wants to merge 2 commits into
mainfrom
claude/talos-readback-accepted-drift-4665
Closed

devantler wants to merge 2 commits into
mainfrom
claude/talos-readback-accepted-drift-4665

Conversation

@devantler

@devantler devantler commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Why

No production deploy can finish. A deploy that failed on 7 October left one worker on a newer boot image, every deploy since fails the final check that all nodes run the expected image, and redeploying cannot fix the node. A second check also stops deploys while the workers are full. That keeps every repair, including the one for the secrets store, from landing.

What

The boot-image check can now be told, in one reviewed and dated record, that a single named node is known to be on a single named image. It then lets the deploy finish with a visible warning instead of failing, and goes back to strict the day after the date. The record added here names that one worker and lapses on 22 October.

This also carries the one-commit fix from #4666, which lets the registry-credential probe run on a worker that is almost full. Each pull request's own deploy has to pass both checks, so neither fix can land alone.

Part of #4665
Part of #4664

⚠️ Merge order: this narrows a deploy safety check, so it waits for the maintainer's approval before it is queued. #4666 becomes empty once this lands.
👉 After merge/promotion: the node still needs repairing; the record only lets deploys through meanwhile. Remove the record once every node is back on one image.

🤖 Generated with Claude Code

…on a recorded image

A deploy that changes the Talos image upgrades one node and then fails leaves
that node on the new image. main cannot roll a node to another image at the
same Talos version, so every later deploy fails the boot-image readback and no
repair can land.

Read a reviewed record that may accept one static node on one named image
until a date. The node is reported as a warning and the result is never the
plain pass; the Talos version, readiness, scheduling and node counts still
bind it. A record that is not exactly the header and at most one well-formed
row is refused before any node is read, and an expired row accepts nothing.

Record the worker the failed deploy of #4197 left on its image.

Part of #4665

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…l probe can make

The runtime pull probe names its node, so the kubelet alone admits it. With
a 16Mi memory request it was refused (OutOfmemory) by a worker that had
15 Mi left to request, the drain was refused, and the production deploy
failed; the recovery deploy of main failed the same way on another worker.

The probe only prints a version, so it now requests 1m CPU and 1Mi memory
and keeps its limits. The fake kubectl rejects a probe that asks for more,
so every probe test holds the manifest to that.

Fixes #4664

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 3491077)
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer (interactive Claude Code session)

Tried as a user — at head 3e9fd375bfcf9d8838b3be50e0fef69e901b28c8.

The user of this change is the next production deploy. I ran the real check, read-only, against production at 19:38Z on 2026-10-08, with the record as committed here:

Check run Result
The check as it is on main Fails: seven nodes on the expected image, one worker on another
The check on this branch Passes, with a warning that names that worker, both images, the issue and the date, and a result line that says one of eight nodes runs a recorded image. It does not print the plain pass
The check on this branch, told it is the day after the record's date Fails, and says the record expired and the readback is strict again

I also confirmed where that worker's image comes from: it is exactly the image #4197 defines, and main defines the other one.

The offline test covers what production cannot show safely: drift on a node the record does not name, the recorded node on another image, a second node on the recorded image, the recorded node while not ready, cordoned or on another Talos version, and eleven malformed records. Weakening the check in eight ways, one at a time, fails the test each time.

For the commit carried from #4666: its Go tests pass here (one package, about six minutes) and the script is lint-clean. I did not exercise the probe on a full worker; that needs a deploy.

Not reachable before merge: a real deploy passing both checks. That is this pull request's own queue deploy, and it is why it waits for the maintainer's approval first.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

There is now a root-repair alternative in #4669 at 733c41cc4fdf2f3559f11b4e69c6a46c7ed0520e: adopt the released same-version image reconciliation and include the exact request-headroom patch from #4666. It preserves the desired production boot image and strict verification; no temporary image exception is needed in that proposal.

Local command regressions and exact-head review are complete. Hosted validation is still finishing, and protected deployment/readback remains required before this is called recovered. I have not changed this branch or treated the proposed exception as approved.

@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: 0fb0d8e0-af02-450a-a55c-56f25daba64a
📥 Commits

Reviewing files that changed from the base of the PR and between 1cf7254 and 3e9fd37.

⛔ Files ignored due to path filters (1)
  • scripts/talos-boot-image-drift-accepted.tsv is excluded by !**/*.tsv
📒 Files selected for processing (7)
  • .github/workflows/ci.yaml
  • AGENTS.md
  • docs/operations/boot-image-drift.md
  • scripts/refresh-flux-ghcr-auth.sh
  • scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go
  • scripts/tests/test-talos-schematic-readback.sh
  • scripts/verify-talos-schematic-readback.sh

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-10T13:01:12.782Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 3057
File: .github/workflows/ci.yaml:622-659
Timestamp: 2026-08-10T13:01:12.782Z
Learning: Repository shell tests and scripts must remain compatible with macOS Bash 3.2. Do not use Bash 4+ features such as `mapfile`; use portable constructs, such as a `while IFS= read -r` loop, instead.

Applied to files:

  • scripts/verify-talos-schematic-readback.sh
🔇 Additional comments (8)
scripts/verify-talos-schematic-readback.sh (2)

42-99: LGTM!


112-151: LGTM!

scripts/tests/test-talos-schematic-readback.sh (1)

26-29: LGTM!

Also applies to: 41-41, 123-250

.github/workflows/ci.yaml (1)

990-991: LGTM!

AGENTS.md (1)

283-284: LGTM!

docs/operations/boot-image-drift.md (1)

1-58: LGTM!

scripts/refresh-flux-ghcr-auth.sh (1)

1719-1723: LGTM!

scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go (1)

3010-3017: LGTM!


📝 Walkthrough

Walkthrough

The Talos readback script now supports a reviewed, dated exception for one static node on one specified non-current schematic. Tests cover accepted and rejected records, and CI runs the readback test when the record changes. Documentation describes exception handling and node recovery. The runtime pull probe’s CPU and memory requests are reduced; its limits are unchanged.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3e9fd

The deploy uses the intended dated exception. No established issue blocks merge, though the required deployment checks remain pending.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (3 skipped: 3… 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 summarizes the main change: allowing deployment to proceed when one node remains on a recorded boot image after a failed deploy.
Description check ✅ Passed The description explains the deployment failure, the temporary recorded-image exception, its expiry, the carried probe fix, and the required follow-up repair.
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 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (3 skipped: 3 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 (interactive Claude Code session)

Closing without merging: the state this was approved for no longer exists.

The maintainer approved queueing this first, with a standing instruction to check it is still relevant before acting. Checked on 2026-10-09 at 12:27Z, read-only:

So nothing here should land as written, and it was not queued. The exception mechanism and its tests stay available on this pull request for the open criterion on #4665 (a reviewed way back when a failed deploy strands a node), if that is the design chosen for it.

@devantler devantler closed this Oct 9, 2026
@devantler
devantler deleted the claude/talos-readback-accepted-drift-4665 branch October 9, 2026 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant