Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/core/v2alpha1/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ const (
ConditionMeta = "Meta"

ConditionClusterRequestReady = "ClusterRequestReady"
ConditionClusterConditionsSynced = "ClusterConditionsSynced"
ConditionPrefixClusterCondition = "Cluster."
ConditionPrefixOIDCAccessReady = "OIDCAccessReady."
ConditionAllAccessReady = "AllAccessReady"
ConditionAllServicesDeleted = "AllServicesDeleted"
Expand Down
40 changes: 28 additions & 12 deletions internal/controllers/managedcontrolplane/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ 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
crNames := sets.New[string]()

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
Expand All @@ -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
Expand All @@ -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())
Expand All @@ -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
}
39 changes: 38 additions & 1 deletion internal/controllers/managedcontrolplane/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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")
Expand Down Expand Up @@ -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 {
Expand Down
59 changes: 56 additions & 3 deletions internal/controllers/managedcontrolplane/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -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))
Expand All @@ -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
Expand Down