Skip to content

test(openshell): explain failed outage redeploys and stuck namespace deletes - #2940

Merged
freeqaz merged 4 commits into
mainfrom
test/openshell-outage-teardown-diagnostics
Oct 19, 2026
Merged

freeqaz merged 4 commits into
mainfrom
test/openshell-outage-teardown-diagnostics

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Two low-severity flakes in the OpenShell provider-delivery proof (QA advisory, OpenShell cell) fail without saying why:

  • Finding 2017: after the gateway-outage step, the redeploy queued during the outage ended failed_permanent with RUNTIME_LOGIN_FAILED after 4 attempts (main ec746df, run 39246110243; third sighting). The proof printed only the work row and recovery flags. By the time the later refresh checks finished, the failed revision's Pods could be gone.
  • Finding 2018: every step passed, then the teardown's kubectl delete namespace --wait --timeout=120s timed out (Show the next operator where to resume an installation #2905, run 39223713789) and turned the cell red. Nothing said which object held the namespace.

Change

Test-only. Both are diagnostics. Retry behaviour, timeouts and assertions are unchanged.

  • 2018: deleteNamespaceExplained (tests/helpers/kubernetes-real.mjs) runs the same delete. When it fails, it appends to the delete's own error:
    • the namespace's phase, finalizers and status conditions;
    • every object left in it, with its finalizers and deletion time.
    • Objects are listed through a jsonpath template of kind, name, finalizers and deletion time only, so no Secret or ConfigMap data is printed.
    • If discovery or the listing fails (say an unavailable aggregated API, NamespaceDeletionDiscoveryFailure), it carries on with what kubectl printed and names the failure.
    • Each read uses the diagnostic request timeout. The whole listing has a 40 s bound, so the 120 s delete plus the listing stay inside the 180 s teardown step limit.
    • The OpenShell proof's workloadNamespace cleanup uses it.
  • 2017: when the redeploy does not activate, failedRecoveryDiagnostic (tests/helpers/openshell-kubernetes-real.mjs) reads two things right away, each within the diagnostic deadline:
    • the worker's events and persisted state (diagnoseRevision);
    • the existing replacement diagnostic (finding 1532): the revision's Pods and the Gateway, Harness and OpenShell supervisor log tails, redacted and budgeted.
    • The text goes to stderr, to the wait's error if the wait failed, and to the redeployActivated assertion message.

Verification

  • tests/conformance/kubernetes-real-kubectl.test.mjs: 44/44, with three new stubbed-kubectl and stub-reader tests.
    • The namespace delete message, exact: conditions, finalizers, objects, events skipped, every read bounded, and the listing's jsonpath template reads metadata only.
    • Namespace gone. A discovery failure keeps the conditions and lists the kinds kubectl still printed. A listing failure keeps the listed objects and never says "no objects left". An unreadable namespace keeps the delete's error.
    • Recovery diagnostic: appended to the wait error and returned. A worker read that throws or stalls gives describe no worker. A describe that throws gives only the error class. A stalled read gives way to the deadline (50 ms in the test).
  • An internal review found four issues, all fixed: discovery failures were fatal to the listing, raw output could be echoed, the listing could outlast the teardown step, and the worker-timeout value leaked through.
  • Open PRs touching these files gain no new conflicted file. The import placement keeps test(openshell): exercise native provider authority #1982 conflict-free in the integration file.
  • The integration file loads (the real proof skips without its cluster).
  • eslint and prettier are clean on the touched files.

Risks

Low. Test-only, and runs only on failure paths.

