Repository navigation
fix(auto): kates auto --dry-run introspects the cluster read-only - #396
Merged
Merged
Conversation
`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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Item 1.9 of the CLI performance and stability plan (finding F22). Independent of the other Wave 1 PRs.
Why
kates auto --dry-runwrites to the cluster. On main (6f4cf61),cli/cmd/auto.go:52never setscollector.ReadOnly, askates deploy --dry-rundoes (cli/cmd/deploy.go:173). So a dry run's introspection still creates:dns-detectpod when no running pod can be read.auto.go:56,65also callos.ExitinsideRunE.Changes
cli/cmd/auto.go:collector.ReadOnly = autoDryRun.defaultExecutor, the OS executor thatkates deployuses too, rather than a freshdetect.NewOSExecutor(). Outside tests this is the same executor, and a test can stand in for kubectl.cmdErr, with the same message and the same exit 1, instead ofos.Exit(1).Tests
Added
TestAutoDryRunCollectsReadOnly. It runsrunAutoagainst the recording executorkates 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'sclusterWritepattern:--dry-run, none matches.kubectl create ns,kubectl run prober-…,kubectl run … dns-detectandkubectl label ns.With main's
auto.go(given only thedefaultExecutorseam, 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.shandscripts/check-cli-compat.sh: OK.cli/**triggers (Build & Test): this PR touches nothing underkates/../mvnw teston main'skates/tree ran 1,254 tests with 0 failures and 0 errors, once for all of Wave 1.Skipped locally:
govulncheck, asgo.modis unchanged.CI only:
./mvnw verifywith the*ITtests, the Kind and Docker jobs, CodeQL and Trivy.Deliberately not done
os.Exit(2).docs/book/10-cli-reference.md:2506documents exit 2 for "kates autocould not inspect a cluster it reached", and an error returned fromRunEexits 1. Carrying an exit code throughExecuteis the exit-code scheme the robustness plan owns, so a comment marks the line and it stays.autostill buffers ten minutes ofhelm --wait --debugoutput silently (also in F22). It is not part of item 1.9.