✅ test(e2e): prove mqweb outage recovery end to end (R2 / REQ-REL-2026-08 AC2)

[konih]

Why

PR #186 (REQ-REL-2026-08) fixed a P0 wedge: a workload CR sat at Synced=False for 23h after a
queue-manager restart because a non-transient-looking error returned (ctrl.Result{}, nil) — no
requeue, no log, and nothing left to re-enqueue it. The independent review of that PR raised
finding R2: no outage-recovery e2e. The spec's AC2 —

Given mqweb is unreachable for longer than one reconcile deadline, when it recovers, then every
affected CR returns to Synced=True Available within one retry interval, and mqObjectExists=true
matches the real MQ state.

— was only ever proven by unit and envtest coverage of the error classification. Nothing exercised
a real outage against a real queue manager. This PR closes that gap.

What changed

Test-only. internal/ and cmd/ are untouched.

• test/e2e/mq_outage_e2e_test.go (new) — one Serial spec, Label("mq", "mq-outage"):
  1. Healthy baseline: QueueManagerConnection Ready, Queue Synced=True, maxdepth=1000
     verified on the queue manager over mqweb.
  2. Outage: the IBM MQ StatefulSet is scaled to zero and the spec confirms mqweb no longer answers.
  3. A Queue spec change (maxdepth=2000) is applied while mqweb is down — this is the
     trigger for the failing reconcile, and the last time anything touches the CR.
  4. Asserts Synced=False with reason Error, never TerminalError. This is the regression
     itself: a transient outage misclassified as terminal is the 23h wedge.
  5. Recovery: the StatefulSet is scaled back up. That is the only action — no CR edit, no
     annotation, no operator restart.
  6. Asserts the Queue returns to Synced=True / Available with mqObjectExists=true, that
     metadata.generation is unchanged across recovery (so it self-healed rather than being
     re-applied), that the connection is Ready again, and that the queue manager really carries
     maxdepth=2000.
• test/e2e/mq_outage_helpers.go (new) — outage induce/restore helpers plus an independent
  mqweb reachability probe that builds a fresh client per call, so a pooled keep-alive connection
  can never make an outage look healthy.
• test/e2e/namespace_helpers.go — adds mkurator-e2e-outage so the spec's CRs are isolated
  from specs that assume a healthy queue manager, and are covered by the existing suite cleanup.

Two deviations from the original brief, both grounded in the code

Both are documented in a comment at the top of the spec.

1. The QMC does not go not-Ready during an outage. QueueManagerConnectionReconciler.fail()
   returns early — keeping Ready=True and requeueing — when the connection is already Ready at the
   observed generation and the error is transient. That is deliberate flap suppression. The spec
   therefore asserts QMC readiness only after recovery.

2. TerminalRetryInterval is not the path under test. A dial failure against an absent mqweb is
   wrapped by mqrest.roundTrip as mqadmin.TransientError, so the self-heal path is the transient
   one that REQ-REL-2026-08 changed: setSyncedError → TransientRequeueInterval (30s), and/or the
   workload watch on QueueManagerConnection readiness, whichever fires first. AC2 only requires
   recovery "within one retry interval", which both paths satisfy — the spec asserts the outcome and
   pins the classification (retryable Error, never TerminalError) rather than pinning which
   trigger wins.

   Correction (independent review): an earlier draft of this section claimed the watch fan-out
   "most likely won that race" in the CI run. That is wrong, and the source contradicts it:
   connectionWatchPredicates() (reconcile_shared.go:513-520) fires only on connectionReadyChanged
   or a generation change, and during the outage the QMC never reconciles or writes status — its
   success path returns a bare ctrl.Result{} and fail() early-returns without a status write — so
   no QMC update event existed at all. The observed timeline is fully explained by the plain transient
   requeue. The QMC-ready watch path is therefore NOT covered by this spec; do not read it as
   guarded here.

What to review

