kubectl: Make scaling smarter

Skip updating resources that already meet the desired replica count.
This change has an impact in both kubectl scale and kubectl delete in
that reapable resources that already have the desired replicas (number
provided via --replicas for scale, or zero for delete) won't be updated
again and a "already scaled" message will be printed (in case of scale).

pkg/client/unversioned/conditions.go

@@ -54,7 +54,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

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
 		}

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
+			switch e.FailureType {
-			if e.FailureType == ScaleUpdateInvalidFailure {
+			case ScaleUpdateInvalidFailure:
+				// if it's invalid we shouldn't keep waiting
 				return false, err
-			}
+			case ScaleUpdateFailure:
-			if e.FailureType == 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 = &parallelism
 	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) {

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 {

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)

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"
+	// test data
-	ns := "default"
+	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{
+			Fns: []testclient.ReactionFunc{
-				&api.ReplicationController{ // GET
+				// GET rc
-					ObjectMeta: api.ObjectMeta{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						Name:      name,
+					return true, toBeReaped, nil
-						Namespace: ns,
-					},
-					Spec: api.ReplicationControllerSpec{
-						Replicas: 0,
-						Selector: map[string]string{"k1": "v1"}},
 				},
-				&api.ReplicationControllerList{ // LIST
+				// LIST rc
-					Items: []api.ReplicationController{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, &api.ReplicationControllerList{
-							ObjectMeta: api.ObjectMeta{
+						Items: []api.ReplicationController{*toBeReaped}}, nil
-								Name:      name,
+				},
-								Namespace: ns,
+				// GET rc
-							},
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-							Spec: api.ReplicationControllerSpec{
+					return true, toBeReaped, nil
-								Replicas: 0,
+				},
-								Selector: map[string]string{"k1": "v1"}},
+				// 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,
+			StopError: nil,
-			ExpectedActions: []string{"get", "list", "get", "update", "get", "get", "delete"},
+			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{
+			Fns: []testclient.ReactionFunc{
-				&api.ReplicationController{ // GET
+				// GET rc
-					ObjectMeta: api.ObjectMeta{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						Name:      name,
+					return true, toBeReaped, nil
-						Namespace: ns,
-					},
-					Spec: api.ReplicationControllerSpec{
-						Replicas: 0,
-						Selector: map[string]string{"k1": "v1"}},
 				},
-				&api.ReplicationControllerList{ // LIST
+				// LIST rc
-					Items: []api.ReplicationController{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, &api.ReplicationControllerList{
-							ObjectMeta: api.ObjectMeta{
+						Items: []api.ReplicationController{*toBeReaped, *noOverlapping}}, nil
-								Name:      "baz",
+				},
-								Namespace: ns,
+				// GET rc
-							},
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-							Spec: api.ReplicationControllerSpec{
+					return true, toBeReaped, nil
-								Replicas: 0,
+				},
-								Selector: map[string]string{"k3": "v3"}},
+				// UPDATE rc
-						},
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, reaped, nil
-							ObjectMeta: api.ObjectMeta{
+				},
-								Name:      name,
+				// GET rc
-								Namespace: ns,
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-							},
+					return true, reaped, nil
-							Spec: api.ReplicationControllerSpec{
+				},
-								Replicas: 0,
+				// GET rc
-								Selector: map[string]string{"k1": "v1"}},
+				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,
+			StopError: nil,
-			ExpectedActions: []string{"get", "list", "get", "update", "get", "get", "delete"},
+			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{
+			Fns: []testclient.ReactionFunc{
-
+				// GET rc
-				&api.ReplicationController{ // GET
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					ObjectMeta: api.ObjectMeta{
+					return true, toBeReaped, nil
-						Name:      name,
-						Namespace: ns,
-					},
-					Spec: api.ReplicationControllerSpec{
-						Replicas: 0,
-						Selector: map[string]string{"k1": "v1"}},
 				},
-				&api.ReplicationControllerList{ // LIST
+				// LIST rc
-					Items: []api.ReplicationController{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, &api.ReplicationControllerList{
-							ObjectMeta: api.ObjectMeta{
+						Items: []api.ReplicationController{*toBeReaped, *overlapping}}, nil
-								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"}},
-						},
-					},
 				},
 			},
-			StopError:       fmt.Errorf("Detected overlapping controllers for rc foo: baz, please manage deletion individually with --cascade=false."),
+			StopError: fmt.Errorf("Detected overlapping controllers for rc foo: baz, please manage deletion individually with --cascade=false."),
-			ExpectedActions: []string{"get", "list"},
+			ExpectedActions: []testclient.Action{
+				testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name),
+				testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}),
+			},
 		},
