From 506e71485f81988c94acad65d2c88a3cfdccfe7c 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 | 20 +++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index 82f6e21f458..b05b96c6220 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -816,9 +816,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) && 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 515ec51b839..df2556a4618 100644 --- a/pkg/controller/deployment/util/deployment_util_test.go +++ b/pkg/controller/deployment/util/deployment_util_test.go @@ -892,14 +892,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: func(i int) *intstr.IntOrString { x := intstr.FromInt(i); return &x }(int(maxUnavailable)), - MaxSurge: func() *intstr.IntOrString { x := intstr.FromInt(0); return &x }(), + MaxSurge: func(i int) *intstr.IntOrString { x := intstr.FromInt(i); return &x }(int(maxSurge)), }, Type: extensions.RollingUpdateDeploymentStrategyType, }, @@ -922,25 +922,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, }, }