Account for IPReservations in IP pool utilization - #13331
Merged
Conversation
`calicoctl ipam show` and the kube-controllers IPAM metrics reported reserved addresses as free. `GetUtilization` counted a block's whole `Unallocated` list as available and never read the `IPReservation` resource, so a reservation covering pool space that no block had been carved from was invisible. `GetUtilization` now reports `Capacity`, `InUse`, `Reserved` and `Available` for every pool and every block. `InUse` and `Reserved` overlap when an address was allocated before it was reserved, so `Available` is computed directly instead of by subtraction. Pool-level counts cover the whole pool CIDR, worked out as a set operation (pool minus reservations minus blocks) so that overlapping or nested reservations count once and space with no block yet is included. `calicoctl ipam show` gains an IPS RESERVED column and takes all four numbers from the library. kube-controllers exports a new `ipam_ippool_reserved` gauge; the existing metrics are unchanged. Also stops `getReservedCIDRs` dereferencing a nil CIDR when an `IPReservation` holds a malformed entry, which the surrounding code already meant to skip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
marvin-tigera
added
release-note-required
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes IPAM utilization reporting so that addresses covered by IPReservation are no longer counted as free in both calicoctl ipam show and kube-controllers IPAM metrics. It extends GetUtilization to surface Capacity, InUse, Reserved, and Available per pool and per block, including reservations that apply to pool CIDR space that has not yet been carved into blocks, using go4.org/netipx for correct set arithmetic with overlapping/nested reservations.
Changes:
- Extend
libcalico-goIPAM utilization reporting to account forIPReservationacross pools/blocks and expose new utilization fields. - Update
calicoctl ipam showto display the new reserved column and consume utilization numbers directly from the library. - Add
ipam_ippool_reservedPrometheus gauge in kube-controllers and add unit/FV test coverage; update IPAM design docs accordingly.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| typha/deps.txt | Add go4.org/netipx to Typha dependency manifest. |
| node/deps.txt | Add go4.org/netipx to Node dependency manifest. |
| libcalico-go/lib/ipam/reserved.go | New helper for pool-wide set arithmetic with reservations and blocks. |
| libcalico-go/lib/ipam/reserved_test.go | Unit tests for pool/reservation/block set arithmetic, including overlap and IPv6. |
| libcalico-go/lib/ipam/ipam.go | Teach GetUtilization to compute Capacity/InUse/Reserved/Available and read IPReservations; fix malformed reservation handling. |
| libcalico-go/lib/ipam/ipam_types.go | Extend utilization structs with new fields and clarify overlap semantics. |
| libcalico-go/lib/ipam/ipam_test.go | Add e2e datastore tests for utilization with reservations and overlap cases. |
| libcalico-go/lib/ipam/ipam_block.go | Add per-block NumReservedAddresses helper. |
| libcalico-go/lib/ipam/ipam_block_test.go | Unit tests for NumReservedAddresses. |
| libcalico-go/deps.txt | Add go4.org/netipx to libcalico-go dependency manifest. |
| kube-controllers/pkg/controllers/node/metrics_fv_test.go | FV expectations for new reserved metric; add helper to create IPReservation. |
| kube-controllers/pkg/controllers/node/ipam.go | Add and publish ipam_ippool_reserved gauge and clear it on pool deletion. |
| kube-controllers/pkg/controllers/node/ipam_test.go | UT coverage that reserved gauge is published only for tracked pools and cleared on deletion. |
| kube-controllers/pkg/controllers/node/fake_client.go | Implement GetUtilization in fake IPAM client for metrics unit tests. |
| kube-controllers/deps.txt | Add go4.org/netipx to kube-controllers dependency manifest. |
| hack/deps.txt | Add go4.org/netipx to hack tooling dependency manifest. |
| go.sum | Add go4.org/netipx checksums. |
| go.mod | Add go4.org/netipx module requirement. |
| felix/deps.txt | Add go4.org/netipx to Felix dependency manifest. |
| e2e/deps.txt | Add go4.org/netipx to e2e dependency manifest. |
| design/ipam/ipam-other-callers.md | Document calicoctl ipam show columns now being sourced from GetUtilization. |
| design/ipam/ipam-gc.md | Document kube-controllers reserved metric behavior and caveats. |
| design/ipam/ipam-datastore.md | Note IPReservation is read by GetUtilization for reporting correctness. |
| design/ipam/ipam-core-library.md | Document new GetUtilization semantics and overlap rules. |
| confd/deps.txt | Add go4.org/netipx to confd dependency manifest. |
| cni-plugin/deps.txt | Add go4.org/netipx to CNI plugin dependency manifest. |
| cmd/deps.txt | Add go4.org/netipx to cmd dependency manifest. |
| calicoctl/deps.txt | Add go4.org/netipx to calicoctl dependency manifest. |
| calicoctl/calicoctl/commands/ipam/show.go | Add “IPS RESERVED” column and consume library-provided utilization fields. |
| app-policy/deps.txt | Add go4.org/netipx to app-policy dependency manifest. |
| apiserver/deps.txt | Add go4.org/netipx to apiserver dependency manifest. |
Ask GetUtilization only for the pools the controller reports on. Left empty it also totals the pseudo-pool that orphaned blocks are listed under, which has no gauge. The fake now honours `args.Pools`, so the existing test proves the request is scoped rather than the results being filtered afterwards. Fold the `go4.org/netipx` require into the direct block instead of leaving it standalone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two CI failures from the kube-controllers metrics FV. The new gauge's help text named `ipam_allocations_in_use`, and the FV asserts metric absence with a substring match, so the help text made an absence check match after the allocations had gone. Dropped the cross-reference and left a note by the metric. The reservation test assumed the gauge would refresh within 10s of creating an IPReservation, but the controller has `list` and not `watch` on the resource, so a reservation change does not wake the sync loop; the gauge refreshes on the next sync from another cause or on the periodic one. The test now creates the reservation and then the pool, so pool creation is what triggers the sync. Documented the cadence on the metric and in design/ipam/ipam-gc.md, including what it would take to make it prompt (a watch plus a ClusterRole change in the chart and in tigera/operator). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fasaxc
commented
Jul 27, 2026
Comment on lines
+39
to
+41
| // /24 while another names a single IP inside it, and an L2 subnet contributes | ||
| // its network and broadcast addresses on top — so the counting is a set | ||
| // operation rather than a sum over the CIDRs. |
Member
Author
There was a problem hiding this comment.
L2 is enterprise only, either leave out the reference or flag that it's ent-only
Member
Author
There was a problem hiding this comment.
Claude: Dropped the L2 clause — overlapping IPReservations justify the set arithmetic on their own, so the comment reads fine without it. The enterprise side documents the subnet-edge source where it is added, so nothing is lost there. Removed the same clause on the enterprise branch too, so the file stays identical across the two and future picks stay clean.
fasaxc
commented
Jul 27, 2026
L2 subnets are an enterprise-only source of reserved addresses, so the comment in `countPoolSpace` should not cite them; overlapping IPReservations already justify the set arithmetic on their own. Pool validation keeps pools far below the point where a count would not fit in an int, so say the saturation is defensive rather than implying it is reachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
caseydavenport
left a comment
caseydavenport
left a comment
Member
There was a problem hiding this comment.
Lookin' pretty good - just one comment about putting GetUtilization on the critical path. WDYT?
updateReservedMetrics called GetUtilization on every IPAM sync, which lists every allocation block. The node controller's sync loop shares a goroutine with leak GC and has been overloaded before, and the controller threw all of that block work away: it read only PoolUtilization.Reserved. kube-controllers now watches IPReservation on its own syncer and counts the covered addresses per pool with a new ipam.NumReservedIPsInCIDR, so the sync loop makes no datastore request for the metric. The arithmetic is still the library's - GetUtilization and the new helper are both thin callers of the set code in reserved.go - so the gauge still agrees with `calicoctl ipam show`. The helper takes IPReservations rather than CIDRs so a variant can take a second kind of reserving resource without reshaping its callers. Watching the resource needs `watch` in the kube-controllers ClusterRole; the chart and manifests carry it here, and tigera/operator needs the matching change. The gauge's help text loses its staleness caveat: a reservation change now wakes the loop, which the metrics FV asserts by creating the reservation after the pool. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 tasks
caseydavenport
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
calicoctl ipam showand the kube-controllers IPAM metrics reported reserved addresses as free.GetUtilizationcounted a block's wholeUnallocatedlist as available and never read theIPReservationresource, so a reservation covering pool space that no block had been carved from was invisible.GetUtilizationnow reportsCapacity,InUse,ReservedandAvailablefor every pool and every block:CapacityInUseReservedAvailableInUseandReserveddeliberately overlap (an address can be reserved after it was handed out), soAvailableis computed directly rather than by subtracting the others fromCapacity.Pool-level counts cover the whole pool CIDR, not just the blocks carved from it — a reservation over unblocked space is still unassignable. They are worked out as a set operation (pool minus reservations minus blocks) using
go4.org/netipx, so overlapping and nested reservations are counted once: oneIPReservationmay cover a/24while another names a single address inside it. Per-block counts keep using the existingaddrFilter, so they agree with what allocation will actually do.calicoctl ipam showgains an IPS RESERVED column and takes all four numbers from the library instead of deriving them. Since the columns can overlap, their percentages no longer necessarily add up to 100.kube-controllers exports a new
ipam_ippool_reservedgauge, labelledippoollikeipam_ippool_size. Existing metrics are unchanged. It does not callGetUtilizationfor it — the IPAM sync loop shares a goroutine with leak GC, so nothing there should list every block. InsteadIPReservationjoins the controller's syncer andipam.NumReservedIPsInCIDRdoes the pool arithmetic on that cached state; both it andGetUtilizationare thin callers of the same set code, so the gauge can't drift from whatcalicoctl ipam showprints.Watching the resource needs
watchonipreservationsin the kube-controllers ClusterRole: the chart and regenerated manifests are here, and tigera/operator#5115 has the operator side. If the two get out of step the List still succeeds and the controller stays in-sync, so the symptom is a hot re-list rather than a stalled controller.Also fixes
getReservedCIDRsdereferencing a nil CIDR when anIPReservationholds a malformed entry — the surrounding code logs "Ignoring" but did not skip.New dependency:
go4.org/netipx(stdlib-only, ~1.3k LOC).CORE-13146
Testing
GetUtilizationtable inlibcalico-go/lib/ipam/ipam_test.gocovering a reservation inside a block, over pool space with no block, over an already-allocated address, and overlapping/nested reservations.reserved_test.go): nesting, duplicates, disjoint, covering the whole pool, IPv6, and saturation for pools larger than anint.NumReservedAddressesspecs inipam_block_test.go.reservedCIDRs: bare IPs, whitespace, and malformed entries (which covers the nil-CIDR fix directly).IPPoolandIPReservationKVPairs through the controller's update path and asserting the gauge appears, changes, clears on reservation delete, and is removed on pool delete.IPReservationsecond, over pool space no block covers, then waits for the gauge to change and change back on delete — it only passes if the watch is live.libcalico-go/lib/ipamsuite, the node controller UTs, and the metrics FV against a freshly built image all pass locally.Release note:
🤖 Generated with Claude Code