fix: update the controlplane resource even when in deletion (#216)

Author: reshnm

VERSION

@@ -1 +1 @@
-v0.42.0-dev
+v0.42.1

charts/mcp-operator/Chart.yaml

@@ -2,8 +2,8 @@ apiVersion: v2
 name: mcp-operator
 description: A Helm chart for the mcp-operator
 type: application
-version: v0.42.0
-appVersion: v0.42.0
+version: v0.42.1
+appVersion: v0.42.1
 home: https://github.com/openmcp-project/mcp-operator
 sources:
   - https://github.com/openmcp-project/mcp-operator

charts/mcp-operator/values.yaml

@@ -14,7 +14,7 @@ deployment:
 
 image:
   repository: ghcr.io/openmcp-project/images/mcp-operator
-  tag: v0.42.0
+  tag: v0.42.1
   pullPolicy: IfNotPresent
 
 imagePullSecrets: []

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/openmcp-project/cluster-provider-gardener/api v0.9.0
 	github.com/openmcp-project/control-plane-operator v0.1.17
 	github.com/openmcp-project/controller-utils v0.23.1
-	github.com/openmcp-project/mcp-operator/api v0.42.0
+	github.com/openmcp-project/mcp-operator/api v0.42.1
 	github.com/openmcp-project/openmcp-operator/api v0.15.2
 	github.com/openmcp-project/openmcp-operator/lib v0.15.2
 	github.com/openmcp-project/service-provider-landscaper v0.7.0

internal/controller/core/cloudorchestrator/controller.go

@@ -163,6 +163,43 @@ func (r *CloudOrchestratorReconciler) reconcile(ctx context.Context, req ctrl.Re
 		return components.ReconcileResult[*openmcpv1alpha1.CloudOrchestrator]{Component: co, ReconcileError: openmcperrors.WithReason(fmt.Errorf("APIServer dependency is ready, but no kubeconfig could be found in its status"), cconst.ReasonDependencyStatusInvalid)}, coreControlPlane, "", ""
 	}
 
+	// only create the ControlPlane resource if it doesn't exist yet and the CO resource is not being deleted
+	if co.DeletionTimestamp.IsZero() && coreControlPlane == nil {
+		// ControlPlane from Core Cluster
+		coreControlPlane = &corev1beta1.ControlPlane{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: utils.PrefixWithNamespace(co.Namespace, co.Name),
+			},
+		}
+	}
+
+	// create or update the ControlPlane resource in the Core Cluster
+	// this will handle both creation and update scenarios
+	// it is not being called when in deletion and the control plane doesn't exist anymore
+	if coreControlPlane != nil {
+		// create or update the CO ControlPlane with the configuration from the openmcpv1alpha1.CloudOrchestrator CR
+		_, err = controllerutil.CreateOrUpdate(ctx, r.CoreClient, coreControlPlane, func() error {
+			spec, err := convertToControlPlaneSpec(&co.Spec, &as.Status)
+			if err != nil {
+				return err
+			}
+			coreControlPlane.Spec = *spec
+
+			// update labels
+			labels, err := r.copyLabels(ctx, co)
+			if err != nil {
+				return err
+			}
+			coreControlPlane.Labels = labels
+			return nil
+		})
+		errs := openmcperrors.NewReasonableErrorList()
+		if err != nil {
+			errs = errs.Append(openmcperrors.Join(errModifyingControlPlane, err))
+			return components.ReconcileResult[*openmcpv1alpha1.CloudOrchestrator]{Component: co, ReconcileError: errs.Aggregate()}, coreControlPlane, "", ""
+		}
+	}
+
 	if !co.DeletionTimestamp.IsZero() {
 		// handle deletion
 		log.Info("Deleting CloudOrchestrator")
@@ -230,33 +267,8 @@ func (r *CloudOrchestratorReconciler) reconcile(ctx context.Context, req ctrl.Re
 		return components.ReconcileResult[*openmcpv1alpha1.CloudOrchestrator]{Component: co, ReconcileError: openmcperrors.WithReason(fmt.Errorf("error setting dependency finalizer on Authorization component resource: %w", err), cconst.ReasonCrateClusterInteractionProblem)}, coreControlPlane, "", ""
 	}
 
-	// ControlPlane from Core Cluster
-	coreControlPlane = &corev1beta1.ControlPlane{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: utils.PrefixWithNamespace(co.Namespace, co.Name),
-		},
-	}
-
-	// create or update the CO ControlPlane with the configuration from the openmcpv1alpha1.CloudOrchestrator CR
-	_, err = controllerutil.CreateOrUpdate(ctx, r.CoreClient, coreControlPlane, func() error {
-		spec, err := convertToControlPlaneSpec(&co.Spec, &as.Status)
-		if err != nil {
-			return err
-		}
-		coreControlPlane.Spec = *spec
-
-		// update labels
-		labels, err := r.copyLabels(ctx, co)
-		if err != nil {
-			return err
-		}
-		coreControlPlane.Labels = labels
-		return nil
-	})
-	errs := openmcperrors.NewReasonableErrorList()
-	if err != nil {
-		errs = errs.Append(openmcperrors.Join(errModifyingControlPlane, err))
-		return components.ReconcileResult[*openmcpv1alpha1.CloudOrchestrator]{Component: co, ReconcileError: errs.Aggregate()}, coreControlPlane, "", ""
+	if coreControlPlane == nil {
+		return components.ReconcileResult[*openmcpv1alpha1.CloudOrchestrator]{Component: co, ReconcileError: openmcperrors.WithReason(fmt.Errorf("CloudOrchestrator ControlPlane resource not found"), cconst.ReasonCOCoreClusterInteractionProblem)}, nil, "", ""
 	}
 
 	// find out if the CO ControlPlane resource is Ready

internal/controller/core/cloudorchestrator/controller_test.go

@@ -428,4 +428,75 @@ var _ = Describe("CO-1153 CloudOrchestrator Controller", func() {
 		Expect(cp.Spec.Target.Kubeconfig).NotTo(BeNil())
 		Expect(cp.Spec.Crossplane).To(BeNil())
 	})
+
+	It("should update the ControlPlane resource even when in deletion", func() {
+		var err error
+
+		env := testEnvSetup(path.Join("testdata", "test-09"), "")
+
+		co := &openmcpv1alpha1.CloudOrchestrator{}
+		err = env.Client(testutils.CrateCluster).Get(env.Ctx, types.NamespacedName{Name: "test", Namespace: "test"}, co)
+		Expect(err).NotTo(HaveOccurred())
+
+		req := testing.RequestFromObject(co)
+		_ = env.ShouldReconcile(coReconciler, req)
+
+		err = env.Client(testutils.CrateCluster).Get(env.Ctx, client.ObjectKeyFromObject(co), co)
+		Expect(err).NotTo(HaveOccurred())
+
+		Expect(co.Status.Conditions).To(ConsistOf(
+			MatchComponentCondition(openmcpv1alpha1.ComponentCondition{
+				Type:   openmcpv1alpha1.CloudOrchestratorComponent.HealthyCondition(),
+				Status: openmcpv1alpha1.ComponentConditionStatusFalse,
+				Reason: cconst.ReasonWaitingForCloudOrchestrator,
+			}),
+			MatchComponentCondition(openmcpv1alpha1.ComponentCondition{
+				Type:   openmcpv1alpha1.CloudOrchestratorComponent.ReconciliationCondition(),
+				Status: openmcpv1alpha1.ComponentConditionStatusTrue,
+			}),
+		))
+
+		cp := &corev1beta1.ControlPlane{}
+		err = env.Client(testutils.COCoreCluster).Get(env.Ctx, types.NamespacedName{
+			Namespace: "",
+			Name:      "test--test",
+		}, cp)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(cp.Spec.Target.Kubeconfig).NotTo(BeNil())
+		// set a finalizer to simulate deletion
+		controllerutil.AddFinalizer(cp, "core.orchestrate.cloud.sap")
+		err = env.Client(testutils.COCoreCluster).Update(env.Ctx, cp)
+		Expect(err).NotTo(HaveOccurred())
+
+		// Delete CO resource
+		err = env.Client(testutils.CrateCluster).Delete(env.Ctx, co)
+		Expect(err).ToNot(HaveOccurred())
+		_ = env.ShouldReconcile(coReconciler, req)
+
+		// check that the ControlPlane resource has a deletion timestamp
+		err = env.Client(testutils.COCoreCluster).Get(env.Ctx, types.NamespacedName{
+			Namespace: "",
+			Name:      "test--test",
+		}, cp)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(cp.DeletionTimestamp).ToNot(BeNil())
+
+		// Update CO resource to trigger an update on the ControlPlane resource
+		err = env.Client(testutils.CrateCluster).Get(env.Ctx, types.NamespacedName{Name: "test", Namespace: "test"}, co)
+		Expect(err).NotTo(HaveOccurred())
+		co.Spec.Kyverno = &openmcpv1alpha1.KyvernoConfig{
+			Version: "8.8.8",
+		}
+		err = env.Client(testutils.CrateCluster).Update(env.Ctx, co)
+		Expect(err).NotTo(HaveOccurred())
+		_ = env.ShouldReconcile(coReconciler, req)
+
+		err = env.Client(testutils.COCoreCluster).Get(env.Ctx, types.NamespacedName{
+			Namespace: "",
+			Name:      "test--test",
+		}, cp)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(cp.Spec.Kyverno).ToNot(BeNil())
+		Expect(cp.Spec.Kyverno.Version).To(Equal("8.8.8"))
+	})
 })

