Merge pull request #136245 from soltysh/clarify_scaling

Eliminate duplicate scale methods deployment controller in favor of a parameter
This commit is contained in:
Kubernetes Prow Robot
2026-01-15 19:13:40 +05:30
committed by GitHub
3 changed files with 17 additions and 22 deletions

View File

@@ -83,7 +83,7 @@ func (dc *DeploymentController) scaleDownOldReplicaSetsForRecreate(ctx context.C
if *(rs.Spec.Replicas) == 0 {
continue
}
scaledRS, updatedRS, err := dc.scaleReplicaSetWithLazyAnnotationUpdate(ctx, rs, 0, deployment)
scaledRS, updatedRS, err := dc.scaleReplicaSet(ctx, rs, 0, deployment, false)
if err != nil {
return false, err
}
@@ -127,6 +127,6 @@ func oldPodsRunning(newRS *apps.ReplicaSet, oldRSs []*apps.ReplicaSet, podMap ma
// scaleUpNewReplicaSetForRecreate scales up new replica set when deployment strategy is "Recreate".
func (dc *DeploymentController) scaleUpNewReplicaSetForRecreate(ctx context.Context, newRS *apps.ReplicaSet, deployment *apps.Deployment) (bool, error) {
scaled, _, err := dc.scaleReplicaSetWithLazyAnnotationUpdate(ctx, newRS, *(deployment.Spec.Replicas), deployment)
scaled, _, err := dc.scaleReplicaSet(ctx, newRS, *(deployment.Spec.Replicas), deployment, false)
return scaled, err
}

View File

@@ -72,14 +72,14 @@ func (dc *DeploymentController) reconcileNewReplicaSet(ctx context.Context, allR
}
if *(newRS.Spec.Replicas) > *(deployment.Spec.Replicas) {
// Scale down.
scaled, _, err := dc.scaleReplicaSetWithLazyAnnotationUpdate(ctx, newRS, *(deployment.Spec.Replicas), deployment)
scaled, _, err := dc.scaleReplicaSet(ctx, newRS, *(deployment.Spec.Replicas), deployment, false)
return scaled, err
}
newReplicasCount, err := deploymentutil.NewRSNewReplicas(deployment, allRSs, newRS)
if err != nil {
return false, err
}
scaled, _, err := dc.scaleReplicaSetWithLazyAnnotationUpdate(ctx, newRS, newReplicasCount, deployment)
scaled, _, err := dc.scaleReplicaSet(ctx, newRS, newReplicasCount, deployment, false)
return scaled, err
}
@@ -178,7 +178,7 @@ func (dc *DeploymentController) cleanupUnhealthyReplicas(ctx context.Context, ol
if newReplicasCount > *(targetRS.Spec.Replicas) {
return nil, 0, fmt.Errorf("when cleaning up unhealthy replicas, got invalid request to scale down %s/%s %d -> %d", targetRS.Namespace, targetRS.Name, *(targetRS.Spec.Replicas), newReplicasCount)
}
_, updatedOldRS, err := dc.scaleReplicaSetWithLazyAnnotationUpdate(ctx, targetRS, newReplicasCount, deployment)
_, updatedOldRS, err := dc.scaleReplicaSet(ctx, targetRS, newReplicasCount, deployment, false)
if err != nil {
return nil, totalScaledDown, err
}
@@ -223,7 +223,7 @@ func (dc *DeploymentController) scaleDownOldReplicaSetsForRollingUpdate(ctx cont
if newReplicasCount > *(targetRS.Spec.Replicas) {
return 0, fmt.Errorf("when scaling down old RS, got invalid request to scale down %s/%s %d -> %d", targetRS.Namespace, targetRS.Name, *(targetRS.Spec.Replicas), newReplicasCount)
}
_, _, err := dc.scaleReplicaSetWithLazyAnnotationUpdate(ctx, targetRS, newReplicasCount, deployment)
_, _, err := dc.scaleReplicaSet(ctx, targetRS, newReplicasCount, deployment, false)
if err != nil {
return totalScaledDown, err
}

View File

@@ -311,7 +311,7 @@ func (dc *DeploymentController) scale(ctx context.Context, deployment *apps.Depl
if *(activeOrLatest.Spec.Replicas) == *(deployment.Spec.Replicas) {
return nil
}
_, _, err := dc.scaleReplicaSetWithLazyAnnotationUpdate(ctx, activeOrLatest, *(deployment.Spec.Replicas), deployment)
_, _, err := dc.scaleReplicaSet(ctx, activeOrLatest, *(deployment.Spec.Replicas), deployment, false)
return err
}
@@ -319,7 +319,7 @@ func (dc *DeploymentController) scale(ctx context.Context, deployment *apps.Depl
// This case handles replica set adoption during a saturated new replica set.
if deploymentutil.IsSaturated(deployment, newRS) {
for _, old := range controller.FilterActiveReplicaSets(oldRSs) {
if _, _, err := dc.scaleReplicaSetWithLazyAnnotationUpdate(ctx, old, 0, deployment); err != nil {
if _, _, err := dc.scaleReplicaSet(ctx, old, 0, deployment, false); err != nil {
return err
}
}
@@ -391,7 +391,7 @@ func (dc *DeploymentController) scale(ctx context.Context, deployment *apps.Depl
}
// TODO: Use transactions when we have them.
if _, _, err := dc.scaleReplicaSet(ctx, rs, nameToSize[rs.Name], deployment); err != nil {
if _, _, err := dc.scaleReplicaSet(ctx, rs, nameToSize[rs.Name], deployment, true); err != nil {
// Return as soon as we fail, the deployment is requeued
return err
}
@@ -400,23 +400,18 @@ func (dc *DeploymentController) scale(ctx context.Context, deployment *apps.Depl
return nil
}
// scaleReplicaSetWithLazyAnnotationUpdate does not update the replica set annotations (DesiredReplicasAnnotation and
// MaxReplicasAnnotation) if no replica set scaling is requested.
// IMPORTANT: This method should not be called when an annotation update is necessary (e.g. scale during a rolling update)
// and scaleReplicaSet should be used instead.
func (dc *DeploymentController) scaleReplicaSetWithLazyAnnotationUpdate(ctx context.Context, rs *apps.ReplicaSet, newScale int32, deployment *apps.Deployment) (bool, *apps.ReplicaSet, error) {
// No need to scale
if *(rs.Spec.Replicas) == newScale {
// scaleReplicaSet acts in two modes:
// 1. lazy update - when forceUpdate is false, it will only act when the replicas
// where changed;
// 2. normal - either replicaset annotations (DesiredReplicasAnnotation and
// MaxReplicasAnnotation) require an update or the replicas have changed.
func (dc *DeploymentController) scaleReplicaSet(ctx context.Context, rs *apps.ReplicaSet, newScale int32, deployment *apps.Deployment, forceUpdate bool) (bool, *apps.ReplicaSet, error) {
// Don't scale, unless it's a forced update or the replicas actually differ
if !forceUpdate && *(rs.Spec.Replicas) == newScale {
return false, rs, nil
}
scaled, newRS, err := dc.scaleReplicaSet(ctx, rs, newScale, deployment)
return scaled, newRS, err
}
func (dc *DeploymentController) scaleReplicaSet(ctx context.Context, rs *apps.ReplicaSet, newScale int32, deployment *apps.Deployment) (bool, *apps.ReplicaSet, error) {
sizeNeedsUpdate := *(rs.Spec.Replicas) != newScale
annotationsNeedUpdate := deploymentutil.ReplicasAnnotationsNeedUpdate(rs, *(deployment.Spec.Replicas), *(deployment.Spec.Replicas)+deploymentutil.MaxSurge(*deployment))
scaled := false