diff --git a/pkg/controller/deployment/recreate.go b/pkg/controller/deployment/recreate.go index f6225249418..8b8b80a0905 100644 --- a/pkg/controller/deployment/recreate.go +++ b/pkg/controller/deployment/recreate.go @@ -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 } diff --git a/pkg/controller/deployment/rolling.go b/pkg/controller/deployment/rolling.go index b0b833bf0ed..d32584881b1 100644 --- a/pkg/controller/deployment/rolling.go +++ b/pkg/controller/deployment/rolling.go @@ -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 } diff --git a/pkg/controller/deployment/sync.go b/pkg/controller/deployment/sync.go index 2112f2b8467..d2700981b51 100644 --- a/pkg/controller/deployment/sync.go +++ b/pkg/controller/deployment/sync.go @@ -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