Skip to content

Commit

Permalink
remove named returns; disconnect error should not requeue reconcile
Browse files Browse the repository at this point in the history
  • Loading branch information
vaspahomov committed Oct 28, 2021
1 parent bf53464 commit 8af1fca
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 17 deletions.
10 changes: 2 additions & 8 deletions pkg/reconciler/managed/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ func NewReconciler(m manager.Manager, of resource.ManagedKind, o ...ReconcilerOp
}

// Reconcile a managed resource with an external resource.
func (r *Reconciler) Reconcile(_ context.Context, req reconcile.Request) (result reconcile.Result, err error) { // nolint:gocyclo
func (r *Reconciler) Reconcile(_ context.Context, req reconcile.Request) (reconcile.Result, error) { // nolint:gocyclo
// NOTE(negz): This method is a well over our cyclomatic complexity goal.
// Be wary of adding additional complexity.

Expand Down Expand Up @@ -745,13 +745,7 @@ func (r *Reconciler) Reconcile(_ context.Context, req reconcile.Request) (result
if disconnectErr := r.external.Disconnect(ctx); disconnectErr != nil {
log.Debug("Cannot disconnect from provider", "error", disconnectErr)
record.Event(managed, event.Warning(reasonCannotDisconnect, disconnectErr))
disconnectErr = errors.Wrap(disconnectErr, errReconcileDisconnect)
managed.SetConditions(xpv1.ReconcileError(disconnectErr))
if err != nil {
err = errors.Wrap(err, disconnectErr.Error())
} else {
err = disconnectErr
}
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(disconnectErr, errReconcileDisconnect)))
}
}()

Expand Down
12 changes: 3 additions & 9 deletions pkg/reconciler/managed/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func TestReconciler(t *testing.T) {
want: want{result: reconcile.Result{Requeue: true}},
},
"ExternalDisconnectError": {
reason: "Errors disconnecting from the provider should return error.",
reason: "Error disconnecting from the provider should not trigger requeue.",
args: args{
m: &fake.Manager{
Client: &test.MockClient{
Expand Down Expand Up @@ -324,10 +324,7 @@ func TestReconciler(t *testing.T) {
WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}),
},
},
want: want{
result: reconcile.Result{RequeueAfter: defaultpollInterval},
err: errors.Wrap(errors.New("boom"), "disconnect failed"),
},
want: want{result: reconcile.Result{RequeueAfter: defaultpollInterval}},
},
"ExternalObserveErrorDisconnectError": {
reason: "Errors disconnecting from the provider after error observing the external resource should trigger a requeue after a short wait and return error.",
Expand Down Expand Up @@ -364,10 +361,7 @@ func TestReconciler(t *testing.T) {
),
},
},
want: want{
result: reconcile.Result{Requeue: true},
err: errors.Wrap(errors.New("boom"), "disconnect failed"),
},
want: want{result: reconcile.Result{Requeue: true}},
},
"ExternalObserveError": {
reason: "Errors observing the external resource should trigger a requeue after a short wait.",
Expand Down

0 comments on commit 8af1fca

Please sign in to comment.