From b929424135c662415469bb2eca41516f5d35abb2 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Thu, 11 Feb 2016 11:39:44 +0100 Subject: [PATCH] Scale deployments fall-back to regular deployment update --- hack/test-cmd.sh | 17 ++- pkg/kubectl/scale.go | 107 +++++++++--------- pkg/kubectl/scale_test.go | 227 +++++++++++++++++--------------------- pkg/kubectl/stop_test.go | 26 ----- 4 files changed, 160 insertions(+), 217 deletions(-) diff --git a/hack/test-cmd.sh b/hack/test-cmd.sh index 7f0fbd84e9..472e85a8d1 100755 --- a/hack/test-cmd.sh +++ b/hack/test-cmd.sh @@ -966,15 +966,14 @@ __EOF__ # Clean-up kubectl delete job/pi "${kube_flags[@]}" - # TODO(madhusudancs): Fix this when Scale group issues are resolved (see issue #18528). - # ### Scale a deployment - # kubectl create -f examples/extensions/deployment.yaml "${kube_flags[@]}" - # # Command - # kubectl scale --current-replicas=3 --replicas=1 deployment/nginx-deployment - # # Post-condition: 1 replica for nginx-deployment - # kube::test::get_object_assert 'deployment nginx-deployment' "{{$deployment_replicas}}" '1' - # # Clean-up - # kubectl delete deployment/nginx-deployment "${kube_flags[@]}" + ### Scale a deployment + kubectl create -f docs/user-guide/deployment.yaml "${kube_flags[@]}" + # Command + kubectl scale --current-replicas=3 --replicas=1 deployment/nginx-deployment + # Post-condition: 1 replica for nginx-deployment + kube::test::get_object_assert 'deployment nginx-deployment' "{{$deployment_replicas}}" '1' + # Clean-up + kubectl delete deployment/nginx-deployment "${kube_flags[@]}" ### Expose a deployment as a service kubectl create -f docs/user-guide/deployment.yaml "${kube_flags[@]}" diff --git a/pkg/kubectl/scale.go b/pkg/kubectl/scale.go index 74fe84a6bc..963f3050a5 100644 --- a/pkg/kubectl/scale.go +++ b/pkg/kubectl/scale.go @@ -48,9 +48,8 @@ func ScalerFor(kind unversioned.GroupKind, c client.Interface) (Scaler, error) { return &ReplicaSetScaler{c.Extensions()}, nil case extensions.Kind("Job"): return &JobScaler{c.Extensions()}, nil - // TODO(madhusudancs): Fix this when Scale group issues are resolved (see issue #18528). - // case extensions.Kind("Deployment"): - // return &DeploymentScaler{c.Extensions()}, nil + case extensions.Kind("Deployment"): + return &DeploymentScaler{c.Extensions()}, nil } return nil, fmt.Errorf("no scaler has been implemented for %q", kind) } @@ -328,57 +327,55 @@ func (precondition *ScalePrecondition) ValidateDeployment(deployment *extensions return nil } -// TODO(madhusudancs): Fix this when Scale group issues are resolved (see issue #18528). -// type DeploymentScaler struct { -// c client.ExtensionsInterface -// } +type DeploymentScaler struct { + c client.ExtensionsInterface +} -// // ScaleSimple is responsible for updating a deployment's desired replicas count. -// func (scaler *DeploymentScaler) ScaleSimple(namespace, name string, preconditions *ScalePrecondition, newSize uint) error { -// deployment, err := scaler.c.Deployments(namespace).Get(name) -// if err != nil { -// return ScaleError{ScaleGetFailure, "Unknown", err} -// } -// if preconditions != nil { -// if err := preconditions.ValidateDeployment(deployment); err != nil { -// return err -// } -// } -// scale, err := extensions.ScaleFromDeployment(deployment) -// if err != nil { -// return ScaleError{ScaleUpdateFailure, deployment.ResourceVersion, err} -// } -// scale.Spec.Replicas = int(newSize) -// if _, err := scaler.c.Scales(namespace).Update("Deployment", scale); err != nil { -// if errors.IsInvalid(err) { -// return ScaleError{ScaleUpdateInvalidFailure, deployment.ResourceVersion, err} -// } -// return ScaleError{ScaleUpdateFailure, deployment.ResourceVersion, err} -// } -// return nil -// } +// ScaleSimple is responsible for updating a deployment's desired replicas count. +func (scaler *DeploymentScaler) ScaleSimple(namespace, name string, preconditions *ScalePrecondition, newSize uint) error { + deployment, err := scaler.c.Deployments(namespace).Get(name) + if err != nil { + return ScaleError{ScaleGetFailure, "Unknown", err} + } + if preconditions != nil { + if err := preconditions.ValidateDeployment(deployment); err != nil { + return err + } + } -// // Scale updates a deployment to a new size, with optional precondition check (if preconditions is not nil), -// // optional retries (if retry is not nil), and then optionally waits for the status to reach desired count. -// func (scaler *DeploymentScaler) Scale(namespace, name string, newSize uint, preconditions *ScalePrecondition, retry, waitForReplicas *RetryParams) error { -// if preconditions == nil { -// preconditions = &ScalePrecondition{-1, ""} -// } -// if retry == nil { -// // Make it try only once, immediately -// retry = &RetryParams{Interval: time.Millisecond, Timeout: time.Millisecond} -// } -// cond := ScaleCondition(scaler, preconditions, namespace, name, newSize) -// if err := wait.Poll(retry.Interval, retry.Timeout, cond); err != nil { -// return err -// } -// if waitForReplicas != nil { -// deployment, err := scaler.c.Deployments(namespace).Get(name) -// if err != nil { -// return err -// } -// return wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, -// client.DeploymentHasDesiredReplicas(scaler.c, deployment)) -// } -// return nil -// } + // TODO(madhusudancs): Fix this when Scale group issues are resolved (see issue #18528). + // For now I'm falling back to regular Deployment update operation. + deployment.Spec.Replicas = int(newSize) + if _, err := scaler.c.Deployments(namespace).Update(deployment); err != nil { + if errors.IsInvalid(err) { + return ScaleError{ScaleUpdateInvalidFailure, deployment.ResourceVersion, err} + } + return ScaleError{ScaleUpdateFailure, deployment.ResourceVersion, err} + } + return nil +} + +// Scale updates a deployment to a new size, with optional precondition check (if preconditions is not nil), +// optional retries (if retry is not nil), and then optionally waits for the status to reach desired count. +func (scaler *DeploymentScaler) Scale(namespace, name string, newSize uint, preconditions *ScalePrecondition, retry, waitForReplicas *RetryParams) error { + if preconditions == nil { + preconditions = &ScalePrecondition{-1, ""} + } + if retry == nil { + // Make it try only once, immediately + retry = &RetryParams{Interval: time.Millisecond, Timeout: time.Millisecond} + } + cond := ScaleCondition(scaler, preconditions, namespace, name, newSize) + if err := wait.Poll(retry.Interval, retry.Timeout, cond); err != nil { + return err + } + if waitForReplicas != nil { + deployment, err := scaler.c.Deployments(namespace).Get(name) + if err != nil { + return err + } + return wait.Poll(waitForReplicas.Interval, waitForReplicas.Timeout, + client.DeploymentHasDesiredReplicas(scaler.c, deployment)) + } + return nil +} diff --git a/pkg/kubectl/scale_test.go b/pkg/kubectl/scale_test.go index ceff36021a..87b674355b 100644 --- a/pkg/kubectl/scale_test.go +++ b/pkg/kubectl/scale_test.go @@ -488,145 +488,118 @@ func TestValidateJob(t *testing.T) { } } -// TODO(madhusudancs): Fix this when Scale group issues are resolved (see issue #18528). +type ErrorDeployments struct { + testclient.FakeDeployments + invalid bool +} -// type ErrorScales struct { -// testclient.FakeScales -// invalid bool -// } +func (c *ErrorDeployments) Update(deployment *extensions.Deployment) (*extensions.Deployment, error) { + if c.invalid { + return nil, kerrors.NewInvalid(extensions.Kind(deployment.Kind), deployment.Name, nil) + } + return nil, errors.New("deployment update failure") +} -// func (c *ErrorScales) Update(kind string, scale *extensions.Scale) (*extensions.Scale, error) { -// if c.invalid { -// return nil, kerrors.NewInvalid(extensions.Kind(scale.Kind), scale.Name, nil) -// } -// return nil, errors.New("scale update failure") -// } +func (c *ErrorDeployments) Get(name string) (*extensions.Deployment, error) { + return &extensions.Deployment{ + Spec: extensions.DeploymentSpec{ + Replicas: 0, + }, + }, nil +} -// func (c *ErrorScales) Get(kind, name string) (*extensions.Scale, error) { -// return &extensions.Scale{ -// Spec: extensions.ScaleSpec{ -// Replicas: 0, -// }, -// }, nil -// } +type ErrorDeploymentClient struct { + testclient.FakeExperimental + invalid bool +} -// type ErrorDeployments struct { -// testclient.FakeDeployments -// invalid bool -// } +func (c *ErrorDeploymentClient) Deployments(namespace string) client.DeploymentInterface { + return &ErrorDeployments{testclient.FakeDeployments{Fake: &c.FakeExperimental, Namespace: namespace}, c.invalid} +} -// func (c *ErrorDeployments) Update(deployment *extensions.Deployment) (*extensions.Deployment, error) { -// if c.invalid { -// return nil, kerrors.NewInvalid(extensions.Kind(deployment.Kind), deployment.Name, nil) -// } -// return nil, errors.New("deployment update failure") -// } +func TestDeploymentScaleRetry(t *testing.T) { + fake := &ErrorDeploymentClient{FakeExperimental: testclient.FakeExperimental{Fake: &testclient.Fake{}}, invalid: false} + scaler := &DeploymentScaler{fake} + preconditions := &ScalePrecondition{-1, ""} + count := uint(3) + name := "foo" + namespace := "default" -// func (c *ErrorDeployments) Get(name string) (*extensions.Deployment, error) { -// return &extensions.Deployment{ -// Spec: extensions.DeploymentSpec{ -// Replicas: 0, -// }, -// }, nil -// } + scaleFunc := ScaleCondition(scaler, preconditions, namespace, name, count) + pass, err := scaleFunc() + if pass != false { + t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass) + } + if err != nil { + t.Errorf("Did not expect an error on update failure, got %v", err) + } + preconditions = &ScalePrecondition{3, ""} + scaleFunc = ScaleCondition(scaler, preconditions, namespace, name, count) + pass, err = scaleFunc() + if err == nil { + t.Errorf("Expected error on precondition failure") + } +} -// type ErrorDeploymentClient struct { -// testclient.FakeExperimental -// invalid bool -// } +func TestDeploymentScale(t *testing.T) { + fake := &testclient.FakeExperimental{Fake: &testclient.Fake{}} + scaler := DeploymentScaler{fake} + preconditions := ScalePrecondition{-1, ""} + count := uint(3) + name := "foo" + scaler.Scale("default", name, count, &preconditions, nil, nil) -// func (c *ErrorDeploymentClient) Deployments(namespace string) client.DeploymentInterface { -// return &ErrorDeployments{testclient.FakeDeployments{Fake: &c.FakeExperimental, Namespace: namespace}, c.invalid} -// } + actions := fake.Actions() + if len(actions) != 2 { + t.Errorf("unexpected actions: %v, expected 2 actions (get, update)", actions) + } + if action, ok := actions[0].(testclient.GetAction); !ok || action.GetResource() != "deployments" || action.GetName() != name { + t.Errorf("unexpected action: %v, expected get-replicationController %s", actions[0], name) + } + if action, ok := actions[1].(testclient.UpdateAction); !ok || action.GetResource() != "deployments" || action.GetObject().(*extensions.Deployment).Spec.Replicas != int(count) { + t.Errorf("unexpected action %v, expected update-deployment with replicas = %d", actions[1], count) + } +} -// func (c *ErrorDeploymentClient) Scales(namespace string) client.ScaleInterface { -// return &ErrorScales{testclient.FakeScales{Fake: &c.FakeExperimental, Namespace: namespace}, c.invalid} -// } +func TestDeploymentScaleInvalid(t *testing.T) { + fake := &ErrorDeploymentClient{FakeExperimental: testclient.FakeExperimental{Fake: &testclient.Fake{}}, invalid: true} + scaler := DeploymentScaler{fake} + preconditions := ScalePrecondition{-1, ""} + count := uint(3) + name := "foo" + namespace := "default" -// func TestDeploymentScaleRetry(t *testing.T) { -// fake := &ErrorDeploymentClient{FakeExperimental: testclient.FakeExperimental{Fake: &testclient.Fake{}}, invalid: false} -// scaler := &DeploymentScaler{fake} -// preconditions := &ScalePrecondition{-1, ""} -// count := uint(3) -// name := "foo" -// namespace := "default" + scaleFunc := ScaleCondition(&scaler, &preconditions, namespace, name, count) + pass, err := scaleFunc() + if pass { + t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass) + } + e, ok := err.(ScaleError) + if err == nil || !ok || e.FailureType != ScaleUpdateInvalidFailure { + t.Errorf("Expected error on invalid update failure, got %v", err) + } +} -// scaleFunc := ScaleCondition(scaler, preconditions, namespace, name, count) -// pass, err := scaleFunc() -// if pass != false { -// t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass) -// } -// if err != nil { -// t.Errorf("Did not expect an error on update failure, got %v", err) -// } -// preconditions = &ScalePrecondition{3, ""} -// scaleFunc = ScaleCondition(scaler, preconditions, namespace, name, count) -// pass, err = scaleFunc() -// if err == nil { -// t.Errorf("Expected error on precondition failure") -// } -// } +func TestDeploymentScaleFailsPreconditions(t *testing.T) { + fake := testclient.NewSimpleFake(&extensions.Deployment{ + Spec: extensions.DeploymentSpec{ + Replicas: 10, + }, + }) + scaler := DeploymentScaler{&testclient.FakeExperimental{fake}} + preconditions := ScalePrecondition{2, ""} + count := uint(3) + name := "foo" + scaler.Scale("default", name, count, &preconditions, nil, nil) -// func TestDeploymentScale(t *testing.T) { -// fake := &testclient.FakeExperimental{Fake: &testclient.Fake{}} -// scaler := DeploymentScaler{fake} -// preconditions := ScalePrecondition{-1, ""} -// count := uint(3) -// name := "foo" -// scaler.Scale("default", name, count, &preconditions, nil, nil) - -// actions := fake.Actions() -// if len(actions) != 2 { -// t.Errorf("unexpected actions: %v, expected 2 actions (get, update)", actions) -// } -// if action, ok := actions[0].(testclient.GetAction); !ok || action.GetResource() != "deployments" || action.GetName() != name { -// t.Errorf("unexpected action: %v, expected get-replicationController %s", actions[0], name) -// } -// // TODO: The testclient needs to support subresources -// if action, ok := actions[1].(testclient.UpdateAction); !ok || action.GetResource() != "Deployment" || action.GetObject().(*extensions.Scale).Spec.Replicas != int(count) { -// t.Errorf("unexpected action %v, expected update-deployment-scale with replicas = %d", actions[1], count) -// } -// } - -// func TestDeploymentScaleInvalid(t *testing.T) { -// fake := &ErrorDeploymentClient{FakeExperimental: testclient.FakeExperimental{Fake: &testclient.Fake{}}, invalid: true} -// scaler := DeploymentScaler{fake} -// preconditions := ScalePrecondition{-1, ""} -// count := uint(3) -// name := "foo" -// namespace := "default" - -// scaleFunc := ScaleCondition(&scaler, &preconditions, namespace, name, count) -// pass, err := scaleFunc() -// if pass { -// t.Errorf("Expected an update failure to return pass = false, got pass = %v", pass) -// } -// e, ok := err.(ScaleError) -// if err == nil || !ok || e.FailureType != ScaleUpdateInvalidFailure { -// t.Errorf("Expected error on invalid update failure, got %v", err) -// } -// } - -// func TestDeploymentScaleFailsPreconditions(t *testing.T) { -// fake := testclient.NewSimpleFake(&extensions.Deployment{ -// Spec: extensions.DeploymentSpec{ -// Replicas: 10, -// }, -// }) -// scaler := DeploymentScaler{&testclient.FakeExperimental{fake}} -// preconditions := ScalePrecondition{2, ""} -// count := uint(3) -// name := "foo" -// scaler.Scale("default", name, count, &preconditions, nil, nil) - -// actions := fake.Actions() -// if len(actions) != 1 { -// t.Errorf("unexpected actions: %v, expected 1 actions (get)", actions) -// } -// if action, ok := actions[0].(testclient.GetAction); !ok || action.GetResource() != "deployments" || action.GetName() != name { -// t.Errorf("unexpected action: %v, expected get-deployment %s", actions[0], name) -// } -// } + actions := fake.Actions() + if len(actions) != 1 { + t.Errorf("unexpected actions: %v, expected 1 actions (get)", actions) + } + if action, ok := actions[0].(testclient.GetAction); !ok || action.GetResource() != "deployments" || action.GetName() != name { + t.Errorf("unexpected action: %v, expected get-deployment %s", actions[0], name) + } +} func TestValidateDeployment(t *testing.T) { zero, ten, twenty := 0, 10, 20 diff --git a/pkg/kubectl/stop_test.go b/pkg/kubectl/stop_test.go index f1cf933628..32a5f52d01 100644 --- a/pkg/kubectl/stop_test.go +++ b/pkg/kubectl/stop_test.go @@ -535,19 +535,6 @@ func TestDeploymentStop(t *testing.T) { Replicas: 0, }, }, - &extensions.Scale{ // UPDATE - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: extensions.ScaleSpec{ - Replicas: 0, - }, - Status: extensions.ScaleStatus{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}, - }, - }, }, StopError: nil, ExpectedActions: []string{"get:deployments", "update:deployments", @@ -558,19 +545,6 @@ func TestDeploymentStop(t *testing.T) { Name: "Deployment with single replicaset", Objs: []runtime.Object{ &deployment, // GET - &extensions.Scale{ // UPDATE - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: extensions.ScaleSpec{ - Replicas: 0, - }, - Status: extensions.ScaleStatus{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}, - }, - }, &extensions.ReplicaSetList{ // LIST Items: []extensions.ReplicaSet{ {