diff --git a/pkg/errors/reconcile.go b/pkg/errors/reconcile.go new file mode 100644 index 000000000..2417d642c --- /dev/null +++ b/pkg/errors/reconcile.go @@ -0,0 +1,49 @@ +/* +Copyright 2023 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package errors + +import ( + "context" + + kerrors "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// SilentlyRequeueOnConflict returns a requeue result and silently drops the +// error if it is a Kubernetes conflict error from the optimistic concurrency +// protocol. +func SilentlyRequeueOnConflict(result reconcile.Result, err error) (reconcile.Result, error) { + if kerrors.IsConflict(Cause(err)) { + return reconcile.Result{Requeue: true}, nil + } + return result, err +} + +// WithSilentRequeueOnConflict wraps a Reconciler and silently drops conflict +// errors and requeues instead. +func WithSilentRequeueOnConflict(r reconcile.Reconciler) reconcile.Reconciler { + return &silentlyRequeueOnConflict{Reconciler: r} +} + +type silentlyRequeueOnConflict struct { + reconcile.Reconciler +} + +func (r *silentlyRequeueOnConflict) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + result, err := r.Reconciler.Reconcile(ctx, req) + return SilentlyRequeueOnConflict(result, err) +} diff --git a/pkg/errors/reconcile_test.go b/pkg/errors/reconcile_test.go new file mode 100644 index 000000000..02b8241ee --- /dev/null +++ b/pkg/errors/reconcile_test.go @@ -0,0 +1,99 @@ +/* +Copyright 2023 The Crossplane Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package errors + +import ( + "testing" + "time" + + "github.com/google/go-cmp/cmp" + kerrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/crossplane/crossplane-runtime/pkg/test" +) + +func TestSilentlyRequeueOnConflict(t *testing.T) { + type args struct { + result reconcile.Result + err error + } + type want struct { + result reconcile.Result + err error + } + tests := []struct { + reason string + args args + want want + }{ + { + reason: "nil error", + args: args{ + result: reconcile.Result{RequeueAfter: time.Second}, + }, + want: want{ + result: reconcile.Result{RequeueAfter: time.Second}, + }, + }, + { + reason: "other error", + args: args{ + result: reconcile.Result{RequeueAfter: time.Second}, + err: New("some other error"), + }, + want: want{ + result: reconcile.Result{RequeueAfter: time.Second}, + err: New("some other error"), + }, + }, + { + reason: "conflict error", + args: args{ + result: reconcile.Result{RequeueAfter: time.Second}, + err: kerrors.NewConflict(schema.GroupResource{Group: "nature", Resource: "stones"}, "foo", New("nested error")), + }, + want: want{ + result: reconcile.Result{Requeue: true}, + }, + }, + { + reason: "nested conflict error", + args: args{ + result: reconcile.Result{RequeueAfter: time.Second}, + err: Wrap( + kerrors.NewConflict(schema.GroupResource{Group: "nature", Resource: "stones"}, "foo", New("nested error")), + "outer error"), + }, + want: want{ + result: reconcile.Result{Requeue: true}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.reason, func(t *testing.T) { + got, err := SilentlyRequeueOnConflict(tt.args.result, tt.args.err) + if diff := cmp.Diff(tt.want.err, err, test.EquateErrors()); diff != "" { + t.Errorf("\n%s\nIgnoreConflict(...): -want error, +got error:\n%s", tt.reason, diff) + } + if diff := cmp.Diff(tt.want.result, got); diff != "" { + t.Errorf("\n%s\nIgnoreConflict(...): -want result, +got result:\n%s", tt.reason, diff) + } + }) + } +} diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index 16d8a2373..4e1f63792 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -22,7 +22,6 @@ import ( "strings" "time" - kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-runtime/pkg/client" @@ -714,19 +713,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu // NOTE(negz): This method is a well over our cyclomatic complexity goal. // Be wary of adding additional complexity. - log := r.log.WithValues("request", req) - - defer func() { - if kerrors.IsConflict(errors.Cause(err)) { - // conflict errors are transient in Kubernetes and completely normal. - // The right reaction is to requeue, but not record the object as error'ing - // or creating events. - log.Debug("Transient conflict error", "error", err) - result.Requeue = true - err = nil - } - }() + defer func() { result, err = errors.SilentlyRequeueOnConflict(result, err) }() + log := r.log.WithValues("request", req) log.Debug("Reconciling") ctx, cancel := context.WithTimeout(ctx, r.timeout+reconcileGracePeriod)