Delete floating IP if not specified explicitly

Currently floating IP remains after cluster deleted.
This commit deletes floating IP when deleting cluster if it is not in
the specs.

Both Non HA and HA uses common parameters to set
openStackCluster.Spec.ControlPlaneEndpoint.
- APIServerFloatingIP
- APIServerPort

Author: hidekazuna

api/v1alpha3/openstackcluster_types.go

@@ -68,18 +68,18 @@ type OpenStackClusterSpec struct {
 
 	// ManagedAPIServerLoadBalancer defines whether a LoadBalancer for the
 	// APIServer should be created. If set to true the following properties are
-	// mandatory: APIServerLoadBalancerFloatingIP, APIServerLoadBalancerPort
+	// mandatory: APIServerFloatingIP, APIServerPort
 	// +optional
 	ManagedAPIServerLoadBalancer bool `json:"managedAPIServerLoadBalancer"`
 
-	// APIServerLoadBalancerFloatingIP is the floatingIP which will be associated
-	// to the APIServer loadbalancer. The floatingIP will be created if it not
+	// APIServerFloatingIP is the floatingIP which will be associated
+	// to the APIServer. The floatingIP will be created if it not
 	// already exists.
-	APIServerLoadBalancerFloatingIP string `json:"apiServerLoadBalancerFloatingIP,omitempty"`
+	APIServerFloatingIP string `json:"apiServerFloatingIP,omitempty"`
 
-	// APIServerLoadBalancerPort is the port on which the listener on the APIServer
-	// loadbalancer will be created
-	APIServerLoadBalancerPort int `json:"apiServerLoadBalancerPort,omitempty"`
+	// APIServerPort is the port on which the listener on the APIServer
+	// will be created
+	APIServerPort int `json:"apiServerPort,omitempty"`
 
 	// APIServerLoadBalancerAdditionalPorts adds additional ports to the APIServerLoadBalancer
 	APIServerLoadBalancerAdditionalPorts []int `json:"apiServerLoadBalancerAdditionalPorts,omitempty"`

config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml

@@ -64,20 +64,20 @@ spec:
           spec:
             description: OpenStackClusterSpec defines the desired state of OpenStackCluster
             properties:
+              apiServerFloatingIP:
+                description: APIServerFloatingIP is the floatingIP which will be associated
+                  to the APIServer. The floatingIP will be created if it not already
+                  exists.
+                type: string
               apiServerLoadBalancerAdditionalPorts:
                 description: APIServerLoadBalancerAdditionalPorts adds additional
                   ports to the APIServerLoadBalancer
                 items:
                   type: integer
                 type: array
-              apiServerLoadBalancerFloatingIP:
-                description: APIServerLoadBalancerFloatingIP is the floatingIP which
-                  will be associated to the APIServer loadbalancer. The floatingIP
-                  will be created if it not already exists.
-                type: string
-              apiServerLoadBalancerPort:
-                description: APIServerLoadBalancerPort is the port on which the listener
-                  on the APIServer loadbalancer will be created
+              apiServerPort:
+                description: APIServerPort is the port on which the listener on the
+                  APIServer will be created
                 type: integer
               bastion:
                 description: Bastion is the OpenStack instance to login the nodes
@@ -361,7 +361,7 @@ spec:
               managedAPIServerLoadBalancer:
                 description: 'ManagedAPIServerLoadBalancer defines whether a LoadBalancer
                   for the APIServer should be created. If set to true the following
-                  properties are mandatory: APIServerLoadBalancerFloatingIP, APIServerLoadBalancerPort'
+                  properties are mandatory: APIServerFloatingIP, APIServerPort'
                 type: boolean
               managedSecurityGroups:
                 description: 'ManagedSecurityGroups defines that kubernetes manages

controllers/openstackcluster_controller.go

@@ -134,6 +134,12 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, log lo
 				return reconcile.Result{}, errors.Errorf("failed to delete bastion: %v", err)
 			}
 			r.Recorder.Eventf(openStackCluster, corev1.EventTypeNormal, "SuccessfulDeleteServer", "Deleted server %s with id %s", bastion.Name, bastion.ID)
+			if openStackCluster.Spec.Bastion.FloatingIP == "" {
+				if err = networkingService.DeleteFloatingIP(bastion.FloatingIP); err != nil {
+					return reconcile.Result{}, errors.Errorf("failed to delete floating IP: %v", err)
+				}
+				r.Recorder.Eventf(openStackCluster, corev1.EventTypeNormal, "SuccessfulDeleteFloatingIP", "Deleted floating IP %s", bastion.FloatingIP)
+			}
 		}
 
 		if bastionSecGroup := openStackCluster.Status.BastionSecurityGroup; bastionSecGroup != nil {
@@ -142,7 +148,6 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, log lo
 				return reconcile.Result{}, errors.Errorf("failed to delete security group: %v", err)
 			}
 			r.Recorder.Eventf(openStackCluster, corev1.EventTypeNormal, "SuccessfulDeleteSecurityGroup", "Deleted security group %s with id %s", bastionSecGroup.Name, bastionSecGroup.ID)
-
 		}
 	}
 
@@ -158,6 +163,12 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, log lo
 			}
 			r.Recorder.Eventf(openStackCluster, corev1.EventTypeNormal, "SuccessfulDeleteLoadBalancer", "Deleted load balancer %s with id %s", apiLb.Name, apiLb.ID)
 
