Skip to content

fix(auto): kates auto --dry-run introspects the cluster read-only - #396

Merged
bmscomp merged 1 commit into
mainfrom
fix/auto-dry-run-read-only
Oct 18, 2026
Merged

bmscomp merged 1 commit into
mainfrom
fix/auto-dry-run-read-only

Conversation

@bmscomp

@bmscomp bmscomp commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Item 1.9 of the CLI performance and stability plan (finding F22). Independent of the other Wave 1 PRs.

Why

kates auto --dry-run writes to the cluster. On main (6f4cf61), cli/cmd/auto.go:52 never sets collector.ReadOnly, as kates deploy --dry-run does (cli/cmd/deploy.go:173). So a dry run's introspection still creates:

  • a namespace and a Secret to audit Secret creation;
  • prober pods to measure inter-zone latency;
  • a dns-detect pod when no running pod can be read.

auto.go:56,65 also call os.Exit inside RunE.

Changes

cli/cmd/auto.go:

  • collector.ReadOnly = autoDryRun.
  • The collector's executor is defaultExecutor, the OS executor that kates deploy uses too, rather than a fresh detect.NewOSExecutor(). Outside tests this is the same executor, and a test can stand in for kubectl.
  • A failed preflight is returned through cmdErr, with the same message and the same exit 1, instead of os.Exit(1).

Tests

Added TestAutoDryRunCollectsReadOnly. It runs runAuto against the recording executor kates deploy --dry-run's test uses, with a chart directory that does not exist, so the run stops right after introspection. It then matches every recorded command against that test's clusterWrite pattern:

  • With --dry-run, none matches.
  • Without it, the control, the same introspection runs kubectl create ns, kubectl run prober-…, kubectl run … dns-detect and kubectl label ns.

With main's auto.go (given only the defaultExecutor seam, so the test never calls a real kubectl) the dry run issues those four kinds of write.

Run locally, passing:

  • cd cli && go test ./..., go test -race ./cmd, go vet ./....
  • golangci-lint run --new-from-rev=origin/main ./...: 0 issues.
  • scripts/check-cli-style.sh and scripts/check-cli-compat.sh: OK.
  • The backend suite, which cli/** triggers (Build & Test): this PR touches nothing under kates/. ./mvnw test on main's kates/ tree ran 1,254 tests with 0 failures and 0 errors, once for all of Wave 1.

Skipped locally: govulncheck, as go.mod is unchanged.

CI only: ./mvnw verify with the *IT tests, the Kind and Docker jobs, CodeQL and Trivy.

Deliberately not done

  • The introspection failure keeps os.Exit(2). docs/book/10-cli-reference.md:2506 documents exit 2 for "kates auto could not inspect a cluster it reached", and an error returned from RunE exits 1. Carrying an exit code through Execute is the exit-code scheme the robustness plan owns, so a comment marks the line and it stays.
  • auto still buffers ten minutes of helm --wait --debug output silently (also in F22). It is not part of item 1.9.

`kates auto --dry-run` previews the Helm install, but its introspection ran
every write probe: a namespace and a Secret to audit Secret creation, prober
pods to measure inter-zone latency, and a dns-detect pod. kates deploy
--dry-run sets the collector ReadOnly for that; auto never did. A failed
preflight also ended the process with os.Exit(1) from inside RunE.

The collector is ReadOnly under --dry-run, and auto takes its executor from
defaultExecutor, as deploy does, so a test can stand in for kubectl. A failed
preflight is returned instead; the exit code stays 1. A failed introspection
keeps os.Exit(2), the code the CLI reference documents for it.
TestAutoDryRunCollectsReadOnly records every command: with --dry-run none
writes, and without it the same introspection does.
@bmscomp
bmscomp merged commit 4a3d4b9 into main Oct 18, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant