From 1923cc60c95943543f036a1c9633f3c4219c917f Mon Sep 17 00:00:00 2001 From: Michail Kargakis Date: Wed, 19 Apr 2017 14:29:39 +0200 Subject: [PATCH] Revert "kubectl: respect deployment strategy parameters for rollout status" This reverts commit d20ac8766eddde3fdb814aadf7338b0209c66436. --- .../deployment/util/deployment_util.go | 13 +---- pkg/kubectl/rollout_status.go | 5 +- pkg/kubectl/rollout_status_test.go | 56 ++++--------------- 3 files changed, 14 insertions(+), 60 deletions(-) diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index cf038b80466..bdb69886ff2 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) || *(deployment.Spec.Replicas) == 0 { + if !IsRollingUpdate(&deployment) { return int32(0) } // Error caught by validation @@ -418,17 +418,6 @@ func MaxUnavailable(deployment extensions.Deployment) int32 { return maxUnavailable } -// MaxUnavailableInternal returns the maximum unavailable pods a rolling deployment can take. -// TODO: remove the duplicate -func MaxUnavailableInternal(deployment internalextensions.Deployment) int32 { - if !(deployment.Spec.Strategy.Type == internalextensions.RollingUpdateDeploymentStrategyType) || deployment.Spec.Replicas == 0 { - return int32(0) - } - // Error caught by validation - _, maxUnavailable, _ := ResolveFenceposts(&deployment.Spec.Strategy.RollingUpdate.MaxSurge, &deployment.Spec.Strategy.RollingUpdate.MaxUnavailable, deployment.Spec.Replicas) - return maxUnavailable -} - // MinAvailable returns the minimum available pods of a given deployment func MinAvailable(deployment *extensions.Deployment) int32 { if !IsRollingUpdate(deployment) { diff --git a/pkg/kubectl/rollout_status.go b/pkg/kubectl/rollout_status.go index 67f4f7f84cb..a0227d55213 100644 --- a/pkg/kubectl/rollout_status.go +++ b/pkg/kubectl/rollout_status.go @@ -78,9 +78,8 @@ func (s *DeploymentStatusViewer) Status(namespace, name string, revision int64) if deployment.Status.Replicas > deployment.Status.UpdatedReplicas { return fmt.Sprintf("Waiting for rollout to finish: %d old replicas are pending termination...\n", deployment.Status.Replicas-deployment.Status.UpdatedReplicas), false, nil } - minRequired := deployment.Spec.Replicas - util.MaxUnavailableInternal(*deployment) - if deployment.Status.AvailableReplicas < minRequired { - return fmt.Sprintf("Waiting for rollout to finish: %d of %d updated replicas are available (minimum required: %d)...\n", deployment.Status.AvailableReplicas, deployment.Status.UpdatedReplicas, minRequired), false, nil + if deployment.Status.AvailableReplicas < deployment.Status.UpdatedReplicas { + return fmt.Sprintf("Waiting for rollout to finish: %d of %d updated replicas are available...\n", deployment.Status.AvailableReplicas, deployment.Status.UpdatedReplicas), false, nil } return fmt.Sprintf("deployment %q successfully rolled out\n", name), true, nil } diff --git a/pkg/kubectl/rollout_status_test.go b/pkg/kubectl/rollout_status_test.go index acf308156cf..53cad4b1ea0 100644 --- a/pkg/kubectl/rollout_status_test.go +++ b/pkg/kubectl/rollout_status_test.go @@ -20,24 +20,17 @@ import ( "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - intstrutil "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" ) -func intOrStringP(i int) *intstrutil.IntOrString { - intstr := intstrutil.FromInt(i) - return &intstr -} - func TestDeploymentStatusViewerStatus(t *testing.T) { tests := []struct { - generation int64 - specReplicas int32 - maxUnavailable *intstrutil.IntOrString - status extensions.DeploymentStatus - msg string - done bool + generation int64 + specReplicas int32 + status extensions.DeploymentStatus + msg string + done bool }{ { generation: 0, @@ -68,9 +61,8 @@ func TestDeploymentStatusViewerStatus(t *testing.T) { done: false, }, { - generation: 1, - specReplicas: 2, - maxUnavailable: intOrStringP(0), + generation: 1, + specReplicas: 2, status: extensions.DeploymentStatus{ ObservedGeneration: 1, Replicas: 2, @@ -79,7 +71,7 @@ func TestDeploymentStatusViewerStatus(t *testing.T) { UnavailableReplicas: 1, }, - msg: "Waiting for rollout to finish: 1 of 2 updated replicas are available (minimum required: 2)...\n", + msg: "Waiting for rollout to finish: 1 of 2 updated replicas are available...\n", done: false, }, { @@ -110,26 +102,9 @@ func TestDeploymentStatusViewerStatus(t *testing.T) { msg: "Waiting for deployment spec update to be observed...\n", done: false, }, - { - generation: 1, - specReplicas: 2, - maxUnavailable: intOrStringP(1), - status: extensions.DeploymentStatus{ - ObservedGeneration: 1, - Replicas: 2, - UpdatedReplicas: 2, - AvailableReplicas: 1, - UnavailableReplicas: 0, - }, - - msg: "deployment \"foo\" successfully rolled out\n", - done: true, - }, } - for i := range tests { - test := tests[i] - t.Logf("testing scenario %d", i) + for _, test := range tests { d := &extensions.Deployment{ ObjectMeta: metav1.ObjectMeta{ Namespace: "bar", @@ -138,27 +113,18 @@ func TestDeploymentStatusViewerStatus(t *testing.T) { Generation: test.generation, }, Spec: extensions.DeploymentSpec{ - Strategy: extensions.DeploymentStrategy{ - Type: extensions.RollingUpdateDeploymentStrategyType, - RollingUpdate: &extensions.RollingUpdateDeployment{ - MaxSurge: *intOrStringP(1), - }, - }, Replicas: test.specReplicas, }, Status: test.status, } - if test.maxUnavailable != nil { - d.Spec.Strategy.RollingUpdate.MaxUnavailable = *test.maxUnavailable - } client := fake.NewSimpleClientset(d).Extensions() dsv := &DeploymentStatusViewer{c: client} msg, done, err := dsv.Status("bar", "foo", 0) if err != nil { - t.Fatalf("unexpected error: %v", err) + t.Fatalf("DeploymentStatusViewer.Status(): %v", err) } if done != test.done || msg != test.msg { - t.Errorf("deployment with generation %d, %d replicas specified, and status:\n%+v\nreturned:\n%q, %t\nwant:\n%q, %t", + t.Errorf("DeploymentStatusViewer.Status() for deployment with generation %d, %d replicas specified, and status %+v returned %q, %t, want %q, %t", test.generation, test.specReplicas, test.status,