lbipam: Allow range notation on CiliumLoadBalancerIPPool

Now that we have the new IP allocator, we can allocate non-CIDR ranges.
The ideal new syntax looks like:

```
[...]
spec:
  blocks:
  - cidr: "10.0.0.0/24"
  - start: "20.0.0.10"
    stop: "20.0.0.30"
  - start: "30.1.2.3"
```

The `cidrs` list has been renamed to blocks since we are now dealing
with blocks of IPs not just CIDRs anymore. The old `cidrs` field is
still supported and takes the same elements, so just an alias for
backwards compatibility.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>

Documentation/network/lb-ipam.rst

@@ -38,9 +38,12 @@ A basic IP Pools with both an IPv4 and IPv6 range looks like this:
     metadata:
       name: "blue-pool"
     spec:
-      cidrs:
+      blocks:
       - cidr: "10.0.10.0/24"
       - cidr: "2004::0/64"
+      - start: "20.0.20.100"
+        stop: "20.0.20.200"
+      - start: "1.2.3.4"
 
 After adding the pool to the cluster, it appears like so.
 
@@ -50,12 +53,6 @@ After adding the pool to the cluster, it appears like so.
     NAME        DISABLED   CONFLICTING   IPS AVAILABLE   AGE
     blue-pool   false      False         65788           2s
 
-.. note::
-    The amount of available IPs in the pool is lower than the actual sum of all
-    usable IPs in the CIDRs because the allocation logic is limited to 65536 IPs
-    per CIDR. CIDRs containing more than 65536 IPs can be broken down into multiple
-    smaller CIDRs to achieve full utilization.
-
 Service Selectors
 -----------------
 
@@ -70,7 +67,7 @@ The pool will allocate to any service if no service selector is specified.
     metadata:
       name: "blue-pool"
     spec:
-      cidrs:
+      blocks:
       - cidr: "20.0.10.0/24"
       serviceSelector:
         matchExpressions:
@@ -81,7 +78,7 @@ The pool will allocate to any service if no service selector is specified.
     metadata:
       name: "red-pool"
     spec:
-      cidrs:
+      blocks:
       - cidr: "20.0.10.0/24"
       serviceSelector:
         matchLabels:
@@ -106,7 +103,7 @@ For example:
     metadata:
       name: "blue-pool"
     spec:
-      cidrs:
+      blocks:
       - cidr: "20.0.10.0/24"
       serviceSelector:
         matchLabels:
@@ -170,7 +167,7 @@ an administrator to slowly drain pool or reserve a pool for future use.
     metadata:
       name: "blue-pool"
     spec:
-      cidrs:
+      blocks:
       - cidr: "20.0.10.0/24"
       disabled: true
 

operator/pkg/lbipam/lbipam.go

@@ -542,6 +542,7 @@ func (ipam *LBIPAM) stripInvalidAllocations(sv *ServiceView) error {
 			}
 		}
 	}
+
 	return errs
 }
 
@@ -1082,21 +1083,20 @@ func (ipam *LBIPAM) handleNewPool(ctx context.Context, pool *cilium_api_v2alpha1
 	}
 
 	ipam.pools[pool.GetName()] = pool