internal/controller/core/cloudorchestrator/testdata/test-09/apiserver.yaml

@@ -0,0 +1,42 @@
+apiVersion: core.openmcp.cloud/v1alpha1
+kind: APIServer
+metadata:
+  name: test
+  namespace: test
+  labels:
+    "openmcp.cloud/mcp-generation": "1"
+spec:
+  desiredRegion:
+    direction: central
+    name: europe
+  type: GardenerDedicated
+status:
+  conditions:
+    - lastTransitionTime: "2024-05-22T08:23:47Z"
+      status: "True"
+      type: apiServerHealthy
+  observedGenerations:
+    internalConfiguration: -1
+    managedControlPlane: 1
+    resource: 0
+  adminAccess:
+    creationTimestamp: "2024-05-22T08:23:47Z"
+    expirationTimestamp: "2024-11-18T08:23:47Z"
+    kubeconfig: |
+      apiVersion: v1
+      clusters:
+      - name: apiserver
+        cluster:
+          server: https://apiserver.dummy
+          certificate-authority-data: ZHVtbXkK
+      contexts:
+      - name: apiserver
+        context:
+          cluster: apiserver
+          user: apiserver
+      current-context: apiserver
+      users:
+      - name: apiserver
+        user:
+          client-certificate-data: ZHVtbXkK
+          client-key-data: ZHVtbXkK

