diff --git a/pkg/controller/api.go b/pkg/controller/api.go index f8efda9b..eb599409 100644 --- a/pkg/controller/api.go +++ b/pkg/controller/api.go @@ -27,6 +27,13 @@ const ( errReconcileRequestFmt = "cannot request the reconciliation of the resource %s/%s after an async %s" ) +// crossplane-runtime error constants +const ( + errXPReconcileCreate = "create failed" + errXPReconcileUpdate = "update failed" + errXPReconcileDelete = "delete failed" +) + const ( rateLimiterCallback = "asyncCallback" ) @@ -119,6 +126,20 @@ func (ac *APICallbacks) callbackFn(name, op string) terraform.CallbackFn { // to do so. So we keep the `LastAsyncOperation` condition. // TODO: move this to the `Synced` condition. tr.SetConditions(resource.LastAsyncOperationCondition(err)) + if err != nil { + wrapMsg := "" + switch op { + case "create": + wrapMsg = errXPReconcileCreate + case "update": + wrapMsg = errXPReconcileUpdate + case "destroy": + wrapMsg = errXPReconcileDelete + } + tr.SetConditions(xpv1.ReconcileError(errors.Wrap(err, wrapMsg))) + } else { + tr.SetConditions(xpv1.ReconcileSuccess()) + } if ac.enableStatusUpdates { tr.SetConditions(resource.AsyncOperationFinishedCondition()) } diff --git a/pkg/controller/external_async_tfpluginfw.go b/pkg/controller/external_async_tfpluginfw.go index 221749f8..a90f6787 100644 --- a/pkg/controller/external_async_tfpluginfw.go +++ b/pkg/controller/external_async_tfpluginfw.go @@ -7,6 +7,7 @@ package controller import ( "context" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" @@ -116,7 +117,7 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Observe(ctx context.Contex ResourceUpToDate: true, }, nil } - n.opTracker.LastOperation.Flush() + n.opTracker.LastOperation.Clear(true) o, err := n.terraformPluginFrameworkExternalClient.Observe(ctx, mg) // clear any previously reported LastAsyncOperation error condition here, @@ -124,6 +125,8 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Observe(ctx context.Contex // not scheduled to be deleted. if err == nil && o.ResourceExists && o.ResourceUpToDate && !meta.WasDeleted(mg) { mg.(resource.Terraformed).SetConditions(resource.LastAsyncOperationCondition(nil)) + mg.(resource.Terraformed).SetConditions(xpv1.ReconcileSuccess()) + n.opTracker.LastOperation.Clear(false) } return o, err } @@ -149,7 +152,7 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Create(_ context.Context, } }() - return managed.ExternalCreation{}, nil + return managed.ExternalCreation{}, n.opTracker.LastOperation.Error() } func (n *terraformPluginFrameworkAsyncExternalClient) Update(_ context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { @@ -173,7 +176,7 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Update(_ context.Context, } }() - return managed.ExternalUpdate{}, nil + return managed.ExternalUpdate{}, n.opTracker.LastOperation.Error() } func (n *terraformPluginFrameworkAsyncExternalClient) Delete(_ context.Context, mg xpresource.Managed) error { @@ -200,5 +203,5 @@ func (n *terraformPluginFrameworkAsyncExternalClient) Delete(_ context.Context, } }() - return nil + return n.opTracker.LastOperation.Error() } diff --git a/pkg/controller/external_async_tfpluginsdk.go b/pkg/controller/external_async_tfpluginsdk.go index 2bf72ee6..2071c1c3 100644 --- a/pkg/controller/external_async_tfpluginsdk.go +++ b/pkg/controller/external_async_tfpluginsdk.go @@ -8,6 +8,7 @@ import ( "context" "time" + xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/logging" "github.com/crossplane/crossplane-runtime/pkg/meta" "github.com/crossplane/crossplane-runtime/pkg/reconciler/managed" @@ -121,7 +122,7 @@ func (n *terraformPluginSDKAsyncExternal) Observe(ctx context.Context, mg xpreso ResourceUpToDate: true, }, nil } - n.opTracker.LastOperation.Flush() + n.opTracker.LastOperation.Clear(true) o, err := n.terraformPluginSDKExternal.Observe(ctx, mg) // clear any previously reported LastAsyncOperation error condition here, @@ -129,6 +130,8 @@ func (n *terraformPluginSDKAsyncExternal) Observe(ctx context.Context, mg xpreso // not scheduled to be deleted. if err == nil && o.ResourceExists && o.ResourceUpToDate && !meta.WasDeleted(mg) { mg.(resource.Terraformed).SetConditions(resource.LastAsyncOperationCondition(nil)) + mg.(resource.Terraformed).SetConditions(xpv1.ReconcileSuccess()) + n.opTracker.LastOperation.Clear(false) } return o, err } @@ -154,7 +157,7 @@ func (n *terraformPluginSDKAsyncExternal) Create(_ context.Context, mg xpresourc } }() - return managed.ExternalCreation{}, nil + return managed.ExternalCreation{}, n.opTracker.LastOperation.Error() } func (n *terraformPluginSDKAsyncExternal) Update(_ context.Context, mg xpresource.Managed) (managed.ExternalUpdate, error) { @@ -178,7 +181,7 @@ func (n *terraformPluginSDKAsyncExternal) Update(_ context.Context, mg xpresourc } }() - return managed.ExternalUpdate{}, nil + return managed.ExternalUpdate{}, n.opTracker.LastOperation.Error() } func (n *terraformPluginSDKAsyncExternal) Delete(_ context.Context, mg xpresource.Managed) error { @@ -205,5 +208,5 @@ func (n *terraformPluginSDKAsyncExternal) Delete(_ context.Context, mg xpresourc } }() - return nil + return n.opTracker.LastOperation.Error() } diff --git a/pkg/terraform/operation.go b/pkg/terraform/operation.go index 3229d2fa..9057080e 100644 --- a/pkg/terraform/operation.go +++ b/pkg/terraform/operation.go @@ -43,14 +43,26 @@ func (o *Operation) MarkEnd() { o.endTime = &now } -// Flush cleans the operation information. +// Flush cleans the operation information including the registered error from +// the last reconciliation. +// Deprecated: Please use Clear, which allows optionally preserving the error +// from the last reconciliation to implement proper SYNC status condition for +// the asynchronous external clients. func (o *Operation) Flush() { + o.Clear(false) +} + +// Clear clears the operation information optionally preserving the last +// registered error from the last reconciliation. +func (o *Operation) Clear(preserveError bool) { o.mu.Lock() defer o.mu.Unlock() o.Type = "" o.startTime = nil o.endTime = nil - o.err = nil + if !preserveError { + o.err = nil + } } // IsEnded returns whether the operation has ended, regardless of its result. diff --git a/pkg/terraform/operation_test.go b/pkg/terraform/operation_test.go index 8700df2b..f33e376d 100644 --- a/pkg/terraform/operation_test.go +++ b/pkg/terraform/operation_test.go @@ -8,9 +8,11 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/pkg/errors" ) func TestOperation(t *testing.T) { + testErr := errors.New("test error") type args struct { calls func(o *Operation) } @@ -54,13 +56,46 @@ func TestOperation(t *testing.T) { args: args{ calls: func(o *Operation) { o.MarkStart("type") + o.SetError(testErr) o.MarkEnd() o.Flush() }, }, want: want{ checks: func(o *Operation) bool { - return o.Type == "" && o.startTime == nil && o.endTime == nil + return o.Type == "" && o.startTime == nil && o.endTime == nil && o.err == nil + }, + result: true, + }, + }, + "ClearedIncludingErrors": { + args: args{ + calls: func(o *Operation) { + o.MarkStart("type") + o.SetError(testErr) + o.MarkEnd() + o.Clear(false) + }, + }, + want: want{ + checks: func(o *Operation) bool { + return o.Type == "" && o.startTime == nil && o.endTime == nil && o.err == nil + }, + result: true, + }, + }, + "ClearedPreservingErrors": { + args: args{ + calls: func(o *Operation) { + o.MarkStart("type") + o.SetError(testErr) + o.MarkEnd() + o.Clear(true) + }, + }, + want: want{ + checks: func(o *Operation) bool { + return o.Type == "" && o.startTime == nil && o.endTime == nil && errors.Is(o.err, testErr) }, result: true, },