From ed5e5a826591dd06030418dd1b1f311a7192820a Mon Sep 17 00:00:00 2001
From: Matthew Booth <mbooth@redhat.com>
Date: Sat, 16 Mar 2024 11:14:27 +0000
Subject: [PATCH] Fix premature attempt to resolve machine resources

We can't resolve machine resources until the cluster is initialised.
---
 controllers/openstackmachine_controller.go | 58 ++++++++++++----------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go
index 593867d831..616179e249 100644
--- a/controllers/openstackmachine_controller.go
+++ b/controllers/openstackmachine_controller.go
@@ -153,36 +153,33 @@ func (r *OpenStackMachineReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	}
 	scope := scope.NewWithLogger(clientScope, log)
 
+	clusterName := names.ClusterName(cluster)
+
+	// Handle deleted machines
+	if !openStackMachine.DeletionTimestamp.IsZero() {
+		return r.reconcileDelete(scope, clusterName, infraCluster, machine, openStackMachine)
+	}
+
+	// Handle non-deleted clusters
+	return r.reconcileNormal(ctx, scope, clusterName, infraCluster, machine, openStackMachine)
+}
+
+func resolveMachineResources(scope *scope.WithLogger, clusterName string, openStackCluster *infrav1.OpenStackCluster, openStackMachine *infrav1.OpenStackMachine, machine *clusterv1.Machine) (bool, error) {
 	// Resolve and store referenced resources
 	changed, err := compute.ResolveReferencedMachineResources(scope,
 		&openStackMachine.Spec, &openStackMachine.Status.ReferencedResources,
-		names.ClusterName(cluster), openStackMachine.Name,
-		infraCluster, getManagedSecurityGroup(infraCluster, machine))
+		clusterName, openStackMachine.Name,
+		openStackCluster, getManagedSecurityGroup(openStackCluster, machine))
 	if err != nil {
-		return reconcile.Result{}, err
+		return false, err
 	}
 	if changed {
 		// If the referenced resources have changed, we need to update the OpenStackMachine status now.
-		return reconcile.Result{}, nil
+		return true, nil
 	}
 
 	// Adopt any existing dependent resources
-	changed, err = compute.AdoptDependentMachineResources(scope, &openStackMachine.Status.ReferencedResources, &openStackMachine.Status.DependentResources)
-	if err != nil {
-		return reconcile.Result{}, err
-	}
-	if changed {
-		// If the dependent resources have changed, we need to update the OpenStackMachine status now.
-		return reconcile.Result{}, nil
-	}
-
-	// Handle deleted machines
-	if !openStackMachine.DeletionTimestamp.IsZero() {
-		return r.reconcileDelete(scope, cluster, infraCluster, machine, openStackMachine)
-	}
-
-	// Handle non-deleted clusters
-	return r.reconcileNormal(ctx, scope, cluster, infraCluster, machine, openStackMachine)
+	return compute.AdoptDependentMachineResources(scope, &openStackMachine.Status.ReferencedResources, &openStackMachine.Status.DependentResources)
 }
 
 func patchMachine(ctx context.Context, patchHelper *patch.Helper, openStackMachine *infrav1.OpenStackMachine, machine *clusterv1.Machine, options ...patch.Option) error {
@@ -253,11 +250,9 @@ func (r *OpenStackMachineReconciler) SetupWithManager(ctx context.Context, mgr c
 		Complete(r)
 }
 
-func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine) (ctrl.Result, error) { //nolint:unparam
+func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, clusterName string, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine) (ctrl.Result, error) { //nolint:unparam
 	scope.Logger().Info("Reconciling Machine delete")
 
-	clusterName := names.ClusterName(cluster)
-
 	computeService, err := compute.NewService(scope)
 	if err != nil {
 		return ctrl.Result{}, err
@@ -268,6 +263,13 @@ func (r *OpenStackMachineReconciler) reconcileDelete(scope *scope.WithLogger, cl
 		return ctrl.Result{}, err
 	}
 
+	// We may have resources to adopt if the cluster is ready
+	if openStackCluster.Status.Ready {
+		if _, err := resolveMachineResources(scope, clusterName, openStackCluster, openStackMachine, machine); err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
 	if openStackCluster.Spec.APIServerLoadBalancer.IsEnabled() {
 		loadBalancerService, err := loadbalancer.NewService(scope)
 		if err != nil {
@@ -480,7 +482,7 @@ func (r *OpenStackMachineReconciler) reconcileDeleteFloatingAddressFromPool(scop
 	return r.Client.Update(context.Background(), claim)
 }
 
-func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope *scope.WithLogger, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine) (_ ctrl.Result, reterr error) {
+func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope *scope.WithLogger, clusterName string, openStackCluster *infrav1.OpenStackCluster, machine *clusterv1.Machine, openStackMachine *infrav1.OpenStackMachine) (_ ctrl.Result, reterr error) {
 	var err error
 
 	// If the OpenStackMachine is in an error state, return early.
@@ -495,12 +497,16 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 		return ctrl.Result{}, nil
 	}
 
-	if !cluster.Status.InfrastructureReady {
+	if !openStackCluster.Status.Ready {
 		scope.Logger().Info("Cluster infrastructure is not ready yet, re-queuing machine")
 		conditions.MarkFalse(openStackMachine, infrav1.InstanceReadyCondition, infrav1.WaitingForClusterInfrastructureReason, clusterv1.ConditionSeverityInfo, "")
 		return ctrl.Result{RequeueAfter: waitForClusterInfrastructureReadyDuration}, nil
 	}
 
+	if changed, err := resolveMachineResources(scope, clusterName, openStackCluster, openStackMachine, machine); changed || err != nil {
+		return ctrl.Result{}, err
+	}
+
 	// Make sure bootstrap data is available and populated.
 	if machine.Spec.Bootstrap.DataSecretName == nil {
 		scope.Logger().Info("Bootstrap data secret reference is not yet available")
@@ -513,8 +519,6 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 	}
 	scope.Logger().Info("Reconciling Machine")
 
-	clusterName := names.ClusterName(cluster)
-
 	computeService, err := compute.NewService(scope)
 	if err != nil {
 		return ctrl.Result{}, err