fix kubectl rolling-update when GC is enabled

pkg/client/unversioned/replication_controllers.go

@@ -33,7 +33,7 @@ type ReplicationControllerInterface interface {
 	Create(ctrl *api.ReplicationController) (*api.ReplicationController, error)
 	Update(ctrl *api.ReplicationController) (*api.ReplicationController, error)
 	UpdateStatus(ctrl *api.ReplicationController) (*api.ReplicationController, error)
-	Delete(name string) error
+	Delete(name string, options *api.DeleteOptions) error
 	Watch(opts api.ListOptions) (watch.Interface, error)
 }
 
@@ -84,8 +84,8 @@ func (c *replicationControllers) UpdateStatus(controller *api.ReplicationControl
 }
 
 // Delete deletes an existing replication controller.
-func (c *replicationControllers) Delete(name string) error {
-	return c.r.Delete().Namespace(c.ns).Resource("replicationControllers").Name(name).Do().Error()
+func (c *replicationControllers) Delete(name string, options *api.DeleteOptions) error {
+	return c.r.Delete().Namespace(c.ns).Resource("replicationControllers").Name(name).Body(options).Do().Error()
 }
 
 // Watch returns a watch.Interface that watches the requested controllers.

pkg/client/unversioned/replication_controllers_test.go

@@ -165,7 +165,7 @@ func TestDeleteController(t *testing.T) {
 		Request:  simple.Request{Method: "DELETE", Path: testapi.Default.ResourcePath(getRCResourceName(), ns, "foo"), Query: simple.BuildQueryValues(nil)},
 		Response: simple.Response{StatusCode: 200},
 	}
-	err := c.Setup(t).ReplicationControllers(ns).Delete("foo")
+	err := c.Setup(t).ReplicationControllers(ns).Delete("foo", nil)
 	defer c.Close()
 	c.Validate(t, nil, err)
 }

pkg/client/unversioned/testclient/fake_replication_controllers.go

@@ -72,7 +72,7 @@ func (c *FakeReplicationControllers) UpdateStatus(controller *api.ReplicationCon
 	return obj.(*api.ReplicationController), err
 }
 
