Tag floating IPs to try to prevent orphaned IPs

Author: bilbobrovall

api/v1alpha8/openstackfloatingippool_types.go

@@ -17,6 +17,8 @@ limitations under the License.
 package v1alpha8
 
 import (
+	"fmt"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
@@ -117,6 +119,10 @@ func (r *OpenStackFloatingIPPool) SetConditions(conditions clusterv1.Conditions)
 	r.Status.Conditions = conditions
 }
 
+func (r *OpenStackFloatingIPPool) GetFloatingIPTag() string {
+	return fmt.Sprintf("cluster-api-provider-openstack-fip-pool-%s", r.Name)
+}
+
 func init() {
 	SchemeBuilder.Register(&OpenStackFloatingIPPool{}, &OpenStackFloatingIPPoolList{})
 }

config/rbac/role.yaml

@@ -137,23 +137,3 @@ rules:
   - list
   - update
   - watch
-- apiGroups:
-  - ipam.cluster.x-k8x.io.cluster.x-k8s.io
-  resources:
-  - ipaddresses
-  verbs:
-  - create
-  - delete
-  - get
-  - list
-  - patch
-  - update
-  - watch
-- apiGroups:
-  - ipam.cluster.x-k8x.io.cluster.x-k8s.io
-  resources:
-  - ipaddresses/status
-  verbs:
-  - get
-  - patch
-  - update

controllers/ipaddress_controller.go

@@ -20,12 +20,14 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"time"
 
 	"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/external"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
@@ -47,6 +49,15 @@ const (
 	openStackFloatingIPPool = "OpenStackFloatingIPPool"
 )
 
+var (
+	backoff = wait.Backoff{
+		Steps:    4,
+		Duration: 10 * time.Millisecond,
+		Factor:   5.0,
+		Jitter:   0.1,
+	}
+)
+
 // OpenStackFloatingIPPoolReconciler reconciles a OpenStackFloatingIPPool object.
 type OpenStackFloatingIPPoolReconciler struct {
 	Client           client.Client
@@ -158,7 +169,16 @@ func (r *OpenStackFloatingIPPoolReconciler) Reconcile(ctx context.Context, req c
 						},
 					}
 
-					if err = r.Client.Create(ctx, ipAddress); err != nil {
+					// Retry creating the IPAddress object
+					err = wait.ExponentialBackoff(backoff, func() (bool, error) {
+						if err := r.Client.Create(ctx, ipAddress); err != nil {
+							return false, nil
+						}
+						return true, nil
+					})
+					if err != nil {
+						// If we failed to create the IPAddress, there might be an IP leak in OpenStack if we also failed to tag the IP after creation
+						scope.Logger().Error(err, "Failed to create IPAddress", "ip", ip)
 						return ctrl.Result{}, err
 					}
 				} else {
@@ -283,6 +303,7 @@ func (r *OpenStackFloatingIPPoolReconciler) reconcileIPAddresses(ctx context.Con
 }
 
 func (r *OpenStackFloatingIPPoolReconciler) getIP(scope scope.Scope, pool *infrav1.OpenStackFloatingIPPool) (string, error) {
+	// There's a potential leak of IPs here, if the reconcile loop fails after we claim an IP but before we create the IPAddress object.
 	var ip string
 
 	networkingService, err := networking.NewService(scope)
@@ -291,6 +312,21 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(scope scope.Scope, pool *infra
 		return "", err
 	}
 
+	// Get tagged floating IPs and add them to the available IPs if they are not present in either the available IPs or the claimed IPs
+	// This is done to prevent leaking floating IPs if to prevent leaking floating IPs if the floating IP was created but the IPAddress object was not
+	taggedFIPs, err := networkingService.GetFloatingIPsByTag(pool.GetFloatingIPTag())
+	if err != nil {
+		scope.Logger().Error(err, "Failed to get floating IPs by tag", "pool", pool.Name)
+		return "", err
+	}
+	for _, taggedIp := range taggedFIPs {
+		if contains(pool.Status.AvailableIPs, taggedIp.FloatingIP) || contains(pool.Status.ClaimedIPs, taggedIp.FloatingIP) {
+			continue
+		}
+		scope.Logger().Info("Tagged floating IP found that was not known to the pool, adding it to the pool", "ip", taggedIp.FloatingIP)
+		pool.Status.AvailableIPs = append(pool.Status.AvailableIPs, taggedIp.FloatingIP)
+	}
+
 	if len(pool.Status.AvailableIPs) > 0 {
 		ip = pool.Status.AvailableIPs[0]
 		pool.Status.AvailableIPs = pool.Status.AvailableIPs[1:]
@@ -315,6 +351,22 @@ func (r *OpenStackFloatingIPPoolReconciler) getIP(scope scope.Scope, pool *infra
 		}
 		return "", err
 	}
+	defer func() {
+		tag := pool.GetFloatingIPTag()
+
+		err := wait.ExponentialBackoff(backoff, func() (bool, error) {
+			if err := networkingService.TagFloatingIP(fp.FloatingIP, tag); err != nil {
+				scope.Logger().Error(err, "Failed to tag floating, retrying", "ip", fp.FloatingIP, "tag", tag)
+				return false, err
+			}
+			return true, nil
+		})
+
+		if err != nil {
+			scope.Logger().Error(err, "Failed to tag floating IP", "ip", fp.FloatingIP, "tag", tag)
+		}
+	}()
+
 	conditions.MarkTrue(pool, infrav1.OpenstackFloatingIPPoolReadyCondition)
 
 	ip = fp.FloatingIP

controllers/openstackfloatingippool_controller.go

@@ -87,6 +87,33 @@ func (s *Service) CreateFloatingIPForPool(pool *infrav1.OpenStackFloatingIPPool)
 	return fp, nil
 }
 
+func (s *Service) TagFloatingIP(ip string, tag string) error {
+	fip, err := s.GetFloatingIP(ip)
+	if err != nil {
+		return err
+	}
+	if fip == nil {
+		return nil
+	}
+
+	mc := metrics.NewMetricPrometheusContext("floating_ip", "update")
+	_, err = s.client.ReplaceAllAttributesTags("floatingips", fip.ID, attributestags.ReplaceAllOpts{
+		Tags: []string{tag},
+	})
+	if mc.ObserveRequest(err) != nil {
+		return err
+	}
+	return nil
+}
+
+func (s *Service) GetFloatingIPsByTag(tag string) ([]floatingips.FloatingIP, error) {
+	fipList, err := s.client.ListFloatingIP(floatingips.ListOpts{Tags: tag})
+	if err != nil {
+		return nil, err
+	}
+	return fipList, nil
+}
+
 func (s *Service) GetFloatingIP(ip string) (*floatingips.FloatingIP, error) {
 	fpList, err := s.client.ListFloatingIP(floatingips.ListOpts{FloatingIP: ip})
 	if err != nil {