From b28a83a4cf779189d72a87e847441888e7918e5d Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Wed, 2 Aug 2017 19:41:33 +1000 Subject: [PATCH 1/3] Migrate to GetControllerOf from meta/v1 package --- pkg/controller/controller_ref_manager.go | 15 +-------------- pkg/controller/cronjob/BUILD | 2 -- .../cronjob/cronjob_controller_test.go | 3 +-- pkg/controller/cronjob/utils.go | 3 +-- pkg/controller/daemon/daemon_controller.go | 18 +++++++++--------- .../deployment/deployment_controller.go | 14 +++++++------- .../deployment/util/deployment_util.go | 6 +++--- pkg/controller/disruption/disruption.go | 12 ++++++------ pkg/controller/history/BUILD | 1 - pkg/controller/history/controller_history.go | 9 ++++----- .../history/controller_history_test.go | 10 +++++----- pkg/controller/job/job_controller.go | 8 ++++---- pkg/controller/replicaset/replica_set.go | 8 ++++---- .../replication/replication_controller.go | 8 ++++---- pkg/controller/statefulset/stateful_set.go | 10 +++++----- .../statefulset/stateful_set_utils_test.go | 3 +-- pkg/kubectl/BUILD | 1 - pkg/kubectl/cmd/BUILD | 1 - pkg/kubectl/cmd/drain.go | 3 +-- pkg/kubectl/history.go | 3 +-- pkg/printers/internalversion/BUILD | 1 - pkg/printers/internalversion/describe.go | 5 ++--- pkg/printers/internalversion/printers.go | 3 +-- test/e2e/apimachinery/BUILD | 1 - test/e2e/apimachinery/garbage_collector.go | 5 ++--- test/e2e/apps/daemon_set.go | 12 ++++++------ test/e2e/apps/deployment.go | 5 ++--- test/e2e/apps/job.go | 5 ++--- test/e2e/apps/statefulset.go | 9 ++++----- test/e2e/framework/util.go | 2 +- 30 files changed, 77 insertions(+), 109 deletions(-) diff --git a/pkg/controller/controller_ref_manager.go b/pkg/controller/controller_ref_manager.go index 64d6b8c0a6..564a67ca54 100644 --- a/pkg/controller/controller_ref_manager.go +++ b/pkg/controller/controller_ref_manager.go @@ -31,19 +31,6 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" ) -// GetControllerOf returns the controllerRef if controllee has a controller, -// otherwise returns nil. -func GetControllerOf(controllee metav1.Object) *metav1.OwnerReference { - ownerRefs := controllee.GetOwnerReferences() - for i := range ownerRefs { - owner := &ownerRefs[i] - if owner.Controller != nil && *owner.Controller == true { - return owner - } - } - return nil -} - type BaseControllerRefManager struct { Controller metav1.Object Selector labels.Selector @@ -78,7 +65,7 @@ func (m *BaseControllerRefManager) CanAdopt() error { // // No reconciliation will be attempted if the controller is being deleted. func (m *BaseControllerRefManager) ClaimObject(obj metav1.Object, match func(metav1.Object) bool, adopt, release func(metav1.Object) error) (bool, error) { - controllerRef := GetControllerOf(obj) + controllerRef := metav1.GetControllerOf(obj) if controllerRef != nil { if controllerRef.UID != m.Controller.GetUID() { // Owned by someone else. Ignore. diff --git a/pkg/controller/cronjob/BUILD b/pkg/controller/cronjob/BUILD index b0b0d7dd77..06c9d0689b 100644 --- a/pkg/controller/cronjob/BUILD +++ b/pkg/controller/cronjob/BUILD @@ -19,7 +19,6 @@ go_library( tags = ["automanaged"], deps = [ "//pkg/api:go_default_library", - "//pkg/controller:go_default_library", "//pkg/util/metrics:go_default_library", "//vendor/github.com/golang/glog:go_default_library", "//vendor/github.com/robfig/cron:go_default_library", @@ -54,7 +53,6 @@ go_test( deps = [ "//pkg/api/install:go_default_library", "//pkg/apis/batch/install:go_default_library", - "//pkg/controller:go_default_library", "//vendor/k8s.io/api/batch/v1:go_default_library", "//vendor/k8s.io/api/batch/v2alpha1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/controller/cronjob/cronjob_controller_test.go b/pkg/controller/cronjob/cronjob_controller_test.go index 4e3a0feee2..e5f22cfda0 100644 --- a/pkg/controller/cronjob/cronjob_controller_test.go +++ b/pkg/controller/cronjob/cronjob_controller_test.go @@ -32,7 +32,6 @@ import ( // For the cronjob controller to do conversions. _ "k8s.io/kubernetes/pkg/api/install" _ "k8s.io/kubernetes/pkg/apis/batch/install" - "k8s.io/kubernetes/pkg/controller" ) // schedule is hourly on the hour @@ -295,7 +294,7 @@ func TestSyncOne_RunOrNot(t *testing.T) { } for i := range jc.Jobs { job := &jc.Jobs[i] - controllerRef := controller.GetControllerOf(job) + controllerRef := metav1.GetControllerOf(job) if controllerRef == nil { t.Errorf("%s: expected job to have ControllerRef: %#v", name, job) } else { diff --git a/pkg/controller/cronjob/utils.go b/pkg/controller/cronjob/utils.go index ee5d75f457..372d048b8c 100644 --- a/pkg/controller/cronjob/utils.go +++ b/pkg/controller/cronjob/utils.go @@ -32,7 +32,6 @@ import ( "k8s.io/apimachinery/pkg/types" ref "k8s.io/client-go/tools/reference" "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/controller" ) // Utilities for dealing with Jobs and CronJobs and time. @@ -61,7 +60,7 @@ func deleteFromActiveList(sj *batchv2alpha1.CronJob, uid types.UID) { // getParentUIDFromJob extracts UID of job's parent and whether it was found func getParentUIDFromJob(j batchv1.Job) (types.UID, bool) { - controllerRef := controller.GetControllerOf(&j) + controllerRef := metav1.GetControllerOf(&j) if controllerRef == nil { return types.UID(""), false diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go index 17a4136451..0fce881823 100644 --- a/pkg/controller/daemon/daemon_controller.go +++ b/pkg/controller/daemon/daemon_controller.go @@ -320,7 +320,7 @@ func (dsc *DaemonSetsController) addHistory(obj interface{}) { } // If it has a ControllerRef, that's all that matters. - if controllerRef := controller.GetControllerOf(history); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(history); controllerRef != nil { ds := dsc.resolveControllerRef(history.Namespace, controllerRef) if ds == nil { return @@ -352,8 +352,8 @@ func (dsc *DaemonSetsController) updateHistory(old, cur interface{}) { return } - curControllerRef := controller.GetControllerOf(curHistory) - oldControllerRef := controller.GetControllerOf(oldHistory) + curControllerRef := metav1.GetControllerOf(curHistory) + oldControllerRef := metav1.GetControllerOf(oldHistory) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. @@ -411,7 +411,7 @@ func (dsc *DaemonSetsController) deleteHistory(obj interface{}) { } } - controllerRef := controller.GetControllerOf(history) + controllerRef := metav1.GetControllerOf(history) if controllerRef == nil { // No controller should care about orphans being deleted. return @@ -435,7 +435,7 @@ func (dsc *DaemonSetsController) addPod(obj interface{}) { } // If it has a ControllerRef, that's all that matters. - if controllerRef := controller.GetControllerOf(pod); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(pod); controllerRef != nil { ds := dsc.resolveControllerRef(pod.Namespace, controllerRef) if ds == nil { return @@ -478,8 +478,8 @@ func (dsc *DaemonSetsController) updatePod(old, cur interface{}) { changedToReady := !podutil.IsPodReady(oldPod) && podutil.IsPodReady(curPod) labelChanged := !reflect.DeepEqual(curPod.Labels, oldPod.Labels) - curControllerRef := controller.GetControllerOf(curPod) - oldControllerRef := controller.GetControllerOf(oldPod) + curControllerRef := metav1.GetControllerOf(curPod) + oldControllerRef := metav1.GetControllerOf(oldPod) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. @@ -539,7 +539,7 @@ func (dsc *DaemonSetsController) deletePod(obj interface{}) { } } - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { // No controller should care about orphans being deleted. return @@ -1068,7 +1068,7 @@ func (dsc *DaemonSetsController) simulate(newPod *v1.Pod, node *v1.Node, ds *ext } // ignore pods that belong to the daemonset when taking into account whether // a daemonset should bind to a node. - if controllerRef := controller.GetControllerOf(pod); controllerRef != nil && controllerRef.UID == ds.UID { + if controllerRef := metav1.GetControllerOf(pod); controllerRef != nil && controllerRef.UID == ds.UID { continue } pods = append(pods, pod) diff --git a/pkg/controller/deployment/deployment_controller.go b/pkg/controller/deployment/deployment_controller.go index 56eecf5956..da60d174b1 100644 --- a/pkg/controller/deployment/deployment_controller.go +++ b/pkg/controller/deployment/deployment_controller.go @@ -205,7 +205,7 @@ func (dc *DeploymentController) addReplicaSet(obj interface{}) { } // If it has a ControllerRef, that's all that matters. - if controllerRef := controller.GetControllerOf(rs); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(rs); controllerRef != nil { d := dc.resolveControllerRef(rs.Namespace, controllerRef) if d == nil { return @@ -260,8 +260,8 @@ func (dc *DeploymentController) updateReplicaSet(old, cur interface{}) { return } - curControllerRef := controller.GetControllerOf(curRS) - oldControllerRef := controller.GetControllerOf(oldRS) + curControllerRef := metav1.GetControllerOf(curRS) + oldControllerRef := metav1.GetControllerOf(oldRS) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. @@ -319,7 +319,7 @@ func (dc *DeploymentController) deleteReplicaSet(obj interface{}) { } } - controllerRef := controller.GetControllerOf(rs) + controllerRef := metav1.GetControllerOf(rs) if controllerRef == nil { // No controller should care about orphans being deleted. return @@ -409,7 +409,7 @@ func (dc *DeploymentController) getDeploymentForPod(pod *v1.Pod) *extensions.Dep // Find the owning replica set var rs *extensions.ReplicaSet var err error - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { // No controller owns this Pod. return nil @@ -425,7 +425,7 @@ func (dc *DeploymentController) getDeploymentForPod(pod *v1.Pod) *extensions.Dep } // Now find the Deployment that owns that ReplicaSet. - controllerRef = controller.GetControllerOf(rs) + controllerRef = metav1.GetControllerOf(rs) if controllerRef == nil { return nil } @@ -542,7 +542,7 @@ func (dc *DeploymentController) getPodMapForDeployment(d *extensions.Deployment, for _, pod := range pods { // Do not ignore inactive Pods because Recreate Deployments need to verify that no // Pods from older versions are running before spinning up new Pods. - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { continue } diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index f994a8c94b..caa108b820 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -579,7 +579,7 @@ func ListReplicaSets(deployment *extensions.Deployment, getRSList RsListFunc) ([ // Only include those whose ControllerRef matches the Deployment. owned := make([]*extensions.ReplicaSet, 0, len(all)) for _, rs := range all { - controllerRef := controller.GetControllerOf(rs) + controllerRef := metav1.GetControllerOf(rs) if controllerRef != nil && controllerRef.UID == deployment.UID { owned = append(owned, rs) } @@ -603,7 +603,7 @@ func ListReplicaSetsInternal(deployment *internalextensions.Deployment, getRSLis // Only include those whose ControllerRef matches the Deployment. filtered := make([]*internalextensions.ReplicaSet, 0, len(all)) for _, rs := range all { - controllerRef := controller.GetControllerOf(rs) + controllerRef := metav1.GetControllerOf(rs) if controllerRef != nil && controllerRef.UID == deployment.UID { filtered = append(filtered, rs) } @@ -637,7 +637,7 @@ func ListPods(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet owned := &v1.PodList{Items: make([]v1.Pod, 0, len(all.Items))} for i := range all.Items { pod := &all.Items[i] - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef != nil && rsMap[controllerRef.UID] { owned.Items = append(owned.Items, *pod) } diff --git a/pkg/controller/disruption/disruption.go b/pkg/controller/disruption/disruption.go index 3755641865..0f27683d98 100644 --- a/pkg/controller/disruption/disruption.go +++ b/pkg/controller/disruption/disruption.go @@ -184,7 +184,7 @@ var ( // getPodReplicaSets finds replicasets which have no matching deployments. func (dc *DisruptionController) getPodReplicaSets(pod *v1.Pod) ([]controllerAndScale, error) { var casSlice []controllerAndScale - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { return nil, nil } @@ -199,7 +199,7 @@ func (dc *DisruptionController) getPodReplicaSets(pod *v1.Pod) ([]controllerAndS if rs.UID != controllerRef.UID { return nil, nil } - controllerRef = controller.GetControllerOf(rs) + controllerRef = metav1.GetControllerOf(rs) if controllerRef != nil && controllerRef.Kind == controllerKindDep.Kind { // Skip RS if it's controlled by a Deployment. return nil, nil @@ -211,7 +211,7 @@ func (dc *DisruptionController) getPodReplicaSets(pod *v1.Pod) ([]controllerAndS // getPodStatefulSet returns the statefulset managing the given pod. func (dc *DisruptionController) getPodStatefulSets(pod *v1.Pod) ([]controllerAndScale, error) { var casSlice []controllerAndScale - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { return nil, nil } @@ -234,7 +234,7 @@ func (dc *DisruptionController) getPodStatefulSets(pod *v1.Pod) ([]controllerAnd // getPodDeployments finds deployments for any replicasets which are being managed by deployments. func (dc *DisruptionController) getPodDeployments(pod *v1.Pod) ([]controllerAndScale, error) { var casSlice []controllerAndScale - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { return nil, nil } @@ -249,7 +249,7 @@ func (dc *DisruptionController) getPodDeployments(pod *v1.Pod) ([]controllerAndS if rs.UID != controllerRef.UID { return nil, nil } - controllerRef = controller.GetControllerOf(rs) + controllerRef = metav1.GetControllerOf(rs) if controllerRef == nil { return nil, nil } @@ -270,7 +270,7 @@ func (dc *DisruptionController) getPodDeployments(pod *v1.Pod) ([]controllerAndS func (dc *DisruptionController) getPodReplicationControllers(pod *v1.Pod) ([]controllerAndScale, error) { var casSlice []controllerAndScale - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { return nil, nil } diff --git a/pkg/controller/history/BUILD b/pkg/controller/history/BUILD index a451a15c3d..1c820282ef 100644 --- a/pkg/controller/history/BUILD +++ b/pkg/controller/history/BUILD @@ -37,7 +37,6 @@ go_library( tags = ["automanaged"], deps = [ "//pkg/client/retry:go_default_library", - "//pkg/controller:go_default_library", "//pkg/util/hash:go_default_library", "//vendor/k8s.io/api/apps/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", diff --git a/pkg/controller/history/controller_history.go b/pkg/controller/history/controller_history.go index f9d461fc6f..da77be2c21 100644 --- a/pkg/controller/history/controller_history.go +++ b/pkg/controller/history/controller_history.go @@ -27,7 +27,6 @@ import ( appsinformers "k8s.io/client-go/informers/apps/v1beta1" clientset "k8s.io/client-go/kubernetes" appslisters "k8s.io/client-go/listers/apps/v1beta1" - "k8s.io/kubernetes/pkg/controller" hashutil "k8s.io/kubernetes/pkg/util/hash" apiequality "k8s.io/apimachinery/pkg/api/equality" @@ -225,7 +224,7 @@ func (rh *realHistory) ListControllerRevisions(parent metav1.Object, selector la } var owned []*apps.ControllerRevision for i := range history { - ref := controller.GetControllerOf(history[i]) + ref := metav1.GetControllerOf(history[i]) if ref == nil || ref.UID == parent.GetUID() { owned = append(owned, history[i]) } @@ -302,7 +301,7 @@ func (rh *realHistory) DeleteControllerRevision(revision *apps.ControllerRevisio func (rh *realHistory) AdoptControllerRevision(parent metav1.Object, parentKind schema.GroupVersionKind, revision *apps.ControllerRevision) (*apps.ControllerRevision, error) { // Return an error if the parent does not own the revision - if owner := controller.GetControllerOf(revision); owner != nil { + if owner := metav1.GetControllerOf(revision); owner != nil { return nil, fmt.Errorf("attempt to adopt revision owned by %v", owner) } // Use strategic merge patch to add an owner reference indicating a controller ref @@ -346,7 +345,7 @@ func (fh *fakeHistory) ListControllerRevisions(parent metav1.Object, selector la var owned []*apps.ControllerRevision for i := range history { - ref := controller.GetControllerOf(history[i]) + ref := metav1.GetControllerOf(history[i]) if ref == nil || ref.UID == parent.GetUID() { owned = append(owned, history[i]) } @@ -431,7 +430,7 @@ func (fh *fakeHistory) UpdateControllerRevision(revision *apps.ControllerRevisio func (fh *fakeHistory) AdoptControllerRevision(parent metav1.Object, parentKind schema.GroupVersionKind, revision *apps.ControllerRevision) (*apps.ControllerRevision, error) { blockOwnerDeletion := true isController := true - if owner := controller.GetControllerOf(revision); owner != nil { + if owner := metav1.GetControllerOf(revision); owner != nil { return nil, fmt.Errorf("attempt to adopt revision owned by %v", owner) } key, err := cache.DeletionHandlingMetaNamespaceKeyFunc(revision) diff --git a/pkg/controller/history/controller_history_test.go b/pkg/controller/history/controller_history_test.go index ce78a3db86..296fb761dc 100644 --- a/pkg/controller/history/controller_history_test.go +++ b/pkg/controller/history/controller_history_test.go @@ -992,7 +992,7 @@ func TestRealHistory_AdoptControllerRevision(t *testing.T) { if !test.err && err != nil { t.Errorf("%s: %s", test.name, err) } - if !test.err && controller.GetControllerOf(adopted).UID != test.parent.GetUID() { + if !test.err && metav1.GetControllerOf(adopted).UID != test.parent.GetUID() { t.Errorf("%s: adoption failed", test.name) } if test.err && err == nil { @@ -1103,7 +1103,7 @@ func TestFakeHistory_AdoptControllerRevision(t *testing.T) { if !test.err && err != nil { t.Errorf("%s: %s", test.name, err) } - if !test.err && controller.GetControllerOf(adopted).UID != test.parent.GetUID() { + if !test.err && metav1.GetControllerOf(adopted).UID != test.parent.GetUID() { t.Errorf("%s: adoption failed", test.name) } if test.err && err == nil { @@ -1211,7 +1211,7 @@ func TestRealHistory_ReleaseControllerRevision(t *testing.T) { if found == nil { return true, nil, errors.NewNotFound(apps.Resource("controllerrevisions"), test.revision.Name) } - if foundParent := controller.GetControllerOf(test.revision); foundParent == nil || + if foundParent := metav1.GetControllerOf(test.revision); foundParent == nil || foundParent.UID != test.parent.GetUID() { return true, nil, errors.NewInvalid( test.revision.GroupVersionKind().GroupKind(), test.revision.Name, nil) @@ -1258,7 +1258,7 @@ func TestRealHistory_ReleaseControllerRevision(t *testing.T) { if adopted == nil { return } - if owner := controller.GetControllerOf(adopted); owner != nil && owner.UID == test.parent.GetUID() { + if owner := metav1.GetControllerOf(adopted); owner != nil && owner.UID == test.parent.GetUID() { t.Errorf("%s: release failed", test.name) } } @@ -1386,7 +1386,7 @@ func TestFakeHistory_ReleaseControllerRevision(t *testing.T) { if adopted == nil { return } - if owner := controller.GetControllerOf(adopted); owner != nil && owner.UID == test.parent.GetUID() { + if owner := metav1.GetControllerOf(adopted); owner != nil && owner.UID == test.parent.GetUID() { t.Errorf("%s: release failed", test.name) } } diff --git a/pkg/controller/job/job_controller.go b/pkg/controller/job/job_controller.go index 50e4f7ad15..45594b6bcc 100644 --- a/pkg/controller/job/job_controller.go +++ b/pkg/controller/job/job_controller.go @@ -193,7 +193,7 @@ func (jm *JobController) addPod(obj interface{}) { } // If it has a ControllerRef, that's all that matters. - if controllerRef := controller.GetControllerOf(pod); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(pod); controllerRef != nil { job := jm.resolveControllerRef(pod.Namespace, controllerRef) if job == nil { return @@ -238,8 +238,8 @@ func (jm *JobController) updatePod(old, cur interface{}) { labelChanged := !reflect.DeepEqual(curPod.Labels, oldPod.Labels) - curControllerRef := controller.GetControllerOf(curPod) - oldControllerRef := controller.GetControllerOf(oldPod) + curControllerRef := metav1.GetControllerOf(curPod) + oldControllerRef := metav1.GetControllerOf(oldPod) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. @@ -289,7 +289,7 @@ func (jm *JobController) deletePod(obj interface{}) { } } - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { // No controller should care about orphans being deleted. return diff --git a/pkg/controller/replicaset/replica_set.go b/pkg/controller/replicaset/replica_set.go index 97b27b7040..27d694b29f 100644 --- a/pkg/controller/replicaset/replica_set.go +++ b/pkg/controller/replicaset/replica_set.go @@ -236,7 +236,7 @@ func (rsc *ReplicaSetController) addPod(obj interface{}) { } // If it has a ControllerRef, that's all that matters. - if controllerRef := controller.GetControllerOf(pod); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(pod); controllerRef != nil { rs := rsc.resolveControllerRef(pod.Namespace, controllerRef) if rs == nil { return @@ -292,8 +292,8 @@ func (rsc *ReplicaSetController) updatePod(old, cur interface{}) { return } - curControllerRef := controller.GetControllerOf(curPod) - oldControllerRef := controller.GetControllerOf(oldPod) + curControllerRef := metav1.GetControllerOf(curPod) + oldControllerRef := metav1.GetControllerOf(oldPod) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. @@ -362,7 +362,7 @@ func (rsc *ReplicaSetController) deletePod(obj interface{}) { } } - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { // No controller should care about orphans being deleted. return diff --git a/pkg/controller/replication/replication_controller.go b/pkg/controller/replication/replication_controller.go index c50fc790b0..5f066382fb 100644 --- a/pkg/controller/replication/replication_controller.go +++ b/pkg/controller/replication/replication_controller.go @@ -231,7 +231,7 @@ func (rm *ReplicationManager) addPod(obj interface{}) { } // If it has a ControllerRef, that's all that matters. - if controllerRef := controller.GetControllerOf(pod); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(pod); controllerRef != nil { rc := rm.resolveControllerRef(pod.Namespace, controllerRef) if rc == nil { return @@ -287,8 +287,8 @@ func (rm *ReplicationManager) updatePod(old, cur interface{}) { return } - curControllerRef := controller.GetControllerOf(curPod) - oldControllerRef := controller.GetControllerOf(oldPod) + curControllerRef := metav1.GetControllerOf(curPod) + oldControllerRef := metav1.GetControllerOf(oldPod) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. @@ -357,7 +357,7 @@ func (rm *ReplicationManager) deletePod(obj interface{}) { } } - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { // No controller should care about orphans being deleted. return diff --git a/pkg/controller/statefulset/stateful_set.go b/pkg/controller/statefulset/stateful_set.go index f9838ff931..b34cbc6e67 100644 --- a/pkg/controller/statefulset/stateful_set.go +++ b/pkg/controller/statefulset/stateful_set.go @@ -170,7 +170,7 @@ func (ssc *StatefulSetController) addPod(obj interface{}) { } // If it has a ControllerRef, that's all that matters. - if controllerRef := controller.GetControllerOf(pod); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(pod); controllerRef != nil { set := ssc.resolveControllerRef(pod.Namespace, controllerRef) if set == nil { return @@ -204,8 +204,8 @@ func (ssc *StatefulSetController) updatePod(old, cur interface{}) { labelChanged := !reflect.DeepEqual(curPod.Labels, oldPod.Labels) - curControllerRef := controller.GetControllerOf(curPod) - oldControllerRef := controller.GetControllerOf(oldPod) + curControllerRef := metav1.GetControllerOf(curPod) + oldControllerRef := metav1.GetControllerOf(oldPod) controllerRefChanged := !reflect.DeepEqual(curControllerRef, oldControllerRef) if controllerRefChanged && oldControllerRef != nil { // The ControllerRef was changed. Sync the old controller, if any. @@ -260,7 +260,7 @@ func (ssc *StatefulSetController) deletePod(obj interface{}) { } } - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { // No controller should care about orphans being deleted. return @@ -316,7 +316,7 @@ func (ssc *StatefulSetController) adoptOrphanRevisions(set *apps.StatefulSet) er } hasOrphans := false for i := range revisions { - if controller.GetControllerOf(revisions[i]) == nil { + if metav1.GetControllerOf(revisions[i]) == nil { hasOrphans = true break } diff --git a/pkg/controller/statefulset/stateful_set_utils_test.go b/pkg/controller/statefulset/stateful_set_utils_test.go index 58dfe05ecf..92dd4c5a54 100644 --- a/pkg/controller/statefulset/stateful_set_utils_test.go +++ b/pkg/controller/statefulset/stateful_set_utils_test.go @@ -31,7 +31,6 @@ import ( apps "k8s.io/api/apps/v1beta1" "k8s.io/api/core/v1" podutil "k8s.io/kubernetes/pkg/api/v1/pod" - "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/history" ) @@ -252,7 +251,7 @@ func TestOverlappingStatefulSets(t *testing.T) { func TestNewPodControllerRef(t *testing.T) { set := newStatefulSet(1) pod := newStatefulSetPod(set, 0) - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { t.Fatalf("No ControllerRef found on new pod") } diff --git a/pkg/kubectl/BUILD b/pkg/kubectl/BUILD index 52a40a1c94..f807020268 100644 --- a/pkg/kubectl/BUILD +++ b/pkg/kubectl/BUILD @@ -135,7 +135,6 @@ 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/daemon:go_default_library", "//pkg/controller/deployment/util:go_default_library", "//pkg/credentialprovider:go_default_library", diff --git a/pkg/kubectl/cmd/BUILD b/pkg/kubectl/cmd/BUILD index 2f3d219652..8f9e881e2e 100644 --- a/pkg/kubectl/cmd/BUILD +++ b/pkg/kubectl/cmd/BUILD @@ -79,7 +79,6 @@ go_library( "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library", "//pkg/client/clientset_generated/internalclientset/typed/rbac/internalversion:go_default_library", "//pkg/client/unversioned:go_default_library", - "//pkg/controller:go_default_library", "//pkg/kubectl:go_default_library", "//pkg/kubectl/cmd/auth:go_default_library", "//pkg/kubectl/cmd/config:go_default_library", diff --git a/pkg/kubectl/cmd/drain.go b/pkg/kubectl/cmd/drain.go index 8f6d511bc0..1f9d48c1ef 100644 --- a/pkg/kubectl/cmd/drain.go +++ b/pkg/kubectl/cmd/drain.go @@ -42,7 +42,6 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/policy" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" - "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/pkg/kubectl/cmd/templates" cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" @@ -286,7 +285,7 @@ func (o *DrainOptions) getController(namespace string, controllerRef *metav1.Own } func (o *DrainOptions) getPodController(pod api.Pod) (*metav1.OwnerReference, error) { - controllerRef := controller.GetControllerOf(&pod) + controllerRef := metav1.GetControllerOf(&pod) if controllerRef == nil { return nil, nil } diff --git a/pkg/kubectl/history.go b/pkg/kubectl/history.go index b1feece151..4e3edb83ec 100644 --- a/pkg/kubectl/history.go +++ b/pkg/kubectl/history.go @@ -37,7 +37,6 @@ import ( "k8s.io/kubernetes/pkg/apis/apps" "k8s.io/kubernetes/pkg/apis/extensions" clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" - "k8s.io/kubernetes/pkg/controller" deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util" sliceutil "k8s.io/kubernetes/pkg/kubectl/util/slice" printersinternal "k8s.io/kubernetes/pkg/printers/internalversion" @@ -273,7 +272,7 @@ func controlledHistories(c externalclientset.Interface, namespace, name string) for i := range historyList.Items { history := historyList.Items[i] // Skip history that doesn't belong to the DaemonSet - if controllerRef := controller.GetControllerOf(&history); controllerRef == nil || controllerRef.UID != ds.UID { + if controllerRef := metav1.GetControllerOf(&history); controllerRef == nil || controllerRef.UID != ds.UID { continue } result = append(result, &history) diff --git a/pkg/printers/internalversion/BUILD b/pkg/printers/internalversion/BUILD index 965d2a4a23..7c093aecf3 100644 --- a/pkg/printers/internalversion/BUILD +++ b/pkg/printers/internalversion/BUILD @@ -81,7 +81,6 @@ go_library( "//pkg/apis/storage/util:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library", - "//pkg/controller:go_default_library", "//pkg/controller/deployment/util:go_default_library", "//pkg/fieldpath:go_default_library", "//pkg/printers:go_default_library", diff --git a/pkg/printers/internalversion/describe.go b/pkg/printers/internalversion/describe.go index d8b3ae409a..6d6e2c6bc0 100644 --- a/pkg/printers/internalversion/describe.go +++ b/pkg/printers/internalversion/describe.go @@ -70,7 +70,6 @@ import ( storageutil "k8s.io/kubernetes/pkg/apis/storage/util" clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" - "k8s.io/kubernetes/pkg/controller" deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util" "k8s.io/kubernetes/pkg/fieldpath" "k8s.io/kubernetes/pkg/printers" @@ -675,7 +674,7 @@ func describePod(pod *api.Pod, events *api.EventList) (string, error) { } func printController(controllee metav1.Object) string { - if controllerRef := controller.GetControllerOf(controllee); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(controllee); controllerRef != nil { return fmt.Sprintf("%s/%s", controllerRef.Kind, controllerRef.Name) } return "" @@ -2923,7 +2922,7 @@ func getPodStatusForController(c coreclient.PodInterface, selector labels.Select return } for _, pod := range rcPods.Items { - controllerRef := controller.GetControllerOf(&pod) + controllerRef := metav1.GetControllerOf(&pod) // Skip pods that are orphans or owned by other controllers. if controllerRef == nil || controllerRef.UID != uid { continue diff --git a/pkg/printers/internalversion/printers.go b/pkg/printers/internalversion/printers.go index 596a8f8340..32fbcce3be 100644 --- a/pkg/printers/internalversion/printers.go +++ b/pkg/printers/internalversion/printers.go @@ -55,7 +55,6 @@ import ( "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/apis/storage" storageutil "k8s.io/kubernetes/pkg/apis/storage/util" - "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/printers" "k8s.io/kubernetes/pkg/util/node" ) @@ -1770,7 +1769,7 @@ func printControllerRevision(obj *apps.ControllerRevision, options printers.Prin Object: runtime.RawExtension{Object: obj}, } - controllerRef := controller.GetControllerOf(obj) + controllerRef := metav1.GetControllerOf(obj) controllerName := "" if controllerRef != nil { withKind := true diff --git a/test/e2e/apimachinery/BUILD b/test/e2e/apimachinery/BUILD index 31ef898ed1..2226d6f06b 100644 --- a/test/e2e/apimachinery/BUILD +++ b/test/e2e/apimachinery/BUILD @@ -23,7 +23,6 @@ go_library( "//pkg/api:go_default_library", "//pkg/api/v1/pod:go_default_library", "//pkg/client/retry:go_default_library", - "//pkg/controller:go_default_library", "//pkg/printers:go_default_library", "//pkg/util/version:go_default_library", "//test/e2e/apps:go_default_library", diff --git a/test/e2e/apimachinery/garbage_collector.go b/test/e2e/apimachinery/garbage_collector.go index d0d38b560b..a797784d85 100644 --- a/test/e2e/apimachinery/garbage_collector.go +++ b/test/e2e/apimachinery/garbage_collector.go @@ -21,7 +21,7 @@ import ( "time" "k8s.io/api/core/v1" - v1beta1 "k8s.io/api/extensions/v1beta1" + "k8s.io/api/extensions/v1beta1" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" apiextensionstestserver "k8s.io/apiextensions-apiserver/test/integration/testserver" @@ -33,7 +33,6 @@ import ( "k8s.io/apiserver/pkg/storage/names" clientset "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/test/e2e/framework" "k8s.io/kubernetes/test/e2e/metrics" @@ -498,7 +497,7 @@ var _ = SIGDescribe("Garbage collector", func() { framework.Failf("Failed to list ReplicaSet %v", err) } for _, replicaSet := range rs.Items { - if controller.GetControllerOf(&replicaSet.ObjectMeta) != nil { + if metav1.GetControllerOf(&replicaSet.ObjectMeta) != nil { framework.Failf("Found ReplicaSet with non nil ownerRef %v", replicaSet) } } diff --git a/test/e2e/apps/daemon_set.go b/test/e2e/apps/daemon_set.go index f2825ecf89..86422035b8 100644 --- a/test/e2e/apps/daemon_set.go +++ b/test/e2e/apps/daemon_set.go @@ -630,7 +630,7 @@ func checkDaemonPodOnNodes(f *framework.Framework, ds *extensions.DaemonSet, nod nodesToPodCount := make(map[string]int) for _, pod := range pods { - if controllerRef := controller.GetControllerOf(&pod); controllerRef == nil || controllerRef.UID != ds.UID { + if controllerRef := metav1.GetControllerOf(&pod); controllerRef == nil || controllerRef.UID != ds.UID { continue } if pod.DeletionTimestamp != nil { @@ -726,7 +726,7 @@ func checkDaemonPodsImageAndAvailability(c clientset.Interface, ds *extensions.D unavailablePods := 0 allImagesUpdated := true for _, pod := range pods { - if controllerRef := controller.GetControllerOf(&pod); controllerRef == nil || controllerRef.UID != ds.UID { + if controllerRef := metav1.GetControllerOf(&pod); controllerRef == nil || controllerRef.UID != ds.UID { continue } podImage := pod.Spec.Containers[0].Image @@ -779,7 +779,7 @@ func checkDaemonSetPodsOrphaned(c clientset.Interface, ns string, label map[stri pods := listDaemonPods(c, ns, label) for _, pod := range pods.Items { // This pod is orphaned only when controller ref is cleared - if controllerRef := controller.GetControllerOf(&pod); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(&pod); controllerRef != nil { return false, nil } } @@ -792,7 +792,7 @@ func checkDaemonSetHistoryOrphaned(c clientset.Interface, ns string, label map[s histories := listDaemonHistories(c, ns, label) for _, history := range histories.Items { // This history is orphaned only when controller ref is cleared - if controllerRef := controller.GetControllerOf(&history); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(&history); controllerRef != nil { return false, nil } } @@ -805,7 +805,7 @@ func checkDaemonSetPodsAdopted(c clientset.Interface, ns string, dsUID types.UID pods := listDaemonPods(c, ns, label) for _, pod := range pods.Items { // This pod is adopted only when its controller ref is update - if controllerRef := controller.GetControllerOf(&pod); controllerRef == nil || controllerRef.UID != dsUID { + if controllerRef := metav1.GetControllerOf(&pod); controllerRef == nil || controllerRef.UID != dsUID { return false, nil } } @@ -818,7 +818,7 @@ func checkDaemonSetHistoryAdopted(c clientset.Interface, ns string, dsUID types. histories := listDaemonHistories(c, ns, label) for _, history := range histories.Items { // This history is adopted only when its controller ref is update - if controllerRef := controller.GetControllerOf(&history); controllerRef == nil || controllerRef.UID != dsUID { + if controllerRef := metav1.GetControllerOf(&history); controllerRef == nil || controllerRef.UID != dsUID { return false, nil } } diff --git a/test/e2e/apps/deployment.go b/test/e2e/apps/deployment.go index 6434fa0160..2dcd4fbe7d 100644 --- a/test/e2e/apps/deployment.go +++ b/test/e2e/apps/deployment.go @@ -38,7 +38,6 @@ import ( extensionsclient "k8s.io/client-go/kubernetes/typed/extensions/v1beta1" extensionsinternal "k8s.io/kubernetes/pkg/apis/extensions" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" - "k8s.io/kubernetes/pkg/controller" deploymentutil "k8s.io/kubernetes/pkg/controller/deployment/util" "k8s.io/kubernetes/pkg/kubectl" utilpointer "k8s.io/kubernetes/pkg/util/pointer" @@ -1380,7 +1379,7 @@ func checkDeploymentReplicaSetsControllerRef(c clientset.Interface, ns string, u rsList := listDeploymentReplicaSets(c, ns, label) for _, rs := range rsList.Items { // This rs is adopted only when its controller ref is update - if controllerRef := controller.GetControllerOf(&rs); controllerRef == nil || controllerRef.UID != uid { + if controllerRef := metav1.GetControllerOf(&rs); controllerRef == nil || controllerRef.UID != uid { return fmt.Errorf("ReplicaSet %s has unexpected controllerRef %v", rs.Name, controllerRef) } } @@ -1392,7 +1391,7 @@ func waitDeploymentReplicaSetsOrphaned(c clientset.Interface, ns string, label m rsList := listDeploymentReplicaSets(c, ns, label) for _, rs := range rsList.Items { // This rs is orphaned only when controller ref is cleared - if controllerRef := controller.GetControllerOf(&rs); controllerRef != nil { + if controllerRef := metav1.GetControllerOf(&rs); controllerRef != nil { return false, nil } } diff --git a/test/e2e/apps/job.go b/test/e2e/apps/job.go index a60b0f34b2..7180f9a7ad 100644 --- a/test/e2e/apps/job.go +++ b/test/e2e/apps/job.go @@ -24,7 +24,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" batchinternal "k8s.io/kubernetes/pkg/apis/batch" - "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/kubectl" "k8s.io/kubernetes/test/e2e/framework" @@ -135,7 +134,7 @@ var _ = SIGDescribe("Job", func() { By("Checking that the Job readopts the Pod") Expect(framework.WaitForPodCondition(f.ClientSet, pod.Namespace, pod.Name, "adopted", framework.JobTimeout, func(pod *v1.Pod) (bool, error) { - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { return false, nil } @@ -154,7 +153,7 @@ var _ = SIGDescribe("Job", func() { By("Checking that the Job releases the Pod") Expect(framework.WaitForPodCondition(f.ClientSet, pod.Namespace, pod.Name, "released", framework.JobTimeout, func(pod *v1.Pod) (bool, error) { - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef != nil { return false, nil } diff --git a/test/e2e/apps/statefulset.go b/test/e2e/apps/statefulset.go index bced008121..766f484249 100644 --- a/test/e2e/apps/statefulset.go +++ b/test/e2e/apps/statefulset.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" clientset "k8s.io/client-go/kubernetes" - "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/test/e2e/framework" ) @@ -145,7 +144,7 @@ var _ = SIGDescribe("StatefulSet", func() { By("Checking that stateful set pods are created with ControllerRef") pod := pods.Items[0] - controllerRef := controller.GetControllerOf(&pod) + controllerRef := metav1.GetControllerOf(&pod) Expect(controllerRef).ToNot(BeNil()) Expect(controllerRef.Kind).To(Equal(ss.Kind)) Expect(controllerRef.Name).To(Equal(ss.Name)) @@ -159,7 +158,7 @@ var _ = SIGDescribe("StatefulSet", func() { By("Checking that the stateful set readopts the pod") Expect(framework.WaitForPodCondition(c, pod.Namespace, pod.Name, "adopted", framework.StatefulSetTimeout, func(pod *v1.Pod) (bool, error) { - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { return false, nil } @@ -179,7 +178,7 @@ var _ = SIGDescribe("StatefulSet", func() { By("Checking that the stateful set releases the pod") Expect(framework.WaitForPodCondition(c, pod.Namespace, pod.Name, "released", framework.StatefulSetTimeout, func(pod *v1.Pod) (bool, error) { - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef != nil { return false, nil } @@ -196,7 +195,7 @@ var _ = SIGDescribe("StatefulSet", func() { By("Checking that the stateful set readopts the pod") Expect(framework.WaitForPodCondition(c, pod.Namespace, pod.Name, "adopted", framework.StatefulSetTimeout, func(pod *v1.Pod) (bool, error) { - controllerRef := controller.GetControllerOf(pod) + controllerRef := metav1.GetControllerOf(pod) if controllerRef == nil { return false, nil } diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 1a9130414b..d622a75167 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -653,7 +653,7 @@ func WaitForPodsRunningReady(c clientset.Interface, ns string, minPods, allowedN notReady++ badPods = append(badPods, pod) default: - if controller.GetControllerOf(&pod) == nil { + if metav1.GetControllerOf(&pod) == nil { Logf("Pod %s is Failed, but it's not controlled by a controller", pod.ObjectMeta.Name) badPods = append(badPods, pod) } From 042b5642b9926267e470fb0a2c0dd2df19eb6c6a Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Wed, 2 Aug 2017 20:05:37 +1000 Subject: [PATCH 2/3] Migrate to NewControllerRef from meta/v1 package --- pkg/controller/controller_ref_manager_test.go | 16 +--------------- pkg/controller/cronjob/utils.go | 15 +-------------- pkg/controller/daemon/daemon_controller.go | 16 +--------------- pkg/controller/daemon/daemon_controller_test.go | 4 ++-- pkg/controller/daemon/update.go | 2 +- .../deployment/deployment_controller_test.go | 4 ++-- pkg/controller/deployment/sync.go | 16 +--------------- pkg/controller/job/job_controller.go | 2 +- pkg/controller/job/job_controller_test.go | 6 +++--- pkg/controller/job/utils.go | 14 -------------- pkg/controller/statefulset/stateful_set_utils.go | 16 +--------------- 11 files changed, 14 insertions(+), 97 deletions(-) diff --git a/pkg/controller/controller_ref_manager_test.go b/pkg/controller/controller_ref_manager_test.go index 368385f771..1af203af8d 100644 --- a/pkg/controller/controller_ref_manager_test.go +++ b/pkg/controller/controller_ref_manager_test.go @@ -36,20 +36,6 @@ var ( controllerUID = "123" ) -func newControllerRef(controller metav1.Object) *metav1.OwnerReference { - var controllerKind = v1beta1.SchemeGroupVersion.WithKind("Fake") - blockOwnerDeletion := true - isController := true - return &metav1.OwnerReference{ - APIVersion: controllerKind.GroupVersion().String(), - Kind: controllerKind.Kind, - Name: "Fake", - UID: controller.GetUID(), - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - } -} - func newPod(podName string, label map[string]string, owner metav1.Object) *v1.Pod { pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -66,7 +52,7 @@ func newPod(podName string, label map[string]string, owner metav1.Object) *v1.Po }, } if owner != nil { - pod.OwnerReferences = []metav1.OwnerReference{*newControllerRef(owner)} + pod.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(owner, v1beta1.SchemeGroupVersion.WithKind("Fake"))} } return pod } diff --git a/pkg/controller/cronjob/utils.go b/pkg/controller/cronjob/utils.go index 372d048b8c..dbdd8dd96c 100644 --- a/pkg/controller/cronjob/utils.go +++ b/pkg/controller/cronjob/utils.go @@ -169,19 +169,6 @@ func getRecentUnmetScheduleTimes(sj batchv2alpha1.CronJob, now time.Time) ([]tim return starts, nil } -func newControllerRef(sj *batchv2alpha1.CronJob) *metav1.OwnerReference { - blockOwnerDeletion := true - isController := true - return &metav1.OwnerReference{ - APIVersion: controllerKind.GroupVersion().String(), - Kind: controllerKind.Kind, - Name: sj.Name, - UID: sj.UID, - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - } -} - // XXX unit test this // getJobFromTemplate makes a Job from a CronJob @@ -204,7 +191,7 @@ func getJobFromTemplate(sj *batchv2alpha1.CronJob, scheduledTime time.Time) (*ba Labels: labels, Annotations: annotations, Name: name, - OwnerReferences: []metav1.OwnerReference{*newControllerRef(sj)}, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(sj, controllerKind)}, }, } if err := api.Scheme.Convert(&sj.Spec.JobTemplate.Spec, &job.Spec, nil); err != nil { diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go index 0fce881823..d98087d3e1 100644 --- a/pkg/controller/daemon/daemon_controller.go +++ b/pkg/controller/daemon/daemon_controller.go @@ -816,7 +816,7 @@ func (dsc *DaemonSetsController) syncNodes(ds *extensions.DaemonSet, podsToDelet for i := 0; i < createDiff; i++ { go func(ix int) { defer createWait.Done() - err := dsc.podControl.CreatePodsOnNode(nodesNeedingDaemonPods[ix], ds.Namespace, &template, ds, newControllerRef(ds)) + err := dsc.podControl.CreatePodsOnNode(nodesNeedingDaemonPods[ix], ds.Namespace, &template, ds, metav1.NewControllerRef(ds, controllerKind)) if err != nil && errors.IsTimeout(err) { // Pod is created but its initialization has timed out. // If the initialization is successful eventually, the @@ -1237,20 +1237,6 @@ func NodeConditionPredicates(nodeInfo *schedulercache.NodeInfo) (bool, []algorit return len(reasons) == 0, reasons } -// newControllerRef creates a ControllerRef pointing to the given DaemonSet. -func newControllerRef(ds *extensions.DaemonSet) *metav1.OwnerReference { - blockOwnerDeletion := true - isController := true - return &metav1.OwnerReference{ - APIVersion: controllerKind.GroupVersion().String(), - Kind: controllerKind.Kind, - Name: ds.Name, - UID: ds.UID, - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - } -} - // byCreationTimestamp sorts a list by creation timestamp, using their names as a tie breaker. type byCreationTimestamp []*extensions.DaemonSet diff --git a/pkg/controller/daemon/daemon_controller_test.go b/pkg/controller/daemon/daemon_controller_test.go index 921cd6fc3d..13ff88db88 100644 --- a/pkg/controller/daemon/daemon_controller_test.go +++ b/pkg/controller/daemon/daemon_controller_test.go @@ -201,7 +201,7 @@ func newPod(podName string, nodeName string, label map[string]string, ds *extens } pod.Name = names.SimpleNameGenerator.GenerateName(podName) if ds != nil { - pod.OwnerReferences = []metav1.OwnerReference{*newControllerRef(ds)} + pod.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(ds, controllerKind)} } return pod } @@ -1808,7 +1808,7 @@ func TestUpdatePodChangeControllerRef(t *testing.T) { pod := newPod("pod1-", "node-0", simpleDaemonSetLabel, ds1) prev := *pod - prev.OwnerReferences = []metav1.OwnerReference{*newControllerRef(ds2)} + prev.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(ds2, controllerKind)} bumpResourceVersion(pod) manager.updatePod(&prev, pod) if got, want := manager.queue.Len(), 2; got != want { diff --git a/pkg/controller/daemon/update.go b/pkg/controller/daemon/update.go index 6c765d0da6..72486594f0 100644 --- a/pkg/controller/daemon/update.go +++ b/pkg/controller/daemon/update.go @@ -339,7 +339,7 @@ func (dsc *DaemonSetsController) snapshot(ds *extensions.DaemonSet, revision int Namespace: ds.Namespace, Labels: labelsutil.CloneAndAddLabel(ds.Spec.Template.Labels, extensions.DefaultDaemonSetUniqueLabelKey, hash), Annotations: ds.Annotations, - OwnerReferences: []metav1.OwnerReference{*newControllerRef(ds)}, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(ds, controllerKind)}, }, Data: runtime.RawExtension{Raw: patch}, Revision: revision, diff --git a/pkg/controller/deployment/deployment_controller_test.go b/pkg/controller/deployment/deployment_controller_test.go index 9298e53e6d..6fbc0de699 100644 --- a/pkg/controller/deployment/deployment_controller_test.go +++ b/pkg/controller/deployment/deployment_controller_test.go @@ -126,7 +126,7 @@ func newReplicaSet(d *extensions.Deployment, name string, replicas int) *extensi UID: uuid.NewUUID(), Namespace: metav1.NamespaceDefault, Labels: d.Spec.Selector.MatchLabels, - OwnerReferences: []metav1.OwnerReference{*newControllerRef(d)}, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(d, controllerKind)}, }, Spec: extensions.ReplicaSetSpec{ Selector: d.Spec.Selector, @@ -810,7 +810,7 @@ func TestUpdateReplicaSetChangeControllerRef(t *testing.T) { // Change ControllerRef and expect both old and new to queue. prev := *rs - prev.OwnerReferences = []metav1.OwnerReference{*newControllerRef(d2)} + prev.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(d2, controllerKind)} next := *rs bumpResourceVersion(&next) dc.updateReplicaSet(&prev, &next) diff --git a/pkg/controller/deployment/sync.go b/pkg/controller/deployment/sync.go index cb0e993ec6..a1a85a5a8f 100644 --- a/pkg/controller/deployment/sync.go +++ b/pkg/controller/deployment/sync.go @@ -306,7 +306,7 @@ func (dc *DeploymentController) getNewReplicaSet(d *extensions.Deployment, rsLis // Make the name deterministic, to ensure idempotence Name: d.Name + "-" + podTemplateSpecHash, Namespace: d.Namespace, - OwnerReferences: []metav1.OwnerReference{*newControllerRef(d)}, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(d, controllerKind)}, }, Spec: extensions.ReplicaSetSpec{ Replicas: new(int32), @@ -651,17 +651,3 @@ func (dc *DeploymentController) isScalingEvent(d *extensions.Deployment, rsList } return false, nil } - -// newControllerRef returns a ControllerRef pointing to the deployment. -func newControllerRef(d *extensions.Deployment) *metav1.OwnerReference { - blockOwnerDeletion := true - isController := true - return &metav1.OwnerReference{ - APIVersion: controllerKind.GroupVersion().String(), - Kind: controllerKind.Kind, - Name: d.Name, - UID: d.UID, - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - } -} diff --git a/pkg/controller/job/job_controller.go b/pkg/controller/job/job_controller.go index 45594b6bcc..6ed99e36d6 100644 --- a/pkg/controller/job/job_controller.go +++ b/pkg/controller/job/job_controller.go @@ -624,7 +624,7 @@ func (jm *JobController) manageJob(activePods []*v1.Pod, succeeded int32, job *b for i := int32(0); i < diff; i++ { go func() { defer wait.Done() - err := jm.podControl.CreatePodsWithControllerRef(job.Namespace, &job.Spec.Template, job, newControllerRef(job)) + err := jm.podControl.CreatePodsWithControllerRef(job.Namespace, &job.Spec.Template, job, metav1.NewControllerRef(job, controllerKind)) if err != nil && errors.IsTimeout(err) { // Pod is created but its initialization has timed out. // If the initialization is successful eventually, the diff --git a/pkg/controller/job/job_controller_test.go b/pkg/controller/job/job_controller_test.go index 9559ff502e..51e909a936 100644 --- a/pkg/controller/job/job_controller_test.go +++ b/pkg/controller/job/job_controller_test.go @@ -109,7 +109,7 @@ func newPodList(count int32, status v1.PodPhase, job *batch.Job) []v1.Pod { Name: fmt.Sprintf("pod-%v", rand.String(10)), Labels: job.Spec.Selector.MatchLabels, Namespace: job.Namespace, - OwnerReferences: []metav1.OwnerReference{*newControllerRef(job)}, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(job, controllerKind)}, }, Status: v1.PodStatus{Phase: status}, } @@ -634,7 +634,7 @@ func newPod(name string, job *batch.Job) *v1.Pod { Name: name, Labels: job.Spec.Selector.MatchLabels, Namespace: job.Namespace, - OwnerReferences: []metav1.OwnerReference{*newControllerRef(job)}, + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(job, controllerKind)}, }, } } @@ -971,7 +971,7 @@ func TestUpdatePodChangeControllerRef(t *testing.T) { // Changed ControllerRef. Expect both old and new to queue. prev := *pod1 - prev.OwnerReferences = []metav1.OwnerReference{*newControllerRef(job2)} + prev.OwnerReferences = []metav1.OwnerReference{*metav1.NewControllerRef(job2, controllerKind)} bumpResourceVersion(pod1) jm.updatePod(&prev, pod1) if got, want := jm.queue.Len(), 2; got != want { diff --git a/pkg/controller/job/utils.go b/pkg/controller/job/utils.go index 438a5e0104..25c3d08c90 100644 --- a/pkg/controller/job/utils.go +++ b/pkg/controller/job/utils.go @@ -19,7 +19,6 @@ package job import ( batch "k8s.io/api/batch/v1" "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func IsJobFinished(j *batch.Job) bool { @@ -30,16 +29,3 @@ func IsJobFinished(j *batch.Job) bool { } return false } - -func newControllerRef(j *batch.Job) *metav1.OwnerReference { - blockOwnerDeletion := true - isController := true - return &metav1.OwnerReference{ - APIVersion: controllerKind.GroupVersion().String(), - Kind: controllerKind.Kind, - Name: j.Name, - UID: j.UID, - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - } -} diff --git a/pkg/controller/statefulset/stateful_set_utils.go b/pkg/controller/statefulset/stateful_set_utils.go index 3c4fcb9a2c..0102fb1502 100644 --- a/pkg/controller/statefulset/stateful_set_utils.go +++ b/pkg/controller/statefulset/stateful_set_utils.go @@ -219,20 +219,6 @@ func allowsBurst(set *apps.StatefulSet) bool { return set.Spec.PodManagementPolicy == apps.ParallelPodManagement } -// newControllerRef returns an ControllerRef pointing to a given StatefulSet. -func newControllerRef(set *apps.StatefulSet) *metav1.OwnerReference { - blockOwnerDeletion := true - isController := true - return &metav1.OwnerReference{ - APIVersion: controllerKind.GroupVersion().String(), - Kind: controllerKind.Kind, - Name: set.Name, - UID: set.UID, - BlockOwnerDeletion: &blockOwnerDeletion, - Controller: &isController, - } -} - // setPodRevision sets the revision of Pod to revision by adding the StatefulSetRevisionLabel func setPodRevision(pod *v1.Pod, revision string) { if pod.Labels == nil { @@ -252,7 +238,7 @@ func getPodRevision(pod *v1.Pod) string { // newStatefulSetPod returns a new Pod conforming to the set's Spec with an identity generated from ordinal. func newStatefulSetPod(set *apps.StatefulSet, ordinal int) *v1.Pod { - pod, _ := controller.GetPodFromTemplate(&set.Spec.Template, set, newControllerRef(set)) + pod, _ := controller.GetPodFromTemplate(&set.Spec.Template, set, metav1.NewControllerRef(set, controllerKind)) pod.Name = getPodName(set, ordinal) updateIdentity(set, pod) updateStorage(set, pod) From 32b78aebf25fb1aba0eb2704a93e24b61f5ce49f Mon Sep 17 00:00:00 2001 From: Mikhail Mazurskiy Date: Wed, 2 Aug 2017 20:36:58 +1000 Subject: [PATCH 3/3] Migrate to IsControlledBy from meta/v1 package --- pkg/controller/daemon/daemon_controller.go | 2 +- pkg/controller/deployment/util/deployment_util.go | 6 ++---- pkg/controller/history/controller_history_test.go | 11 +++++------ pkg/kubectl/history.go | 7 +++---- test/e2e/apps/daemon_set.go | 4 ++-- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go index d98087d3e1..ce2d90ef41 100644 --- a/pkg/controller/daemon/daemon_controller.go +++ b/pkg/controller/daemon/daemon_controller.go @@ -1068,7 +1068,7 @@ func (dsc *DaemonSetsController) simulate(newPod *v1.Pod, node *v1.Node, ds *ext } // ignore pods that belong to the daemonset when taking into account whether // a daemonset should bind to a node. - if controllerRef := metav1.GetControllerOf(pod); controllerRef != nil && controllerRef.UID == ds.UID { + if metav1.IsControlledBy(pod, ds) { continue } pods = append(pods, pod) diff --git a/pkg/controller/deployment/util/deployment_util.go b/pkg/controller/deployment/util/deployment_util.go index caa108b820..45419f48dd 100644 --- a/pkg/controller/deployment/util/deployment_util.go +++ b/pkg/controller/deployment/util/deployment_util.go @@ -579,8 +579,7 @@ func ListReplicaSets(deployment *extensions.Deployment, getRSList RsListFunc) ([ // Only include those whose ControllerRef matches the Deployment. owned := make([]*extensions.ReplicaSet, 0, len(all)) for _, rs := range all { - controllerRef := metav1.GetControllerOf(rs) - if controllerRef != nil && controllerRef.UID == deployment.UID { + if metav1.IsControlledBy(rs, deployment) { owned = append(owned, rs) } } @@ -603,8 +602,7 @@ func ListReplicaSetsInternal(deployment *internalextensions.Deployment, getRSLis // Only include those whose ControllerRef matches the Deployment. filtered := make([]*internalextensions.ReplicaSet, 0, len(all)) for _, rs := range all { - controllerRef := metav1.GetControllerOf(rs) - if controllerRef != nil && controllerRef.UID == deployment.UID { + if metav1.IsControlledBy(rs, deployment) { filtered = append(filtered, rs) } } diff --git a/pkg/controller/history/controller_history_test.go b/pkg/controller/history/controller_history_test.go index 296fb761dc..ab5845eb34 100644 --- a/pkg/controller/history/controller_history_test.go +++ b/pkg/controller/history/controller_history_test.go @@ -992,7 +992,7 @@ func TestRealHistory_AdoptControllerRevision(t *testing.T) { if !test.err && err != nil { t.Errorf("%s: %s", test.name, err) } - if !test.err && metav1.GetControllerOf(adopted).UID != test.parent.GetUID() { + if !test.err && !metav1.IsControlledBy(adopted, test.parent) { t.Errorf("%s: adoption failed", test.name) } if test.err && err == nil { @@ -1103,7 +1103,7 @@ func TestFakeHistory_AdoptControllerRevision(t *testing.T) { if !test.err && err != nil { t.Errorf("%s: %s", test.name, err) } - if !test.err && metav1.GetControllerOf(adopted).UID != test.parent.GetUID() { + if !test.err && !metav1.IsControlledBy(adopted, test.parent) { t.Errorf("%s: adoption failed", test.name) } if test.err && err == nil { @@ -1211,8 +1211,7 @@ func TestRealHistory_ReleaseControllerRevision(t *testing.T) { if found == nil { return true, nil, errors.NewNotFound(apps.Resource("controllerrevisions"), test.revision.Name) } - if foundParent := metav1.GetControllerOf(test.revision); foundParent == nil || - foundParent.UID != test.parent.GetUID() { + if !metav1.IsControlledBy(test.revision, test.parent) { return true, nil, errors.NewInvalid( test.revision.GroupVersionKind().GroupKind(), test.revision.Name, nil) } @@ -1258,7 +1257,7 @@ func TestRealHistory_ReleaseControllerRevision(t *testing.T) { if adopted == nil { return } - if owner := metav1.GetControllerOf(adopted); owner != nil && owner.UID == test.parent.GetUID() { + if metav1.IsControlledBy(adopted, test.parent) { t.Errorf("%s: release failed", test.name) } } @@ -1386,7 +1385,7 @@ func TestFakeHistory_ReleaseControllerRevision(t *testing.T) { if adopted == nil { return } - if owner := metav1.GetControllerOf(adopted); owner != nil && owner.UID == test.parent.GetUID() { + if metav1.IsControlledBy(adopted, test.parent) { t.Errorf("%s: release failed", test.name) } } diff --git a/pkg/kubectl/history.go b/pkg/kubectl/history.go index 4e3edb83ec..625bbda8ac 100644 --- a/pkg/kubectl/history.go +++ b/pkg/kubectl/history.go @@ -271,11 +271,10 @@ func controlledHistories(c externalclientset.Interface, namespace, name string) } for i := range historyList.Items { history := historyList.Items[i] - // Skip history that doesn't belong to the DaemonSet - if controllerRef := metav1.GetControllerOf(&history); controllerRef == nil || controllerRef.UID != ds.UID { - continue + // Only add history that belongs to the DaemonSet + if metav1.IsControlledBy(&history, ds) { + result = append(result, &history) } - result = append(result, &history) } return ds, result, nil } diff --git a/test/e2e/apps/daemon_set.go b/test/e2e/apps/daemon_set.go index 86422035b8..17a00686f6 100644 --- a/test/e2e/apps/daemon_set.go +++ b/test/e2e/apps/daemon_set.go @@ -630,7 +630,7 @@ func checkDaemonPodOnNodes(f *framework.Framework, ds *extensions.DaemonSet, nod nodesToPodCount := make(map[string]int) for _, pod := range pods { - if controllerRef := metav1.GetControllerOf(&pod); controllerRef == nil || controllerRef.UID != ds.UID { + if !metav1.IsControlledBy(&pod, ds) { continue } if pod.DeletionTimestamp != nil { @@ -726,7 +726,7 @@ func checkDaemonPodsImageAndAvailability(c clientset.Interface, ds *extensions.D unavailablePods := 0 allImagesUpdated := true for _, pod := range pods { - if controllerRef := metav1.GetControllerOf(&pod); controllerRef == nil || controllerRef.UID != ds.UID { + if !metav1.IsControlledBy(&pod, ds) { continue } podImage := pod.Spec.Containers[0].Image