Skip to content

Commit b82a7ea

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #816 from vincepri/add-ensure-owner-ref
✨ Add controllerutil.EnsureOwnerReference
2 parents fd64b8f + 05bc80c commit b82a7ea

File tree

2 files changed

+177
-27
lines changed

2 files changed

+177
-27
lines changed

‎pkg/controller/controllerutil/controllerutil.go

Lines changed: 81 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2727
"k8s.io/apimachinery/pkg/runtime"
2828
"k8s.io/apimachinery/pkg/runtime/schema"
29+
"k8s.io/utils/pointer"
2930
"sigs.k8s.io/controller-runtime/pkg/client"
3031
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
3132
)
@@ -55,47 +56,102 @@ func newAlreadyOwnedError(Object metav1.Object, Owner metav1.OwnerReference) *Al
5556
// Since only one OwnerReference can be a controller, it returns an error if
5657
// there is another OwnerReference with Controller flag set.
5758
func SetControllerReference(owner, controlled metav1.Object, scheme *runtime.Scheme) error {
59+
// Validate the owner.
5860
ro, ok := owner.(runtime.Object)
5961
if !ok {
6062
return fmt.Errorf("%T is not a runtime.Object, cannot call SetControllerReference", owner)
6163
}
62-
63-
ownerNs := owner.GetNamespace()
64-
if ownerNs != "" {
65-
objNs := controlled.GetNamespace()
66-
if objNs == "" {
67-
return fmt.Errorf("cluster-scoped resource must not have a namespace-scoped owner, owner's namespace %s", ownerNs)
68-
}
69-
if ownerNs != objNs {
70-
return fmt.Errorf("cross-namespace owner references are disallowed, owner's namespace %s, obj's namespace %s", owner.GetNamespace(), controlled.GetNamespace())
71-
}
64+
if err := validateOwner(owner, controlled); err != nil {
65+
return err
7266
}
7367

68+
// Create a new controller ref.
7469
gvk, err := apiutil.GVKForObject(ro, scheme)
7570
if err != nil {
7671
return err
7772
}
73+
ref := metav1.OwnerReference{
74+
APIVersion: gvk.GroupVersion().String(),
75+
Kind: gvk.Kind,
76+
Name: owner.GetName(),
77+
UID: owner.GetUID(),
78+
BlockOwnerDeletion: pointer.BoolPtr(true),
79+
Controller: pointer.BoolPtr(true),
80+
}
81+
82+
// Return early with an error if the object is already controlled.
83+
if existing := metav1.GetControllerOf(controlled); existing != nil && !referSameObject(*existing, ref) {
84+
return newAlreadyOwnedError(controlled, *existing)
85+
}
7886

79-
// Create a new ref
80-
ref := *metav1.NewControllerRef(owner, schema.GroupVersionKind{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind})
87+
// Update owner references and return.
88+
upsertOwnerRef(ref, controlled)
89+
return nil
90+
}
8191

82-
existingRefs := controlled.GetOwnerReferences()
83-
fi := -1
84-
for i, r := range existingRefs {
85-
if referSameObject(ref, r) {
86-
fi = i
87-
} else if r.Controller != nil && *r.Controller {
88-
return newAlreadyOwnedError(controlled, r)
89-
}
92+
// EnsureOwnerReference is a helper method to make sure the given object contains
93+
// an object reference to the object provided.
94+
// If a reference already exists, it'll be overwritten with the newly provided version.
95+
func EnsureOwnerReference(owner, object metav1.Object, scheme *runtime.Scheme) error {
96+
// Validate the owner.
97+
ro, ok := owner.(runtime.Object)
98+
if !ok {
99+
return fmt.Errorf("%T is not a runtime.Object, cannot call SetControllerReference", owner)
100+
}
101+
if err := validateOwner(owner, object); err != nil {
102+
return err
103+
}
104+
105+
// Create a new owner ref.
106+
gvk, err := apiutil.GVKForObject(ro, scheme)
107+
if err != nil {
108+
return err
90109
}
91-
if fi == -1 {
92-
existingRefs = append(existingRefs, ref)
110+
ref := metav1.OwnerReference{
111+
APIVersion: gvk.GroupVersion().String(),
112+
Kind: gvk.Kind,
113+
UID: owner.GetUID(),
114+
Name: owner.GetName(),
115+
}
116+
117+
// Update owner references and return.
118+
upsertOwnerRef(ref, object)
119+
return nil
120+
121+
}
122+
123+
func upsertOwnerRef(ref metav1.OwnerReference, object metav1.Object) {
124+
owners := object.GetOwnerReferences()
125+
idx := indexOwnerRef(owners, ref)
126+
if idx == -1 {
127+
owners = append(owners, ref)
93128
} else {
94-
existingRefs[fi] = ref
129+
owners[idx] = ref
130+
}
131+
object.SetOwnerReferences(owners)
132+
}
133+
134+
// indexOwnerRef returns the index of the owner reference in the slice if found, or -1.
135+
func indexOwnerRef(ownerReferences []metav1.OwnerReference, ref metav1.OwnerReference) int {
136+
for index, r := range ownerReferences {
137+
if referSameObject(r, ref) {
138+
return index
139+
}
95140
}
141+
return -1
142+
}
96143

97-
// Update owner references
98-
controlled.SetOwnerReferences(existingRefs)
144+
func validateOwner(owner, object metav1.Object) error {
145+
ownerNs := owner.GetNamespace()
146+
if ownerNs != "" {
147+
objNs := object.GetNamespace()
148+
if objNs == "" {
149+
return fmt.Errorf("cluster-scoped resource must not have a namespace-scoped owner, owner's namespace %s", ownerNs)
150+
}
151+
if ownerNs != objNs {
152+
return fmt.Errorf("cross-namespace owner references are disallowed, owner's namespace %s, obj's namespace %s", owner.GetNamespace(), object.GetNamespace())
153+
}
154+
}
99155
return nil
100156
}
101157

‎pkg/controller/controllerutil/controllerutil_test.go

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ import (
2121
"fmt"
2222
"math/rand"
2323

24-
"sigs.k8s.io/controller-runtime/pkg/client"
25-
2624
. "github.com/onsi/ginkgo"
2725
. "github.com/onsi/gomega"
2826
appsv1 "k8s.io/api/apps/v1"
@@ -32,10 +30,80 @@ import (
3230
"k8s.io/apimachinery/pkg/runtime"
3331
"k8s.io/apimachinery/pkg/types"
3432
"k8s.io/client-go/kubernetes/scheme"
33+
"sigs.k8s.io/controller-runtime/pkg/client"
3534
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3635
)
3736

3837
var _ = Describe("Controllerutil", func() {
38+
Describe("EnsureOwnerReference", func() {
39+
It("should set ownerRef on an empty list", func() {
40+
rs := &appsv1.ReplicaSet{}
41+
dep := &extensionsv1beta1.Deployment{
42+
ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid"},
43+
}
44+
Expect(controllerutil.EnsureOwnerReference(dep, rs, scheme.Scheme)).ToNot(HaveOccurred())
45+
Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
46+
Name: "foo",
47+
Kind: "Deployment",
48+
APIVersion: "extensions/v1beta1",
49+
UID: "foo-uid",
50+
}))
51+
})
52+
53+
It("should not duplicate owner references", func() {
54+
rs := &appsv1.ReplicaSet{
55+
ObjectMeta: metav1.ObjectMeta{
56+
OwnerReferences: []metav1.OwnerReference{
57+
{
58+
Name: "foo",
59+
Kind: "Deployment",
60+
APIVersion: "extensions/v1beta1",
61+
UID: "foo-uid",
62+
},
63+
},
64+
},
65+
}
66+
dep := &extensionsv1beta1.Deployment{
67+
ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid"},
68+
}
69+
70+
Expect(controllerutil.EnsureOwnerReference(dep, rs, scheme.Scheme)).ToNot(HaveOccurred())
71+
Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
72+
Name: "foo",
73+
Kind: "Deployment",
74+
APIVersion: "extensions/v1beta1",
75+
UID: "foo-uid",
76+
}))
77+
})
78+
79+
It("should update the reference", func() {
80+
rs := &appsv1.ReplicaSet{
81+
ObjectMeta: metav1.ObjectMeta{
82+
OwnerReferences: []metav1.OwnerReference{
83+
{
84+
Name: "foo",
85+
Kind: "Deployment",
86+
APIVersion: "extensions/v1alpha1",
87+
UID: "foo-uid-1",
88+
},
89+
},
90+
},
91+
}
92+
dep := &extensionsv1beta1.Deployment{
93+
ObjectMeta: metav1.ObjectMeta{Name: "foo", UID: "foo-uid-2"},
94+
}
95+
96+
Expect(controllerutil.EnsureOwnerReference(dep, rs, scheme.Scheme)).ToNot(HaveOccurred())
97+
Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
98+
Name: "foo",
99+
Kind: "Deployment",
100+
APIVersion: "extensions/v1beta1",
101+
UID: "foo-uid-2",
102+
}))
103+
104+
})
105+
})
106+
39107
Describe("SetControllerReference", func() {
40108
It("should set the OwnerReference if it can find the group version kind", func() {
41109
rs := &appsv1.ReplicaSet{}
@@ -116,6 +184,32 @@ var _ = Describe("Controllerutil", func() {
116184
}))
117185
})
118186

187+
It("should replace the owner reference if it's already present", func() {
188+
t := true
189+
rsOwners := []metav1.OwnerReference{
190+
{
191+
Name: "foo",
192+
Kind: "Deployment",
193+
APIVersion: "extensions/v1alpha1",
194+
UID: "foo-uid",
195+
Controller: &t,
196+
BlockOwnerDeletion: &t,
197+
},
198+
}
199+
rs := &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default", OwnerReferences: rsOwners}}
200+
dep := &extensionsv1beta1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default", UID: "foo-uid"}}
201+
202+
Expect(controllerutil.SetControllerReference(dep, rs, scheme.Scheme)).NotTo(HaveOccurred())
203+
Expect(rs.OwnerReferences).To(ConsistOf(metav1.OwnerReference{
204+
Name: "foo",
205+
Kind: "Deployment",
206+
APIVersion: "extensions/v1beta1",
207+
UID: "foo-uid",
208+
Controller: &t,
209+
BlockOwnerDeletion: &t,
210+
}))
211+
})
212+
119213
It("should return an error if it's setting a cross-namespace owner reference", func() {
120214
rs := &appsv1.ReplicaSet{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "namespace1"}}
121215
dep := &extensionsv1beta1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "namespace2", UID: "foo-uid"}}

0 commit comments

Comments
 (0)