Merge pull request #281 from vishesh92/fix-base-reconciler

Add checks to ensure that ReconcileDelete runs
kubernetes-sigs · Jul 20, 2023 · 41c2908 · 41c2908
2 parents 0d26a04 + fd14ade
commit 41c2908
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 5 deletions.
diff --git a/controllers/cloudstackmachine_controller.go b/controllers/cloudstackmachine_controller.go
@@ -107,7 +107,7 @@ func (reconciler *CloudStackMachineReconciler) Reconcile(ctx context.Context, re
 	r := NewCSMachineReconciliationRunner()
 	r.UsingBaseReconciler(reconciler.ReconcilerBase).ForRequest(req).WithRequestCtx(ctx)
 	r.WithAdditionalCommonStages(
-		r.GetParent(r.ReconciliationSubject, r.CAPIMachine),
+		r.RunIf(func() bool { return r.ReconciliationSubject.GetDeletionTimestamp().IsZero() }, r.GetParent(r.ReconciliationSubject, r.CAPIMachine)),
 		r.RequeueIfCloudStackClusterNotReady,
 		r.SetFailureDomainOnCSMachine,
 		r.GetFailureDomainByName(func() string { return r.ReconciliationSubject.Spec.FailureDomainName }, r.FailureDomain),
@@ -174,7 +174,8 @@ func (r *CloudStackMachineReconciliationRunner) ConsiderAffinity() (ctrl.Result,
 func (r *CloudStackMachineReconciliationRunner) SetFailureDomainOnCSMachine() (retRes ctrl.Result, reterr error) {
 	if r.ReconciliationSubject.Spec.FailureDomainName == "" {
 		var name string
-		if r.CAPIMachine.Spec.FailureDomain != nil &&
+		// CAPIMachine is null if it's been deleted but we're still reconciling the CS machine.
+		if r.CAPIMachine != nil && r.CAPIMachine.Spec.FailureDomain != nil &&
 			(util.IsControlPlaneMachine(r.CAPIMachine) || // Is control plane machine -- CAPI will specify.
 				*r.CAPIMachine.Spec.FailureDomain != "") { // Or potentially another machine controller specified.
 			name = *r.CAPIMachine.Spec.FailureDomain

diff --git a/controllers/utils/base_reconciler.go b/controllers/utils/base_reconciler.go
@@ -19,10 +19,11 @@ package utils
 import (
 	"context"
 	"fmt"
-	"k8s.io/client-go/tools/record"
 	"strings"
 	"time"
 
+	"k8s.io/client-go/tools/record"
+
 	"github.com/go-logr/logr"
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
@@ -311,7 +312,7 @@ func (r *ReconciliationRunner) CheckOwnedObjectsDeleted(gvks ...schema.GroupVers
 
 // RequeueIfCloudStackClusterNotReady requeues the reconciliation request if the CloudStackCluster is not ready.
 func (r *ReconciliationRunner) RequeueIfCloudStackClusterNotReady() (ctrl.Result, error) {
-	if !r.CSCluster.Status.Ready {
+	if r.CSCluster.DeletionTimestamp.IsZero() && !r.CSCluster.Status.Ready {
 		r.Log.Info("CloudStackCluster not ready. Requeuing.")
 		return ctrl.Result{RequeueAfter: RequeueTimeout}, nil
 	}
@@ -402,7 +403,7 @@ func (r *ReconciliationRunner) RunBaseReconciliationStages() (res ctrl.Result, r
 		r.SetupPatcher,
 		r.GetCAPICluster,
 		r.GetCSCluster,
-		r.RequeueIfMissingBaseCRs,
+		r.RunIf(func() bool { return r.ReconciliationSubject.GetDeletionTimestamp().IsZero() }, r.RequeueIfMissingBaseCRs),
 		r.CheckIfPaused}
 	baseStages = append(
 		append(baseStages, r.additionalCommonStages...),