-func (c *FakeReplicationControllers) Delete(name string) error {
+func (c *FakeReplicationControllers) Delete(name string, options *api.DeleteOptions) error {
 	_, err := c.Fake.Invokes(NewDeleteAction("replicationcontrollers", c.Namespace, name), &api.ReplicationController{})
 	return err
 }

pkg/kubectl/rolling_updater.go

@@ -513,11 +513,11 @@ func (r *RollingUpdater) cleanupWithClients(oldRc, newRc *api.ReplicationControl
 	case DeleteRollingUpdateCleanupPolicy:
 		// delete old rc
 		fmt.Fprintf(config.Out, "Update succeeded. Deleting %s\n", oldRc.Name)
-		return r.c.ReplicationControllers(r.ns).Delete(oldRc.Name)
+		return r.c.ReplicationControllers(r.ns).Delete(oldRc.Name, nil)
 	case RenameRollingUpdateCleanupPolicy:
 		// delete old rc
 		fmt.Fprintf(config.Out, "Update succeeded. Deleting old controller: %s\n", oldRc.Name)
-		if err := r.c.ReplicationControllers(r.ns).Delete(oldRc.Name); err != nil {
+		if err := r.c.ReplicationControllers(r.ns).Delete(oldRc.Name, nil); err != nil {
 			return err
 		}
 		fmt.Fprintf(config.Out, "Renaming %s to %s\n", newRc.Name, oldRc.Name)
@@ -533,13 +533,28 @@ func Rename(c client.ReplicationControllersNamespacer, rc *api.ReplicationContro
 	oldName := rc.Name
 	rc.Name = newName
 	rc.ResourceVersion = ""
-
-	_, err := c.ReplicationControllers(rc.Namespace).Create(rc)
+	// First delete the oldName RC and orphan its pods.
+	trueVar := true
+	err := c.ReplicationControllers(rc.Namespace).Delete(oldName, &api.DeleteOptions{OrphanDependents: &trueVar})
+	if err != nil && !errors.IsNotFound(err) {
+		return err
+	}
+	err = wait.Poll(5*time.Second, 60*time.Second, func() (bool, error) {
+		_, err := c.ReplicationControllers(rc.Namespace).Get(oldName)
+		if err == nil {
+			return false, nil
+		} else if errors.IsNotFound(err) {
+			return true, nil
+		} else {
+			return false, err
+		}
+	})
 	if err != nil {
 		return err
 	}
-	err = c.ReplicationControllers(rc.Namespace).Delete(oldName)
-	if err != nil && !errors.IsNotFound(err) {
+	// Then create the same RC with the new name.
+	_, err = c.ReplicationControllers(rc.Namespace).Create(rc)
+	if err != nil {
 		return err
 	}
 	return nil

pkg/kubectl/rolling_updater_test.go

@@ -27,6 +27,7 @@ import (
 	"time"
 
 	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/errors"
 	"k8s.io/kubernetes/pkg/api/testapi"
 	apitesting "k8s.io/kubernetes/pkg/api/testing"
 	"k8s.io/kubernetes/pkg/api/unversioned"
@@ -1143,20 +1144,27 @@ func TestRollingUpdater_cleanupWithClients(t *testing.T) {
 				"delete",
 			},
 		},
-		{
-			name:      "rename",
-			policy:    RenameRollingUpdateCleanupPolicy,
-			responses: []runtime.Object{rcExisting},
-			expected: []string{
-				"get",
-				"update",
-				"get",
-				"get",
-				"delete",
-				"create",
-				"delete",
-			},
-		},
+		//{
+		// This cases is separated to a standalone
+		// TestRollingUpdater_cleanupWithClients_Rename. We have to do this
+		// because the unversioned fake client is unable to delete objects.
+		// TODO: uncomment this case when the unversioned fake client uses
+		// pkg/client/testing/core.
+		//	{
+		//		name:      "rename",
+		//		policy:    RenameRollingUpdateCleanupPolicy,
+		//		responses: []runtime.Object{rcExisting},
+		//		expected: []string{
+		//			"get",
+		//			"update",
+		//			"get",
+		//			"get",
+		//			"delete",
+		//			"create",
+		//			"delete",
+		//		},
+		//	},
+		//},
 	}
 
 	for _, test := range tests {
@@ -1189,6 +1197,39 @@ func TestRollingUpdater_cleanupWithClients(t *testing.T) {
 	}
 }
 
+// TestRollingUpdater_cleanupWithClients_Rename tests the rename cleanup policy. It's separated to
+// a standalone test because the unversioned fake client is unable to delete
+// objects.
+// TODO: move this test back to TestRollingUpdater_cleanupWithClients
+// when the fake client uses pkg/client/testing/core in the future.
+func TestRollingUpdater_cleanupWithClients_Rename(t *testing.T) {
+	rc := oldRc(2, 2)
+	rcExisting := newRc(1, 3)
+	expectedActions := []string{"delete", "get", "create"}
+	fake := &testclient.Fake{}
+	fake.AddReactor("*", "*", func(action testclient.Action) (handled bool, ret runtime.Object, err error) {
+		switch action.(type) {
+		case testclient.CreateAction:
+			return true, nil, nil
+		case testclient.GetAction:
+			return true, nil, errors.NewNotFound(unversioned.GroupResource{}, "")
+		case testclient.DeleteAction:
+			return true, nil, nil
+		}
+		return false, nil, nil
+	})
+
+	err := Rename(fake, rcExisting, rc.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for j, action := range fake.Actions() {
+		if e, a := expectedActions[j], action.GetVerb(); e != a {
+			t.Errorf("unexpected action: expected %s, got %s", e, a)
+		}
+	}
+}
+
 func TestFindSourceController(t *testing.T) {
 	ctrl1 := api.ReplicationController{
 		ObjectMeta: api.ObjectMeta{

pkg/kubectl/stop.go

@@ -202,7 +202,7 @@ func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout
 			return err
 		}
 	}
-	return rc.Delete(name)
+	return rc.Delete(name, nil)
 }
 
 // TODO(madhusudancs): Implement it when controllerRef is implemented - https://github.com/kubernetes/kubernetes/issues/2210

test/e2e/density.go

@@ -628,7 +628,7 @@ var _ = framework.KubeDescribe("Density", func() {
 			By("Removing additional replication controllers if any")
 			for i := 1; i <= nodeCount; i++ {
 				name := additionalPodsPrefix + "-" + strconv.Itoa(i)
-				c.ReplicationControllers(ns).Delete(name)
+				c.ReplicationControllers(ns).Delete(name, nil)
 			}
 		})
 	}

test/e2e/resource_quota.go

@@ -258,7 +258,7 @@ var _ = framework.KubeDescribe("ResourceQuota", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		By("Deleting a ReplicationController")
-		err = f.Client.ReplicationControllers(f.Namespace.Name).Delete(replicationController.Name)
+		err = f.Client.ReplicationControllers(f.Namespace.Name).Delete(replicationController.Name, nil)
 		Expect(err).NotTo(HaveOccurred())
 
 		By("Ensuring resource quota status released usage")

test/e2e/service.go

@@ -1970,7 +1970,7 @@ func (t *ServiceTestFixture) Cleanup() []error {
 		// TODO(mikedanese): Wait.
 
 		// Then, delete the RC altogether.
-		if err := t.Client.ReplicationControllers(t.Namespace).Delete(rcName); err != nil {
+		if err := t.Client.ReplicationControllers(t.Namespace).Delete(rcName, nil); err != nil {
 			errs = append(errs, err)
 		}
 	}

test/e2e/serviceloadbalancers.go

@@ -136,7 +136,7 @@ func (h *haproxyControllerTester) start(namespace string) (err error) {
 }
 
 func (h *haproxyControllerTester) stop() error {
-	return h.client.ReplicationControllers(h.rcNamespace).Delete(h.rcName)
+	return h.client.ReplicationControllers(h.rcNamespace).Delete(h.rcName, nil)
 }
 
 func (h *haproxyControllerTester) lookup(ingressKey string) string {

test/integration/framework/master_utils.go

@@ -351,7 +351,7 @@ func StartPods(namespace string, numPods int, host string, restClient *client.Cl
 	} else {
 		// Delete the rc, otherwise when we restart master components for the next benchmark
 		// the rc controller will race with the pods controller in the rc manager.
-		return restClient.ReplicationControllers(namespace).Delete(rc.Name)
+		return restClient.ReplicationControllers(namespace).Delete(rc.Name, nil)
 	}
 }