Skip to content

Commit ae8a7c2

Browse files
committed
CreateOrPatch
This patch introduces a variation on the controllerutil.CreateOrUpdate function named CreateOrPatch. Instead of issuing update calls, the new function uses a patch to perform a more surgical update to the remote data. Additionally, the implementation relies on logic similar to the PatchHelper mechanics in the Cluster API util/patch package. The resource is converted to unstructured data first in order to patch the resource and any potential status separately. Two new OperationResult values were added: 1. OperationResultUpdatedStatus - the resource and its status were updated 2. OperationResultUpdatedStatusOnly - only the status was updated
1 parent d88045b commit ae8a7c2

File tree

2 files changed

+304
-0
lines changed

2 files changed

+304
-0
lines changed

pkg/controller/controllerutil/controllerutil.go

+108
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ package controllerutil
1919
import (
2020
"context"
2121
"fmt"
22+
"reflect"
2223

2324
"k8s.io/apimachinery/pkg/api/equality"
2425
"k8s.io/apimachinery/pkg/api/errors"
2526
"k8s.io/apimachinery/pkg/api/meta"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2729
"k8s.io/apimachinery/pkg/runtime"
2830
"k8s.io/apimachinery/pkg/runtime/schema"
2931
"k8s.io/utils/pointer"
@@ -180,6 +182,10 @@ const ( // They should complete the sentence "Deployment default/foo has been ..
180182
OperationResultCreated OperationResult = "created"
181183
// OperationResultUpdated means that an existing resource is updated
182184
OperationResultUpdated OperationResult = "updated"
185+
// OperationResultUpdatedStatus means that an existing resource and its status is updated
186+
OperationResultUpdatedStatus OperationResult = "updatedStatus"
187+
// OperationResultUpdatedStatusOnly means that only an existing status is updated
188+
OperationResultUpdatedStatusOnly OperationResult = "updatedStatusOnly"
183189
)
184190

185191
// CreateOrUpdate creates or updates the given object in the Kubernetes
@@ -223,6 +229,108 @@ func CreateOrUpdate(ctx context.Context, c client.Client, obj runtime.Object, f
223229
return OperationResultUpdated, nil
224230
}
225231

232+
// CreateOrPatch creates or patches the given object in the Kubernetes
233+
// cluster. The object's desired state must be reconciled with the before
234+
// state inside the passed in callback MutateFn.
235+
//
236+
// The MutateFn is called regardless of creating or updating an object.
237+
//
238+
// It returns the executed operation and an error.
239+
func CreateOrPatch(ctx context.Context, c client.Client, obj runtime.Object, f MutateFn) (OperationResult, error) {
240+
key, err := client.ObjectKeyFromObject(obj)
241+
if err != nil {
242+
return OperationResultNone, err
243+
}
244+
245+
if err := c.Get(ctx, key, obj); err != nil {
246+
if !errors.IsNotFound(err) {
247+
return OperationResultNone, err
248+
}
249+
if f != nil {
250+
if err := mutate(f, key, obj); err != nil {
251+
return OperationResultNone, err
252+
}
253+
}
254+
if err := c.Create(ctx, obj); err != nil {
255+
return OperationResultNone, err
256+
}
257+
return OperationResultCreated, nil
258+
}
259+
260+
// Create patches for the object and its possible status.
261+
objPatch := client.MergeFrom(obj.DeepCopyObject())
262+
statusPatch := client.MergeFrom(obj.DeepCopyObject())
263+
264+
// Create a copy of the original object as well as converting that copy to
265+
// unstructured data.
266+
before, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj.DeepCopyObject())
267+
if err != nil {
268+
return OperationResultNone, err
269+
}
270+
271+
// Attempt to extract the status from the resource for easier comparison later
272+
beforeStatus, hasBeforeStatus, err := unstructured.NestedFieldCopy(before, "status")
273+
if err != nil {
274+
return OperationResultNone, err
275+
}
276+
277+
// If the resource contains a status then remove it from the unstructured
278+
// copy to avoid unnecessary patching later.
279+
if hasBeforeStatus {
280+
unstructured.RemoveNestedField(before, "status")
281+
}
282+
283+
// Mutate the original object.
284+
if f != nil {
285+
if err := mutate(f, key, obj); err != nil {
286+
return OperationResultNone, err
287+
}
288+
}
289+
290+
// Convert the resource to unstructured to compare against our before copy.
291+
after, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
292+
if err != nil {
293+
return OperationResultNone, err
294+
}
295+
296+
// Attempt to extract the status from the resource for easier comparison later
297+
afterStatus, hasAfterStatus, err := unstructured.NestedFieldCopy(after, "status")
298+
if err != nil {
299+
return OperationResultNone, err
300+
}
301+
302+
// If the resource contains a status then remove it from the unstructured
303+
// copy to avoid unnecessary patching later.
304+
if hasAfterStatus {
305+
unstructured.RemoveNestedField(after, "status")
306+
}
307+
308+
result := OperationResultNone
309+
310+
if !reflect.DeepEqual(before, after) {
311+
// Only issue a Patch if the before and after resources (minus status) differ
312+
if err := c.Patch(ctx, obj, objPatch); err != nil {
313+
return result, err
314+
}
315+
result = OperationResultUpdated
316+
}
317+
318+
if (hasBeforeStatus || hasAfterStatus) && !reflect.DeepEqual(beforeStatus, afterStatus) {
319+
// Only issue a Status Patch if the resource has a status and the beforeStatus
320+
// and afterStatus copies differ
321+
if err := c.Status().Patch(ctx, obj, statusPatch); err != nil {
322+
return result, err
323+
}
324+
if result == OperationResultUpdated {
325+
result = OperationResultUpdatedStatus
326+
} else {
327+
result = OperationResultUpdatedStatusOnly
328+
}
329+
}
330+
331+
return result, nil
332+
}
333+
226334
// mutate wraps a MutateFn and applies validation to its result
227335
func mutate(f MutateFn, key client.ObjectKey, obj runtime.Object) error {
228336
if err := f(); err != nil {

pkg/controller/controllerutil/controllerutil_test.go

+196
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,195 @@ var _ = Describe("Controllerutil", func() {
406406
})
407407
})
408408

