Merge pull request #17161 from soltysh/strip_reaper

Auto commit by PR queue bot

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")

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
 }

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,
+			StopError: nil,
-			StopMessage: "foo stopped",
 			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,
+			StopError: nil,
-			StopMessage: "foo stopped",
 			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) {

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")

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")

test/e2e/util.go

@@ -1717,7 +1717,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

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
 	}