Skip to content

Commit 2fe301a

Browse files
calindcalind
committed
Get in line with client.Create and client.Update functions
1 parent 4855d31 commit 2fe301a

File tree

2 files changed

+140
-84
lines changed

2 files changed

+140
-84
lines changed

‎pkg/controller/controllerutil/controllerutil.go

Lines changed: 58 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -108,68 +108,80 @@ type OperationType string
108108
const ( // They should complete the sentence "Deployment default/foo has been ..."
109109
// OperationNoop means that the resource has not been changed
110110
OperationNoop = "unchanged"
111-
// OperationCreated means that a new resource has been created
112-
OperationCreated = "created"
113-
// OperationUpdated means that an existing resource has been updated
114-
OperationUpdated = "updated"
111+
// OperationCreate means that a new resource is created
112+
OperationCreate = "created"
113+
// OperationUpdate means that an existing resource is updated
114+
OperationUpdate = "updated"
115115
)
116116

117-
// CreateOrUpdate creates or updates a kubernetes resource. It takes in a key and
118-
// a placeholder for the existing object and returns the operation executed
119-
func CreateOrUpdate(ctx context.Context, c client.Client, key client.ObjectKey, existing runtime.Object, t TransformFn) (OperationType, error) {
120-
err := c.Get(ctx, key, existing)
117+
// CreateOrUpdate creates or updates a kubernetes resource. It takes in a
118+
// desired object state and an optional list of reconcile functions which
119+
// reconcile the desired object with the existing state
120+
func CreateOrUpdate(ctx context.Context, c client.Client, desired runtime.Object, reconciles ...ReconcileFn) (OperationType, error) {
121+
// op is the operation we are going to attempt
122+
var op OperationType = OperationNoop
123+
// obj is the object to be created/updated
121124
var obj runtime.Object
122125

126+
existing, err := getExistingObject(ctx, c, desired)
127+
128+
// decide about the operation we are going to attempt
123129
if errors.IsNotFound(err) {
124-
// Create a new zero value object so that the in parameter of
125-
// TransformFn is always a "clean" object, with only Name and Namespace
126-
// set
127-
zero := reflect.New(reflect.TypeOf(existing).Elem()).Interface()
128-
129-
// Set Namespace and Name from the lookup key
130-
zmeta, ok := zero.(v1.Object)
131-
if !ok {
132-
return OperationNoop, fmt.Errorf("is not a %T a metav1.Object, cannot call CreateOrUpdate", zero)
133-
}
134-
zmeta.SetNamespace(key.Namespace)
135-
zmeta.SetName(key.Name)
130+
op = OperationCreate
131+
obj = desired.DeepCopyObject()
132+
} else if err == nil {
133+
op = OperationUpdate
134+
obj = desired.DeepCopyObject()
135+
} else {
136+
return OperationNoop, err
137+
}
136138

137-
// Apply the TransformFn
138-
obj, err = t(zero.(runtime.Object))
139+
// reconcile the object with the existing object
140+
for _, r := range reconciles {
141+
err = r(obj, existing)
139142
if err != nil {
140143
return OperationNoop, err
141144
}
145+
}
142146

143-
// Create the new object
147+
switch op {
148+
case OperationCreate:
144149
err = c.Create(ctx, obj)
145-
if err != nil {
146-
return OperationNoop, err
150+
case OperationUpdate:
151+
if reflect.DeepEqual(existing, obj) {
152+
return OperationNoop, nil
147153
}
154+
err = c.Update(ctx, obj)
155+
default:
156+
panic("This should be unreachable")
157+
}
148158

149-
return OperationCreated, err
150-
} else if err != nil {
159+
if err != nil {
151160
return OperationNoop, err
152-
} else {
153-
obj, err = t(existing.DeepCopyObject())
154-
if err != nil {
155-
return OperationNoop, err
156-
}
161+
}
162+
return op, nil
163+
}
157164

158-
if !reflect.DeepEqual(existing, obj) {
159-
err = c.Update(ctx, obj)
160-
if err != nil {
161-
return OperationNoop, err
162-
}
165+
// getExistingObject returns the existing object for the passed in desired
166+
// object. If there is no existing object, returns an zero value object with the
167+
// type of desired object
168+
func getExistingObject(ctx context.Context, c client.Client, desired runtime.Object) (runtime.Object, error) {
169+
mo, ok := desired.(v1.Object)
170+
if !ok {
171+
return nil, fmt.Errorf("%T is not a metav1.Object, cannot call getExistingObject", desired)
172+
}
173+
key := client.ObjectKey{
174+
Name: mo.GetName(),
175+
Namespace: mo.GetNamespace(),
176+
}
163177

164-
return OperationUpdated, err
165-
}
178+
existing := reflect.New(reflect.TypeOf(desired).Elem()).Interface().(runtime.Object)
179+
err := c.Get(ctx, key, existing)
166180

167-
return OperationNoop, nil
168-
}
181+
return existing, err
169182
}
170183

171-
// TransformFn is a function which take in a kubernetes object and returns the
172-
// desired state of that object.
173-
// It is safe to mutate the object inside this function, since it's always
174-
// called with an object's deep copy.
175-
type TransformFn func(in runtime.Object) (runtime.Object, error)
184+
// ReconcileFn should mutate the desired object state and reconcile it with the
185+
// existing object. When there is no existing object, a zero value object of
186+
// desired object type gets passed in.
187+
type ReconcileFn func(desired, existing runtime.Object) error

‎pkg/controller/controllerutil/controllerutil_test.go

Lines changed: 82 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package controllerutil_test
22

33
import (
44
"context"
5+
"fmt"
6+
"math/rand"
57

68
. "github.com/onsi/ginkgo"
79
. "github.com/onsi/gomega"
@@ -98,17 +100,49 @@ var _ = Describe("Controllerutil", func() {
98100
})
99101

100102
Describe("CreateOrUpdate", func() {
103+
var deploy *appsv1.Deployment
104+
var deplKey types.NamespacedName
105+
106+
BeforeEach(func() {
107+
deploy = &appsv1.Deployment{
108+
Spec: appsv1.DeploymentSpec{
109+
Selector: &metav1.LabelSelector{
110+
MatchLabels: map[string]string{"foo": "bar"},
111+
},
112+
Template: corev1.PodTemplateSpec{
113+
ObjectMeta: metav1.ObjectMeta{
114+
Labels: map[string]string{
115+
"foo": "bar",
116+
},
117+
},
118+
Spec: corev1.PodSpec{
119+
Containers: []corev1.Container{
120+
corev1.Container{
121+
Name: "foo",
122+
Image: "busybox",
123+
},
124+
},
125+
},
126+
},
127+
},
128+
}
101129

102-
It("creates a new object if one doesn't exists", func() {
103-
deplKey := types.NamespacedName{Name: "test-create", Namespace: "default"}
104-
depl := &appsv1.Deployment{}
130+
deploy.Name = fmt.Sprintf("deploy-%d", rand.Int31())
131+
deploy.Namespace = "default"
105132

106-
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deplKey, depl, createDeployment)
133+
deplKey = types.NamespacedName{
134+
Name: deploy.Name,
135+
Namespace: deploy.Namespace,
136+
}
137+
})
138+
139+
It("creates a new object if one doesn't exists", func() {
140+
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy)
107141

108142
By("returning OperationCreated")
109-
Expect(op).Should(BeEquivalentTo(controllerutil.OperationCreated))
143+
Expect(op).To(BeEquivalentTo(controllerutil.OperationCreate))
110144

111-
By("returning returning no error")
145+
By("returning no error")
112146
Expect(err).ShouldNot(HaveOccurred())
113147

114148
By("actually having the deployment created")
@@ -117,22 +151,15 @@ var _ = Describe("Controllerutil", func() {
117151
})
118152

119153
It("update existing object", func() {
120-
deplKey := types.NamespacedName{Name: "test-update", Namespace: "default"}
121-
d, _ := createDeployment(&appsv1.Deployment{})
122-
depl := d.(*appsv1.Deployment)
123-
depl.Name = "test-update"
124-
depl.Namespace = "default"
125-
126154
var scale int32 = 2
155+
Expect(c.Create(context.TODO(), deploy)).Should(Succeed())
127156

128-
Expect(c.Create(context.TODO(), depl)).Should(Succeed())
129-
130-
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deplKey, &appsv1.Deployment{}, deploymentScaler(scale))
157+
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy, deploymentScaler(scale))
131158

132159
By("returning OperationUpdated")
133-
Expect(op).Should(BeEquivalentTo(controllerutil.OperationUpdated))
160+
Expect(op).Should(BeEquivalentTo(controllerutil.OperationUpdate))
134161

135-
By("returning returning no error")
162+
By("returning no error")
136163
Expect(err).ShouldNot(HaveOccurred())
137164

138165
By("actually having the deployment scaled")
@@ -142,21 +169,46 @@ var _ = Describe("Controllerutil", func() {
142169
})
143170

144171
It("updates only changed objects", func() {
145-
deplKey := types.NamespacedName{Name: "test-idempotency", Namespace: "default"}
146-
depl := &appsv1.Deployment{}
172+
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy)
147173

148-
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deplKey, depl, createDeployment)
149-
Expect(op).Should(BeEquivalentTo(controllerutil.OperationCreated))
174+
Expect(op).Should(BeEquivalentTo(controllerutil.OperationCreate))
150175
Expect(err).ShouldNot(HaveOccurred())
151176

152-
op, err = controllerutil.CreateOrUpdate(context.TODO(), c, deplKey, depl, deploymentIdentity)
177+
op, err = controllerutil.CreateOrUpdate(context.TODO(), c, deploy, deploymentIdentity)
153178

154179
By("returning OperationNoop")
155180
Expect(op).Should(BeEquivalentTo(controllerutil.OperationNoop))
156181

157-
By("returning returning no error")
182+
By("returning no error")
158183
Expect(err).ShouldNot(HaveOccurred())
159184
})
185+
186+
It("allows chaining transforms", func() {
187+
scaleToTwo := deploymentScaler(2)
188+
scaleToThree := deploymentScaler(3)
189+
190+
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy, scaleToTwo, scaleToThree)
191+
192+
Expect(op).Should(BeEquivalentTo(controllerutil.OperationCreate))
193+
Expect(err).ShouldNot(HaveOccurred())
194+
195+
By("applying the last scale")
196+
fetched := &appsv1.Deployment{}
197+
Expect(c.Get(context.TODO(), deplKey, fetched)).Should(Succeed())
198+
Expect(*fetched.Spec.Replicas).To(Equal(int32(3)))
199+
})
200+
201+
It("doesn't mutate the desired object", func() {
202+
scaleToTwo := deploymentScaler(2)
203+
scaleToThree := deploymentScaler(3)
204+
205+
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy, scaleToTwo, scaleToThree)
206+
207+
Expect(op).Should(BeEquivalentTo(controllerutil.OperationCreate))
208+
Expect(err).ShouldNot(HaveOccurred())
209+
210+
Expect(deploy.Spec.Replicas).To(BeNil())
211+
})
160212
})
161213
})
162214

