Merge pull request #653 from hidekazuna/floating

✨ Delete automatically created floating IP when deleting cluster

Author: k8s-ci-robot

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)
+			}
 		}
 	}
 
@@ -392,26 +403,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
+}