kubectl: Use v1.5-compatible ownership logic when listing dependents.

In particular, we should not assume ControllerRefs are necessarily set.
However, we can still use ControllerRefs that do exist to avoid
interfering with controllers that do use it.

pkg/controller/deployment/util/deployment_util.go

@@ -518,14 +518,15 @@ func GetAllReplicaSets(deployment *extensions.Deployment, c clientset.Interface)
 	return oldRSes, allOldRSes, newRS, nil
 }
 
-// GetAllReplicaSetsV15 is a compatibility function that behaves the way
+// GetAllReplicaSetsV15 is a compatibility function that emulates the behavior
-// GetAllReplicaSets() used to in v1.5.x.
+// from v1.5.x (list matching objects by selector) except that it leaves out
+// objects that are explicitly marked as being controlled by something else.
 func GetAllReplicaSetsV15(deployment *extensions.Deployment, c clientset.Interface) ([]*extensions.ReplicaSet, []*extensions.ReplicaSet, *extensions.ReplicaSet, error) {
 	rsList, err := ListReplicaSetsV15(deployment, rsListFromClient(c))
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	podList, err := ListPodsV15(deployment, podListFromClient(c))
+	podList, err := ListPodsV15(deployment, rsList, podListFromClient(c))
 	if err != nil {
 		return nil, nil, nil, err
 	}
@@ -564,8 +565,9 @@ func GetNewReplicaSet(deployment *extensions.Deployment, c clientset.Interface)
 	return FindNewReplicaSet(deployment, rsList)
 }
 
