diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index bdb69886ff2..9b0ba5ebffc 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -410,7 +410,7 @@ func SetReplicasAnnotations(rs *extensions.ReplicaSet, desiredReplicas, maxRepli // MaxUnavailable returns the maximum unavailable pods a rolling deployment can take. func MaxUnavailable(deployment extensions.Deployment) int32 { - if !IsRollingUpdate(&deployment) { + if !IsRollingUpdate(&deployment) || *(deployment.Spec.Replicas) == 0 { return int32(0) } // Error caught by validation @@ -828,12 +828,12 @@ func IsRollingUpdate(deployment *extensions.Deployment) bool { return deployment.Spec.Strategy.Type == extensions.RollingUpdateDeploymentStrategyType } -// DeploymentComplete considers a deployment to be complete once its desired replicas equals its -// updatedReplicas, no old pods are running, and it doesn't violate minimum availability. +// DeploymentComplete considers a deployment to be complete once all of its desired replicas +// are updated and available, and no old pods are running. func DeploymentComplete(deployment *extensions.Deployment, newStatus *extensions.DeploymentStatus) bool { return newStatus.UpdatedReplicas == *(deployment.Spec.Replicas) && newStatus.Replicas == *(deployment.Spec.Replicas) && - newStatus.AvailableReplicas >= *(deployment.Spec.Replicas)-MaxUnavailable(*deployment) && + newStatus.AvailableReplicas == *(deployment.Spec.Replicas) && newStatus.ObservedGeneration >= deployment.Generation } diff --git a/pkg/controller/deployment/util/deployment_util_test.go b/pkg/controller/deployment/util/deployment_util_test.go index 18869302880..9e5615b3d92 100644 --- a/pkg/controller/deployment/util/deployment_util_test.go +++ b/pkg/controller/deployment/util/deployment_util_test.go @@ -960,37 +960,43 @@ func TestDeploymentComplete(t *testing.T) { expected bool }{ { - name: "complete", + name: "not complete: min but not all pods become available", d: deployment(5, 5, 5, 4, 1, 0), - expected: true, + expected: false, }, { - name: "not complete", + name: "not complete: min availability is not honored", d: deployment(5, 5, 5, 3, 1, 0), expected: false, }, { - name: "complete #2", + name: "complete", d: deployment(5, 5, 5, 5, 0, 0), expected: true, }, { - name: "not complete #2", + name: "not complete: all pods are available but not updated", d: deployment(5, 5, 4, 5, 0, 0), expected: false, }, { - name: "not complete #3", + name: "not complete: still running old pods", // old replica set: spec.replicas=1, status.replicas=1, status.availableReplicas=1 // new replica set: spec.replicas=1, status.replicas=1, status.availableReplicas=0 d: deployment(1, 2, 1, 1, 0, 1), expected: false, }, + { + name: "not complete: one replica deployment never comes up", + + d: deployment(1, 1, 1, 0, 1, 1), + expected: false, + }, } for _, test := range tests {