You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Replace the tools/cleanup bash script with a standalone Go test-automation binary that reuses AICR's own libraries (pkg/recipe, pkg/k8s) to clean up a cluster in a recipe-aware way.
This is explicitly not an aicr subcommand: cluster teardown is a CI / test-harness concern, not something the product CLI (snapshot → recipe → validate → bundle) should surface. The binary lives under tools/ (or cmd/) and is invoked by test automation and by developers resetting a dev cluster.
Follow-up to #1672 / PR #1773, which fixed the immediate data-loss bugs in the bash script (missed check-created namespaces; no exclusion for out-of-band installs). The bash fix stands on its own; this issue captures the better long-term design surfaced during that review.
Motivation
The current bash script hardcodes two lists that duplicate knowledge elsewhere and drift over time:
AICR_NAMESPACES — a static copy of defaultNamespace entries from recipes/registry.yaml. Add a component, forget to update cleanup, and cleanup silently misses it.
AICR_CRD_PATTERNS — a hand-maintained list of CRD API groups that lives nowhere structured today.
A Go tool can do better on three axes:
Recipe-scoped cleanup (the main prize). Resolve a recipe through pkg/recipe and clean up exactly what that recipe installs (namespaces + release names derived from the resolved component set). This solves tools/cleanup: misses check-created namespaces; deletes out-of-band installs #1672's part 2 at the root: a component the recipe excludes is never in the cleanup set, so there is nothing to fence out. The --exclude-* flags become a fallback rather than the primary safety mechanism.
Convention alignment. Use pkg/k8s/pod watch-based waits (the repo forbids poll-sleep), pkg/errors with codes, context timeouts, and errgroup fan-out for the independent read-only probes. No new distributed artifact beyond the Go toolchain we already build with.
Registry gap: CRD API groups are not modeled.registry.yaml records helm repo/chart/version and defaultNamespace, but not the CRD groups a chart installs. To make CRD cleanup registry-driven rather than a hand-maintained list, add a crdGroups: [...] field per component in registry.yaml. This is a prerequisite enhancement.
Helm uninstall. Reimplementing uninstall in Go means vendoring the Helm SDK (heavy) or still shelling out to the helm binary. Shelling to helm uninstall is likely the pragmatic choice ("boring first"); client-go handles CRDs, namespaces, and the finalizer-rescue.
Summary
Replace the
tools/cleanupbash script with a standalone Go test-automation binary that reuses AICR's own libraries (pkg/recipe,pkg/k8s) to clean up a cluster in a recipe-aware way.This is explicitly not an
aicrsubcommand: cluster teardown is a CI / test-harness concern, not something the product CLI (snapshot → recipe → validate → bundle) should surface. The binary lives undertools/(orcmd/) and is invoked by test automation and by developers resetting a dev cluster.Follow-up to #1672 / PR #1773, which fixed the immediate data-loss bugs in the bash script (missed check-created namespaces; no exclusion for out-of-band installs). The bash fix stands on its own; this issue captures the better long-term design surfaced during that review.
Motivation
The current bash script hardcodes two lists that duplicate knowledge elsewhere and drift over time:
AICR_NAMESPACES— a static copy ofdefaultNamespaceentries fromrecipes/registry.yaml. Add a component, forget to update cleanup, and cleanup silently misses it.AICR_CRD_PATTERNS— a hand-maintained list of CRD API groups that lives nowhere structured today.A Go tool can do better on three axes:
pkg/recipeand clean up exactly what that recipe installs (namespaces + release names derived from the resolved component set). This solves tools/cleanup: misses check-created namespaces; deletes out-of-band installs #1672's part 2 at the root: a component the recipe excludes is never in the cleanup set, so there is nothing to fence out. The--exclude-*flags become a fallback rather than the primary safety mechanism.pkg/k8s/podwatch-based waits (the repo forbids poll-sleep),pkg/errorswith codes, context timeouts, anderrgroupfan-out for the independent read-only probes. No new distributed artifact beyond the Go toolchain we already build with.tools/cleanup_test.sh) added in PR fix(cleanup): backstop check namespaces; fence out-of-band installs #1773.Design notes / open questions
registry.yamlrecords helm repo/chart/version anddefaultNamespace, but not the CRD groups a chart installs. To make CRD cleanup registry-driven rather than a hand-maintained list, add acrdGroups: [...]field per component inregistry.yaml. This is a prerequisite enhancement.helmbinary. Shelling tohelm uninstallis likely the pragmatic choice ("boring first"); client-go handles CRDs, namespaces, and the finalizer-rescue.--dry-run,--keep-crds/--keep-namespaces, and the exclusion semantics from tools/cleanup: misses check-created namespaces; deletes out-of-band installs #1672.tools/vscmd/and how test automation invokes it (Make target, directgo run, or a built binary in the e2e/kwok harnesses).Acceptance criteria
crdGroupsfield added toregistry.yamland consumed for CRD cleanup.tools/cleanup_test.shand the bash script.tools/cleanup/make cleanup.