From 64f778804df8987e7e31403542f46f62359a0ea4 Mon Sep 17 00:00:00 2001 From: Maciej Szulik Date: Thu, 12 Nov 2015 15:42:29 +0100 Subject: [PATCH] Remove string from Reaper.Stop signature --- pkg/kubectl/cmd/delete.go | 2 +- pkg/kubectl/stop.go | 62 +++++++++++----------- pkg/kubectl/stop_test.go | 30 ++--------- test/e2e/daemon_set.go | 2 +- test/e2e/job.go | 2 +- test/e2e/util.go | 2 +- test/integration/framework/master_utils.go | 2 +- 7 files changed, 41 insertions(+), 61 deletions(-) diff --git a/pkg/kubectl/cmd/delete.go b/pkg/kubectl/cmd/delete.go index 7baaba3967f..5c5822f6a0a 100644 --- a/pkg/kubectl/cmd/delete.go +++ b/pkg/kubectl/cmd/delete.go @@ -165,7 +165,7 @@ func ReapResult(r *resource.Result, f *cmdutil.Factory, out io.Writer, isDefault if gracePeriod >= 0 { options = api.NewDeleteOptions(int64(gracePeriod)) } - if _, err := reaper.Stop(info.Namespace, info.Name, timeout, options); err != nil { + if err := reaper.Stop(info.Namespace, info.Name, timeout, options); err != nil { return cmdutil.AddSourceToErr("stopping", info.Source, err) } cmdutil.PrintSuccess(mapper, shortOutput, out, info.Mapping.Resource, info.Name, "deleted") diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index c082273810b..39da3ed8d87 100644 --- a/pkg/kubectl/stop.go +++ b/pkg/kubectl/stop.go @@ -41,7 +41,7 @@ const ( // timeout is how long we'll wait for the termination to be successful // gracePeriod is time given to an API object for it to delete itself cleanly (e.g. pod shutdown) type Reaper interface { - Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) + Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error } type NoSuchReaperError struct { @@ -118,15 +118,15 @@ func getOverlappingControllers(c client.ReplicationControllerInterface, rc *api. return matchingRCs, nil } -func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) { +func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { rc := reaper.ReplicationControllers(namespace) scaler, err := ScalerFor("ReplicationController", *reaper) if err != nil { - return "", err + return err } ctrl, err := rc.Get(name) if err != nil { - return "", err + return err } if timeout == 0 { timeout = Timeout + time.Duration(10*ctrl.Spec.Replicas)*time.Second @@ -154,7 +154,7 @@ func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout overlappingCtrls, err := getOverlappingControllers(rc, ctrl) if err != nil { - return "", fmt.Errorf("error getting replication controllers: %v", err) + return fmt.Errorf("error getting replication controllers: %v", err) } exactMatchRCs := []api.ReplicationController{} overlapRCs := []string{} @@ -166,7 +166,7 @@ func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout } } if len(overlapRCs) > 0 { - return "", fmt.Errorf( + return fmt.Errorf( "Detected overlapping controllers for rc %v: %v, please manage deletion individually with --cascade=false.", ctrl.Name, strings.Join(overlapRCs, ",")) } @@ -175,19 +175,19 @@ 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 + return err } } if err := rc.Delete(name); err != nil { - return "", err + return err } - return fmt.Sprintf("%s stopped", name), nil + return nil } -func (reaper *DaemonSetReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) { +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 { - return "", err + return err } // We set the nodeSelector to a random label. This label is nearly guaranteed @@ -201,7 +201,7 @@ func (reaper *DaemonSetReaper) Stop(namespace, name string, timeout time.Duratio ds.ResourceVersion = "" if ds, err = reaper.Extensions().DaemonSets(namespace).Update(ds); err != nil { - return "", err + return err } // Wait for the daemon set controller to kill all the daemon pods. @@ -212,25 +212,25 @@ func (reaper *DaemonSetReaper) Stop(namespace, name string, timeout time.Duratio } return updatedDS.Status.CurrentNumberScheduled+updatedDS.Status.NumberMisscheduled == 0, nil }); err != nil { - return "", err + return err } if err := reaper.Extensions().DaemonSets(namespace).Delete(name); err != nil { - return "", err + return err } - return fmt.Sprintf("%s stopped", name), nil + return nil } -func (reaper *JobReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) { +func (reaper *JobReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { jobs := reaper.Extensions().Jobs(namespace) pods := reaper.Pods(namespace) scaler, err := ScalerFor("Job", *reaper) if err != nil { - return "", err + return err } job, err := jobs.Get(name) if err != nil { - return "", err + return err } if timeout == 0 { // we will never have more active pods than job.Spec.Parallelism @@ -242,13 +242,13 @@ 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 + return err } // at this point only dead pods are left, that should be removed selector, _ := extensions.PodSelectorAsSelector(job.Spec.Selector) podList, err := pods.List(selector, fields.Everything()) if err != nil { - return "", err + return err } errList := []error{} for _, pod := range podList.Items { @@ -257,36 +257,36 @@ func (reaper *JobReaper) Stop(namespace, name string, timeout time.Duration, gra } } if len(errList) > 0 { - return "", utilerrors.NewAggregate(errList) + return utilerrors.NewAggregate(errList) } // once we have all the pods removed we can safely remove the job itself if err := jobs.Delete(name, gracePeriod); err != nil { - return "", err + return err } - return fmt.Sprintf("%s stopped", name), nil + return nil } -func (reaper *PodReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) { +func (reaper *PodReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { pods := reaper.Pods(namespace) _, err := pods.Get(name) if err != nil { - return "", err + return err } if err := pods.Delete(name, gracePeriod); err != nil { - return "", err + return err } - return fmt.Sprintf("%s stopped", name), nil + return nil } -func (reaper *ServiceReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) { +func (reaper *ServiceReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) error { services := reaper.Services(namespace) _, err := services.Get(name) if err != nil { - return "", err + return err } if err := services.Delete(name); err != nil { - return "", err + return err } - return fmt.Sprintf("%s stopped", name), nil + return nil } diff --git a/pkg/kubectl/stop_test.go b/pkg/kubectl/stop_test.go index 9b74a14ea13..f0772c1a0a2 100644 --- a/pkg/kubectl/stop_test.go +++ b/pkg/kubectl/stop_test.go @@ -37,7 +37,6 @@ func TestReplicationControllerStop(t *testing.T) { Name string Objs []runtime.Object StopError error - StopMessage string ExpectedActions []string }{ { @@ -67,7 +66,6 @@ func TestReplicationControllerStop(t *testing.T) { }, }, StopError: nil, - StopMessage: "foo stopped", ExpectedActions: []string{"get", "list", "get", "update", "get", "get", "delete"}, }, { @@ -106,7 +104,6 @@ func TestReplicationControllerStop(t *testing.T) { }, }, StopError: nil, - StopMessage: "foo stopped", ExpectedActions: []string{"get", "list", "get", "update", "get", "get", "delete"}, }, { @@ -146,7 +143,6 @@ func TestReplicationControllerStop(t *testing.T) { }, }, StopError: fmt.Errorf("Detected overlapping controllers for rc foo: baz, please manage deletion individually with --cascade=false."), - StopMessage: "", ExpectedActions: []string{"get", "list"}, }, @@ -197,7 +193,6 @@ func TestReplicationControllerStop(t *testing.T) { }, StopError: fmt.Errorf("Detected overlapping controllers for rc foo: baz,zaz, please manage deletion individually with --cascade=false."), - StopMessage: "", ExpectedActions: []string{"get", "list"}, }, @@ -239,7 +234,6 @@ func TestReplicationControllerStop(t *testing.T) { }, StopError: nil, - StopMessage: "foo stopped", ExpectedActions: []string{"get", "list", "delete"}, }, } @@ -247,16 +241,12 @@ func TestReplicationControllerStop(t *testing.T) { for _, test := range tests { fake := testclient.NewSimpleFake(test.Objs...) reaper := ReplicationControllerReaper{fake, time.Millisecond, time.Millisecond} - s, err := reaper.Stop(ns, name, 0, nil) + err := reaper.Stop(ns, name, 0, nil) if !reflect.DeepEqual(err, test.StopError) { t.Errorf("%s unexpected error: %v", test.Name, err) continue } - if s != test.StopMessage { - t.Errorf("%s expected '%s', got '%s'", test.Name, test.StopMessage, s) - 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)) @@ -281,7 +271,6 @@ func TestJobStop(t *testing.T) { Name string Objs []runtime.Object StopError error - StopMessage string ExpectedActions []string }{ { @@ -316,8 +305,7 @@ func TestJobStop(t *testing.T) { }, }, }, - StopError: nil, - StopMessage: "foo stopped", + StopError: nil, ExpectedActions: []string{"get:jobs", "get:jobs", "update:jobs", "get:jobs", "get:jobs", "list:pods", "delete:jobs"}, }, @@ -364,8 +352,7 @@ func TestJobStop(t *testing.T) { }, }, }, - StopError: nil, - StopMessage: "foo stopped", + StopError: nil, ExpectedActions: []string{"get:jobs", "get:jobs", "update:jobs", "get:jobs", "get:jobs", "list:pods", "delete:pods", "delete:jobs"}, }, @@ -374,16 +361,12 @@ func TestJobStop(t *testing.T) { for _, test := range tests { fake := testclient.NewSimpleFake(test.Objs...) reaper := JobReaper{fake, time.Millisecond, time.Millisecond} - s, err := reaper.Stop(ns, name, 0, nil) + err := reaper.Stop(ns, name, 0, nil) if !reflect.DeepEqual(err, test.StopError) { t.Errorf("%s unexpected error: %v", test.Name, err) continue } - if s != test.StopMessage { - t.Errorf("%s expected '%s', got '%s'", test.Name, test.StopMessage, s) - 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)) @@ -499,7 +482,7 @@ func TestSimpleStop(t *testing.T) { if err != nil { t.Errorf("unexpected error: %v (%s)", err, test.test) } - s, err := reaper.Stop("default", "foo", 0, nil) + err = reaper.Stop("default", "foo", 0, nil) if err != nil && !test.expectError { t.Errorf("unexpected error: %v (%s)", err, test.test) } @@ -507,9 +490,6 @@ func TestSimpleStop(t *testing.T) { if test.expectError { t.Errorf("unexpected non-error: %v (%s)", err, test.test) } - if s != "foo stopped" { - t.Errorf("unexpected return: %s (%s)", s, test.test) - } } actions := fake.Actions() if len(test.actions) != len(actions) { diff --git a/test/e2e/daemon_set.go b/test/e2e/daemon_set.go index 403d84d30ec..de591c17d76 100644 --- a/test/e2e/daemon_set.go +++ b/test/e2e/daemon_set.go @@ -100,7 +100,7 @@ var _ = Describe("Daemon set", func() { Logf("Check that reaper kills all daemon pods for %s", dsName) dsReaper, err := kubectl.ReaperFor("DaemonSet", c) Expect(err).NotTo(HaveOccurred()) - _, err = dsReaper.Stop(ns, dsName, 0, nil) + err = dsReaper.Stop(ns, dsName, 0, nil) Expect(err).NotTo(HaveOccurred()) err = wait.Poll(dsRetryPeriod, dsRetryTimeout, checkRunningOnNoNodes(f, label)) Expect(err).NotTo(HaveOccurred(), "error waiting for daemon pod to be reaped") diff --git a/test/e2e/job.go b/test/e2e/job.go index e1a26142891..bd657d9b266 100644 --- a/test/e2e/job.go +++ b/test/e2e/job.go @@ -175,7 +175,7 @@ var _ = Describe("Job", func() { reaper, err := kubectl.ReaperFor("Job", f.Client) Expect(err).NotTo(HaveOccurred()) timeout := 1 * time.Minute - _, err = reaper.Stop(f.Namespace.Name, job.Name, timeout, api.NewDeleteOptions(0)) + err = reaper.Stop(f.Namespace.Name, job.Name, timeout, api.NewDeleteOptions(0)) Expect(err).NotTo(HaveOccurred()) By("Ensuring job was deleted") diff --git a/test/e2e/util.go b/test/e2e/util.go index 4b0478987ca..5f749b9f58c 100644 --- a/test/e2e/util.go +++ b/test/e2e/util.go @@ -1685,7 +1685,7 @@ func DeleteRC(c *client.Client, ns, name string) error { return err } startTime := time.Now() - _, err = reaper.Stop(ns, name, 0, api.NewDeleteOptions(0)) + err = reaper.Stop(ns, name, 0, api.NewDeleteOptions(0)) if apierrs.IsNotFound(err) { Logf("RC %s was already deleted: %v", name, err) return nil diff --git a/test/integration/framework/master_utils.go b/test/integration/framework/master_utils.go index 258aa5d46dc..be379af1aab 100644 --- a/test/integration/framework/master_utils.go +++ b/test/integration/framework/master_utils.go @@ -193,7 +193,7 @@ func StopRC(rc *api.ReplicationController, restClient *client.Client) error { if err != nil || reaper == nil { return err } - _, err = reaper.Stop(rc.Namespace, rc.Name, 0, nil) + err = reaper.Stop(rc.Namespace, rc.Name, 0, nil) if err != nil { return err }