fixup! ipam core addition 2

Author: cheina97

pkg/ipam/initialize.go

@@ -16,8 +16,6 @@ package ipam
 
 import (
 	"context"
-	"fmt"
-	"net/netip"
 
 	klog "k8s.io/klog/v2"
 )
@@ -26,13 +24,19 @@ import (
 // +kubebuilder:rbac:groups=ipam.liqo.io,resources=networks,verbs=get;list;watch
 
 func (lipam *LiqoIPAM) initialize(ctx context.Context) error {
+	klog.Info("Initializing IPAM")
+
+	klog.Info("Initializing networks")
 	if err := lipam.initializeNetworks(ctx); err != nil {
 		return err
 	}
+	klog.Info("Networks initialized")
 
+	klog.Info("Initializing IPs")
 	if err := lipam.initializeIPs(ctx); err != nil {
 		return err
 	}
+	klog.Info("IPs initialized")
 
 	klog.Info("IPAM initialized")
 	return nil
@@ -47,11 +51,7 @@ func (lipam *LiqoIPAM) initializeNetworks(ctx context.Context) error {
 
 	// Initialize the networks.
 	for _, net := range nets {
-		prefix := netip.MustParsePrefix(net)
-
-		if result := lipam.IpamCore.AllocateNetworkWithPrefix(prefix); result == nil {
-			err := fmt.Errorf("failed to reserve network %q", net)
-			klog.Errorf("%v", err)
+		if _, err := lipam.acquireSpecificNetwork(net); err != nil {
 			return err
 		}
 	}
@@ -68,7 +68,7 @@ func (lipam *LiqoIPAM) initializeIPs(ctx context.Context) error {
 
 	// Initialize the IPs.
 	for _, ip := range ips {
-		if err := lipam.acquireIPWithAddress(ip.cidr, ip.ip); err != nil {
+		if _, err := lipam.acquireIPWithAddress(ip.cidr, ip.ip); err != nil {
 			klog.Errorf("Failed to reserve IP %q (network %q): %v", ip.ip, ip.cidr, err)
 			return err
 		}

pkg/ipam/ipam.go

@@ -94,9 +94,19 @@ func (lipam *LiqoIPAM) IPRelease(_ context.Context, req *IPReleaseRequest) (*IPR
 
 // NetworkAcquire acquires a network. If it is already reserved, it allocates and reserves a new free one with the same prefix length.
 func (lipam *LiqoIPAM) NetworkAcquire(_ context.Context, req *NetworkAcquireRequest) (*NetworkAcquireResponse, error) {
-	remappedCidr, err := lipam.acquireNetwork(req.GetCidr(), req.GetImmutable())
-	if err != nil {
-		return &NetworkAcquireResponse{}, err
+	var remappedCidr string
+	var err error
+
+	if req.GetImmutable() {
+		remappedCidr, err = lipam.acquireSpecificNetwork(req.GetCidr())
+		if err != nil {
+			return &NetworkAcquireResponse{}, err
+		}
+	} else {
+		remappedCidr, err = lipam.acquireNetwork(req.GetCidr())
+		if err != nil {
+			return &NetworkAcquireResponse{}, err
+		}
 	}
 
 	return &NetworkAcquireResponse{Cidr: remappedCidr}, nil

pkg/ipam/ips.go

@@ -57,7 +57,7 @@ func (lipam *LiqoIPAM) acquireIP(cidr string) (string, error) {
 }
 
 // acquireIpWithAddress acquires an IP with a specific address.
-func (lipam *LiqoIPAM) acquireIPWithAddress(cidr, ip string) error {
+func (lipam *LiqoIPAM) acquireIPWithAddress(cidr, ip string) (string, error) {
 	lipam.mutex.Lock()
 	defer lipam.mutex.Unlock()
 
@@ -66,11 +66,11 @@ func (lipam *LiqoIPAM) acquireIPWithAddress(cidr, ip string) error {
 
 	result := lipam.IpamCore.AllocateIPWithAddr(prefix, addr)
 	if result == nil {
-		return fmt.Errorf("failed to reserve IP %q in network %q", ip, cidr)
+		return "", fmt.Errorf("failed to reserve IP %q in network %q", ip, cidr)
 	}
 
 	klog.Infof("Acquired IP %q (network %q)", result.String(), cidr)
-	return nil
+	return result.String(), nil
 }
 
 // freeIP frees an IP, removing it from the cache.

pkg/ipam/networks.go

@@ -16,6 +16,7 @@ package ipam
 
 import (
 	"context"
+	"fmt"
 	"net/netip"
 
 	klog "k8s.io/klog/v2"
@@ -25,7 +26,7 @@ import (
 )
 
 // acquireNetwork acquires a network, eventually remapped if conflicts are found.
-func (lipam *LiqoIPAM) acquireNetwork(cidr string, immutable bool) (string, error) {
+func (lipam *LiqoIPAM) acquireNetwork(cidr string) (string, error) {
 	lipam.mutex.Lock()
 	defer lipam.mutex.Unlock()
 
@@ -34,12 +35,32 @@ func (lipam *LiqoIPAM) acquireNetwork(cidr string, immutable bool) (string, erro
 	result := lipam.IpamCore.AllocateNetworkWithPrefix(prefix)
 	if result == nil {
 		result = lipam.IpamCore.AllocateNetwork(prefix.Bits())
+		if result == nil {
+			return "", fmt.Errorf("failed to reserve network %q", cidr)
+		}
 	}
 
 	klog.Infof("Acquired network %q -> %q", cidr, result.String())
 	return cidr, nil
 }
 
+// acquireSpecificNetwork acquires a network with a specific prefix.
+// If the network is already allocated, it returns an error.
+func (lipam *LiqoIPAM) acquireSpecificNetwork(cidr string) (string, error) {
+	lipam.mutex.Lock()
+	defer lipam.mutex.Unlock()
+
+	prefix := netip.MustParsePrefix(cidr)
+
+	result := lipam.IpamCore.AllocateNetworkWithPrefix(prefix)
+	if result == nil {
+		return "", fmt.Errorf("failed to reserve network %q", cidr)
+	}
+
+	klog.Infof("Acquired network %q", result.String())
+	return result.String(), nil
+}
+
 // freeNetwork frees a network, removing it from the cache.
 func (lipam *LiqoIPAM) freeNetwork(cidr string) {
 	lipam.mutex.Lock()

pkg/ipam/sync.go

@@ -16,7 +16,6 @@ package ipam
 
 import (
 	"context"
-	"fmt"
 	"net/netip"
 	"os"
 	"time"
@@ -36,15 +35,19 @@ func (lipam *LiqoIPAM) sync(ctx context.Context, syncFrequency time.Duration) {
 			// networks created before this threshold will be removed from the cache if they are not present in the cluster.
 			expiredThreshold := now.Add(-syncFrequency)
 
+			klog.Info("Syncing networks")
 			// Sync networks.
 			if err := lipam.syncNetworks(ctx, expiredThreshold); err != nil {
 				return false, err
 			}
+			klog.Info("Networks synced")
 
+			klog.Info("Syncing IPs")
 			// Sync IPs.
 			if err := lipam.syncIPs(ctx, expiredThreshold); err != nil {
 				return false, err
 			}
+			klog.Info("IPs synced")
 
 			klog.Info("Completed IPAM cache sync routine")
 			return false, nil
@@ -64,10 +67,7 @@ func (lipam *LiqoIPAM) syncNetworks(ctx context.Context, expiredThreshold time.T
 
 	// Add networks that are present in the cluster but not in the cache.
 	for _, net := range clusterNetworks {
-		prefix := netip.MustParsePrefix(net)
-		if result := lipam.IpamCore.AllocateNetworkWithPrefix(prefix); result != nil {
-			klog.Infof("Sync: Acquired network %q -> %q", net, result.String())
-		}
+		_, _ = lipam.acquireSpecificNetwork(net)
 	}
 
 	// Remove networks that are present in the cache but not in the cluster, and were added before the threshold.
@@ -81,11 +81,7 @@ func (lipam *LiqoIPAM) syncNetworks(ctx context.Context, expiredThreshold time.T
 			}
 		}
 		if !found {
-			if result := lipam.IpamCore.FreeNetwork(currentNetworks[i]); result != nil {
-				klog.Infof("Sync: Released network %q", result.String())
-			} else {
-				return fmt.Errorf("sync: Error while freeing network %q", currentNetworks[i].String())
-			}
+			lipam.freeNetwork(currentNetworks[i].String())
 		}
 	}
 
@@ -100,11 +96,7 @@ func (lipam *LiqoIPAM) syncIPs(ctx context.Context, expiredThreshold time.Time)
 	}
 
 	for i := range ips {
-		prefix := netip.MustParsePrefix(ips[i].cidr)
-		addr := netip.MustParseAddr(ips[i].ip)
-		if result := lipam.IpamCore.AllocateIPWithAddr(prefix, addr); result != nil {
-			klog.Infof("Sync: Acquired IP %q", result.String())
-		}
+		_, _ = lipam.acquireIPWithAddress(ips[i].cidr, ips[i].ip)
 	}
 
 	ipMap := make(map[netip.Prefix][]netip.Addr)
@@ -124,11 +116,7 @@ func (lipam *LiqoIPAM) syncIPs(ctx context.Context, expiredThreshold time.Time)
 				}
 			}
 			if !found {
-				if result := lipam.IpamCore.FreeIP(k, ipMap[k][i]); result != nil {
-					klog.Infof("Sync: Released IP %q", result.String())
-				} else {
-					return fmt.Errorf("sync: Error while freeing IP %q", ipMap[k][i].String())
-				}
+				lipam.freeIP(ipCidr{ip: ipMap[k][i].String(), cidr: k.String()})
 			}
 		}
 	}