Skip to content

Commit 770c4ab

Browse files
committed
add more testing to assert on cases not handled
Signed-off-by: Troy Connor <[email protected]>
1 parent d0ae418 commit 770c4ab

File tree

2 files changed

+23
-7
lines changed

2 files changed

+23
-7
lines changed

pkg/controller/controllerutil/controllerutil.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,19 +96,21 @@ func SetControllerReference(owner, controlled metav1.Object, scheme *runtime.Sch
9696
func RemoveControllerReference(owner, controlled metav1.Object) error {
9797
owners := controlled.GetOwnerReferences()
9898
length := len(owners)
99+
result := []metav1.OwnerReference{}
99100
if length < 1 {
100101
return fmt.Errorf("%T does not have any owner references", controlled)
101102
}
102-
index := 0
103-
for i := 0; i < length; i++ {
104-
if owners[i].Name == owner.GetName() {
105-
owners = append(owners[:index], owners[index+1:]...)
103+
for _, ownerref := range owners {
104+
if ownerref.Name == owner.GetName() {
105+
continue
106106
}
107-
index++
107+
result = append(result, ownerref)
108108
}
109-
if length == len(owners) {
109+
110+
if len(result) == len(owners) {
110111
return fmt.Errorf("%T does not have an owner reference for %T", controlled, owner)
111112
}
113+
controlled.SetOwnerReferences(result)
112114
return nil
113115
}
114116

pkg/controller/controllerutil/controllerutil_test.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,8 @@ var _ = Describe("Controllerutil", func() {
272272
Controller: &t,
273273
BlockOwnerDeletion: &t,
274274
}))
275-
276275
Expect(controllerutil.RemoveControllerReference(dep, rs)).NotTo(HaveOccurred())
276+
Expect(len(rs.GetOwnerReferences())).To(BeEquivalentTo(0))
277277
})
278278
It("should fail and return an error if the length is less than 1", func() {
279279
rs := &appsv1.ReplicaSet{}
@@ -293,6 +293,20 @@ var _ = Describe("Controllerutil", func() {
293293
Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).NotTo(HaveOccurred())
294294
Expect(controllerutil.RemoveControllerReference(dep2, rs)).To(HaveOccurred())
295295
})
296+
It("should only delete the controller reference and not the other owner references", func() {
297+
rs := &appsv1.ReplicaSet{}
298+
dep := &extensionsv1beta1.Deployment{
299+
ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid"},
300+
}
301+
dep2 := &extensionsv1beta1.Deployment{
302+
ObjectMeta: metav1.ObjectMeta{Name: "bar", UID: "bar-uid"},
303+
}
304+
Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).NotTo(HaveOccurred())
305+
Expect(controllerutil.SetOwnerReference(dep2, rs, scheme.Scheme)).NotTo(HaveOccurred())
306+
Expect(len(rs.GetOwnerReferences())).To(BeEquivalentTo(2))
307+
Expect(controllerutil.RemoveControllerReference(dep, rs)).NotTo(HaveOccurred())
308+
Expect(len(rs.GetOwnerReferences())).To(BeEquivalentTo(1))
309+
})
296310

297311
})
298312

0 commit comments

Comments
 (0)