From 312ccad3c1ac2e660ce8d267a513ae381eda9b98 Mon Sep 17 00:00:00 2001 From: Dan Mace Date: Wed, 15 Apr 2015 14:28:59 -0400 Subject: [PATCH] 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 | 2 +- pkg/kubectl/cmd/util/factory.go | 2 +- pkg/kubectl/resize.go | 35 ++++++++++++++---- pkg/kubectl/resize_test.go | 6 ++-- pkg/kubectl/rolling_updater.go | 55 +++++++++++++++++++++++------ pkg/kubectl/rolling_updater_test.go | 4 +-- pkg/kubectl/stop.go | 2 +- 7 files changed, 81 insertions(+), 25 deletions(-) diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index 7b81a729585..32a65c4ccbd 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/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) diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index e9c4c4a7d1f..2cd314940c0 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/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) diff --git a/pkg/kubectl/resize.go b/pkg/kubectl/resize.go index 229768e5c85..ca82e65d84b 100644 --- a/pkg/kubectl/resize.go +++ b/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 := rc.Get(name) + controller, err := resizer.c.GetReplicationController(namespace, 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) +} diff --git a/pkg/kubectl/resize_test.go b/pkg/kubectl/resize_test.go index 50c711168d0..5c2d482af35 100644 --- a/pkg/kubectl/resize_test.go +++ b/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" diff --git a/pkg/kubectl/rolling_updater.go b/pkg/kubectl/rolling_updater.go index f350aaf22a0..0c4a901e8f7 100644 --- a/pkg/kubectl/rolling_updater.go +++ b/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, - client.ControllerHasDesiredReplicas(r.c, rc)); err != nil { + if err = wait.Poll(interval, timeout, r.c.ControllerHasDesiredReplicas(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) } diff --git a/pkg/kubectl/rolling_updater_test.go b/pkg/kubectl/rolling_updater_test.go index a13297e13ae..2c05b79002c 100644 --- a/pkg/kubectl/rolling_updater_test.go +++ b/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 { diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index 3e7b89f4f44..771b16d6c6b 100644 --- a/pkg/kubectl/stop.go +++ b/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 }