@@ -166,24 +218,16 @@ type errMetaObj struct {
166218
metav1.ObjectMeta
167219
}
168220

169-
var createDeployment controllerutil.TransformFn = func(in runtime.Object) (runtime.Object, error) {
170-
out := in.(*appsv1.Deployment)
171-
out.Spec.Selector = &metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}}
172-
out.Spec.Template.ObjectMeta.Labels = map[string]string{"foo": "bar"}
173-
out.Spec.Template.Spec.Containers = []corev1.Container{corev1.Container{Name: "foo", Image: "busybox"}}
174-
return out, nil
175-
}
176-
177-
var deploymentIdentity controllerutil.TransformFn = func(in runtime.Object) (runtime.Object, error) {
178-
return in, nil
221+
var deploymentIdentity controllerutil.ReconcileFn = func(obj, existing runtime.Object) error {
222+
existing.(*appsv1.Deployment).DeepCopyInto(obj.(*appsv1.Deployment))
223+
return nil
179224
}
180225

181-
func deploymentScaler(replicas int32) controllerutil.TransformFn {
182-
fn := func(in runtime.Object) (runtime.Object, error) {
183-
d, _ := createDeployment(in)
184-
out := d.(*appsv1.Deployment)
226+
func deploymentScaler(replicas int32) controllerutil.ReconcileFn {
227+
fn := func(obj, existing runtime.Object) error {
228+
out := obj.(*appsv1.Deployment)
185229
out.Spec.Replicas = &replicas
186-
return out, nil
230+
return nil
187231
}
188232
return fn
189233
}

0 commit comments

Comments
 (0)