409+
Describe("CreateOrPatch", func() {
410+
var deploy *appsv1.Deployment
411+
var deplSpec appsv1.DeploymentSpec
412+
var deplKey types.NamespacedName
413+
var specr controllerutil.MutateFn
414+
415+
BeforeEach(func() {
416+
deploy = &appsv1.Deployment{
417+
ObjectMeta: metav1.ObjectMeta{
418+
Name: fmt.Sprintf("deploy-%d", rand.Int31()),
419+
Namespace: "default",
420+
},
421+
}
422+
423+
deplSpec = appsv1.DeploymentSpec{
424+
Selector: &metav1.LabelSelector{
425+
MatchLabels: map[string]string{"foo": "bar"},
426+
},
427+
Template: corev1.PodTemplateSpec{
428+
ObjectMeta: metav1.ObjectMeta{
429+
Labels: map[string]string{
430+
"foo": "bar",
431+
},
432+
},
433+
Spec: corev1.PodSpec{
434+
Containers: []corev1.Container{
435+
{
436+
Name: "busybox",
437+
Image: "busybox",
438+
},
439+
},
440+
},
441+
},
442+
}
443+
444+
deplKey = types.NamespacedName{
445+
Name: deploy.Name,
446+
Namespace: deploy.Namespace,
447+
}
448+
449+
specr = deploymentSpecr(deploy, deplSpec)
450+
})
451+
452+
It("creates a new object if one doesn't exists", func() {
453+
op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
454+
455+
By("returning no error")
456+
Expect(err).NotTo(HaveOccurred())
457+
458+
By("returning OperationResultCreated")
459+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
460+
461+
By("actually having the deployment created")
462+
fetched := &appsv1.Deployment{}
463+
Expect(c.Get(context.TODO(), deplKey, fetched)).To(Succeed())
464+
465+
By("being mutated by MutateFn")
466+
Expect(fetched.Spec.Template.Spec.Containers).To(HaveLen(1))
467+
Expect(fetched.Spec.Template.Spec.Containers[0].Name).To(Equal(deplSpec.Template.Spec.Containers[0].Name))
468+
Expect(fetched.Spec.Template.Spec.Containers[0].Image).To(Equal(deplSpec.Template.Spec.Containers[0].Image))
469+
})
470+
471+
It("patches existing object", func() {
472+
var scale int32 = 2
473+
op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
474+
Expect(err).NotTo(HaveOccurred())
475+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
476+
477+
op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentScaler(deploy, scale))
478+
By("returning no error")
479+
Expect(err).NotTo(HaveOccurred())
480+
481+
By("returning OperationResultUpdated")
482+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultUpdated))
483+
484+
By("actually having the deployment scaled")
485+
fetched := &appsv1.Deployment{}
486+
Expect(c.Get(context.TODO(), deplKey, fetched)).To(Succeed())
487+
Expect(*fetched.Spec.Replicas).To(Equal(scale))
488+
})
489+
490+
It("patches only changed objects", func() {
491+
op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
492+
493+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
494+
Expect(err).NotTo(HaveOccurred())
495+
496+
op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentIdentity)
497+
By("returning no error")
498+
Expect(err).NotTo(HaveOccurred())
499+
500+
By("returning OperationResultNone")
501+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
502+
})
503+
504+
It("patches only changed status", func() {
505+
op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
506+
507+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
508+
Expect(err).NotTo(HaveOccurred())
509+
510+
deployStatus := appsv1.DeploymentStatus{
511+
ReadyReplicas: 1,
512+
Replicas: 3,
513+
}
514+
op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentStatusr(deploy, deployStatus))
515+
By("returning no error")
516+
Expect(err).NotTo(HaveOccurred())
517+
518+
By("returning OperationResultUpdatedStatusOnly")
519+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultUpdatedStatusOnly))
520+
})
521+
522+
It("patches resource and status", func() {
523+
op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
524+
525+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
526+
Expect(err).NotTo(HaveOccurred())
527+
528+
replicas := int32(3)
529+
deployStatus := appsv1.DeploymentStatus{
530+
ReadyReplicas: 1,
531+
Replicas: replicas,
532+
}
533+
op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, func() error {
534+
Expect(deploymentScaler(deploy, replicas)()).To(Succeed())
535+
return deploymentStatusr(deploy, deployStatus)()
536+
})
537+
By("returning no error")
538+
Expect(err).NotTo(HaveOccurred())
539+
540+
By("returning OperationResultUpdatedStatus")
541+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultUpdatedStatus))
542+
})
543+
544+
It("errors when MutateFn changes object name on creation", func() {
545+
op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, func() error {
546+
Expect(specr()).To(Succeed())
547+
return deploymentRenamer(deploy)()
548+
})
549+
550+
By("returning error")
551+
Expect(err).To(HaveOccurred())
552+
553+
By("returning OperationResultNone")
554+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
555+
})
556+
557+
It("errors when MutateFn renames an object", func() {
558+
op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
559+
560+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
561+
Expect(err).NotTo(HaveOccurred())
562+
563+
op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentRenamer(deploy))
564+
565+
By("returning error")
566+
Expect(err).To(HaveOccurred())
567+
568+
By("returning OperationResultNone")
569+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
570+
})
571+
572+
It("errors when object namespace changes", func() {
573+
op, err := controllerutil.CreateOrPatch(context.TODO(), c, deploy, specr)
574+
575+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
576+
Expect(err).NotTo(HaveOccurred())
577+
578+
op, err = controllerutil.CreateOrPatch(context.TODO(), c, deploy, deploymentNamespaceChanger(deploy))
579+
580+
By("returning error")
581+
Expect(err).To(HaveOccurred())
582+
583+
By("returning OperationResultNone")
584+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
585+
})
586+
587+
It("aborts immediately if there was an error initially retrieving the object", func() {
588+
op, err := controllerutil.CreateOrPatch(context.TODO(), errorReader{c}, deploy, func() error {
589+
Fail("Mutation method should not run")
590+
return nil
591+
})
592+
593+
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
594+
Expect(err).To(HaveOccurred())
595+
})
596+
})
597+
409598
Describe("Finalizers", func() {
410599
var obj runtime.Object = &errRuntimeObj{}
411600
var deploy *appsv1.Deployment
@@ -473,6 +662,13 @@ func deploymentSpecr(deploy *appsv1.Deployment, spec appsv1.DeploymentSpec) cont
473662
}
474663
}
475664

665+
func deploymentStatusr(deploy *appsv1.Deployment, status appsv1.DeploymentStatus) controllerutil.MutateFn {
666+
return func() error {
667+
deploy.Status = status
668+
return nil
669+
}
670+
}
671+
476672
var deploymentIdentity controllerutil.MutateFn = func() error {
477673
return nil
478674
}

0 commit comments

Comments
 (0)