• Is scaling the IBM MQ StatefulSet acceptable as the outage mechanism? Breaking the Service
  selector or adding a NetworkPolicy was rejected: both leave already-established keep-alive
  connections usable, so a reconcile reusing an idle connection would still succeed and the spec
  would prove nothing. Removing the pod severs every connection and takes the listener away.
• Is the induce-by-edit acceptable? The edit happens before recovery, purely to force a
  reconcile against an unreachable mqweb. The passive alternative (waiting for the drift resync) was
  rejected because DriftResyncAfter jitters between 5 and 10 minutes. The metadata.generation
  assertion proves nothing touched the CR during recovery.
• Label choice: mq but deliberately not slow, so it runs on every PR (the PR filter is
  (smoke || mq) && !slow). It costs a few minutes per run. If you would rather keep PR e2e lean,
  adding slow is a one-word change — but then the regression is only guarded post-merge.

Test plan — stated plainly

Gate	Result
task test:run (unit + envtest, race, coverage)	pass — 92.7% internal/, 92.5% api/
task lint + task arch:lint	pass — 0 issues
task verify	pass
task format:check	pass
go vet -tags e2e ./test/e2e/	pass
go test -tags e2e -c ./test/e2e/ (tagged build)	pass
golangci-lint --build-tags e2e on the new files	no new findings (only the package-wide Ginkgo dot-import notice)
KURATOR_E2E_MQ=1 task test:e2e locally	not executed — see below
e2e (kustomize) on this PR	pass — 19 of 22 specs, 0 failed, the new spec among them

The e2e suite could not be run on the authoring host. task cluster:up brings the kind cluster up
fine, but the IBM MQ Helm chart pins
nodeAffinity: kubernetes.io/arch in [amd64, s390x, ppc64le], and the host is Apple Silicon
(arm64) — ibm-mq-0 stays Pending with 0/1 nodes are available: 1 node(s) didn't match Pod's node affinity/selector, and the Helm release times out. The proof is therefore this PR's own E2E
workflow on ubuntu-latest (amd64), which was watched to completion. Verbatim from that run:

STEP: scaling the IBM MQ workload to zero so mqweb becomes unreachable
STEP: applying a Queue spec change while mqweb is unreachable
STEP: expecting Synced=False with the retryable reason Error (never TerminalError)
  False|Error|mqweb request failed: Post "https://ibm-mq.ibm-mq.svc:9443/ibmmq/rest/v3/admin/
  action/qmgr/QM1/mqsc": dial tcp 10.96.178.48:9443: connect: connection refused
STEP: scaling the IBM MQ workload back up
STEP: expecting the Queue to self-heal to Synced=True with no CR edit, annotation, or restart
  True|Available|Queue matches spec        mqObjectExists: true
STEP: confirming recovery was unattended: the Queue spec was never touched after the outage
  generation 2 before recovery, 2 after
STEP: confirming the connection recovered on its own
  True|mqweb connection is healthy
STEP: confirming mqObjectExists=true matches the real queue manager state

Ran 19 of 22 Specs ... SUCCESS! -- 19 Passed | 0 Failed. The outage was real (a refused dial, not
a simulated condition), and the whole spec cost ~57s wall clock.

Risks

• The spec takes the shared queue manager down. It is Serial, and it restores the workload and
  waits for mqweb to answer again in DeferCleanup before anything else runs, including on failure.
• Queue-manager restart time dominates the runtime (~15s on CI, since the PVC stays warm). The restore wait reuses the suite's most
  generous existing helper (qmcWatchRecoveryEventuallyTimeout, 8m) rather than a new constant; the
  recovery assertion uses mqSyncedEventuallyTimeout (3m), which comfortably covers the mqrest
  circuit breaker's 30s open window plus a 30s transient requeue.
• The StatefulSet name is discovered rather than hard-coded, and overridable via
  KURATOR_E2E_MQ_STATEFULSET.

Out of scope

