Merge pull request #2595 from troy0820/troy0820/remove-controller-ref-bug

 🐛Remove owner passed in to RemoveControllerReference

Author: k8s-ci-robot

pkg/controller/controllerutil/controllerutil.go

@@ -171,7 +171,32 @@ func RemoveControllerReference(owner, object metav1.Object, scheme *runtime.Sche
 	if ok := HasControllerReference(object); !ok {
 		return fmt.Errorf("%T does not have a owner reference with controller equals true", object)
 	}
-	return RemoveOwnerReference(owner, object, scheme)
+	ro, ok := owner.(runtime.Object)
+	if !ok {
+		return fmt.Errorf("%T is not a runtime.Object, cannot call RemoveControllerReference", owner)
+	}
+	gvk, err := apiutil.GVKForObject(ro, scheme)
+	if err != nil {
+		return err
+	}
+	ownerRefs := object.GetOwnerReferences()
+	index := indexOwnerRef(ownerRefs, metav1.OwnerReference{
+		APIVersion: gvk.GroupVersion().String(),
+		Name:       owner.GetName(),
+		Kind:       gvk.Kind,
+	})
+
+	if index == -1 {
+		return fmt.Errorf("%T does not have an controller reference for %T", object, owner)
+	}
+
+	if ownerRefs[index].Controller == nil || !*ownerRefs[index].Controller {
+		return fmt.Errorf("%T owner is not the controller reference for %T", owner, object)
+	}
+
+	ownerRefs = append(ownerRefs[:index], ownerRefs[index+1:]...)
+	object.SetOwnerReferences(ownerRefs)
+	return nil
 }
 
 func upsertOwnerRef(ref metav1.OwnerReference, object metav1.Object) {
@@ -219,7 +244,6 @@ func referSameObject(a, b metav1.OwnerReference) bool {
 	if err != nil {
 		return false
 	}
-
 	return aGV.Group == bGV.Group && a.Kind == b.Kind && a.Name == b.Name
 }
 

pkg/controller/controllerutil/controllerutil_test.go

@@ -181,6 +181,7 @@ var _ = Describe("Controllerutil", func() {
 			Expect(controllerutil.RemoveOwnerReference(obj, rs, scheme.Scheme)).To(HaveOccurred())
 			Expect(rs.GetOwnerReferences()).To(HaveLen(1))
 		})
+
 		It("should error when trying to remove an owner that doesn't exist", func() {
 			rs := &appsv1.ReplicaSet{
 				ObjectMeta: metav1.ObjectMeta{},
@@ -229,6 +230,22 @@ var _ = Describe("Controllerutil", func() {
 			Expect(controllerutil.RemoveControllerReference(dep, rs, scheme.Scheme)).To(HaveOccurred())
 		})
 
+		It("should error when RemoveControllerReference passed in owner is not the owner", func() {
+			rs := &appsv1.ReplicaSet{
+				ObjectMeta: metav1.ObjectMeta{},
+			}
+			dep := &extensionsv1beta1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid-2"},
+			}
+			dep2 := &extensionsv1beta1.Deployment{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo-2", UID: "foo-uid-42"},
+			}
+			Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).ToNot(HaveOccurred())
+			Expect(controllerutil.SetOwnerReference(dep2, rs, scheme.Scheme)).ToNot(HaveOccurred())
+			Expect(controllerutil.RemoveControllerReference(dep2, rs, scheme.Scheme)).To(HaveOccurred())
+			Expect(rs.GetOwnerReferences()).To(HaveLen(2))
+		})
+
 		It("should not error when RemoveControllerReference owner's controller is set to true", func() {
 			rs := &appsv1.ReplicaSet{
 				ObjectMeta: metav1.ObjectMeta{},