diff --git a/pkg/controller/deployment/util/BUILD b/pkg/controller/deployment/util/BUILD index 17aac73be64..7b4e638136b 100644 --- a/pkg/controller/deployment/util/BUILD +++ b/pkg/controller/deployment/util/BUILD @@ -37,6 +37,7 @@ go_library( "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/apimachinery/pkg/labels", "//vendor:k8s.io/apimachinery/pkg/runtime", + "//vendor:k8s.io/apimachinery/pkg/types", "//vendor:k8s.io/apimachinery/pkg/util/errors", "//vendor:k8s.io/apimachinery/pkg/util/intstr", "//vendor:k8s.io/apimachinery/pkg/util/wait", diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index 7ccb7b6102e..b0fcfcb0d37 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/errors" intstrutil "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/wait" @@ -507,7 +508,7 @@ func GetAllReplicaSets(deployment *extensions.Deployment, c clientset.Interface) if err != nil { return nil, nil, nil, err } - podList, err := listPods(deployment, c) + podList, err := listPods(deployment, rsList, c) if err != nil { return nil, nil, nil, err } @@ -529,7 +530,7 @@ func GetOldReplicaSets(deployment *extensions.Deployment, c clientset.Interface) if err != nil { return nil, nil, err } - podList, err := listPods(deployment, c) + podList, err := listPods(deployment, rsList, c) if err != nil { return nil, nil, err } @@ -563,8 +564,8 @@ func listReplicaSets(deployment *extensions.Deployment, c clientset.Interface) ( } // listReplicaSets lists all Pods the given deployment targets with the given client interface. -func listPods(deployment *extensions.Deployment, c clientset.Interface) (*v1.PodList, error) { - return ListPods(deployment, +func listPods(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet, c clientset.Interface) (*v1.PodList, error) { + return ListPods(deployment, rsList, func(namespace string, options metav1.ListOptions) (*v1.PodList, error) { return c.Core().Pods(namespace).List(options) }) @@ -575,28 +576,65 @@ type rsListFunc func(string, metav1.ListOptions) ([]*extensions.ReplicaSet, erro type podListFunc func(string, metav1.ListOptions) (*v1.PodList, error) // 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. func ListReplicaSets(deployment *extensions.Deployment, getRSList rsListFunc) ([]*extensions.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; - // or use controllerRef, see https://github.com/kubernetes/kubernetes/issues/2210 + // 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 { return nil, err } options := metav1.ListOptions{LabelSelector: selector.String()} - return getRSList(namespace, options) + all, err := getRSList(namespace, options) + if err != nil { + return all, err + } + // Only include those whose ControllerRef matches the Deployment. + owned := 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) + } + } + return owned, nil } // ListPods returns a list of pods the given deployment targets. -func ListPods(deployment *extensions.Deployment, getPodList podListFunc) (*v1.PodList, error) { +// This needs a list of ReplicaSets for the Deployment, +// which can be found with ListReplicaSets(). +// 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. +func ListPods(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) + all, err := getPodList(namespace, options) + if err != nil { + return all, err + } + // Only include those whose ControllerRef points to a ReplicaSet that is in + // turn owned by this Deployment. + rsMap := make(map[types.UID]bool, len(rsList)) + for _, rs := range rsList { + rsMap[rs.UID] = true + } + owned := &v1.PodList{Items: make([]v1.Pod, 0, len(all.Items))} + for i := range all.Items { + pod := &all.Items[i] + controllerRef := controller.GetControllerOf(pod) + if controllerRef != nil && rsMap[controllerRef.UID] { + owned.Items = append(owned.Items, *pod) + } + } + return owned, nil } // EqualIgnoreHash returns true if two given podTemplateSpec are equal, ignoring the diff in value of Labels[pod-template-hash] diff --git a/pkg/controller/deployment/util/deployment_util_test.go b/pkg/controller/deployment/util/deployment_util_test.go index 7bca36b9d20..add606ce043 100644 --- a/pkg/controller/deployment/util/deployment_util_test.go +++ b/pkg/controller/deployment/util/deployment_util_test.go @@ -105,11 +105,23 @@ func newPod(now time.Time, ready bool, beforeSec int) v1.Pod { } } +func newRSControllerRef(rs *extensions.ReplicaSet) *metav1.OwnerReference { + isController := true + return &metav1.OwnerReference{ + APIVersion: "extensions/v1beta1", + Kind: "ReplicaSet", + Name: rs.GetName(), + UID: rs.GetUID(), + Controller: &isController, + } +} + // generatePodFromRS creates a pod, with the input ReplicaSet's selector and its template func generatePodFromRS(rs extensions.ReplicaSet) v1.Pod { return v1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Labels: rs.Labels, + Labels: rs.Labels, + OwnerReferences: []metav1.OwnerReference{*newRSControllerRef(&rs)}, }, Spec: rs.Spec.Template.Spec, } @@ -161,14 +173,26 @@ func generateRSWithLabel(labels map[string]string, image string) extensions.Repl } } +func newDControllerRef(d *extensions.Deployment) *metav1.OwnerReference { + isController := true + return &metav1.OwnerReference{ + APIVersion: "extensions/v1beta1", + Kind: "Deployment", + Name: d.GetName(), + UID: d.GetUID(), + Controller: &isController, + } +} + // generateRS creates a replica set, with the input deployment's template as its template func generateRS(deployment extensions.Deployment) extensions.ReplicaSet { template := GetNewReplicaSetTemplate(&deployment) return extensions.ReplicaSet{ ObjectMeta: metav1.ObjectMeta{ - UID: randomUID(), - Name: v1.SimpleNameGenerator.GenerateName("replicaset"), - Labels: template.Labels, + UID: randomUID(), + Name: v1.SimpleNameGenerator.GenerateName("replicaset"), + Labels: template.Labels, + OwnerReferences: []metav1.OwnerReference{*newDControllerRef(&deployment)}, }, Spec: extensions.ReplicaSetSpec{ Replicas: func() *int32 { i := int32(0); return &i }(), @@ -291,7 +315,8 @@ func TestGetOldRCs(t *testing.T) { oldRS2.Status.FullyLabeledReplicas = *(oldRS2.Spec.Replicas) oldPod2 := generatePodFromRS(oldRS2) - // create 1 ReplicaSet that existed before the deployment, with the same labels as the deployment + // create 1 ReplicaSet that existed before the deployment, + // with the same labels as the deployment, but no ControllerRef. existedPod := generatePod(newDeployment.Spec.Template.Labels, "foo") existedRS := generateRSWithLabel(newDeployment.Spec.Template.Labels, "foo") existedRS.Status.FullyLabeledReplicas = *(existedRS.Spec.Replicas) @@ -345,7 +370,7 @@ func TestGetOldRCs(t *testing.T) { }, }, }, - []*extensions.ReplicaSet{&oldRS, &oldRS2, &existedRS}, + []*extensions.ReplicaSet{&oldRS, &oldRS2}, }, } diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index b172c0d8874..bcde917991d 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -3515,7 +3515,22 @@ func WaitForDeploymentWithCondition(c clientset.Interface, ns, deploymentName, r func logPodsOfDeployment(c clientset.Interface, deployment *extensions.Deployment) { minReadySeconds := deployment.Spec.MinReadySeconds - podList, err := deploymentutil.ListPods(deployment, + rsList, err := deploymentutil.ListReplicaSets(deployment, + func(namespace string, options metav1.ListOptions) ([]*extensions.ReplicaSet, error) { + rsList, err := c.Extensions().ReplicaSets(namespace).List(options) + if err != nil { + return nil, err + } + ret := make([]*extensions.ReplicaSet, 0, len(rsList.Items)) + for i := range rsList.Items { + ret = append(ret, &rsList.Items[i]) + } + return ret, nil + }) + if err != nil { + Logf("Failed to list ReplicaSets of Deployment %s: %v", deployment.Name, err) + } + podList, err := deploymentutil.ListPods(deployment, rsList, func(namespace string, options metav1.ListOptions) (*v1.PodList, error) { return c.Core().Pods(namespace).List(options) })