fix(controller): delete k8s worker Deployments during WD cleanup - #508
Open
tomba7 wants to merge 1 commit into
Open
fix(controller): delete k8s worker Deployments during WD cleanup#508tomba7 wants to merge 1 commit into
tomba7 wants to merge 1 commit into
Conversation
…nt cleanup ## Summary handleDeletion never tore down the child worker k8s Deployments, so their pods kept polling. The server rejects `DeleteVersion` (even with `SkipDrainage=true`) while a version still has active pollers, so version cleanup failed on every retry and the finalizer was never removed, which is the deadlock. ownerRef GC could not break it either, since GC is itself blocked on the finalizer. Add an explicit k8s Deployment teardown step before WD version deletion. Pollers linger in the server cache for a few minutes after pods die, so the later steps requeue until they age out. ## Testing - `go test ./internal/controller -count=1` - `go vet ./internal/controller`
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.
Why the deadlock happened
Two things could remove a version's worker pods:
executePlan, via the sunset path, but only for deprecated versions, which
are already scaled to zero by then and so aren't polling anyway.
ownerReference GC which reaps every child Deployment, active one
included, once the WD object is gone. This is what normally cleans up an active
version.
Note: The active version is the one still running pods, so it's the one we need
gone, and its Deployment has to go before its WDV can be deleted, not after.
Manually deleting the WD (repro step #3) blocks both. Reconcile sees the
deletionTimestamp, enters handleDeletion and returns, never reaching
executePlan. And GC won't touch the children while the finalizer keeps the WD alive.
So the pods keep polling, and the server rejects DeleteVersion since the version
has active pollers, even with SkipDrainage set. Redirecting current to
unversioned doesn't help either since it just reroutes new tasks but doesn't stop
the currently running pods.
So handleDeletion keeps erroring, finalizer never gets removed, pods keep polling,
WDVs on the Temporal server stay non deletable forever. This is the deadlock.
How the fix works
We break the cycle at the only point that isn't blocked, by deleting child Deployments
directly, i.e. by handling it inside handleDeletion itself, before asking the server
to delete the WDV.
The cleanup sequence gains a new step 3:
The new step lists the WD's children with GetDeploymentState, the same lookup normal
reconciliation uses, so it only ever sees this WD's own worker Deployments.
It deletes each one, ignoring NotFound so that retries are idempotent. Any other error
requeues after 10s with the finalizer intact. Iteration is over the DeploymentsByTime
slice rather than the Deployments map so that the log lines stay stable across retries
Deleting the Deployments terminates the pods and stops the polling. The server caches
pollers for a few minutes after they vanish, so steps 4-5 usually still fail on the
first pass, which is expected. And each failure just requeues.
Once the cached pollers age out, DeleteVersion succeeds, the finalizer comes off, and K8s
removes the WD. Everything here is inside the deletion path, so normal rollout and sunset
behavior is untouched.
Testing
Verified manually on a local cluster running Temporal server v1.31.2 with the controller
built from this branch
WD stayed in Terminating, the worker pods stayed Running, and the controller logged
cannot be deleted since it has active pollerson every 10s requeue.the pods terminated, steps 4-5 requeued for ~5 minutes until the cached pollers aged
out, then the finalizer was removed and the WD was deleted.
is gone.
go build ./...andgo test ./internal/controller/...passsedFixes #463.