Merge pull request #6871 from ironcladlou/rolling-update-refactor

Use narrowly scoped interfaces for client access

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.NewRollingUpdaterClient(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.NewResizerClient(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,37 @@ 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
+}
+
+func NewResizerClient(c client.Interface) ResizerClient {
+	return &realResizerClient{c}
+}
+
+// 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{NewResizerClient(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{NewResizerClient(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{NewResizerClient(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,54 @@ 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
+}
+
+func NewRollingUpdaterClient(c client.Interface) RollingUpdaterClient {
+	return &realRollingUpdaterClient{c}
+}
+
+// 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),
+			NewRollingUpdaterClient(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{NewRollingUpdaterClient(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", NewResizerClient(*reaper))
 	if err != nil {
 		return "", err
 	}