yconverge kubectl passthrough + support converge-mode#3
Merged
Conversation
…gs to debug Six zap.Info call sites moved to zap.Debug: - yconverge.go: "converge plan", "converge dependency", "converge target" -- the orchestration scaffolding around what gets applied. With kubectl wrapping coming next they'd be noise on top of kubectl's own output. - checks.go: the per-check "check" lines for wait / rollout / exec. The check kinds emit their own progress to the user (kubectl wait, kubectl rollout status, the exec command's stdout) so a structured pre-line is just duplication. - k8sapply.go: the per-resource "applied" line. Same logic -- kubectl apply already prints `<kind>/<name> <verb>` in the shape developers know. `-v` / --verbose still surfaces all of these. The next two commits switch yconverge's apply / wait / rollout to kubectl shellouts so the default output matches what `kubectl yconverge` users expect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switches yconverge's three user-visible operations off client-go helpers and onto direct `kubectl` invocations with stdout / stderr forwarded to the host process. The wire effect is identical (server-side apply with --force-conflicts, --field-manager=y-cluster, the same wait / rollout-status semantics) -- what changes is what the developer sees: $ kubectl yconverge --context=local -k base configmap/db-config serverside-applied service/db serverside-applied deployment.apps/backend condition met instead of the structured zap "applied / check" lines from the client-go path. Why: kubectl-yconverge's audience already lives in kubectl's output shape. Wrapping the API instead of forking the binary gave us typed errors but stripped the line shape every consumer parses mentally for create / configured / unchanged signals. pkg/k8sapply and pkg/k8swait stay -- they're used by pkg/provision/envoygateway.Install where the audience is operators reading provision logs, not developers iterating on a kustomize tree, and the typed errors / CRD-first ordering matter for that path's retry semantics. pkg/yconverge/kubectl.go is the new home for the helpers. They are private to the package; the public API of pkg/yconverge (Run, Options, Result) is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five label-routed kubectl invocations match the bash plugin's
order from ystack@90e8923 one-for-one. `pkg/k8sapply.Apply`'s
"every resource is server-side-force" was a regression: ystack
kustomizations annotate immutable resources (Jobs by name) with
yolean.se/converge-mode=replace, one-shots with =create, and
expect those modes' semantics. The Go binary silently overrode
all of them with SSA-force.
The five steps:
1. yolean.se/converge-mode=create kubectl create --save-config
(tolerates AlreadyExists +
empty selector match -> the
documented "skip if exists")
2. yolean.se/converge-mode=replace kubectl delete
(tolerates "No resources found"
on the first run)
3. yolean.se/converge-mode=serverside-force
kubectl apply --server-side
--force-conflicts
4. yolean.se/converge-mode=serverside kubectl apply --server-side
5. label!=create,!=serverside,!=serverside-force
kubectl apply
(replace-mode resources land
here too, freshly applied
after step 2's delete)
Each step's stderr is captured: tolerated substrings ("no objects
passed to apply" when a selector matches nothing, "AlreadyExists"
on create idempotency) suppress the whole invocation; anything
else is forwarded to the user's stderr before the error
propagates so the diagnostic isn't swallowed.
--dry-run=server forwards through every step, including the
delete, so a replace-mode resource's plan is provably non-
mutating end-to-end -- the bash plugin's behaviour preserved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…esToSIGKILL
`sleep infinity` is a GNU coreutils extension. macOS BSD sleep
rejects it with "invalid time interval" -- bash then exits
before the test has a chance to send SIGTERM, and stopVM
short-circuits through the "process is already dead" branch.
The test passed by coincidence on Linux because GNU sleep
accepts the literal; on macOS the elapsed-time assertion
caught the race ("181us; SIGKILL escalation path not exercised").
`sleep 60` is portable and well past the test's 6s grace
budget.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pulls the per-step argv builder out of kubectlApply into a pure applySteps(opts) []applyStep so the kubectl invocation each mode produces is testable without spawning kubectl. Five tests pin the contract: - Order + count: the bash plugin runs five label-routed steps in a fixed order (create -> delete -> serverside-force -> serverside -> plain-apply); a refactor that reshuffles them fails loudly. - Per-step argv: full slice match for non-dry-run Options. Drift in --save-config / --server-side / --force-conflicts / --field-manager=y-cluster / the negative selector formula shows up as a test diff rather than as silent semantic change against a cluster. - Dry-run forwarding: --dry-run=server must reach every step including delete, so a replace-mode resource's dry-run is provably non-mutating end-to-end. - No dry-run by default: an empty Options.DryRun must not inject --dry-run=anything. - Tolerated stderr substrings: the documented idempotent paths (empty selector match, AlreadyExists on re-create) are swallowed; anything else surfaces. Loosening these would silently hide a real failure. The runtime keeps using applySteps; the refactor is a mechanical extraction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three e2e tests against the shared kwok cluster, each exercising one mode-specific contract that the previous "every resource is SSA-force" path got wrong: - TestConvergeMode_CreateSkipsIfExists: the regression that motivated the branch. label=create renders to `kubectl create --save-config`, which means a re-run with edited source data must NOT update the cluster value. Run twice with foo=original -> foo=changed; assert cluster keeps "original". - TestConvergeMode_ReplaceRecreates: label=replace runs delete + apply, so the resource UID changes between runs even when the spec is byte-identical. Captures uid1 from run 1, uid2 from run 2; uid1 != uid2. - TestConvergeMode_MixedKustomization: a single kustomize tree with all four mode buckets (create / serverside-force / serverside / unlabelled) lands cleanly. Doubles as the tolerated-stderr smoke: the four un-used mode steps must swallow "no objects passed to <verb>" rather than failing the whole run. The output-forwarding contract (kubectl-style per-resource lines reaching the user's stdout) is the next commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The whole motivation for switching yconverge from client-go SSA to kubectl shellouts was so the developer running `kubectl yconverge` sees the line shapes they're used to from running kubectl directly. Pin that with a subprocess-based e2e test: - Plain-apply step prints "configmap/<name> created" on first run -- kubectl client-side-apply's verb. - serverside-force step prints "configmap/<name> serverside-applied" -- distinct verb because the step uses --server-side. - "No resources found" (kubectl delete's empty-match line on stdout, exit 0) MUST NOT reach the user; otherwise a vanilla yconverge run is noisy. The empty-match assertion forced a small fix in runKubectlTolerant: kubectl delete writes "No resources found" to STDOUT (not stderr) with exit 0. The previous design only suppressed stderr on tolerated substrings, so the line leaked through. Add a parallel stdoutSuppress list and route the delete step's empty-match handling through it. The bash plugin used the same two-channel approach (stderr-tolerate + stdout-suppress) for the same reason. applyStep grows a stdoutSuppress field; argv unit tests updated to pin both channels' contracts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds TestConvergeMode_DependencyGetsSameApplyPath. Builds a tmp CUE module with a base/ kustomization carrying converge-mode=replace on its ConfigMap, imported by a dependent/ kustomization. Runs yconverge against the dependent twice; asserts the base ConfigMap's UID changes between runs. If the dep path silently bypassed label routing (e.g. a future "fast path" that just shells out a single kubectl apply per dep step instead of the five-step plan), the UID would stay the same on the second run and the test fails. The wiring is shared in code (Run -> convergeSingle -> kubectlApply -> applySteps for both deps and the final target), but until now no test proved that identity. Reading the source is not sufficient: a future refactor that introduces a different code path for deps would compile and pass the existing ordering tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four headers, written to opts.Stdout (defaults to os.Stdout) in
the same sink as forwarded kubectl output:
yconverge dependency <relpath> before each dep step
yconverge converge-mode=<mode> before each non-empty mode bucket
(suppressed alongside an empty
bucket's kubectl output)
yconverge target <relpath> before the final step, only
when at least one dep ran
yconverge check N/total <kind> before each check, before it runs
Sample run shape:
$ kubectl yconverge --context=local -k example-replace-dependent
yconverge dependency yconverge/itest/example-replace
yconverge converge-mode=replace
job.batch "example-replace-job" deleted from default namespace
job.batch/example-replace-job created
yconverge target yconverge/itest/example-replace-dependent
configmap/example-replace-dependent unchanged
yconverge check 1/1 exec
Design choices:
- Plain text on stdout, NOT zap. These lines are part of the
command's UI, like kubectl's `<kind>/<name> created` lines;
zap is for diagnostics on stderr (`-v` keeps the existing
Debug "converge plan / target" structured lines available).
No duplication: events are emitted in exactly one channel.
- Replace-mode's two kubectl invocations (delete + recreate)
group under one "converge-mode=replace" header. applyGroups
encodes the structure: each group is one mode and holds
the one or two invocations behind it. The runner emits the
header once per group, only if at least one invocation
produced non-suppressed output.
- Default (unlabelled) bucket gets no header -- kubectl's
per-resource lines speak for themselves. The selector now
also excludes replace so we don't double-apply the resources
the replace group's second invocation already created.
CheckRunner gains a Stdout field that yconverge.convergeSingle
threads through; defaults to os.Stdout.
kubectlWait / kubectlRolloutStatus take an io.Writer arg so
their kubectl output lands on the same sink as the apply path
(otherwise check progress would race with applies on stdout).
Five unit tests in pkg/yconverge/kubectl_test.go pin the group
structure (count, mode order, replace's two invocations,
default-excludes-replace, dry-run forwarding, no-dry-run-by-
default, tolerance contract).
Three e2e tests in e2e/converge_progress_test.go run the binary
as a subprocess and assert:
- The four headers appear in expected order on a deps+target+check
fixture.
- A no-deps run does NOT print 'yconverge target' (user already
knows what they passed).
- An unlabelled-only run does NOT print any 'yconverge converge-mode='
header.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
solsson
changed the title
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
1 participant
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.
Reintroduces behavior from the bash kubectl-yconverge impl + adds dependency and checks output.