Merge pull request #20030 from kubernetes/revert-18169-smarter-scaling

Revert "kubectl: Make scaling smarter"

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.Spec.Parallelism != nil && job.Status.Active == *job.Spec.Parallelism {
+		if job.Status.Active == *job.Spec.Parallelism {
 			return true, nil
 		}
 		// otherwise count successful

pkg/kubectl/cmd/scale.go

@@ -146,10 +146,6 @@ 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,7 +17,6 @@ limitations under the License.
 package kubectl
 
 import (
-	goerrors "errors"
 	"fmt"
 	"strconv"
 	"time"
@@ -80,13 +79,8 @@ 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 {
@@ -118,14 +112,12 @@ func ScaleCondition(r Scaler, precondition *ScalePrecondition, namespace, name s
 		case nil:
 			return true, nil
 		case ScaleError:
-			switch e.FailureType {
-			case ScaleUpdateInvalidFailure:
-				// if it's invalid we shouldn't keep waiting
+			// if it's invalid we shouldn't keep waiting
+			if e.FailureType == ScaleUpdateInvalidFailure {
 				return false, err
-			case ScaleUpdateFailure:
+			}
+			if e.FailureType == ScaleUpdateFailure {
 				return false, nil
-			case AlreadyScaled:
-				return false, err
 			}
 		}
 		return false, err
@@ -157,10 +149,8 @@ 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}
@@ -226,9 +216,6 @@ 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) {
@@ -292,9 +279,6 @@ 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,31 +132,6 @@ 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
@@ -387,32 +362,6 @@ 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 {
@@ -677,31 +626,6 @@ 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,6 +23,7 @@ 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"
@@ -82,6 +83,30 @@ 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{})
@@ -99,11 +124,6 @@ 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)
@@ -161,9 +181,7 @@ 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 {
-			if scaleErr, ok := err.(ScaleError); !ok || scaleErr.FailureType != AlreadyScaled {
-				return err
-			}
+			return err
 		}
 	}
 	if err := rc.Delete(name); err != nil {
@@ -172,11 +190,6 @@ 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 {
@@ -214,11 +227,6 @@ 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)
@@ -240,9 +248,7 @@ 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 {
-		if scaleErr, ok := err.(ScaleError); !ok || scaleErr.FailureType != AlreadyScaled {
-			return err
-		}
+		return err
 	}
 	// at this point only dead pods are left, that should be removed
 	selector, _ := extensions.LabelSelectorAsSelector(job.Spec.Selector)
@@ -270,10 +276,6 @@ 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)
@@ -287,10 +289,6 @@ 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,6 +19,7 @@ package kubectl
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
@@ -30,508 +31,355 @@ 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) {
-	// 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
+	name := "foo"
+	ns := "default"
 	tests := []struct {
 		Name            string
-		Fns             []testclient.ReactionFunc
+		Objs            []runtime.Object
 		StopError       error
-		ExpectedActions []testclient.Action
+		ExpectedActions []string
 	}{
 		{
 			Name: "OnlyOneRC",
-			Fns: []testclient.ReactionFunc{
-				// GET rc
-				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					return true, toBeReaped, nil
+			Objs: []runtime.Object{
+				&api.ReplicationController{ // GET
+					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
+				&api.ReplicationControllerList{ // LIST
+					Items: []api.ReplicationController{
+						{
+							ObjectMeta: api.ObjectMeta{
+								Name:      name,
+								Namespace: ns,
+							},
+							Spec: api.ReplicationControllerSpec{
+								Replicas: 0,
+								Selector: map[string]string{"k1": "v1"}},
+						},
+					},
 				},
 			},
-			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),
-			},
+			StopError:       nil,
+			ExpectedActions: []string{"get", "list", "get", "update", "get", "get", "delete"},
 		},
 		{
 			Name: "NoOverlapping",
-			Fns: []testclient.ReactionFunc{
-				// GET rc
-				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					return true, toBeReaped, nil
+			Objs: []runtime.Object{
+				&api.ReplicationController{ // GET
+					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
+				&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"}},
+						},
+					},
 				},
 			},
-			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),
-			},
+			StopError:       nil,
+			ExpectedActions: []string{"get", "list", "get", "update", "get", "get", "delete"},
 		},
 		{
 			Name: "OverlappingError",
-			Fns: []testclient.ReactionFunc{
-				// GET rc
-				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					return true, toBeReaped, nil
+			Objs: []runtime.Object{
+
+				&api.ReplicationController{ // GET
+					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
+				&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"}},
+						},
+					},
 				},
 			},
-			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{}),
-			},
+			StopError:       fmt.Errorf("Detected overlapping controllers for rc foo: baz, please manage deletion individually with --cascade=false."),
+			ExpectedActions: []string{"get", "list"},
 		},
+
 		{
 			Name: "OverlappingButSafeDelete",
-			Fns: []testclient.ReactionFunc{
-				// GET rc
-				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					return true, &overlappingButSafe().Items[0], nil
+			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"}},
 				},
-				// LIST rc
-				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					return true, overlappingButSafe(), 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"}},
+						},
+					},
 				},
 			},
