fix(scripts): keep the node re-read diagnostic when kubectl fails silently by devantler · Pull Request #4057 · devantler-tech/platform

devantler · 2026-09-22T21:10:52Z

Why

A production rollout stops when it cannot safely take a node out of service. When the check that re-reads the node failed without printing an error, the operator was told the node could not be claimed but not why. That happens when the cluster API is unhealthy, which is when the reason matters most.

What

A silent failure now produces a clear fallback message saying the node re-read failed. A real error message from the cluster is still passed through unchanged. This matches how the equivalent Flux check already behaves.

Fixes #3886

🤖 Generated with Claude Code

https://claude.ai/code/session_011Aut24ni8XbKPYyY5mYrbb

Generated by Claude Code

…ently When the node re-read after a refused cordon claim failed without writing stderr, the diagnostic file was zero bytes, so copying it succeeded while truncating the result file. The fallback line never fired, the empty file was skipped, and the operator saw a bare "Could not atomically claim and cordon" with no cause. An empty diagnostic now counts as a failed copy, exactly as the Flux parent re-read already does, so the deterministic fallback line is emitted. A non-empty diagnostic still reaches the operator verbatim. Fixes #3886 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Aut24ni8XbKPYyY5mYrbb

devantler · 2026-09-22T21:11:05Z

Evidence for head 9d6fb73d25e0ddf4e5bdd23fde703da4079636df

The fix is the one-line [[ ! -s … ]] || guard on the node-claim re-read. It mirrors the Flux parent-fence re-read, which already has the guard.

Tests added

The fake kubectl now records a refused claim and can fail the next node re-read once, with or without stderr. Two tests use it:

TestSilentNodeRereadFailureStillEmitsADiagnostic: the re-read fails with no stderr. The test asserts the cordon-claim: node re-read failed; its diagnostic could not be read fallback reaches the output, and that no drain happens.
TestNodeRereadFailureDiagnosticSurvives: the re-read fails with a real diagnostic. The test asserts cordon-claim: <that diagnostic> survives verbatim and the fallback does not appear.

Both first assert that the fixture really failed the re-read, via its marker file, so neither can pass because the path was never reached.

RED / GREEN

RED, test code with main's script: TestSilentNodeRereadFailureStillEmitsADiagnostic fails with expected document to contain "cordon-claim: node re-read failed; its diagnostic could not be read". This is the silence Node re-read loses its diagnostic when kubectl fails without writing stderr #3886 describes.
GREEN, this head: both new tests pass, together with the neighbouring claim tests (TestConcurrentCordonBeforeAtomicClaimStopsTheRoll, TestRetriedClaimStillRefusesWhenTheRetryBudgetIsExhausted, TestUnrelatedNodeWriteBeforeAtomicClaimIsRetried).
The companion test passes on main by design, since main already copies a non-empty diagnostic. Its job is to block an over-broad fix. Ablation: with the guard replaced by an unconditional fallback, it fails on both assertions (the diagnostic is missing and the fallback appears). Restored afterwards.

