Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

✨ custom equalities in CreateOrUpdate #2673

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+32 −1
Open

✨ custom equalities in CreateOrUpdate #2673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion pkg/controller/controllerutil/controllerutil.go
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/conversion"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/utils/ptr"
Expand Down Expand Up @@ -274,6 +275,11 @@ const ( // They should complete the sentence "Deployment default/foo has been ..
// Note: changes made by MutateFn to any sub-resource (status...), will be
// discarded.
func CreateOrUpdate(ctx context.Context, c client.Client, obj client.Object, f MutateFn) (OperationResult, error) {
return CreateOrUpdateWithEqualities(ctx, c, obj, equality.Semantic, f)
}

// CreateOrUpdateWithEqualities is like CreateOrUpdate but allows customizing the equality check.
func CreateOrUpdateWithEqualities(ctx context.Context, c client.Client, obj client.Object, eq conversion.Equalities, f MutateFn) (OperationResult, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively maybe add a variadic opts arg to CreateOrUpdate that allows setting a custom comparison?

Also, wouldn't it be better to use a function type for this rather than a single-method interface, given that functions can be implemented inline and interfaces can not?

key := client.ObjectKeyFromObject(obj)
if err := c.Get(ctx, key, obj); err != nil {
if !apierrors.IsNotFound(err) {
Expand All @@ -293,7 +299,7 @@ func CreateOrUpdate(ctx context.Context, c client.Client, obj client.Object, f M
return OperationResultNone, err
}

if equality.Semantic.DeepEqual(existing, obj) {
if eq.DeepEqual(existing, obj) {
return OperationResultNone, nil
}

Expand Down
25 changes: 25 additions & 0 deletions pkg/controller/controllerutil/controllerutil_test.go
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -507,6 +508,30 @@ var _ = Describe("Controllerutil", func() {
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
})

It("uses custom equality check as-is", func() {
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy, specr)

Expect(op).To(BeEquivalentTo(controllerutil.OperationResultCreated))
Expect(err).NotTo(HaveOccurred())

eq := equality.Semantic.Copy()
eq.AddFunc(func(a, b appsv1.DeploymentStrategy) bool {
// intentionallity ignore and return true
return true
})
op, err = controllerutil.CreateOrUpdateWithEqualities(context.TODO(), c, deploy, eq, func() error {
deploy.Spec.Strategy = appsv1.DeploymentStrategy{
Type: appsv1.RecreateDeploymentStrategyType, // just any change
}
return nil
})
By("returning no error")
Expect(err).NotTo(HaveOccurred())

By("returning OperationResultNone")
Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone))
})

It("errors when MutateFn changes object name on creation", func() {
op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy, func() error {
Expect(specr()).To(Succeed())
Expand Down