fix(controller): re-apply worker resources when tracked object is missing - #445
frossbeamish wants to merge 2 commits into
Conversation
…sing The WRT apply path skips the SSA apply whenever the rendered hash matches the LastAppliedHash recorded in the WRT status. The hash alone is not a safe signal: when a version is sunset its rendered resources are deleted, but the status entry for that build ID can survive (the status update that would drop it can hit a conflict, or the build can be re-registered before the next successful status write). If the same build ID returns, the stale hash matches the unchanged render and the resource is never re-created. For a WRT rendering a ScaledObject this leaves the returning version's Deployment with no autoscaler: the controller starts it at 1 replica expecting the scaler to own replicas, and nothing ever scales it. Guard the skip with an existence check on the rendered object (a cache read), falling through to the SSA apply on NotFound or on an indeterminate error. Extract the WRT apply block into applyWorkerResourceTemplates so it is unit-testable in isolation.
|
|
|
@frossbeamish hey, sorry about this, but the code this PR touches was recently changed to fix a related issue where the sort order of drained versions was causing unintended updates to the WD status. I'm afraid you will need to rebase and see how to fit this PR into that new code path. |
|
@frossbeamish ping on this. couple requests... a) please sign the CLA so we can move forward on this PR, b) as noted in my comment above, a bunch of this code changed since you submitted this PR and will need to be rebased and c) please create a tracking issue on Github that describes how to reproduce the issue you are seeing (if you are still seeing it!) Thank you kindly! |
3 participants
What
Guards the WRT apply hash-skip with an existence check on the rendered object. If the object referenced by an unchanged
LastAppliedHashno longer exists, the controller falls through to the SSA apply (create-or-update) instead of skipping. Also extracts the WRT apply block fromexecutePlanintoapplyWorkerResourceTemplatesso it can be unit-tested without a Temporal client.Why
We hit this in a live cluster running v1.8.0. Sequence:
DeleteWorkerResources).status.versionsentry for that build ID survives the window (the status update that would drop it can hit an optimistic-concurrency conflict, and the build can be re-registered before the next successful status write, in our case the same build ID was re-registered ~4 minutes after the sunset delete).RenderedHash == LastAppliedHashand the SSA apply is skipped forever.spec.Replicas == nilpath, expecting the scaler resource to own replicas), but the ScaledObject is never re-created. Result: the current version runs a single unscaled pod, which in our case OOM-looped under the full task-queue load and stalled all workflows on that queue.The hash match tells the controller the render is unchanged; it says nothing about whether the applied object still exists. The existence check is a cache read against an informer the controller already maintains for these GVKs, so the API-server-load rationale for the skip is preserved.
Testing
internal/controller/execplan_wrt_test.go:go test ./internal/controller/ ./internal/planner/green;go build ./...,go vet,gofmtclean.