From 3537bdfbd3a47e5c515ce0280a54b165c3ec705c Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Fri, 5 Sep 2025 14:46:29 +0200 Subject: [PATCH] pass Cluster conditions to MCP --- api/core/v2alpha1/constants.go | 2 + .../managedcontrolplane/clusters.go | 40 +++++++++---- .../managedcontrolplane/controller.go | 39 +++++++++++- .../managedcontrolplane/controller_test.go | 59 ++++++++++++++++++- 4 files changed, 124 insertions(+), 16 deletions(-) diff --git a/api/core/v2alpha1/constants.go b/api/core/v2alpha1/constants.go index 5a79499..5c7a150 100644 --- a/api/core/v2alpha1/constants.go +++ b/api/core/v2alpha1/constants.go @@ -28,6 +28,8 @@ const ( ConditionMeta = "Meta" ConditionClusterRequestReady = "ClusterRequestReady" + ConditionClusterConditionsSynced = "ClusterConditionsSynced" + ConditionPrefixClusterCondition = "Cluster." ConditionPrefixOIDCAccessReady = "OIDCAccessReady." ConditionAllAccessReady = "AllAccessReady" ConditionAllServicesDeleted = "AllServicesDeleted" diff --git a/internal/controllers/managedcontrolplane/clusters.go b/internal/controllers/managedcontrolplane/clusters.go index 590f48d..9664ac3 100644 --- a/internal/controllers/managedcontrolplane/clusters.go +++ b/internal/controllers/managedcontrolplane/clusters.go @@ -17,7 +17,7 @@ import ( corev2alpha1 "github.com/openmcp-project/openmcp-operator/api/core/v2alpha1" ) -func (r *ManagedControlPlaneReconciler) deleteRelatedClusterRequests(ctx context.Context, mcp *corev2alpha1.ManagedControlPlaneV2, platformNamespace string) (sets.Set[string], errutils.ReasonableError) { +func (r *ManagedControlPlaneReconciler) deleteRelatedClusterRequests(ctx context.Context, mcp *corev2alpha1.ManagedControlPlaneV2, platformNamespace string) (sets.Set[string], *clustersv1alpha1.Cluster, errutils.ReasonableError) { log := logging.FromContextOrPanic(ctx) // delete depending cluster requests, if any @@ -25,7 +25,7 @@ func (r *ManagedControlPlaneReconciler) deleteRelatedClusterRequests(ctx context if mcp == nil { log.Debug("MCP is nil, no need to check for cluster requests") - return crNames, nil + return crNames, nil, nil } // identify cluster request finalizers @@ -37,7 +37,7 @@ func (r *ManagedControlPlaneReconciler) deleteRelatedClusterRequests(ctx context if crNames.Len() == 0 { log.Debug("No cluster request finalizers found on MCP") - return crNames, nil + return crNames, nil, nil } // fetch cluster requests, if any exist @@ -56,18 +56,34 @@ func (r *ManagedControlPlaneReconciler) deleteRelatedClusterRequests(ctx context resources[crName] = cr } if rerr := errs.Aggregate(); rerr != nil { - return sets.KeySet(resources), rerr + return sets.KeySet(resources), nil, rerr } // delete cluster requests + var cluster *clustersv1alpha1.Cluster errs = errutils.NewReasonableErrorList() for crName, cr := range resources { - if crName == mcp.Name && len(resources) > 1 { - // skip the MCP's main ClusterRequest for now - // we want to make sure that all other ClusterRequests are deleted first - // in case the corresponding clusters are hosting resources that depend on the MCP cluster - log.Debug("Skipping deletion of MCP's primary ClusterRequest, because there are other ClusterRequests to delete first", "crName", crName, "namespace", cr.GetNamespace()) - continue + if crName == mcp.Name { + // this is the primary ClusterRequest for the MCP cluster + // try to fetch the corresponding Cluster resource + // to sync its conditions to the MCP + if cr != nil && cr.Status.Cluster != nil { + cluster = &clustersv1alpha1.Cluster{} + cluster.Name = cr.Status.Cluster.Name + cluster.Namespace = cr.Status.Cluster.Namespace + if err := r.PlatformCluster.Client().Get(ctx, client.ObjectKeyFromObject(cluster), cluster); err != nil { + // only log the error, this is not critical and should not break the function + log.Error(fmt.Errorf("unable to get Cluster '%s/%s': %w", cluster.Namespace, cluster.Name, err), "error trying to fetch the primary MCP Cluster resource for condition sync") + cluster = nil + } + } + if len(resources) > 1 { + // skip the MCP's main ClusterRequest for now + // we want to make sure that all other ClusterRequests are deleted first + // in case the corresponding clusters are hosting resources that depend on the MCP cluster + log.Debug("Skipping deletion of MCP's primary ClusterRequest, because there are other ClusterRequests to delete first", "crName", crName, "namespace", cr.GetNamespace()) + continue + } } if !cr.GetDeletionTimestamp().IsZero() { log.Debug("ClusterRequest resource already marked for deletion", "crName", crName, "namespace", cr.GetNamespace()) @@ -84,8 +100,8 @@ func (r *ManagedControlPlaneReconciler) deleteRelatedClusterRequests(ctx context } } if rerr := errs.Aggregate(); rerr != nil { - return sets.KeySet(resources), rerr + return sets.KeySet(resources), cluster, rerr } - return sets.KeySet(resources), nil + return sets.KeySet(resources), cluster, nil } diff --git a/internal/controllers/managedcontrolplane/controller.go b/internal/controllers/managedcontrolplane/controller.go index eff3616..27ce453 100644 --- a/internal/controllers/managedcontrolplane/controller.go +++ b/internal/controllers/managedcontrolplane/controller.go @@ -242,6 +242,26 @@ func (r *ManagedControlPlaneReconciler) handleCreateOrUpdate(ctx context.Context log.Debug("ClusterRequest is ready", "clusterRequestName", cr.Name, "clusterRequestNamespace", cr.Namespace) createCon(corev2alpha1.ConditionClusterRequestReady, metav1.ConditionTrue, "", "ClusterRequest is ready") + // fetch Cluster conditions to display them on the MCP + cluster := &clustersv1alpha1.Cluster{} + if cr.Status.Cluster == nil { + // should not happen if the ClusterRequest is granted + rr.ReconcileError = errutils.WithReason(fmt.Errorf("ClusterRequest '%s/%s' does not have a ClusterRef set", cr.Namespace, cr.Name), cconst.ReasonInternalError) + createCon(corev2alpha1.ConditionClusterConditionsSynced, metav1.ConditionFalse, rr.ReconcileError.Reason(), rr.ReconcileError.Error()) + return rr + } + cluster.Name = cr.Status.Cluster.Name + cluster.Namespace = cr.Status.Cluster.Namespace + if err := r.PlatformCluster.Client().Get(ctx, client.ObjectKeyFromObject(cluster), cluster); err != nil { + rr.ReconcileError = errutils.WithReason(fmt.Errorf("unable to get Cluster '%s/%s': %w", cluster.Namespace, cluster.Name, err), cconst.ReasonPlatformClusterInteractionProblem) + createCon(corev2alpha1.ConditionClusterConditionsSynced, metav1.ConditionFalse, rr.ReconcileError.Reason(), rr.ReconcileError.Error()) + return rr + } + for _, con := range cluster.Status.Conditions { + createCon(corev2alpha1.ConditionPrefixClusterCondition+con.Type, con.Status, con.Reason, con.Message) + } + createCon(corev2alpha1.ConditionClusterConditionsSynced, metav1.ConditionTrue, "", "Cluster conditions have been synced to MCP") + // manage AccessRequests allAccessReady, removeConditions, rerr := r.manageAccessRequests(ctx, mcp, platformNamespace, cr, createCon) rr.ConditionsToRemove = removeConditions.UnsortedList() @@ -259,6 +279,7 @@ func (r *ManagedControlPlaneReconciler) handleCreateOrUpdate(ctx context.Context return rr } +//nolint:gocyclo func (r *ManagedControlPlaneReconciler) handleDelete(ctx context.Context, mcp *corev2alpha1.ManagedControlPlaneV2) ReconcileResult { log := logging.FromContextOrPanic(ctx) log.Info("Handling deletion of ManagedControlPlane resource") @@ -325,12 +346,28 @@ func (r *ManagedControlPlaneReconciler) handleDelete(ctx context.Context, mcp *c log.Debug("All AccessRequests deleted") // delete cluster requests related to this MCP - remainingCRs, rerr := r.deleteRelatedClusterRequests(ctx, mcp, platformNamespace) + remainingCRs, primaryCluster, rerr := r.deleteRelatedClusterRequests(ctx, mcp, platformNamespace) if rerr != nil { rr.ReconcileError = rerr createCon(corev2alpha1.ConditionAllClusterRequestsDeleted, metav1.ConditionFalse, rr.ReconcileError.Reason(), rr.ReconcileError.Error()) return rr } + if primaryCluster != nil { + // sync Cluster conditions to the MCP + for _, con := range primaryCluster.Status.Conditions { + createCon(corev2alpha1.ConditionPrefixClusterCondition+con.Type, con.Status, con.Reason, con.Message) + } + createCon(corev2alpha1.ConditionClusterConditionsSynced, metav1.ConditionTrue, "", "Cluster conditions have been synced to MCP") + } else { + // since this point is only reached if no error occurred during r.deleteRelatedClusterRequests, we can assume that the primaryCluster is nil because it does not exist + for _, con := range mcp.Status.Conditions { + // remove all conditions that were synced from the Cluster from the MCP to avoid having unhealthy leftovers + if strings.HasPrefix(con.Type, corev2alpha1.ConditionPrefixClusterCondition) { + rr.ConditionsToRemove = append(rr.ConditionsToRemove, con.Type) + } + } + createCon(corev2alpha1.ConditionClusterConditionsSynced, metav1.ConditionTrue, "", "Primary Cluster for MCP does not exist anymore") + } finalizersToRemove := sets.New(filters.FilterSlice(mcp.Finalizers, func(args ...any) bool { fin, ok := args[0].(string) if !ok { diff --git a/internal/controllers/managedcontrolplane/controller_test.go b/internal/controllers/managedcontrolplane/controller_test.go index 53772af..c81bdd9 100644 --- a/internal/controllers/managedcontrolplane/controller_test.go +++ b/internal/controllers/managedcontrolplane/controller_test.go @@ -62,7 +62,7 @@ func defaultTestSetup(testDirPathSegments ...string) (*managedcontrolplane.Manag } envB := testutils.NewComplexEnvironmentBuilder(). WithFakeClient(platform, install.InstallOperatorAPIsPlatform(runtime.NewScheme())). - WithDynamicObjectsWithStatus(platform, &clustersv1alpha1.ClusterRequest{}, &clustersv1alpha1.AccessRequest{}). + WithDynamicObjectsWithStatus(platform, &clustersv1alpha1.ClusterRequest{}, &clustersv1alpha1.AccessRequest{}, &clustersv1alpha1.Cluster{}). WithFakeClient(onboarding, install.InstallOperatorAPIsOnboarding(runtime.NewScheme())). WithReconcilerConstructor(mcpRec, func(clients ...client.Client) reconcile.Reconciler { mcpr, err := managedcontrolplane.NewManagedControlPlaneReconciler(clusters.NewTestClusterFromClient(platform, clients[0]), clusters.NewTestClusterFromClient(onboarding, clients[1]), nil, cfg.ManagedControlPlane) @@ -131,15 +131,44 @@ var _ = Describe("ManagedControlPlane Controller", func() { WithReason(cconst.ReasonWaitingForClusterRequest)), )) - // fake ClusterRequest ready status + // fake ClusterRequest ready status and Cluster resource By("fake: ClusterRequest readiness") + cluster := &clustersv1alpha1.Cluster{} + cluster.SetName("cluster-01") + cluster.SetNamespace(platformNamespace) + cluster.Spec.Purposes = []string{rec.Config.MCPClusterPurpose} + Expect(env.Client(platform).Create(env.Ctx, cluster)).To(Succeed()) + cluster.Status.Conditions = []metav1.Condition{ + { + Type: "TestCondition1", + Status: metav1.ConditionTrue, + Reason: "TestReason", + Message: "This is a test condition", + LastTransitionTime: metav1.Now(), + ObservedGeneration: 1, + }, + { + Type: "TestCondition2", + Status: metav1.ConditionFalse, + Reason: "TestReason", + Message: "This is another test condition", + LastTransitionTime: metav1.Now(), + ObservedGeneration: 1, + }, + } + Expect(env.Client(platform).Status().Update(env.Ctx, cluster)).To(Succeed()) cr.Status.Phase = clustersv1alpha1.REQUEST_GRANTED + cr.Status.Cluster = &commonapi.ObjectReference{ + Name: cluster.Name, + Namespace: cluster.Namespace, + } Expect(env.Client(platform).Status().Update(env.Ctx, cr)).To(Succeed()) // reconcile the MCP again // expected outcome: // - multiple access requests have been created on the platform cluster, one for each configured OIDC provider // - the mcp has conditions that reflect that it is waiting for the access requests (one for each OIDC provider and one overall one) + // - the mcp has taken over the conditions from the Cluster resource with a prefix // - the mcp should be requeued with a short requeueAfter duration By("second MCP reconciliation") res = env.ShouldReconcile(mcpRec, testutils.RequestFromObject(mcp)) @@ -154,6 +183,19 @@ var _ = Describe("ManagedControlPlane Controller", func() { WithType(corev2alpha1.ConditionAllAccessReady). WithStatus(metav1.ConditionFalse). WithReason(cconst.ReasonWaitingForAccessRequest)), + MatchCondition(TestCondition(). + WithType(corev2alpha1.ConditionPrefixClusterCondition+"TestCondition1"). + WithStatus(metav1.ConditionTrue). + WithReason("TestReason"). + WithMessage("This is a test condition")), + MatchCondition(TestCondition(). + WithType(corev2alpha1.ConditionPrefixClusterCondition+"TestCondition2"). + WithStatus(metav1.ConditionFalse). + WithReason("TestReason"). + WithMessage("This is another test condition")), + MatchCondition(TestCondition(). + WithType(corev2alpha1.ConditionClusterConditionsSynced). + WithStatus(metav1.ConditionTrue)), )) oidcProviders := []commonapi.OIDCProviderConfig{*rec.Config.DefaultOIDCProvider.DeepCopy()} oidcProviders[0].RoleBindings = mcp.Spec.IAM.RoleBindings @@ -552,10 +594,12 @@ var _ = Describe("ManagedControlPlane Controller", func() { WithReason(cconst.ReasonWaitingForClusterRequestDeletion)), )) - // remove finalizer from cr + // remove finalizer from cr and remove Cluster resource By("fake: removing finalizer from primary ClusterRequest") controllerutil.RemoveFinalizer(cr, "dummy") Expect(env.Client(platform).Update(env.Ctx, cr)).To(Succeed()) + Expect(env.Client(platform).Delete(env.Ctx, cluster)).To(Succeed()) + Expect(env.Client(platform).Get(env.Ctx, client.ObjectKeyFromObject(cluster), cluster)).To(MatchError(apierrors.IsNotFound, "IsNotFound")) // add finalizer to MCP namespace By("fake: adding finalizer to MCP namespace") @@ -567,6 +611,7 @@ var _ = Describe("ManagedControlPlane Controller", func() { // expected outcome: // - the MCP namespace has a deletion timestamp // - the MCP has a condition stating that it is waiting for the MCP namespace to be deleted + // - the Cluster conditions should have been removed // - the MCP should be requeued with a short requeueAfter duration By("fifth MCP reconciliation after delete") res = env.ShouldReconcile(mcpRec, testutils.RequestFromObject(mcp)) @@ -580,6 +625,14 @@ var _ = Describe("ManagedControlPlane Controller", func() { WithType(corev2alpha1.ConditionMeta). WithStatus(metav1.ConditionFalse). WithReason(cconst.ReasonWaitingForNamespaceDeletion)), + MatchCondition(TestCondition(). + WithType(corev2alpha1.ConditionClusterConditionsSynced). + WithStatus(metav1.ConditionTrue)), + )) + Expect(mcp.Status.Conditions).ToNot(ContainElements( + MatchFields(IgnoreExtras, Fields{ + "Type": ContainSubstring(corev2alpha1.ConditionPrefixClusterCondition), + }), )) // remove finalizer from MCP namespace