From a304ef0a3e9c99cc014a397b6059e5f41a213ba0 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Thu, 17 Sep 2026 23:12:14 +0200 Subject: [PATCH 1/2] fix(deploy): retry the Flux parent fence when controller churn breaks its CAS The GHCR bridge fences the image-verification policy handoff with an optimistic-concurrency JSON patch on the parent Flux Kustomization: test /metadata/resourceVersion and /metadata/uid, then add the owner annotation and spec.suspend. That object's status is rewritten continuously by its own controller, so its resourceVersion moves on its own and the test is lost to nothing but ordinary churn -- and on a serialized merge queue it is lost exactly when the queue is busiest. A lost test killed the whole production deploy and evicted the PR. Retry the read->patch cycle instead, bounded at 5 attempts with the existing sync interval as backoff, and only when the re-read proves the rejection was contention: the resourceVersion demonstrably moved, the object is the same UID, is still well-formed, is unfenced by anyone, and is not suspended. Every other rejection -- a permission denial, a validation error, anything at an unchanged resourceVersion -- still fails closed on the first attempt, and a foreign owner found by the re-read is refused on sight with the same wording the pre-acquisition check uses rather than retried against. The ambiguous-response adoption path is unchanged and still takes precedence, so EXIT cleanup continues to own a fence whose patch response was lost. Fixes #3046 Co-Authored-By: Claude Opus 5 (1M context) --- scripts/refresh-flux-ghcr-auth.sh | 184 +++++++++++++----- .../fake_kubectl_test.go | 32 +++ .../rollout_convergence_test.go | 107 ++++++++++ 3 files changed, 276 insertions(+), 47 deletions(-) diff --git a/scripts/refresh-flux-ghcr-auth.sh b/scripts/refresh-flux-ghcr-auth.sh index 9efa2964c..ced010620 100755 --- a/scripts/refresh-flux-ghcr-auth.sh +++ b/scripts/refresh-flux-ghcr-auth.sh @@ -4665,8 +4665,47 @@ flux_policy_parent_is_released() { ' "${flux_policy_parent_state_file}" >/dev/null } +flux_policy_parent_has_any_owner() { + jq -e \ + --arg annotation "${FLUX_POLICY_PARENT_OWNER_ANNOTATION}" ' + ((.metadata.annotations // {})[$annotation] // "") != "" + ' "${flux_policy_parent_state_file}" >/dev/null +} + +# A killed transaction leaves the owner annotation behind, so this refusal — not the +# malformed/suspended one — is what an orphaned parent fence actually hits. It needs +# the same pointer the child-handoff refusal already carries. Emitted from two places +# (the pre-acquisition check and the contention re-read), so the wording is shared: +# an operator must not have to tell two differently-worded refusals apart. +refuse_owned_flux_policy_parent() { + echo "::error::Another transaction already owns the parent Flux policy handoff; refusing cluster mutation. Run './scripts/refresh-flux-ghcr-auth.sh --fences' to list every held fence with its liveness evidence and exact release command, and see docs/dr/runbook.md → 'Recover an orphaned GHCR deploy fence'." +} + +# Whether a re-read parent is still the object this transaction may fence: the same +# object (UID), well-formed, unfenced by anyone, and not suspended. Only then is a +# rejected acquisition contention rather than a conflict. +flux_policy_parent_claim_preconditions_still_hold() { + jq -e \ + --arg uid "${flux_policy_parent_uid}" \ + --arg annotation "${FLUX_POLICY_PARENT_OWNER_ANNOTATION}" ' + .kind == "Kustomization" + and .metadata.uid == $uid + and (.metadata.resourceVersion | type == "string" and length > 0) + and ((.metadata.annotations // {}) | type == "object") + and (((.metadata.annotations // {})[$annotation] // "") == "") + and ((.spec.suspend // false) == false) + ' "${flux_policy_parent_state_file}" >/dev/null +} + pause_flux_policy_parent() { local resource_version attempt annotations_present + local max_attempts="${FLUX_POLICY_PARENT_CLAIM_MAX_ATTEMPTS:-5}" + local reread_resource_version + # The re-read below needs its own stderr sink. Pointed at the result file it would + # succeed, write nothing, and truncate the rejection that explains why the fence was + # refused — leaving a bare refusal with no cause in exactly the case that matters + # most, another actor or the controller competing for the object. + local reread_error_file="${flux_policy_parent_result_file}.reread" # The parent/child ownership annotations are a separate fail-closed fence: # even if this process loses the synchronization Lease during acquisition, a @@ -4687,14 +4726,8 @@ pause_flux_policy_parent() { echo "::error::Could not inspect the parent Flux reconciliation before the image-verification policy handoff." return 1 fi - if jq -e \ - --arg annotation "${FLUX_POLICY_PARENT_OWNER_ANNOTATION}" ' - ((.metadata.annotations // {})[$annotation] // "") != "" - ' "${flux_policy_parent_state_file}" >/dev/null; then - # A killed transaction leaves this annotation behind, so this branch — not the - # malformed/suspended one below — is what an orphaned parent fence actually hits. - # It needs the same pointer the child-handoff refusal already carries. - echo "::error::Another transaction already owns the parent Flux policy handoff; refusing cluster mutation. Run './scripts/refresh-flux-ghcr-auth.sh --fences' to list every held fence with its liveness evidence and exact release command, and see docs/dr/runbook.md → 'Recover an orphaned GHCR deploy fence'." + if flux_policy_parent_has_any_owner; then + refuse_owned_flux_policy_parent return 1 fi if ! jq -e ' @@ -4711,44 +4744,55 @@ pause_flux_policy_parent() { return 1 fi - resource_version="$(jq -er '.metadata.resourceVersion' \ - "${flux_policy_parent_state_file}")" flux_policy_parent_uid="$(jq -er '.metadata.uid' \ "${flux_policy_parent_state_file}")" flux_policy_parent_owner="${sync_lease_holder}" - annotations_present="$(jq -r \ - '(.metadata.annotations? | type) == "object"' \ - "${flux_policy_parent_state_file}")" - jq -n \ - --arg resource_version "${resource_version}" \ - --arg uid "${flux_policy_parent_uid}" \ - --arg owner_path "${FLUX_POLICY_PARENT_OWNER_JSON_PATH}" \ - --arg owner "${flux_policy_parent_owner}" \ - --argjson annotations_present "${annotations_present}" ' - [ - {op: "test", path: "/metadata/resourceVersion", value: $resource_version}, - {op: "test", path: "/metadata/uid", value: $uid} - ] - + (if $annotations_present then [] else - [{op: "add", path: "/metadata/annotations", value: {}}] - end) - + [ - {op: "add", path: $owner_path, value: $owner}, - {op: "add", path: "/spec/suspend", value: true} - ] - ' >"${flux_policy_parent_patch_file}" - if kubectl \ - --context "${KUBE_CONTEXT}" \ - --namespace flux-system \ - patch "${FLUX_KUSTOMIZATION_RESOURCE}" \ - "${IMAGE_VERIFICATION_FLUX_PARENT_KUSTOMIZATION}" \ - --type=json \ - --patch-file="${flux_policy_parent_patch_file}" \ - -o json \ - >"${flux_policy_parent_state_file}" \ - 2>"${flux_policy_parent_result_file}"; then - flux_policy_parent_acquired=true - else + + # The contended object is a Flux Kustomization whose status the controller rewrites + # continuously, so its resourceVersion moves on its own and this CAS is lost exactly + # when Flux is busiest — which, on a serialized merge queue, is the moment the next + # deploy starts. Losing it is contention, not a conflict, so it is retried; every + # other rejection, and every state that is no longer ours to fence, still fails + # closed on the first attempt (#3046). + attempt=1 + while :; do + resource_version="$(jq -er '.metadata.resourceVersion' \ + "${flux_policy_parent_state_file}")" + annotations_present="$(jq -r \ + '(.metadata.annotations? | type) == "object"' \ + "${flux_policy_parent_state_file}")" + jq -n \ + --arg resource_version "${resource_version}" \ + --arg uid "${flux_policy_parent_uid}" \ + --arg owner_path "${FLUX_POLICY_PARENT_OWNER_JSON_PATH}" \ + --arg owner "${flux_policy_parent_owner}" \ + --argjson annotations_present "${annotations_present}" ' + [ + {op: "test", path: "/metadata/resourceVersion", value: $resource_version}, + {op: "test", path: "/metadata/uid", value: $uid} + ] + + (if $annotations_present then [] else + [{op: "add", path: "/metadata/annotations", value: {}}] + end) + + [ + {op: "add", path: $owner_path, value: $owner}, + {op: "add", path: "/spec/suspend", value: true} + ] + ' >"${flux_policy_parent_patch_file}" + if kubectl \ + --context "${KUBE_CONTEXT}" \ + --namespace flux-system \ + patch "${FLUX_KUSTOMIZATION_RESOURCE}" \ + "${IMAGE_VERIFICATION_FLUX_PARENT_KUSTOMIZATION}" \ + --type=json \ + --patch-file="${flux_policy_parent_patch_file}" \ + -o json \ + >"${flux_policy_parent_state_file}" \ + 2>"${flux_policy_parent_result_file}"; then + flux_policy_parent_acquired=true + break + fi + # A lost patch response is ambiguous. Re-read and adopt only the exact # UID/owner/suspend tuple written by this transaction so EXIT cleanup owns # the durable fence even when kubectl reported failure. @@ -4757,14 +4801,60 @@ pause_flux_policy_parent() { --namespace flux-system \ get "${FLUX_KUSTOMIZATION_RESOURCE}" \ "${IMAGE_VERIFICATION_FLUX_PARENT_KUSTOMIZATION}" \ - -o json >"${flux_policy_parent_state_file}" || - ! flux_policy_parent_is_owned; then + -o json >"${flux_policy_parent_state_file}" \ + 2>"${reread_error_file}"; then + # A failed re-read is now the actionable cause, so it replaces the patch + # rejection in the emitted output. The redirection truncates the 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}" \ + >"${flux_policy_parent_result_file}" 2>/dev/null; then + echo "parent re-read failed; its diagnostic could not be read" \ + >"${flux_policy_parent_result_file}" + fi + break + fi + if flux_policy_parent_is_owned; then + flux_policy_parent_acquired=true + break + fi + + # An owner that is not ours is a genuine foreign fence, never contention: refuse it + # with the same wording the pre-acquisition check uses, so the two are one thing to + # an operator rather than two. + if flux_policy_parent_has_any_owner; then + rm -f "${reread_error_file}" emit_safe_operation_output "flux-policy-parent-patch" \ "${flux_policy_parent_result_file}" - echo "::error::Could not atomically pause or adopt the parent Flux policy handoff." + refuse_owned_flux_policy_parent return 1 fi - flux_policy_parent_acquired=true + + # Only a rejection whose resourceVersion demonstrably MOVED is contention. A + # rejection at an unchanged resourceVersion is something else entirely -- a + # permission denial, a validation error -- and retrying it would just repeat a + # request the server has already refused on its merits. + reread_resource_version="$(jq -r '.metadata.resourceVersion // ""' \ + "${flux_policy_parent_state_file}")" + if [[ "${reread_resource_version}" == "${resource_version}" ]] || + ((attempt >= max_attempts)) || + ! flux_policy_parent_claim_preconditions_still_hold || + [[ -e "${sync_lease_lost_file}" ]]; then + break + fi + + attempt=$((attempt + 1)) + sleep "${SYNC_INTERVAL}" + done + + rm -f "${reread_error_file}" + if [[ "${flux_policy_parent_acquired}" != "true" ]]; then + emit_safe_operation_output "flux-policy-parent-patch" \ + "${flux_policy_parent_result_file}" + echo "::error::Could not atomically pause or adopt the parent Flux policy handoff." + return 1 fi # New parent reconciliations now stop at spec.suspend. After a mandatory diff --git a/scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go b/scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go index d47b2f569..febc410e0 100644 --- a/scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go +++ b/scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go @@ -508,6 +508,38 @@ func fakeKubectlPatchFluxPolicyParent(args []string, namespace, patchFile string appendEnvFile("OPERATION_LOG", "flux-policy-parent-patch-rejected\n") return commandFailure(56, "%s", rejection) } + // Ordinary Flux controller churn advances the parent's resourceVersion between + // the script's read and its patch, so the CAS test fails against an object that + // is still perfectly claimable. Model exactly that -- advance the stored version + // and reject -- for the first N acquisition attempts. Note this deliberately + // MOVES the version, which is what separates contention from the rejection knob + // above: that one rejects without moving anything, and must never be retried. + if budget := parseInt( + os.Getenv("FAKE_FLUX_POLICY_PARENT_CAS_CHURN_REJECTIONS"), 0, + ); budget > 0 { + fired := parseInt(markerContent("flux-policy-parent-cas-churn-count"), 0) + if fired < budget { + setMarkerContent( + "flux-policy-parent-cas-churn-count", + strconv.Itoa(fired+1), + ) + setMarkerContent( + "flux-policy-parent-resource-version", + incrementDecimal(currentResourceVersion), + ) + if os.Getenv("FAKE_FLUX_POLICY_PARENT_FOREIGN_OWNER_AFTER_CAS_CHURN") == "true" { + setMarkerContent( + "flux-policy-parent-owner", + "fixture-foreign-transaction", + ) + } + appendEnvFile("OPERATION_LOG", "flux-policy-parent-cas-churn:flux-system\n") + return commandFailure( + 56, + "Error from server (Invalid): the server rejected our request due to an error in our request", + ) + } + } owner := patchValueString(patch, "add", ownerPath) if !hasPatchOperation(patch, "test", "/metadata/resourceVersion", currentResourceVersion) || owner == "" || diff --git a/scripts/tests/refresh-flux-ghcr-auth/rollout_convergence_test.go b/scripts/tests/refresh-flux-ghcr-auth/rollout_convergence_test.go index 8bc3a7e1f..27858981e 100644 --- a/scripts/tests/refresh-flux-ghcr-auth/rollout_convergence_test.go +++ b/scripts/tests/refresh-flux-ghcr-auth/rollout_convergence_test.go @@ -1330,6 +1330,113 @@ func TestRejectedFluxParentPatchPreservesSafeDiagnostic(t *testing.T) { } } +// The parent Flux Kustomization's status is rewritten continuously by its own +// controller, so the fence CAS is lost to nothing but ordinary churn -- and, on a +// serialized merge queue, it is lost precisely when the queue is busiest. Losing it +// used to kill the whole production deploy and evict the PR (#3046). +func TestFluxParentFenceRetriesWhenControllerChurnBreaksItsCAS(t *testing.T) { + t.Parallel() + f := newFixture(t) + result := f.runHelper(validConfig(), nil, map[string]string{ + "FAKE_FLUX_POLICY_PARENT_CAS_CHURN_REJECTIONS": "2", + }) + requireSuccessResult(t, result) + operations := readLines(f.operationLog) + // Assert the contention actually HAPPENED, not merely that the run passed: a + // fixture knob that silently stopped firing would make this test vacuous. + if got := strings.Count( + strings.Join(operations, "\n"), "flux-policy-parent-cas-churn:flux-system", + ); got != 2 { + t.Fatalf("CAS rejections = %d, want the 2 the fixture injected", got) + } + // ... and that the fence still landed, released, and let the transaction through. + requireLine(t, operations, "flux-policy-parent-pause:flux-system") + requireLine(t, operations, "root-patch") + requireLine(t, operations, "flux-policy-parent-resume:flux-system") + for _, marker := range []string{ + "flux-policy-parent-owner", "flux-policy-parent-suspended", + } { + if pathExists(filepath.Join(f.syncStateDir, marker)) { + t.Fatalf("converged fence left %s behind", marker) + } + } +} + +// Contention is retried; a foreign owner is not. The retry re-reads before deciding, +// so this is the case where that re-read must change the verdict -- and it must say +// which of the two it found, or an operator cannot tell a busy controller from a +// competing transaction. +func TestFluxParentFenceRefusesAForeignOwnerFoundByTheContentionReRead(t *testing.T) { + t.Parallel() + f := newFixture(t) + result := f.runHelper(validConfig(), nil, map[string]string{ + "FAKE_FLUX_POLICY_PARENT_CAS_CHURN_REJECTIONS": "5", + "FAKE_FLUX_POLICY_PARENT_FOREIGN_OWNER_AFTER_CAS_CHURN": "true", + }) + requireFailureResult(t, result) + output := result.stdout + result.stderr + requireContains(t, output, + "Another transaction already owns the parent Flux policy handoff") + requireNotContains(t, output, "Could not atomically pause or adopt") + operations := readLines(f.operationLog) + // Exactly one: a foreign fence is refused on sight rather than retried against, + // even though the fixture would happily reject four more times. + if got := strings.Count( + strings.Join(operations, "\n"), "flux-policy-parent-cas-churn:flux-system", + ); got != 1 { + t.Fatalf("CAS rejections = %d, want a single refusal on sight", got) + } + for _, unexpected := range []string{ + "flux-policy-parent-pause:flux-system", "flux-policy-pause:infrastructure", + "root-patch", + } { + requireNoLine(t, operations, unexpected) + } + // The foreign owner marker is the fixture's own, but the SUSPEND is not: refusing + // must never have paused anyone else's parent. + if pathExists(filepath.Join(f.syncStateDir, "flux-policy-parent-suspended")) { + t.Fatal("refused foreign fence suspended the parent anyway") + } +} + +func TestFluxParentFenceStillFailsClosedWhenCASRetriesAreExhausted(t *testing.T) { + t.Parallel() + f := newFixture(t) + result := f.runHelper(validConfig(), nil, map[string]string{ + "FAKE_FLUX_POLICY_PARENT_CAS_CHURN_REJECTIONS": "99", + }) + requireFailureResult(t, result) + output := result.stdout + result.stderr + requireContains(t, output, + "Could not atomically pause or adopt the parent Flux policy handoff") + // The last rejection's own text still reaches the operator through the bounded, + // printable helper -- a retry that swallowed the cause would be worse than none. + requireContains(t, output, + "flux-policy-parent-patch: Error from server (Invalid): "+ + "the server rejected our request due to an error in our request") + operations := readLines(f.operationLog) + // Bounded, and bounded at the budget: neither one attempt (no retry at all) nor + // unbounded (a deploy that never gives up is its own outage). + if got := strings.Count( + strings.Join(operations, "\n"), "flux-policy-parent-cas-churn:flux-system", + ); got != 5 { + t.Fatalf("CAS rejections = %d, want the bounded budget of 5", got) + } + for _, unexpected := range []string{ + "flux-policy-parent-pause:flux-system", "flux-policy-pause:infrastructure", + "root-patch", + } { + requireNoLine(t, operations, unexpected) + } + for _, marker := range []string{ + "flux-policy-parent-owner", "flux-policy-parent-suspended", + } { + if pathExists(filepath.Join(f.syncStateDir, marker)) { + t.Fatalf("exhausted CAS retries left %s", marker) + } + } +} + func TestAmbiguousFluxFenceAcquisitionIsAdoptedAndCleaned(t *testing.T) { t.Parallel() for _, test := range []struct { From 066ffe0a3b5f2fc243fbf097259613fd7bace108 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Fri, 18 Sep 2026 03:19:49 +0200 Subject: [PATCH 2/2] fix(deploy): treat an empty re-read diagnostic as a failed copy A parent re-read that exits non-zero without writing stderr leaves the diagnostic file at zero bytes. Copying it then succeeds while the redirection has already truncated the result file, and emit_safe_operation_output skips an empty file, so the operator is told the fence could not be taken but never why. Guard on the file being non-empty as well as on the copy succeeding, and cover both directions: one test drives a silent failure and asserts the fallback line is emitted, the other drives a failure carrying stderr and asserts that text survives verbatim rather than being replaced by the fallback. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/refresh-flux-ghcr-auth.sh | 15 ++++--- .../fake_kubectl_test.go | 13 ++++++ .../rollout_convergence_test.go | 41 +++++++++++++++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/scripts/refresh-flux-ghcr-auth.sh b/scripts/refresh-flux-ghcr-auth.sh index ced010620..701917257 100755 --- a/scripts/refresh-flux-ghcr-auth.sh +++ b/scripts/refresh-flux-ghcr-auth.sh @@ -4804,12 +4804,15 @@ pause_flux_policy_parent() { -o json >"${flux_policy_parent_state_file}" \ 2>"${reread_error_file}"; then # A failed re-read is now the actionable cause, so it replaces the patch - # rejection in the emitted output. The redirection truncates the 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}" \ + # rejection 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 the 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}" \ >"${flux_policy_parent_result_file}" 2>/dev/null; then echo "parent re-read failed; its diagnostic could not be read" \ >"${flux_policy_parent_result_file}" diff --git a/scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go b/scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go index febc410e0..5dff33137 100644 --- a/scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go +++ b/scripts/tests/refresh-flux-ghcr-auth/fake_kubectl_test.go @@ -390,6 +390,19 @@ func fakeKubectlPatchFluxPolicyKustomization(args []string, namespace, patchFile } func fakeKubectlGetFluxPolicyParent(args []string, namespace string) int { + // A re-read that fails after the patch response was lost. The silent variant + // leaves the caller's diagnostic file at zero bytes, so copying it succeeds + // while carrying nothing; the diagnostic variant writes real stderr, whose + // content must survive to the operator. + if markerExists("flux-policy-parent-patch-response-lost") { + diagnostic := os.Getenv("FAKE_FLUX_POLICY_PARENT_REREAD_FAILURE_DIAGNOSTIC") + if diagnostic != "" { + return commandFailure(92, "%s", diagnostic) + } + if os.Getenv("FAKE_FLUX_POLICY_PARENT_REREAD_SILENT_FAILURE") == "true" { + return 92 + } + } if namespace != "flux-system" || (!containsArg(args, "-o") && !containsArg(args, "--output")) { return commandFailure(91, "invalid parent Flux Kustomization lookup") diff --git a/scripts/tests/refresh-flux-ghcr-auth/rollout_convergence_test.go b/scripts/tests/refresh-flux-ghcr-auth/rollout_convergence_test.go index 27858981e..1ef42d28d 100644 --- a/scripts/tests/refresh-flux-ghcr-auth/rollout_convergence_test.go +++ b/scripts/tests/refresh-flux-ghcr-auth/rollout_convergence_test.go @@ -2085,3 +2085,44 @@ func TestRuntimeProbeKubeletRejectionFailsFastAndDistinctly(t *testing.T) { // Kubelet's raw message can carry node detail; it must not reach the log. requireNotContains(t, output, "Node didn't have enough resource") } + +// A parent 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 fence could not be taken. +func TestSilentFluxParentRereadFailureStillEmitsADiagnostic(t *testing.T) { + t.Parallel() + + f := newFixture(t) + result := f.runHelper(validConfig(), nil, map[string]string{ + "FAKE_FLUX_POLICY_PARENT_PATCH_RESPONSE_LOST": "true", + "FAKE_FLUX_POLICY_PARENT_REREAD_SILENT_FAILURE": "true", + }) + requireFailureResult(t, result) + + if !pathExists(filepath.Join(f.syncStateDir, "flux-policy-parent-patch-response-lost")) { + t.Fatal("fixture did not lose the applied fence patch response") + } + requireContains(t, result.stdout+result.stderr, + "flux-policy-parent-patch: parent re-read failed; its diagnostic could not be read") +} + +// 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 TestFluxParentRereadFailureDiagnosticSurvives(t *testing.T) { + t.Parallel() + + const diagnostic = "Error from server (Forbidden): kustomizations.kustomize.toolkit.fluxcd.io is forbidden" + + f := newFixture(t) + result := f.runHelper(validConfig(), nil, map[string]string{ + "FAKE_FLUX_POLICY_PARENT_PATCH_RESPONSE_LOST": "true", + "FAKE_FLUX_POLICY_PARENT_REREAD_FAILURE_DIAGNOSTIC": diagnostic, + }) + requireFailureResult(t, result) + + output := result.stdout + result.stderr + requireContains(t, output, "flux-policy-parent-patch: "+diagnostic) + requireNotContains(t, output, "its diagnostic could not be read") +}