[octavia-ingress-controller] Add annotations to keep floating IP and/or specify an existing floating IP (kubernetes#2166)

* Add annotation to keep floationIP

* Add annotation to specify floating ip to use on LB when creating ingress

* Add doc for octavia.ingress.kubernetes.io/keep-floatingip & octavia.ingress.kubernetes.io/floatingip annotations

* Remove debug logs

* Change annotation syntax, don't create a new FIP, if user requested a particular one, add additional check if FIP already binded to correct port, add ability to update FIP of an existing ingress by updating annotation

* Add missing else

* Log format

* Create fonctions to attach/detach fips to port

* Fix bug when no fip provided in annotation the lb was created in private mode and improve openstack neutron fip logic

Author: ccleouf66

docs/octavia-ingress-controller/using-octavia-ingress-controller.md

@@ -15,6 +15,7 @@
     - [Create an Ingress resource](#create-an-ingress-resource)
   - [Enable TLS encryption](#enable-tls-encryption)
   - [Allow CIDRs](#allow-cidrs)
+  - [Creating Ingress by specifying a floating IP](#creating-ingress-by-specifying-a-floating-ip)
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
@@ -504,3 +505,50 @@ spec:
               port:
                 number: 8080
 ```
+
+## Creating Ingress by specifying a floating IP
+
+Sometimes it's useful to use an existing available floating IP rather than creating a new one, especially in the automation scenario. In the example below, 122.112.219.229 is an available floating IP created in the OpenStack Networking service.
+
+You can also specify to not delete the floating IP when the ingress will be deleted. By default, if not specified, the floating IP
+is deleted with the loadbalancer when the ingress if removed on kubernetes. 
+
+Create a new depolyment:
+```shell script
+kubectl create deployment test-web --replicas 3 --image nginx --port 80
+```
+
+Create a service type NodePort:
+```shell script
+kubectl expose deployment test-web --type NodePort
+```
+
+Create an ingress using a specific floating IP:
+```yaml
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: test-web-ingress
+  annotations:
+    kubernetes.io/ingress.class: "openstack"
+    octavia.ingress.kubernetes.io/internal: "false"
+    octavia.ingress.kubernetes.io/keep-floatingip: "true"        # floating ip will not be deleted when ingress is deleted
+    octavia.ingress.kubernetes.io/floatingip: "122.112.219.229"  # define the floating to use 
+spec:
+  rules:
+  - host: test-web.foo.bar.com
+    http:
+      paths:
+      - path: /
+        pathType: Prefix
+        backend:
+          service:
+            name: test-web
+            port:
+              number: 80
+```
+
+If the floating IP is available you can test it with:
+```shell script
+curl -H "host: test-web.foo.bar.com" http://122.112.219.229
+```

pkg/ingress/controller/controller.go

@@ -101,6 +101,16 @@ const (
 	// Default to true.
 	IngressAnnotationInternal = "octavia.ingress.kubernetes.io/internal"
 
+	// IngressAnnotationLoadBalancerKeepFloatingIP is the annotation used on the Ingress
+	// to indicate that we want to keep the floatingIP after the ingress deletion. The Octavia LoadBalancer will be deleted
+	// but not the floatingIP. That mean this floatingIP can be reused on another ingress without editing the dns area or update the whitelist.
+	// Default to false.
+	IngressAnnotationLoadBalancerKeepFloatingIP = "octavia.ingress.kubernetes.io/keep-floatingip"
+
+	// IngressAnnotationFloatingIp is the key of the annotation on an ingress to set floating IP that will be associated to LoadBalancers.
+	// If the floatingIP is not available, an error will be returned.
+	IngressAnnotationFloatingIP = "octavia.ingress.kubernetes.io/floatingip"
+
 	// IngressAnnotationSourceRangesKey is the key of the annotation on an ingress to set allowed IP ranges on their LoadBalancers.
 	// It should be a comma-separated list of CIDRs.
 	IngressAnnotationSourceRangesKey = "octavia.ingress.kubernetes.io/whitelist-source-range"
@@ -589,15 +599,24 @@ func (c *Controller) deleteIngress(ing *nwv1.Ingress) error {
 		return nil
 	}
 
-	// Delete the floating IP for the load balancer VIP. We don't check if the Ingress is internal or not, just delete
-	// any floating IPs associated with the load balancer VIP port.
-	logger.Debug("deleting floating IP")
-
-	if _, err = c.osClient.EnsureFloatingIP(true, loadbalancer.VipPortID, "", ""); err != nil {
-		return fmt.Errorf("failed to delete floating IP: %v", err)
+	// Manage the floatingIP
+	keepFloatingSetting := getStringFromIngressAnnotation(ing, IngressAnnotationLoadBalancerKeepFloatingIP, "false")
+	keepFloating, err := strconv.ParseBool(keepFloatingSetting)
+	if err != nil {
+		return fmt.Errorf("unknown annotation %s: %v", IngressAnnotationLoadBalancerKeepFloatingIP, err)
 	}
 
-	logger.WithFields(log.Fields{"lbID": loadbalancer.ID}).Info("VIP or floating IP deleted")
+	if !keepFloating {
+		// Delete the floating IP for the load balancer VIP. We don't check if the Ingress is internal or not, just delete
+		// any floating IPs associated with the load balancer VIP port.
+		logger.WithFields(log.Fields{"lbID": loadbalancer.ID, "VIP": loadbalancer.VipAddress}).Info("deleting floating IPs associated with the load balancer VIP port")
+
+		if _, err = c.osClient.EnsureFloatingIP(true, loadbalancer.VipPortID, "", "", ""); err != nil {
+			return fmt.Errorf("failed to delete floating IP: %v", err)
+		}
+
+		logger.WithFields(log.Fields{"lbID": loadbalancer.ID}).Info("VIP or floating IP deleted")
+	}
 
 	// Delete security group managed for the Ingress backend service
 	if c.config.Octavia.ManageSecurityGroups {
@@ -934,15 +953,24 @@ func (c *Controller) ensureIngress(ing *nwv1.Ingress) error {
 	address := lb.VipAddress
 	// Allocate floating ip for loadbalancer vip if the external network is configured and the Ingress is not internal.
 	if !isInternal && c.config.Octavia.FloatingIPNetwork != "" {
-		logger.Info("creating floating IP")
 
-		description := fmt.Sprintf("Floating IP for Kubernetes ingress %s in namespace %s from cluster %s", ingName, ingNamespace, clusterName)
-		address, err = c.osClient.EnsureFloatingIP(false, lb.VipPortID, c.config.Octavia.FloatingIPNetwork, description)
+		floatingIPSetting := getStringFromIngressAnnotation(ing, IngressAnnotationFloatingIP, "")
 		if err != nil {
-			return fmt.Errorf("failed to create floating IP: %v", err)
+			return fmt.Errorf("unknown annotation %s: %v", IngressAnnotationFloatingIP, err)
 		}
 
-		logger.WithFields(log.Fields{"fip": address}).Info("floating IP created")
+		description := fmt.Sprintf("Floating IP for Kubernetes ingress %s in namespace %s from cluster %s", ingName, ingNamespace, clusterName)
+
+		if floatingIPSetting != "" {
+			logger.Info("try to use floating IP: ", floatingIPSetting)
+		} else {
+			logger.Info("creating new floating IP")
+		}
+		address, err = c.osClient.EnsureFloatingIP(false, lb.VipPortID, floatingIPSetting, c.config.Octavia.FloatingIPNetwork, description)
+		if err != nil {
+			return fmt.Errorf("failed to use provided floating IP %s : %v", floatingIPSetting, err)
+		}
+		logger.Info("floating IP ", address, " configured")
 	}
 
 	// Update ingress status

pkg/ingress/controller/openstack/neutron.go

@@ -47,6 +47,33 @@ func (os *OpenStack) getFloatingIPs(listOpts floatingips.ListOpts) ([]floatingip
 	return allFIPs, nil
 }
 
+func (os *OpenStack) createFloatingIP(portID string, floatingNetworkID string, description string) (*floatingips.FloatingIP, error) {
+	floatIPOpts := floatingips.CreateOpts{
+		PortID:            portID,
+		FloatingNetworkID: floatingNetworkID,
+		Description:       description,
+	}
+	return floatingips.Create(os.neutron, floatIPOpts).Extract()
+}
+
+// associateFloatingIP associate an unused floating IP to a given Port
+func (os *OpenStack) associateFloatingIP(fip *floatingips.FloatingIP, portID string, description string) (*floatingips.FloatingIP, error) {
+	updateOpts := floatingips.UpdateOpts{
+		PortID:      &portID,
+		Description: &description,
+	}
+	return floatingips.Update(os.neutron, fip.ID, updateOpts).Extract()
+}
+
+// disassociateFloatingIP disassociate a floating IP from a port
+func (os *OpenStack) disassociateFloatingIP(fip *floatingips.FloatingIP, description string) (*floatingips.FloatingIP, error) {
+	updateDisassociateOpts := floatingips.UpdateOpts{
+		PortID:      new(string),
+		Description: &description,
+	}
+	return floatingips.Update(os.neutron, fip.ID, updateDisassociateOpts).Extract()
+}
+
 // GetSubnet get a subnet by the given ID.
 func (os *OpenStack) GetSubnet(subnetID string) (*subnets.Subnet, error) {
 	subnet, err := subnets.Get(os.neutron, subnetID).Extract()
@@ -71,7 +98,7 @@ func (os *OpenStack) getPorts(listOpts ports.ListOpts) ([]ports.Port, error) {
 }
 
 // EnsureFloatingIP makes sure a floating IP is allocated for the port
-func (os *OpenStack) EnsureFloatingIP(needDelete bool, portID string, floatingIPNetwork string, description string) (string, error) {
+func (os *OpenStack) EnsureFloatingIP(needDelete bool, portID string, existingfloatingIP string, floatingIPNetwork string, description string) (string, error) {
 	listOpts := floatingips.ListOpts{PortID: portID}
 	fips, err := os.getFloatingIPs(listOpts)
 	if err != nil {
@@ -94,18 +121,64 @@ func (os *OpenStack) EnsureFloatingIP(needDelete bool, portID string, floatingIP
 	}
 
 	var fip *floatingips.FloatingIP
-	if len(fips) == 0 {
-		floatIPOpts := floatingips.CreateOpts{
-			PortID:            portID,
+
+	if existingfloatingIP == "" {
+		if len(fips) == 1 {
+			fip = &fips[0]
+		} else {
+			fip, err = os.createFloatingIP(portID, floatingIPNetwork, description)
+			if err != nil {
+				return "", err
+			}
+		}
+	} else {
+		// if user provide FIP
+		// check if provided fip is available
+		opts := floatingips.ListOpts{
+			FloatingIP:        existingfloatingIP,
 			FloatingNetworkID: floatingIPNetwork,
-			Description:       description,
 		}
-		fip, err = floatingips.Create(os.neutron, floatIPOpts).Extract()
+		osFips, err := os.getFloatingIPs(opts)
 		if err != nil {
 			return "", err
 		}
-	} else {
-		fip = &fips[0]
+		if len(osFips) != 1 {
+			return "", fmt.Errorf("error when searching floating IPs %s, %d floating IPs found", existingfloatingIP, len(osFips))
+		}
+		// check if fip is already attached to the correct port
+		if osFips[0].PortID == portID {
+			return osFips[0].FloatingIP, nil
+		}
+		// check if fip is already used by other port
+		// We might consider if here we shouldn't detach that FIP instead of returning error
+		if osFips[0].PortID != "" {
+			return "", fmt.Errorf("floating IP %s already used by port %s", osFips[0].FloatingIP, osFips[0].PortID)
+		}
+
+		// if port don't have fip
+		if len(fips) == 0 {
+			fip, err = os.associateFloatingIP(&osFips[0], portID, description)
+			if err != nil {
+				return "", err
+			}
+		} else if osFips[0].FloatingIP != fips[0].FloatingIP {
+			// disassociate old fip : if update fip without disassociate
+			// Openstack retrun http 409 error
+			// "Cannot associate floating IP with port using fixed
+			//  IP, as that fixed IP already has a floating IP on
+			//  external network"
+			_, err = os.disassociateFloatingIP(&fips[0], "")
+			if err != nil {
+				return "", err
+			}
+			// associate new fip
+			fip, err = os.associateFloatingIP(&osFips[0], portID, description)
+			if err != nil {
+				return "", err
+			}
+		} else {
+			fip = &fips[0]
+		}
 	}
 
 	return fip.FloatingIP, nil