fix(controller): re-apply worker resources when tracked object is missing#445
Open
frossbeamish wants to merge 1 commit into
Open
fix(controller): re-apply worker resources when tracked object is missing#445frossbeamish wants to merge 1 commit into
frossbeamish wants to merge 1 commit 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.
|
|
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
2 participants
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.
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.