-// GetNewReplicaSetV15 is a compatibility function that behaves the way
+// GetNewReplicaSetV15 is a compatibility function that emulates the behavior
-// GetNewReplicaSet() used to in v1.5.x.
+// from v1.5.x (list matching objects by selector) except that it leaves out
+// objects that are explicitly marked as being controlled by something else.
 func GetNewReplicaSetV15(deployment *extensions.Deployment, c clientset.Interface) (*extensions.ReplicaSet, error) {
 	rsList, err := ListReplicaSetsV15(deployment, rsListFromClient(c))
 	if err != nil {
@@ -628,26 +630,10 @@ func ListReplicaSets(deployment *extensions.Deployment, getRSList rsListFunc) ([
 	return owned, nil
 }
 
-// ListReplicaSetsV15 is a compatibility function that behaves the way
+// ListReplicaSetsV15 is a compatibility function that emulates the behavior
-// ListReplicaSets() used to in v1.5.x.
+// from v1.5.x (list matching objects by selector) except that it leaves out
+// objects that are explicitly marked as being controlled by something else.
 func ListReplicaSetsV15(deployment *extensions.Deployment, getRSList rsListFunc) ([]*extensions.ReplicaSet, error) {
-	namespace := deployment.Namespace
-	selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
-	if err != nil {
-		return nil, err
-	}
-	options := metav1.ListOptions{LabelSelector: selector.String()}
-	return getRSList(namespace, options)
-}
-
-// ListReplicaSets returns a slice of RSes the given deployment targets.
-// Note that this does NOT attempt to reconcile ControllerRef (adopt/orphan),
-// because only the controller itself should do that.
-// However, it does filter out anything whose ControllerRef doesn't match.
-// TODO: Remove the duplicate.
-func ListReplicaSetsInternal(deployment *internalextensions.Deployment, getRSList func(string, metav1.ListOptions) ([]*internalextensions.ReplicaSet, error)) ([]*internalextensions.ReplicaSet, error) {
-	// TODO: Right now we list replica sets by their labels. We should list them by selector, i.e. the replica set's selector
-	//       should be a superset of the deployment's selector, see https://github.com/kubernetes/kubernetes/issues/19830.
 	namespace := deployment.Namespace
 	selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
 	if err != nil {
@@ -656,17 +642,49 @@ func ListReplicaSetsInternal(deployment *internalextensions.Deployment, getRSLis
 	options := metav1.ListOptions{LabelSelector: selector.String()}
 	all, err := getRSList(namespace, options)
 	if err != nil {
-		return all, err
+		return nil, err
 	}
-	// Only include those whose ControllerRef matches the Deployment.
+	// Since this function maintains compatibility with v1.5, the objects we want
-	owned := make([]*internalextensions.ReplicaSet, 0, len(all))
+	// do not necessarily have ControllerRefs pointing to us.
+	// However, we can at least avoid interfering with other controllers that do
+	// use ControllerRef.
+	filtered := make([]*extensions.ReplicaSet, 0, len(all))
 	for _, rs := range all {
 		controllerRef := controller.GetControllerOf(rs)
-		if controllerRef != nil && controllerRef.UID == deployment.UID {
+		if controllerRef != nil && controllerRef.UID != deployment.UID {
-			owned = append(owned, rs)
+			continue
 		}
+		filtered = append(filtered, rs)
 	}
-	return owned, nil
+	return filtered, nil
+}
+
+// ListReplicaSetsInternalV15 is ListReplicaSetsV15 for internalextensions.
+// TODO: Remove the duplicate when call sites are updated to ListReplicaSetsV15.
+func ListReplicaSetsInternalV15(deployment *internalextensions.Deployment, getRSList func(string, metav1.ListOptions) ([]*internalextensions.ReplicaSet, error)) ([]*internalextensions.ReplicaSet, error) {
+	namespace := deployment.Namespace
+	selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
+	if err != nil {
+		return nil, err
+	}
+	options := metav1.ListOptions{LabelSelector: selector.String()}
+	all, err := getRSList(namespace, options)
+	if err != nil {
+		return nil, err
+	}
+	// Since this function maintains compatibility with v1.5, the objects we want
+	// do not necessarily have ControllerRefs pointing to us.
+	// However, we can at least avoid interfering with other controllers that do
+	// use ControllerRef.
+	filtered := make([]*internalextensions.ReplicaSet, 0, len(all))
+	for _, rs := range all {
+		controllerRef := controller.GetControllerOf(rs)
+		if controllerRef != nil && controllerRef.UID != deployment.UID {
+			continue
+		}
+		filtered = append(filtered, rs)
+	}
+	return filtered, nil
 }
 
 // ListPods returns a list of pods the given deployment targets.
@@ -703,16 +721,39 @@ func ListPods(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet
 	return owned, nil
 }
 
-// ListPodsV15 is a compatibility function that behaves the way
+// ListPodsV15 is a compatibility function that emulates the behavior
-// ListPods() used to in v1.5.x.
+// from v1.5.x (list matching objects by selector) except that it leaves out
-func ListPodsV15(deployment *extensions.Deployment, getPodList podListFunc) (*v1.PodList, error) {
+// objects that are explicitly marked as being controlled by something else.
+func ListPodsV15(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet, getPodList podListFunc) (*v1.PodList, error) {
 	namespace := deployment.Namespace
 	selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
 	if err != nil {
 		return nil, err
 	}
 	options := metav1.ListOptions{LabelSelector: selector.String()}
-	return getPodList(namespace, options)
+	podList, err := getPodList(namespace, options)
+	if err != nil {
+		return nil, err
+	}
+	// Since this function maintains compatibility with v1.5, the objects we want
+	// do not necessarily have ControllerRefs pointing to one of our ReplicaSets.
+	// However, we can at least avoid interfering with other controllers that do
+	// use ControllerRef.
+	rsMap := make(map[types.UID]bool, len(rsList))
+	for _, rs := range rsList {
+		rsMap[rs.UID] = true
+	}
+	filtered := make([]v1.Pod, 0, len(podList.Items))
+	for i := range podList.Items {
+		pod := &podList.Items[i]
+		controllerRef := controller.GetControllerOf(pod)
+		if controllerRef != nil && !rsMap[controllerRef.UID] {
+			continue
+		}
+		filtered = append(filtered, *pod)
+	}
+	podList.Items = filtered
+	return podList, nil
 }
 
 // EqualIgnoreHash returns true if two given podTemplateSpec are equal, ignoring the diff in value of Labels[pod-template-hash]

pkg/kubectl/BUILD

@@ -72,6 +72,7 @@ go_library(
         "//pkg/client/clientset_generated/internalclientset/typed/extensions/internalversion:go_default_library",
         "//pkg/client/retry:go_default_library",
         "//pkg/client/unversioned:go_default_library",
+        "//pkg/controller:go_default_library",
         "//pkg/controller/deployment/util:go_default_library",
         "//pkg/credentialprovider:go_default_library",
         "//pkg/kubectl/resource:go_default_library",

pkg/kubectl/history.go

@@ -65,7 +65,7 @@ func (h *DeploymentHistoryViewer) ViewHistory(namespace, name string, revision i
 	if err != nil {
 		return "", fmt.Errorf("failed to retrieve deployment %s: %v", name, err)
 	}
-	_, allOldRSs, newRS, err := deploymentutil.GetAllReplicaSets(deployment, versionedClient)
+	_, allOldRSs, newRS, err := deploymentutil.GetAllReplicaSetsV15(deployment, versionedClient)
 	if err != nil {
 		return "", fmt.Errorf("failed to retrieve replica sets from deployment %s: %v", name, err)
 	}

pkg/kubectl/rollback.go

@@ -140,7 +140,7 @@ func simpleDryRun(deployment *extensions.Deployment, c clientset.Interface, toRe
 		return "", fmt.Errorf("failed to convert deployment, %v", err)
 	}
 	versionedClient := versionedClientsetForDeployment(c)
-	_, allOldRSs, newRS, err := deploymentutil.GetAllReplicaSets(externalDeployment, versionedClient)
+	_, allOldRSs, newRS, err := deploymentutil.GetAllReplicaSetsV15(externalDeployment, versionedClient)
 	if err != nil {
 		return "", fmt.Errorf("failed to retrieve replica sets from deployment %s: %v", deployment.Name, err)
 	}

pkg/kubectl/stop.go

@@ -37,6 +37,7 @@ import (
 	batchclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/batch/internalversion"
 	coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
 	extensionsclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/extensions/internalversion"
+	"k8s.io/kubernetes/pkg/controller"
 	deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util"
 	"k8s.io/kubernetes/pkg/util"
 )
@@ -355,7 +356,16 @@ func (reaper *StatefulSetReaper) Stop(namespace, name string, timeout time.Durat
 	}
 
 	errList := []error{}
-	for _, pod := range podList.Items {
+	for i := range podList.Items {
+		pod := &podList.Items[i]
+		// Since the client must maintain compatibility with a v1.5 server,
+		// we can't assume the Pods will have ControllerRefs pointing to 'ss'.
+		// However, we can at least avoid interfering with other controllers
+		// that do use ControllerRef.
+		controllerRef := controller.GetControllerOf(pod)
+		if controllerRef != nil && controllerRef.UID != ss.UID {
+			continue
+		}
 		if err := pods.Delete(pod.Name, gracePeriod); err != nil {
 			if !errors.IsNotFound(err) {
 				errList = append(errList, err)
@@ -448,7 +458,7 @@ func (reaper *DeploymentReaper) Stop(namespace, name string, timeout time.Durati
 	}
 
 	// Stop all replica sets belonging to this Deployment.
-	rss, err := deploymentutil.ListReplicaSetsInternal(deployment,
+	rss, err := deploymentutil.ListReplicaSetsInternalV15(deployment,
 		func(namespace string, options metav1.ListOptions) ([]*extensions.ReplicaSet, error) {
 			rsList, err := reaper.rsClient.ReplicaSets(namespace).List(options)
 			if err != nil {

pkg/kubectl/stop_test.go

@@ -476,6 +476,7 @@ func TestDeploymentStop(t *testing.T) {
 				&deployment, // GET
 				&extensions.ReplicaSetList{ // LIST
 					Items: []extensions.ReplicaSet{
+						// ReplicaSet owned by this Deployment.
 						{
 							ObjectMeta: metav1.ObjectMeta{
 								Name:      name,
@@ -484,7 +485,7 @@ func TestDeploymentStop(t *testing.T) {
 								OwnerReferences: []metav1.OwnerReference{
 									{
 										APIVersion: extensions.SchemeGroupVersion.String(),
-										Kind:       "ReplicaSet",
+										Kind:       "Deployment",
 										Name:       deployment.Name,
 										UID:        deployment.UID,
 										Controller: &trueVar,
@@ -495,6 +496,72 @@ func TestDeploymentStop(t *testing.T) {
 								Template: template,
 							},
 						},
+						// ReplicaSet owned by something else (should be ignored).
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      "rs2",
+								Namespace: ns,
+								Labels:    map[string]string{"k1": "v1"},
+								OwnerReferences: []metav1.OwnerReference{
+									{
+										APIVersion: extensions.SchemeGroupVersion.String(),
+										Kind:       "Deployment",
+										Name:       "somethingelse",
+										UID:        uuid.NewUUID(),
+										Controller: &trueVar,
+									},
+								},
+							},
+							Spec: extensions.ReplicaSetSpec{
+								Template: template,
+							},
+						},
+					},
+				},
+			},
+			StopError: nil,
+			ExpectedActions: []string{"get:deployments", "update:deployments",
+				"get:deployments", "list:replicasets", "get:replicasets",
+				"get:replicasets", "update:replicasets", "get:replicasets",
+				"get:replicasets", "delete:replicasets", "delete:deployments"},
+		},
+		{
+			Name: "Deployment with single replicaset, no ControllerRef (old cluster)",
+			Objs: []runtime.Object{
+				&deployment, // GET
+				&extensions.ReplicaSetList{ // LIST
+					Items: []extensions.ReplicaSet{
+						// ReplicaSet that matches but with no ControllerRef.
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      name,
+								Namespace: ns,
+								Labels:    map[string]string{"k1": "v1"},
+							},
+							Spec: extensions.ReplicaSetSpec{
+								Template: template,
+							},
+						},
+						// ReplicaSet owned by something else (should be ignored).
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      "rs2",
+								Namespace: ns,
+								Labels:    map[string]string{"k1": "v1"},
+								OwnerReferences: []metav1.OwnerReference{
+									{
+										APIVersion: extensions.SchemeGroupVersion.String(),
+										Kind:       "Deployment",
+										Name:       "somethingelse",
+										UID:        uuid.NewUUID(),
+										Controller: &trueVar,
+									},
+								},
+							},
+							Spec: extensions.ReplicaSetSpec{
+								Template: template,
+							},
+						},
 					},
 				},
 			},

test/e2e/framework/util.go

@@ -3555,7 +3555,7 @@ func logPodsOfDeployment(c clientset.Interface, deployment *extensions.Deploymen
 	var podList *v1.PodList
 	var err error
 	if v15Compatible {
-		podList, err = deploymentutil.ListPodsV15(deployment, podListFunc)
+		podList, err = deploymentutil.ListPodsV15(deployment, rsList, podListFunc)
 	} else {
 		podList, err = deploymentutil.ListPods(deployment, rsList, podListFunc)
 	}