Validation (CI's `🔐 Validate Flux GHCR credential bridge` step, run locally)

go test -timeout=40m ./scripts/tests/refresh-flux-ghcr-auth → ok … 1034.5s (full package)
bash -n ✅ · shellcheck scripts/refresh-flux-ghcr-auth.sh scripts/ghcr-auth-lib.sh scripts/refresh-flux-ghcr-auth-safety.sh ✅ · bash scripts/tests/test-refresh-flux-ghcr-auth-safety.sh → All GHCR auth safety regression tests passed. · gofmt -l clean

Behaviour

This path only runs during a production Talos rollout, and I have no cluster access here. The fake-kubectl harness above is the observation surface: it drives the real script through the refused-claim → failed-re-read path and reads the operator-facing output.

Generated by Claude Code

devantler · 2026-09-23T05:08:54Z

@coderabbitai review

Generated by Claude Code

coderabbitai · 2026-09-23T05:09:10Z

@devantler I’ll review the changes at head 9d6fb73d25e0ddf4e5bdd23fde703da4079636df.

✅ Action performed

Review finished.

coderabbitai · 2026-09-23T05:11:38Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 67e6f749-d57f-48f7-bddd-531f37dd6c7e

📥 Commits

Reviewing files that changed from the base of the PR and between ab06803 and 420edfa.

📒 Files selected for processing (3)

scripts/refresh-flux-ghcr-auth.sh
scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go
scripts/tests/refresh-flux-ghcr-auth/rollout_core_test.go

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

📜 Recent review details

🔇 Additional comments (3)

📝 Walkthrough

Walkthrough

When a node re-read fails, the claim path now writes a fallback diagnostic if captured stderr is empty or cannot be copied. The fake kubectl fixture can simulate silent and diagnostic-producing failures. New tests check that both diagnostic cases reach output and that node draining does not run after the failed claim.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 420ed

The node re-read failure should now report a diagnostic even when kubectl is silent. No actionable issue remains in the supplied review evidence.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files.	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 describes the main change: preserving or providing a diagnostic when the node re-read fails silently.
Description check	✅ Passed	The description explains the silent-failure fallback, preservation of real diagnostics, related tests, and the linked issue. It is directly related to the changeset.
Linked Issues check	✅ Passed	Issue `#3886` requires a fallback when the node re-read diagnostic is empty and verbatim preservation when it is non-empty. `claim_node_cordon_ownership` now checks the diagnostic file size before copyi…
Out of Scope Changes check	✅ Passed	The production change is limited to the node-claim re-read diagnostic path. The fake `kubectl` support and the two tests directly implement the acceptance criteria in issue `#3886`. No unrelated change …

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 · 2026-09-23T05:12:24Z

Resolution record for CodeRabbit's pre-merge Docstring Coverage warning at head 9d6fb73d25e0ddf4e5bdd23fde703da4079636df (summary): refuted, no change.

The count is a generic 80% threshold over the seven functions the diff touches. It is not a convention in this repository. Both new tests already open with a comment explaining the scenario and why it matters, and so does the changed shell function. The functions the metric counts as missing are the existing fake-kubectl dispatchers (fakeKubectlGetNode, fakeKubectlPatchNode). This PR adds one branch to each, and each branch has its own comment. Adding boilerplate docstrings to those dispatchers would be unrelated churn in a one-concern bug fix. CodeRabbit's review found nothing actionable ("No actionable comments were generated").

Generated by Claude Code

devantler · 2026-09-23T09:30:28Z

@coderabbitai review

coderabbitai · 2026-09-23T09:30:44Z

@devantler I’ll review PR #4057 at head 420edfab1d7cb9025b8169f0b4e8f9ab1fa50561.

⚠️ Action not completed

Review rate limited.

devantler · 2026-09-23T10:22:22Z

@coderabbitai review

coderabbitai · 2026-09-23T10:22:41Z

@devantler I’ll request a review of PR #4057 at head 420edfab1d7cb9025b8169f0b4e8f9ab1fa50561.

⚠️ Action not completed

No files to review.

devantler · 2026-09-23T10:22:47Z

@coderabbitai review

Generated by Claude Code

coderabbitai · 2026-09-23T10:23:22Z

@devantler, I’ll request a review of PR #4057 at head 420edfab1d7cb9025b8169f0b4e8f9ab1fa50561. The previous request at this head reported “No files to review,” so this request may have the same result.

⚠️ Action not completed

Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.

devantler · 2026-09-23T10:23:25Z

@coderabbitai full review

coderabbitai · 2026-09-23T10:23:58Z

@devantler I’ll request a full review of PR #4057. This will review the entire changeset rather than only commits eligible for incremental review.

✅ Action performed

Full review finished.

devantler · 2026-09-23T10:24:13Z

@codex review

chatgpt-codex-connector · 2026-09-23T10:24:34Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-23T10:26:37.547874Z	`420edfa`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector · 2026-09-23T10:26:35Z

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 420edfab1d

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-project-automation Bot added this to 🌊 Project Board Sep 22, 2026

github-project-automation Bot moved this to 🫴 Ready in 🌊 Project Board Sep 22, 2026

devantler marked this pull request as ready for review September 23, 2026 05:17

devantler added this pull request to the merge queue Sep 23, 2026

github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Sep 23, 2026

Merge branch 'main' into claude/flux-auth-node-reread-diagnostic-3886

420edfa

devantler added this pull request to the merge queue Sep 23, 2026

Merged via the queue into main with commit 0601feb Sep 23, 2026
30 checks passed

devantler deleted the claude/flux-auth-node-reread-diagnostic-3886 branch September 23, 2026 11:42

github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Sep 23, 2026

devantler mentioned this pull request Sep 29, 2026

Merge queue evicts PRs after 60 min while merge_group deploys take 36-80 min, so successful deploys merge nothing #3097

Closed

1 of 3 tasks

Conversation

devantler commented Sep 22, 2026

Why

What

Uh oh!

devantler commented Sep 22, 2026

Tests added

RED / GREEN

Validation (CI's 🔐 Validate Flux GHCR credential bridge step, run locally)

Behaviour

Uh oh!

devantler commented Sep 23, 2026

Uh oh!

coderabbitai Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

❌ Failed checks (1 warning)

Uh oh!

devantler commented Sep 23, 2026

Uh oh!

Uh oh!

devantler commented Sep 23, 2026

Uh oh!

coderabbitai Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devantler commented Sep 23, 2026

Uh oh!

coderabbitai Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devantler commented Sep 23, 2026

Uh oh!

coderabbitai Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devantler commented Sep 23, 2026

Uh oh!

coderabbitai Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devantler commented Sep 23, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot commented Sep 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Validation (CI's `🔐 Validate Flux GHCR credential bridge` step, run locally)

coderabbitai Bot commented Sep 23, 2026 •

edited

Loading

coderabbitai Bot commented Sep 23, 2026 •

edited

Loading

coderabbitai Bot commented Sep 23, 2026 •

edited

Loading

coderabbitai Bot commented Sep 23, 2026 •

edited

Loading

coderabbitai Bot commented Sep 23, 2026 •

edited

Loading

coderabbitai Bot commented Sep 23, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 23, 2026 •

edited

Loading