Skip to content

Commit 79a5fd1

Browse files
adracusadracus
committed
Breaking: CreateOrUse -> CreateOrUseAndPatch
`CreateOrUse` only satisfied the use case to use an existing object. In case we need to update the existing object when it matches, the new function now patches the used object if the mutation yielded changes. Updated test cases and removed variant methods. Everybody should use the basic `CreateOrUseAndPatch` function accepting `[]client.Object`.
1 parent 2a9ec2c commit 79a5fd1

File tree

2 files changed

+65
-182
lines changed

2 files changed

+65
-182
lines changed

‎clientutils/clientutils.go‎

Lines changed: 37 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
"github.com/onmetal/controller-utils/metautils"
2525
"github.com/onmetal/controller-utils/unstructuredutils"
26+
"k8s.io/apimachinery/pkg/api/equality"
2627
apierrors "k8s.io/apimachinery/pkg/api/errors"
2728
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2829
"k8s.io/apimachinery/pkg/conversion"
@@ -447,23 +448,19 @@ func setObject(dst, src client.Object) error {
447448
return nil
448449
}
449450

450-
// CreateOrUseOperationResult is the result of a CreateOrUse call.
451-
type CreateOrUseOperationResult string
452-
453-
const (
454-
// CreateOrUseOperationResultUsed indicates that a match has been found and no object was created.
455-
CreateOrUseOperationResultUsed CreateOrUseOperationResult = "used"
456-
// CreateOrUseOperationResultCreated indicates that no match has been found and a new object was created.
457-
CreateOrUseOperationResultCreated CreateOrUseOperationResult = "created"
458-
// CreateOrUseOperationResultNone indicates that no match has been found / no object was created.
459-
CreateOrUseOperationResultNone CreateOrUseOperationResult = "unchanged"
460-
)
461-
462-
// CreateOrUse traverses through a slice of objects and tries to find a matching object using matchFunc.
463-
// If it does, the matching object is set to the object and the function returns the other objects.
464-
// If it matches multiple times, the winning object is the oldest.
465-
// If it does not match, initFunc is called and the new object is created.
466-
func CreateOrUse(ctx context.Context, c client.Client, objects []client.Object, obj client.Object, matchFunc func() (bool, error), initFunc func() error) (CreateOrUseOperationResult, []client.Object, error) {
451+
// CreateOrUseAndPatch traverses through a slice of objects and tries to find a matching object using matchFunc.
452+
// If it does, the matching object is set to the object, optionally patched and returned.
453+
// If multiple objects match, the winning object is the oldest.
454+
// If no object matches, initFunc is called and the new object is created.
455+
// mutateFunc is optional, if none is specified no mutation will happen.
456+
func CreateOrUseAndPatch(
457+
ctx context.Context,
458+
c client.Client,
459+
objects []client.Object,
460+
obj client.Object,
461+
matchFunc func() (bool, error),
462+
mutateFunc func() error,
463+
) (controllerutil.OperationResult, []client.Object, error) {
467464
var (
468465
base = obj.DeepCopyObject().(client.Object)
469466
best client.Object
@@ -472,12 +469,12 @@ func CreateOrUse(ctx context.Context, c client.Client, objects []client.Object,
472469
for _, object := range objects {
473470
object := object
474471
if err := setObject(obj, object); err != nil {
475-
return CreateOrUseOperationResultNone, nil, err
472+
return controllerutil.OperationResultNone, nil, err
476473
}
477474

478475
match, err := matchFunc()
479476
if err != nil {
480-
return CreateOrUseOperationResultNone, nil, err
477+
return controllerutil.OperationResultNone, nil, err
481478
}
482479

483480
switch {
@@ -494,57 +491,36 @@ func CreateOrUse(ctx context.Context, c client.Client, objects []client.Object,
494491
}
495492
if best != nil {
496493
if err := setObject(obj, best); err != nil {
497-
return CreateOrUseOperationResultNone, nil, err
494+
return controllerutil.OperationResultNone, nil, err
495+
}
496+
baseObj := obj.DeepCopyObject().(client.Object)
497+
if mutateFunc != nil {
498+
if err := mutateFunc(); err != nil {
499+
return controllerutil.OperationResultNone, nil, err
500+
}
501+
}
502+
if equality.Semantic.DeepEqual(baseObj, obj) {
503+
return controllerutil.OperationResultNone, other, nil
498504
}
499-
return CreateOrUseOperationResultUsed, other, nil
505+
506+
if err := c.Patch(ctx, obj, client.MergeFrom(baseObj)); err != nil {
507+
return controllerutil.OperationResultNone, nil, err
508+
}
509+
return controllerutil.OperationResultUpdated, other, nil
500510
}
501511

502512
if err := setObject(obj, base); err != nil {
503-
return CreateOrUseOperationResultNone, nil, err
513+
return controllerutil.OperationResultNone, nil, err
504514
}
505-
if initFunc != nil {
506-
if err := initFunc(); err != nil {
507-
return CreateOrUseOperationResultNone, nil, err
515+
if mutateFunc != nil {
516+
if err := mutateFunc(); err != nil {
517+
return controllerutil.OperationResultNone, nil, err
508518
}
509519
}
510520
if err := c.Create(ctx, obj); err != nil {
511-
return CreateOrUseOperationResultNone, nil, err
512-
}
513-
return CreateOrUseOperationResultCreated, other, nil
514-
}
515-
516-
// CreateOrUseWithList is a shorthand for using CreateOrUse with a client.ObjectList containing the objects.
517-
// Caution: In contrast to CreateOrUse, if setting the list elements fails, the unmatched objects are unknown.
518-
// The operation result will still reflect what happened.
519-
func CreateOrUseWithList(ctx context.Context, c client.Client, list client.ObjectList, obj client.Object, matchFunc func() (bool, error), initFunc func() error) (CreateOrUseOperationResult, error) {
520-
items, err := metautils.ExtractList(list)
521-
if err != nil {
522-
return CreateOrUseOperationResultNone, err
523-
}
524-
525-
res, items, err := CreateOrUse(ctx, c, items, obj, matchFunc, initFunc)
526-
if err != nil {
527-
return res, err
521+
return controllerutil.OperationResultNone, nil, err
528522
}
529-
530-
return res, metautils.SetList(list, items)
531-
}
532-
533-
// CreateOrUseWithObjectSlicePointer is a shorthand for using CreateOrUse with an object slice containing the objects.
534-
// Caution: In contrast to CreateOrUse, if setting the slice elements fails, the unmatched objects are unknown.
535-
// The operation result will still reflect what happened.
536-
func CreateOrUseWithObjectSlicePointer(ctx context.Context, c client.Client, slicePtr interface{}, obj client.Object, matchFunc func() (bool, error), initFunc func() error) (CreateOrUseOperationResult, error) {
537-
items, err := metautils.ExtractObjectSlicePointer(slicePtr)
538-
if err != nil {
539-
return CreateOrUseOperationResultNone, err
540-
}
541-
542-
res, items, err := CreateOrUse(ctx, c, items, obj, matchFunc, initFunc)
543-
if err != nil {
544-
return res, err
545-
}
546-
547-
return res, metautils.SetObjectSlice(slicePtr, items)
523+
return controllerutil.OperationResultCreated, other, nil
548524
}
549525

550526
// DeleteIfExists deletes the given object, if it exists. It returns any non apierrors.IsNotFound error

‎clientutils/clientutils_test.go‎

Lines changed: 28 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"errors"
2020
"fmt"
21+
"reflect"
2122
"strings"
2223

2324
"github.com/golang/mock/gomock"
@@ -642,7 +643,7 @@ var _ = Describe("Clientutils", func() {
642643
})
643644
})
644645

645-
Describe("CreateOrUse", func() {
646+
Describe("CreateOrUseAndPatch", func() {
646647
var (
647648
cm1, cm2, cm3 corev1.ConfigMap
648649
)
@@ -667,154 +668,60 @@ var _ = Describe("Clientutils", func() {
667668
}
668669
})
669670

670-
It("should use an object if it matches", func() {
671-
cm := &corev1.ConfigMap{}
672-
res, other, err := CreateOrUse(ctx, c, []client.Object{&cm1, &cm2, &cm3}, cm, func() (bool, error) {
673-
return cm.Name == "n3", nil
674-
}, func() error {
675-
Fail("init should not be called")
676-
return nil
677-
})
671+
It("should use an object if it matches and patch it when it's mutated", func() {
672+
annotations := map[string]string{"foo": "bar"}
673+
withoutAnnotations := &corev1.ConfigMap{}
674+
withAnnotations := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Annotations: annotations}}
675+
expectedPatchData, err := client.MergeFrom(withoutAnnotations).Data(withAnnotations)
678676
Expect(err).NotTo(HaveOccurred())
679-
Expect(other).To(Equal([]client.Object{&cm1, &cm2}))
680-
Expect(res).To(Equal(CreateOrUseOperationResultUsed))
681-
Expect(cm).To(Equal(&cm3))
682-
})
683677

684-
It("should create a new object if it does not match", func() {
685-
cm := &corev1.ConfigMap{}
686-
c.EXPECT().Create(ctx, cm)
687-
res, other, err := CreateOrUse(ctx, c, []client.Object{&cm1, &cm2, &cm3}, cm, func() (bool, error) {
688-
return false, nil
689-
}, func() error {
690-
cm.Name = "n4"
691-
return nil
692-
})
693-
Expect(err).NotTo(HaveOccurred())
694-
Expect(other).To(Equal([]client.Object{&cm1, &cm2, &cm3}))
695-
Expect(res).To(Equal(CreateOrUseOperationResultCreated))
696-
Expect(cm).To(Equal(&corev1.ConfigMap{
697-
ObjectMeta: metav1.ObjectMeta{
698-
Name: "n4",
699-
},
700-
}))
701-
})
702-
})
703-
704-
Describe("CreateOrUseWithList", func() {
705-
var (
706-
cm1, cm2, cm3 corev1.ConfigMap
707-
)
708-
BeforeEach(func() {
709-
cm1 = corev1.ConfigMap{
710-
ObjectMeta: metav1.ObjectMeta{
711-
Namespace: "foo",
712-
Name: "n1",
713-
},
714-
}
715-
cm2 = corev1.ConfigMap{
716-
ObjectMeta: metav1.ObjectMeta{
717-
Namespace: "foo",
718-
Name: "n2",
719-
},
720-
}
721-
cm3 = corev1.ConfigMap{
722-
ObjectMeta: metav1.ObjectMeta{
723-
Namespace: "foo",
724-
Name: "n3",
725-
},
726-
}
727-
})
728-
729-
It("should use an object if it matches", func() {
678+
c.EXPECT().Patch(ctx, gomock.AssignableToTypeOf(&corev1.ConfigMap{}), gomock.AssignableToTypeOf(reflect.TypeOf((*client.Patch)(nil)).Elem())).
679+
Do(func(_ context.Context, cm *corev1.ConfigMap, patch client.Patch, opts ...client.PatchOption) {
680+
Expect(patch.Data(cm)).To(Equal(expectedPatchData))
681+
cm.Annotations = annotations
682+
})
730683
cm := &corev1.ConfigMap{}
731-
list := &corev1.ConfigMapList{Items: []corev1.ConfigMap{cm1, cm2, cm3}}
732-
res, err := CreateOrUseWithList(ctx, c, list, cm, func() (bool, error) {
684+
res, other, err := CreateOrUseAndPatch(ctx, c, []client.Object{&cm1, &cm2, &cm3}, cm, func() (bool, error) {
733685
return cm.Name == "n3", nil
734686
}, func() error {
735-
Fail("init should not be called")
687+
cm.Annotations = annotations
736688
return nil
737689
})
738690
Expect(err).NotTo(HaveOccurred())
739-
Expect(list.Items).To(Equal([]corev1.ConfigMap{cm1, cm2}))
740-
Expect(res).To(Equal(CreateOrUseOperationResultUsed))
741-
Expect(cm).To(Equal(&cm3))
742-
})
743-
744-
It("should create a new object if it does not match", func() {
745-
cm := &corev1.ConfigMap{}
746-
list := &corev1.ConfigMapList{Items: []corev1.ConfigMap{cm1, cm2, cm3}}
747-
c.EXPECT().Create(ctx, cm)
748-
res, err := CreateOrUseWithList(ctx, c, list, cm, func() (bool, error) {
749-
return false, nil
750-
}, func() error {
751-
cm.Name = "n4"
752-
return nil
753-
})
754-
Expect(err).NotTo(HaveOccurred())
755-
Expect(list.Items).To(Equal([]corev1.ConfigMap{cm1, cm2, cm3}))
756-
Expect(res).To(Equal(CreateOrUseOperationResultCreated))
691+
Expect(other).To(Equal([]client.Object{&cm1, &cm2}))
692+
Expect(res).To(Equal(controllerutil.OperationResultUpdated))
757693
Expect(cm).To(Equal(&corev1.ConfigMap{
758694
ObjectMeta: metav1.ObjectMeta{
759-
Name: "n4",
695+
Namespace: "foo",
696+
Name: "n3",
697+
Annotations: annotations,
760698
},
761699
}))
762700
})
763-
})
764-
765-
Describe("CreateOrUseWithObjectSlicePointer", func() {
766-
var (
767-
cm1, cm2, cm3 corev1.ConfigMap
768-
slice []corev1.ConfigMap
769-
)
770-
BeforeEach(func() {
771-
cm1 = corev1.ConfigMap{
772-
ObjectMeta: metav1.ObjectMeta{
773-
Namespace: "foo",
774-
Name: "n1",
775-
},
776-
}
777-
cm2 = corev1.ConfigMap{
778-
ObjectMeta: metav1.ObjectMeta{
779-
Namespace: "foo",
780-
Name: "n2",
781-
},
782-
}
783-
cm3 = corev1.ConfigMap{
784-
ObjectMeta: metav1.ObjectMeta{
785-
Namespace: "foo",
786-
Name: "n3",
787-
},
788-
}
789-
slice = []corev1.ConfigMap{cm1, cm2, cm3}
790-
})
791701

792-
It("should use an object if it matches", func() {
702+
It("should use an object without updating it if it's mutation semantically equals its original", func() {
793703
cm := &corev1.ConfigMap{}
794-
res, err := CreateOrUseWithObjectSlicePointer(ctx, c, &slice, cm, func() (bool, error) {
704+
res, other, err := CreateOrUseAndPatch(ctx, c, []client.Object{&cm1, &cm2, &cm3}, cm, func() (bool, error) {
795705
return cm.Name == "n3", nil
796-
}, func() error {
797-
Fail("init should not be called")
798-
return nil
799-
})
706+
}, nil)
800707
Expect(err).NotTo(HaveOccurred())
801-
Expect(slice).To(Equal([]corev1.ConfigMap{cm1, cm2}))
802-
Expect(res).To(Equal(CreateOrUseOperationResultUsed))
708+
Expect(other).To(Equal([]client.Object{&cm1, &cm2}))
709+
Expect(res).To(Equal(controllerutil.OperationResultNone))
803710
Expect(cm).To(Equal(&cm3))
804711
})
805712

806-
It("should create a new object if it does not match", func() {
713+
It("should create a new object if none matches", func() {
807714
cm := &corev1.ConfigMap{}
808715
c.EXPECT().Create(ctx, cm)
809-
res, err := CreateOrUseWithObjectSlicePointer(ctx, c, &slice, cm, func() (bool, error) {
716+
res, other, err := CreateOrUseAndPatch(ctx, c, []client.Object{&cm1, &cm2, &cm3}, cm, func() (bool, error) {
810717
return false, nil
811718
}, func() error {
812719
cm.Name = "n4"
813720
return nil
814721
})
815722
Expect(err).NotTo(HaveOccurred())
816-
Expect(slice).To(Equal([]corev1.ConfigMap{cm1, cm2, cm3}))
817-
Expect(res).To(Equal(CreateOrUseOperationResultCreated))
723+
Expect(other).To(Equal([]client.Object{&cm1, &cm2, &cm3}))
724+
Expect(res).To(Equal(controllerutil.OperationResultCreated))
818725
Expect(cm).To(Equal(&corev1.ConfigMap{
819726
ObjectMeta: metav1.ObjectMeta{
820727
Name: "n4",

0 commit comments

Comments
 (0)