internal/controller/core/cloudorchestrator/testdata/test-09/authentication.yaml

@@ -0,0 +1,10 @@
+apiVersion: core.openmcp.cloud/v1alpha1
+kind: Authentication
+metadata:
+  generation: 1
+  labels:
+    openmcp.cloud/mcp-generation: "1"
+  name: test
+  namespace: test
+spec:
+  enableSystemIdentityProvider: true

internal/controller/core/cloudorchestrator/testdata/test-09/authorization.yaml

@@ -0,0 +1,18 @@
+apiVersion: core.openmcp.cloud/v1alpha1
+kind: Authorization
+metadata:
+  generation: 1
+  labels:
+    openmcp.cloud/mcp-generation: "1"
+  name: test
+  namespace: test
+spec:
+  roleBindings:
+  - role: admin
+    subjects:
+    - apiGroup: rbac.authorization.k8s.io
+      kind: User
+      name: john.doe@example.com
+  - role: view
+    subjects: []
+    

internal/controller/core/cloudorchestrator/testdata/test-09/cloudorchestrator.yaml

@@ -0,0 +1,23 @@
+apiVersion: core.openmcp.cloud/v1alpha1
+kind: CloudOrchestrator
+metadata:
+  name: test
+  namespace: test
+  labels:
+    "openmcp.cloud/mcp-generation": "1"
+  finalizers:
+    - cloudorchestrator.openmcp.cloud
+spec:
+  crossplane:
+    version: 1.17.0
+    providers:
+      - name: provider-kubernetes
+        version: 0.14.1
+  btpServiceOperator:
+    version: 0.6.0
+  externalSecretsOperator:
+    version: 0.10.0
+  kyverno:
+    version: 3.2.7
+  flux:
+    version: 3.2.0