From 1b6c3a00dc81cfbc9518e3f9b71478507d2fdd65 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Thu, 23 Oct 2025 16:06:02 +0200 Subject: [PATCH 1/6] only delete cluster if all other finalizers on cluster resource are gone --- internal/controller/cluster_controller.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/controller/cluster_controller.go b/internal/controller/cluster_controller.go index 5466e74..5b867cd 100644 --- a/internal/controller/cluster_controller.go +++ b/internal/controller/cluster_controller.go @@ -92,13 +92,28 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct } func (r *ClusterReconciler) handleDelete(ctx context.Context, cluster *clustersv1alpha1.Cluster) (ctrl.Result, error) { + log := logf.FromContext(ctx) requeue := smartrequeue.FromContext(ctx) cluster.Status.Phase = commonapi.StatusPhaseTerminating - if !controllerutil.ContainsFinalizer(cluster, Finalizer) { + // check if there are any foreign finalizers on the Cluster resource + foreignFinalizers := make([]string, 0, len(cluster.Finalizers)) + found := false + for _, fin := range cluster.Finalizers { + if fin != Finalizer { + foreignFinalizers = append(foreignFinalizers, fin) + } else { + found = true + } + } + if !found { // Nothing to do return ctrl.Result{}, nil } + if len(foreignFinalizers) > 0 { + log.Info("Postponing cluster deletion until foreign finalizers are removed", "foreignFinalizers", foreignFinalizers) + return requeue.Progressing() + } name := kindName(cluster) From 4314193f835dd574bd306c4ef6a1295be7f0eb66 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Wed, 29 Oct 2025 17:10:49 +0100 Subject: [PATCH 2/6] move foreign finalizer detection in helper method --- internal/controller/cluster_controller.go | 26 +++++++++++++++-------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/internal/controller/cluster_controller.go b/internal/controller/cluster_controller.go index 5b867cd..6ca12b5 100644 --- a/internal/controller/cluster_controller.go +++ b/internal/controller/cluster_controller.go @@ -97,15 +97,7 @@ func (r *ClusterReconciler) handleDelete(ctx context.Context, cluster *clustersv cluster.Status.Phase = commonapi.StatusPhaseTerminating // check if there are any foreign finalizers on the Cluster resource - foreignFinalizers := make([]string, 0, len(cluster.Finalizers)) - found := false - for _, fin := range cluster.Finalizers { - if fin != Finalizer { - foreignFinalizers = append(foreignFinalizers, fin) - } else { - found = true - } - } + foreignFinalizers, found := identifyFinalizers(cluster) if !found { // Nothing to do return ctrl.Result{}, nil @@ -260,3 +252,19 @@ func isClusterProviderResponsible(cluster *clustersv1alpha1.Cluster) bool { func runsOnLocalHost() bool { return os.Getenv("KIND_ON_LOCAL_HOST") == "true" } + +// identifyFinalizers checks two things for the given object: +// 1. If the 'clusters.openmcp.cloud/finalizer' finalizer is present (second return value). +// 2. Which other finalizers are present (first return value). +func identifyFinalizers(obj client.Object) ([]string, bool) { + foreignFinalizers := make([]string, 0, len(obj.GetFinalizers())) + found := false + for _, fin := range obj.GetFinalizers() { + if fin != Finalizer { + foreignFinalizers = append(foreignFinalizers, fin) + } else { + found = true + } + } + return foreignFinalizers, found +} From f58c2a3ccbee9dd54ae4dd4cd4df4fa131d30778 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Thu, 30 Oct 2025 15:22:45 +0100 Subject: [PATCH 3/6] add unit tests for helper function --- go.mod | 7 ++++ go.sum | 2 + internal/controller/controller_test.go | 51 ++++++++++++++++++++++++++ internal/controller/suite_test.go | 14 +++++++ 4 files changed, 74 insertions(+) create mode 100644 internal/controller/controller_test.go create mode 100644 internal/controller/suite_test.go diff --git a/go.mod b/go.mod index 4255319..615da14 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,8 @@ go 1.25.1 godebug default=go1.23 require ( + github.com/onsi/ginkgo/v2 v2.25.3 + github.com/onsi/gomega v1.38.2 github.com/openmcp-project/controller-utils v0.23.1 github.com/openmcp-project/openmcp-operator/api v0.15.1 github.com/stretchr/testify v1.11.1 @@ -24,6 +26,7 @@ require ( al.essio.dev/pkg/shellescape v1.5.1 // indirect cel.dev/expr v0.24.0 // indirect github.com/BurntSushi/toml v1.4.0 // indirect + github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect @@ -42,11 +45,13 @@ require ( github.com/go-openapi/jsonpointer v0.21.2 // indirect github.com/go-openapi/jsonreference v0.21.0 // indirect github.com/go-openapi/swag v0.23.1 // indirect + github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/btree v1.1.3 // indirect github.com/google/cel-go v0.26.0 // indirect github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect + github.com/google/pprof v0.0.0-20250820193118-f64d9cf942d6 // indirect github.com/google/uuid v1.6.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect @@ -79,6 +84,7 @@ require ( go.opentelemetry.io/otel/sdk v1.34.0 // indirect go.opentelemetry.io/otel/trace v1.35.0 // indirect go.opentelemetry.io/proto/otlp v1.5.0 // indirect + go.uber.org/automaxprocs v1.6.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect @@ -91,6 +97,7 @@ require ( golang.org/x/term v0.35.0 // indirect golang.org/x/text v0.29.0 // indirect golang.org/x/time v0.12.0 // indirect + golang.org/x/tools v0.37.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250303144028-a0af3efb3deb // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250303144028-a0af3efb3deb // indirect diff --git a/go.sum b/go.sum index 4b67133..c477527 100644 --- a/go.sum +++ b/go.sum @@ -118,6 +118,8 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/prashantv/gostub v1.1.0 h1:BTyx3RfQjRHnUWaGF9oQos79AlQ5k8WNktv7VGvVH4g= +github.com/prashantv/gostub v1.1.0/go.mod h1:A5zLQHz7ieHGG7is6LLXLz7I8+3LZzsrV0P1IAHhP5U= github.com/prometheus/client_golang v1.23.0 h1:ust4zpdl9r4trLY/gSjlm07PuiBq2ynaXXlptpfy8Uc= github.com/prometheus/client_golang v1.23.0/go.mod h1:i/o0R9ByOnHX0McrTMTyhYvKE4haaf2mW08I+jGAjEE= github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go new file mode 100644 index 0000000..9807307 --- /dev/null +++ b/internal/controller/controller_test.go @@ -0,0 +1,51 @@ +package controller + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + corev1 "k8s.io/api/core/v1" +) + +var _ = Describe("Helper Functions Test", func() { + + Context("identifyFinalizers", func() { + + It("should work correctly with no finalizers on the object at all", func() { + obj := &corev1.Namespace{} + ff, own := identifyFinalizers(obj) + Expect(ff).To(BeEmpty()) + Expect(own).To(BeFalse()) + }) + + It("should work correctly with only the own finalizer on the object", func() { + obj := &corev1.Namespace{} + controllerutil.AddFinalizer(obj, Finalizer) + ff, own := identifyFinalizers(obj) + Expect(ff).To(BeEmpty()) + Expect(own).To(BeTrue()) + }) + + It("should work correctly with only other finalizers on the object", func() { + obj := &corev1.Namespace{} + controllerutil.AddFinalizer(obj, "other/finalizer1") + controllerutil.AddFinalizer(obj, "other/finalizer2") + ff, own := identifyFinalizers(obj) + Expect(ff).To(ConsistOf("other/finalizer1", "other/finalizer2")) + Expect(own).To(BeFalse()) + }) + + It("should work correctly with both own and other finalizers on the object", func() { + obj := &corev1.Namespace{} + controllerutil.AddFinalizer(obj, "other/finalizer1") + controllerutil.AddFinalizer(obj, Finalizer) + controllerutil.AddFinalizer(obj, "other/finalizer2") + ff, own := identifyFinalizers(obj) + Expect(ff).To(ConsistOf("other/finalizer1", "other/finalizer2")) + Expect(own).To(BeTrue()) + }) + + }) + +}) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go new file mode 100644 index 0000000..50df4cb --- /dev/null +++ b/internal/controller/suite_test.go @@ -0,0 +1,14 @@ +package controller + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestControllers(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecs(t, "Controller Test Suite") +} From d6dc5dfd01092278ed13fef7483cb9ad188c0078 Mon Sep 17 00:00:00 2001 From: Maximilian Techritz Date: Thu, 30 Oct 2025 17:25:26 +0100 Subject: [PATCH 4/6] refactor: identifyFinalizers to table driven test --- internal/controller/controller_test.go | 89 +++++++++++++++----------- internal/controller/suite_test.go | 14 ---- 2 files changed, 51 insertions(+), 52 deletions(-) delete mode 100644 internal/controller/suite_test.go diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 9807307..61adf81 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -1,51 +1,64 @@ package controller import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "reflect" + "testing" corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) -var _ = Describe("Helper Functions Test", func() { - - Context("identifyFinalizers", func() { +func TestIdentifyFinalizers(t *testing.T) { + tests := []struct { + name string + inputFinalizers []string + expectedForeignFinalizers []string + expectedOwnFinalizer bool + }{ + { + name: "no finalizers on the object at all", + inputFinalizers: []string{}, + expectedForeignFinalizers: []string{}, + expectedOwnFinalizer: false, + }, + { + name: "only the own finalizer on the object", + inputFinalizers: []string{Finalizer}, + expectedForeignFinalizers: []string{}, + expectedOwnFinalizer: true, + }, + { + name: "only other finalizers on the object", + inputFinalizers: []string{"other/finalizer1", "other/finalizer2"}, + expectedForeignFinalizers: []string{"other/finalizer1", "other/finalizer2"}, + expectedOwnFinalizer: false, + }, + { + name: "both own and other finalizers on the object", + inputFinalizers: []string{"other/finalizer1", Finalizer, "other/finalizer2"}, + expectedForeignFinalizers: []string{"other/finalizer1", "other/finalizer2"}, + expectedOwnFinalizer: true, + }, + } - It("should work correctly with no finalizers on the object at all", func() { + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { obj := &corev1.Namespace{} - ff, own := identifyFinalizers(obj) - Expect(ff).To(BeEmpty()) - Expect(own).To(BeFalse()) - }) - It("should work correctly with only the own finalizer on the object", func() { - obj := &corev1.Namespace{} - controllerutil.AddFinalizer(obj, Finalizer) - ff, own := identifyFinalizers(obj) - Expect(ff).To(BeEmpty()) - Expect(own).To(BeTrue()) - }) + // Add input finalizers to the objeXx + for _, finalizer := range tt.inputFinalizers { + controllerutil.AddFinalizer(obj, finalizer) + } - It("should work correctly with only other finalizers on the object", func() { - obj := &corev1.Namespace{} - controllerutil.AddFinalizer(obj, "other/finalizer1") - controllerutil.AddFinalizer(obj, "other/finalizer2") - ff, own := identifyFinalizers(obj) - Expect(ff).To(ConsistOf("other/finalizer1", "other/finalizer2")) - Expect(own).To(BeFalse()) - }) + foreignFinalizers, ownFinalizer := identifyFinalizers(obj) - It("should work correctly with both own and other finalizers on the object", func() { - obj := &corev1.Namespace{} - controllerutil.AddFinalizer(obj, "other/finalizer1") - controllerutil.AddFinalizer(obj, Finalizer) - controllerutil.AddFinalizer(obj, "other/finalizer2") - ff, own := identifyFinalizers(obj) - Expect(ff).To(ConsistOf("other/finalizer1", "other/finalizer2")) - Expect(own).To(BeTrue()) + // Assert the results + if !reflect.DeepEqual(foreignFinalizers, tt.expectedForeignFinalizers) { + t.Errorf("identifyFinalizers() foreignFinalizers = %v, want %v", foreignFinalizers, tt.expectedForeignFinalizers) + } + if ownFinalizer != tt.expectedOwnFinalizer { + t.Errorf("identifyFinalizers() ownFinalizer = %v, want %v", ownFinalizer, tt.expectedOwnFinalizer) + } }) - - }) - -}) + } +} diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go deleted file mode 100644 index 50df4cb..0000000 --- a/internal/controller/suite_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package controller - -import ( - "testing" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -func TestControllers(t *testing.T) { - RegisterFailHandler(Fail) - - RunSpecs(t, "Controller Test Suite") -} From 8095ef778eb9e00dce46da7e57a78c26d2774e3d Mon Sep 17 00:00:00 2001 From: Maximilian Techritz Date: Thu, 30 Oct 2025 17:26:07 +0100 Subject: [PATCH 5/6] chore: clean-up go.mod --- go.mod | 7 ------- go.sum | 2 -- 2 files changed, 9 deletions(-) diff --git a/go.mod b/go.mod index 615da14..4255319 100644 --- a/go.mod +++ b/go.mod @@ -5,8 +5,6 @@ go 1.25.1 godebug default=go1.23 require ( - github.com/onsi/ginkgo/v2 v2.25.3 - github.com/onsi/gomega v1.38.2 github.com/openmcp-project/controller-utils v0.23.1 github.com/openmcp-project/openmcp-operator/api v0.15.1 github.com/stretchr/testify v1.11.1 @@ -26,7 +24,6 @@ require ( al.essio.dev/pkg/shellescape v1.5.1 // indirect cel.dev/expr v0.24.0 // indirect github.com/BurntSushi/toml v1.4.0 // indirect - github.com/Masterminds/semver/v3 v3.4.0 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect @@ -45,13 +42,11 @@ require ( github.com/go-openapi/jsonpointer v0.21.2 // indirect github.com/go-openapi/jsonreference v0.21.0 // indirect github.com/go-openapi/swag v0.23.1 // indirect - github.com/go-task/slim-sprig/v3 v3.0.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/btree v1.1.3 // indirect github.com/google/cel-go v0.26.0 // indirect github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect - github.com/google/pprof v0.0.0-20250820193118-f64d9cf942d6 // indirect github.com/google/uuid v1.6.0 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.26.3 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect @@ -84,7 +79,6 @@ require ( go.opentelemetry.io/otel/sdk v1.34.0 // indirect go.opentelemetry.io/otel/trace v1.35.0 // indirect go.opentelemetry.io/proto/otlp v1.5.0 // indirect - go.uber.org/automaxprocs v1.6.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect @@ -97,7 +91,6 @@ require ( golang.org/x/term v0.35.0 // indirect golang.org/x/text v0.29.0 // indirect golang.org/x/time v0.12.0 // indirect - golang.org/x/tools v0.37.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250303144028-a0af3efb3deb // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250303144028-a0af3efb3deb // indirect diff --git a/go.sum b/go.sum index c477527..4b67133 100644 --- a/go.sum +++ b/go.sum @@ -118,8 +118,6 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/prashantv/gostub v1.1.0 h1:BTyx3RfQjRHnUWaGF9oQos79AlQ5k8WNktv7VGvVH4g= -github.com/prashantv/gostub v1.1.0/go.mod h1:A5zLQHz7ieHGG7is6LLXLz7I8+3LZzsrV0P1IAHhP5U= github.com/prometheus/client_golang v1.23.0 h1:ust4zpdl9r4trLY/gSjlm07PuiBq2ynaXXlptpfy8Uc= github.com/prometheus/client_golang v1.23.0/go.mod h1:i/o0R9ByOnHX0McrTMTyhYvKE4haaf2mW08I+jGAjEE= github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= From ede8f2b08d2c5c1848bd7432ed63b5ec9cb16a8d Mon Sep 17 00:00:00 2001 From: Maximilian Techritz Date: Thu, 30 Oct 2025 17:26:50 +0100 Subject: [PATCH 6/6] fix: remove comments --- internal/controller/controller_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/controller/controller_test.go b/internal/controller/controller_test.go index 61adf81..de24730 100644 --- a/internal/controller/controller_test.go +++ b/internal/controller/controller_test.go @@ -45,14 +45,12 @@ func TestIdentifyFinalizers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { obj := &corev1.Namespace{} - // Add input finalizers to the objeXx for _, finalizer := range tt.inputFinalizers { controllerutil.AddFinalizer(obj, finalizer) } foreignFinalizers, ownFinalizer := identifyFinalizers(obj) - // Assert the results if !reflect.DeepEqual(foreignFinalizers, tt.expectedForeignFinalizers) { t.Errorf("identifyFinalizers() foreignFinalizers = %v, want %v", foreignFinalizers, tt.expectedForeignFinalizers) }