-			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{}),
-			},
+
+			StopError:       fmt.Errorf("Detected overlapping controllers for rc foo: baz,zaz, please manage deletion individually with --cascade=false."),
+			ExpectedActions: []string{"get", "list"},
 		},
+
 		{
 			Name: "TwoExactMatchRCs",
-			Fns: []testclient.ReactionFunc{
-				// GET rc
-				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					return true, toBeReaped, nil
-				},
-				// 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: []testclient.Action{
-				testclient.NewGetAction("replicationcontrollers", api.NamespaceDefault, name),
-				testclient.NewListAction("replicationcontrollers", api.NamespaceDefault, api.ListOptions{}),
-				testclient.NewDeleteAction("replicationcontrollers", api.NamespaceDefault, name),
-			},
-		},
-	}
+			Objs: []runtime.Object{
 
-	for _, test := range tests {
-		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(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(test.ExpectedActions) != len(actions) {
-			t.Errorf("%s: unexpected actions:\n%v\nexpected\n%v\n", test.Name, actions, test.ExpectedActions)
-		}
-		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) {
-	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
-		Fns             []testclient.ReactionFunc
-		StopError       error
-		ExpectedActions []testclient.Action
-	}{
-		{
-			Name: "OnlyOneJob",
-			Fns: []testclient.ReactionFunc{
-				// GET job
-				func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
-					return true, toBeReaped, nil
+				&api.ReplicationController{ // GET
+					ObjectMeta: api.ObjectMeta{
+						Name:      name,
+						Namespace: ns,
+					},
+					Spec: api.ReplicationControllerSpec{
+						Replicas: 0,
+						Selector: 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: []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",
-			Fns: []testclient.ReactionFunc{
-				// 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
-				},
-				// 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"},
-								},
+				&api.ReplicationControllerList{ // LIST
+					Items: []api.ReplicationController{
+						{
+							ObjectMeta: api.ObjectMeta{
+								Name:      "zaz",
+								Namespace: ns,
 							},
+							Spec: api.ReplicationControllerSpec{
+								Replicas: 0,
+								Selector: map[string]string{"k1": "v1"}},
 						},
-					}, nil
-				},
-				// 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
+						{
+							ObjectMeta: api.ObjectMeta{
+								Name:      name,
+								Namespace: ns,
+							},
+							Spec: api.ReplicationControllerSpec{
+								Replicas: 0,
+								Selector: map[string]string{"k1": "v1"}},
+						},
+					},
 				},
 			},
-			StopError: nil,
-			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),
-			},
+
+			StopError:       nil,
+			ExpectedActions: []string{"get", "list", "delete"},
 		},
 	}
 
 	for _, test := range tests {
-		*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(api.NamespaceDefault, name, 0, nil)
+		fake := testclient.NewSimpleFake(test.Objs...)
+		reaper := ReplicationControllerReaper{fake, time.Millisecond, time.Millisecond}
+		err := reaper.Stop(ns, name, 0, nil)
 		if !reflect.DeepEqual(err, test.StopError) {
 			t.Errorf("%s unexpected error: %v", test.Name, err)
 			continue
 		}
 
 		actions := fake.Actions()
-		if len(test.ExpectedActions) != len(actions) {
-			t.Errorf("%s: unexpected actions:\n%v\nexpected\n%v\n", test.Name, actions, test.ExpectedActions)
+		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
 		}
-		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)
+		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)
+			}
+		}
+	}
+}
+
+func TestJobStop(t *testing.T) {
+	name := "foo"
+	ns := "default"
+	zero := 0
+	tests := []struct {
+		Name            string
+		Objs            []runtime.Object
+		StopError       error
+		ExpectedActions []string
+	}{
+		{
+			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"},
+						},
+					},
+				},
+				&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"},
+								},
+							},
+						},
+					},
+				},
+			},
+			StopError: nil,
+			ExpectedActions: []string{"get:jobs", "get:jobs", "update:jobs",
+				"get:jobs", "get:jobs", "list:pods", "delete:jobs"},
+		},
+		{
+			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"},
+						},
+					},
+				},
+				&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"},
+								},
+							},
+						},
+					},
+				},
+				&api.PodList{ // LIST
+					Items: []api.Pod{
+						{
+							ObjectMeta: api.ObjectMeta{
+								Name:      "pod1",
+								Namespace: ns,
+								Labels:    map[string]string{"k1": "v1"},
+							},
+						},
+					},
+				},
+			},
+			StopError: nil,
+			ExpectedActions: []string{"get:jobs", "get:jobs", "update:jobs",
+				"get:jobs", "get:jobs", "list:pods", "delete:pods", "delete:jobs"},
+		},
+	}
+
+	for _, test := range tests {
+		fake := testclient.NewSimpleFake(test.Objs...)
+		reaper := JobReaper{fake, time.Millisecond, time.Millisecond}
+		err := reaper.Stop(ns, 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
+		}
+		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)
 			}
 		}
 	}