From cf137ef851e2dc24e2a0c8d7f2c95ba0172abd3c Mon Sep 17 00:00:00 2001 From: Utku Ozdemir Date: Fri, 15 Nov 2024 11:06:22 +0100 Subject: [PATCH] feat: add `WithIgnoreTeardownWhile` to `qtransform` controllers This is the opposite of `WithIgnoreTeardownUntil` - the runtime will not run teardown while the specified finalizers are still on the input resource. This further facilitates the ordering of "which controller runs its finalizerRemoval before/after which one". Signed-off-by: Utku Ozdemir --- pkg/controller/generic/qtransform/options.go | 34 +++++++++---- .../generic/qtransform/qtransform.go | 10 ++-- .../generic/qtransform/qtransform_test.go | 51 +++++++++++++++++++ 3 files changed, 82 insertions(+), 13 deletions(-) diff --git a/pkg/controller/generic/qtransform/options.go b/pkg/controller/generic/qtransform/options.go index 8360709..11c3001 100644 --- a/pkg/controller/generic/qtransform/options.go +++ b/pkg/controller/generic/qtransform/options.go @@ -57,12 +57,13 @@ func mapperFuncFromGeneric[I generic.ResourceWithRD](generic MapperFuncGeneric[I // ControllerOptions configures QTransformController. type ControllerOptions struct { - mappers map[namespaceType]mapperFunc - leftoverFinalizers map[resource.Finalizer]struct{} - extraInputs []controller.Input - extraOutputs []controller.Output - primaryOutputKind controller.OutputKind - concurrency optional.Optional[uint] + mappers map[namespaceType]mapperFunc + ignoreTeardownUntilFinalizers map[resource.Finalizer]struct{} + ignoreTeardownWhileFinalizers map[resource.Finalizer]struct{} + extraInputs []controller.Input + extraOutputs []controller.Output + primaryOutputKind controller.OutputKind + concurrency optional.Optional[uint] } // ControllerOption is an option for QTransformController. @@ -139,12 +140,27 @@ func WithConcurrency(n uint) ControllerOption { // to be the last one not done with the resource. func WithIgnoreTeardownUntil(finalizers ...resource.Finalizer) ControllerOption { return func(o *ControllerOptions) { - if o.leftoverFinalizers == nil { - o.leftoverFinalizers = map[resource.Finalizer]struct{}{} + if o.ignoreTeardownUntilFinalizers == nil { + o.ignoreTeardownUntilFinalizers = map[resource.Finalizer]struct{}{} } for _, fin := range finalizers { - o.leftoverFinalizers[fin] = struct{}{} + o.ignoreTeardownUntilFinalizers[fin] = struct{}{} + } + } +} + +// WithIgnoreTeardownWhile ignores input resource teardown while the input resource still has the mentioned finalizers. +// +// It is the opposite of WithIgnoreTeardownUntil. +func WithIgnoreTeardownWhile(finalizers ...resource.Finalizer) ControllerOption { + return func(o *ControllerOptions) { + if o.ignoreTeardownWhileFinalizers == nil { + o.ignoreTeardownWhileFinalizers = map[resource.Finalizer]struct{}{} + } + + for _, fin := range finalizers { + o.ignoreTeardownWhileFinalizers[fin] = struct{}{} } } } diff --git a/pkg/controller/generic/qtransform/qtransform.go b/pkg/controller/generic/qtransform/qtransform.go index b9ff283..d795cfc 100644 --- a/pkg/controller/generic/qtransform/qtransform.go +++ b/pkg/controller/generic/qtransform/qtransform.go @@ -200,19 +200,21 @@ func (ctrl *QController[Input, Output]) Reconcile(ctx context.Context, logger *z // if there's an option to ignore finalizers, check if we should ignore tearing down // and perform "normal" reconcile instead - if ctrl.options.leftoverFinalizers != nil { + if ctrl.options.ignoreTeardownUntilFinalizers != nil || ctrl.options.ignoreTeardownWhileFinalizers != nil { for _, fin := range *in.Metadata().Finalizers() { if fin == ctrl.ControllerName { continue } - if _, present := ctrl.options.leftoverFinalizers[fin]; present { + if _, present = ctrl.options.ignoreTeardownUntilFinalizers[fin]; present { continue } - ignoreTearingDown = true + if _, present = ctrl.options.ignoreTeardownWhileFinalizers[fin]; present { + ignoreTearingDown = true - break + break + } } } diff --git a/pkg/controller/generic/qtransform/qtransform_test.go b/pkg/controller/generic/qtransform/qtransform_test.go index a34a719..25a59c0 100644 --- a/pkg/controller/generic/qtransform/qtransform_test.go +++ b/pkg/controller/generic/qtransform/qtransform_test.go @@ -417,6 +417,7 @@ func TestDestroy(t *testing.T) { }) } +//nolint:dupl func TestDestroyWithIgnoreTeardownUntil(t *testing.T) { setup(t, func(ctx context.Context, st state.State, runtime *runtime.Runtime) { require.NoError(t, runtime.RegisterQController(NewABCController(qtransform.WithIgnoreTeardownUntil("extra-finalizer")))) @@ -466,6 +467,56 @@ func TestDestroyWithIgnoreTeardownUntil(t *testing.T) { }) } +//nolint:dupl +func TestDestroyWithIgnoreTeardownWhile(t *testing.T) { + setup(t, func(ctx context.Context, st state.State, runtime *runtime.Runtime) { + require.NoError(t, runtime.RegisterQController(NewABCController(qtransform.WithIgnoreTeardownWhile("extra-finalizer")))) + + for _, a := range []*A{ + NewA("1", ASpec{}), + NewA("2", ASpec{}), + NewA("3", ASpec{}), + } { + require.NoError(t, st.Create(ctx, a)) + } + + rtestutils.AssertResources(ctx, t, st, []resource.ID{"transformed-1", "transformed-2", "transformed-3"}, func(*B, *assert.Assertions) {}) + + // destroy without extra finalizers should work immediately + rtestutils.Destroy[*A](ctx, t, st, []resource.ID{"1"}) + rtestutils.AssertNoResource[*B](ctx, t, st, "transformed-1") + + // add two finalizers to '2' + require.NoError(t, st.AddFinalizer(ctx, NewA("2", ASpec{}).Metadata(), "extra-finalizer", "other-finalizer")) + + // teardown input '2' + _, err := st.Teardown(ctx, NewA("2", ASpec{}).Metadata()) + require.NoError(t, err) + + // the output 'transformed-2' should not be torn down yet + rtestutils.AssertResources(ctx, t, st, []resource.ID{"transformed-2", "transformed-3"}, func(r *B, asrt *assert.Assertions) { + asrt.Equal(resource.PhaseRunning, r.Metadata().Phase()) + }) + + // remove extra-finalizer + require.NoError(t, st.RemoveFinalizer(ctx, NewA("2", ASpec{}).Metadata(), "extra-finalizer")) + // + // the output 'transformed-2' should be destroyed now + rtestutils.AssertNoResource[*B](ctx, t, st, "transformed-2") + // + // the input '2' should no longer have controller finalizer + rtestutils.AssertResources(ctx, t, st, []resource.ID{"2"}, func(r *A, asrt *assert.Assertions) { + asrt.False(r.Metadata().Finalizers().Has("QTransformABCController")) + }) + + // remove other-finalizer + require.NoError(t, st.RemoveFinalizer(ctx, NewA("2", ASpec{}).Metadata(), "other-finalizer")) + + // the input '2' should be destroyed now + rtestutils.Destroy[*A](ctx, t, st, []resource.ID{"2"}) + }) +} + func TestDestroyOutputFinalizers(t *testing.T) { setup(t, func(ctx context.Context, st state.State, runtime *runtime.Runtime) { require.NoError(t, runtime.RegisterQController(NewABNoFinalizerRemovalController()))