fix(scripts): keep the node re-read diagnostic when kubectl fails silently

scripts/refresh-flux-ghcr-auth.sh

@@ -2196,12 +2196,15 @@ claim_node_cordon_ownership() {
       --output json \
       >"${state_file}" 2>"${reread_error_file}"; then
       # A failed re-read is now the actionable cause, so it replaces the claim
-      # conflict in the emitted output. The redirection truncates result_file
-      # before cat runs, so a failed copy would leave it EMPTY -- and
-      # emit_safe_operation_output skips an empty file entirely, which is the
-      # very silence this block exists to prevent. Fall back to a deterministic
-      # non-empty line instead of discarding the failure.
-      if ! cat "${reread_error_file}" >"${result_file}" 2>/dev/null; then
+      # conflict in the emitted output. Two things can leave that output empty:
+      # the copy fails, or kubectl exits non-zero having written no stderr, so the
+      # diagnostic file is zero bytes and copying it SUCCEEDS while the redirection
+      # truncates result_file. emit_safe_operation_output skips an empty file
+      # entirely, which is the very silence this block exists to prevent, so an
+      # empty diagnostic counts as a failed copy and falls back to a deterministic
+      # non-empty line.
+      if [[ ! -s "${reread_error_file}" ]] ||
+        ! cat "${reread_error_file}" >"${result_file}" 2>/dev/null; then
         echo "node re-read failed; its diagnostic could not be read" \
           >"${result_file}"
       fi

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

@@ -1721,6 +1721,19 @@ func fakeKubectlGetNode(args []string) int {
 		}
 		return 0
 	}
+	// The re-read that follows a refused cordon claim fails, once. The silent
+	// variant exits non-zero without writing stderr, so the caller's diagnostic
+	// file is zero bytes and copying it succeeds while carrying nothing; the
+	// diagnostic variant writes real stderr, whose content must survive.
+	if nodeName == os.Getenv("FAKE_NODE_REREAD_FAILURE_NODE") &&
+		markerExists("claim-failed-"+nodeName) &&
+		!markerExists("node-reread-failed-"+nodeName) {
+		touchMarker("node-reread-failed-" + nodeName)
+		if diagnostic := os.Getenv("FAKE_NODE_REREAD_FAILURE_DIAGNOSTIC"); diagnostic != "" {
+			return commandFailure(92, "%s", diagnostic)
+		}
+		return 92
+	}
 	if nodeName == os.Getenv("FAKE_RECOVERY_ADVANCES_BEFORE_RELEASE_NODE") &&
 		!markerExists("recovery-advanced-before-release-"+nodeName) {
 		var recoveryRecord map[string]any
@@ -2196,6 +2209,7 @@ func fakeKubectlPatchNode(args []string, patchFile string) int {
 		// and the claim must still refuse rather than drain.
 		if nodeName == os.Getenv("FAKE_CLAIM_FAIL_NODE") {
 			setMarkerContent("resource-version-"+nodeName, incrementDecimal(currentResourceVersion))
+			touchMarker("claim-failed-" + nodeName)
 			return commandFailure(56, "resourceVersion test failed")
 		}
 		// An unrelated writer (a kubelet status heartbeat, the cloud

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

@@ -409,6 +409,52 @@ func TestConcurrentCordonBeforeAtomicClaimStopsTheRoll(t *testing.T) {
 	}
 }
 
+// A node re-read can fail without writing anything to stderr. The diagnostic
+// file is then zero bytes rather than absent, so copying it succeeds while
+// carrying nothing -- and emit_safe_operation_output skips an empty file. The
+// operator must still be told why the node claim could not be re-verified.
+func TestSilentNodeRereadFailureStillEmitsADiagnostic(t *testing.T) {
+	t.Parallel()
+	f := newFixture(t)
+	result := f.runHelper(validConfig(), nil, map[string]string{
+		"FAKE_CLAIM_FAIL_NODE":          "prod-worker-1",
+		"FAKE_NODE_REREAD_FAILURE_NODE": "prod-worker-1",
+	})
+	requireFailureResult(t, result)
+
+	if !pathExists(filepath.Join(f.syncStateDir, "node-reread-failed-prod-worker-1")) {
+		t.Fatal("fixture did not fail the node re-read after the refused claim")
+	}
+	output := result.stdout + result.stderr
+	requireContains(t, output, "Could not atomically claim and cordon")
+	requireContains(t, output, "cordon-claim: node re-read failed; its diagnostic could not be read")
+	requireNoLine(t, readLines(f.operationLog), "node-drain:prod-worker-1")
+}
+
+// The empty-diagnostic guard must not swallow a real one: when the failed
+// re-read does write stderr, that text is what the operator needs, not the
+// deterministic fallback.
+func TestNodeRereadFailureDiagnosticSurvives(t *testing.T) {
+	t.Parallel()
+
+	const diagnostic = "Error from server (Forbidden): nodes \"prod-worker-1\" is forbidden"
+
+	f := newFixture(t)
+	result := f.runHelper(validConfig(), nil, map[string]string{
+		"FAKE_CLAIM_FAIL_NODE":                "prod-worker-1",
+		"FAKE_NODE_REREAD_FAILURE_NODE":       "prod-worker-1",
+		"FAKE_NODE_REREAD_FAILURE_DIAGNOSTIC": diagnostic,
+	})
+	requireFailureResult(t, result)
+
+	if !pathExists(filepath.Join(f.syncStateDir, "node-reread-failed-prod-worker-1")) {
+		t.Fatal("fixture did not fail the node re-read after the refused claim")
+	}
+	output := result.stdout + result.stderr
+	requireContains(t, output, "cordon-claim: "+diagnostic)
+	requireNotContains(t, output, "its diagnostic could not be read")
+}
+
 func TestPDBBlockedDrainRestoresOriginalSchedulability(t *testing.T) {
 	t.Parallel()
 	f := newFixture(t)