From 4eeee423c20243c5e2fdbb62999bfc1b9085f584 Mon Sep 17 00:00:00 2001 From: Kumar Gaurav Sharma Date: Thu, 1 Jul 2021 20:45:59 -0700 Subject: [PATCH] Provide custom resource delete progress for long running delete --- mocks/pkg/types/aws_resource_manager.go | 19 ++++++++++++++----- pkg/runtime/reconciler.go | 16 ++++++++++++---- pkg/types/aws_resource_manager.go | 5 +++-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/mocks/pkg/types/aws_resource_manager.go b/mocks/pkg/types/aws_resource_manager.go index c3ccdea..c081352 100644 --- a/mocks/pkg/types/aws_resource_manager.go +++ b/mocks/pkg/types/aws_resource_manager.go @@ -55,17 +55,26 @@ func (_m *AWSResourceManager) Create(_a0 context.Context, _a1 types.AWSResource) } // Delete provides a mock function with given fields: _a0, _a1 -func (_m *AWSResourceManager) Delete(_a0 context.Context, _a1 types.AWSResource) error { +func (_m *AWSResourceManager) Delete(_a0 context.Context, _a1 types.AWSResource) (types.AWSResource, error) { ret := _m.Called(_a0, _a1) - var r0 error - if rf, ok := ret.Get(0).(func(context.Context, types.AWSResource) error); ok { + var r0 types.AWSResource + if rf, ok := ret.Get(0).(func(context.Context, types.AWSResource) types.AWSResource); ok { r0 = rf(_a0, _a1) } else { - r0 = ret.Error(0) + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.AWSResource) + } } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, types.AWSResource) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } // ReadOne provides a mock function with given fields: _a0, _a1 diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 4a1d886..5693c31 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -349,26 +349,34 @@ func (r *resourceReconciler) cleanup( defer exit(err) rlog.Enter("rm.ReadOne") - latest, err := rm.ReadOne(ctx, current) + observed, err := rm.ReadOne(ctx, current) rlog.Exit("rm.ReadOne", err) if err != nil { if err == ackerr.NotFound { // If the aws resource is not found, remove finalizer - return r.setResourceUnmanaged(ctx, latest) + return r.setResourceUnmanaged(ctx, current) } return err } rlog.Enter("rm.Delete") - err = rm.Delete(ctx, latest) + latest, err := rm.Delete(ctx, observed) rlog.Exit("rm.Delete", err) + if latest != nil { + // The Delete operation is likely asynchronous and has likely set a Status + // field on the returned CR to something like `deleting`. Here, we patchResource() + // in order to save these Status field modifications. + _ = r.patchResource(ctx, current, latest) + } if err != nil { + // NOTE: Delete() implementations that have asynchronously-completing + // deletions should return a RequeueNeededAfter. return err } // Now that external AWS service resources have been appropriately cleaned // up, we remove the finalizer representing the CR is managed by ACK, // allowing the CR to be deleted by the Kubernetes API server - if err = r.setResourceUnmanaged(ctx, latest); err != nil { + if err = r.setResourceUnmanaged(ctx, current); err != nil { return err } rlog.Info("deleted resource") diff --git a/pkg/types/aws_resource_manager.go b/pkg/types/aws_resource_manager.go index 620d810..bb6acfd 100644 --- a/pkg/types/aws_resource_manager.go +++ b/pkg/types/aws_resource_manager.go @@ -59,8 +59,9 @@ type AWSResourceManager interface { ) (AWSResource, error) // Delete attempts to destroy the supplied AWSResource in the backend AWS - // service API. - Delete(context.Context, AWSResource) error + // service API, returning an AWSResource representing the + // resource being deleted (if delete is asynchronous and takes time) + Delete(context.Context, AWSResource) (AWSResource, error) // ARNFromName returns an AWS Resource Name from a given string name. This // is useful for constructing ARNs for APIs that require ARNs in their // GetAttributes operations but all we have (for new CRs at least) is a