Skip to content

Commit 8516dce

Browse files
committed
Delay external delete call when other finalizers exist
Signed-off-by: Bob Haddleton <[email protected]>
1 parent 678177c commit 8516dce

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

pkg/reconciler/managed/reconciler.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu
10941094
if meta.WasDeleted(managed) {
10951095
log = log.WithValues("deletion-timestamp", managed.GetDeletionTimestamp())
10961096

1097+
if len(managed.GetFinalizers()) > 1 {
1098+
// There are other controllers monitoring this resource so preserve the external instance
1099+
// until all other finalizers have been removed
1100+
log.Debug("Delay external deletion until all finalizers have been removed")
1101+
return reconcile.Result{Requeue: true}, nil
1102+
}
1103+
10971104
if observation.ResourceExists && policy.ShouldDelete() {
10981105
deletion, err := external.Delete(externalCtx, managed)
10991106
if err != nil {

pkg/reconciler/managed/reconciler_test.go

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,51 @@ func TestReconciler(t *testing.T) {
204204
},
205205
want: want{result: reconcile.Result{Requeue: true}},
206206
},
207-
"ExternalCreatePending": {
207+
"ExtraFinalizersDelayDelete": {
208+
reason: "The existence of multiple finalizers should trigger a requeue after a short wait.",
209+
args: args{
210+
m: &fake.Manager{
211+
Client: &test.MockClient{
212+
MockGet: test.NewMockGetFn(nil, func(obj client.Object) error {
213+
mg :=
214+
asManaged(obj, 42)
215+
mg.SetDeletionTimestamp(&now)
216+
mg.SetDeletionPolicy(xpv1.DeletionDelete)
217+
mg.SetFinalizers([]string{FinalizerName, "finalizer2"})
218+
return nil
219+
}),
220+
MockStatusUpdate: test.MockSubResourceUpdateFn(func(_ context.Context, _ client.Object,
221+
_ ...client.SubResourceUpdateOption) error {
222+
want :=
223+
newManaged(42)
224+
want.SetDeletionTimestamp(&now)
225+
want.SetDeletionPolicy(xpv1.DeletionDelete)
226+
want.SetFinalizers([]string{FinalizerName, "finalizer2"})
227+
want.SetConditions(xpv1.Deleting().WithObservedGeneration(42))
228+
return nil
229+
}),
230+
},
231+
Scheme: fake.SchemeWith(&fake.Managed{}),
232+
},
233+
mg: resource.ManagedKind(fake.GVK(&fake.Managed{})),
234+
o: []ReconcilerOption{
235+
WithInitializers(),
236+
WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })),
237+
WithExternalConnector(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) {
238+
c := &ExternalClientFns{
239+
ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) {
240+
return ExternalObservation{ResourceExists: true}, nil
241+
},
242+
DisconnectFn: func(_ context.Context) error {
243+
return nil
244+
},
245+
}
246+
return c, nil
247+
})),
248+
},
249+
},
250+
want: want{result: reconcile.Result{Requeue: true}},
251+
}, "ExternalCreatePending": {
208252
reason: "We should return early if the managed resource appears to be pending creation. We might have leaked a resource and don't want to create another.",
209253
args: args{
210254
m: &fake.Manager{

0 commit comments

Comments
 (0)