Add tests for SetControllerReference

Author: calind

pkg/controller/controllerutil/controllerutil.go

@@ -28,6 +28,8 @@ import (
 // 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 OwnerRecerence 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 {
@@ -50,9 +52,10 @@ func SetControllerReference(owner, object v1.Object, scheme *runtime.Scheme) err
 	existingRefs := object.GetOwnerReferences()
 	fi := -1
 	for i, r := range existingRefs {
-		if r.Kind == gvk.Kind && r.Name == owner.GetName() {
+		if r.Kind == gvk.Kind && r.Name == owner.GetName() && r.UID == owner.GetUID() {
 			fi = i
-			break
+		} else if r.Controller != nil && *r.Controller {
+			return fmt.Errorf("Object %s/%s is already owned by another %s controller %s", object.GetNamespace(), object.GetName(), r.Kind, r.Name)
 		}
 	}
 	if fi == -1 {

pkg/controller/controllerutil/controllerutil_test.go

@@ -43,6 +43,58 @@ var _ = Describe("Controllerutil", func() {
 			rs := &appsv1.ReplicaSet{}
 			Expect(controllerutil.SetControllerReference(&errMetaObj{}, rs, scheme.Scheme)).To(HaveOccurred())
 		})
+
+		It("should return an error if object and owner are not in the same namespace", func() {
+			rs := &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "one"}}
+			dep := &extensionsv1beta1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "two"}}
+
+			Expect(controllerutil.SetControllerReference(dep, 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:               "foo",
+					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"}}
+
+			Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).To(HaveOccurred())
+		})
+
+		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,
+			}))
+		})
 	})
 })