From b1882b67062799e7275b6af25a4c33bd23959ecc Mon Sep 17 00:00:00 2001 From: Michail Kargakis Date: Sat, 21 Jan 2017 22:54:21 +0100 Subject: [PATCH] controller: old pods should block deployment completeness --- .../deployment/util/deployment_util.go | 3 ++- .../deployment/util/deployment_util_test.go | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index b520eda5835f..4b69d7a819fe 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -800,9 +800,10 @@ func IsRollingUpdate(deployment *extensions.Deployment) bool { } // DeploymentComplete considers a deployment to be complete once its desired replicas equals its -// updatedReplicas and it doesn't violate minimum availability. +// updatedReplicas, no old pods are running, and it doesn't violate minimum availability. 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) } diff --git a/pkg/controller/deployment/util/deployment_util_test.go b/pkg/controller/deployment/util/deployment_util_test.go index 722b12914de2..d8a6f51fcaea 100644 --- a/pkg/controller/deployment/util/deployment_util_test.go +++ b/pkg/controller/deployment/util/deployment_util_test.go @@ -894,13 +894,14 @@ func TestRemoveCondition(t *testing.T) { } func TestDeploymentComplete(t *testing.T) { - deployment := func(desired, current, updated, available, maxUnavailable int32) *extensions.Deployment { + deployment := func(desired, current, updated, available, maxUnavailable, maxSurge int32) *extensions.Deployment { return &extensions.Deployment{ Spec: extensions.DeploymentSpec{ Replicas: desired, Strategy: extensions.DeploymentStrategy{ RollingUpdate: &extensions.RollingUpdateDeployment{ MaxUnavailable: intstr.FromInt(int(maxUnavailable)), + MaxSurge: intstr.FromInt(int(maxSurge)), }, Type: extensions.RollingUpdateDeploymentStrategyType, }, @@ -923,25 +924,33 @@ func TestDeploymentComplete(t *testing.T) { { name: "complete", - d: deployment(5, 5, 5, 4, 1), + d: deployment(5, 5, 5, 4, 1, 0), expected: true, }, { name: "not complete", - d: deployment(5, 5, 5, 3, 1), + d: deployment(5, 5, 5, 3, 1, 0), expected: false, }, { name: "complete #2", - d: deployment(5, 5, 5, 5, 0), + d: deployment(5, 5, 5, 5, 0, 0), expected: true, }, { name: "not complete #2", - d: deployment(5, 5, 4, 5, 0), + d: deployment(5, 5, 4, 5, 0, 0), + expected: false, + }, + { + name: "not complete #3", + + // 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, }, }