+			if openStackCluster.Spec.APIServerFloatingIP == "" {
+				if err = networkingService.DeleteFloatingIP(apiLb.IP); err != nil {
+					return reconcile.Result{}, errors.Errorf("failed to delete floating IP: %v", err)
+				}
+				r.Recorder.Eventf(openStackCluster, corev1.EventTypeNormal, "SuccessfulDeleteFloatingIP", "Deleted floating IP %s", apiLb.IP)
+			}
 		}
 	}
 
@@ -389,26 +400,14 @@ func (r *OpenStackClusterReconciler) reconcileNetworkComponents(log logr.Logger,
 			return errors.Errorf("failed to reconcile router: %v", err)
 		}
 	}
-
-	if openStackCluster.Spec.ControlPlaneEndpoint.IsZero() {
-		var controlPlaneEndpointHost string
+	if !openStackCluster.Spec.ControlPlaneEndpoint.IsValid() {
 		var port int32
-		if openStackCluster.Spec.ManagedAPIServerLoadBalancer {
-			controlPlaneEndpointHost = openStackCluster.Spec.APIServerLoadBalancerFloatingIP
-			if openStackCluster.Spec.APIServerLoadBalancerPort == 0 {
-				port = 6443
-			} else {
-				port = int32(openStackCluster.Spec.APIServerLoadBalancerPort)
-			}
+		if openStackCluster.Spec.APIServerPort == 0 {
+			port = 6443
 		} else {
-			controlPlaneEndpointHost = openStackCluster.Spec.ControlPlaneEndpoint.Host
-			if openStackCluster.Spec.ControlPlaneEndpoint.Port == 0 {
-				port = 6443
-			} else {
-				port = openStackCluster.Spec.ControlPlaneEndpoint.Port
-			}
+			port = int32(openStackCluster.Spec.APIServerPort)
 		}
-		fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, controlPlaneEndpointHost)
+		fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster.Spec.APIServerFloatingIP)
 		if err != nil {
 			return errors.Errorf("Floating IP cannot be got or created: %v", err)
 		}

controllers/openstackmachine_controller.go

@@ -202,6 +202,11 @@ func (r *OpenStackMachineReconciler) reconcileDelete(ctx context.Context, logger
 		return ctrl.Result{}, err
 	}
 
+	networkingService, err := networking.NewService(osProviderClient, clientOpts, logger)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+
 	loadBalancerService, err := loadbalancer.NewService(osProviderClient, clientOpts, logger, openStackCluster.Spec.UseOctavia)
 	if err != nil {
 		return ctrl.Result{}, err
@@ -236,6 +241,15 @@ func (r *OpenStackMachineReconciler) reconcileDelete(ctx context.Context, logger
 	logger.Info("OpenStack machine deleted successfully")
 	r.Recorder.Eventf(openStackMachine, corev1.EventTypeNormal, "SuccessfulDeleteServer", "Deleted server %s with id %s", instance.Name, instance.ID)
 
+	if util.IsControlPlaneMachine(machine) && openStackCluster.Spec.APIServerFloatingIP == "" {
+		if err = networkingService.DeleteFloatingIP(instance.FloatingIP); err != nil {
+			handleUpdateMachineError(logger, openStackMachine, errors.Errorf("error deleting Openstack floating IP: %v", err))
+			return ctrl.Result{}, nil
+		}
+		logger.Info("OpenStack floating IP deleted successfully", "Floating IP", instance.FloatingIP)
+		r.Recorder.Eventf(openStackMachine, corev1.EventTypeNormal, "SuccessfulDeleteFloatingIP", "Deleted floating IP %s", instance.FloatingIP)
+	}
+
 	// Instance is deleted so remove the finalizer.
 	controllerutil.RemoveFinalizer(openStackMachine, infrav1.MachineFinalizer)
 	logger.Info("Reconciled Machine delete successfully")

docs/configuration.md

@@ -114,7 +114,7 @@ Note: If your openstack cluster does not already have a public network, you shou
 
 ## Floating IP
 
-A floating IP is automatically created and associated with the load balancer or controller node, but you can specify the floating IP explicitly. When `managedAPIServerLoadBalancer: true`, `spec.apiServerLoadBalancerFlotingIP` of `OpenStackCluster` is used. When `managedAPIServerLoadBalancer: false`, `spec.controlPlaneEndpoint.host` of `OpenStackCluster` is used.
+A floating IP is automatically created and associated with the load balancer or controller node, but you can specify the floating IP explicitly by `spec.apiServerFloatingIP` of `OpenStackCluster`.
 
 You have to be able to create a floating IP in your OpenStack in advance. You can create one using,
 

pkg/cloud/services/networking/floatingip.go

@@ -66,3 +66,14 @@ func checkIfFloatingIPExists(client *gophercloud.ServiceClient, ip string) (*flo
 	}
 	return &fpList[0], nil
 }
+
+func (s *Service) DeleteFloatingIP(ip string) error {
+	fip, err := checkIfFloatingIPExists(s.client, ip)
+	if err != nil {
+		return err
+	}
+	if fip != nil {
+		return floatingips.Delete(s.client, fip.ID).ExtractErr()
+	}
+	return nil
+}