diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index ff10001c683..ee7b3995c13 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/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 -// GetAllReplicaSets() used to in v1.5.x. +// GetAllReplicaSetsV15 is a compatibility function that emulates the behavior +// 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 -// GetNewReplicaSet() used to in v1.5.x. +// GetNewReplicaSetV15 is a compatibility function that emulates the behavior +// 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 -// ListReplicaSets() used to in v1.5.x. +// ListReplicaSetsV15 is a compatibility function that emulates the behavior +// 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. - owned := make([]*internalextensions.ReplicaSet, 0, len(all)) + // 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([]*extensions.ReplicaSet, 0, len(all)) for _, rs := range all { controllerRef := controller.GetControllerOf(rs) - if controllerRef != nil && controllerRef.UID == deployment.UID { - owned = append(owned, rs) + if controllerRef != nil && controllerRef.UID != deployment.UID { + 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 -// ListPods() used to in v1.5.x. -func ListPodsV15(deployment *extensions.Deployment, getPodList podListFunc) (*v1.PodList, error) { +// ListPodsV15 is a compatibility function that emulates the behavior +// 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 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] diff --git a/pkg/kubectl/BUILD b/pkg/kubectl/BUILD index 9138c980c93..2df52fc48ea 100644 --- a/pkg/kubectl/BUILD +++ b/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", diff --git a/pkg/kubectl/history.go b/pkg/kubectl/history.go index 0c9398bbb84..f8077d0027a 100644 --- a/pkg/kubectl/history.go +++ b/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) } diff --git a/pkg/kubectl/rollback.go b/pkg/kubectl/rollback.go index f57143227f5..398a0079f39 100644 --- a/pkg/kubectl/rollback.go +++ b/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) } diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index 9ccd9e40792..04df667c628 100644 --- a/pkg/kubectl/stop.go +++ b/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 { diff --git a/pkg/kubectl/stop_test.go b/pkg/kubectl/stop_test.go index 95a8b243472..ea3109ffa26 100644 --- a/pkg/kubectl/stop_test.go +++ b/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, + }, + }, }, }, }, diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index d71ea81f2cd..658d6b16b8e 100644 --- a/test/e2e/framework/util.go +++ b/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) }