Merge pull request #83022 from mm4tt/controller_ref

Optimize GetControllerOf method and add a benchmark for it

pkg/controller/controller_ref_manager.go

@@ -64,7 +64,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 := metav1.GetControllerOf(obj)
+	controllerRef := metav1.GetControllerOfNoCopy(obj)
 	if controllerRef != nil {
 		if controllerRef.UID != m.Controller.GetUID() {
 			// Owned by someone else. Ignore.

pkg/controller/history/controller_history.go

@@ -224,7 +224,7 @@ func (rh *realHistory) ListControllerRevisions(parent metav1.Object, selector la
 	}
 	var owned []*apps.ControllerRevision
 	for i := range history {
-		ref := metav1.GetControllerOf(history[i])
+		ref := metav1.GetControllerOfNoCopy(history[i])
 		if ref == nil || ref.UID == parent.GetUID() {
 			owned = append(owned, history[i])
 		}
@@ -292,7 +292,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 := metav1.GetControllerOf(revision); owner != nil {
+	if owner := metav1.GetControllerOfNoCopy(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

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/controller_ref.go

@@ -22,7 +22,7 @@ import (
 
 // IsControlledBy checks if the  object has a controllerRef set to the given owner
 func IsControlledBy(obj Object, owner Object) bool {
-	ref := GetControllerOf(obj)
+	ref := GetControllerOfNoCopy(obj)
 	if ref == nil {
 		return false
 	}
@@ -31,9 +31,20 @@ func IsControlledBy(obj Object, owner Object) bool {
 
 // GetControllerOf returns a pointer to a copy of the controllerRef if controllee has a controller
 func GetControllerOf(controllee Object) *OwnerReference {
-	for _, ref := range controllee.GetOwnerReferences() {
+	ref := GetControllerOfNoCopy(controllee)
-		if ref.Controller != nil && *ref.Controller {
+	if ref == nil {
-			return &ref
+		return nil
+	}
+	cp := *ref
+	return &cp
+}
+
+// GetControllerOf returns a pointer to the controllerRef if controllee has a controller
+func GetControllerOfNoCopy(controllee Object) *OwnerReference {
+	refs := controllee.GetOwnerReferences()
+	for i := range refs {
+		if refs[i].Controller != nil && *refs[i].Controller {
+			return &refs[i]
 		}
 	}
 	return nil

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/controller_ref_test.go

@@ -97,6 +97,39 @@ func TestGetControllerOf(t *testing.T) {
 	}
 }
 
+func BenchmarkGetControllerOf(b *testing.B) {
+	gvk := schema.GroupVersionKind{
+		Group:   "group",
+		Version: "v1",
+		Kind:    "Kind",
+	}
+	obj1 := &metaObj{
+		ObjectMeta: ObjectMeta{
+			UID:  "9d0cdf8a-dedc-11e9-bf91-42010a800167",
+			Name: "my-object",
+		},
+	}
+	controllerRef := NewControllerRef(obj1, gvk)
+	controllerRef2 := *controllerRef
+	controllerRef2.Controller = nil
+	obj2 := &metaObj{
+		ObjectMeta: ObjectMeta{
+			UID:             "uid2",
+			Name:            "name1",
+			OwnerReferences: []OwnerReference{controllerRef2, controllerRef2, *controllerRef},
+		},
+	}
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	for n := 0; n < b.N; n++ {
+		c := GetControllerOf(obj2)
+		if c.Name != controllerRef.Name || c.UID != controllerRef.UID {
+			b.Errorf("Incorrect result of GetControllerOf: %v", c)
+		}
+	}
+}
+
 func TestIsControlledBy(t *testing.T) {
 	gvk := schema.GroupVersionKind{
 		Group:   "group",