Use narrowly scoped interfaces for client access

Use custom narrowly scoped interfaces for client access from the
RollingUpdater and Resizer. This allows for more flexible downstream
integration and unit testing without imposing a burden to implement
the entire client.Interface for just a handful of methods.

pkg/kubectl/cmd/rollingupdate.go

@@ -111,7 +111,7 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg
 		return err
 	}
 
-	updater := kubectl.NewRollingUpdater(newRc.Namespace, client)
+	updater := kubectl.NewRollingUpdater(newRc.Namespace, &kubectl.RealRollingUpdaterClient{client})
 
 	// fetch rc
 	oldRc, err := client.ReplicationControllers(newRc.Namespace).Get(oldName)

pkg/kubectl/cmd/util/factory.go

@@ -197,7 +197,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 			if err != nil {
 				return nil, err
 			}
-			return kubectl.ResizerFor(mapping.Kind, client)
+			return kubectl.ResizerFor(mapping.Kind, &kubectl.RealResizerClient{client})
 		},
 		Reaper: func(mapping *meta.RESTMapping) (kubectl.Reaper, error) {
 			client, err := clients.ClientForVersion(mapping.APIVersion)

pkg/kubectl/resize.go

@@ -89,7 +89,7 @@ type Resizer interface {
 	ResizeSimple(namespace, name string, preconditions *ResizePrecondition, newSize uint) (string, error)
 }
 
-func ResizerFor(kind string, c client.Interface) (Resizer, error) {
+func ResizerFor(kind string, c ResizerClient) (Resizer, error) {
 	switch kind {
 	case "ReplicationController":
 		return &ReplicationControllerResizer{c}, nil
@@ -98,7 +98,7 @@ func ResizerFor(kind string, c client.Interface) (Resizer, error) {
 }
 
 type ReplicationControllerResizer struct {
-	client.Interface
+	c ResizerClient
 }
 
 type RetryParams struct {
@@ -122,8 +122,7 @@ func ResizeCondition(r Resizer, precondition *ResizePrecondition, namespace, nam
 }
 
 func (resizer *ReplicationControllerResizer) ResizeSimple(namespace, name string, preconditions *ResizePrecondition, newSize uint) (string, error) {
-	rc := resizer.ReplicationControllers(namespace)
+	controller, err := resizer.c.GetReplicationController(namespace, name)
-	controller, err := rc.Get(name)
 	if err != nil {
 		return "", ControllerResizeError{ControllerResizeGetFailure, "Unknown", err}
 	}
@@ -134,7 +133,7 @@ func (resizer *ReplicationControllerResizer) ResizeSimple(namespace, name string
 	}
 	controller.Spec.Replicas = int(newSize)
 	// TODO: do retry on 409 errors here?
-	if _, err := rc.Update(controller); err != nil {
+	if _, err := resizer.c.UpdateReplicationController(namespace, controller); err != nil {
 		return "", ControllerResizeError{ControllerResizeUpdateFailure, controller.ResourceVersion, err}
 	}
 	// TODO: do a better job of printing objects here.
@@ -159,9 +158,33 @@ func (resizer *ReplicationControllerResizer) Resize(namespace, name string, newS
 	if waitForReplicas != nil {
 		rc := &api.ReplicationController{ObjectMeta: api.ObjectMeta{Namespace: namespace, Name: name}}
 		if err := wait.Poll(waitForReplicas.interval, waitForReplicas.timeout,
-			client.ControllerHasDesiredReplicas(resizer, rc)); err != nil {
+			resizer.c.ControllerHasDesiredReplicas(rc)); err != nil {
 			return err
 		}
 	}
 	return nil
 }
+
+// ResizerClient abstracts access to ReplicationControllers.
+type ResizerClient interface {
+	GetReplicationController(namespace, name string) (*api.ReplicationController, error)
+	UpdateReplicationController(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error)
+	ControllerHasDesiredReplicas(rc *api.ReplicationController) wait.ConditionFunc
+}
+
+// RealResizerClient is a ResizerClient which uses a Kube client.
+type RealResizerClient struct {
+	Client client.Interface
+}
+
+func (c *RealResizerClient) GetReplicationController(namespace, name string) (*api.ReplicationController, error) {
+	return c.Client.ReplicationControllers(namespace).Get(name)
+}
+
+func (c *RealResizerClient) UpdateReplicationController(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) {
+	return c.Client.ReplicationControllers(namespace).Update(rc)
+}
+
+func (c *RealResizerClient) ControllerHasDesiredReplicas(rc *api.ReplicationController) wait.ConditionFunc {
+	return client.ControllerHasDesiredReplicas(c.Client, rc)
+}

pkg/kubectl/resize_test.go

@@ -43,7 +43,7 @@ func (c *ErrorReplicationControllerClient) ReplicationControllers(namespace stri
 
 func TestReplicationControllerResizeRetry(t *testing.T) {
 	fake := &ErrorReplicationControllerClient{Fake: testclient.Fake{}}
-	resizer := ReplicationControllerResizer{fake}
+	resizer := ReplicationControllerResizer{&RealResizerClient{fake}}
 	preconditions := ResizePrecondition{-1, ""}
 	count := uint(3)
 	name := "foo"
@@ -67,7 +67,7 @@ func TestReplicationControllerResizeRetry(t *testing.T) {
 
 func TestReplicationControllerResize(t *testing.T) {
 	fake := &testclient.Fake{}
-	resizer := ReplicationControllerResizer{fake}
+	resizer := ReplicationControllerResizer{&RealResizerClient{fake}}
 	preconditions := ResizePrecondition{-1, ""}
 	count := uint(3)
 	name := "foo"
@@ -90,7 +90,7 @@ func TestReplicationControllerResizeFailsPreconditions(t *testing.T) {
 			Replicas: 10,
 		},
 	})
-	resizer := ReplicationControllerResizer{fake}
+	resizer := ReplicationControllerResizer{&RealResizerClient{fake}}
 	preconditions := ResizePrecondition{2, ""}
 	count := uint(3)
 	name := "foo"

pkg/kubectl/rolling_updater.go

@@ -31,13 +31,13 @@ import (
 // fault-tolerant way.
 type RollingUpdater struct {
 	// Client interface for creating and updating controllers
-	c client.Interface
+	c RollingUpdaterClient
 	// Namespace for resources
 	ns string
 }
 
 // NewRollingUpdater creates a RollingUpdater from a client
-func NewRollingUpdater(namespace string, c client.Interface) *RollingUpdater {
+func NewRollingUpdater(namespace string, c RollingUpdaterClient) *RollingUpdater {
 	return &RollingUpdater{
 		c,
 		namespace,
@@ -95,7 +95,7 @@ func (r *RollingUpdater) Update(out io.Writer, oldRc, newRc *api.ReplicationCont
 		newRc.ObjectMeta.Annotations[desiredReplicasAnnotation] = fmt.Sprintf("%d", desired)
 		newRc.ObjectMeta.Annotations[sourceIdAnnotation] = sourceId
 		newRc.Spec.Replicas = 0
-		newRc, err = r.c.ReplicationControllers(r.ns).Create(newRc)
+		newRc, err = r.c.CreateReplicationController(r.ns, newRc)
 		if err != nil {
 			return err
 		}
@@ -147,7 +147,7 @@ func (r *RollingUpdater) Update(out io.Writer, oldRc, newRc *api.ReplicationCont
 		}
 	}
 	// Clean up annotations
-	if newRc, err = r.c.ReplicationControllers(r.ns).Get(newName); err != nil {
+	if newRc, err = r.c.GetReplicationController(r.ns, newName); err != nil {
 		return err
 	}
 	delete(newRc.ObjectMeta.Annotations, sourceIdAnnotation)
@@ -158,11 +158,11 @@ func (r *RollingUpdater) Update(out io.Writer, oldRc, newRc *api.ReplicationCont
 	}
 	// delete old rc
 	fmt.Fprintf(out, "Update succeeded. Deleting %s\n", oldName)
-	return r.c.ReplicationControllers(r.ns).Delete(oldName)
+	return r.c.DeleteReplicationController(r.ns, oldName)
 }
 
 func (r *RollingUpdater) getExistingNewRc(sourceId, name string) (rc *api.ReplicationController, existing bool, err error) {
-	if rc, err = r.c.ReplicationControllers(r.ns).Get(name); err == nil {
+	if rc, err = r.c.GetReplicationController(r.ns, name); err == nil {
 		existing = true
 		annotations := rc.ObjectMeta.Annotations
 		source := annotations[sourceIdAnnotation]
@@ -184,17 +184,50 @@ func (r *RollingUpdater) resizeAndWait(rc *api.ReplicationController, retry *Ret
 	if err := resizer.Resize(rc.Namespace, rc.Name, uint(rc.Spec.Replicas), &ResizePrecondition{-1, ""}, retry, wait); err != nil {
 		return nil, err
 	}
-	return r.c.ReplicationControllers(r.ns).Get(rc.ObjectMeta.Name)
+	return r.c.GetReplicationController(r.ns, rc.ObjectMeta.Name)
 }
 
 func (r *RollingUpdater) updateAndWait(rc *api.ReplicationController, interval, timeout time.Duration) (*api.ReplicationController, error) {
-	rc, err := r.c.ReplicationControllers(r.ns).Update(rc)
+	rc, err := r.c.UpdateReplicationController(r.ns, rc)
 	if err != nil {
 		return nil, err
 	}
-	if err = wait.Poll(interval, timeout,
+	if err = wait.Poll(interval, timeout, r.c.ControllerHasDesiredReplicas(rc)); err != nil {
-		client.ControllerHasDesiredReplicas(r.c, rc)); err != nil {
 		return nil, err
 	}
-	return r.c.ReplicationControllers(r.ns).Get(rc.ObjectMeta.Name)
+	return r.c.GetReplicationController(r.ns, rc.ObjectMeta.Name)
+}
+
+// RollingUpdaterClient abstracts access to ReplicationControllers.
+type RollingUpdaterClient interface {
+	GetReplicationController(namespace, name string) (*api.ReplicationController, error)
+	UpdateReplicationController(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error)
+	CreateReplicationController(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error)
+	DeleteReplicationController(namespace, name string) error
+	ControllerHasDesiredReplicas(rc *api.ReplicationController) wait.ConditionFunc
+}
+
+// RealRollingUpdaterClient is a RollingUpdaterClient which uses a Kube client.
+type RealRollingUpdaterClient struct {
+	Client client.Interface
+}
+
+func (c *RealRollingUpdaterClient) GetReplicationController(namespace, name string) (*api.ReplicationController, error) {
+	return c.Client.ReplicationControllers(namespace).Get(name)
+}
+
+func (c *RealRollingUpdaterClient) UpdateReplicationController(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) {
+	return c.Client.ReplicationControllers(namespace).Update(rc)
+}
+
+func (c *RealRollingUpdaterClient) CreateReplicationController(namespace string, rc *api.ReplicationController) (*api.ReplicationController, error) {
+	return c.Client.ReplicationControllers(namespace).Create(rc)
+}
+
+func (c *RealRollingUpdaterClient) DeleteReplicationController(namespace, name string) error {
+	return c.Client.ReplicationControllers(namespace).Delete(name)
+}
+
+func (c *RealRollingUpdaterClient) ControllerHasDesiredReplicas(rc *api.ReplicationController) wait.ConditionFunc {
+	return client.ControllerHasDesiredReplicas(c.Client, rc)
 }

pkg/kubectl/rolling_updater_test.go

@@ -253,7 +253,7 @@ Update succeeded. Deleting foo-v1
 
 	for _, test := range tests {
 		updater := RollingUpdater{
-			fakeClientFor("default", test.responses),
+			&RealRollingUpdaterClient{fakeClientFor("default", test.responses)},
 			"default",
 		}
 		var buffer bytes.Buffer
@@ -296,7 +296,7 @@ Update succeeded. Deleting foo-v1
 		{newRc(3, 3), nil},
 		{newRc(3, 3), nil},
 	}
-	updater := RollingUpdater{fakeClientFor("default", responses), "default"}
+	updater := RollingUpdater{&RealRollingUpdaterClient{fakeClientFor("default", responses)}, "default"}
 
 	var buffer bytes.Buffer
 	if err := updater.Update(&buffer, rc, rcExisting, 0, time.Millisecond, time.Millisecond); err != nil {

pkg/kubectl/stop.go

@@ -65,7 +65,7 @@ type objInterface interface {
 
 func (reaper *ReplicationControllerReaper) Stop(namespace, name string) (string, error) {
 	rc := reaper.ReplicationControllers(namespace)
-	resizer, err := ResizerFor("ReplicationController", *reaper)
+	resizer, err := ResizerFor("ReplicationController", &RealResizerClient{*reaper})
 	if err != nil {
 		return "", err
 	}