add more testing to assert on cases not handled

Signed-off-by: Troy Connor <[email protected]>

pkg/controller/controllerutil/controllerutil.go

@@ -91,24 +91,25 @@ func SetControllerReference(owner, controlled metav1.Object, scheme *runtime.Sch
 	return nil
 }
 
-// RemoveControllerReference is a helper method to make sure the given object removes an controller reference to the object provided.
+// RemoveControllerReference is a helper method to make sure the given object removes a controller reference to the object provided.
 // This allows you to remove the owner to establish a new owner of the object in a subsequent call.
-func RemoveControllerReference(owner, controlled metav1.Object) error {
-	owners := controlled.GetOwnerReferences()
+func RemoveControllerReference(owner, object metav1.Object) error {
+	owners := object.GetOwnerReferences()
 	length := len(owners)
+	result := []metav1.OwnerReference{}
 	if length < 1 {
-		return fmt.Errorf("%T does not have any owner references", controlled)
+		return fmt.Errorf("%T does not have any owner references", object)
 	}
-	index := 0
-	for i := 0; i < length; i++ {
-		if owners[i].Name == owner.GetName() {
-			owners = append(owners[:index], owners[index+1:]...)
+	for _, ownerref := range owners {
+		if ownerref.Name == owner.GetName() {
+			continue
 		}
-		index++
+		result = append(result, ownerref)
 	}
-	if length == len(owners) {
-		return fmt.Errorf("%T does not have an owner reference for %T", controlled, owner)
+	if len(result) == len(owners) {
+		return fmt.Errorf("%T does not have an owner reference for %T", object, owner)
 	}
+	object.SetOwnerReferences(result)
 	return nil
 }
 

pkg/controller/controllerutil/controllerutil_test.go

@@ -272,8 +272,8 @@ var _ = Describe("Controllerutil", func() {
 				Controller:         &t,
 				BlockOwnerDeletion: &t,
 			}))
-
 			Expect(controllerutil.RemoveControllerReference(dep, rs)).NotTo(HaveOccurred())
+			Expect(len(rs.GetOwnerReferences())).To(BeEquivalentTo(0))
 		})
 		It("should fail and return an error if the length is less than 1", func() {
 			rs := &appsv1.ReplicaSet{}
@@ -293,6 +293,20 @@ var _ = Describe("Controllerutil", func() {
 			Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).NotTo(HaveOccurred())
 			Expect(controllerutil.RemoveControllerReference(dep2, rs)).To(HaveOccurred())
 		})
+		It("should only delete the controller reference and not the other owner references", func() {
+			rs := &appsv1.ReplicaSet{}
+			dep := &extensionsv1beta1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid"},
+			}
+			dep2 := &extensionsv1beta1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{Name: "bar", UID: "bar-uid"},
+			}
+			Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).NotTo(HaveOccurred())
+			Expect(controllerutil.SetOwnerReference(dep2, rs, scheme.Scheme)).NotTo(HaveOccurred())
+			Expect(len(rs.GetOwnerReferences())).To(BeEquivalentTo(2))
+			Expect(controllerutil.RemoveControllerReference(dep, rs)).NotTo(HaveOccurred())
+			Expect(len(rs.GetOwnerReferences())).To(BeEquivalentTo(1))
+		})
 
 	})