• Any change to internal/, cmd/, docs/, or deploy manifests. Shortening
  --terminal-retry-interval for tests would have needed manifest changes owned elsewhere and is
  not required, since the path under test is the 30s transient requeue.
• ADR-0014 wording. The ADR's consequence "transient outages self-heal without manual CR edits when
  MQ returns" is now backed by a test; no text change needed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[konih]

Independent review — APPROVE (head 0eb4cf5)

Reviewed by a reviewer that neither authored the spec nor wrote the lane brief. No P0/P1/P2.

The test is not vacuous — verified from source, not from the author's claims

• The outage is real. StatefulSet scaled to 0, confirmed by an independent fresh-client probe; CI shows a genuine dial tcp 10.96.178.48:9443: connect: connection refused.
• The non-vacuity gate is genuinely blocking (mq_outage_e2e_test.go:108-115) — an Eventually requiring status=="False" and reason=="Error". A no-op outage times out rather than passing.
• Recovery is unattended. No CR is touched between outage and recovery — only scale --replicas=1.
• Assertions are meaningful: unchanged metadata.generation (2 → 2), mqObjectExists=true, and a live mqweb GetQueue asserting maxdepth=2000.
• The spec really executed: [e2e] SPEC START/PASS ... [mq, mq-outage, Serial] 09:01:38 → 09:02:35 (55.6s), inside Ran 19 of 22 ... SUCCESS!.

The load-bearing result — a mutation proof the reviewer derived independently: connectionWatchPredicates() fires only on QMC ready-change/generation-change; the QMC never reconciles or writes status during the outage; workloadReconcilePredicates() needs a generation bump. So if setSyncedError regressed to (ctrl.Result{}, nil) on transient errors, nothing would re-enqueue the Queue and the recovery Eventually would time out. That is what makes the absent TDD red step acceptable for a regression e2e — the spec provably fails if the wedge returns.

Both corrections the author made to my (the coordinator's) lane brief are confirmed in source: flap suppression at queuemanagerconnection_controller.go:127-130, and dial failures wrapped as mqadmin.TransientError at mqrest/resilience.go:190-193.

Label question — recommendation: keep mq, do NOT add slow

This guards precisely the requeue-chain-aliveness class that unit/envtest claimed to cover and didn't — which is why R2 was filed in the first place. 57s on a 346s suite is proportionate, and post-merge-only guarding would let the wedge be reintroduced and merged.

P3 findings (recorded, none blocking)

1. PR description overstated the trigger — claimed the watch fan-out "most likely won that race". Source contradicts it; no QMC update event existed during the outage, so the plain transient requeue explains the timeline. The QMC-ready watch path is not covered by this spec. ✅ Corrected in the PR body (description-only edit; the reviewed diff is unchanged).
2. mq_outage_e2e_test.go:140-141 — By("confirming the connection recovered on its own") asserts a condition that never left Ready=True, so it is effectively a no-op; the By text overstates what is proven.
3. mq_outage_e2e_test.go:134 — the 3m mqSyncedEventuallyTimeout admits both the 30s transient requeue and the 2m terminal backstop, so AC2's "within one retry interval" is not tightly bound; a regression demoting recovery to the terminal backstop would still pass. Tightening trades real coverage for flake risk, hence a note.
4. mq_outage_e2e_test.go:45 — Serial but not pinned last; seven MQ specs ran after it, so the shared QM restart lands mid-suite. Mitigated (restoreMQWeb blocks on readyReplicas==1 and an authenticated Ping, and the next spec re-verifies its MQSC prerequisite), but spec-ordering drift is unguarded.

Flake risk assessed low by construction — the 3m recovery clock starts only after restoreMQWeb confirms a successful authenticated Ping, so cold-start sits outside the window — with the caveat that the empirical sample is N=1.

2–4 are logged to the INBOX rather than fixed here: each is a test-file edit that would invalidate this APPROVE and cost another review round for no correctness gain.

---

Ready for maintainer merge.