-
 		{
 			Name: "OverlappingButSafeDelete",
-			Objs: []runtime.Object{
+			Fns: []testclient.ReactionFunc{
-
+				// GET rc
-				&api.ReplicationController{ // GET
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					ObjectMeta: api.ObjectMeta{
+					return true, &overlappingButSafe().Items[0], nil
-						Name:      name,
-						Namespace: ns,
-					},
-					Spec: api.ReplicationControllerSpec{
-						Replicas: 0,
-						Selector: map[string]string{"k1": "v1", "k2": "v2"}},
 				},
-				&api.ReplicationControllerList{ // LIST
+				// LIST rc
-					Items: []api.ReplicationController{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, overlappingButSafe(), nil
-							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"}},
-						},
-					},
 				},
 			},
-
+			StopError: fmt.Errorf("Detected overlapping controllers for rc foo: baz,zaz, please manage deletion individually with --cascade=false."),
-			StopError:       fmt.Errorf("Detected overlapping controllers for rc foo: baz,zaz, please manage deletion individually with --cascade=false."),
+			ExpectedActions: []testclient.Action{
-			ExpectedActions: []string{"get", "list"},
+				testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name),
+				testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}),
+			},
 		},
-
 		{
 			Name: "TwoExactMatchRCs",
-			Objs: []runtime.Object{
+			Fns: []testclient.ReactionFunc{
-
+				// GET rc
-				&api.ReplicationController{ // GET
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					ObjectMeta: api.ObjectMeta{
+					return true, toBeReaped, nil
-						Name:      name,
-						Namespace: ns,
-					},
-					Spec: api.ReplicationControllerSpec{
-						Replicas: 0,
-						Selector: map[string]string{"k1": "v1"}},
 				},
-				&api.ReplicationControllerList{ // LIST
+				// LIST rc
-					Items: []api.ReplicationController{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, exactMatches(), nil
-							ObjectMeta: api.ObjectMeta{
+				},
-								Name:      "zaz",
+				// GET rc
-								Namespace: ns,
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-							},
+					return true, reaped, nil
-							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"}},
-						},
-					},
 				},
 			},