For the owner (2017): the runtime logs only that the Codex login failed, not why, since the stderr could echo a credential. So the Harness section shows the failed login phase and the supervisor's DENIED lines, not the provider's text. If the next sighting shows a transient egress or proxy failure right after the gateway restart, there are two fixes:

  • Product: longer retry window for runtime login after a dependency outage (the worker's backoff is about 15 s, finding 887).
  • Test: wait for OpenShell readiness beyond the StatefulSet before expecting activation.

… times out

Finding 2018: the OpenShell provider-delivery cell passed every step and
then went red in teardown because kubectl delete namespace --wait timed
out after 120 s, with nothing saying which object held the namespace.

deleteNamespaceExplained runs the same delete and, when it fails, adds
the namespace's phase, finalizers and status conditions plus every object
left in it (kind/name, finalizers, deletion time) to the delete's own
error. Each read is bounded by the diagnostic request timeout and the
whole listing by the diagnostic deadline. The delete itself, its timeout
and the teardown step limit are unchanged.
…ailed

Finding 2017: after the OpenShell gateway outage, the redeploy queued
during it ended failed_permanent with RUNTIME_LOGIN_FAILED after 4
attempts. The proof printed only the work row and recovery flags, so the
login failure's cause was unknown, and by the time the later refresh
checks finished the failed revision's Pods could be gone.

When the redeploy does not activate, failedRecoveryDiagnostic reads, at
once, the worker's events and persisted state (diagnoseRevision) and the
existing replacement diagnostic (the revision's Pods and the Gateway,
Harness and OpenShell supervisor log tails, redacted and budgeted). Each
read is bounded by the diagnostic deadline; a failed or stalled read only
says so. The text goes to stderr, to the wait's error when the wait
failed, and to the redeployActivated assertion message. Retry behaviour
and the assertions are unchanged.
@freeqaz
freeqaz requested a review from a team as a code owner October 19, 2026 13:30
@clawsweeper

clawsweeper Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

…removes

Keeps the open native-authority PR (#1982) free of a new conflict in this
file: it deletes the neighbouring OPENSHELL_KUBERNETES_WORKLOAD_IDENTITY
import.
…es and print metadata only

Review fixes for findings 2017 and 2018:
- An unavailable aggregated API makes kubectl api-resources exit 1 while
  still printing the other kinds (NamespaceDeletionDiscoveryFailure, a
  common cause of a stuck namespace). The listing now goes on with them
  and keeps the namespace's phase and conditions.
- Objects are listed through a jsonpath template of kind, name,
  finalizers and deletion time, so a Secret's or ConfigMap's data is
  never printed, and raw output is never echoed.
- The listing has its own 40 s bound, so the 120 s delete plus the
  listing stay within the 180 s teardown step limit.
- A stalled worker read in the recovery diagnostic now gives describe no
  worker instead of the deadline's default text.
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 19, 2026
@clawsweeper

clawsweeper Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR adds bounded diagnostics for failed OpenShell outage recovery and namespace deletion in the real-cluster test harness.

Example: Deleting namespace oce-held times out because a Sandbox retains openshell.io/sandbox.

  • Before: The test reports the namespace deletion timeout.
  • After: The original timeout also includes namespace conditions and remaining objects with their finalizers.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Useful and focused, with no concrete defect found; confidence in cluster-specific diagnostic output remains limited to source inspection and supplied unit verification.
Proof confidence 🦐 gold shrimp (3/6) This maintainer-authored, test-only PR is outside the ordinary contributor proof gate; supplied stubbed-reader verification supports diagnostic behavior but establishes no live Kubernetes failure-path result.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Test only · Worth it: Yes · Fix scope: Complete
User problem: Contributors investigating failed OpenShell QA cells lack timely evidence identifying failed recovery or objects blocking namespace deletion.
Reason: The maintainer-authored direction addresses concrete diagnostic gaps without broadening runtime behavior or weakening acceptance criteria.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it adds useful failure diagnostics without weakening assertions, and no actionable correctness or security defect was found.

Priority: P3
Reviewed head: 2cad0e5f21bc65f03eab74a71acd0527868d62c5

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: Supplied verification does not exercise either new diagnostic against a real cluster or demonstrate the diagnostic regression failing on the base and passing on the head.
Agent review details

How this fits together

The OpenShell integration harness drives OCC and Kubernetes workflows; these failure paths read worker state and cluster metadata and append diagnostic text to existing test failures.

flowchart TD
  A[OpenShell integration test] --> B[Outage recovery diagnostic]
  A --> C[Namespace cleanup diagnostic]
  B --> D[Worker state and runtime logs]
  C --> E[Kubernetes object metadata]
  D --> F[Existing test failure]
  E --> F
Loading

Technical review

Best possible solution:

Retain the focused diagnostics through the existing test owners while leaving investigation of the underlying runtime and deletion failures separate.

Do we have a high-confidence way to reproduce the issue?

Current-main source confirms that namespace deletion lacks remaining-object diagnostics and outage recovery lacks the immediate replacement diagnostic; the reported cluster failures were not independently reproduced.

Is this the best way to solve the issue?

The implementation appropriately reuses existing deadlines, redaction, and replacement diagnostics while preserving failure outcomes and avoiding product changes.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 7afa66cb5532.

Provenance checked

  • Namespace teardown keeps the original intent (28207c6: Bound cleanup steps and stop the worker before deleting resources it uses; the PR retains that ordering and the same delete operation.)
  • Gateway-outage recovery assertions keeps the original intent (f1bcaa9: Prove deployments remain queued during gateway failure and activate after recovery; added diagnostics preserve both expectations.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

Likely related people:

  • Free Wortley: Raw commit 42fb0a9 adds tests/helpers/kubernetes-real.mjs:77 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 42fb0a903372; files: tests/helpers/kubernetes-real.mjs)

Labels

Label changes:

  • add P3: This test-harness improvement makes existing failures easier to investigate without changing product behavior.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P3: This test-harness improvement makes existing failures easier to investigate without changing product behavior.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 19, 2026, 9:56 AM ET / 13:56 UTC.

@freeqaz
freeqaz merged commit b891f54 into main Oct 19, 2026
81 checks passed
@freeqaz
freeqaz deleted the test/openshell-outage-teardown-diagnostics branch October 19, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant