Merge pull request #5365 from alexander-demicev/waitforclusterdependencies

✨ Wait for AWSCluster dependent object to be deleted

Author: k8s-ci-robot

controllers/awscluster_controller.go

@@ -58,6 +58,10 @@ import (
 	"sigs.k8s.io/cluster-api/util/predicates"
 )
 
+const (
+	deleteRequeueAfter = 20 * time.Second
+)
+
 var defaultAWSSecurityGroupRoles = []infrav1.SecurityGroupRole{
 	infrav1.SecurityGroupAPIServerLB,
 	infrav1.SecurityGroupLB,
@@ -191,21 +195,34 @@ func (r *AWSClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 
 	// Handle deleted clusters
 	if !awsCluster.DeletionTimestamp.IsZero() {
-		return ctrl.Result{}, r.reconcileDelete(ctx, clusterScope)
+		return r.reconcileDelete(ctx, clusterScope)
 	}
 
 	// Handle non-deleted clusters
 	return r.reconcileNormal(clusterScope)
 }
 
-func (r *AWSClusterReconciler) reconcileDelete(ctx context.Context, clusterScope *scope.ClusterScope) error {
+func (r *AWSClusterReconciler) reconcileDelete(ctx context.Context, clusterScope *scope.ClusterScope) (ctrl.Result, error) {
 	if !controllerutil.ContainsFinalizer(clusterScope.AWSCluster, infrav1.ClusterFinalizer) {
 		clusterScope.Info("No finalizer on AWSCluster, skipping deletion reconciliation")
-		return nil
+		return reconcile.Result{}, nil
 	}
 
 	clusterScope.Info("Reconciling AWSCluster delete")
 
+	numDependencies, err := r.dependencyCount(ctx, clusterScope)
+	if err != nil {
+		clusterScope.Error(err, "error getting AWSCluster dependencies")
+		return reconcile.Result{}, err
+	}
+
+	if numDependencies > 0 {
+		clusterScope.Info("AWSCluster cluster still has dependencies - requeue needed", "dependencyCount", numDependencies)
+		return reconcile.Result{RequeueAfter: deleteRequeueAfter}, nil
+	}
+
+	clusterScope.Info("AWSCluster has no dependent CAPI resources, proceeding with its deletion")
+
 	ec2svc := r.getEC2Service(clusterScope)
 	elbsvc := r.getELBService(clusterScope)
 	networkSvc := r.getNetworkService(*clusterScope)
@@ -257,12 +274,12 @@ func (r *AWSClusterReconciler) reconcileDelete(ctx context.Context, clusterScope
 	}
 
 	if len(allErrs) > 0 {
-		return kerrors.NewAggregate(allErrs)
+		return reconcile.Result{}, kerrors.NewAggregate(allErrs)
 	}
 
 	// Cluster is deleted so remove the finalizer.
 	controllerutil.RemoveFinalizer(clusterScope.AWSCluster, infrav1.ClusterFinalizer)
-	return nil
+	return reconcile.Result{}, nil
 }
 
 func (r *AWSClusterReconciler) reconcileLoadBalancer(clusterScope *scope.ClusterScope, awsCluster *infrav1.AWSCluster) (*time.Duration, error) {
@@ -484,3 +501,24 @@ func (r *AWSClusterReconciler) checkForExternalControlPlaneLoadBalancer(clusterS
 		return nil
 	}
 }
+
+func (r *AWSClusterReconciler) dependencyCount(ctx context.Context, clusterScope *scope.ClusterScope) (int, error) {
+	clusterName := clusterScope.Name()
+	namespace := clusterScope.Namespace()
+
+	clusterScope.Info("Looking for AWSCluster dependencies")
+
+	listOptions := []client.ListOption{
+		client.InNamespace(namespace),
+		client.MatchingLabels(map[string]string{clusterv1.ClusterNameLabel: clusterName}),
+	}
+
+	machines := &infrav1.AWSMachineList{}
+	if err := r.Client.List(ctx, machines, listOptions...); err != nil {
+		return 0, fmt.Errorf("failed to list machines for cluster %s/%s: %w", namespace, clusterName, err)
+	}
+
+	clusterScope.Debug("Found dependent machines", "count", len(machines.Items))
+
+	return len(machines.Items), nil
+}

controllers/awscluster_controller_test.go

@@ -568,7 +568,7 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
 		_, err = reconciler.reconcileNormal(cs)
 		g.Expect(err.Error()).To(ContainSubstring("The maximum number of VPCs has been reached"))
 
-		err = reconciler.reconcileDelete(ctx, cs)
+		_, err = reconciler.reconcileDelete(ctx, cs)
 		g.Expect(err).To(BeNil())
 	})
 	t.Run("Should successfully delete AWSCluster with managed VPC", func(t *testing.T) {
@@ -646,7 +646,7 @@ func TestAWSClusterReconcilerIntegrationTests(t *testing.T) {
 			return sgSvc
 		}
 
-		err = reconciler.reconcileDelete(ctx, cs)
+		_, err = reconciler.reconcileDelete(ctx, cs)
 		g.Expect(err).To(BeNil())
 		expectAWSClusterConditions(g, cs.AWSCluster, []conditionAssertion{
 			{infrav1.LoadBalancerReadyCondition, corev1.ConditionFalse, clusterv1.ConditionSeverityInfo, clusterv1.DeletedReason},

controllers/awscluster_controller_unit_test.go

@@ -420,7 +420,7 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 					},
 				)
 				g.Expect(err).To(BeNil())
-				err = reconciler.reconcileDelete(ctx, cs)
+				_, err = reconciler.reconcileDelete(ctx, cs)
 				g.Expect(err).To(BeNil())
 				g.Expect(awsCluster.GetFinalizers()).ToNot(ContainElement(infrav1.ClusterFinalizer))
 			})
@@ -449,7 +449,7 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 					},
 				)
 				g.Expect(err).To(BeNil())
-				err = reconciler.reconcileDelete(ctx, cs)
+				_, err = reconciler.reconcileDelete(ctx, cs)
 				g.Expect(err).ToNot(BeNil())
 				g.Expect(awsCluster.GetFinalizers()).To(ContainElement(infrav1.ClusterFinalizer))
 			})
@@ -474,7 +474,7 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 					},
 				)
 				g.Expect(err).To(BeNil())
-				err = reconciler.reconcileDelete(ctx, cs)
+				_, err = reconciler.reconcileDelete(ctx, cs)
 				g.Expect(err).ToNot(BeNil())
 				g.Expect(awsCluster.GetFinalizers()).To(ContainElement(infrav1.ClusterFinalizer))
 			})
@@ -499,7 +499,7 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 					},
 				)
 				g.Expect(err).To(BeNil())
-				err = reconciler.reconcileDelete(ctx, cs)
+				_, err = reconciler.reconcileDelete(ctx, cs)
 				g.Expect(err).ToNot(BeNil())
 				g.Expect(awsCluster.GetFinalizers()).To(ContainElement(infrav1.ClusterFinalizer))
 			})
@@ -524,7 +524,7 @@ func TestAWSClusterReconcileOperations(t *testing.T) {
 					},
 				)
 				g.Expect(err).To(BeNil())
-				err = reconciler.reconcileDelete(ctx, cs)
+				_, err = reconciler.reconcileDelete(ctx, cs)
 				g.Expect(err).ToNot(BeNil())
 				g.Expect(awsCluster.GetFinalizers()).To(ContainElement(infrav1.ClusterFinalizer))
 			})