Fix duplicate controller references

Author: calind

pkg/controller/controllerutil/controllerutil.go

@@ -25,9 +25,30 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 )
 
+// AlreadyOwnedError is an error returned if the object you are trying to assign
+// a controller reference is already owned by another controller Object is the
+// subject and Owner is the reference for the current owner
+type AlreadyOwnedError struct {
+	Object v1.Object
+	Owner  v1.OwnerReference
+}
+
+func (e *AlreadyOwnedError) Error() string {
+	return fmt.Sprintf("Object %s/%s is already owned by another %s controller %s", e.Object.GetNamespace(), e.Object.GetName(), e.Owner.Kind, e.Owner.Name)
+}
+
+func newAlreadyOwnedError(Object v1.Object, Owner v1.OwnerReference) *AlreadyOwnedError {
+	return &AlreadyOwnedError{
+		Object: Object,
+		Owner:  Owner,
+	}
+}
+
 // SetControllerReference sets owner as a Controller OwnerReference on owned.
 // This is used for garbage collection of the owned object and for
 // reconciling the owner object on changes to owned (with a Watch + EnqueueRequestForOwner).
+// Since only one OwnerReference can be a controller, it returns an error if
+// there is another OwnerReference with Controller flag set.
 func SetControllerReference(owner, object v1.Object, scheme *runtime.Scheme) error {
 	ro, ok := owner.(runtime.Object)
 	if !ok {
@@ -42,7 +63,37 @@ func SetControllerReference(owner, object v1.Object, scheme *runtime.Scheme) err
 	// Create a new ref
 	ref := *v1.NewControllerRef(owner, schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind})
 
-	// Add it to the child
-	object.SetOwnerReferences(append(object.GetOwnerReferences(), ref))
+	existingRefs := object.GetOwnerReferences()
+	fi := -1
+	for i, r := range existingRefs {
+		if referSameObject(ref, r) {
+			fi = i
+		} else if r.Controller != nil && *r.Controller {
+			return newAlreadyOwnedError(object, r)
+		}
+	}
+	if fi == -1 {
+		existingRefs = append(existingRefs, ref)
+	} else {
+		existingRefs[fi] = ref
+	}
+
+	// Update owner references
+	object.SetOwnerReferences(existingRefs)
 	return nil
 }
+
+// Returns true if a and b point to the same object
+func referSameObject(a, b v1.OwnerReference) bool {
+	aGV, err := schema.ParseGroupVersion(a.APIVersion)
+	if err != nil {
+		return false
+	}
+
+	bGV, err := schema.ParseGroupVersion(b.APIVersion)
+	if err != nil {
+		return false
+	}
+
+	return aGV == bGV && a.Kind == b.Kind && a.Name == b.Name
+}

pkg/controller/controllerutil/controllerutil_test.go

@@ -43,6 +43,54 @@ var _ = Describe("Controllerutil", func() {
 			rs := &appsv1.ReplicaSet{}
 			Expect(controllerutil.SetControllerReference(&errMetaObj{}, rs, scheme.Scheme)).To(HaveOccurred())
 		})
+
+		It("should return an error if object is already owned by another controller", func() {
+			t := true
+			rsOwners := []metav1.OwnerReference{
+				metav1.OwnerReference{
+					Name:               "bar",
+					Kind:               "Deployment",
+					APIVersion:         "extensions/v1beta1",
+					UID:                "bar-uid",
+					Controller:         &t,
+					BlockOwnerDeletion: &t,
+				},
+			}
+			rs := &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default", OwnerReferences: rsOwners}}
+			dep := &extensionsv1beta1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default", UID: "foo-uid"}}
+
+			err := controllerutil.SetControllerReference(dep, rs, scheme.Scheme)
+
+			Expect(err).To(HaveOccurred())
+			Expect(err).To(BeAssignableToTypeOf(&controllerutil.AlreadyOwnedError{}))
+		})
+
+		It("should not duplicate existing owner reference", func() {
+			f := false
+			t := true
+			rsOwners := []metav1.OwnerReference{
+				metav1.OwnerReference{
+					Name:               "foo",
+					Kind:               "Deployment",
+					APIVersion:         "extensions/v1beta1",
+					UID:                "foo-uid",
+					Controller:         &f,
+					BlockOwnerDeletion: &t,
+				},
+			}
+			rs := &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default", OwnerReferences: rsOwners}}
+			dep := &extensionsv1beta1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default", UID: "foo-uid"}}
+
+			Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).NotTo(HaveOccurred())
+			Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
+				Name:               "foo",
+				Kind:               "Deployment",
+				APIVersion:         "extensions/v1beta1",
+				UID:                "foo-uid",
+				Controller:         &t,
+				BlockOwnerDeletion: &t,
+			}))
+		})
 	})
 })