Merge pull request #4953 from serngawy/issue-4936

🐛 ROSA: Fix issue-4936 delete rosaMachinePool and rosaControlPlane

Author: k8s-ci-robot

controlplane/rosa/controllers/rosacontrolplane_controller.go

@@ -57,6 +57,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/cloud/scope"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/logger"
 	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/rosa"
+	"sigs.k8s.io/cluster-api-provider-aws/v2/pkg/utils"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/util"
 	capiannotations "sigs.k8s.io/cluster-api/util/annotations"
@@ -316,6 +317,16 @@ func (r *ROSAControlPlaneReconciler) reconcileNormal(ctx context.Context, rosaSc
 func (r *ROSAControlPlaneReconciler) reconcileDelete(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (res ctrl.Result, reterr error) {
 	rosaScope.Info("Reconciling ROSAControlPlane delete")
 
+	// Deleting MachinePools first.
+	deleted, err := r.deleteMachinePools(ctx, rosaScope)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+	if !deleted {
+		// Reconcile after 1 min giving time for machinePools to be deleted.
+		return ctrl.Result{RequeueAfter: time.Minute}, nil
+	}
+
 	ocmClient, err := rosa.NewOCMClient(ctx, rosaScope)
 	if err != nil {
 		// TODO: need to expose in status, as likely the credentials are invalid
@@ -332,8 +343,9 @@ func (r *ROSAControlPlaneReconciler) reconcileDelete(ctx context.Context, rosaSc
 		return ctrl.Result{}, err
 	}
 	if cluster == nil {
-		// cluster is fully deleted, remove finalizer.
+		// cluster and machinepools are deleted, removing finalizer.
 		controllerutil.RemoveFinalizer(rosaScope.ControlPlane, ROSAControlPlaneFinalizer)
+
 		return ctrl.Result{}, nil
 	}
 
@@ -366,6 +378,30 @@ func (r *ROSAControlPlaneReconciler) reconcileDelete(ctx context.Context, rosaSc
 	return ctrl.Result{RequeueAfter: time.Second * 60}, nil
 }
 
+// deleteMachinePools check if the controlplane has related machinePools and delete them.
+func (r *ROSAControlPlaneReconciler) deleteMachinePools(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (bool, error) {
+	machinePools, err := utils.GetMachinePools(ctx, rosaScope.Client, rosaScope.Cluster.Name, rosaScope.Cluster.Namespace)
+	if err != nil {
+		return false, err
+	}
+
+	var errs []error
+	for id, mp := range machinePools {
+		if !mp.DeletionTimestamp.IsZero() {
+			continue
+		}
+		if err = rosaScope.Client.Delete(ctx, &machinePools[id]); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	if len(errs) > 0 {
+		return false, kerrors.NewAggregate(errs)
+	}
+
+	return len(machinePools) == 0, nil
+}
+
 func (r *ROSAControlPlaneReconciler) reconcileClusterVersion(rosaScope *scope.ROSAControlPlaneScope, ocmClient *ocm.Client, cluster *cmv1.Cluster) error {
 	version := rosaScope.ControlPlane.Spec.Version
 	if version == rosa.RawVersionID(cluster.Version()) {

exp/controllers/rosamachinepool_controller.go

@@ -107,7 +107,7 @@ func (r *ROSAMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	cluster, err := util.GetClusterFromMetadata(ctx, r.Client, machinePool.ObjectMeta)
 	if err != nil {
 		log.Info("Failed to retrieve Cluster from MachinePool")
-		return reconcile.Result{}, nil
+		return ctrl.Result{}, nil
 	}
 
 	if annotations.IsPaused(cluster, rosaMachinePool) {
@@ -124,7 +124,7 @@ func (r *ROSAMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 	controlPlane := &rosacontrolplanev1.ROSAControlPlane{}
 	if err := r.Client.Get(ctx, controlPlaneKey, controlPlane); err != nil {
 		log.Info("Failed to retrieve ControlPlane from MachinePool")
-		return reconcile.Result{}, nil
+		return ctrl.Result{}, err
 	}
 
 	machinePoolScope, err := scope.NewRosaMachinePoolScope(scope.RosaMachinePoolScopeParams{
@@ -138,7 +138,7 @@ func (r *ROSAMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		Endpoints:       r.Endpoints,
 	})
 	if err != nil {
-		return ctrl.Result{}, errors.Wrap(err, "failed to create scope")
+		return ctrl.Result{}, errors.Wrap(err, "failed to create rosaMachinePool scope")
 	}
 
 	rosaControlPlaneScope, err := scope.NewROSAControlPlaneScope(scope.ROSAControlPlaneScopeParams{
@@ -149,10 +149,10 @@ func (r *ROSAMachinePoolReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		Endpoints:      r.Endpoints,
 	})
 	if err != nil {
-		return ctrl.Result{}, errors.Wrap(err, "failed to create control plane scope")
+		return ctrl.Result{}, errors.Wrap(err, "failed to create rosaControlPlane scope")
 	}
 
-	if !controlPlane.Status.Ready {
+	if !controlPlane.Status.Ready && controlPlane.ObjectMeta.DeletionTimestamp.IsZero() {
 		log.Info("Control plane is not ready yet")
 		err := machinePoolScope.RosaMchinePoolReadyFalse(expinfrav1.WaitingForRosaControlPlaneReason, "")
 		return ctrl.Result{}, err
@@ -311,6 +311,7 @@ func (r *ROSAMachinePoolReconciler) reconcileDelete(
 		if err := ocmClient.DeleteNodePool(machinePoolScope.ControlPlane.Status.ID, nodePool.ID()); err != nil {
 			return err
 		}
+		machinePoolScope.Info("Successfully deleted NodePool %s", nodePool.ID())
 	}
 
 	controllerutil.RemoveFinalizer(machinePoolScope.RosaMachinePool, expinfrav1.RosaMachinePoolFinalizer)

pkg/utils/utils.go

@@ -0,0 +1,27 @@
+// Package utils has the common functions that can be used for cluster-api-provider-aws repo.
+package utils
+
+import (
+	"context"
+	"fmt"
+
+	crclient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	expclusterv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
+)
+
+// GetMachinePools belong to a cluster.
+func GetMachinePools(ctx context.Context, client crclient.Client, clusterName string, clusterNS string) ([]expclusterv1.MachinePool, error) {
+	machinePoolList := expclusterv1.MachinePoolList{}
+	listOptions := []crclient.ListOption{
+		crclient.InNamespace(clusterNS),
+		crclient.MatchingLabels(map[string]string{clusterv1.ClusterNameLabel: clusterName}),
+	}
+
+	if err := client.List(ctx, &machinePoolList, listOptions...); err != nil {
+		return []expclusterv1.MachinePool{}, fmt.Errorf("failed to list machine pools for cluster %s: %v", clusterName, err)
+	}
+
+	return machinePoolList.Items, nil
+}