diff --git a/pkg/controller/controllerutil/controllerutil.go b/pkg/controller/controllerutil/controllerutil.go index d918eeaa42..3edc98e541 100644 --- a/pkg/controller/controllerutil/controllerutil.go +++ b/pkg/controller/controllerutil/controllerutil.go @@ -121,57 +121,42 @@ const ( // They should complete the sentence "Deployment default/foo has been .. // // It returns the executed operation and an error. func CreateOrUpdate(ctx context.Context, c client.Client, obj runtime.Object, f MutateFn) (OperationResult, error) { - // op is the operation we are going to attempt - op := OperationResultNone - - // get the existing object meta - metaObj, ok := obj.(v1.Object) - if !ok { - return OperationResultNone, fmt.Errorf("%T does not implement metav1.Object interface", obj) + key, err := client.ObjectKeyFromObject(obj) + if err != nil { + return OperationResultNone, err } - // retrieve the existing object - key := client.ObjectKey{ - Name: metaObj.GetName(), - Namespace: metaObj.GetNamespace(), + if err := c.Get(ctx, key, obj); err != nil { + if errors.IsNotFound(err) { + if err := c.Create(ctx, obj); err != nil { + return OperationResultNone, err + } + return OperationResultCreated, nil + } + return OperationResultNone, err } - err := c.Get(ctx, key, obj) - // reconcile the existing object existing := obj.DeepCopyObject() - existingObjMeta := existing.(v1.Object) - existingObjMeta.SetName(metaObj.GetName()) - existingObjMeta.SetNamespace(metaObj.GetNamespace()) - - if e := f(obj); e != nil { - return OperationResultNone, e - } - - if metaObj.GetName() != existingObjMeta.GetName() { - return OperationResultNone, fmt.Errorf("ReconcileFn cannot mutate objects name") + if err := f(obj); err != nil { + return OperationResultNone, err } - if metaObj.GetNamespace() != existingObjMeta.GetNamespace() { - return OperationResultNone, fmt.Errorf("ReconcileFn cannot mutate objects namespace") + if reflect.DeepEqual(existing, obj) { + return OperationResultNone, nil } - if errors.IsNotFound(err) { - err = c.Create(ctx, obj) - op = OperationResultCreated - } else if err == nil { - if reflect.DeepEqual(existing, obj) { - return OperationResultNone, nil - } - err = c.Update(ctx, obj) - op = OperationResultUpdated - } else { + newKey, err := client.ObjectKeyFromObject(obj) + if err != nil { return OperationResultNone, err } + if key != newKey { + return OperationResultNone, fmt.Errorf("MutateFn cannot mutate object namespace and/or object name") + } - if err != nil { - op = OperationResultNone + if err := c.Update(ctx, obj); err != nil { + return OperationResultNone, err } - return op, err + return OperationResultUpdated, nil } // MutateFn is a function which mutates the existing object into it's desired state. diff --git a/pkg/controller/controllerutil/controllerutil_test.go b/pkg/controller/controllerutil/controllerutil_test.go index 89bde6329b..b30f83b170 100644 --- a/pkg/controller/controllerutil/controllerutil_test.go +++ b/pkg/controller/controllerutil/controllerutil_test.go @@ -21,6 +21,8 @@ import ( "fmt" "math/rand" + "sigs.k8s.io/controller-runtime/pkg/client" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" @@ -232,6 +234,16 @@ var _ = Describe("Controllerutil", func() { By("returning error") Expect(err).To(HaveOccurred()) }) + + It("aborts immediately if there was an error initially retrieving the object", func() { + op, err := controllerutil.CreateOrUpdate(context.TODO(), errorReader{c}, deploy, func(_ runtime.Object) error { + Fail("Mutation method should not run") + return nil + }) + + Expect(op).To(BeEquivalentTo(controllerutil.OperationResultNone)) + Expect(err).To(HaveOccurred()) + }) }) }) @@ -273,3 +285,11 @@ func deploymentScaler(replicas int32) controllerutil.MutateFn { } return fn } + +type errorReader struct { + client.Client +} + +func (e errorReader) Get(ctx context.Context, key client.ObjectKey, into runtime.Object) error { + return fmt.Errorf("unexpected error") +}