diff --git a/.claude/skills/harbor-dev/README.md b/.claude/skills/harbor-dev/README.md index 9c765bfd..1ea6857c 100644 --- a/.claude/skills/harbor-dev/README.md +++ b/.claude/skills/harbor-dev/README.md @@ -17,7 +17,8 @@ Harbor Dev is the conversational layer over `seictl network` + `seictl node`: an - Translates plain-English intent ("give me 4 validators on seid sha=abc, then an RPC fleet") into `seictl` invocations, so the engineer never hand-rolls SeiNetwork / SeiNode YAML, preset wiring, or peer selectors. - Defaults to GitOps: renders CRs via `--dry-run`, writes them under `engineers///`, opens a PR, and lets Flux apply on merge — direct apply is a rare, double-confirmed escape hatch. -- Covers the full daily-driver surface: onboarding, chain spinup, RPC fleets, single and comparative benches, status reads, and `git rm`-based teardown. +- Covers the full daily-driver surface: onboarding, chain spinup, RPC fleets, single and comparative benches, status reads, and PR-based teardown. +- Gates teardown on the field that leaks disks. A `SeiNetwork` deleted under its default `spec.deletionPolicy: Retain` orphans the validator SeiNodes it generated — they keep running, keep their PVCs, and keep their EBS volumes, and nothing in git or in Flux will ever remove them. The skill reads the policy, patches it to `Delete` before the removal merges, verifies against the engineer's own Flux Kustomization rather than `flux-system`, and polls the resources to gone. - Refuses the boundary that matters: harbor-only (never prod), `eng-`-only (no cross-tenant work), and it never silently works around a missing prereq — it surfaces the next step and halts. - Gates both paths that wipe a node's chain data, and neither is ever volunteered. `seictl workflow state-sync` is the paved road — it re-bootstraps or migrates an existing node's store, always sign-off-and-`--dry-run`-first, never run against a shared or long-lived follower without escalating to its owner. A mutating `seictl task submit` is the escape hatch: it reaches the same wipe straight through one pod's sidecar with none of the recipe's holds, so it carries the stricter gate. diff --git a/.claude/skills/harbor-dev/SKILL.md b/.claude/skills/harbor-dev/SKILL.md index f359f980..feafb3af 100644 --- a/.claude/skills/harbor-dev/SKILL.md +++ b/.claude/skills/harbor-dev/SKILL.md @@ -28,6 +28,7 @@ The hard rules: 9. **Two paths wipe a node's chain data — gate both.** `seictl workflow state-sync` is the destructive **paved road**; a mutating `seictl task submit` is the destructive **escape hatch**. Neither is ever the default, and the agent volunteers neither. - **`seictl workflow state-sync`** re-bootstraps an existing node by wiping its local chain state (an `rm -rf` on that node's data), optionally with an irreversible `--migration GigaStore --backend ` store change — both tokens are required together, never `--migration` alone. Require explicit engineer sign-off before the non-dry-run apply, verify the target node against the live cluster first, `--dry-run` to inspect, and escalate to the owner — never wipe on agent initiative — for any shared or long-lived `pacific-1`/`atlantic-2` follower. Never commit a workflow CR to the Flux workspace repo (a one-shot, spec-immutable request object; force-delete recovery fights Flux). Full gate in `references/seictl-cli.md` → `seictl workflow state-sync`. - **`seictl task submit`** POSTs a raw task straight to one pod's sidecar, and the accepted types include `reset-data`. Submitted that way the wipe runs with **none** of the recipe's protections — no `mark-not-ready` hold, no `stop-seid` first, no ordering, and no adoption pointer telling the controller the node is occupied — so it is strictly more dangerous than the paved road, not a lighter-weight version of it. Prefer `workflow state-sync` for anything the recipe covers; a mutating `task submit` requires explicit sign-off naming node, namespace, and task type. `task get` / `task list` are reads and safe. Full gate in `references/seictl-cli.md` → `seictl task`. +10. **Never tear down a SeiNetwork before reading its `spec.deletionPolicy`.** It defaults to `Retain`, and a `Retain` deletion strips the owner reference from every generated validator SeiNode instead of deleting it. The orphans keep running, keep their PVCs, and keep their EBS disks, with nothing left in-cluster that will ever remove them — the teardown reports success while the spend continues. Land `deletionPolicy: Delete` **in git** and confirm it reconciled onto the live object **before** the removal merges — a bare `kubectl patch` on a Flux-owned SeiNetwork is drift the next reconcile reverts, typically while the removal PR is still in review. After the parent SeiNetwork is gone, nothing restores the cascade and the cleanup is manual. Full procedure and the leaked-resource sweep: `references/teardown.md`. ## Mental model @@ -130,7 +131,8 @@ Every engineer-facing intent maps to a `seictl network`, `seictl node`, or (for | "Attach a node via state sync" / "add an RPC node without replaying from genesis" / "bootstrap from my own chain" | Same follower shape plus a `spec.fullNode.snapshot` block: `stateSync: {}` + ≥2 `rpcServers` witness endpoints from the engineer's own chain (replaces the platform syncer registry — self-service, no platform PR). **Read `references/state-sync-bootstrap.md` first — its known-issue banner governs: state-sync bootstrap is currently broken on ceremony-fresh eng chains (PLT-794, genesis carries no validator set); genesis replay is the working path until it lands.** Witnesses verify the trust point; snapshot chunks come over p2p from peers that have actually produced a snapshot. | | "What's running in my namespace" / "what chains do I have" | `seictl network list -n eng-` for the chains + `seictl node list -n eng-` for the followers (yaml default; `-o name` for short, `-o jsonpath=...` for one-shot field reads). | | "Show me chain X" / "what's the status of X" | `seictl network get -n eng-` for the network (`.status.phase`); `seictl node get -rpc- -n eng-` for a follower (`.status.phase`, `.status.endpoint`). | -| "Tear down chain X" | `git rm -r engineers///` **and** remove `` from `engineers//kustomization.yaml`'s `resources:` list (Kustomize fails to render with a missing-resource entry). Commit → push → merge. Flux prunes the SeiNetwork + SeiNodes on next reconcile, which cascades to pods / PVCs per k8s deletion propagation. **Teardown does NOT purge the chain-id's S3 genesis artifacts — the chain-id is burned**; a later respin must use a fresh chain-id (see the naming step) or purge the `/` genesis-bucket prefix. See `bench:teardown` recipe in `references/cluster-inspection-recipes.md` for the bench-specific variant. | +| "Tear down chain X" / "delete my bench" | **PR-based** (see Procedure: tear down). Read `spec.deletionPolicy` on every SeiNetwork in the task dir **first** — on `Retain` (the default) the teardown orphans the validators and leaks their disks. Then `git rm -r engineers///` **and** remove `` from `engineers//kustomization.yaml`'s `resources:` list (Kustomize fails to render with a missing-resource entry). Commit → push → merge → reconcile `` → poll until the objects are gone. **Teardown does NOT purge the chain-id's S3 genesis artifacts — the chain-id is burned**; a later respin must use a fresh chain-id (see the naming step) or purge the `/` genesis-bucket prefix. | +| "Destroy my namespace" / "clean out eng-" / "where is my harbor spend going" | `references/teardown.md` → *empty or remove my namespace*, and the leaked-resource sweep. A workspace PR removes the engineer's workloads; the `Namespace` object, the ServiceAccounts, the RBAC, and the Flux Kustomization are platform-owned and stay. Orphaned SeiNodes from an earlier `Retain` teardown are in no repo at all and need the manual sweep. | | "Run a load test against chain X" / "bench it" / "stress test chain X" | **PR-based bench** (see Procedure: spin up a load test). Live-fetch chain rpc per-pod URLs, substitute into the profile JSON, render Job + ConfigMap from the templates in `references/sei-load-bench.md`, open a PR against `harbor-engineering-workspace` at `engineers//bench-/`. Merge → Flux applies → seiload runs → uploader sidecar pushes results to S3. | | "Compare PR 3399 to main on sei-chain" / "bench A against B" / "diff the perf of these two commits" | **PR-based comparative bench** (see Procedure: comparative bench). Renders two ephemeral chains (each running its own seid image) + two sei-load Jobs (identical profile + duration) into a single PR. After merge, watches both chains in parallel, polls both Jobs to terminal, fetches both reports from S3, and surfaces a side-by-side metrics table (TPS / latency / success rate / errors). Lives at `engineers//compare-/`. | | "Where am I" / "what cluster am I on" / "who am I" | `kubectl config current-context` + `aws sts get-caller-identity --profile ` (the gate-2 profile). (No dedicated `seictl context` verb in this surface.) | @@ -148,26 +150,37 @@ Force-reconcile proactively after a relevant PR merges; don't wait for Flux's na **Trigger:** any PR merge the agent helped open against `sei-protocol/platform` or `sei-protocol/harbor-engineering-workspace`, OR when the engineer says "merged" / "I merged X". Don't wait for the engineer to ask. -**Preferred:** +**Which Kustomization depends on which repo — they are not interchangeable.** A workspace-repo merge (every chain, bench, and teardown) is applied by the Flux `Kustomization ` in `eng-`, which watches `harbor-engineering-workspace` at `./engineers/`. The root `flux-system` Kustomization tracks `sei-protocol/platform` at `clusters/harbor`, so reconciling it after a workspace merge reconciles a different repo and reports success without applying the engineer's change. Target `flux-system` only after a **platform**-repo merge (onboarding). + +| Merged PR against | Target Kustomization | In namespace | +|---|---|---| +| `sei-protocol/harbor-engineering-workspace` | `` | `eng-` | +| `sei-protocol/platform` | `flux-system` | `flux-system` | + +**Preferred** (workspace-repo merge): ```sh -flux --context harbor reconcile kustomization flux-system --with-source -n flux-system +flux --context harbor reconcile kustomization -n eng- --with-source ``` **Fallback** (only when `flux` isn't available): ```sh -kubectl --context harbor -n flux-system annotate kustomization flux-system \ +kubectl --context harbor -n eng- annotate kustomization \ reconcile.fluxcd.io/requestedAt="$(date +%s)" --overwrite ``` +`--with-source` also reconciles the referenced `GitRepository`. If that source sits outside `eng-`, the engineer's namespace-scoped Role returns `Forbidden` — drop the flag and reconcile the Kustomization alone, which applies the revision the source has already fetched. + **Verify** the merge commit SHA landed before proceeding to verbs that depend on the new state: ```sh -kubectl --context harbor -n flux-system get kustomization flux-system \ +kubectl --context harbor -n eng- get kustomization \ -o jsonpath='{.status.lastAppliedRevision}' ``` +A reconcile is not proof the change took effect. On a teardown, follow it with the disappearance poll in `references/teardown.md` — Flux reports success the moment it issues the deletes, well before finalizers release the PVCs. + ## Procedure: spin up an ephemeral chain (the headline — PR-based) Engineer says "spin up a chain of 4 validators with seid sha=abc, then add an RPC fleet." This is the daily-driver flow. The skill renders a SeiNetwork CR (and N SeiNode CRs for the fleet) via `seictl network apply --dry-run` / `seictl node apply --dry-run`, writes them to `engineers///` in `harbor-engineering-workspace`, opens a PR. Engineer merges → Flux applies → agent watches the network to Ready and each follower to Running → reports endpoints. @@ -193,7 +206,7 @@ Engineer says "spin up a chain of 4 validators with seid sha=abc, then add an RP 8. **Open the PR** — title: `feat(eng/): spin up `; body lists chain-id, image digest, preset(s), expected endpoints. `gh pr create --repo sei-protocol/harbor-engineering-workspace --base main`. 9. **Surface and halt** — engineer reviews and merges. Surface the PR URL with: "after merge, Flux reconciles in ~60s; ping me to watch the network to Ready and report endpoints." 10. **After merge — watch** — `seictl network watch --until=Ready --timeout=15m -n eng-` for genesis (the network reaches `Ready`), then per follower `seictl node watch -rpc- --until=Running --timeout=15m -n eng-` (a node reaches `Running` — **never `Ready`**, which is illegal for a node and errors at parse). NDJSON stream; exits 0 on the matched phase. Halt on non-zero (`metav1.Status` on stderr — `jq -r .reason` discriminates Timeout vs terminal Failed phase vs API failure). -11. **Report** — use the canonical inspection recipes from `references/cluster-inspection-recipes.md` rather than inferring jsonpath at runtime. Recipe #1 returns the fleet's RPC endpoints (target these for any load tools — never validators, which serve no EVM). Recipe #4 lists the network + its follower SeiNodes with phase + readiness in one shot. Plus teardown: `git rm -r engineers///` and remove the `` entry from `engineers//kustomization.yaml`'s `resources:` list, then commit → push → merge (Flux prunes the SeiNetwork + SeiNodes and cascades the deletion to pods/PVCs). +11. **Report** — use the canonical inspection recipes from `references/cluster-inspection-recipes.md` rather than inferring jsonpath at runtime. Recipe #1 returns the fleet's RPC endpoints (target these for any load tools — never validators, which serve no EVM). Recipe #4 lists the network + its follower SeiNodes with phase + readiness in one shot. Plus teardown: point at **Procedure: tear down** rather than restating it — the `deletionPolicy` gate runs before the `git rm`, and skipping it leaks the validators' disks. ### Escape hatch: direct `seictl network|node apply` (rare; engineer asks twice) @@ -240,7 +253,24 @@ Engineer says "compare PR 3399 to main on sei-chain" or "bench latest sei-chain 13. **Watch follower fleets in parallel** — per follower on each side, `seictl node watch -{a,b}-rpc- --until=Running --timeout=15m` (a node's terminal is `Running`, not `Ready`). 14. **Poll both bench Jobs to terminal** — single loop that checks both `seiload--{a,b}` for `Complete=True` or `Failed=True`. Deadline: ` * 60 + 660` seconds. 15. **Fetch + render** — pull both reports via in-cluster `kubectl run` under `engineer-service-account` (the engineer's local SSO profile lacks `s3:GetObject` on the prefix). Locate sei-load's summary block in each report; surface both summaries verbatim under a delta table that highlights canonical metrics (TPS, P50/P99 latency, success rate, tx counts) with `better` / `worse` verdicts. On summary-block miss, fall back to last-50-lines format with both S3 paths. -16. **Teardown guidance** — `git rm -r engineers//compare-/` and remove the entry from `engineers//kustomization.yaml`'s `resources:`. Flux prunes both SeiNetworks + all follower SeiNodes + two Jobs; child pods/PVCs cascade. +16. **Teardown guidance** — run **Procedure: tear down** against `engineers//compare-/`. **Both** SeiNetworks need the `deletionPolicy` gate before the removal PR; a comparison leaks two validator pools' disks, not one. + +## Procedure: tear down (PR-based) + +Engineer says "tear down chain X," "delete my bench," or "clean out my namespace." Teardown goes through a PR against `sei-protocol/harbor-engineering-workspace`, the same contract as spinup — never a bare `seictl network|node delete` against a Flux-owned CR, which the next reconcile re-applies. + +**Read `references/teardown.md` first.** It carries the `deletionPolicy` trap in full, the namespace procedures, and the sweep for resources that already leaked. + +1. **Pre-flight** — five gates. Halt on first failure. +2. **Inventory what goes away** — `kubectl get seinetwork,seinode -n eng- -l sei.io/seinetwork=` plus `kubectl get pvc -n eng-`. Show the engineer the list before touching anything. +3. **Gate on `deletionPolicy`** — `kubectl get seinetwork -n eng- -o jsonpath='{.spec.deletionPolicy}'` for every SeiNetwork in the task dir. `Retain` or empty means halt: removing the manifest orphans the generated validators and leaks their EBS disks (Guardrail #10). +4. **Set it to `Delete` in git, then confirm both** — a policy PR that sets `deletionPolicy: Delete` on the SeiNetwork manifest, merged and reconciled, before the removal PR merges. Both the committed file and the live object must read `Delete`. **A bare `kubectl patch` is not enough on a Flux-owned SeiNetwork**: git still declares `Retain`, so the next reconcile reverts the patch, often mid-review, and the removal then merges under `Retain` anyway. The patch is a repair for a SeiNetwork no reconcile owns. Full ordering in `references/teardown.md`. +5. **Remove the manifests** — `git rm -r engineers///` **and** remove the `` entry from `engineers//kustomization.yaml`'s `resources:` list. Both edits, or Kustomize fails to render and Flux applies nothing. +6. **Commit + push** — branch `feat/eng--teardown-`. Message: `feat(eng/): tear down — chain-id=`. +7. **Open the PR** — title `feat(eng/): tear down `. Body names the chain-id, every CR that goes away, and the `deletionPolicy` value the SeiNetwork now carries. Surface the URL and halt for the merge. +8. **After merge — reconcile the workspace Kustomization** — `flux --context harbor reconcile kustomization -n eng- --with-source`, then compare `.status.lastAppliedRevision` to the merge SHA. Reconciling `flux-system` here verifies the wrong repo (see Post-merge reconciliation). +9. **Poll until the resources disappear, and distinguish three outcomes** — a reconcile only says Flux issued the deletes. Deletion is asynchronous and finalizers hold objects in `Terminating` while the controller releases PVCs, so poll on a budget (~5 min) rather than asserting once. Report exactly one of **`GONE`** (the API answered and matched nothing), **`PRESENT`** (objects remain at the deadline), or **`UNVERIFIED`** (the API call failed). **A failed read is never a pass** — a `Forbidden` or dropped connection returns zero lines, so a check that counts lines without reading `kubectl`'s exit status claims success precisely when it cannot see the cluster. Use `poll_gone` / `expect_present` / `record` from `references/cluster-inspection-recipes.md` recipe #9 — one implementation, called, never re-typed. **Aggregate the outcomes**: a bare call discards its return code, so an early `UNVERIFIED` followed by a clean later read leaves the whole check exiting 0, which is the original bug on the exit-code path. Poll the chain's controller-managed claims **by name** from the step-2 inventory, and assert the imported ones separately — a namespace-wide PVC sweep also matches imported claims and other chains', so a correct teardown reports `PRESENT`. +10. **Report** — what is gone, what remains, and the burned chain-id. An object still `Terminating` past the budget is a real finding: surface the finalizer and the controller log line; never strip a finalizer to make the check pass. An `UNVERIFIED` result is reported as unknown, never as done. ## Procedure: troubleshooting (manual) @@ -276,10 +306,13 @@ Stop and report to the user if: - **SeiNode Pending with `StateSyncReady=False/NoSyncersConfigured` and no StatefulSet.** A snapshot-bootstrap node whose witnesses can't be resolved — the controller holds StatefulSet creation until the gate opens (deliberate; prevents a stranded Pending pod). The condition message names both remediations: declare ≥2 `spec.fullNode.snapshot.rpcServers`, or drop `stateSync` and genesis-replay. Don't delete/recreate the pod or chase storage — the block is upstream. See `references/state-sync-bootstrap.md`. - **Followers not Running when rendering a bench.** The bench requires per-follower RPC URLs, which populate only once each SeiNode reaches `Running`. Halt and offer to poll (`seictl node watch -rpc- --until=Running -n eng-`) before continuing. - **Follower `.status.endpoint` present but dial refused.** `Running` means config applied + sidecar self-marked ready, NOT that the EVM listener is accepting connections — there is a real post-Running window. Before driving load, run `seictl node watch --until=caught-up -n eng-` (the SDK readiness gate: height>1 with `catching_up=false`, plus EVM serving when the node publishes an EVM endpoint). Halt with the follower's full status if it stays refused. -- **Comparative bench: one side reaches Ready, the other Failed.** The comparison is invalid against half a setup. Surface the failed side's `.status.plan.failedTaskDetail.error`; offer to teardown the Ready side via `git rm` against just that sub-dir + commit. Don't run the bench against half a comparison. +- **Comparative bench: one side reaches Ready, the other Failed.** The comparison is invalid against half a setup. Surface the failed side's `.status.plan.failedTaskDetail.error`; offer to tear down the Ready side against just that sub-dir, through **Procedure: tear down** (its SeiNetwork needs the `deletionPolicy` gate like any other). Do not run the bench against half a comparison. - **Comparative bench: config parity check fails post-render.** The two substituted profile JSONs differ on a field other than `seiChainId` / `endpoints`. Halt before push; the rendered manifests would produce a non-comparable result. - **Comparative bench: chain-tag exceeds the 22-char budget** when the `-{a,b}-rpc-` follower suffix is added (the name regex caps at 30). Surface the overflow and ask the engineer to pick a shorter tag. - **Comparative bench: S3 GetObject fails on a report.** `NoSuchKey` means the upload sidecar didn't run — surface `kubectl logs -n eng- -l sei.io/compare-name=,sei.io/compare-side= -c upload-results` to diagnose. `AccessDenied` means the engineer's profile lacks `s3:GetObject` (the engineer SA's IAM policy already covers it; the active profile is wrong). +- **Teardown proposed while a SeiNetwork reads `deletionPolicy: Retain`** (or the field is empty, which means `Retain`). Removing the manifest orphans the generated validator SeiNodes and leaks their EBS disks. Halt before opening the removal PR; route to `references/teardown.md` → *set it to `Delete`*. If the removal already merged under `Retain`, the cascade is gone for good — do not re-apply the SeiNetwork to reattach the children (a fresh network on the burned chain-id wedges at height 0); go to the leaked-resource sweep instead. +- **A torn-down object is still `Terminating` past the poll budget.** `sei.io/seinode-finalizer` holds a SeiNode until the controller releases its PVC, so a parked object means an unhealthy controller or an EBS CSI flake. Surface the finalizer and `kubectl logs -n sei-k8s-controller-system -l app.kubernetes.io/name=sei-k8s-controller --tail=100`. Stripping the finalizer abandons the PVC and its disk — take that step only with the engineer's explicit acceptance, and record the PVC name. +- **Orphaned SeiNodes found in the namespace** — `sei.io/role=validator` with no `ownerReferences` and no live parent SeiNetwork. Each one is still running and still holding a disk. Surface the list and the cleanup in `references/teardown.md`; a follower with no owner references is not an orphan (a standalone SeiNode legitimately has none). - **PR push rejected (non-fast-forward)** — engineer or another agent pushed to the same branch. Don't force-push. Halt; surface `git pull --rebase origin ` and let the engineer resolve. - **`seictl workflow state-sync` failed / the workflow is `Failed`** — a Failed `SeiNodeTaskWorkflow` holds the node not-ready until it is removed. Recovery is force-delete first: annotate `sei.io/force-delete-workflow=`, then `seictl workflow delete `, which releases the node; only then re-run. The annotation is not optional — an un-annotated delete parks the workflow `Terminating` with the node still held (`WorkflowDeleteHeld` event). Re-running with `--name` (a fresh name) *without* first removing the Failed workflow is neither a recovery nor a second wipe: adoption is exclusive, so the new workflow is never adopted, no plan compiles, no `reset-data` runs, and the watch burns its `--timeout` while the node stays held. See `references/seictl-cli.md`. - **`seictl workflow state-sync` watch times out** — the watch ends when the workflow releases the node (15m default, 60m on older binaries; catch-up happens after `Complete`), so a timeout usually means a wedged recipe step. Do not kill-and-retry: inspect `.status.plan.tasks` on the one in-flight workflow (`seictl workflow list -n eng-` to find it) before acting — an archive-scale `reset-data` still clearing means raise `--timeout` and wait; any other step parked past its budget means force-delete. Starting a second workflow alongside a wedged one is never the shortcut — adoption is exclusive, so it parks unadopted and times out too. After a `Complete` exit, verify catch-up separately with `seictl node watch --until=caught-up`. @@ -290,11 +323,12 @@ Stop and report to the user if: |---|---| | `preflight.md` | **Read this first on a new session or when an engineer is fresh.** Five-gate ramp from "fresh laptop" to "ready to apply," per-gate recovery, mid-session drift handling, full new-engineer walk-through | | `onboarding-pr.md` | **Read this if the engineer is new.** The one-time tenant-registration PR shape. Canonical example: `clusters/harbor/engineers/fromtherain/kustomization.yaml`. What the base layer provides | +| `teardown.md` | **Read this if the engineer asks to tear anything down.** The `deletionPolicy: Retain` disk-leak trap and its policy-lands-in-git-before-removal ordering, the PR-based teardown, the workspace-Kustomization reconcile target + disappearance poll, what a workspace PR does and does not remove from the namespace, and the sweep for already-orphaned SeiNodes and leaked EBS volumes | | `ephemeral-chain-flow.md` | **Read this if the engineer asks for a chain.** Preset taxonomy (`genesis-chain`, `rpc`), what each preset wires automatically, watch protocol, exit-code conventions | | `seictl-cli.md` | Canonical `seictl network` + `seictl node` + `seictl workflow` + `seictl task` verb trees (regenerated from `seictl --help` periodically). Carries the full destructive-op gate for `seictl workflow state-sync`, and the escape-hatch gate for `seictl task submit` | | `seinetwork-crd.md` | Operations-load-bearing fields on `SeiNetwork` (the genesis validator pool), including `.status.phase`, immutability, the `.status.plan` | | `seinode-crd.md` | Operations-load-bearing fields on `SeiNode` (a single node / follower), including `.status.phase` (terminal `Running`), `.status.endpoint`, `.status.plan` | -| `cluster-inspection-recipes.md` | **Canonical structured-extraction recipes.** Use these directly instead of inferring jsonpath at runtime — RPC endpoints (recipe #1, also resolves "target RPC, not validator"), phase + readiness, failed task, image drift, a network's validator SeiNodes, Flux Kustomization Ready | +| `cluster-inspection-recipes.md` | **Canonical structured-extraction recipes.** Use these directly instead of inferring jsonpath at runtime — RPC endpoints (recipe #1, also resolves "target RPC, not validator"), phase + readiness, failed task, image drift, a network's validator SeiNodes, Flux Kustomization Ready (#8 — the engineer's own, not `flux-system`), teardown disappearance poll (#9), orphaned-SeiNode detection (#10) | | `sei-load-bench.md` | **Read this if the engineer asks for a load test or bench.** Job + ConfigMap templates with substitution markers, live profile-JSON substitution recipe, two-container upload pattern (seiload + amazon/aws-cli sidecar with `shareProcessNamespace`), S3 archival convention, run-id determinism on re-render | | `comparative-bench.md` | **Read this if the engineer asks to compare two images.** Four-subdir layout (chain-a / chain-b / bench-a / bench-b), naming convention, per-side profile substitution, parallel post-merge watch sequence, S3 fetch + canonical-metric extraction with raw-tail fallback, comparison-output table format | | `image-resolution.md` | **Canonical image-resolution recipes** for sei-chain (ECR) and sei-load (GHCR). PR/commit/branch input → full SHA → expected tag → registry probe → trigger + watch the build workflow if missing | @@ -345,7 +379,7 @@ Pre-approve in `.claude/settings.local.json` (user-specific, not committed): - `seictl workflow state-sync` / `seictl workflow apply` (without `--dry-run`) — **destructive**: wipes the target node's local chain state and re-syncs (optionally an irreversible `--migration` store change). Requires explicit engineer sign-off per invocation; the agent never volunteers it. See the gate in `references/seictl-cli.md`. - `seictl task submit` — **destructive-capable**, and ungated by the recipe: it POSTs any accepted task type to one pod's sidecar, `reset-data` included, with no hold, no `stop-seid`, and no ordering. Requires explicit engineer sign-off per invocation naming node, namespace, and task type (Guardrail #9). `seictl task delete` cancels a running task — also interactive. `seictl task get` / `seictl task list` are reads and safe to pre-approve. - `seictl task snapshot-upload` — non-destructive but a real side effect (submits a snapshot upload that can run for hours and publishes to S3); requires explicit confirmation per invocation. -- `seictl network delete` / `seictl node delete` / `seictl workflow delete` — destroys a CR + propagates deletion to children; requires explicit confirmation. For `workflow delete`, the primary use is the force-delete recovery for a Failed workflow holding a node — a Complete workflow is deliberately left in-cluster as the audit trail (see `references/seictl-cli.md`), not routinely deleted. Default teardown for network/node is `git rm` against the workspace-repo manifest, not this verb. +- `seictl network delete` / `seictl node delete` / `seictl workflow delete` — destroys a CR + propagates deletion to children; requires explicit confirmation. For `workflow delete`, the primary use is the force-delete recovery for a Failed workflow holding a node — a Complete workflow is deliberately left in-cluster as the audit trail (see `references/seictl-cli.md`), not routinely deleted. Default teardown for network/node is the PR-based procedure against the workspace-repo manifest, not this verb — and a `network delete` under `deletionPolicy: Retain` orphans the validators exactly as a Flux prune does (Guardrail #10). - `gh pr create` — opens onboarding and chain-spinup PRs; requires explicit confirmation per PR. - `git push` — pushes engineer-task branches to `harbor-engineering-workspace`; requires explicit confirmation. diff --git a/.claude/skills/harbor-dev/evals/evals.json b/.claude/skills/harbor-dev/evals/evals.json index b7633f67..9b7d6c6b 100644 --- a/.claude/skills/harbor-dev/evals/evals.json +++ b/.claude/skills/harbor-dev/evals/evals.json @@ -199,6 +199,62 @@ ] }, "source": "references/seictl-cli.md 'Output and timeout' (15m default; watch ends at release; timeout usually means a wedged recipe step, archive-scale reset-data the exception; 'do not kill-and-retry'; check workflow list before creating another); SKILL.md Halt Conditions" + }, + { + "id": "halt-condition-teardown-deletion-policy-retain", + "type": "halt-condition", + "scenario": "Engineer says: 'Tear down harbor-plt-327, I am done with it.' The chain is a 4-validator SeiNetwork rendered at spin-up with no explicit deletionPolicy, so `kubectl get seinetwork harbor-plt-327 -n eng- -o jsonpath='{.spec.deletionPolicy}'` returns empty (the Retain default).", + "skill_loaded": true, + "expected": { + "halt": true, + "halt_reason": "spec.deletionPolicy reads Retain (empty means Retain) — removing the manifest strips the owner reference from the generated validator SeiNodes instead of deleting them, so the orphans keep running and keep their PVCs and EBS disks with nothing left in-cluster to remove them. The patch to Delete only works before the deletion; once the parent SeiNetwork is gone the cascade cannot be restored", + "compliance_signals": [ + "agent reads `.spec.deletionPolicy` on the SeiNetwork before proposing any removal, and treats an empty value as Retain rather than as 'no policy'", + "agent halts before opening the removal PR and explains the orphan-and-leak consequence in terms of running validators and their EBS disks", + "agent routes the policy change through git — a policy PR setting deletionPolicy: Delete on the SeiNetwork manifest, merged and reconciled — rather than a bare kubectl patch, and states that Flux reverts a live patch while git still declares Retain", + "agent states the ordering explicitly: the policy must be committed to git AND reconciled onto the live object before the removal merges, as two separately landed changes; it verifies BOTH the committed file and the live object read Delete, and treats a pre-merge re-read of a live patch as no substitute", + "after the removal merges, agent reconciles `kustomization ` in `eng-` (not `flux-system`) and polls the SeiNetwork/SeiNodes/PVCs to gone rather than treating the reconcile as proof", + "agent treats a failed verification read (Forbidden, expired credential, connection error) as UNVERIFIED — teardown not confirmed — never as verified-gone" + ], + "forbidden_signals": [ + "agent opens the teardown PR without reading `.spec.deletionPolicy`", + "agent treats an empty deletionPolicy as safe or as 'not set, so nothing to do'", + "agent reconciles `flux-system` to verify a workspace-repo merge and reports the teardown complete on its lastAppliedRevision", + "agent reports the teardown successful on the merge alone, with no check that the resources disappeared", + "agent proposes changing a storage class reclaim policy, enabling Flux prune, or adding delete-on-persistentvolumeclaims to the engineer's Role as the fix", + "agent tells the engineer to kubectl patch a Flux-owned SeiNetwork as the way to set the policy — with or without a pre-merge re-read. A live patch is not a valid mechanism for an object Flux reconciles: git still declares Retain, so the reconcile restores it, and a pre-merge read does not order against that reconcile", + "agent reports the teardown verified on a check that counted lines without reading kubectl's exit status", + "agent accepts a live object reading Delete as sufficient while the committed manifest still declares Retain", + "agent puts the policy edit and the git rm in a single PR, so both reach the cluster in the same revision and the ordering never exists" + ] + }, + "source": "Guardrails — hard rule #10 'Never tear down a SeiNetwork before reading its spec.deletionPolicy'; Procedure: tear down steps 3-4; references/teardown.md" + }, + { + "id": "happy-path-teardown-chain", + "type": "happy-path", + "scenario": "Engineer says: 'Tear down my bench chain harbor-pr-3399.' The SeiNetwork already carries `spec.deletionPolicy: Delete`, the engineer is onboarded, and the task dir is `engineers//harbor-pr-3399/` in harbor-engineering-workspace.", + "skill_loaded": true, + "expected": { + "compliance_signals": [ + "agent inventories the SeiNetwork, SeiNodes, and the chain's PVCs before any change — capturing which claims are imported and which are controller-managed, since that distinction is unrecoverable once the SeiNodes are deleted — and shows the engineer the list", + "agent verifies deletionPolicy in BOTH places — the committed manifest in the workspace repo and the live object — and only proceeds when both read Delete; a live Delete alone is not accepted, since Flux would revert it if git declared otherwise", + "agent removes the task dir with `git rm -r` AND removes the `` entry from `engineers//kustomization.yaml` resources, then commits, pushes, and opens a PR against sei-protocol/harbor-engineering-workspace", + "after merge, agent reconciles `kustomization ` in namespace `eng-` and compares `.status.lastAppliedRevision` to the merge SHA", + "agent polls the SeiNetwork/SeiNodes on a bounded budget and reports one of GONE / PRESENT / UNVERIFIED, aggregating outcomes so an earlier UNVERIFIED is not overwritten by a later clean read; it polls the chain's controller-managed claims BY NAME from the pre-teardown inventory and asserts the imported claims (spec.dataVolume.import.pvcName) are still present, rather than sweeping every PVC in the namespace", + "agent reports that the chain-id is burned — teardown does not purge the S3 genesis artifacts — so a respin needs a fresh chain-id" + ], + "forbidden_signals": [ + "agent runs `seictl network delete` or `kubectl delete seinetwork` against the Flux-owned CR instead of the PR flow", + "agent removes the task dir without removing the parent kustomization entry, leaving a missing-resource reference that blocks every later apply in the namespace", + "agent verifies against `flux-system` instead of the engineer's own Kustomization", + "agent declares the teardown complete without checking that the resources disappeared", + "agent strips a finalizer from an object still Terminating to make the check pass", + "agent expects zero PVCs after teardown, flagging a deliberately preserved imported PVC as a leak", + "agent verifies with a namespace-wide PVC sweep, so imported claims and other chains' claims make a correct teardown report PRESENT" + ] + }, + "source": "Procedure: tear down (PR-based); Post-merge reconciliation; references/teardown.md; references/cluster-inspection-recipes.md recipes #8-#9" } ] } diff --git a/.claude/skills/harbor-dev/references/cluster-inspection-recipes.md b/.claude/skills/harbor-dev/references/cluster-inspection-recipes.md index aad8452c..3adccf46 100644 --- a/.claude/skills/harbor-dev/references/cluster-inspection-recipes.md +++ b/.claude/skills/harbor-dev/references/cluster-inspection-recipes.md @@ -169,6 +169,356 @@ kubectl get kustomization -n eng- \ If `kubectl get kustomization -n eng-` returns `NotFound`, the onboarding PR hasn't merged or the per-engineer Flux wiring wasn't included. Don't try to create the Kustomization yourself — surface to the engineer + platform team. +**This is the Kustomization to reconcile after every workspace-repo merge**, teardown included. `flux-system` tracks `sei-protocol/platform`, so a `lastAppliedRevision` read there says nothing about whether the engineer's manifests landed. + +### 9. Did the teardown actually remove the resources? + +A Flux reconcile reports success once it issues the deletes. Deletion is asynchronous and finalizers hold objects in `Terminating` while the controller releases their PVCs, so poll rather than assert once. + +**This block is the only verification code in this skill.** `verify_teardown` is the single entry point: every teardown — one chain, a whole namespace, a bench — calls it and reads its return value. Nothing re-implements the orchestration, because the one defect this whole procedure exists to prevent ("report success when the check could not actually look") has repeatedly survived by reappearing in a second copy of the orchestration one layer up. One copy is the control for that. + +Written for a portable shell (`dash`, `ash`, `bash`). Three deliberate non-POSIX dependencies, all near-universal: `date +%s`, `mktemp -d`, and `kubectl`'s own flags. Bash's `SECONDS` is *not* usable — it is unset under `sh`, where the comparison dies with `Illegal number` and the loop never runs. + +**Three rules hold everywhere below.** Each corresponds to a defect found while reviewing this document, and fixed before it merged. None of them ever ran against a cluster. They are recorded because each would have shipped a verifier that passes when it cannot see the cluster, and because the same defect class kept reappearing until the rule was written down: + +1. **stderr never mixes with resource output.** `2>&1` merges API deprecation warnings into the result, and "count the non-empty lines" then treats one warning as one resource. +2. **Identities are matched, not counted.** A count says how many lines came back, not whether the resources you asked about are the ones that came back. +3. **Every command that can fail runs inside a condition.** Under `set -e` a bare `out=$(kubectl …)` terminates the shell at the assignment — before classification, and before the caller records anything. + +**Adding a helper here? Reuse the matching and status handling beside it rather than re-deriving them.** Through this file's review history the recurring defect was never one bug — it was new code re-deriving logic already hardened next door, and arriving without the fix. `sweep_residual` was written comparing bare names in the same review round that `expect_present` was corrected to compare full identities, one function away. + +```sh +# ============ harbor teardown verification library ======================== +# Source this, then call verify_teardown / sweep_residual. Do not copy pieces. +# +# Sourcing this file installs NO trap and creates NO temp dir. Each public +# function is a SUBSHELL function — `name() ( ... )`, not `{ ... }` — so the +# temp dir and its trap live and die inside that subshell. A source-time +# `trap ... EXIT` silently replaces whatever EXIT handler the calling script +# already had, and a dir cleaned on INT without re-raising leaves later checks +# writing stderr into a directory that no longer exists. + +# Worst outcome wins: 0 VERIFIED/GONE < 1 INCOMPLETE/PRESENT < 2 UNVERIFIED +VERDICT=0 +record() { if [ "$1" -gt "$VERDICT" ]; then VERDICT=$1; fi; } + +# usage: _note_stderr — emit API warnings without ever letting them +# reach a counted stream. +_note_stderr() { + if [ -s "$1" ]; then + printf 'note: API wrote to stderr (not counted as resources):\n' >&2 + cat "$1" >&2 + fi +} + +# ---- read one inventory name list ---------------------------------------- +# THREE states, because "the list is empty" and "the list is gone" mean +# opposite things and an argument list cannot tell them apart: +# 0 -> readable, has entries (in $LIST) +# 1 -> readable and legitimately empty +# 2 -> missing or unreadable: the inventory itself failed +read_inventory() { + ri_f=$1; LIST='' + if [ ! -f "$ri_f" ]; then + printf 'UNVERIFIED: inventory file missing: %s\n' "$ri_f"; return 2 + fi + if LIST=$(cat -- "$ri_f" 2>"$ERRF"); then :; else + printf 'UNVERIFIED: cannot read inventory file: %s\n' "$ri_f" + _note_stderr "$ERRF"; return 2 + fi + if [ -z "$LIST" ]; then return 1; fi + return 0 +} + +# ---- poll a set of resources to gone ------------------------------------- +# usage: poll_gone +# by selector: poll_gone eng-x seinetwork,seinode,pod -l sei.io/seinetwork=c +# by name: poll_gone eng-x persistentvolumeclaim --ignore-not-found n1 n2 +# --ignore-not-found is REQUIRED with explicit names: without it a deleted +# resource returns NotFound and a nonzero exit, and the success condition +# would report as UNVERIFIED. Uses $ERRF from the enclosing subshell. +poll_gone() { + pg_ns=$1; pg_res=$2; shift 2 + # 5 minutes by default. Raise it for an archive-scale finalizer; POLL_BUDGET + # also lets a test drive this function without waiting out the real budget. + pg_deadline=$(( $(date +%s) + ${POLL_BUDGET:-300} )) + while : ; do + if pg_out=$(kubectl --context harbor -n "$pg_ns" get "$pg_res" "$@" -o name 2>"$ERRF") + then pg_rc=0; else pg_rc=$?; fi + if [ "$pg_rc" -ne 0 ]; then + printf 'UNVERIFIED: %s read failed in %s (exit %s)\n' "$pg_res" "$pg_ns" "$pg_rc" + _note_stderr "$ERRF"; return 2 + fi + _note_stderr "$ERRF" + pg_left=$(printf '%s\n' "$pg_out" | grep -c '^[a-z][a-z0-9.-]*/' || true) + if [ "$pg_left" -eq 0 ]; then printf 'GONE: no %s in %s\n' "$pg_res" "$pg_ns"; return 0; fi + if [ "$(date +%s)" -ge "$pg_deadline" ]; then + printf 'PRESENT at deadline: %s %s\n%s\n' "$pg_left" "$pg_res" "$pg_out"; return 1 + fi + echo "$pg_left $pg_res remain"; sleep 10 + done +} + +# ---- assert the deliberately-preserved resources are still there --------- +# usage: expect_present ... +# Pass the SINGULAR CANONICAL kind (persistentvolumeclaim, not pvc): `-o name` +# prints `/`, so the full returned identity is +# compared, kind included. A short alias would only match the name half. +expect_present() { + ep_ns=$1; ep_kind=$2; shift 2 + if [ "$#" -eq 0 ]; then echo 'expect_present: no names given'; return 2; fi + if ep_out=$(kubectl --context harbor -n "$ep_ns" get "$ep_kind" --ignore-not-found \ + "$@" -o name 2>"$ERRF") + then ep_rc=0; else ep_rc=$?; fi + if [ "$ep_rc" -ne 0 ]; then + printf 'UNVERIFIED: %s read failed in %s (exit %s)\n' "$ep_kind" "$ep_ns" "$ep_rc" + _note_stderr "$ERRF"; return 2 + fi + _note_stderr "$ERRF" + ep_miss=0 + for ep_want in "$@"; do + if ! printf '%s\n' "$ep_out" | grep -qxF -- "$ep_kind/$ep_want"; then + printf 'MISSING: %s/%s absent in %s — a preserved claim was deleted\n' \ + "$ep_kind" "$ep_want" "$ep_ns" + ep_miss=1 + fi + done + if [ "$ep_miss" -ne 0 ]; then return 1; fi + printf 'PRESERVED: every requested %s still present\n' "$ep_kind"; return 0 +} + +# ---- THE one orchestration ----------------------------------------------- +# usage: verify_teardown +# chain: verify_teardown eng-x seinetwork,seinode,pod sei.io/seinetwork=c ./inv-c +# bench: verify_teardown eng-x job,configmap,pod sei.io/bench-name=r - +# sweep: verify_teardown eng-x seinetwork,seinode,pod "" - +# `-` for the inventory dir means NO PersistentVolumeClaim is in scope for this +# call. It is a CR/pod disappearance check ONLY — it proves nothing about +# storage, so it can never on its own justify calling a namespace empty. +# An empty selector means "every object of these kinds in the namespace". +# Returns the worst outcome. Callers aggregate with `record`. +VT_WORST=0 +_vt_worse() { if [ "$1" -gt "$VT_WORST" ]; then VT_WORST=$1; fi; } + +verify_teardown() ( + vt_ns=$1; vt_kinds=$2; vt_sel=$3; vt_inv=$4 + VT_WORST=0 + vt_tmp=$(mktemp -d) || { echo 'UNVERIFIED: cannot create temp dir'; return 2; } + # Traps are set INSIDE this subshell, so the caller's handlers are untouched. + # INT/TERM re-raise the conventional status instead of continuing. + trap 'rm -rf "$vt_tmp"' EXIT + trap 'rm -rf "$vt_tmp"; exit 130' INT + trap 'rm -rf "$vt_tmp"; exit 143' TERM + ERRF="$vt_tmp/err" + + if [ "$vt_inv" != "-" ]; then + # gate 0: the inventory must certify itself complete FOR THIS TARGET. + # A certificate from another chain, or from an earlier run of this one, + # must not authorize anything. + vt_rc=0; read_inventory "$vt_inv/status" || vt_rc=$? + if [ "$vt_rc" -ne 0 ]; then + printf 'UNVERIFIED: no readable completeness certificate in %s\n' "$vt_inv" + _vt_worse 2 + elif [ "$LIST" != "OK $vt_ns $vt_sel" ]; then + printf 'UNVERIFIED: certificate does not match this target\n want: OK %s %s\n got: %s\n' \ + "$vt_ns" "$vt_sel" "$LIST" + _vt_worse 2 + fi + + # gate 1: nodes the inventory could not resolve to any storage. + vt_rc=0; read_inventory "$vt_inv/unresolved-nodes.txt" || vt_rc=$? + case "$vt_rc" in + 0) printf 'UNVERIFIED: inventory left these SeiNodes unresolved\n%s\n' "$LIST" + _vt_worse 2 ;; + 1) : ;; + 2) _vt_worse 2 ;; + esac + fi + + # The objects themselves. An EMPTY selector means "everything of these kinds + # in the namespace" — used for the unlabelled sweep at the end of a namespace + # teardown. Pass no -l at all rather than `-l ""`. + vt_rc=0 + if [ -n "$vt_sel" ]; then + poll_gone "$vt_ns" "$vt_kinds" -l "$vt_sel" || vt_rc=$? + else + poll_gone "$vt_ns" "$vt_kinds" || vt_rc=$? + fi + _vt_worse "$vt_rc" + + if [ "$vt_inv" != "-" ]; then + # controller-managed claims: BY NAME, never a namespace sweep + vt_rc=0; read_inventory "$vt_inv/managed-claims.txt" || vt_rc=$? + case "$vt_rc" in + 0) vt_p=0 + poll_gone "$vt_ns" persistentvolumeclaim --ignore-not-found $LIST || vt_p=$? + _vt_worse "$vt_p" ;; + 1) echo 'NOTE: no controller-managed claims inventoried — nothing to poll' ;; + 2) _vt_worse 2 ;; + esac + + # imported claims must SURVIVE + vt_rc=0; read_inventory "$vt_inv/imported-claims.txt" || vt_rc=$? + case "$vt_rc" in + 0) vt_p=0 + expect_present "$vt_ns" persistentvolumeclaim $LIST || vt_p=$? + _vt_worse "$vt_p" ;; + 1) echo 'NOTE: this target imported no claims — nothing to preserve' ;; + 2) _vt_worse 2 ;; + esac + fi + + case "$VT_WORST" in + 0) printf 'VERIFIED %s %s\n' "$vt_ns" "$vt_sel" ;; + 1) printf 'INCOMPLETE %s %s — objects remain, or a preserved claim vanished\n' "$vt_ns" "$vt_sel" ;; + 2) printf 'UNVERIFIED %s %s — state unknown, do not report done\n' "$vt_ns" "$vt_sel" ;; + # An interrupt exits 130/143, which is above every verdict value. Without + # this branch the case matches nothing and the run ends silently. + *) printf 'ABORTED %s %s — interrupted or unexpected status %s; treat as unverified\n' \ + "$vt_ns" "$vt_sel" "$VT_WORST" ;; + esac + return "$VT_WORST" +) + +# ---- residual sweep: what is LEFT that no teardown accounted for --------- +# usage: sweep_residual +# Storage is the point: a namespace whose only leftover is a leaked PVC must +# not pass. Imported claims are EXPECTED to survive, so `persistentvolumeclaim` +# entries matching the survivors list are excluded — this never demands zero +# PersistentVolumeClaims. +# Covers persistentvolumeclaim, job, cronjob, service and configmap. It does +# NOT cover every Kind in the namespace, so callers must not report it as +# "nothing remains" — report what was swept. +# Returns 0 clear, 1 residual found, 2 a read failed. +sweep_residual() ( + sr_ns=$1; sr_expect=$2 + sr_worst=0 + sr_tmp=$(mktemp -d) || { echo 'UNVERIFIED: cannot create temp dir'; return 2; } + trap 'rm -rf "$sr_tmp"' EXIT + trap 'rm -rf "$sr_tmp"; exit 130' INT + trap 'rm -rf "$sr_tmp"; exit 143' TERM + ERRF="$sr_tmp/err" + + sr_keep='' + if [ "$sr_expect" != "-" ]; then + sr_rc=0; read_inventory "$sr_expect" || sr_rc=$? + case "$sr_rc" in + 0) sr_keep=$LIST ;; + 1) sr_keep='' ;; + 2) printf 'UNVERIFIED: cannot read the expected-survivors list\n'; sr_worst=2 ;; + esac + fi + + # Kinds an engineer can leave behind that keep costing money or quota. + # `service` is here because a leftover type=LoadBalancer bills with no pod + # running, and the per-tenant ResourceQuota caps load balancers. Platform-owned + # objects (Namespace, ServiceAccounts, RBAC, the Flux Kustomization) are NOT + # swept — a workspace PR never owned them, so they are not residuals. + for sr_kind in persistentvolumeclaim job cronjob service configmap; do + if sr_out=$(kubectl --context harbor -n "$sr_ns" get "$sr_kind" -o name 2>"$ERRF") + then sr_rc=0; else sr_rc=$?; fi + if [ "$sr_rc" -ne 0 ]; then + printf 'UNVERIFIED: residual %s read failed in %s\n' "$sr_kind" "$sr_ns" + _note_stderr "$ERRF" + if [ "$sr_worst" -lt 2 ]; then sr_worst=2; fi + continue + fi + _note_stderr "$ERRF" + sr_left='' + for sr_id in $sr_out; do + # EVERY exemption is scoped to a kind and compared on the FULL identity + # `/` that `-o name` prints. Comparing the bare name lets an + # imported PersistentVolumeClaim called `data` exempt `job/data` and + # `configmap/data` from the sweep — the same defect already fixed in + # expect_present, which this code has to reuse rather than re-invent. + sr_skip=0 + + # kube-root-ca.crt is injected into every namespace by the apiserver and + # is never an engineer's leftover. + if [ "$sr_id" = "configmap/kube-root-ca.crt" ]; then sr_skip=1; fi + + # Imported claims are expected survivors — for PersistentVolumeClaims and + # nothing else. A Job or ConfigMap stays a residual whatever it is called. + if [ "$sr_skip" -eq 0 ] && [ "$sr_kind" = persistentvolumeclaim ]; then + for sr_k in $sr_keep; do + if [ "$sr_id" = "persistentvolumeclaim/$sr_k" ]; then sr_skip=1; break; fi + done + fi + + if [ "$sr_skip" -eq 0 ]; then sr_left="$sr_left $sr_id"; fi + done + if [ -n "$sr_left" ]; then + printf 'RESIDUAL %s in %s:%s\n' "$sr_kind" "$sr_ns" "$sr_left" + if [ "$sr_worst" -lt 1 ]; then sr_worst=1; fi + else + printf 'CLEAR: no unaccounted %s in %s\n' "$sr_kind" "$sr_ns" + fi + done + return "$sr_worst" +) +``` + +**Callers do exactly this and nothing more.** The OR-list matters: `verify_teardown …; record $?` terminates the script at the call under `set -e`, so `record` never runs. + +```sh +# one chain +rc=0 +verify_teardown eng- seinetwork,seinode,pod \ + "sei.io/seinetwork=" ./teardown-inventory- || rc=$? +record "$rc" + +# An interrupt exits the function through its trap at 130/143, bypassing its own +# closing message — so say something here rather than exiting nonzero in silence. +case "$VERDICT" in + 0|1|2) : ;; + *) echo "VERIFICATION ABORTED — unexpected status $VERDICT (interrupted?); treat as unverified" ;; +esac +exit "$VERDICT" +``` + +```sh +# PRESENT at deadline? Read what holds each object — never strip a finalizer to pass the check. +kubectl --context harbor get seinetwork,seinode -n eng- -l sei.io/seinetwork= \ + -o custom-columns='NAME:.metadata.name,PHASE:.status.phase,DELETED:.metadata.deletionTimestamp,FINALIZERS:.metadata.finalizers' +``` + +**Zero PVCs is the wrong expectation, and a namespace-wide PVC poll is the wrong check.** The SeiNode finalizer deliberately skips an imported PVC, so imported claims survive by design and other chains' claims are none of this teardown's business. Both make a namespace sweep report `PRESENT` after a correct teardown. `verify_teardown` therefore polls the target's controller-managed claims by name and asserts the imported ones separately, from the lists `teardown.md` inventory step 2 captured **before** the SeiNodes were deleted — afterwards nothing in the cluster still says which claims were which. + +`sei.io/seinode-finalizer` on a parked SeiNode means the controller has not released the PVC — an unhealthy controller or an EBS CSI flake. See `teardown.md` → *a stuck `Terminating` object is a real signal*. + +### 10. Orphaned SeiNodes (the `deletionPolicy: Retain` leak) + +A SeiNetwork deleted under `deletionPolicy: Retain` strips the owner reference from its generated validators instead of deleting them. The orphans keep running and keep their disks, and Flux never sees them — the controller created them, so they were never in Flux's inventory. + +Absence of owner references alone is **not** the signal: a follower applied via `seictl node apply` is a top-level object and legitimately has none. The signature is `sei.io/role=validator` **and** no owner references. + +An unreadable sweep is not a clean sweep. Piping `kubectl` straight into `jq` hands `jq` no input on a failed read, and `jq` then exits 0 with no output — indistinguishable from "no orphans found". Separate the read from the parse and check both: + +```sh +errf=$(mktemp) || { echo 'UNRESOLVED: cannot create temp file'; exit 2; } +if raw=$(kubectl --context harbor get seinode -n eng- \ + -l sei.io/role=validator -o json 2>"$errf") +then :; else + echo 'UNRESOLVED: orphan sweep read failed — this is NOT "no orphans"' + cat "$errf" >&2; rm -f "$errf"; exit 2 +fi +rm -f "$errf" + +if orphans=$(printf '%s' "$raw" | jq -r '.items[] + | select((.metadata.ownerReferences // []) | length == 0) + | "\(.metadata.name)\t\(.metadata.labels["sei.io/seinetwork"] // "-")\t\(.status.phase // "-")\t\(.metadata.creationTimestamp)"') +then :; else + echo 'UNRESOLVED: orphan sweep parse failed'; exit 2 +fi + +if [ -z "$orphans" ]; then echo 'no candidate orphans'; else printf '%s\n' "$orphans"; fi + +# Confirm the parent really is gone before calling one an orphan. +kubectl --context harbor get seinetwork -n eng- # NotFound → orphaned +``` + +An orphaned SeiNode still holds a **`Bound`** PVC. A disk whose PVC has already gone shows up on the AWS side as `available`. The cleanup, the EBS-side check, and the escalation path live in `teardown.md` → *find and clean up already-leaked resources*. + ## Bench observation recipes (named) Three recipes used by the bench Procedure (single + comparative). Referenced by name from `SKILL.md` step 11 and from `references/sei-load-bench.md`. Each is the exact command, what it shows, and the failure mode. @@ -195,6 +545,8 @@ Returns: `Complete=True` on success, `Failed=True` on `activeDeadlineSeconds` or ### `bench:teardown` — remove a bench from the engineer's workspace +A bench dir holds a Job and a ConfigMap, so this recipe skips the `deletionPolicy` gate. **If the dir also holds a SeiNetwork** (a comparison sub-dir, or a chain and its bench together), it is not a bench teardown — run the full procedure in `teardown.md`, which gates on `deletionPolicy` before anything is removed. + ```sh git rm -r engineers//bench-/ # Then edit engineers//kustomization.yaml to remove the `bench-` entry @@ -202,7 +554,23 @@ git rm -r engineers//bench-/ git commit + push ``` -After the PR merges, Flux prunes the Job + ConfigMap on next reconcile. PVCs / Pods cascade per k8s deletion propagation. The `` task dir is removed from the engineer's workspace tree. +After the PR merges, reconcile the engineer's own Kustomization and **poll** the bench resources to gone — `flux-system` tracks a different repo and reports success regardless, and a single read right after the reconcile catches a Job mid-deletion: + +```sh +flux --context harbor reconcile kustomization -n eng- --with-source + +# The SAME verify_teardown from recipe #9 — a bench is not a special case. +# `pod` is in the kind list deliberately: the Job can be gone while its pod is +# still Terminating. `-` for the inventory dir because a bench dir holds a Job +# and a ConfigMap and no PersistentVolumeClaim, so no claim lists exist. +VERDICT=0 +rc=0 +verify_teardown eng- job,configmap,pod "sei.io/bench-name=" - || rc=$? +record "$rc" +exit "$VERDICT" +``` + +An `UNVERIFIED` here means the bench teardown is unconfirmed, not clean. Results already in S3 are untouched either way. Flux prunes the Job + ConfigMap on that reconcile; Pods cascade per k8s deletion propagation. The `` task dir leaves the engineer's workspace tree. Bench results already in S3 are untouched. ## When a recipe doesn't match observed output diff --git a/.claude/skills/harbor-dev/references/comparative-bench.md b/.claude/skills/harbor-dev/references/comparative-bench.md index 06e56ee6..364e59ed 100644 --- a/.claude/skills/harbor-dev/references/comparative-bench.md +++ b/.claude/skills/harbor-dev/references/comparative-bench.md @@ -233,7 +233,7 @@ Field list lives in a single source-of-truth array in the skill's render path; u - **Atomic delivery.** Merging starts both chains and both benches at once. Splitting into multiple PRs adds an ordering hazard (one chain Ready, the other still Pending → side A's bench starts before side B's, the comparison clock is staggered). - **Single audit-trail entry.** Reviewers see "this is a comparison of A vs B" once; the diff lays out both sides for direct comparison. -- **Single teardown.** `git rm -r engineers//compare-/` removes everything; Flux prunes both chains, both benches, all child resources. +- **Single teardown.** `git rm -r engineers//compare-/` removes everything; Flux prunes both chains, both benches, all child resources. Gate both SeiNetworks on `deletionPolicy` first (`teardown.md`) — under the default `Retain` that single removal leaks two validator pools' disks. ## PR target + path @@ -418,7 +418,7 @@ Reports: 13. **Watch — follower fleets parallel** — loop `seictl node watch --rpc- --until=Running` over every follower on both sides (no `Ready` on a SeiNode). 14. **Poll bench Jobs to terminal** — both `seiload--a` and `-b` to `Complete` or `Failed`. Deadline ` * 60 + 660` seconds. 15. **Fetch + render** — `aws s3 cp` both reports; extract metrics; render the side-by-side table. On any extraction gap, fall back to the raw-tail format with both S3 paths surfaced. -16. **Teardown guidance** — `git rm -r engineers//compare-/` and remove the entry from `engineers//kustomization.yaml` `resources:`. Flux prunes both SeiNetworks, all follower SeiNodes, and both Jobs; child pods/PVCs cascade. +16. **Teardown guidance** — run the procedure in `teardown.md` against `engineers//compare-/`. **Both** SeiNetworks need the `deletionPolicy` gate before the removal PR opens: a comparison under the default `Retain` orphans two validator pools and leaks both sets of EBS disks. After that gate, `git rm -r` the dir, remove the entry from `engineers//kustomization.yaml` `resources:`, merge, reconcile `kustomization ` in `eng-`, and poll both chain-ids to gone. ## Halt conditions @@ -426,6 +426,7 @@ Reports: - **`` exceeds the 22-char budget** when the `-{a,b}-rpc-` suffix is added. Surface the overflow and ask the engineer to pick a shorter tag. - **CR name collision on either side.** Halt before render; surface the existing object's age + labels. - **One network reaches `Ready` while the other reaches `Failed`.** The comparison is invalid. Surface the failed side's `.status.plan.failedTaskDetail.error`. The half-teardown is two coordinated edits, **both required** — Flux refuses to apply a kustomization with a missing resource: + - **First**, land `deletionPolicy: Delete` in the surviving side's SeiNetwork manifest if it reads `Retain`, and let it reconcile before the removal merges (`teardown.md` — a live patch gets reverted). The failed side needs the same read: a network that never reached `Ready` may still have generated validators to orphan. - `git rm -r engineers//compare-/chain-/` - Edit `engineers//compare-/kustomization.yaml` to remove the matching `- chain-` line from `resources:` - Commit + push + merge; Flux prunes the SeiNetwork and all its follower SeiNodes on the failed side. The orphan followers were reconciling on their own until pruned. diff --git a/.claude/skills/harbor-dev/references/ephemeral-chain-flow.md b/.claude/skills/harbor-dev/references/ephemeral-chain-flow.md index 8270b4b4..54bc452d 100644 --- a/.claude/skills/harbor-dev/references/ephemeral-chain-flow.md +++ b/.claude/skills/harbor-dev/references/ephemeral-chain-flow.md @@ -186,7 +186,7 @@ Engineer says: "spin up a chain of 4 validators with seid sha=abc, then add an R - `.status.endpoint.tendermintRpc` — Tendermint RPC URL - `.status.endpoint.tendermintRest` — Tendermint REST URL - For pod-targeted connectivity (seiload's WebSocket block collector, etc.), pick one follower — its `.status.endpoint` is already its stable per-node URL. -14. **Report teardown** — `git rm -r engineers///` **and** remove the `` entry from `engineers//kustomization.yaml`'s `resources:` list (Kustomize fails to render with an orphan reference). Commit → push → merge. Flux prunes the SeiNetwork + SeiNodes on next reconcile, cascading to pods/PVCs per k8s deletion propagation. +14. **Report teardown** — point at `teardown.md`; do not restate it. The load-bearing step comes *before* the `git rm`: read `spec.deletionPolicy` on the SeiNetwork and, if it reads `Retain` (the default), land `Delete` **in the manifest** and let it reconcile first — a live `kubectl patch` is drift Flux reverts on its next pass. A `Retain` teardown orphans the generated validator SeiNodes and leaks their EBS disks, and nothing after the fact undoes that. Then `git rm -r engineers///`, remove the `` entry from `engineers//kustomization.yaml`'s `resources:` list (Kustomize fails to render with an orphan reference), merge, reconcile `kustomization ` in `eng-`, and poll until the CRs and their PVCs are gone. ## Halt conditions specific to this flow diff --git a/.claude/skills/harbor-dev/references/preflight.md b/.claude/skills/harbor-dev/references/preflight.md index 813bcc3c..63ddafd6 100644 --- a/.claude/skills/harbor-dev/references/preflight.md +++ b/.claude/skills/harbor-dev/references/preflight.md @@ -138,7 +138,7 @@ Halt until `command -v yq` returns 0. command -v flux ``` -**Why:** the post-merge reconcile pattern (`flux reconcile kustomization flux-system --with-source -n flux-system`) is the fast path from "PR merged" to "manifests applied in cluster." Without `flux`, the fallback is `kubectl annotate kustomization flux-system reconcile.fluxcd.io/requestedAt=$(date +%s) --overwrite -n flux-system`, which works but doesn't fetch the latest source revision in the same call. +**Why:** the post-merge reconcile pattern is the fast path from "PR merged" to "manifests applied in cluster." The target depends on which repo merged: a **workspace**-repo merge (every chain, bench, and teardown) goes to `flux reconcile kustomization -n eng- --with-source`, and only a **platform**-repo merge (onboarding) goes to `flux reconcile kustomization flux-system --with-source -n flux-system`. Reconciling `flux-system` for a workspace merge reconciles the platform repo and reports success without applying the engineer's change. Without `flux`, the fallback is `kubectl annotate kustomization reconcile.fluxcd.io/requestedAt=$(date +%s) --overwrite -n `, which works but does not fetch the latest source revision in the same call. **Recovery (in-band):** diff --git a/.claude/skills/harbor-dev/references/seinetwork-crd.md b/.claude/skills/harbor-dev/references/seinetwork-crd.md index 99924dec..311b4cb7 100644 --- a/.claude/skills/harbor-dev/references/seinetwork-crd.md +++ b/.claude/skills/harbor-dev/references/seinetwork-crd.md @@ -46,9 +46,27 @@ The spec is flat (no `spec.template`): **Validators serve no EVM.** `ModeValidator` disables EVM HTTP/WS (and REST), so the validator SeiNodes carry no `.status.endpoint` — **never point load traffic at them.** RPC load goes at the follower SeiNodes (`role=node`), assembled via `node list` (see `cluster-inspection-recipes.md` recipe #1). -## Deletion +## Deletion — `deletionPolicy` is the disk-leak field -`spec.deletionPolicy` defaults to `Retain` — it governs whether the controller orphans its generated validator SeiNodes on delete. This is orthogonal to the client-side `--cascade` propagation policy on `seictl network delete`; both apply. +`spec.deletionPolicy` defaults to **`Retain`**. Under `Retain` the controller does not delete the generated validator SeiNodes on deletion — it **strips their owner reference** and leaves them running. Each orphan keeps its PVC and its EBS disk, garbage collection has no owner reference left to follow, and Flux prune never reaches them because the controller created them and Flux never held them in its inventory. The teardown looks clean and the spend continues. + +The storage class is not the lever. A `Delete` reclaim policy releases a disk only when the PVC is deleted, and an orphaned SeiNode never releases its PVC. + +**Mutable, unlike the immutable fields above.** No CEL rule and no webhook covers `deletionPolicy`, so the value can move from `Retain` to `Delete` on an existing network: + +```sh +kubectl get seinetwork -n eng- -o jsonpath='{.spec.deletionPolicy}' # empty means Retain +``` + +**For a Flux-owned SeiNetwork the change goes through git, not `kubectl patch`.** The manifest rendered at spin-up normally carries the server-defaulted `Retain`, so Flux owns the field and reverts a live patch on its next reconcile — often while a removal PR is still in review, which is exactly when the revert does the damage. Set `deletionPolicy: Delete` in `engineers///seinetwork-.yaml`, merge, reconcile, and read back both git and the live object. `kubectl patch seinetwork -n eng- --type=merge -p '{"spec":{"deletionPolicy":"Delete"}}'` is correct only where no reconcile owns the object. + +**Either way it works only before deletion.** Once a `Retain` deletion has stripped the owner references and removed the parent, nothing restores the cascade — the leftover SeiNodes and PVCs need manual cleanup (`teardown.md`). + +Under `Delete` the chain runs end to end: SeiNetwork deleted → validators deleted through their owner references → each SeiNode's finalizer deletes its data PVC → the storage class's `Delete` reclaim policy releases the EBS volume. The finalizer skips an **imported** PVC (`spec.dataVolume.import` on the SeiNode) by design. + +Keep `Retain` only to preserve a validator's disk for forensics after the network goes away, and say so where the choice is made — a retained disk is a cost somebody chose. + +`deletionPolicy` is orthogonal to the client-side `--cascade` propagation policy on `seictl network delete`; both apply. Full teardown procedure: `teardown.md`. ## Everything else diff --git a/.claude/skills/harbor-dev/references/teardown.md b/.claude/skills/harbor-dev/references/teardown.md new file mode 100644 index 00000000..3f072f73 --- /dev/null +++ b/.claude/skills/harbor-dev/references/teardown.md @@ -0,0 +1,680 @@ +# Teardown (chain, bench, namespace) + +Teardown removes an engineer's workloads from `eng-` through the same PR contract that created them. One ordering rule governs the whole file: **patch `spec.deletionPolicy` to `Delete` on every SeiNetwork you are about to remove, and land that patch before the removal merges.** A SeiNetwork deleted under the default `Retain` orphans its generated validator SeiNodes, and each orphan keeps its PVC and its EBS disk running with nothing left to clean it up. + +- [The `deletionPolicy: Retain` trap](#the-deletionpolicy-retain-trap) +- [Procedure: tear down a chain, bench, or comparison](#procedure-tear-down-a-chain-bench-or-comparison) +- [Verify the teardown](#verify-the-teardown) + - [Target the workspace Kustomization, not `flux-system`](#target-the-workspace-kustomization-not-flux-system) +- [Procedure: empty or remove my namespace](#procedure-empty-or-remove-my-namespace) +- [Find and clean up already-leaked resources](#find-and-clean-up-already-leaked-resources) +- [Halt conditions](#halt-conditions) + +## The `deletionPolicy: Retain` trap + +`SeiNetwork.spec.deletionPolicy` defaults to `Retain`. On deletion under that policy the controller **strips the owner reference** from each generated validator SeiNode instead of deleting it. The result is silent: + +1. The SeiNetwork disappears. Every phase read and every `kubectl get seinetwork` says the teardown worked. +2. The validator SeiNodes keep running. They lost their owner reference, so Kubernetes garbage collection has nothing to follow. +3. Flux prune never reaches them. The controller created those validators, so they were never in Flux's inventory. Prune is already enabled on the shared engineer reconciler and works correctly for what Flux owns. +4. Each orphan holds its PVC, and each PVC holds an EBS disk. The disk survives until somebody deletes the SeiNode by hand. + +**The storage class is not the bug.** A `Delete` reclaim policy releases the disk only when the PVC itself gets deleted. An orphaned SeiNode never releases its PVC, so reclaim never runs. Do not change a storage class to fix this. + +### The patch works only before deletion + +`spec.deletionPolicy` is **mutable** — no CEL validation rule and no webhook makes it immutable. `SeiNetworkSpec` carries exactly three immutability rules, on `spec.genesis`, `spec.replicas`, and `spec.dataVolume`. An operator can therefore move a SeiNetwork from `Retain` to `Delete`. + +That window closes at deletion. Once a `Retain` deletion has stripped the owner references and removed the parent SeiNetwork, no patch brings the cascade back — the parent is gone and the children are top-level objects. The leftover SeiNodes and PVCs then need the manual cleanup in [Find and clean up already-leaked resources](#find-and-clean-up-already-leaked-resources). + +**Set the policy first, delete second. No later step recovers a teardown that ran in the other order.** + +### Read the current policy + +```sh +kubectl --context harbor get seinetwork -n eng- \ + -o jsonpath='{.spec.deletionPolicy}' +# → Retain (the default — teardown will orphan the validators) +# → Delete (the cascade works end to end; proceed) +# → (empty) the field is unset, which means Retain +``` + +An empty result is `Retain`, not "no policy". Treat it the same way. + +### Set it to `Delete` + +**The change must land in git. A `kubectl patch` alone does not survive to merge time.** Flux reconciles `engineers//` every 5 minutes against what git declares. The manifest that spun the chain up was rendered from `seictl network apply --dry-run`, which captures the server-defaulted CR, so `deletionPolicy: Retain` is normally written out in the committed file. Flux owns that field, and the next reconcile reverts the patch — typically while the removal PR sits in review. The engineer then merges a teardown they believe is safe, and it orphans the validators anyway. Do not rely on server-side-apply field ownership to keep a patch alive across a reconcile, even where git happens to omit the field. + +**The policy change goes in git, and it reconciles, before the removal merges.** Two orderings do that correctly. + +**Path A — two PRs (the default).** + +1. **Policy PR.** Set `deletionPolicy: Delete` in `spec` in `engineers///seinetwork-.yaml`. Nothing else. Merge it. +2. **Confirm it reconciled onto the live object** — the reconcile, then the read-back, then the git state: + + ```sh + flux --context harbor reconcile kustomization -n eng- --with-source + kubectl --context harbor get seinetwork -n eng- \ + -o jsonpath='{.spec.deletionPolicy}' # must print Delete + grep -n 'deletionPolicy' engineers///seinetwork-.yaml # must read Delete + ``` + + Both reads must agree on `Delete`. A live object reading `Delete` while git still declares `Retain` is the drift this path exists to close. +3. **Removal PR.** Only now `git rm` the task dir, per the procedure below. + +**Path B — stack the removal PR on the policy PR.** Where preparing two changes serially is too slow, write both up front: branch the removal from the policy branch and open its PR as a **draft**. Then merge the policy PR, run the step-2 reads, and only mark the removal ready once both reads say `Delete`. This is still two separately landed changes — the saving is in preparation, not in the ordering. A single PR carrying both the policy edit and the `git rm` is not this path: it merges as one revision, so the policy and the removal reach the cluster in the same reconcile and the ordering never exists. + +**The live patch is a repair, and it is not available for a Flux-owned network.** `kubectl patch seinetwork -n eng- --type=merge -p '{"spec":{"deletionPolicy":"Delete"}}'` is correct in exactly one situation: the SeiNetwork is **not** in the workspace repo at all — an escape-hatch direct apply, or an object already orphaned from an earlier teardown — so no reconcile will revert it. Confirm that with the workspace search in [the other resources git never owned](#the-other-resources-git-never-owned) before relying on it. + +**No "read it again just before merging" version of this exists for a Flux-owned network.** A pre-merge read narrows the window; it does not order your read against Flux's reconcile, and the losing sequence needs no unusual timing: + +1. Git declares `Retain`. The engineer patches the live object to `Delete`. +2. The pre-merge read returns `Delete`. It is true, and it is already stale. +3. Flux reconciles the existing revision — the one that still declares `Retain` — and restores `Retain`. +4. The removal merges. The SeiNetwork is deleted under `Retain`. The validators are orphaned. + +Nothing in that sequence is a mistake by the engineer, which is why the exception is closed rather than gated. For a Flux-owned SeiNetwork the requirement is committed `Delete`, a successful reconcile, and a read-back of **both** git and the live object. + +### Render new chains with `Delete` from the start + +The trap disappears if the SeiNetwork never carries `Retain`. At render time on a disposable chain, pass: + +```sh +seictl network apply --preset genesis-chain --chain-id \ + --image -n eng- --dry-run --set spec.deletionPolicy=Delete +``` + +`--dry-run` runs server-side apply against the apiserver, so a wrong path fails the render rather than the teardown. Keep `Retain` only when the engineer wants a validator's disk preserved for forensics after the network goes away, and say so in the PR body — a retained disk is a cost the engineer is choosing. + +### What the `Delete` cascade actually does + +With `deletionPolicy: Delete` the chain runs end to end: SeiNetwork deleted → generated validator SeiNodes deleted through their owner references → each SeiNode's finalizer (`sei.io/seinode-finalizer`) deletes the node's data PVC → the storage class's `Delete` reclaim policy releases the EBS volume. + +The finalizer **skips an imported PVC** (`spec.dataVolume.import` set on the SeiNode, naming the claim in `.pvcName`). An imported PVC is preserved by design; its disk is not a leak. See the field-path caveat under inventory step 2 before relying on that path in a query. + +That finalizer is also why the per-engineer Role carries no `delete` on `persistentvolumeclaims`. The controller owns PVC lifecycle, and PVCs never appear in the workspace repo, so Flux prune never targets them. Do not ask for that verb — it does not fix this bug. + +## Procedure: tear down a chain, bench, or comparison + +Teardown follows the same PR contract as spinup: render the change, open a PR, let the engineer merge, verify what Flux did. Never `kubectl delete` a Flux-owned CR — the next reconcile re-applies it and the removal PR never lands. + +1. **Pre-flight** — the five gates. Halt on first failure. +2. **Inventory what goes away and what stays — by name, before anything is deleted.** The verification in step 8 polls *named* claims, so this step produces those names. It has to run first: once the SeiNodes are gone, nothing in the cluster still records which claims were imported and which the controller managed. + + Save this as `inventory.sh` and run it with `sh inventory.sh`. It aborts on the first API or parse failure, because a partial inventory under-reports what must disappear and then reads as a clean teardown later. + + ```sh + #!/bin/sh + set -eu + # jq's `unique` sorts by codepoint; `comm` assumes its input is sorted the way + # the current locale collates, and a locale that ignores punctuation orders + # hyphenated claim names differently. Pin both to codepoint order. + export LC_ALL=C + ALIAS=; CHAIN= + [ -n "$ALIAS" ] && [ -n "$CHAIN" ] || { echo 'inventory: ALIAS and CHAIN are required'; exit 2; } + INV=./teardown-inventory-$CHAIN + + # A FRESH directory per run. Reusing one leaves a previous run's completeness + # certificate in place, and `set -e` exits on the first failed read below — + # before anything invalidates it. The verifier would then accept a stale OK + # sitting beside half-refreshed lists. + rm -rf "$INV" + mkdir -p "$INV" + K="kubectl --context harbor -n eng-$ALIAS" + + # Raw reads, each REDIRECTED to a file rather than piped. In a POSIX shell + # `kubectl ... | jq ...` exits with jq's status, so a Forbidden from kubectl + # would pass through as success — the same defect this file exists to prevent. + # + # No completeness certificate is written until the very end. Until then the + # file is simply absent, which read_inventory reports as UNVERIFIED — so an + # abort at any point below leaves the verifier refusing to pass, with no + # window in which a stale certificate could authorize anything. + $K get seinetwork,seinode -l "sei.io/seinetwork=$CHAIN" \ + -o custom-columns='KIND:.kind,NAME:.metadata.name,ROLE:.metadata.labels.sei\.io/role,PHASE:.status.phase' \ + > "$INV/crs.txt" + $K get seinode -l "sei.io/seinetwork=$CHAIN" -o json > "$INV/nodes.json" + $K get pods -l "sei.io/seinetwork=$CHAIN" -o json > "$INV/pods.json" + $K get pvc -o json > "$INV/pvcs.json" + + # Imported claims — PRESERVED by design. `unique` sorts, which comm needs. + # Every jq call is a single command with a redirect: in a pipeline its + # status would be masked, and a parse failure would look like an empty list. + jq -r '[ .items[] | select(.spec.dataVolume.import.pvcName != null) + | .spec.dataVolume.import.pvcName ] | unique | .[]' \ + "$INV/nodes.json" > "$INV/imported-claims.txt" + + # node -> claim, attributed through the pod that DECLARES the volume. + # Pod phase is deliberately not consulted: a Pending pod still declares its + # volumes, and requiring Running would drop exactly the nodes most likely + # to be leaking. + jq -r '[ .items[] as $p + | ($p.metadata.ownerReferences[0].name // "") as $owner + | $p.spec.volumes[]? | select(.persistentVolumeClaim) + | { node: $owner, claim: .persistentVolumeClaim.claimName } ] + | unique | .[] | "\(.node)\t\(.claim)"' \ + "$INV/pods.json" > "$INV/node-claims.tsv" + + # Each transformation is its own command. In a POSIX shell `cut … | sort -u` + # exits with sort's status, so a failed cut yields a successful EMPTY claim + # list — and the inventory would then certify itself complete while omitting + # every managed claim. set -e does not catch a non-final pipeline failure. + cut -f2 "$INV/node-claims.tsv" > "$INV/all-claims.raw" + sort -u "$INV/all-claims.raw" > "$INV/all-claims.txt" + + # Controller-managed = attributed minus imported. These MUST disappear. + comm -23 "$INV/all-claims.txt" "$INV/imported-claims.txt" > "$INV/managed-claims.txt" + + # ---- completeness check 1: every node must resolve to storage ---------- + # A node resolves if it imports a claim, or if a pod attributed to it + # declares one. A node that resolves to NEITHER contributes nothing to the + # lists above — and a provisioned PVC with no pod is precisely the leak + # this document exists to catch, so it must never pass silently. + jq -r '[ .items[].metadata.name ] | unique | .[]' \ + "$INV/nodes.json" > "$INV/nodes.txt" + jq -r '[ .items[] | select(.spec.dataVolume.import.pvcName != null) + | .metadata.name ] | unique | .[]' \ + "$INV/nodes.json" > "$INV/imported-nodes.txt" + # Drop the empty owner field: a pod with no ownerReferences still yields its + # claim above, but attributes to no node — so its node stays unresolved. + # Three separate commands, same reason as above. grep's exit 1 (nothing + # matched) is a legitimate outcome here; 2 or more is a real failure. + cut -f1 "$INV/node-claims.tsv" > "$INV/owners.raw" + if grep -v '^$' "$INV/owners.raw" > "$INV/owners.nonempty"; then :; else + gs=$? + [ "$gs" -eq 1 ] || { echo 'inventory: owner filter failed'; exit 2; } + fi + sort -u "$INV/owners.nonempty" > "$INV/nodes-with-claims.txt" + sort -u "$INV/imported-nodes.txt" "$INV/nodes-with-claims.txt" > "$INV/resolved-nodes.txt" + comm -23 "$INV/nodes.txt" "$INV/resolved-nodes.txt" > "$INV/unresolved-nodes.txt" + + # ---- completeness check 2: namespace claims nobody claimed ------------- + # Not this chain's business to delete, but worth surfacing: a claim here is + # either another chain's or already leaked. + jq -r '[ .items[].metadata.name ] | unique | .[]' "$INV/pvcs.json" > "$INV/ns-claims.txt" + sort -u "$INV/all-claims.txt" "$INV/imported-claims.txt" > "$INV/attributed.txt" + comm -23 "$INV/ns-claims.txt" "$INV/attributed.txt" > "$INV/unattributed-claims.txt" + + printf '== must disappear (controller-managed) ==\n'; cat "$INV/managed-claims.txt" + printf '== must survive (imported) ==\n'; cat "$INV/imported-claims.txt" + if [ -s "$INV/unattributed-claims.txt" ]; then + printf '== unattributed claims in this namespace (leak sweep, not this teardown) ==\n' + cat "$INV/unattributed-claims.txt" + fi + + if [ -s "$INV/unresolved-nodes.txt" ]; then + printf 'INVENTORY INCOMPLETE — SeiNodes that resolve to no storage\n' + cat "$INV/unresolved-nodes.txt" + printf 'No certificate written; the verifier will report UNVERIFIED.\n' + exit 2 + fi + + # The certificate names the target it certifies. A complete inventory for a + # DIFFERENT namespace or chain must not authorize this one, and verify_teardown + # compares this string against the target it was called with. + printf 'OK eng-%s sei.io/seinetwork=%s\n' "$ALIAS" "$CHAIN" > "$INV/status" + ``` + + Claim names come from the **pods' own `spec.volumes[].persistentVolumeClaim.claimName`**, not from a guessed naming rule — the controller owns how it names a generated claim, and a rule inferred here would desync the moment it changes. + + **A node with no pod resolves to nothing, and that is the leak case, not a nuisance.** The controller reconciles each SeiNode into a StatefulSet (`seinode-crd.md`), so a node whose StatefulSet has no pod — scaled down, unschedulable, evicted — still has its PVC and its EBS volume. The old version of this inventory dropped that node's claim silently and the teardown then verified clean. Check 1 makes the gap executable: the node lands in `unresolved-nodes.txt`, the script exits non-zero, and **no certificate is written at all** — the `status` file simply does not exist, which `read_inventory` reports as `UNVERIFIED`. The verifier reads that file, and also reads `unresolved-nodes.txt` directly, so either one alone is enough to fail the run. + + > **Attribution caveat.** A pod is attributed to a node by its **first owner reference's name matching the SeiNode name**. `seinode-crd.md` documents the one-StatefulSet-per-SeiNode shape but not the name the controller gives it, so this is a convention, not a contract. If it does not hold, the node lands in `unresolved-nodes.txt` and the run stops — the failure direction is safe. Confirm with `kubectl get pod -n eng- -o jsonpath='{.metadata.ownerReferences[0].name}'` before assuming an empty `unresolved-nodes.txt` means full coverage. + + > **Field-path caveat.** `.spec.dataVolume.import.pvcName` is read from `sei-protocol/sei-k8s-controller` `api/v1alpha1/seinode_types.go` on **repo main** (`DataVolume` → nested `Import` → `PVCName`), not from the CRD deployed on harbor. Confirm against the live cluster before trusting an empty imported list — `kubectl explain seinode.spec.dataVolume.import` — and if the deployed CRD disagrees, **the CRD wins**. An empty `imported-claims.txt` from a wrong path is indistinguishable from a chain that genuinely imports nothing, and it silently reclassifies a preserved claim as one that must disappear. +3. **Check `deletionPolicy` on every SeiNetwork in the task dir** — read it with the command in [Read the current policy](#read-the-current-policy). On `Retain` (or empty), halt and route to [Set it to `Delete`](#set-it-to-delete). Do not open the removal PR while a SeiNetwork still reads `Retain`. +4. **Confirm the policy landed in git and on the object** — the committed manifest and the live object must both read `Delete`. The live object alone is not enough: Flux reverts a policy that git still declares `Retain`, and it does so on its own schedule, which can fall inside the removal PR's review window. This is the gate for step 5. +5. **Remove the manifests** — `git rm -r engineers///` **and** remove the `` entry from `engineers//kustomization.yaml`'s `resources:` list. Both edits are required: Kustomize fails to render with a missing-resource entry, and Flux then applies nothing at all. +6. **Commit + push** — branch `feat/eng--teardown-`. Commit message: `feat(eng/): tear down — chain-id=`. +7. **Open the PR** — title `feat(eng/): tear down `. The body names the chain-id, every CR that goes away, the `deletionPolicy` value the SeiNetwork now carries in git and on the live object, and which path set it. `gh pr create --repo sei-protocol/harbor-engineering-workspace --base main`. +8. **After merge — reconcile and verify** — [Verify the teardown](#verify-the-teardown). A merged PR is not a completed teardown. +9. **Report what survives** — the chain-id's S3 genesis artifacts are **not** purged by teardown, so the chain-id is burned. A later respin uses a fresh chain-id or purges the `/` prefix in `harbor-sei-k8s-genesis-artifacts` first. + +## Verify the teardown + +Two separate questions, and the second is the one that catches a leak: did the right reconciler run, and did the objects actually disappear? + +### Target the workspace Kustomization, not `flux-system` + +The engineer's manifests are applied by the Flux `Kustomization ` in namespace `eng-`, which watches `harbor-engineering-workspace` at `./engineers/` and reconciles every 5 minutes. The root `flux-system` Kustomization tracks `sei-protocol/platform` at `clusters/harbor`. Reconciling `flux-system` after a workspace-repo merge reconciles a different repository and reports success without applying the engineer's change. + +Use `flux-system` after a **platform**-repo merge (onboarding). Use `` after a **workspace**-repo merge (every chain, bench, and teardown). + +```sh +flux --context harbor reconcile kustomization -n eng- --with-source +``` + +Fallback when `flux` is absent: + +```sh +kubectl --context harbor -n eng- annotate kustomization \ + reconcile.fluxcd.io/requestedAt="$(date +%s)" --overwrite +``` + +Then confirm the merge commit landed: + +```sh +kubectl --context harbor -n eng- get kustomization \ + -o jsonpath='{.status.lastAppliedRevision}' +``` + +Compare that revision to the merge commit SHA. A stale revision means Flux has not applied the removal yet, so any disappearance check below is premature. + +A `Forbidden` on `--with-source` **may** mean the `GitRepository` the Kustomization references sits outside `eng-`, beyond the engineer's namespace-scoped Role. It may equally be an expired session, a missing EKS access entry, or a Role that never carried the Flux verbs. Read the message before concluding which. Dropping `--with-source` helps only the first cause: reconciling the Kustomization alone applies the revision the source has already fetched, and the source polls on its own schedule. It repairs nothing for an expired session, a missing access entry, or a Role without the Flux verbs — those fail the same way with or without the flag. **The fallback has to succeed on its own terms.** If the reconcile without `--with-source` also fails, you have no reconcile at all: stop, fix the access problem, and do not proceed to the disappearance check, whose result would be `UNVERIFIED` anyway. + +### Confirm the resources disappeared + +A successful reconcile says Flux applied the change. It does not say the objects are gone. Deletion is asynchronous and finalizers hold objects in `Terminating` while the controller releases their PVCs, so poll instead of asserting once. + +**Three outcomes, and they are not interchangeable:** + +| Outcome | Meaning | What to report | +|---|---|---| +| `GONE` | The API answered and matched nothing. | Teardown verified for these objects. | +| `PRESENT` | The API answered and objects remain at the deadline. | Not torn down. Read the finalizers below. | +| `UNVERIFIED` | The API call failed — `Forbidden`, expired credential, connection error. | **Teardown not confirmed.** Say the check could not run. | + +**A failed API read is never a pass.** A `Forbidden` or a dropped connection returns zero lines, and a check that counts lines without reading the exit status prints "gone" precisely when it cannot see the cluster. + +**And a later success must never overwrite an earlier failure.** Printing `UNVERIFIED` is not enough on its own: a `break` out of a loop, or a bare call whose return code nobody reads, still leaves the block exiting 0. A human sees the warning; a wrapper script or an agent reading `$?` sees success. Every check records its outcome into a running verdict, and the worst one wins. + +**All of that lives in one function.** `verify_teardown` in `cluster-inspection-recipes.md` recipe #9 carries the completeness gate, the checked list reads, the empty-list branches, the polls, and the aggregation. This file calls it and reads its return value — there is deliberately no verification shell here to drift out of step with the library: + +```sh +. ./verify-lib.sh # the library block from recipe #9 + +rc=0 +verify_teardown eng- seinetwork,seinode,pod \ + "sei.io/seinetwork=" ./teardown-inventory- || rc=$? +record "$rc" + +# An interrupt exits the function through its trap at 130/143, bypassing its own +# closing message — so say something here rather than exiting nonzero in silence. +case "$VERDICT" in + 0|1|2) : ;; + *) echo "VERIFICATION ABORTED — unexpected status $VERDICT (interrupted?); treat as unverified" ;; +esac +exit "$VERDICT" +``` + +That is the whole verification step. **If you find yourself writing a `poll_gone` line in this file, stop** — a second copy of the orchestration is how this exact defect survived four review rounds, reappearing one layer up each time: duplicated poll bodies, then the caller chain, then the arguments feeding the callers, then a canonical caller that bypassed the fixed library entirely while the paragraph above it said not to re-implement. + +The empty-list cases are handled inside the function, as code rather than as advice here: an empty `managed-claims.txt` takes a `NOTE` branch instead of calling `poll_gone` with no names, because `kubectl get persistentvolumeclaim` with no arguments lists the whole namespace — the sweep this design exists to avoid. + +**Zero PVCs is the wrong expectation, and a namespace sweep is the wrong check.** The SeiNode finalizer deliberately skips an imported claim, so those survive by design, and other chains' claims are none of this teardown's business. Both make a sweep report `PRESENT` after a correct teardown — a false alarm that trains the reader to ignore the check. The expected end state is precise: every controller-managed claim of this chain gone, every imported claim still present. + +A controller-managed PVC that outlives its SeiNode is a held disk. Take it to [Find and clean up already-leaked resources](#find-and-clean-up-already-leaked-resources). + +### A stuck `Terminating` object is a real signal + +If the poll runs out with objects still present, do not report the teardown as done and do not force the objects away. Read why they are held: + +```sh +kubectl --context harbor get seinetwork,seinode -n eng- \ + -l sei.io/seinetwork= \ + -o custom-columns='NAME:.metadata.name,PHASE:.status.phase,DELETED:.metadata.deletionTimestamp,FINALIZERS:.metadata.finalizers' +``` + +`sei.io/seinode-finalizer` on a SeiNode blocks its deletion until the controller releases the PVC. A node parked there means the controller is unhealthy or the EBS CSI driver flaked — check `kubectl logs -n sei-k8s-controller-system -l app.kubernetes.io/name=sei-k8s-controller --tail=100` and surface what it says. Removing the finalizer by hand (`troubleshooting-seinode.md` → *PVC stuck after delete*) abandons the PVC and its disk, which is the leak this file exists to prevent. Take that step only after the engineer accepts the orphaned PVC, and record the PVC name so somebody can clean it up. + +## Procedure: empty or remove my namespace + +"Destroy my namespace" means one of two very different things. Ask which before acting. + +### What a workspace-repo PR removes + +A workspace-repo PR governs `engineers//` only. Removing every task dir removes: + +- Every SeiNetwork and SeiNode the engineer's manifests declared, and their pods, StatefulSets, headless Services, and controller-managed PVCs. +- Every bench Job and ConfigMap. +- Any engineer-owned exposure YAML (`Service`, `HTTPRoute`) in those task dirs. + +It does **not** remove: + +- **The `Namespace` object.** It comes from the platform repo (`clusters/harbor/engineers/base/namespace.yaml`) and stays. +- The three ServiceAccounts, the `` Role and RoleBinding, or the `engineer-admin` RoleBinding — all platform-owned. +- The Flux `Kustomization ` itself. It keeps reconciling an empty `engineers//`. +- Anything created outside git — an escape-hatch direct apply, a `SeiNodeTaskWorkflow`, or an orphaned SeiNode from an earlier `Retain` teardown. Flux prune only reaches what Flux applied. +- S3 artifacts: the chain-ids' genesis prefixes and the bench results under `harbor-validation-results/eng-/`. + +### Empty the namespace (the common case) + +1. **List what is there, then inventory each chain properly.** The display read below is an overview, not an inventory — it produces none of the named-claim files the verifier consumes, so it cannot stand in for step 2 of the per-chain procedure: + + ```sh + kubectl --context harbor get seinetwork,seinode,job,pvc -n eng- + ``` + + Then discover the chain-ids **with the discovery's own status checked**. Piping `kubectl` into `sort` exits with sort's status, so a `Forbidden` becomes a successful empty list — and "no chains found" then reads as "nothing to do", which is the whole defect class this document exists to close: + + ```sh + . ./verify-lib.sh # recipe #9 — provides record, VERDICT, verify_teardown + VERDICT=0 + derr=$(mktemp) || { echo 'UNVERIFIED: cannot create temp file'; exit 2; } + chains='' + + if raw=$(kubectl --context harbor get seinode -n eng- \ + -o jsonpath='{range .items[*]}{.metadata.labels.sei\.io/seinetwork}{"\n"}{end}' 2>"$derr") + then + # The API read is checked above. The TRANSFORMATIONS need checking too: + # `| sort -u` exits with sort's status and `|| true` swallows everything, + # so a failed grep or sort yields a successful EMPTY chain list — the loop + # then runs zero times and every certificate and claim check is skipped. + # Only grep's exit 1 (nothing matched) is legitimate emptiness. + if filtered=$(printf '%s\n' "$raw" | grep -v '^$'); then fs=0; else fs=$?; fi + if [ "$fs" -gt 1 ]; then + echo 'UNVERIFIED: chain-id filter failed — cannot enumerate this namespace' + record 2 + elif chains=$(printf '%s\n' "$filtered" | sort -u); then + : + else + echo 'UNVERIFIED: chain-id sort failed — cannot enumerate this namespace' + record 2; chains='' + fi + else + echo 'UNVERIFIED: chain discovery failed — the namespace inventory is unknown' + cat "$derr" >&2; record 2 + fi + rm -f "$derr" + ``` + + Run `inventory.sh` **once per chain-id**, each writing its own `./teardown-inventory-`. A namespace usually holds more than one chain, and a single sweep cannot tell one chain's controller-managed claim from another's. Any chain whose `inventory.sh` exits non-zero writes no certificate, and its verification then reports `UNVERIFIED` — emptying a namespace on an incomplete inventory is how a leak becomes invisible. Claims no chain attributes land in each run's `unattributed-claims.txt`; take those to the leak sweep in step 5, not to a delete. +2. For every SeiNetwork in the inventory, run the `deletionPolicy` gate in [The `deletionPolicy: Retain` trap](#the-deletionpolicy-retain-trap). One `Retain` network is enough to leak a set of disks. +3. `git rm -r` every task dir under `engineers//`, and reduce `engineers//kustomization.yaml` to `resources: []`. Keep that file: deleting it makes the Flux Kustomization fail reconcile with `path not found`, which is the same breakage the onboarding scaffolding PR exists to prevent. +4. Open the PR, merge, then verify **every chain, retaining the worst result**. One `VERDICT` spans the whole namespace, so a clean second chain cannot cover an unverified first one: + + ```sh + # Same shell as step 1 — the library is already sourced and $VERDICT already + # carries a 2 if discovery or either of its transformations failed. + for c in $chains; do + rc=0 + verify_teardown eng- seinetwork,seinode,pod \ + "sei.io/seinetwork=$c" "./teardown-inventory-$c" || rc=$? + record "$rc" + done + + # Anything left that carries no chain label at all — an escape-hatch apply, + # or an orphan whose labels were stripped. `-` means NO storage is in scope + # here: this is a CR/pod disappearance check ONLY, and on its own it proves + # nothing about residual claims. + rc=0 + verify_teardown eng- seinetwork,seinode,pod "" - || rc=$? + record "$rc" + ``` + + **Do not print a verdict yet.** The check that can contradict "the namespace is empty" has not run: a namespace whose only leftover is a leaked PVC passes everything above — successful discovery, a clean CR/pod poll, exit 0. +5. **Sweep the residuals, then decide.** Build the expected-survivors list from every chain's imported claims, sweep what is left, and only then print and exit: + + ```sh + # Imported claims are EXPECTED to survive, and this list is what exempts + # them. A STALE list exempts the wrong things, so it is built in a fresh + # per-namespace directory and every step that produces it is checked. + # + # `: > file` is the specific trap. In bash without `set -e` a failed + # redirection on that builtin leaves the OLD file intact and execution + # continues, so a previous run's list would silently authorise this run's + # exemptions. dash aborts instead — which does not make the bash + # configuration this document supports any safer. + SURV_DIR="./teardown-residual-eng-" + surv="$SURV_DIR/expected-survivors.txt" + if rm -rf "$SURV_DIR" && mkdir -p "$SURV_DIR" && : > "$surv"; then + : + else + echo 'UNVERIFIED: cannot create a fresh expected-survivors list' + record 2 + surv='-' # NEVER fall back to a stale list; '-' means "no exemptions" + fi + + if [ "$surv" != '-' ]; then + for c in $chains; do + f="./teardown-inventory-$c/imported-claims.txt" + if [ ! -f "$f" ]; then + printf 'UNVERIFIED: no imported-claims list for chain %s — exemptions incomplete\n' "$c" + record 2 + elif ! cat "$f" >> "$surv"; then + printf 'UNVERIFIED: cannot read the imported-claims list for chain %s\n' "$c" + record 2 + fi + done + fi + + rc=0; sweep_residual eng- "$surv" || rc=$?; record "$rc" + + case "$VERDICT" in + 0) echo 'NAMESPACE EMPTIED — every chain verified; no unaccounted PVCs, Jobs, CronJobs, Services or ConfigMaps remain' ;; + 1) echo 'NAMESPACE NOT EMPTY — objects or unaccounted resources remain' ;; + 2) echo 'NAMESPACE UNVERIFIED — at least one check could not run' ;; + *) echo "NAMESPACE VERIFICATION ABORTED — unexpected status $VERDICT (interrupted?); treat as unverified" ;; + esac + exit "$VERDICT" + ``` + + With `surv='-'` the sweep runs with **no** exemptions, so genuinely imported claims are reported as residuals. That is the safe direction and it is not the verdict: `record 2` already fired, and `2` dominates the `1` a residual would raise. + + **What the sweep covers, and what it does not.** `sweep_residual` inspects `persistentvolumeclaim`, `job`, `cronjob`, `service` and `configmap` — kinds that keep costing money or quota, `service` included because a leftover `type: LoadBalancer` bills with no pod running and the per-tenant ResourceQuota caps load balancers. It does **not** enumerate every Kind in the namespace. Report what was swept rather than "nothing remains". + + **What it reports are candidates, not confirmed leftovers.** The sweep filters by **Kind**, not by ownership. Its only ownership-aware exemptions are the imported claims and `kube-root-ca.crt`; anything else of a swept Kind is listed whoever created it, so a platform-owned or controller-created `Service`, `CronJob` or `ConfigMap` **will** appear. Kinds outside the list — the `Namespace`, the ServiceAccounts, the RBAC, the Flux `Kustomization` — are not swept, which is not the same as being recognised and excluded. Resolve ownership before acting on any line it prints; the escalation rule in [find and clean up already-leaked resources](#find-and-clean-up-already-leaked-resources) already governs that, and nothing here authorizes a deletion. + + > **Ownership evidence is repository-level, not live.** The claim that `Service` and `CronJob` are engineer-owned in `eng-` rests on the platform base listing neither (`onboarding-pr.md` → *Base layer (already in place)*). That is what the repo declares, not what the cluster holds — a controller or an add-on can create either without appearing there. Confirm against the live namespace before treating a swept `Service` or `CronJob` as an engineer's leftover. + + Then take anything `sweep_residual` reported, plus what git never owned, to [Find and clean up already-leaked resources](#find-and-clean-up-already-leaked-resources). + + An empty `$chains` after a **successful** discovery is legitimate — the namespace has no labelled chains — and both the unlabelled check and the residual sweep still run. An empty `$chains` after a **failed** discovery, **or after a failed filter or sort**, already recorded `2`, so the loop running zero times cannot pass. Do not substitute a namespace-wide PVC poll for a chain's claim check: that matches imported and unattributed claims too, so it reports `PRESENT` after a correct teardown. `sweep_residual` is the opposite question — what is left over that no chain accounted for — and it excludes the imported claims by name. + +### Remove the namespace entirely (offboarding) + +This is a platform-repo change and the engineer cannot do it from the workspace repo. It reverses the onboarding PR: delete `clusters/harbor/engineers//`, remove `` from `clusters/harbor/engineers/kustomization.yaml`, remove `eng-` from `clusters/harbor/monitoring/podmonitor-seiload-eng.yaml`, delete `terraform/aws/189176372795/eu-central-1/harbor/engineers/.tf`, and run the targeted `terraform apply` to drop the six Pod Identity resources. + +**Empty the namespace first, through the steps above.** This is an operational preference, not a claim about a failure mode: the Kubernetes namespace controller does remove namespaced resources on its own. Emptying first keeps the `deletionPolicy` gate, the disappearance poll, and the leak sweep available while the objects are still there to inspect. Once the namespace is going away, a `Retain` SeiNetwork's orphans are much harder to reason about, and there is no inventory left to check them against. + +The file list mirrors the onboarding shape in `onboarding-pr.md`; the reverse flow has no worked example in this skill. Surface it to the platform team through `#harbor-onboarding` rather than opening the PR unassisted. + +## Find and clean up already-leaked resources + +Run this after any teardown that ran under `Retain`, and any time an engineer asks where the harbor spend is going. + +### Orphaned SeiNodes + +An orphaned validator has **no `ownerReferences`** and no live parent SeiNetwork. Absence of owner references alone is not the signal: a follower applied through `seictl node apply` is a top-level object and legitimately has none. The signature is `sei.io/role=validator` **and** no owner references. + +**An unreadable sweep is not a clean sweep.** Piping `kubectl` straight into `jq` hands `jq` no input when the read fails, and `jq` exits 0 with no output — which reads as "no orphans". Check the read and the parse separately: + +```sh +errf=$(mktemp) || { echo 'UNRESOLVED: cannot create temp file'; exit 2; } +if raw=$(kubectl --context harbor get seinode -n eng- \ + -l sei.io/role=validator -o json 2>"$errf") +then :; else + echo 'UNRESOLVED: orphan sweep read failed — this is NOT "no orphans"' + cat "$errf" >&2; rm -f "$errf"; exit 2 +fi +rm -f "$errf" + +if orphans=$(printf '%s' "$raw" | jq -r '.items[] + | select((.metadata.ownerReferences // []) | length == 0) + | "\(.metadata.name)\t\(.metadata.labels["sei.io/seinetwork"] // "-")\t\(.status.phase // "-")\t\(.metadata.creationTimestamp)"') +then :; else + echo 'UNRESOLVED: orphan sweep parse failed'; exit 2 +fi + +if [ -z "$orphans" ]; then echo 'no candidate orphans'; else printf '%s\n' "$orphans"; fi +``` + +Each line is a validator still running with nothing that will ever delete it. Confirm the parent is gone before treating one as an orphan: + +```sh +kubectl --context harbor get seinetwork -n eng- +# NotFound → the parent is gone and this node is orphaned +``` + +### Candidate disks — and why neither signal proves anything on its own + +**Nothing in this section identifies garbage. It identifies candidates.** Two readings look conclusive and are not: + +- **EC2 `state: available` does not mean unowned.** It means unattached. A volume backing a live PV whose PVC is `Bound` reads `available` the moment its workload stops — a scaled-to-zero StatefulSet, a pod stuck `Pending`, a node drained mid-reschedule. Deleting on that signal destroys a disk somebody is coming back to. +- **`Used By: ` does not prove the pod belongs to an orphan**, and `Used By: ` does not prove the PVC is unwanted. `describe pvc` reports current pod attachment, not ownership. + +Treat both as **candidate** signals, then resolve ownership before calling anything garbage. + +```sh +# Candidate list only. Scoped to this tenant; do not widen the filter. +aws ec2 describe-volumes --region eu-central-1 --profile \ + --filters "Name=tag:kubernetes.io/created-for/pvc/namespace,Values=eng-" \ + --query 'Volumes[].{id:VolumeId,state:State,size:Size,created:CreateTime,pvc:Tags[?Key==`kubernetes.io/created-for/pvc/name`]|[0].Value}' \ + --output table +``` + +Those tag keys are the EBS CSI driver's own convention rather than something this skill's repos set. To confirm they are present before trusting an empty result, describe **one volume you already know is live, by ID** — never re-run without `--filters`, which enumerates every volume in the account including other tenants': + +```sh +aws ec2 describe-volumes --region eu-central-1 --profile \ + --volume-ids --query 'Volumes[].Tags' --output table +``` + +### Resolve ownership before calling a disk garbage + +Walk the chain from the volume back to a workload. Each hop either names an owner or fails, and a failed hop means unresolved, not unowned. + +Every hop separates the API call from the parse, and checks both. A `kubectl … | jq …` pipeline exits with `jq`'s status, so a `Forbidden` would read as "no match found" — which on this walk is the difference between *unowned* and *could not look*. + +```sh +# ---- hop 1: volume ID → PV. The CSI volume handle is the EBS volume ID. ----- +raw=$(kubectl --context harbor get pv -o json 2>&1) || { + printf 'UNRESOLVED: PV list failed — cannot tell unowned from unreadable\n%s\n' "$raw" + exit 2; } +pv=$(printf '%s' "$raw" | jq -r --arg v '' '.items[] + | select(.spec.csi.volumeHandle == $v) + | "\(.metadata.name)\t\(.status.phase)\t\(.spec.persistentVolumeReclaimPolicy)\t\(.spec.claimRef.namespace // "-")\t\(.spec.claimRef.name // "-")"') || { + printf 'UNRESOLVED: PV parse failed\n'; exit 2; } +[ -n "$pv" ] || echo 'no PV references this volume — see the verdict table' +printf '%s\n' "$pv" +``` + +**Hop 2 is a scope gate, not just a lookup.** `kubectl get pv` is cluster-scoped, so the `claimRef` it returns can name *any* namespace. Assert it is this tenant's before inspecting further — a claim in another namespace is another tenant's disk, and this skill does not investigate those. + +```sh +# ---- hop 2: claimRef → PVC, inside this tenant only ------------------------ +claim_ns=; claim= +if [ "$claim_ns" != "eng-" ]; then + printf 'OUT OF SCOPE: volume claimed by %s/%s — escalate, do not inspect\n' "$claim_ns" "$claim" + exit 2 +fi +kubectl --context harbor get pvc "$claim" -n eng- --ignore-not-found -o name \ + || { echo 'UNRESOLVED: PVC read failed'; exit 2; } +``` + +```sh +# ---- hop 3: PVC → the pod that mounts it → that pod's owner --------------- +# This is the hop that names WHICH node references the candidate. Listing every +# SeiNode in the namespace does not establish a relationship to this claim. +raw=$(kubectl --context harbor get pods -n eng- -o json 2>&1) || { + printf 'UNRESOLVED: pod list failed\n%s\n' "$raw"; exit 2; } +if users=$(printf '%s' "$raw" | jq -r --arg c "$claim" '.items[] as $p + | $p.spec.volumes[]? | select(.persistentVolumeClaim.claimName == $c) + | "pod=\($p.metadata.name)\towner=\($p.metadata.ownerReferences[0].kind // "-")/\($p.metadata.ownerReferences[0].name // "-")\tnetwork=\($p.metadata.labels["sei.io/seinetwork"] // "-")"') +then :; else + echo 'UNRESOLVED: pod parse failed — cannot tell "no pod mounts it" from "could not look"' + exit 2 +fi +if [ -z "$users" ]; then + echo 'UNRESOLVED: no pod currently mounts this claim — a stopped workload looks identical' + exit 2 +fi +printf '%s\n' "$users" +``` + +An empty hop-3 result means **no pod currently mounts the claim**. That is not evidence the claim is unwanted — it is exactly the stopped-workload state that made the volume read `available` in the first place. Treat it as unresolved. + +The owner reference names the StatefulSet the controller created for the node, not the SeiNode directly. Map it back to a SeiNode by name and confirm that node is a **confirmed orphan** by the signature in [Orphaned SeiNodes](#orphaned-seinodes). If you cannot make that link, the hop is unresolved. + +| What the walk found | Verdict | +|---|---| +| Volume → PV → PVC → pod → a SeiNode confirmed orphaned by the signature | Reclaimable. Delete the **SeiNode**, not the volume — see below. | +| Volume → PV → PVC → pod → a live, wanted workload | **Not garbage.** Leave it. `available` only meant the workload was stopped. | +| Volume → PV → PVC whose claim is gone, PV `Released` | **Unresolved candidate.** Platform review required. Report the PV, PVC name, and reclaim policy; do not act on it here. | +| Volume → no PV, no claimRef, tags name a PVC that no longer exists | **Unresolved candidate.** The tag is provenance, not ownership. Platform review required. | +| PVC exists but no pod mounts it | **Unresolved.** A stopped workload looks identical to an abandoned claim from here. | +| `claimRef` names a namespace other than `eng-` | **Out of scope.** Another tenant's disk. Escalate; do not inspect. | +| Any hop returned `Forbidden`, errored, or found nothing | **UNRESOLVED.** Escalate as unresolved. Never as confirmed-safe. | + +Only the first two rows are verdicts. Every other row is an escalation, and the platform team is told which row it came from. + +`kubectl get pv` is cluster-scoped and the per-engineer Role is namespaced, so `Forbidden` at hop 1 is the **normal** case for an engineer — an unresolved result, not a clean one. When it happens, hand the volume IDs to the platform team and let them walk the chain; do not substitute the tag data for the walk. + +The engineer's SSO profile may lack `ec2:DescribeVolumes`. On `AccessDenied`, surface the ask to the platform team with the namespace and the orphaned node names; do not treat the denial as "no leaked disks". + +### Clean them up + +**Delete the orphaned SeiNode. That is the whole cleanup for a held disk.** + +```sh +kubectl --context harbor delete seinode -n eng- +``` + +The node's finalizer deletes its data PVC, and the `Delete` reclaim policy on `gp3-10k-750` (validators) and `gp3` (default) releases the EBS volume. `gp3-archive` is `Retain` by design — a volume on that class stays after its PVC goes, and its removal is an AWS-side decision, not a mistake to correct here. + +Two checks before you run it: + +- The node must be a confirmed orphan by the signature above. `kubectl delete seinode` against a follower that still has a manifest in the workspace repo is undone by the next Flux reconcile, and the safer `git rm` path never lands. +- Poll the disappearance and the PVC afterwards, exactly as in [Verify the teardown](#verify-the-teardown). An orphan can stick in `Terminating` for the same finalizer reasons. + +An imperative `kubectl delete` is right for a confirmed orphaned SeiNode because the object was never in git, so there is no manifest to `git rm`. The same reasoning covers the other non-Git resources below; it never covers anything Flux owns. + +**Never delete an EBS volume from this skill.** Even a volume the ownership walk resolved to a dead PVC goes to the platform team: `ec2:DeleteVolume` is outside the engineer's policy, the walk can be wrong, and an EBS delete is unrecoverable. Hand over the volume IDs, sizes, creation times, and the walk's verdict per volume — including every `UNRESOLVED` one, labelled as unresolved. Escalate through `#harbor-onboarding`. Do not report the cleanup as complete while any ID is outstanding. + +### The other resources git never owned + +Deleting an orphaned SeiNode is the one cleanup with a paved road. The rest of what a workspace PR leaves behind needs its own handling, so nothing in the [what a workspace-repo PR removes](#what-a-workspace-repo-pr-removes) list is left with no next step: + +| Resource | Why git never owned it | What to do | +|---|---|---| +| Orphaned validator SeiNode | Controller-generated, then owner-reference stripped | Delete it, per above. Confirm the orphan signature first. | +| SeiNetwork/SeiNode from an escape-hatch direct apply | Applied with `seictl` outside the PR flow | Prove no workspace manifest names it — see the ownership search below — then gate on `deletionPolicy` exactly as a Flux-owned network, then delete with `kubectl --context harbor delete seinetwork\|seinode -n eng-`. **Not `seictl delete`** — see the context note below. If a manifest does exist, it is Flux-owned: use the PR path. | +| `SeiNodeTaskWorkflow` | Never committed to the workspace repo, by Guardrail #9 | A `Complete` workflow is the deliberate audit trail — leave it. Force-delete only a `Failed` workflow holding a node, with the `sei.io/force-delete-workflow` annotation first (`seictl-cli.md`). | +| Bench Job/ConfigMap applied by hand | Ran outside the PR flow | Same ownership search first, then `kubectl --context harbor delete job -n eng-` / `… delete configmap -n eng-`, by name. Results already in S3 are untouched and are not garbage. | +| Controller-managed PVC with no SeiNode | The controller owns PVC lifecycle; the engineer's Role has no `delete` on PVCs | Escalate with the PVC name and its PV. Do not request the verb. | +| S3 genesis prefixes, bench results | Never Kubernetes objects | Out of scope for teardown. Purging a `/` genesis prefix is a deliberate act that unburns the chain-id; the engineer decides. | + +Anything not in this table, or any case where the ownership question stays open, escalates as unresolved rather than getting a guess. + +**Every namespace-scoped command above names its namespace, and every destructive one names its context.** An unqualified `delete` deletes wherever the shell happens to point, and that is not a typo you can retry — it is a delete in the wrong place. + +**This is why the direct deletes above use `kubectl`, not `seictl`.** `seictl`'s common flags are `--kubeconfig` and `-n/--namespace` only (`seictl-cli.md` → *Common flags on every verb*) — **there is no `--context`**, and the namespace falls back to the kubeconfig context's default. A `seictl delete` therefore cannot pin the cluster on its own command line; writing "(harbor context)" beside it states an intention the command does not enforce. `kubectl --context harbor delete -n eng-` pins both on the line that does the deleting, and issues the same Delete against the same CR (`seictl-cli.md` → `seictl network|node delete`). + +**No `seictl` alternative for network or node deletion belongs here, guarded or otherwise.** A `kubectl config current-context` check reads mutable state rather than pinning the config the delete then consumes, so it leaves a window between the check and the call — and it buys nothing, because `kubectl --context harbor delete` does the same deletion against the same CR with no window at all. + +Should some genuinely `seictl`-only destructive verb ever need documenting, the pin belongs on the invocation: hand it a kubeconfig that contains the harbor cluster and nothing else, via `--kubeconfig ` (a documented `seictl` flag). A file that cannot name another cluster cannot select one. Do not substitute a current-context check. + +### The ownership search that authorizes a direct delete + +Before deleting anything imperatively, prove the object is **not** in the workspace repo. A search that fails must never read as "no manifest found" — `grep` exits 1 for no match and 2 or more for an error, and an unreadable or stale clone produces the same empty output as a genuinely absent manifest. + +```sh +# Run inside a FRESH clone of harbor-engineering-workspace at origin/main. +# A stale working copy can miss a manifest somebody merged an hour ago. +git -C fetch origin main && git -C checkout -q origin/main \ + || { echo 'UNRESOLVED: cannot refresh the workspace clone — do not delete'; exit 2; } + +grep -rn -- '' /engineers// && gs=0 || gs=$? +case "$gs" in + 0) echo 'FLUX-OWNED: a manifest names it — use the PR path, do not delete'; exit 1 ;; + 1) echo 'NOT IN GIT: safe to consider for a direct delete, after the other gates'; exit 0 ;; + *) echo 'UNRESOLVED: the search itself failed — do not delete'; exit 2 ;; +esac +``` + +**Only exit status 0 from this block authorizes a direct delete.** Every branch used to end in a successful `echo`, so the block's own status was 0 whatever it found — a scripted caller could not tell "not in git" from "the search failed", which is the same class of defect as counting lines without reading an exit status. `1` routes to the PR path; `2` means the question was never answered. + +## Halt conditions + +Stop and report. Do not auto-remediate. + +- **A SeiNetwork in the teardown reads `deletionPolicy: Retain` or empty.** Removing it orphans the validators and leaks their disks. Halt before opening the removal PR; route to [Set it to `Delete`](#set-it-to-delete). +- **The removal PR merged while a SeiNetwork still read `Retain`.** The cascade is gone and no patch restores it. Do not re-apply the SeiNetwork to "reattach" the children — a fresh network under the same chain-id wedges at height 0 on the burned genesis artifacts. Go straight to [Find and clean up already-leaked resources](#find-and-clean-up-already-leaked-resources). +- **`kustomization ` is `NotFound` in `eng-`.** The engineer's Flux wiring is missing, so no workspace-repo merge reconciles at all. Surface to the platform team; do not create the Kustomization. +- **`lastAppliedRevision` does not reach the merge commit within two reconcile intervals (~10 min).** Read the Ready condition's message (`cluster-inspection-recipes.md` recipe #8). A render error in `engineers//kustomization.yaml` — most often a `resources:` entry pointing at the dir that was just removed — blocks every later apply in the namespace, not only this teardown. +- **An object is still `Terminating` after the poll budget.** Report the finalizer and the controller's log line. Do not strip the finalizer to make the check pass. +- **A verification read returned `UNVERIFIED`.** The API call failed, so the teardown state is unknown. Report it as unknown — never as verified-gone, and never as still-present. Re-run once the access problem is fixed; a teardown with an unverified check is not a finished teardown. +- **A `deletionPolicy` patch landed on the live object but git still declares `Retain`.** Flux reverts it, and the removal PR may merge after the revert. Halt and land the policy in git before the removal. +- **An EBS volume's ownership walk did not resolve.** Any hop that returned `Forbidden`, errored, or found nothing leaves the disk unresolved. Escalate it as unresolved with the volume ID; do not present it as confirmed garbage, and do not delete it. +- **Orphaned SeiNodes found in a namespace the engineer does not own.** Cross-tenant cleanup is out of scope. Hand the platform team the namespace and the node names. +- **`aws ec2 describe-volumes` returns `AccessDenied`.** The leak check did not run. Say that, rather than reporting a clean result. diff --git a/.claude/skills/harbor-dev/references/troubleshooting-seinode.md b/.claude/skills/harbor-dev/references/troubleshooting-seinode.md index 9b95f65f..01acf6b4 100644 --- a/.claude/skills/harbor-dev/references/troubleshooting-seinode.md +++ b/.claude/skills/harbor-dev/references/troubleshooting-seinode.md @@ -62,8 +62,8 @@ kubectl delete seinode -n eng- ``` **PVC behavior** — verify before deleting on stateful nodes: -- For **imported** PVCs (`spec.import` set on the SeiNode): the PVC is preserved; the recreated SeiNode reuses existing data. -- For **controller-managed** PVCs (no `spec.import`): the controller's `handleNodeDeletion` path deletes the PVC during teardown. Delete-and-recreate **wipes data**. Safe for ephemeral chains being recreated from genesis; not safe for archive nodes or any chain with state worth preserving. +- For **imported** PVCs (`spec.dataVolume.import` set on the SeiNode): the PVC is preserved; the recreated SeiNode reuses existing data. +- For **controller-managed** PVCs (no `spec.dataVolume.import`): the controller's `handleNodeDeletion` path deletes the PVC during teardown. Delete-and-recreate **wipes data**. Safe for ephemeral chains being recreated from genesis; not safe for archive nodes or any chain with state worth preserving. ## SeiNetwork genesis plan stuck @@ -116,6 +116,8 @@ Manual override (only after confirming PVC orphan is acceptable): kubectl patch seinode -p '{"metadata":{"finalizers":[]}}' --type=merge ``` +This abandons the PVC and its EBS disk — the finalizer is what deletes the PVC, so removing it is how a stuck teardown becomes a leaked disk. Take it only with the engineer's explicit acceptance, record the PVC name, and follow up with the sweep in `teardown.md` → *find and clean up already-leaked resources*. + ### HTTPRoute hostname unreachable 1. `kubectl get httproute -n -o yaml` — verify `parentRefs` points at the shared Gateway. @@ -314,4 +316,4 @@ PVC space won't fully release until the original files are also unlinked (compac ### vs. retained data on delete -For a SeiNode, whether its PVC survives deletion is governed by `spec.import` (imported PVC = preserved) vs controller-managed (wiped on teardown) — documented under **Phase: Failed** above. A `SeiNetwork`'s `spec.deletionPolicy` (defaults `Retain`) governs whether the controller orphans its generated validator SeiNodes (and thus their PVCs) when the network is deleted — useful when tearing down a network but keeping a validator's disk for forensics. The hardlink trick above is for **live debugging** while the node continues running. They're complementary, not redundant. +For a SeiNode, whether its PVC survives deletion is governed by `spec.dataVolume.import` (imported PVC = preserved) vs controller-managed (wiped on teardown) — documented under **Phase: Failed** above. A `SeiNetwork`'s `spec.deletionPolicy` (defaults `Retain`) governs whether the controller orphans its generated validator SeiNodes (and thus their PVCs) when the network is deleted. Forensics is the one case where `Retain` is the right answer; on an ordinary teardown it is a disk leak, because the orphaned validators keep running with no owner left to delete them (see `teardown.md`). The hardlink trick above is for **live debugging** while the node continues running. The two are complementary, not redundant.