You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A Deployment or StatefulSet whose rollout is stuck, or whose controller has not observed the current spec, could report Healthy. This PR makes the default converging and grace handlers of both kinds use one rule for health, so a stuck
rollout reports Updating during the grace period and Degraded or Down after it. A StatefulSet with the OnDelete
strategy and a paused Deployment still report Healthy once their replicas are ready, because their controllers do not
roll out new pods on their own.
Changes
DefaultConvergingStatusHandler of both kinds reports Healthy only when the controller observed the current
generation, all desired replicas are ready, and the rollout is complete. A ready but unfinished rollout reports Updating (Creating right after create).
Deployment rollout check: updatedReplicas must equal the desired count, and replicas must not be more than updatedReplicas (no old pods remain, which matters with a surge). A paused Deployment only waits for extra replicas to scale down,
because the deployment controller scales it but does not roll it out. The generation and readiness checks still apply.
StatefulSet: replicas must not be more than the desired count under every update strategy, because the
controller removes extra pods whatever the strategy is. The rollout check then follows the update strategy:
RollingUpdate without a partition: updatedReplicas equals the desired count and currentRevision equals updateRevision.
RollingUpdate with a partition more than zero: updatedReplicas reaches the desired count minus the partition.
The revisions are not compared, because the pods below the partition keep the current revision.
OnDelete: no rollout check.
DefaultGraceStatusHandler of both kinds reports Down when replicas are desired and none are ready, Healthy only
when the converging handler reports Healthy, and Degraded for all other states, including a stale observedGeneration. This removes the "Grace inconsistency detected" log for these cases.
New "Status Handlers" sections in docs/primitives/deployment.md and docs/primitives/statefulset.md, synced to the
plugin references. GoDoc on the builders and resources no longer says that the handlers compare only ReadyReplicas.
Challenges
The StatefulSet controller sets currentRevision to updateRevision only when a rolling update completes, and it
never does so while a partition holds pods back. That is why the revision comparison applies only to RollingUpdate
without a partition. A partition at or above the replica count gives a target of zero updated replicas, so such a
StatefulSet is healthy when its replicas are ready. The partition counts from spec.ordinals.start, as in the
controller, so the target does not depend on the start ordinal.
Related
The DaemonSet grace handler has the same stale-generation gap when desiredNumberScheduled > 0, and neither DaemonSet
handler reads updatedNumberScheduled.
The ReplicaSet grace handler does not check observedGeneration. A ReplicaSet has no rollout, so only the generation
part applies.
The MessageQueue grace handler example in docs/custom-resource.md copies the same pattern without a generation
check.
Testing
TestDefaultHandlers_UnfinishedRollout in both packages calls the converging handler (what the component reports
before the grace period expires) and the grace handler (what it reports after) on the same object. The cases are a
stale generation with all replicas ready, a stale generation with none ready, a rollout stuck with all old replicas
ready, and a just-created object. The StatefulSet cases add a revision mismatch with all replicas updated and a
partitioned rollout that is not done. Every case except the stale generation with none ready failed before the fix. That
case pins the Down result. TestDefaultHandlers_RolloutHeldBack covers
the partition (also with a custom start ordinal) and OnDelete cases that must stay Healthy. TestDefaultHandlers_PausedRollout covers a paused Deployment: Healthy with all old replicas ready, and not healthy
with a stale generation or missing ready replicas. A case with more updated replicas than desired (a scale-down in progress) reports Waiting for scale-down in both packages, also for a StatefulSet with a partition or OnDelete and for a paused
Deployment. The existing healthy fixtures now set updatedReplicas
and replicas, because the contract now reads them.
A new e2e spec, "Grace Period — Stuck Rollout", rolls a two-replica Deployment with maxUnavailable: 0 to a pod
template whose readiness probe always fails. It checks that the status shows two ready and one updated replica, that the
condition is Updating before the grace period expires, and that it is Degraded after. make all passes, and make e2e-primitives passes for PRIMITIVE=deployment and PRIMITIVE=statefulset.
The default status handlers of the Deployment and StatefulSet primitives
read only ReadyReplicas. The deployment controller keeps old pods until
new pods are ready, so a rollout whose new pods never become ready kept
ReadyReplicas at the desired count. Convergence then reported Healthy.
The grace handlers also did not check observedGeneration. With a stale
generation, convergence reported Updating and grace reported Healthy.
After the grace period the component logged "Grace inconsistency
detected" and kept the progress reason.
Both handlers of each kind now use the same rule. The controller must
observe the current generation, all desired replicas must be ready, and
the rollout must be complete. For a Deployment, updatedReplicas must
reach the desired count and no old replicas can remain. For a
StatefulSet, the rule follows the update strategy:
- RollingUpdate without a partition: updatedReplicas reaches the desired
count and currentRevision equals updateRevision.
- RollingUpdate with a partition: updatedReplicas reaches the desired
count minus the partition. The revisions stay different by design.
- OnDelete: no rollout check, because the controller does not replace
pods.
The grace handlers report Down when replicas are desired and none are
ready, Healthy only when convergence is Healthy, and Degraded otherwise.
Fixes#217
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hange
The deployment controller does not roll out a paused Deployment. After a
pod template change, updatedReplicas stays below the desired count until
the Deployment is resumed, so the new rollout check reported Updating and
then Degraded for a pause that the user chose. The default handlers now
skip the rollout check for a paused Deployment. The generation and
readiness checks still apply.
The stuck-rollout e2e spec now stores its readiness switch in an
atomic.Bool, because the reconcile goroutine reads it while the test
goroutine writes it.
The StatefulSet docs now say that the partition counts from
spec.ordinals.start, as in the statefulset controller, and a test pins
the partition target for a custom start ordinal.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Some GoDoc sentences for the Deployment and StatefulSet status handlers
were false for some inputs:
- The converging handlers listed which progress status each operation
gives. A ready count that differs is checked before the rollout, and
a stale generation gives Updating for any operation, so the list was
wrong. The GoDoc now only says that the handler reports Creating,
Updating, or Scaling.
- The grace handlers said Down needs Spec.Replicas above zero. A nil
Spec.Replicas counts as 1, so the GoDoc now names the desired count.
- The builder and resource GoDoc said that the rollout is complete,
which is not true for a paused Deployment or an OnDelete StatefulSet.
They now refer to the exceptions in DefaultConvergingStatusHandler.
- The StatefulSet GoDoc said that an OnDelete rollout is always
complete. It now says that Healthy does not mean the pods run the
update revision.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 1. The three Copilot threads have their own replies. The review had no suppressed comments. This comment records one more finding from a local review, which has no thread.
Finding
Verdict
Change
The rollout check ignored Spec.Paused. After a paused Deployment's template changed, it reported Updating and then Degraded, although the controller never rolls out a paused Deployment.
Accepted
Fixed in b48faa5. The rollout check now skips a paused Deployment. The generation and readiness checks still apply. TestDefaultHandlers_PausedRollout covers it, and the GoDoc and docs/primitives/deployment.md say so.
cfdf4d0 corrects handler GoDoc sentences that were false for some inputs, for example a nil Spec.Replicas.
Poll for Updating before waiting on replica counts
e2e/primitives/deployment_test.go:391
This assertion can fail on a slow cluster even when the handlers work correctly. The preceding replica-status wait allows 120 seconds, but grace expires 30 seconds after the condition becomes unready. If the expected counts arrive after grace expires, reconciliation can already have changed the condition to Degraded; this permanently stuck rollout will not return to Updating. Poll for Updating immediately after triggering the template change, before waiting for the replica counts. Use the normal timeout to allow the initial reconciliation to start.
The stuck rollout spec waited up to two minutes for the stall counts
before it checked for Updating. The grace period is 30 seconds, so on a
slow cluster the condition could already be Degraded at that check, and
a stuck rollout never returns to Updating. The spec now checks Updating
right after the template change, then waits for the stall counts.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 2. Copilot resolved the three round-1 findings. It reported one more finding in the review body, under "Previously missed", so the finding has no thread.
Finding
Verdict
Change
e2e/primitives/deployment_test.go: the stuck-rollout spec waited up to 120s for the stall counts before it checked for Updating. The grace period is 30s, so on a slow cluster the condition could already be Degraded.
Accepted
Fixed in 0eb25d4. The spec checks for Updating right after the template change, then waits for the stall counts, then for Degraded after the grace period. The deployment e2e passes 9 of 9.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Deployment health can still be reported before excess current-revision replicas are removed.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Deployment health passes before updated replicas reach desired count
pkg/primitives/deployment/handlers.go:84
With 3 desired replicas, ReadyReplicas == 3 and UpdatedReplicas == Replicas == 4, both handlers still return Healthy. This can occur while the latest ReplicaSet is scaling down: no old replicas remain, but the total count has not reached the desired count. Require UpdatedReplicas to equal the desired count, retaining the existing check for old replicas. Add this case to the regression tests and update the Deployment GoDoc and documentation to match.
The rollout check accepted updatedReplicas above the desired count. A
Deployment whose newest ReplicaSet still scales down could report
Healthy with 4 updated replicas and 3 desired, while one of the 4 pods
was not ready. A StatefulSet without a partition had the same gap.
Both handlers now require updatedReplicas to equal the desired count,
and report "Waiting for scale-down" while it is higher. A StatefulSet
with a partition keeps the lower bound only, because all of its pods
count as updated when the revision did not change.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 3. Copilot reported one more finding in the review body, under "Previously missed", so the finding has no thread.
Finding
Verdict
Change
pkg/primitives/deployment/handlers.go: with 3 desired, 3 ready, and 4 updated and 4 total replicas, both handlers returned Healthy while the new ReplicaSet was still scaling down.
Accepted
Fixed in 354bb65. UpdatedReplicas must now equal the desired count, and the handlers report Waiting for scale-down: 4/3 replicas. The StatefulSet had the same gap for RollingUpdate without a partition, and the same commit fixes it there. Each kind has a new test case, and the GoDoc and docs say "equals".
The StatefulSet handlers returned early for OnDelete and for a
partitioned RollingUpdate, so a StatefulSet with more replicas than
desired reported Healthy under those strategies. The controller removes
extra pods whatever the update strategy is, so both handlers now check
status.replicas against the desired count before the strategy rules and
report "Waiting for scale-down" while it is higher.
A paused Deployment had the same gap: the paused early return skipped
the scale-down check. The deployment controller still scales a paused
Deployment, so the paused branch now waits for status.replicas to reach
the desired count.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Description
Fixes #217
A Deployment or StatefulSet whose rollout is stuck, or whose controller has not observed the current spec, could report
Healthy. This PR makes the default converging and grace handlers of both kinds use one rule for health, so a stuckrollout reports
Updatingduring the grace period andDegradedorDownafter it. A StatefulSet with theOnDeletestrategy and a paused Deployment still report
Healthyonce their replicas are ready, because their controllers do notroll out new pods on their own.
Changes
DefaultConvergingStatusHandlerof both kinds reportsHealthyonly when the controller observed the currentgeneration, all desired replicas are ready, and the rollout is complete. A ready but unfinished rollout reports
Updating(Creatingright after create).updatedReplicasmust equal the desired count, andreplicasmust not be more thanupdatedReplicas(no old pods remain, which matters with a surge). A paused Deployment only waits for extra replicas to scale down,because the deployment controller scales it but does not roll it out. The generation and readiness checks still apply.
replicasmust not be more than the desired count under every update strategy, because thecontroller removes extra pods whatever the strategy is. The rollout check then follows the update strategy:
RollingUpdatewithout a partition:updatedReplicasequals the desired count andcurrentRevisionequalsupdateRevision.RollingUpdatewith a partition more than zero:updatedReplicasreaches the desired count minus the partition.The revisions are not compared, because the pods below the partition keep the current revision.
OnDelete: no rollout check.DefaultGraceStatusHandlerof both kinds reportsDownwhen replicas are desired and none are ready,Healthyonlywhen the converging handler reports
Healthy, andDegradedfor all other states, including a staleobservedGeneration. This removes the "Grace inconsistency detected" log for these cases.docs/primitives/deployment.mdanddocs/primitives/statefulset.md, synced to theplugin references. GoDoc on the builders and resources no longer says that the handlers compare only
ReadyReplicas.Challenges
The StatefulSet controller sets
currentRevisiontoupdateRevisiononly when a rolling update completes, and itnever does so while a partition holds pods back. That is why the revision comparison applies only to
RollingUpdatewithout a partition. A partition at or above the replica count gives a target of zero updated replicas, so such a
StatefulSet is healthy when its replicas are ready. The partition counts from
spec.ordinals.start, as in thecontroller, so the target does not depend on the start ordinal.
Related
desiredNumberScheduled > 0, and neither DaemonSethandler reads
updatedNumberScheduled.observedGeneration. A ReplicaSet has no rollout, so only the generationpart applies.
MessageQueuegrace handler example indocs/custom-resource.mdcopies the same pattern without a generationcheck.
Testing
TestDefaultHandlers_UnfinishedRolloutin both packages calls the converging handler (what the component reportsbefore the grace period expires) and the grace handler (what it reports after) on the same object. The cases are a
stale generation with all replicas ready, a stale generation with none ready, a rollout stuck with all old replicas
ready, and a just-created object. The StatefulSet cases add a revision mismatch with all replicas updated and a
partitioned rollout that is not done. Every case except the stale generation with none ready failed before the fix. That
case pins the
Downresult.TestDefaultHandlers_RolloutHeldBackcoversthe partition (also with a custom start ordinal) and
OnDeletecases that must stayHealthy.TestDefaultHandlers_PausedRolloutcovers a paused Deployment:Healthywith all old replicas ready, and not healthywith a stale generation or missing ready replicas. A case with more updated replicas than desired (a scale-down in progress) reports
Waiting for scale-downin both packages, also for a StatefulSet with a partition orOnDeleteand for a pausedDeployment. The existing healthy fixtures now set
updatedReplicasand
replicas, because the contract now reads them.A new e2e spec, "Grace Period — Stuck Rollout", rolls a two-replica Deployment with
maxUnavailable: 0to a podtemplate whose readiness probe always fails. It checks that the status shows two ready and one updated replica, that the
condition is
Updatingbefore the grace period expires, and that it isDegradedafter.make allpasses, andmake e2e-primitivespasses forPRIMITIVE=deploymentandPRIMITIVE=statefulset.🤖 Generated with Claude Code