Allow for allocating all valid host IPs from ReservedSubnets

Allow allocation of first/last IPs in reserved subnets when they are valid host addresses.
Ensure that IPv4 network (.0) and broadcast addresses are automatically excluded
from reserved subnet allocators to prevent invalid IP assignments.

Signed-off-by: Patryk Diak <[email protected]>

Author: kyrtapz

go-controller/pkg/allocator/ip/allocator.go

@@ -90,7 +90,30 @@ type Range struct {
 }
 
 // NewAllocatorCIDRRange creates a Range over a net.IPNet, calling allocatorFactory to construct the backing store.
+// It excludes the network address (.0) and broadcast address (IPv4 only) from allocation.
 func NewAllocatorCIDRRange(cidr *net.IPNet, allocatorFactory allocator.AllocatorFactory) (*Range, error) {
+	r, err := NewAllocatorFullCIDRRange(cidr, allocatorFactory)
+	if err != nil {
+		return nil, err
+	}
+
+	if utilnet.IsIPv4CIDR(cidr) {
+		// Don't use the IPv4 network's broadcast address.
+		r.max--
+	}
+	// Don't use the network's ".0" address.
+	r.base.Add(r.base, big.NewInt(1))
+	r.max--
+
+	r.max = maximum(0, r.max)
+	// Reconfigure the allocator to use the new max value
+	r.alloc, err = allocatorFactory(r.max, r.net.String())
+	return r, err
+}
+
+// NewAllocatorFullCIDRRange creates a Range over a net.IPNet without excluding any IPs,
+// calling allocatorFactory to construct the backing store.
+func NewAllocatorFullCIDRRange(cidr *net.IPNet, allocatorFactory allocator.AllocatorFactory) (*Range, error) {
 	max := utilnet.RangeSize(cidr)
 	base := utilnet.BigForIP(cidr.IP)
 	rangeSpec := cidr.String()
@@ -100,19 +123,11 @@ func NewAllocatorCIDRRange(cidr *net.IPNet, allocatorFactory allocator.Allocator
 		if max > 65536 {
 			max = 65536
 		}
-	} else {
-		// Don't use the IPv4 network's broadcast address.
-		max--
 	}
-
-	// Don't use the network's ".0" address.
-	base.Add(base, big.NewInt(1))
-	max--
-
 	r := Range{
 		net:  cidr,
 		base: base,
-		max:  maximum(0, int(max)),
+		max:  int(max),
 	}
 	var err error
 	r.alloc, err = allocatorFactory(r.max, rangeSpec)
@@ -213,14 +228,37 @@ func (r *Range) Has(ip net.IP) bool {
 }
 
 // Reserved returns true if the provided IP can't be allocated. This is *only*
-// true for the network and broadcast addresses.
+// true for the network and broadcast addresses of the original CIDR.
 func (r *Range) Reserved(ip net.IP) bool {
 	if !r.net.Contains(ip) {
 		return false
 	}
 
-	offset := calculateIPOffset(r.base, ip)
-	return offset == -1 || offset == r.max
+	// For IPv4, reserve network (.0) and broadcast addresses
+	if utilnet.IsIPv4CIDR(r.net) {
+		// Network address is the base IP of the original CIDR
+		networkAddr := r.net.IP
+		if ip.Equal(networkAddr) {
+			return true
+		}
+
+		// Broadcast address is the last IP in the original CIDR
+		rangeSize := utilnet.RangeSize(r.net)
+		broadcastAddr, _ := utilnet.GetIndexedIP(r.net, int(rangeSize)-1)
+		if ip.Equal(broadcastAddr) {
+			return true
+		}
+	}
+
+	// For IPv6, only reserve the network address (no broadcast concept)
+	if utilnet.IsIPv6CIDR(r.net) {
+		networkAddr := r.net.IP
+		if ip.Equal(networkAddr) {
+			return true
+		}
+	}
+
+	return false
 }
 
 // contains returns true and the offset if the ip is in the range, and false

go-controller/pkg/allocator/ip/subnet/allocator.go

@@ -10,6 +10,7 @@ import (
 	iputils "github.com/containernetworking/plugins/pkg/ip"
 
 	"k8s.io/klog/v2"
+	utilnet "k8s.io/utils/net"
 
 	bitmapallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/bitmap"
 	ipallocator "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/allocator/ip"
@@ -84,9 +85,9 @@ func newIPAMAllocator(cidr *net.IPNet) (ipallocator.ContinuousAllocator, error)
 }
 
 // newReservedIPAMAllocator provides an ipam interface which can be used for IPAM
-// allocations for a given cidr using static IP allocations only.
+// allocations for a given cidr using static IP allocations only. All IPs are available.
 func newReservedIPAMAllocator(cidr *net.IPNet) (ipallocator.StaticAllocator, error) {
-	return ipallocator.NewAllocatorCIDRRange(cidr, func(max int, rangeSpec string) (bitmapallocator.Interface, error) {
+	return ipallocator.NewAllocatorFullCIDRRange(cidr, func(max int, rangeSpec string) (bitmapallocator.Interface, error) {
 		return bitmapallocator.NewRoundRobinAllocationMap(max, rangeSpec), nil
 	})
 }
@@ -109,12 +110,20 @@ func (allocator *allocator) AddOrUpdateSubnet(config SubnetConfig) error {
 		klog.Warningf("Replacing subnets %v with %v for %s", util.StringSlice(subnetInfo.subnets), util.StringSlice(config.Subnets), config.Name)
 	}
 	var ipams []ipallocator.ContinuousAllocator
+
+	// subnetBoundaryIPs holds network and broadcast addresses for IPv4 subnets.
+	// These are automatically excluded from reserved subnet allocators to prevent allocation.
+	var subnetBoundaryIPs []net.IP
 	for _, subnet := range config.Subnets {
 		ipam, err := allocator.ipamFunc(subnet)
 		if err != nil {
 			return fmt.Errorf("failed to initialize IPAM of subnet %s for %s: %w", subnet, config.Name, err)
 		}
 		ipams = append(ipams, ipam)
+
+		if utilnet.IsIPv4CIDR(subnet) {
+			subnetBoundaryIPs = append(subnetBoundaryIPs, subnet.IP, util.SubnetBroadcastIP(*subnet))
+		}
 	}
 
 	// reservedSubnets is a subset of subnets, and it should not be used by automatic IPAM
@@ -141,6 +150,15 @@ func (allocator *allocator) AddOrUpdateSubnet(config SubnetConfig) error {
 			return fmt.Errorf("failed to initialize IPAM of reserved subnet %s for %s: %w", reservedSubnet, config.Name, err)
 		}
 		staticIPAMs = append(staticIPAMs, ipam)
+
+		// Exclude network and broadcast addresses from reserved subnet allocators
+		for _, excludedSubnetIP := range subnetBoundaryIPs {
+			if reservedSubnet.Contains(excludedSubnetIP) {
+				if err := ipam.Allocate(excludedSubnetIP); err != nil {
+					return fmt.Errorf("failed to exclude %s from reserved subnet allocator %s: %w", excludedSubnetIP, ipam.CIDR(), err)
+				}
+			}
+		}
 	}
 	allocator.cache[config.Name] = subnetInfo{
 		subnets:     config.Subnets,
@@ -243,6 +261,7 @@ func (allocator *allocator) AllocateIPPerSubnet(name string, ips []*net.IPNet) e
 					err = fmt.Errorf("failed to allocate IP %s for %s: attempted to reserve multiple IPs in the same static IPAM instance", ipnet.IP, name)
 					return err
 				}
+
 				if err = staticIPAM.Allocate(ipnet.IP); err != nil {
 					return err
 				}

go-controller/pkg/allocator/ip/subnet/allocator_test.go

@@ -250,9 +250,9 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
 			"10.1.1.0/24",
 		}
 		reservedSubnets := []string{
-			"10.1.1.0/28", // Reserve 10.1.1.0-15
+			"10.1.1.16/28", // Reserve 10.1.1.16-31
 		}
-		expectedIP := "10.1.1.16/24"
+		expectedIP := "10.1.1.1/24"
 
 		err := allocator.AddOrUpdateSubnet(SubnetConfig{
 			Name:            subnetName,
@@ -266,8 +266,14 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
 		gomega.Expect(ips).To(gomega.HaveLen(1))
 		gomega.Expect(ips[0].String()).To(gomega.Equal(expectedIP))
 
-		// Should be able to allocate specific IPs from reserved range
-		reservedIPs, err := util.ParseIPNets([]string{"10.1.1.5/24"})
+		// Should be able to allocate the first IP from the reserved range
+		reservedIPs, err := util.ParseIPNets([]string{"10.1.1.16/24"})
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+		err = allocator.AllocateIPPerSubnet(subnetName, reservedIPs)
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+
+		// Should be able to allocate the last IP from the reserved range
+		reservedIPs, err = util.ParseIPNets([]string{"10.1.1.31/24"})
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 		err = allocator.AllocateIPPerSubnet(subnetName, reservedIPs)
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
@@ -282,7 +288,8 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
 			"10.1.1.0/24",
 		}
 		reservedSubnets := []string{
-			"10.1.1.0/28", // Reserve 10.1.1.0-15
+			"10.1.1.0/28",   // Reserve 10.1.1.0-15
+			"10.1.1.240/28", // Reserve 10.1.1.240-255
 		}
 		excludeSubnets := []string{
 			"10.1.1.200/29", // Exclude 10.1.1.200-207
@@ -313,6 +320,18 @@ var _ = ginkgo.Describe("Subnet IP allocator operations", func() {
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 		err = allocator.AllocateIPPerSubnet(subnetName, excludedIPs)
 		gomega.Expect(err).To(gomega.MatchError(ipam.ErrAllocated))
+
+		// Should not be able to allocate the network IP
+		reservedIPs, err = util.ParseIPNets([]string{"10.1.1.0/24"})
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+		err = allocator.AllocateIPPerSubnet(subnetName, reservedIPs)
+		gomega.Expect(err).To(gomega.HaveOccurred())
+
+		// Should not be able to allocate the broadcast IP
+		reservedIPs, err = util.ParseIPNets([]string{"10.1.1.255/24"})
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+		err = allocator.AllocateIPPerSubnet(subnetName, reservedIPs)
+		gomega.Expect(err).To(gomega.HaveOccurred())
 	})
 
 })