fix: enforce preallocated IPs to be allocated from the start of the CIDR

pkg/ipam/initialize.go

@@ -55,10 +55,9 @@ func (lipam *LiqoIPAM) initializeNetworks(ctx context.Context) error {
 		if _, err := lipam.networkAcquireSpecific(net); err != nil {
 			return err
 		}
-		for i := 0; i < int(netdetails.preallocated); i++ {
-			if _, err := lipam.ipAcquire(net); err != nil {
-				return errors.Join(err, lipam.networkRelease(net, 0))
-			}
+
+		if err := lipam.acquirePreallocatedIPs(net, netdetails.preallocated); err != nil {
+			return errors.Join(err, lipam.networkRelease(net, 0))
 		}
 	}
 

pkg/ipam/ipam.go

@@ -164,11 +164,8 @@ func (lipam *LiqoIPAM) NetworkAcquire(_ context.Context, req *NetworkAcquireRequ
 		}
 	}
 
-	for i := 0; i < int(req.GetPreAllocated()); i++ {
-		_, err := lipam.ipAcquire(*remappedCidr)
-		if err != nil {
-			return &NetworkAcquireResponse{}, errors.Join(err, lipam.networkRelease(*remappedCidr, 0))
-		}
+	if err := lipam.acquirePreallocatedIPs(*remappedCidr, req.GetPreAllocated()); err != nil {
+		return &NetworkAcquireResponse{}, errors.Join(err, lipam.networkRelease(*remappedCidr, 0))
 	}
 
 	return &NetworkAcquireResponse{Cidr: remappedCidr.String()}, nil

pkg/ipam/ipam_test.go

@@ -29,13 +29,16 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	ipamv1alpha1 "github.com/liqotech/liqo/apis/ipam/v1alpha1"
 	"github.com/liqotech/liqo/pkg/consts"
 	grpcutils "github.com/liqotech/liqo/pkg/utils/grpc"
 	"github.com/liqotech/liqo/pkg/utils/testutil"
 )
 
 var _ = Describe("IPAM integration tests", func() {
 	const (
+		testNamespace = "test"
+
 		grpcAddress = "0.0.0.0"
 		grpcPort    = consts.IpamPort
 	)
@@ -60,6 +63,11 @@ var _ = Describe("IPAM integration tests", func() {
 
 		ipamClient IPAMClient
 		conn       *grpc.ClientConn
+
+		addPreAllocated = func(nw *ipamv1alpha1.Network, preAllocated uint32) *ipamv1alpha1.Network {
+			nw.Spec.PreAllocated = preAllocated
+			return nw
+		}
 	)
 
 	BeforeEach(func() {
@@ -614,6 +622,28 @@ var _ = Describe("IPAM integration tests", func() {
 				// should not interfere with preAllocated IP
 				Expect(ipamServer.ipIsAvailable(netip.MustParseAddr("10.20.0.0"), netip.MustParsePrefix("10.20.0.0/16"))).To(BeFalse())
 			})
+
+			It("should not release an IP that is preallocated", func() {
+				_, err := ipamClient.IPRelease(ctx, &IPReleaseRequest{
+					Cidr: "10.20.0.0/16",
+					Ip:   "10.20.0.0",
+				})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ipamServer.ipIsAvailable(netip.MustParseAddr("10.20.0.0"), netip.MustParsePrefix("10.20.0.0/16"))).To(BeTrue())
+
+				// Run sync.
+				// Add the network with the preallocated IP to the cluster (i.e., inject to the client),
+				// so that the sync routine does not delete it.
+				cl := fakeClientBuilder.WithObjects(
+					addPreAllocated(testutil.FakeNetwork("net1", testNamespace, "10.20.0.0/16", nil), 1),
+				).Build()
+				ipamServer.Client = cl
+				Expect(ipamServer.syncNetworks(ctx)).To(Succeed())
+				Expect(ipamServer.syncIPs(ctx)).To(Succeed())
+
+				// The preallocated IP should now be allocated again
+				Expect(ipamServer.ipIsAvailable(netip.MustParseAddr("10.20.0.0"), netip.MustParsePrefix("10.20.0.0/16"))).To(BeFalse())
+			})
 		})
 
 		When("releasing an IP from an unallocated network", func() {

pkg/ipam/networks.go

@@ -23,6 +23,7 @@ import (
 	klog "k8s.io/klog/v2"
 
 	ipamv1alpha1 "github.com/liqotech/liqo/apis/ipam/v1alpha1"
+	ipamutils "github.com/liqotech/liqo/pkg/utils/ipam"
 )
 
 // networkAcquire acquires a network, eventually remapped if conflicts are found.
@@ -59,6 +60,29 @@ func (lipam *LiqoIPAM) networkAcquireSpecific(prefix netip.Prefix) (*netip.Prefi
 	return result, nil
 }
 
+func (lipam *LiqoIPAM) acquirePreallocatedIPs(prefix netip.Prefix, preallocated uint32) error {
+	// Check if the network can allocate all preallocated IPs.
+	if prefix.Bits() < int(preallocated) {
+		return fmt.Errorf("network %s can not preallocate %d IPs (insufficient size)", prefix.String(), preallocated)
+	}
+
+	// Range over the first N IPs of the network, where N is the number of preallocated IPs.
+	for addr := range ipamutils.FirstNIPsFromPrefix(prefix, preallocated) {
+		available, err := lipam.ipIsAvailable(addr, prefix)
+		if err != nil {
+			return err
+		}
+		if available {
+			if err := lipam.ipAcquireWithAddr(addr, prefix); err != nil {
+				return err
+			}
+		}
+		// Else, the IP is already reserved. Do nothing.
+	}
+
+	return nil
+}
+
 // networkRelease frees a network, removing it from the cache.
 func (lipam *LiqoIPAM) networkRelease(prefix netip.Prefix, gracePeriod time.Duration) error {
 	result := lipam.IpamCore.NetworkRelease(prefix, gracePeriod)

pkg/ipam/sync.go

@@ -69,13 +69,13 @@ func syncNetworkAcquire(lipam *LiqoIPAM, clusterNetworks map[netip.Prefix]prefix
 			if _, err := lipam.networkAcquireSpecific(clusterNetwork); err != nil {
 				return fmt.Errorf("failed to acquire network %q: %w", clusterNetwork, err)
 			}
-			for i := 0; i < int(clusterNetworkDetails.preallocated); i++ {
-				if _, err := lipam.ipAcquire(clusterNetwork); err != nil {
-					return errors.Join(err, lipam.networkRelease(clusterNetwork, 0))
-				}
-			}
+		}
+
+		if err := lipam.acquirePreallocatedIPs(clusterNetwork, clusterNetworkDetails.preallocated); err != nil {
+			return errors.Join(err, lipam.networkRelease(clusterNetwork, 0))
 		}
 	}
+
 	return nil
 }
 

pkg/ipam/sync_test.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	ipamv1alpha1 "github.com/liqotech/liqo/apis/ipam/v1alpha1"
 	ipamcore "github.com/liqotech/liqo/pkg/ipam/core"
 	"github.com/liqotech/liqo/pkg/utils/testutil"
 )
@@ -54,6 +55,11 @@ var _ = Describe("Sync routine tests", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 			Expect(server.ipAcquireWithAddr(addr, prefix)).Should(Succeed())
 		}
+
+		addPreAllocated = func(nw *ipamv1alpha1.Network, preAllocated uint32) *ipamv1alpha1.Network {
+			nw.Spec.PreAllocated = preAllocated
+			return nw
+		}
 	)
 
 	BeforeEach(func() {
@@ -230,5 +236,62 @@ var _ = Describe("Sync routine tests", func() {
 				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.4"), netip.MustParsePrefix("10.0.0.0/24"))).To(Equal(true))
 			})
 		})
+
+		Context("Sync preAllocated IPs", func() {
+			BeforeEach(func() {
+				client := fakeClientBuilder.Build()
+
+				ipamCore, err := ipamcore.NewIpam([]netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")})
+				Expect(err).To(BeNil())
+
+				fakeIpamServer = &LiqoIPAM{
+					Client:   client,
+					IpamCore: ipamCore,
+					opts: &ServerOptions{
+						GraphvizEnabled: false,
+						SyncGracePeriod: syncGracePeriod,
+					},
+				}
+
+				// Acquire network with preAllocated IPs
+				_, err = fakeIpamServer.NetworkAcquire(ctx, &NetworkAcquireRequest{
+					Cidr:         "10.0.0.0/24",
+					Immutable:    true,
+					PreAllocated: 3,
+				})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(fakeIpamServer.networkIsAvailable(netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse())
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.0"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse()) // preAllocated
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.1"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse()) // preAllocated
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.2"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse()) // preAllocated
+
+				// release preallocated IP
+				_, err = fakeIpamServer.IPRelease(ctx, &IPReleaseRequest{
+					Cidr: "10.0.0.0/24",
+					Ip:   "10.0.0.1",
+				})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.0"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse())
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.1"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeTrue())
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.2"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse())
+			})
+
+			It("it should re-allocate preAllocated IP if deleted", func() {
+				// Run sync.
+				// Add the network with the preallocated IP to the cluster (i.e., inject to the client),
+				// so that the sync routine does not delete it.
+				cl := fakeClientBuilder.WithObjects(
+					addPreAllocated(testutil.FakeNetwork("net1", testNamespace, "10.0.0.0/24", nil), 3),
+				).Build()
+				fakeIpamServer.Client = cl
+				Expect(fakeIpamServer.syncNetworks(ctx)).To(Succeed())
+				Expect(fakeIpamServer.syncIPs(ctx)).To(Succeed())
+
+				// The preAllocated IP should be re-allocated
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.0"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse())
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.1"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse())
+				Expect(fakeIpamServer.ipIsAvailable(netip.MustParseAddr("10.0.0.2"), netip.MustParsePrefix("10.0.0.0/24"))).To(BeFalse())
+			})
+		})
 	})
 })

pkg/utils/ipam/ips.go

@@ -18,6 +18,9 @@ import (
 	"encoding/binary"
 	"fmt"
 	"net"
+	"net/netip"
+
+	"iter"
 
 	ipamv1alpha1 "github.com/liqotech/liqo/apis/ipam/v1alpha1"
 	networkingv1beta1 "github.com/liqotech/liqo/apis/networking/v1beta1"
@@ -96,3 +99,16 @@ func NetFirstAndLastIP(networkCIDR string) (first, last net.IP, err error) {
 
 	return first, last, nil
 }
+
+// FirstNIPsFromPrefix returns an iterator with first num IPs from the given prefix.
+func FirstNIPsFromPrefix(prefix netip.Prefix, num uint32) iter.Seq[netip.Addr] {
+	return func(yield func(netip.Addr) bool) {
+		addr := prefix.Addr()
+		for i := 0; i < int(num); i++ {
+			if !yield(addr) {
+				return
+			}
+			addr = addr.Next()
+		}
+	}
+}