diff --git a/pkg/controller/deployment/recreate.go b/pkg/controller/deployment/recreate.go index a142a855d6a..6da2704a466 100644 --- a/pkg/controller/deployment/recreate.go +++ b/pkg/controller/deployment/recreate.go @@ -18,7 +18,6 @@ package deployment import ( "context" - apps "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -84,7 +83,7 @@ func (dc *DeploymentController) scaleDownOldReplicaSetsForRecreate(ctx context.C if *(rs.Spec.Replicas) == 0 { continue } - scaledRS, updatedRS, err := dc.scaleReplicaSet(ctx, rs, 0, deployment) + scaledRS, updatedRS, err := dc.scaleReplicaSetAndRecordEvent(ctx, rs, 0, deployment) if err != nil { return false, err } @@ -128,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.scaleReplicaSet(ctx, newRS, *(deployment.Spec.Replicas), deployment) + scaled, _, err := dc.scaleReplicaSetAndRecordEvent(ctx, newRS, *(deployment.Spec.Replicas), deployment) return scaled, err } diff --git a/pkg/controller/deployment/rolling.go b/pkg/controller/deployment/rolling.go index dbb8249c736..8da48469a23 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.scaleReplicaSet(ctx, newRS, *(deployment.Spec.Replicas), deployment) + scaled, _, err := dc.scaleReplicaSetAndRecordEvent(ctx, newRS, *(deployment.Spec.Replicas), deployment) return scaled, err } newReplicasCount, err := deploymentutil.NewRSNewReplicas(deployment, allRSs, newRS) if err != nil { return false, err } - scaled, _, err := dc.scaleReplicaSet(ctx, newRS, newReplicasCount, deployment) + scaled, _, err := dc.scaleReplicaSetAndRecordEvent(ctx, newRS, newReplicasCount, deployment) 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.scaleReplicaSet(ctx, targetRS, newReplicasCount, deployment) + _, updatedOldRS, err := dc.scaleReplicaSetAndRecordEvent(ctx, targetRS, newReplicasCount, deployment) 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.scaleReplicaSet(ctx, targetRS, newReplicasCount, deployment) + _, _, err := dc.scaleReplicaSetAndRecordEvent(ctx, targetRS, newReplicasCount, deployment) if err != nil { return totalScaledDown, err } diff --git a/pkg/controller/deployment/sync.go b/pkg/controller/deployment/sync.go index e12c424e375..9f631dd02a7 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.scaleReplicaSet(ctx, activeOrLatest, *(deployment.Spec.Replicas), deployment) + _, _, err := dc.scaleReplicaSetAndRecordEvent(ctx, activeOrLatest, *(deployment.Spec.Replicas), deployment) 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.scaleReplicaSet(ctx, old, 0, deployment); err != nil { + if _, _, err := dc.scaleReplicaSetAndRecordEvent(ctx, old, 0, deployment); err != nil { return err } } @@ -390,6 +390,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 { // Return as soon as we fail, the deployment is requeued return err @@ -399,14 +400,23 @@ func (dc *DeploymentController) scale(ctx context.Context, deployment *apps.Depl return nil } -func (dc *DeploymentController) scaleReplicaSet(ctx context.Context, rs *apps.ReplicaSet, newScale int32, deployment *apps.Deployment) (scaled bool, newRS *apps.ReplicaSet, err error) { +func (dc *DeploymentController) scaleReplicaSetAndRecordEvent(ctx context.Context, rs *apps.ReplicaSet, newScale int32, deployment *apps.Deployment) (bool, *apps.ReplicaSet, error) { + // No need to scale if *(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 + var err error if sizeNeedsUpdate || annotationsNeedUpdate { oldScale := *(rs.Spec.Replicas) rsCopy := rs.DeepCopy()