review fixes

config/rbac/role.yaml

@@ -102,7 +102,6 @@ rules:
   - create
   - delete
   - get
-  - list
   - patch
   - update
   - watch

controllers/openstackmachine_controller.go

@@ -75,7 +75,7 @@ const (
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackmachines,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=openstackmachines/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machines;machines/status,verbs=get;list;watch
-// +kubebuilder:rbac:groups=ipam.cluster.x-k8s.io,resources=ipaddressclaims;ipaddressclaims/status,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=ipam.cluster.x-k8s.io,resources=ipaddressclaims;ipaddressclaims/status,verbs=get;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=ipam.cluster.x-k8s.io,resources=ipaddresses;ipaddresses/status,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=secrets;,verbs=get;list;watch
 // +kubebuilder:rbac:groups="",resources=events,verbs=get;list;watch;create;update;patch
@@ -359,7 +359,7 @@ func (r *OpenStackMachineReconciler) reconcileFloatingAddressFromPool(ctx contex
 
 		instanceAddresses := instanceNS.Addresses()
 		for _, instanceAddress := range instanceAddresses {
-			if instanceAddress.Address == address.Spec.Address {
+			if instanceAddress.Address == address.Spec.Address && instanceAddress.Type == corev1.NodeExternalIP {
 				openStackMachine.Status.FloatingAddressFromPoolReady = pointer.Bool(true)
 				conditions.MarkTrue(openStackMachine, infrav1.FloatingAddressFromPoolReadyCondition)
 				return nil
@@ -372,23 +372,30 @@ func (r *OpenStackMachineReconciler) reconcileFloatingAddressFromPool(ctx contex
 		}
 		if fip == nil {
 			conditions.MarkFalse(openStackMachine, infrav1.FloatingAddressFromPoolReadyCondition, infrav1.FloatingAddressFromPoolErrorReason, clusterv1.ConditionSeverityError, "floating IP does not exist")
-			return errors.New("floating IP does not exist in ")
+			return fmt.Errorf("floating IP %q does not exist", address.Spec.Address)
 		}
 
 		port, err := computeService.GetManagementPort(openStackCluster, instanceStatus)
 		if err != nil {
 			return err
 		}
 
-		if err = networkingService.AssociateFloatingIP(openStackMachine, fip, port.ID); err != nil {
-			return err
-		}
-
+		// Add finalizer to claim to prevent deletion until the floating IP is released
 		if controllerutil.AddFinalizer(claim, infrav1.IPClaimMachineFinalizer) {
 			if err := r.Client.Update(ctx, claim); err != nil {
 				return err
 			}
 		}
+		if err = networkingService.AssociateFloatingIP(openStackMachine, fip, port.ID); err != nil {
+			// Remove finalizer since we failed to associate the floating IP
+			if controllerutil.RemoveFinalizer(claim, infrav1.IPClaimMachineFinalizer) {
+				if err := r.Client.Update(ctx, claim); err != nil {
+					return err
+				}
+			}
+			return err
+		}
+
 		openStackMachine.Status.FloatingAddressFromPoolReady = pointer.Bool(true)
 		conditions.MarkTrue(openStackMachine, infrav1.FloatingAddressFromPoolReadyCondition)
 	} else {