-	for _, cidrBlock := range pool.Spec.Cidrs {
-		prefix, err := netip.ParsePrefix(string(cidrBlock.Cidr))
+	blocks := append(pool.Spec.Cidrs, pool.Spec.Blocks...)
+	for _, ipBlock := range blocks {
+		from, to, fromCidr, err := ipRangeFromBlock(ipBlock)
 		if err != nil {
-			return fmt.Errorf("Error parsing cidr '%s': %w", cidrBlock.Cidr, err)
+			return fmt.Errorf("Error parsing ip block: %w", err)
 		}
 
-		from, to := rangeFromPrefix(prefix)
-
 		lbRange, err := NewLBRange(from, to, pool)
 		if err != nil {
-			return fmt.Errorf("Error making LB Range for '%s': %w", cidrBlock.Cidr, err)
+			return fmt.Errorf("Error making LB Range for '%s': %w", ipBlock.Cidr, err)
 		}
 
 		// If AllowFirstLastIPs is no or unspecified, mark the first and last IP as allocated upon range creation.
-		if pool.Spec.AllowFirstLastIPs != cilium_api_v2alpha1.AllowFirstLastIPYes {
+		if fromCidr && pool.Spec.AllowFirstLastIPs != cilium_api_v2alpha1.AllowFirstLastIPYes {
 			// TODO: in 1.16 switch from default no to default yes.
 			// https://github.com/cilium/cilium/issues/28591
 			from, to := lbRange.alloc.Range()
@@ -1113,6 +1113,33 @@ func (ipam *LBIPAM) handleNewPool(ctx context.Context, pool *cilium_api_v2alpha1
 	return nil
 }
 
+func ipRangeFromBlock(block cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock) (to, from netip.Addr, fromCidr bool, err error) {
+	if string(block.Cidr) != "" {
+		prefix, err := netip.ParsePrefix(string(block.Cidr))
+		if err != nil {
+			return netip.Addr{}, netip.Addr{}, false, fmt.Errorf("Error parsing cidr '%s': %w", block.Cidr, err)
+		}
+
+		to, from = rangeFromPrefix(prefix)
+		return to, from, true, nil
+	}
+
+	from, err = netip.ParseAddr(block.Start)
+	if err != nil {
+		return netip.Addr{}, netip.Addr{}, false, fmt.Errorf("error parsing start ip '%s': %w", block.Start, err)
+	}
+	if block.Stop == "" {
+		return from, from, false, nil
+	}
+
+	to, err = netip.ParseAddr(block.Stop)
+	if err != nil {
+		return netip.Addr{}, netip.Addr{}, false, fmt.Errorf("error parsing stop ip '%s': %w", block.Stop, err)
+	}
+
+	return from, to, false, nil
+}
+
 func (ipam *LBIPAM) handlePoolModified(ctx context.Context, pool *cilium_api_v2alpha1.CiliumLoadBalancerIPPool) error {
 	changedAllowFirstLastIPs := false
 	if existingPool, ok := ipam.pools[pool.GetName()]; ok {
@@ -1122,32 +1149,43 @@ func (ipam *LBIPAM) handlePoolModified(ctx context.Context, pool *cilium_api_v2a
 
 	ipam.pools[pool.GetName()] = pool
 
-	var newCIDRs []netip.Prefix
-	for _, newBlock := range pool.Spec.Cidrs {
-		cidr, err := netip.ParsePrefix(string(newBlock.Cidr))
+	type rng struct {
+		from, to netip.Addr
+		fromCidr bool
+	}
+	var newRanges []rng
+	blocks := append(pool.Spec.Cidrs, pool.Spec.Blocks...)
+	for _, newBlock := range blocks {
+		from, to, fromCidr, err := ipRangeFromBlock(newBlock)
 		if err != nil {
-			return fmt.Errorf("Error parsing cidr '%s': %w", newBlock.Cidr, err)
+			return fmt.Errorf("Error parsing ip block: %w", err)
 		}
-		newCIDRs = append(newCIDRs, cidr)
+
+		newRanges = append(newRanges, rng{
+			from:     from,
+			to:       to,
+			fromCidr: fromCidr,
+		})
 	}
 
 	existingRanges, _ := ipam.rangesStore.GetRangesForPool(pool.GetName())
 
 	// Remove existing ranges that no longer exist
 	for _, extRange := range existingRanges {
 		found := false
-		for _, newCIDR := range newCIDRs {
-			from, to := rangeFromPrefix(newCIDR)
-			if extRange.EqualCIDR(from, to) {
+		fromCidr := false
+		for _, newRange := range newRanges {
+			if extRange.EqualCIDR(newRange.from, newRange.to) {
 				found = true
+				fromCidr = newRange.fromCidr
 				break
 			}
 		}
 
 		if found {
 			// If the AllowFirstLastIPs state changed
-			if changedAllowFirstLastIPs {
-				// TODO: in 1.16 switch from default no to default yes.
+			if fromCidr && changedAllowFirstLastIPs {
+				// TODO in 1.16 switch from default no to default yes.
 				// https://github.com/cilium/cilium/issues/28591
 				if pool.Spec.AllowFirstLastIPs == cilium_api_v2alpha1.AllowFirstLastIPYes {
 					// If we are allowing first and last IPs again, free them for allocation
@@ -1175,12 +1213,10 @@ func (ipam *LBIPAM) handlePoolModified(ctx context.Context, pool *cilium_api_v2a
 	}
 
 	// Add new ranges that were added
-	for _, newCIDR := range newCIDRs {
-		from, to := rangeFromPrefix(newCIDR)
-
+	for _, newRange := range newRanges {
 		found := false
 		for _, extRange := range existingRanges {
-			if extRange.EqualCIDR(from, to) {
+			if extRange.EqualCIDR(newRange.from, newRange.to) {
 				found = true
 				break
 			}
@@ -1190,21 +1226,21 @@ func (ipam *LBIPAM) handlePoolModified(ctx context.Context, pool *cilium_api_v2a
 			continue
 		}
 
-		newRange, err := NewLBRange(from, to, pool)
+		newLBRange, err := NewLBRange(newRange.from, newRange.to, pool)
 		if err != nil {
-			return fmt.Errorf("Error while making new LB range for CIDR '%s': %w", newCIDR.String(), err)
+			return fmt.Errorf("Error while making new LB range for range '%s - %s': %w", newRange.from, newRange.to, err)
 		}
 
 		// If AllowFirstLastIPs is no or default, mark the first and last IP as allocated upon range creation.
-		if pool.Spec.AllowFirstLastIPs != cilium_api_v2alpha1.AllowFirstLastIPYes {
+		if newRange.fromCidr && pool.Spec.AllowFirstLastIPs != cilium_api_v2alpha1.AllowFirstLastIPYes {
 			// TODO: in 1.16 switch from default no to default yes.
 			// https://github.com/cilium/cilium/issues/28591
-			from, to := newRange.alloc.Range()
-			newRange.alloc.Alloc(from, true)
-			newRange.alloc.Alloc(to, true)
+			from, to := newLBRange.alloc.Range()
+			newLBRange.alloc.Alloc(from, true)
+			newLBRange.alloc.Alloc(to, true)
 		}
 
-		ipam.rangesStore.Add(newRange)
+		ipam.rangesStore.Add(newLBRange)
 	}
 
 	existingRanges, _ = ipam.rangesStore.GetRangesForPool(pool.GetName())

operator/pkg/lbipam/lbipam_fixture_test.go

@@ -319,9 +319,9 @@ func (fix *LBIPAMTestFixture) patchedObj(tracker k8s_testing.ObjectTracker, acti
 
 // mkPool is a constructor function to assist in the creation of new pool objects.
 func mkPool(uid types.UID, name string, cidrs []string) *cilium_api_v2alpha1.CiliumLoadBalancerIPPool {
-	var blocks []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolCIDRBlock
+	var blocks []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock
 	for _, cidr := range cidrs {
-		blocks = append(blocks, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolCIDRBlock{
+		blocks = append(blocks, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
 			Cidr: cilium_api_v2alpha1.IPv4orIPv6CIDR(cidr),
 		})
 	}

operator/pkg/lbipam/lbipam_test.go

@@ -102,7 +102,7 @@ func TestConflictResolution(t *testing.T) {
 	}
 
 	// Remove the conflicting range
-	poolB.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolCIDRBlock{
+	poolB.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
 		{
 			Cidr: cilium_api_v2alpha1.IPv4orIPv6CIDR("FF::0/48"),
 		},
@@ -159,7 +159,7 @@ func TestPoolInternalConflict(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	pool.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolCIDRBlock{
+	pool.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
 		{
 			Cidr: "10.0.10.0/24",
 		},
@@ -1534,7 +1534,7 @@ func TestAddRange(t *testing.T) {
 		return
 	}
 
-	poolA.Spec.Cidrs = append(poolA.Spec.Cidrs, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolCIDRBlock{
+	poolA.Spec.Cidrs = append(poolA.Spec.Cidrs, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
 		Cidr: "10.0.20.0/24",
 	})
 
@@ -1857,7 +1857,7 @@ func TestRangeDelete(t *testing.T) {
 	}, 500*time.Millisecond)
 
 	// Add a new CIDR, this should not have any effect on the existing service.
-	poolA.Spec.Cidrs = append(poolA.Spec.Cidrs, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolCIDRBlock{
+	poolA.Spec.Cidrs = append(poolA.Spec.Cidrs, cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
 		Cidr: "10.0.20.0/24",
 	})
 	_, err := fixture.poolClient.Update(context.Background(), poolA, meta_v1.UpdateOptions{})
@@ -1894,7 +1894,7 @@ func TestRangeDelete(t *testing.T) {
 	}, time.Second)
 
 	// Remove the existing range, this should trigger the re-allocation of the existing service
-	poolA.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolCIDRBlock{
+	poolA.Spec.Cidrs = []cilium_api_v2alpha1.CiliumLoadBalancerIPPoolIPBlock{
 		{
 			Cidr: "10.0.20.0/24",
 		},
@@ -2440,3 +2440,66 @@ func TestChangePoolSelector(t *testing.T) {
 		t.Fatal("Expected service status update")
 	}
 }
+
+func TestRangeFromPrefix(t *testing.T) {
+	type test struct {
+		name   string
+		prefix string
+		from   string
+		to     string
+	}
+
+	tests := []test{
+		{
+			name:   "/24",
+			prefix: "10.0.0.0/24",
+			from:   "10.0.0.0",
+			to:     "10.0.0.255",
+		},
+		{
+			name:   "/25",
+			prefix: "10.0.0.0/25",
+			from:   "10.0.0.0",
+			to:     "10.0.0.128",
+		},
+		{
+			name:   "offset prefix",
+			prefix: "10.0.0.12/24",
+			from:   "10.0.0.0",
+			to:     "10.0.0.255",
+		},
+		{
+			name:   "ipv6",
+			prefix: "::0000/112",
+			from:   "::0000",
+			to:     "::FFFF",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(tt *testing.T) {
+			prefix, err := netip.ParsePrefix(test.prefix)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			expectedTo, err := netip.ParseAddr(test.to)
+			if err != nil {
+				tt.Fatal(err)
+			}
+
+			expectedFrom, err := netip.ParseAddr(test.from)
+			if err != nil {
+				tt.Fatal(err)
+			}
+
+			from, to := rangeFromPrefix(prefix)
+			if to.Compare(expectedTo) != 0 {
+				tt.Fatalf("expected '%s', got '%s'", expectedTo, to)
+			}
+			if from.Compare(expectedFrom) != 0 {
+				tt.Fatalf("expected '%s', got '%s'", expectedFrom, from)
+			}
+		})
+	}
+}

pkg/bgpv1/manager/route_policy_reconciler_test.go

@@ -31,7 +31,7 @@ var (
 			},
 		},
 		Spec: v2alpha1api.CiliumLoadBalancerIPPoolSpec{
-			Cidrs: []v2alpha1api.CiliumLoadBalancerIPPoolCIDRBlock{
+			Cidrs: []v2alpha1api.CiliumLoadBalancerIPPoolIPBlock{
 				{
 					Cidr: "192.168.0.0/24",
 				},
@@ -45,7 +45,7 @@ var (
 			},
 		},
 		Spec: v2alpha1api.CiliumLoadBalancerIPPoolSpec{
-			Cidrs: []v2alpha1api.CiliumLoadBalancerIPPoolCIDRBlock{
+			Cidrs: []v2alpha1api.CiliumLoadBalancerIPPoolIPBlock{
 				{
 					Cidr: "10.100.99.0/24", // UPDATED
 				},

pkg/bgpv1/test/objects.go

@@ -97,7 +97,7 @@ func newLBPoolObj(conf lbPoolConfig) v2alpha1.CiliumLoadBalancerIPPool {
 		obj.Labels = conf.labels
 	}
 	for _, cidr := range conf.cidrs {
-		obj.Spec.Cidrs = append(obj.Spec.Cidrs, v2alpha1.CiliumLoadBalancerIPPoolCIDRBlock{Cidr: v2alpha1.IPv4orIPv6CIDR(cidr)})
+		obj.Spec.Cidrs = append(obj.Spec.Cidrs, v2alpha1.CiliumLoadBalancerIPPoolIPBlock{Cidr: v2alpha1.IPv4orIPv6CIDR(cidr)})
 	}
 	return obj
 }