feat: remove need for service finalizers on MCP (#208)

Author: Diaphteiros

api/core/v2alpha1/constants.go

@@ -21,8 +21,6 @@ const (
 
 	MCPFinalizer = GroupName + "/mcp"
 
-	// ServiceDependencyFinalizerPrefix is the prefix for the dependency finalizers that are added to MCP resources by associated services.
-	ServiceDependencyFinalizerPrefix = "services.openmcp.cloud/"
 	// ClusterRequestFinalizerPrefix is the prefix for the finalizers that are added to MCP resources for cluster requests.
 	ClusterRequestFinalizerPrefix = "request.clusters.openmcp.cloud/"
 )

internal/controllers/managedcontrolplane/controller_test.go

@@ -588,22 +588,14 @@ var _ = Describe("ManagedControlPlane Controller", func() {
 			Expect(obj.GetDeletionTimestamp().IsZero()).To(BeTrue())
 		}
 
-		// remove service finalizers
-		By("fake: removing service finalizers")
+		// remove service resource finalizers
+		By("fake: removing service resource finalizers")
 		for _, obj := range serviceResources {
 			By("fake: removing finalizer from service resource: " + obj.GetObjectKind().GroupVersionKind().Kind)
 			Expect(env.Client(onboarding).Get(env.Ctx, client.ObjectKeyFromObject(obj), obj)).To(Succeed())
 			controllerutil.RemoveFinalizer(obj, "dummy")
 			Expect(env.Client(onboarding).Update(env.Ctx, obj)).To(Succeed())
 		}
-		newFins := []string{}
-		for _, fin := range mcp.Finalizers {
-			if !strings.HasPrefix(fin, corev2alpha1.ServiceDependencyFinalizerPrefix) {
-				newFins = append(newFins, fin)
-			}
-		}
-		mcp.Finalizers = newFins
-		Expect(env.Client(onboarding).Update(env.Ctx, mcp)).To(Succeed())
 
 		// reconcile the MCP again
 		// expected outcome:

internal/controllers/managedcontrolplane/services.go

@@ -3,11 +3,9 @@ package managedcontrolplane
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	errutils "github.com/openmcp-project/controller-utils/pkg/errors"
@@ -18,45 +16,32 @@ import (
 	providerv1alpha1 "github.com/openmcp-project/openmcp-operator/api/provider/v1alpha1"
 )
 
-// deleteDependingServices deletes service resources that belong to service providers which have a 'services.openmcp.cloud/<name>' finalizer on the ManagedControlPlane.
+// deleteDependingServices lists all service providers and checks for all of their registered service resources whether there exists one for the given MCP. If so, it triggers their deletion.
 // It returns a set of service provider names for which still resources exist (should be in deletion by the time this function returns) and the total number of resources that are still left.
 // Deletion of the MCP should wait until the set is empty and the count is zero.
 func (r *ManagedControlPlaneReconciler) deleteDependingServices(ctx context.Context, mcp *corev2alpha1.ManagedControlPlaneV2) (map[string][]*unstructured.Unstructured, errutils.ReasonableError) {
 	log := logging.FromContextOrPanic(ctx)
 
 	// delete depending service resources, if any
-	serviceProviderNames := sets.New[string]()
 
 	if mcp == nil {
 		log.Debug("MCP is nil, no need to check for services")
 		return nil, nil
 	}
 
-	// identify service finalizers
-	for _, fin := range mcp.Finalizers {
-		if service, ok := strings.CutPrefix(fin, corev2alpha1.ServiceDependencyFinalizerPrefix); ok {
-			serviceProviderNames.Insert(service)
-		}
-	}
-
-	if serviceProviderNames.Len() == 0 {
-		log.Debug("No service finalizers found on MCP")
-		return nil, nil
+	// list all ServiceProvider resources to identify service resources
+	sps := &providerv1alpha1.ServiceProviderList{}
+	if err := r.PlatformCluster.Client().List(ctx, sps); err != nil {
+		return nil, errutils.WithReason(fmt.Errorf("failed to list ServiceProviders: %w", err), cconst.ReasonPlatformClusterInteractionProblem)
 	}
 
 	// fetch service resources, if any exist
 	resources := map[string][]*unstructured.Unstructured{}
 	errs := errutils.NewReasonableErrorList()
-	for providerName := range serviceProviderNames {
-		sp := &providerv1alpha1.ServiceProvider{}
-		sp.SetName(providerName)
-		if err := r.PlatformCluster.Client().Get(ctx, client.ObjectKeyFromObject(sp), sp); err != nil {
-			errs.Append(errutils.WithReason(fmt.Errorf("failed to get ServiceProvider %s: %w", providerName, err), cconst.ReasonPlatformClusterInteractionProblem))
-			continue
-		}
-
+	for _, sp := range sps.Items {
 		if len(sp.Status.Resources) == 0 {
-			errs.Append(errutils.WithReason(fmt.Errorf("a dependency finalizer for ServiceProvider '%s' exist on MCP, but the provider does not expose any service resources", providerName), cconst.ReasonInternalError))
+			log.Debug("ServiceProvider has no registered service resources", "providerName", sp.Name)
+			continue
 		}
 		serviceResources := []*unstructured.Unstructured{}
 		for _, resourceType := range sp.Status.Resources {
@@ -67,14 +52,15 @@ func (r *ManagedControlPlaneReconciler) deleteDependingServices(ctx context.Cont
 			res.SetNamespace(mcp.Namespace)
 			if err := r.OnboardingCluster.Client().Get(ctx, client.ObjectKeyFromObject(res), res); err != nil {
 				if !apierrors.IsNotFound(err) {
-					errs.Append(errutils.WithReason(fmt.Errorf("error getting service resource [%s.%s] '%s/%s' for ServiceProvider '%s': %w", res.GetKind(), res.GetAPIVersion(), res.GetNamespace(), res.GetName(), providerName, err), cconst.ReasonOnboardingClusterInteractionProblem))
+					errs.Append(errutils.WithReason(fmt.Errorf("error getting service resource [%s.%s] '%s/%s' for ServiceProvider '%s': %w", res.GetKind(), res.GetAPIVersion(), res.GetNamespace(), res.GetName(), sp.Name, err), cconst.ReasonOnboardingClusterInteractionProblem))
 				}
 				continue
 			}
+			log.Debug("Found service resource for ServiceProvider", "resourceKind", res.GetKind(), "resourceAPIVersion", res.GetAPIVersion(), "providerName", sp.Name)
 			serviceResources = append(serviceResources, res)
 		}
 
-		resources[providerName] = serviceResources
+		resources[sp.Name] = serviceResources
 	}
 	if rerr := errs.Aggregate(); rerr != nil {
 		return nil, rerr
@@ -85,7 +71,7 @@ func (r *ManagedControlPlaneReconciler) deleteDependingServices(ctx context.Cont
 	remainingResources := map[string][]*unstructured.Unstructured{}
 	for providerName, serviceResources := range resources {
 		if len(serviceResources) == 0 {
-			log.Debug("No service resources found for ServiceProvider", "providerName", providerName)
+			log.Debug("No remaining service resources found for ServiceProvider", "providerName", providerName)
 			continue
 		}
 		remainingServiceResources := []*unstructured.Unstructured{}

internal/controllers/managedcontrolplane/testdata/test-01/onboarding/mcp-01.yaml

@@ -3,9 +3,6 @@ kind: ManagedControlPlaneV2
 metadata:
   name: mcp-01
   namespace: test
-  finalizers:
-  - services.openmcp.cloud/sp-01
-  - services.openmcp.cloud/sp-02
 spec:
   iam:
     tokens:

internal/controllers/managedcontrolplane/testdata/test-01/onboarding/mcp-03.yaml

@@ -3,9 +3,6 @@ kind: ManagedControlPlaneV2
 metadata:
   name: mcp-03
   namespace: test
-  finalizers:
-  - services.openmcp.cloud/sp-01
-  - services.openmcp.cloud/sp-02
 spec:
   iam:
     tokens: