diff --git a/pkg/client/unversioned/conditions.go b/pkg/client/unversioned/conditions.go index 4556a643745..e2a27b6b1b1 100644 --- a/pkg/client/unversioned/conditions.go +++ b/pkg/client/unversioned/conditions.go @@ -112,7 +112,7 @@ func JobHasDesiredParallelism(c ExtensionsInterface, job *extensions.Job) wait.C } // desired parallelism can be either the exact number, in which case return immediately - if job.Status.Active == *job.Spec.Parallelism { + if job.Spec.Parallelism != nil && job.Status.Active == *job.Spec.Parallelism { return true, nil } // otherwise count successful diff --git a/pkg/kubectl/cmd/scale.go b/pkg/kubectl/cmd/scale.go index 18e9fedbffd..8cc4c21c8b6 100644 --- a/pkg/kubectl/cmd/scale.go +++ b/pkg/kubectl/cmd/scale.go @@ -146,6 +146,10 @@ func RunScale(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []stri errs := []error{} for _, info := range infos { if err := scaler.Scale(info.Namespace, info.Name, uint(count), precondition, retry, waitForReplicas); err != nil { + if scaleErr, ok := err.(kubectl.ScaleError); ok && scaleErr.FailureType == kubectl.AlreadyScaled { + cmdutil.PrintSuccess(mapper, shortOutput, out, info.Mapping.Resource, info.Name, "already scaled") + continue + } errs = append(errs, err) continue } diff --git a/pkg/kubectl/scale.go b/pkg/kubectl/scale.go index 80f07df9201..4603e7ac55d 100644 --- a/pkg/kubectl/scale.go +++ b/pkg/kubectl/scale.go @@ -17,6 +17,7 @@ limitations under the License. package kubectl import ( + goerrors "errors" "fmt" "strconv" "time" @@ -79,8 +80,13 @@ const ( ScaleGetFailure ScaleErrorType = iota ScaleUpdateFailure ScaleUpdateInvalidFailure + // AlreadyScaled is not really an error but we need a way to surface to the client that + // the scaling didn't happen because we already have the desired state the user asked for. + AlreadyScaled ) +var alreadyScaledErr = goerrors.New("desired replicas already equals the requested replicas") + // A ScaleError is returned when a scale request passes // preconditions but fails to actually scale the controller. type ScaleError struct { @@ -112,12 +118,14 @@ func ScaleCondition(r Scaler, precondition *ScalePrecondition, namespace, name s case nil: return true, nil case ScaleError: - // if it's invalid we shouldn't keep waiting - if e.FailureType == ScaleUpdateInvalidFailure { + switch e.FailureType { + case ScaleUpdateInvalidFailure: + // if it's invalid we shouldn't keep waiting return false, err - } - if e.FailureType == ScaleUpdateFailure { + case ScaleUpdateFailure: return false, nil + case AlreadyScaled: + return false, err } } return false, err @@ -149,8 +157,10 @@ func (scaler *ReplicationControllerScaler) ScaleSimple(namespace, name string, p return err } } + if controller.Spec.Replicas == int(newSize) { + return ScaleError{AlreadyScaled, controller.ResourceVersion, alreadyScaledErr} + } controller.Spec.Replicas = int(newSize) - // TODO: do retry on 409 errors here? if _, err := scaler.c.ReplicationControllers(namespace).Update(controller); err != nil { if errors.IsInvalid(err) { return ScaleError{ScaleUpdateInvalidFailure, controller.ResourceVersion, err} @@ -216,6 +226,9 @@ func (scaler *JobScaler) ScaleSimple(namespace, name string, preconditions *Scal } } parallelism := int(newSize) + if job.Spec.Parallelism != nil && *job.Spec.Parallelism == parallelism { + return ScaleError{AlreadyScaled, job.ResourceVersion, alreadyScaledErr} + } job.Spec.Parallelism = ¶llelism if _, err := scaler.c.Jobs(namespace).Update(job); err != nil { if errors.IsInvalid(err) { @@ -279,6 +292,9 @@ func (scaler *DeploymentScaler) ScaleSimple(namespace, name string, precondition } } scale := extensions.ScaleFromDeployment(deployment) + if scale.Spec.Replicas == int(newSize) { + return ScaleError{AlreadyScaled, deployment.ResourceVersion, alreadyScaledErr} + } scale.Spec.Replicas = int(newSize) if _, err := scaler.c.Scales(namespace).Update("Deployment", scale); err != nil { if errors.IsInvalid(err) { diff --git a/pkg/kubectl/scale_test.go b/pkg/kubectl/scale_test.go index 02d222ecfe5..2814c699fd2 100644 --- a/pkg/kubectl/scale_test.go +++ b/pkg/kubectl/scale_test.go @@ -132,6 +132,31 @@ func TestReplicationControllerScaleFailsPreconditions(t *testing.T) { } } +func TestReplicationControllerAlreadyScaled(t *testing.T) { + fake := testclient.NewSimpleFake(&api.ReplicationController{ + Spec: api.ReplicationControllerSpec{ + Replicas: 3, + }, + }) + scaler := ReplicationControllerScaler{fake} + count := uint(3) + name := "foo" + err := scaler.Scale("default", name, count, nil, nil, nil) + if err == nil { + t.Fatal("expected AlreadyScaled error, got nil") + } + if scaleErr, ok := err.(ScaleError); !ok || scaleErr.FailureType != AlreadyScaled { + t.Fatalf("expected AlreadyScaled error, got %s", scaleErr.FailureType) + } + actions := fake.Actions() + if len(actions) != 1 { + t.Fatalf("unexpected actions: %v, expected 1 action (get)", actions) + } + if action, ok := actions[0].(testclient.GetAction); !ok || action.GetResource() != "replicationcontrollers" || action.GetName() != name { + t.Errorf("unexpected action: %v, expected get-replicationController %s", actions[0], name) + } +} + func TestValidateReplicationController(t *testing.T) { tests := []struct { preconditions ScalePrecondition @@ -362,6 +387,32 @@ func TestJobScaleFailsPreconditions(t *testing.T) { } } +func TestJobAlreadyScaled(t *testing.T) { + three := 3 + fake := testclient.NewSimpleFake(&extensions.Job{ + Spec: extensions.JobSpec{ + Parallelism: &three, + }, + }) + scaler := JobScaler{&testclient.FakeExperimental{fake}} + count := uint(3) + name := "foo" + err := scaler.Scale("default", name, count, nil, nil, nil) + if err == nil { + t.Fatal("expected AlreadyScaled error, got nil") + } + if scaleErr, ok := err.(ScaleError); !ok || scaleErr.FailureType != AlreadyScaled { + t.Fatalf("expected AlreadyScaled error, got %s", scaleErr.FailureType) + } + actions := fake.Actions() + if len(actions) != 1 { + t.Fatalf("unexpected actions: %v, expected 1 action (get)", actions) + } + if action, ok := actions[0].(testclient.GetAction); !ok || action.GetResource() != "jobs" || action.GetName() != name { + t.Errorf("unexpected action: %v, expected get-job %s", actions[0], name) + } +} + func TestValidateJob(t *testing.T) { zero, ten, twenty := 0, 10, 20 tests := []struct { @@ -626,6 +677,31 @@ func TestDeploymentScaleFailsPreconditions(t *testing.T) { } } +func TestDeploymentAlreadyScaled(t *testing.T) { + fake := testclient.NewSimpleFake(&extensions.Deployment{ + Spec: extensions.DeploymentSpec{ + Replicas: 3, + }, + }) + scaler := DeploymentScaler{&testclient.FakeExperimental{fake}} + count := uint(3) + name := "foo" + err := scaler.Scale("default", name, count, nil, nil, nil) + if err == nil { + t.Fatal("expected AlreadyScaled error, got nil") + } + if scaleErr, ok := err.(ScaleError); !ok || scaleErr.FailureType != AlreadyScaled { + t.Fatalf("expected AlreadyScaled error, got %s", scaleErr.FailureType) + } + actions := fake.Actions() + if len(actions) != 1 { + t.Fatalf("unexpected actions: %v, expected 1 action (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 tests := []struct { diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index 11409dfc11a..36c21903617 100644 --- a/pkg/kubectl/stop.go +++ b/pkg/kubectl/stop.go @@ -23,7 +23,6 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/errors" - "k8s.io/kubernetes/pkg/api/meta" "k8s.io/kubernetes/pkg/api/unversioned" "k8s.io/kubernetes/pkg/apis/extensions" client "k8s.io/kubernetes/pkg/client/unversioned" @@ -83,30 +82,6 @@ func ReaperForReplicationController(c client.Interface, timeout time.Duration) ( return &ReplicationControllerReaper{c, Interval, timeout}, nil } -type ReplicationControllerReaper struct { - client.Interface - pollInterval, timeout time.Duration -} -type DaemonSetReaper struct { - client.Interface - pollInterval, timeout time.Duration -} -type JobReaper struct { - client.Interface - pollInterval, timeout time.Duration -} -type PodReaper struct { - client.Interface -} -type ServiceReaper struct { - client.Interface -} - -type objInterface interface { - Delete(name string) error - Get(name string) (meta.Object, error) -} - // getOverlappingControllers finds rcs that this controller overlaps, as well as rcs overlapping this controller. func getOverlappingControllers(c client.ReplicationControllerInterface, rc *api.ReplicationController) ([]api.ReplicationController, error) { rcs, err := c.List(api.ListOptions{}) @@ -124,6 +99,11 @@ func getOverlappingControllers(c client.ReplicationControllerInterface, rc *api. return matchingRCs, nil } +type ReplicationControllerReaper struct { + client.Interface + pollInterval, timeout time.Duration +} + func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { rc := reaper.ReplicationControllers(namespace) scaler, err := ScalerFor(api.Kind("ReplicationController"), *reaper) @@ -181,7 +161,9 @@ func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout retry := NewRetryParams(reaper.pollInterval, reaper.timeout) waitForReplicas := NewRetryParams(reaper.pollInterval, timeout) if err = scaler.Scale(namespace, name, 0, nil, retry, waitForReplicas); err != nil { - return err + if scaleErr, ok := err.(ScaleError); !ok || scaleErr.FailureType != AlreadyScaled { + return err + } } } if err := rc.Delete(name); err != nil { @@ -190,6 +172,11 @@ func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout return nil } +type DaemonSetReaper struct { + client.Interface + pollInterval, timeout time.Duration +} + func (reaper *DaemonSetReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { ds, err := reaper.Extensions().DaemonSets(namespace).Get(name) if err != nil { @@ -227,6 +214,11 @@ func (reaper *DaemonSetReaper) Stop(namespace, name string, timeout time.Duratio return nil } +type JobReaper struct { + client.Interface + pollInterval, timeout time.Duration +} + func (reaper *JobReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { jobs := reaper.Extensions().Jobs(namespace) pods := reaper.Pods(namespace) @@ -248,7 +240,9 @@ func (reaper *JobReaper) Stop(namespace, name string, timeout time.Duration, gra retry := NewRetryParams(reaper.pollInterval, reaper.timeout) waitForJobs := NewRetryParams(reaper.pollInterval, timeout) if err = scaler.Scale(namespace, name, 0, nil, retry, waitForJobs); err != nil { - return err + if scaleErr, ok := err.(ScaleError); !ok || scaleErr.FailureType != AlreadyScaled { + return err + } } // at this point only dead pods are left, that should be removed selector, _ := extensions.LabelSelectorAsSelector(job.Spec.Selector) @@ -276,6 +270,10 @@ func (reaper *JobReaper) Stop(namespace, name string, timeout time.Duration, gra return nil } +type PodReaper struct { + client.Interface +} + func (reaper *PodReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { pods := reaper.Pods(namespace) _, err := pods.Get(name) @@ -289,6 +287,10 @@ func (reaper *PodReaper) Stop(namespace, name string, timeout time.Duration, gra return nil } +type ServiceReaper struct { + client.Interface +} + func (reaper *ServiceReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { services := reaper.Services(namespace) _, err := services.Get(name) diff --git a/pkg/kubectl/stop_test.go b/pkg/kubectl/stop_test.go index 2e2b5d343ea..a2dac554a46 100644 --- a/pkg/kubectl/stop_test.go +++ b/pkg/kubectl/stop_test.go @@ -19,7 +19,6 @@ package kubectl import ( "fmt" "reflect" - "strings" "testing" "time" @@ -31,355 +30,508 @@ import ( "k8s.io/kubernetes/pkg/runtime" ) +const ( + name = "foo" +) + +func overlappingButSafe() *api.ReplicationControllerList { + return &api.ReplicationControllerList{ + Items: []api.ReplicationController{ + { + ObjectMeta: api.ObjectMeta{ + Name: name, + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 1, + Selector: map[string]string{"k1": "v1", "k2": "v2"}}, + }, + { + ObjectMeta: api.ObjectMeta{ + Name: "baz", + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{"k1": "v1", "k2": "v2", "k3": "v3"}}, + }, + { + ObjectMeta: api.ObjectMeta{ + Name: "zaz", + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 3, + Selector: map[string]string{"k1": "v1"}}, + }, + }, + } +} + +func exactMatches() *api.ReplicationControllerList { + return &api.ReplicationControllerList{ + Items: []api.ReplicationController{ + { + ObjectMeta: api.ObjectMeta{ + Name: "zaz", + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 3, + Selector: map[string]string{"k1": "v1"}}, + }, + { + ObjectMeta: api.ObjectMeta{ + Name: name, + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 3, + Selector: map[string]string{"k1": "v1"}}, + }, + }, + } +} + func TestReplicationControllerStop(t *testing.T) { - name := "foo" - ns := "default" + // test data + toBeReaped := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: name, + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 5, + Selector: map[string]string{"k1": "v1"}}, + } + reaped := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: name, + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 0, + Selector: map[string]string{"k1": "v1"}}, + } + noOverlapping := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: "baz", + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 3, + Selector: map[string]string{"k3": "v3"}}, + } + overlapping := &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: "baz", + Namespace: api.NamespaceDefault, + }, + Spec: api.ReplicationControllerSpec{ + Replicas: 2, + Selector: map[string]string{"k1": "v1", "k2": "v2"}}, + } + + // tests tests := []struct { Name string - Objs []runtime.Object + Fns []testclient.ReactionFunc StopError error - ExpectedActions []string + ExpectedActions []testclient.Action }{ { Name: "OnlyOneRC", - Objs: []runtime.Object{ - &api.ReplicationController{ // GET - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, + Fns: []testclient.ReactionFunc{ + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil }, - &api.ReplicationControllerList{ // LIST - Items: []api.ReplicationController{ - { - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, - }, - }, + // LIST rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, &api.ReplicationControllerList{ + Items: []api.ReplicationController{*toBeReaped}}, nil + }, + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil + }, + // UPDATE rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // DELETE rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil }, }, - StopError: nil, - ExpectedActions: []string{"get", "list", "get", "update", "get", "get", "delete"}, + StopError: nil, + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}), + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewUpdateAction("replicationcontrollers", api.NamespaceDefault, reaped), + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewDeleteAction("replicationcontrollers", api.NamespaceDefault, name), + }, + }, + { + Name: "RCWithNoPods", + Fns: []testclient.ReactionFunc{ + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // LIST rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, &api.ReplicationControllerList{ + Items: []api.ReplicationController{*reaped}}, nil + }, + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // DELETE rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + }, + StopError: nil, + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}), + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewDeleteAction("replicationcontrollers", api.NamespaceDefault, name), + }, }, { Name: "NoOverlapping", - Objs: []runtime.Object{ - &api.ReplicationController{ // GET - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, + Fns: []testclient.ReactionFunc{ + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil }, - &api.ReplicationControllerList{ // LIST - Items: []api.ReplicationController{ - { - ObjectMeta: api.ObjectMeta{ - Name: "baz", - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k3": "v3"}}, - }, - { - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, - }, - }, + // LIST rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, &api.ReplicationControllerList{ + Items: []api.ReplicationController{*toBeReaped, *noOverlapping}}, nil + }, + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil + }, + // UPDATE rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // DELETE rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil }, }, - StopError: nil, - ExpectedActions: []string{"get", "list", "get", "update", "get", "get", "delete"}, + StopError: nil, + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}), + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewUpdateAction("replicationcontrollers", api.NamespaceDefault, reaped), + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewDeleteAction("replicationcontrollers", api.NamespaceDefault, name), + }, }, { Name: "OverlappingError", - Objs: []runtime.Object{ - - &api.ReplicationController{ // GET - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, + Fns: []testclient.ReactionFunc{ + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil }, - &api.ReplicationControllerList{ // LIST - Items: []api.ReplicationController{ - { - ObjectMeta: api.ObjectMeta{ - Name: "baz", - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1", "k2": "v2"}}, - }, - { - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, - }, - }, + // LIST rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, &api.ReplicationControllerList{ + Items: []api.ReplicationController{*toBeReaped, *overlapping}}, nil }, }, - StopError: fmt.Errorf("Detected overlapping controllers for rc foo: baz, please manage deletion individually with --cascade=false."), - ExpectedActions: []string{"get", "list"}, + StopError: fmt.Errorf("Detected overlapping controllers for rc foo: baz, please manage deletion individually with --cascade=false."), + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}), + }, }, - { Name: "OverlappingButSafeDelete", - Objs: []runtime.Object{ - - &api.ReplicationController{ // GET - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1", "k2": "v2"}}, + Fns: []testclient.ReactionFunc{ + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, &overlappingButSafe().Items[0], nil }, - &api.ReplicationControllerList{ // LIST - Items: []api.ReplicationController{ - { - ObjectMeta: api.ObjectMeta{ - Name: "baz", - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1", "k2": "v2", "k3": "v3"}}, - }, - { - ObjectMeta: api.ObjectMeta{ - Name: "zaz", - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, - }, - { - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1", "k2": "v2"}}, - }, - }, + // LIST rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, overlappingButSafe(), nil }, }, - - StopError: fmt.Errorf("Detected overlapping controllers for rc foo: baz,zaz, please manage deletion individually with --cascade=false."), - ExpectedActions: []string{"get", "list"}, + StopError: fmt.Errorf("Detected overlapping controllers for rc foo: baz,zaz, please manage deletion individually with --cascade=false."), + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}), + }, }, - { Name: "TwoExactMatchRCs", - Objs: []runtime.Object{ - - &api.ReplicationController{ // GET - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, + Fns: []testclient.ReactionFunc{ + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil }, - &api.ReplicationControllerList{ // LIST - Items: []api.ReplicationController{ - { - ObjectMeta: api.ObjectMeta{ - Name: "zaz", - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, - }, - { - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: api.ReplicationControllerSpec{ - Replicas: 0, - Selector: map[string]string{"k1": "v1"}}, - }, - }, + // LIST rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, exactMatches(), nil + }, + // GET rc + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil }, }, - - StopError: nil, - ExpectedActions: []string{"get", "list", "delete"}, + StopError: nil, + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name), + testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}), + testclient.NewDeleteAction("replicationcontrollers", api.NamespaceDefault, name), + }, }, } for _, test := range tests { - fake := testclient.NewSimpleFake(test.Objs...) + toBeReaped.Spec.Replicas = 5 + fake := &testclient.Fake{} + for i, reaction := range test.Fns { + fake.AddReactor(test.ExpectedActions[i].GetVerb(), test.ExpectedActions[i].GetResource(), reaction) + } reaper := ReplicationControllerReaper{fake, time.Millisecond, time.Millisecond} - err := reaper.Stop(ns, name, 0, nil) + err := reaper.Stop(api.NamespaceDefault, name, 0, nil) if !reflect.DeepEqual(err, test.StopError) { - t.Errorf("%s unexpected error: %v", test.Name, err) + t.Errorf("%s: unexpected error: %v", test.Name, err) continue } actions := fake.Actions() - if len(actions) != len(test.ExpectedActions) { - t.Errorf("%s unexpected actions: %v, expected %d actions got %d", test.Name, actions, len(test.ExpectedActions), len(actions)) - continue + if len(test.ExpectedActions) != len(actions) { + t.Errorf("%s: unexpected actions:\n%v\nexpected\n%v\n", test.Name, actions, test.ExpectedActions) } - for i, verb := range test.ExpectedActions { - if actions[i].GetResource() != "replicationcontrollers" { - t.Errorf("%s unexpected action: %+v, expected %s-replicationController", test.Name, actions[i], verb) - } - if actions[i].GetVerb() != verb { - t.Errorf("%s unexpected action: %+v, expected %s-replicationController", test.Name, actions[i], verb) + for i, action := range actions { + testAction := test.ExpectedActions[i] + if !testAction.Matches(action.GetVerb(), action.GetResource()) { + t.Errorf("%s: unexpected action: %#v; expected %v", test.Name, action, testAction) } } } } func TestJobStop(t *testing.T) { - name := "foo" - ns := "default" zero := 0 + one := 1 + toBeReaped := &extensions.Job{ + ObjectMeta: api.ObjectMeta{ + Name: name, + Namespace: api.NamespaceDefault, + }, + Spec: extensions.JobSpec{ + Parallelism: &one, + Selector: &extensions.LabelSelector{ + MatchLabels: map[string]string{"k1": "v1"}, + }, + }, + } + reaped := &extensions.Job{ + ObjectMeta: api.ObjectMeta{ + Name: name, + Namespace: api.NamespaceDefault, + }, + Spec: extensions.JobSpec{ + Parallelism: &zero, + Selector: &extensions.LabelSelector{ + MatchLabels: map[string]string{"k1": "v1"}, + }, + }, + } + tests := []struct { Name string - Objs []runtime.Object + Fns []testclient.ReactionFunc StopError error - ExpectedActions []string + ExpectedActions []testclient.Action }{ { Name: "OnlyOneJob", - Objs: []runtime.Object{ - &extensions.Job{ // GET - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: extensions.JobSpec{ - Parallelism: &zero, - Selector: &extensions.LabelSelector{ - MatchLabels: map[string]string{"k1": "v1"}, - }, - }, + Fns: []testclient.ReactionFunc{ + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil }, - &extensions.JobList{ // LIST - Items: []extensions.Job{ - { - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: extensions.JobSpec{ - Parallelism: &zero, - Selector: &extensions.LabelSelector{ - MatchLabels: map[string]string{"k1": "v1"}, - }, - }, - }, - }, + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil + }, + // UPDATE job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + *toBeReaped.Spec.Parallelism = 0 + return true, toBeReaped, nil + }, + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil + }, + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil + }, + // LIST pods + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, &api.PodList{}, nil + }, + // DELETE job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil }, }, StopError: nil, - ExpectedActions: []string{"get:jobs", "get:jobs", "update:jobs", - "get:jobs", "get:jobs", "list:pods", "delete:jobs"}, + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewUpdateAction("jobs", api.NamespaceDefault, toBeReaped), + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewListAction("pods", api.NamespaceDefault, api.ListOptions{}), + testclient.NewDeleteAction("jobs", api.NamespaceDefault, name), + }, }, { Name: "JobWithDeadPods", - Objs: []runtime.Object{ - &extensions.Job{ // GET - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: extensions.JobSpec{ - Parallelism: &zero, - Selector: &extensions.LabelSelector{ - MatchLabels: map[string]string{"k1": "v1"}, - }, - }, + Fns: []testclient.ReactionFunc{ + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil }, - &extensions.JobList{ // LIST - Items: []extensions.Job{ - { - ObjectMeta: api.ObjectMeta{ - Name: name, - Namespace: ns, - }, - Spec: extensions.JobSpec{ - Parallelism: &zero, - Selector: &extensions.LabelSelector{ - MatchLabels: map[string]string{"k1": "v1"}, + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil + }, + // UPDATE job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + *toBeReaped.Spec.Parallelism = 0 + return true, toBeReaped, nil + }, + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil + }, + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil + }, + // LIST pods + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, &api.PodList{ + Items: []api.Pod{ + { + ObjectMeta: api.ObjectMeta{ + Name: "pod1", + Namespace: api.NamespaceDefault, + Labels: map[string]string{"k1": "v1"}, }, }, }, - }, + }, nil }, - &api.PodList{ // LIST - Items: []api.Pod{ - { - ObjectMeta: api.ObjectMeta{ - Name: "pod1", - Namespace: ns, - Labels: map[string]string{"k1": "v1"}, - }, - }, - }, + // DELETE pod + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, nil, nil + }, + // DELETE job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, toBeReaped, nil }, }, StopError: nil, - ExpectedActions: []string{"get:jobs", "get:jobs", "update:jobs", - "get:jobs", "get:jobs", "list:pods", "delete:pods", "delete:jobs"}, + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewUpdateAction("jobs", api.NamespaceDefault, toBeReaped), + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewListAction("pods", api.NamespaceDefault, api.ListOptions{}), + testclient.NewDeleteAction("pods", api.NamespaceDefault, name), + testclient.NewDeleteAction("jobs", api.NamespaceDefault, name), + }, + }, + { + Name: "JobWithNoParallelism", + Fns: []testclient.ReactionFunc{ + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // GET job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + // LIST pods + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, &api.PodList{}, nil + }, + // DELETE job + func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + return true, reaped, nil + }, + }, + StopError: nil, + ExpectedActions: []testclient.Action{ + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewGetAction("jobs", api.NamespaceDefault, name), + testclient.NewListAction("pods", api.NamespaceDefault, api.ListOptions{}), + testclient.NewDeleteAction("jobs", api.NamespaceDefault, name), + }, }, } for _, test := range tests { - fake := testclient.NewSimpleFake(test.Objs...) + *toBeReaped.Spec.Parallelism = one + fake := &testclient.Fake{} + for i, reaction := range test.Fns { + fake.AddReactor(test.ExpectedActions[i].GetVerb(), test.ExpectedActions[i].GetResource(), reaction) + } reaper := JobReaper{fake, time.Millisecond, time.Millisecond} - err := reaper.Stop(ns, name, 0, nil) + err := reaper.Stop(api.NamespaceDefault, name, 0, nil) if !reflect.DeepEqual(err, test.StopError) { t.Errorf("%s unexpected error: %v", test.Name, err) continue } actions := fake.Actions() - if len(actions) != len(test.ExpectedActions) { - t.Errorf("%s unexpected actions: %v, expected %d actions got %d", test.Name, actions, len(test.ExpectedActions), len(actions)) - continue + if len(test.ExpectedActions) != len(actions) { + t.Errorf("%s: unexpected actions:\n%v\nexpected\n%v\n", test.Name, actions, test.ExpectedActions) } - for i, expAction := range test.ExpectedActions { - action := strings.Split(expAction, ":") - if actions[i].GetVerb() != action[0] { - t.Errorf("%s unexpected verb: %+v, expected %s", test.Name, actions[i], expAction) - } - if actions[i].GetResource() != action[1] { - t.Errorf("%s unexpected resource: %+v, expected %s", test.Name, actions[i], expAction) + for i, action := range actions { + testAction := test.ExpectedActions[i] + if !testAction.Matches(action.GetVerb(), action.GetResource()) { + t.Errorf("%s: unexpected action: %#v; expected %v", test.Name, action, testAction) } } }