Account for IPReservations in IP pool utilization

calicoctl/calicoctl/commands/ipam/show.go

@@ -17,7 +17,6 @@ package ipam
 import (
 	"context"
 	"fmt"
-	"math"
 	"os"
 	"reflect"
 	"sort"
@@ -209,37 +208,41 @@ func ShowBlockUtilization(ctx context.Context, ipamClient ipam.Interface, showBl
 	}
 	t := table.NewWriter()
 	t.SetOutputMirror(os.Stdout)
-	t.AppendHeader(table.Row{"GROUPING", "CIDR", "IPS TOTAL", "IPS IN USE", "IPS FREE"})
+	t.AppendHeader(table.Row{"GROUPING", "CIDR", "IPS TOTAL", "IPS IN USE", "IPS RESERVED", "IPS FREE"})
 	t.SetColumnConfigs([]table.ColumnConfig{
 		{Name: "IPS TOTAL", Align: text.AlignRight},
 		{Name: "IPS IN USE", Align: text.AlignRight},
+		{Name: "IPS RESERVED", Align: text.AlignRight},
 		{Name: "IPS FREE", Align: text.AlignRight},
 	})
-	genRow := func(kind, cidr string, inUse, capacity float64) table.Row {
+	// IN USE counts allocated IPs and RESERVED counts IPs that an IPReservation
+	// covers; an IP allocated before it was reserved falls into both, so the
+	// percentages need not add up to 100.  FREE counts the IPs that are neither,
+	// which is why it comes from the library rather than being derived here.
+	genRow := func(kind, cidr string, capacity, inUse, reserved, free int) table.Row {
+		withPercentage := func(n int) string {
+			return fmt.Sprintf("%.5g (%.f%%)", float64(n), 100*float64(n)/float64(capacity))
+		}
 		return table.Row{
 			kind,
 			cidr,
-			fmt.Sprintf("%.5g", capacity),
-			// Note: the '+capacity/2' bits here give us rounding to the nearest
-			// integer, instead of rounding down, and so ensure that the two percentages
-			// add up to 100.
-			fmt.Sprintf("%.5g (%.f%%)", inUse, 100*inUse/capacity),
-			fmt.Sprintf("%.5g (%.f%%)", capacity-inUse, 100*(capacity-inUse)/capacity),
+			fmt.Sprintf("%.5g", float64(capacity)),
+			withPercentage(inUse),
+			withPercentage(reserved),
+			withPercentage(free),
 		}
 	}
 	for _, poolUse := range usage {
 		var blockRows []table.Row
-		var poolInUse float64
 		for _, blockUse := range poolUse.Blocks {
-			blockRows = append(blockRows, genRow("Block", blockUse.CIDR.String(), float64(blockUse.Capacity-blockUse.Available), float64(blockUse.Capacity)))
-			poolInUse += float64(blockUse.Capacity - blockUse.Available)
+			blockRows = append(blockRows, genRow("Block", blockUse.CIDR.String(),
+				blockUse.Capacity, blockUse.InUse, blockUse.Reserved, blockUse.Available))
 		}
-		ones, bits := poolUse.CIDR.Mask.Size()
-		poolCapacity := math.Pow(2, float64(bits-ones))
-		if ones > 0 {
+		if ones, _ := poolUse.CIDR.Mask.Size(); ones > 0 {
 			// Only show the IP Pool row for a real IP Pool and not for the orphaned
 			// block case.
-			t.AppendRow(genRow("IP Pool", poolUse.CIDR.String(), poolInUse, poolCapacity))
+			t.AppendRow(genRow("IP Pool", poolUse.CIDR.String(),
+				poolUse.Capacity, poolUse.InUse, poolUse.Reserved, poolUse.Available))
 		}
 		if showBlocks {
 			t.AppendRows(blockRows)

design/ipam/ipam-core-library.md

@@ -20,6 +20,11 @@ here. A few methods carry design-relevant constraints worth calling out:
 
 - **`AutoAssign`** returns block-masked CIDRs, not `/32` (or `/128`). Callers narrow at the boundary. This is load-bearing for the CNI plugin's per-block route programming.
 - **`AssignIP`** enforces the target pool's `allowedUses` when `AssignIPArgs.IntendedUse` is non-empty: it fails if the pool containing the requested IP does not permit that use, mirroring the `filterPoolsByUse` filter `AutoAssign` applies. Callers that leave `IntendedUse` empty are exempt (back-compat). This closes a gap where a specific-IP request (e.g. the CNI `ipAddrs` annotation) could draw from a pool not sanctioned for its use.
+- **`GetUtilization`** reports `Capacity`, `InUse`, `Reserved` and `Available` per pool and per block. `InUse` (allocated) and `Reserved` (covered by an `IPReservation`) overlap when an
+  address was allocated before it was reserved, so `Capacity` is not their sum plus `Available`; `Available` counts addresses that are neither, and is the only one of the four that
+  answers "how many can still be handed out". Consumers must read it rather than deriving it. Pool-level counts span the whole pool CIDR, including space no block covers yet - a
+  reservation over unblocked space is still unassignable - so they are computed as a set operation (pool minus reservations minus blocks, via `go4.org/netipx`) rather than summed
+  from the blocks. Reservations may overlap and nest arbitrarily, which is why a set is needed and not a sum over CIDRs.
 - **`ReleaseIPs`** takes `ReleaseOptions` with a sequence number; every release path must plumb it through (see [CAS retry and sequence numbers](#cas-retry-and-sequence-numbers)).
 - **`SetOwnerAttributes`** is KubeVirt-only and swaps owner attributes under preconditions, without releasing and re-allocating. Felix's live-migration monitor is the only non-CNI
   caller.
@@ -32,6 +37,8 @@ here. A few methods carry design-relevant constraints worth calling out:
 - Don't leak `crd.projectcalico.org/v1` types through new public APIs. The `lib/v3` -> `lib/internalapi` rename (https://github.com/projectcalico/calico/pull/11870) exists to keep
   that boundary clean.
 - `AutoAssign` returning block-masked CIDRs is load-bearing for the CNI plugin's routing. Don't quietly switch to `/32`.
+- Anything that makes an address unassignable has to be discounted by `GetUtilization` as well as by the allocation path, or the reporting surfaces over-count free addresses. The
+  two must be fed from the same set of reserved CIDRs: allocation and the per-block counts share the `addrFilter`, and the pool-level counts use the same CIDRs as a set.
 
 ## AutoAssign and host affinity
 

design/ipam/ipam-datastore.md

@@ -83,7 +83,8 @@ the block syncer plus a handle-side scan rather than a watch. See https://github
 
 Both stored as CRDs alongside the other IPAM resources. [`ipam_config.go`](../../libcalico-go/lib/backend/k8s/resources/ipam_config.go) wraps the singleton `IPAMConfig`. Storage
 shape only - field semantics, defaults, and `StrictAffinity` / `MaxBlocksPerHost` / `AutoAllocateBlocks` interactions live in
-[`ipam-core-library.md`](./ipam-core-library.md#ipamconfig). `IPReservation` is read at allocation time and converted into an ordinal filter; never participates in CAS.
+[`ipam-core-library.md`](./ipam-core-library.md#ipamconfig). `IPReservation` is read at allocation time and converted into an ordinal filter, and again by `GetUtilization` so that
+the reporting surfaces don't count reserved addresses as free; never participates in CAS.
 
 **Review notes**
 

design/ipam/ipam-gc.md

@@ -214,6 +214,17 @@ variants exist for backward compatibility.
 `updateMetrics` recomputes from scratch every sync - one walk over all blocks, no incremental state. The full-recompute *is* the consistency check; switching to incremental updates
 without a separate consistency check loses the protection.
 
+`ipam_ippool_reserved` is the exception to that walk: reservations make addresses unassignable without allocating them, and can cover pool space no block has been carved from, so the
+number isn't in the block state the controller tracks. `updateReservedMetrics` asks the library instead (`GetUtilization`, see
+[ipam-core-library](./ipam-core-library.md#public-api-surface)) and publishes only pools the controller knows about, skipping the pseudo-pool that `GetUtilization` reports for
+orphaned blocks. Being per-pool rather than per-node, it is labelled `ippool` only, like `ipam_ippool_size`. It may overlap `ipam_allocations_in_use`, so usable capacity is
+`ipam_ippool_size - ipam_allocations_in_use - ipam_ippool_reserved` only when no reserved address is also allocated.
+
+The controller does **not** watch `IPReservation` - kube-controllers has `list` on it but not `watch`, and adding one is an operator RBAC change - so creating or deleting a
+reservation does not by itself wake the sync loop. The gauge refreshes on the next IPAM sync from any other cause (pool, node or block change) or on the periodic sync, so it can
+lag a reservation change by up to that period. Acceptable for a capacity number; if it ever needs to be prompt, the fix is to watch the resource and update the ClusterRole in both
+the chart and tigera/operator.
+
 **Review notes**
 
 - `ipam_allocations_gc_candidates > 0` for extended periods is the canonical "GC is stuck" signal. Alert on it.

design/ipam/ipam-other-callers.md

@@ -23,10 +23,15 @@ migrate` parses the tunnel prefixes, and the CNI DEL path releases by both the h
 IPAM-meaningful subcommands; `configure` and `split` are admin-CRUD on `IPAMConfig` / `IPPool`. The `check` algorithm reuses the validity heuristics the GC applies, but exposed for
 manual review without a running controller.
 
+`show` prints IPS TOTAL / IN USE / RESERVED / FREE per pool and per block, taking all four straight from `GetUtilization` rather than deriving any of them: IN USE and RESERVED can
+cover the same address, so the columns need not sum to the total, and FREE is the only column that means "still assignable". See
+[ipam-core-library](./ipam-core-library.md#public-api-surface).
+
 **Review notes**
 
 - New tunnel handle prefixes need to be added to `calicoctl datastore migrate`, which rewrites tunnel handle IDs during node renames.
 - `check` and the GC share validity logic. If you change one, check the other doesn't drift.
+- The `show` columns are a reporting surface, not a derivation. If a new mechanism withholds addresses, it has to reach `GetUtilization` or `show` silently over-counts FREE.
 
 ## Node tunnel-address allocator
 

go.mod

@@ -92,6 +92,7 @@ require (
 	go.etcd.io/etcd/client/v2 v2.305.31
 	go.etcd.io/etcd/client/v3 v3.6.12
 	go.yaml.in/yaml/v3 v3.0.4
+	go4.org/netipx v0.0.0-20231129151722-fdeea329fbba
 	golang.org/x/mod v0.38.0
 	golang.org/x/net v0.57.0
 	golang.org/x/oauth2 v0.36.0

go.sum

@@ -890,6 +890,8 @@ go.yaml.in/yaml/v2 v2.4.4/go.mod h1:gMZqIpDtDqOfM0uNfy0SkpRhvUryYH0Z6wdMYcacYXQ=
 go.yaml.in/yaml/v3 v3.0.3/go.mod h1:tBHosrYAkRZjRAOREWbDnBXUf08JOwYq++0QNwQiWzI=
 go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc=
 go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
+go4.org/netipx v0.0.0-20231129151722-fdeea329fbba h1:0b9z3AuHCjxk0x/opv64kcgZLBseWJUpBw5I82+2U4M=
+go4.org/netipx v0.0.0-20231129151722-fdeea329fbba/go.mod h1:PLyyIXexvUFg3Owu6p/WfdlivPbZJsZdgWZlrGope/Y=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=

kube-controllers/pkg/controllers/node/fake_client.go

@@ -299,6 +299,10 @@ type fakeIPAMClient struct {
 	// releaseHostAffinityErrors maps host names to errors that ReleaseHostAffinities
 	// should return, simulating per-node "block not empty" failures during cleanup.
 	releaseHostAffinityErrors map[string]error
+
+	// utilization is what GetUtilization returns; the metrics sync calls it to
+	// find out how much of each pool is reserved.  Empty unless a test sets it.
+	utilization []*ipam.PoolUtilization
 }
 
 // gcBlocks returns the CIDRs of the blocks GarbageCollectColdIPs was called with.
@@ -452,7 +456,18 @@ func (f *fakeIPAMClient) RemoveIPAMHost(ctx context.Context, affinityCfg ipam.Af
 
 // GetUtilization returns IP utilization info for the specified pools, or for all pools.
 func (f *fakeIPAMClient) GetUtilization(ctx context.Context, args ipam.GetUtilizationArgs) ([]*ipam.PoolUtilization, error) {
-	panic("not implemented") // TODO: Implement
+	f.Lock()
+	defer f.Unlock()
+	if len(args.Pools) == 0 {
+		return f.utilization, nil
+	}
+	var wanted []*ipam.PoolUtilization
+	for _, poolUse := range f.utilization {
+		if slices.Contains(args.Pools, poolUse.Name) {
+			wanted = append(wanted, poolUse)
+		}
+	}
+	return wanted, nil
 }
 
 // EnsureBlock returns single IPv4/IPv6 IPAM block for a host as specified by the provided BlockArgs.

kube-controllers/pkg/controllers/node/ipam.go

@@ -17,8 +17,10 @@ package node
 import (
 	"context"
 	"fmt"
+	"maps"
 	"math"
 	"net"
+	"slices"
 	"strings"
 	"time"
 
@@ -62,6 +64,7 @@ var (
 	// Single dimension metrics. Legacy metrics are replaced by multidimensional equivalents above. Retain for
 	// backwards compatibility.
 	poolSizeGauge          *prometheus.GaugeVec
+	poolReservedGauge      *prometheus.GaugeVec
 	legacyAllocationsGauge *prometheus.GaugeVec
 	legacyBlocksGauge      *prometheus.GaugeVec
 	legacyBorrowedGauge    *prometheus.GaugeVec
@@ -98,6 +101,15 @@ func init() {
 	}, []string{"ippool"})
 	prometheus.MustRegister(poolSizeGauge)
 
+	poolReservedGauge = prometheus.NewGaugeVec(prometheus.GaugeOpts{
+		Name: "ipam_ippool_reserved",
+		// Careful: the metrics FV asserts on substrings of this output, so naming
+		// another metric here would make its absence checks match this help text.
+		Help: "Number of addresses in the IP Pool that an IPReservation covers, and so cannot be assigned. " +
+			"Refreshed on each IPAM sync, so a reservation change may take until the next one to appear.",
+	}, []string{"ippool"})
+	prometheus.MustRegister(poolReservedGauge)
+
 	// Total IP allocations.
 	legacyAllocationsGauge = prometheus.NewGaugeVec(prometheus.GaugeOpts{
 		Name: "ipam_allocations_per_node",
@@ -679,7 +691,7 @@ func (c *IPAMController) onPoolUpdated(pool *apiv3.IPPool) {
 
 func (c *IPAMController) onPoolDeleted(poolName string) {
 	unregisterMetricVectorsForPool(poolName)
-	clearPoolSizeMetric(poolName)
+	clearPoolMetrics(poolName)
 
 	c.poolManager.onPoolDeleted(poolName)
 }
@@ -766,9 +778,40 @@ func (c *IPAMController) updateMetrics() {
 	for node, num := range legacyBorrowedIPsByNode {
 		legacyBorrowedGauge.WithLabelValues(node).Set(float64(num))
 	}
+
+	c.updateReservedMetrics()
+
 	log.Debug("IPAM metrics updated")
 }
 
+// updateReservedMetrics publishes how much of each pool an IPReservation covers.
+// Unlike the counts above, this cannot be derived from the blocks we track: a
+// reservation makes addresses unassignable without allocating them, and it can
+// cover pool space that no block has been carved from yet.  So ask IPAM, which
+// reads the IPReservations and does the arithmetic over the whole pool CIDR.
+func (c *IPAMController) updateReservedMetrics() {
+	// Ask only for the pools we report on.  Left empty, GetUtilization would also
+	// work out the totals for the pseudo-pool it reports orphaned blocks under,
+	// which has no gauge.
+	pools := slices.Collect(maps.Keys(c.poolManager.allPools))
+	if len(pools) == 0 {
+		// An empty list means "every pool", which is not what we want here.
+		return
+	}
+
+	ctx, cancelCtx := context.WithTimeout(context.TODO(), 10*time.Second)
+	defer cancelCtx()
+
+	usage, err := c.client.IPAM().GetUtilization(ctx, ipam.GetUtilizationArgs{Pools: pools})
+	if err != nil {
+		log.WithError(err).Warn("Failed to get IP pool utilization; reserved-IP metrics may be stale")
+		return
+	}
+	for _, poolUse := range usage {
+		poolReservedGauge.With(prometheus.Labels{"ippool": poolUse.Name}).Set(float64(poolUse.Reserved))
+	}
+}
+
 // releaseUnusedBlocks looks at known empty blocks, and releases their affinity
 // if appropriate. A block is a candidate for having its affinity released if:
 //
@@ -1615,8 +1658,9 @@ func publishPoolSizeMetric(pool *apiv3.IPPool) {
 	poolSizeGauge.With(prometheus.Labels{"ippool": pool.Name}).Set(poolSize)
 }
 
-func clearPoolSizeMetric(poolName string) {
+func clearPoolMetrics(poolName string) {
 	poolSizeGauge.Delete(prometheus.Labels{"ippool": poolName})
+	poolReservedGauge.Delete(prometheus.Labels{"ippool": poolName})
 }
 
 // When we stop tracking a node, clear counters to prevent accumulation of stale metrics.

kube-controllers/pkg/controllers/node/ipam_test.go

@@ -23,6 +23,8 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	apiv3 "github.com/projectcalico/api/pkg/apis/projectcalico/v3"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -234,6 +236,36 @@ var _ = Describe("IPAM controller UTs", func() {
 		done()
 	})
 
+	It("should publish the reserved-IP gauge for the pools it tracks", func() {
+		c.Start(stopChan)
+		resume := c.pause()
+		defer resume()
+
+		poolReservedGauge.Reset()
+		poolName := "reserved-gauge-test-pool"
+
+		cli.IPAM().(*fakeIPAMClient).utilization = []*ipam.PoolUtilization{
+			{Name: poolName, Reserved: 6},
+			// Not tracked by the controller, so it should never be asked for -
+			// GetUtilization reports one of these for orphaned blocks.  The fake
+			// honours args.Pools, so a gauge here means we asked too broadly.
+			{Name: "orphaned allocation blocks", Reserved: 99},
+		}
+		c.onPoolUpdated(&apiv3.IPPool{
+			ObjectMeta: metav1.ObjectMeta{Name: poolName},
+			Spec:       apiv3.IPPoolSpec{CIDR: "10.0.0.0/24"},
+		})
+
+		c.updateReservedMetrics()
+
+		Expect(testutil.ToFloat64(poolReservedGauge.With(prometheus.Labels{"ippool": poolName}))).To(Equal(6.0))
+		Expect(testutil.CollectAndCount(poolReservedGauge)).To(Equal(1), "only tracked pools should be reported")
+
+		// Deleting the pool should take its gauge with it.
+		c.onPoolDeleted(poolName)
+		Expect(testutil.CollectAndCount(poolReservedGauge)).To(BeZero())
+	})
+
 	Describe("VMI allocation validation", func() {
 		makeVMIAllocation := func(ns, vmName string) *allocation {
 			return &allocation{