-
+			StopError: nil,
-			StopError:       nil,
+			ExpectedActions: []testclient.Action{
-			ExpectedActions: []string{"get", "list", "delete"},
+				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) {
+		if len(test.ExpectedActions) != len(actions) {
-			t.Errorf("%s unexpected actions: %v, expected %d actions got %d", test.Name, actions, len(test.ExpectedActions), len(actions))
+			t.Errorf("%s: unexpected actions:\n%v\nexpected\n%v\n", test.Name, actions, test.ExpectedActions)
-			continue
 		}
-		for i, verb := range test.ExpectedActions {
+		for i, action := range actions {
-			if actions[i].GetResource() != "replicationcontrollers" {
+			testAction := test.ExpectedActions[i]
-				t.Errorf("%s unexpected action: %+v, expected %s-replicationController", test.Name, actions[i], verb)
+			if !testAction.Matches(action.GetVerb(), action.GetResource()) {
-			}
+				t.Errorf("%s: unexpected action: %#v; expected %v", test.Name, action, testAction)
-			if actions[i].GetVerb() != verb {
-				t.Errorf("%s unexpected action: %+v, expected %s-replicationController", test.Name, actions[i], verb)
 			}
 		}
 	}
 }
 
 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{
+			Fns: []testclient.ReactionFunc{
-				&extensions.Job{ // GET
+				// GET job
-					ObjectMeta: api.ObjectMeta{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						Name:      name,
+					return true, toBeReaped, nil
-						Namespace: ns,
-					},
-					Spec: extensions.JobSpec{
-						Parallelism: &zero,
-						Selector: &extensions.LabelSelector{
-							MatchLabels: map[string]string{"k1": "v1"},
-						},
-					},
 				},
-				&extensions.JobList{ // LIST
+				// GET job
-					Items: []extensions.Job{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, toBeReaped, nil
-							ObjectMeta: api.ObjectMeta{
+				},
-								Name:      name,
+				// UPDATE job
-								Namespace: ns,
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-							},
+					*toBeReaped.Spec.Parallelism = 0
-							Spec: extensions.JobSpec{
+					return true, toBeReaped, nil
-								Parallelism: &zero,
+				},
-								Selector: &extensions.LabelSelector{
+				// GET job
-									MatchLabels: map[string]string{"k1": "v1"},
+				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",
+			ExpectedActions: []testclient.Action{
-				"get:jobs", "get:jobs", "list:pods", "delete:jobs"},
+				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{
+			Fns: []testclient.ReactionFunc{
-				&extensions.Job{ // GET
+				// GET job
-					ObjectMeta: api.ObjectMeta{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						Name:      name,
+					return true, toBeReaped, nil
-						Namespace: ns,
-					},
-					Spec: extensions.JobSpec{
-						Parallelism: &zero,
-						Selector: &extensions.LabelSelector{
-							MatchLabels: map[string]string{"k1": "v1"},
-						},
-					},
 				},
-				&extensions.JobList{ // LIST
+				// GET job
-					Items: []extensions.Job{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, toBeReaped, nil
-							ObjectMeta: api.ObjectMeta{
+				},
-								Name:      name,
+				// UPDATE job
-								Namespace: ns,
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-							},
+					*toBeReaped.Spec.Parallelism = 0
-							Spec: extensions.JobSpec{
+					return true, toBeReaped, nil
-								Parallelism: &zero,
+				},
-								Selector: &extensions.LabelSelector{
+				// GET job
-									MatchLabels: map[string]string{"k1": "v1"},
+				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
+				// DELETE pod
-					Items: []api.Pod{
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-						{
+					return true, nil, nil
-							ObjectMeta: api.ObjectMeta{
+				},
-								Name:      "pod1",
+				// DELETE job
-								Namespace: ns,
+				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-								Labels:    map[string]string{"k1": "v1"},
+					return true, toBeReaped, nil
-							},
-						},
-					},
 				},
 			},
 			StopError: nil,
-			ExpectedActions: []string{"get:jobs", "get:jobs", "update:jobs",
+			ExpectedActions: []testclient.Action{
-				"get:jobs", "get:jobs", "list:pods", "delete:pods", "delete:jobs"},
+				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) {
+		if len(test.ExpectedActions) != len(actions) {
-			t.Errorf("%s unexpected actions: %v, expected %d actions got %d", test.Name, actions, len(test.ExpectedActions), len(actions))
+			t.Errorf("%s: unexpected actions:\n%v\nexpected\n%v\n", test.Name, actions, test.ExpectedActions)
-			continue
 		}
-		for i, expAction := range test.ExpectedActions {
+		for i, action := range actions {
-			action := strings.Split(expAction, ":")
+			testAction := test.ExpectedActions[i]
-			if actions[i].GetVerb() != action[0] {
+			if !testAction.Matches(action.GetVerb(), action.GetResource()) {
-				t.Errorf("%s unexpected verb: %+v, expected %s", test.Name, actions[i], expAction)
+				t.Errorf("%s: unexpected action: %#v; expected %v", test.Name, action, testAction)
-			}
-			if actions[i].GetResource() != action[1] {
-				t.Errorf("%s unexpected resource: %+v, expected %s", test.Name, actions[i], expAction)
 			}
 		}
 	}