Address the comments

Author: komer3

internal/controller/linodecluster_controller.go

@@ -229,34 +229,15 @@ func (r *LinodeClusterReconciler) performPreflightChecks(ctx context.Context, lo
 				// The condition is already set in reconcilePreflightLinodeVPCCheck, so we don't need to set it again
 				return res, err
 			}
-
-			// Only set to true if there was no error
-			conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
-				Type:   ConditionPreflightLinodeVPCReady,
-				Status: metav1.ConditionTrue,
-				Reason: "LinodeVPCReady", // We have to set the reason to not fail object patching
-			})
 		}
 	}
 
 	if clusterScope.LinodeCluster.Spec.NodeBalancerFirewallRef != nil {
 		if !reconciler.ConditionTrue(clusterScope.LinodeCluster, ConditionPreflightLinodeNBFirewallReady) {
 			res, err := r.reconcilePreflightLinodeFirewallCheck(ctx, logger, clusterScope)
 			if err != nil || !res.IsZero() {
-				conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
-					Type:   ConditionPreflightLinodeNBFirewallReady,
-					Status: metav1.ConditionFalse,
-					Reason: "LinodeFirewallNotYetAvailable", // We have to set the reason to not fail object patching
-				})
 				return res, err
 			}
-
-			// Only set to true if there was no error
-			conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
-				Type:   ConditionPreflightLinodeNBFirewallReady,
-				Status: metav1.ConditionTrue,
-				Reason: "LinodeFirewallReady", // We have to set the reason to not fail object patching
-			})
 		}
 	}
 
@@ -289,17 +270,37 @@ func (r *LinodeClusterReconciler) reconcilePreflightLinodeFirewallCheck(ctx cont
 				Reason:  util.CreateError,
 				Message: err.Error(),
 			})
+
 			return ctrl.Result{}, err
 		}
+		conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
+			Type:   ConditionPreflightLinodeNBFirewallReady,
+			Status: metav1.ConditionFalse,
+			Reason: "LinodeFirewallNotYetAvailable", // We have to set the reason to not fail object patching
+		})
+
 		return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
 	}
 	if linodeFirewall.Spec.FirewallID == nil {
 		logger.Info("Linode firewall not yet available")
+		conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
+			Type:   ConditionPreflightLinodeNBFirewallReady,
+			Status: metav1.ConditionFalse,
+			Reason: "LinodeFirewallNotYetAvailable", // We have to set the reason to not fail object patching
+		})
+
 		return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
 	}
 
 	r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "Linode firewall is now available")
 
+	// Only set to true if there was no error
+	conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
+		Type:   ConditionPreflightLinodeNBFirewallReady,
+		Status: metav1.ConditionTrue,
+		Reason: "LinodeFirewallReady", // We have to set the reason to not fail object patching
+	})
+
 	return ctrl.Result{}, nil
 }
 
@@ -316,7 +317,7 @@ func (r *LinodeClusterReconciler) reconcilePreflightLinodeVPCCheck(ctx context.C
 				Reason:  util.CreateError,
 				Message: fmt.Sprintf("VPC with ID %d not found: %v", vpcID, err),
 			})
-			return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, err
+			return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
 		}
 		// VPC exists, verify it has at least one subnet
 		if len(vpc.Subnets) == 0 {
@@ -328,9 +329,17 @@ func (r *LinodeClusterReconciler) reconcilePreflightLinodeVPCCheck(ctx context.C
 				Reason:  util.CreateError,
 				Message: err.Error(),
 			})
-			return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, err
+			return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
 		}
 		r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), fmt.Sprintf("VPC with ID %d is available", vpcID))
+
+		// Only set to true if there was no error
+		conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
+			Type:   ConditionPreflightLinodeVPCReady,
+			Status: metav1.ConditionTrue,
+			Reason: "LinodeVPCReady", // We have to set the reason to not fail object patching
+		})
+
 		return ctrl.Result{}, nil
 	}
 
@@ -359,12 +368,29 @@ func (r *LinodeClusterReconciler) reconcilePreflightLinodeVPCCheck(ctx context.C
 			})
 			return ctrl.Result{}, err
 		}
+		conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
+			Type:   ConditionPreflightLinodeVPCReady,
+			Status: metav1.ConditionFalse,
+			Reason: "LinodeVPCNotYetAvailable", // We have to set the reason to not fail object patching
+		})
 		return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
 	} else if !linodeVPC.Status.Ready {
 		logger.Info("LinodeVPC is not yet available")
+		conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
+			Type:   ConditionPreflightLinodeVPCReady,
+			Status: metav1.ConditionFalse,
+			Reason: "LinodeVPCNotYetAvailable", // We have to set the reason to not fail object patching
+		})
 		return ctrl.Result{RequeueAfter: reconciler.DefaultClusterControllerReconcileDelay}, nil
 	}
 	r.Recorder.Event(clusterScope.LinodeCluster, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "LinodeVPC is now available")
+
+	// Only set to true if there was no error
+	conditions.Set(clusterScope.LinodeCluster, metav1.Condition{
+		Type:   ConditionPreflightLinodeVPCReady,
+		Status: metav1.ConditionTrue,
+		Reason: "LinodeVPCReady", // We have to set the reason to not fail object patching
+	})
 	return ctrl.Result{}, nil
 }
 

internal/controller/linodemachine_controller.go

@@ -383,16 +383,13 @@ func (r *LinodeMachineReconciler) validateVPC(ctx context.Context, vpcID int, ma
 }
 
 func (r *LinodeMachineReconciler) reconcilePreflightVPC(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope, vpcRef *corev1.ObjectReference) (ctrl.Result, error) {
-	// Check if machine has direct VPCID
+	// LinodeMachine VPCID takes precedence over LinodeCluster VPCID
 	if machineScope.LinodeMachine.Spec.VPCID != nil {
 		if err := r.validateVPC(ctx, *machineScope.LinodeMachine.Spec.VPCID, machineScope, logger, "Machine"); err != nil {
 			return ctrl.Result{}, err
 		}
 		return ctrl.Result{}, nil
-	}
-
-	// Check if cluster has direct VPCID
-	if machineScope.LinodeCluster.Spec.VPCID != nil {
+	} else if machineScope.LinodeCluster.Spec.VPCID != nil {
 		if err := r.validateVPC(ctx, *machineScope.LinodeCluster.Spec.VPCID, machineScope, logger, "Cluster"); err != nil {
 			return ctrl.Result{}, err
 		}

internal/controller/linodemachine_controller_helpers.go

@@ -152,13 +152,10 @@ func newCreateConfig(ctx context.Context, machineScope *scope.MachineScope, gzip
 
 // configureVPCInterface handles all VPC configuration scenarios and adds the appropriate interface
 func configureVPCInterface(ctx context.Context, machineScope *scope.MachineScope, createConfig *linodego.InstanceCreateOptions, logger logr.Logger) error {
-	// First check if a direct VPCID is specified on the machine
+	// First check if a direct VPCID is specified on the machine then the cluster
 	if machineScope.LinodeMachine.Spec.VPCID != nil {
 		return addVPCInterfaceFromDirectID(ctx, machineScope, createConfig, logger, *machineScope.LinodeMachine.Spec.VPCID)
-	}
-
-	// Next check if a direct VPCID is specified on the cluster
-	if machineScope.LinodeCluster.Spec.VPCID != nil {
+	} else if machineScope.LinodeCluster.Spec.VPCID != nil {
 		return addVPCInterfaceFromDirectID(ctx, machineScope, createConfig, logger, *machineScope.LinodeCluster.Spec.VPCID)
 	}