net: delete the superseded node-CIDR controller - #676
Open
Philip Lombardi (plombardi89) wants to merge 1 commit into
Open
net: delete the superseded node-CIDR controller#676Philip Lombardi (plombardi89) wants to merge 1 commit into
Philip Lombardi (plombardi89) wants to merge 1 commit into
Conversation
internal/net/controller/controller.go is a 542-line Kubernetes node controller
that allocates pod CIDRs per node. Nothing constructs it. Its package is
imported by cmd/unbounded-net-controller, which is what kept it out of every
previous sweep, but the symbols that import reaches are NewSiteController,
NewGatewayPoolController, NewPeeringAggregationController,
NewManagedKubeProxyController, ManagedKubeProxyOptions, NodeSiteLabel,
TunnelMTUAnnotation and WireGuardPubKeyAnnotation. Not Controller.
Without -test, `make deadcode` reports every one of its methods unreachable:
InformerSynced, NewController, enqueueNode, matchesNodeFilter, Run, runWorker,
processNextWorkItem, syncHandler, allocateCIDRsForNode, patchNodeCIDRs,
InitializeAllocator and DryRun. With -test, four remain unreachable, and the
only thing holding the rest up is controller_helpers_test.go, whose five tests
exist for no other purpose. Site-based pod CIDR assignment superseded this; the
live path is SiteController.buildAssignmentAllocator.
The file and its test go together. Keeping the tests would have kept twelve
dead methods alive and moved them from one section of the report to the other.
This is the one deletion in the series where intent is inferred from the call
graph rather than read from a document. If there is a plan to revive this
controller, this is the commit to push back on.
The cascade:
- allocator.ContainsCIDR had no caller at all.
- allocator.ParseCIDRs had no caller in production. Both live callers of
NewAllocator - site_controller.go and webhook/validation.go - build their
pools with splitCIDRBlocks. It was exported API kept alive entirely by
allocator_test.go, so it moves into that file as an unexported helper and
the tests are unchanged apart from the name.
- computeReachableRoutes was a two-line wrapper around computeReachable
called only from peering_aggregation_controller_test.go. The wrapper goes
and the test calls computeReachable directly, which is what it was actually
exercising.
- SiteController.GetSiteForNode had no reference anywhere.
The three unobserved promauto metric vars in controller/metrics.go are
deliberately left alone; they are tracked in #672.
Refs #670
This was referenced Aug 26, 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.
Part of the #670 dead-code sweep, split by component. The tooling that produced these findings is #673; this PR is deletions only and is independent of the other eight — the file sets are disjoint, so they can be reviewed and merged in any order and in parallel.
Why two analyzers were needed to find this
unusedcannot see dead exported code underinternal/. That is a documented design decision, not a gap:unused/unused.go:48-62holds that a package uses its exported named types and a named type uses its exported methods, so every method on an exported type is transitively live because the type is exported.x/tools/cmd/deadcodedoes not close that gap on its own either.x/tools/go/callgraph/rta/rta.go:281-287,433-437— converting a value to an interface materializes its runtime type and marks every exported method of that type reachable, because reflection could call any of them. Onevar i any = xanywhere buys all ofx's exported methods a clean bill of health.So #673 adds both
deadcodeandhack/cmd/unreferenced, which resolves identifiers instead of tracing reachability. Neither subsumes the other, and findings below are attributed to whichever one saw them.net: delete the superseded node-CIDR controller
internal/net/controller/controller.go is a 542-line Kubernetes node controller
that allocates pod CIDRs per node. Nothing constructs it. Its package is
imported by cmd/unbounded-net-controller, which is what kept it out of every
previous sweep, but the symbols that import reaches are NewSiteController,
NewGatewayPoolController, NewPeeringAggregationController,
NewManagedKubeProxyController, ManagedKubeProxyOptions, NodeSiteLabel,
TunnelMTUAnnotation and WireGuardPubKeyAnnotation. Not Controller.
Without -test,
make deadcodereports every one of its methods unreachable:InformerSynced, NewController, enqueueNode, matchesNodeFilter, Run, runWorker,
processNextWorkItem, syncHandler, allocateCIDRsForNode, patchNodeCIDRs,
InitializeAllocator and DryRun. With -test, four remain unreachable, and the
only thing holding the rest up is controller_helpers_test.go, whose five tests
exist for no other purpose. Site-based pod CIDR assignment superseded this; the
live path is SiteController.buildAssignmentAllocator.
The file and its test go together. Keeping the tests would have kept twelve
dead methods alive and moved them from one section of the report to the other.
This is the one deletion in the series where intent is inferred from the call
graph rather than read from a document. If there is a plan to revive this
controller, this is the commit to push back on.
The cascade:
NewAllocator - site_controller.go and webhook/validation.go - build their
pools with splitCIDRBlocks. It was exported API kept alive entirely by
allocator_test.go, so it moves into that file as an unexported helper and
the tests are unchanged apart from the name.
called only from peering_aggregation_controller_test.go. The wrapper goes
and the test calls computeReachable directly, which is what it was actually
exercising.
The three unobserved promauto metric vars in controller/metrics.go are
deliberately left alone; they are tracked in #672.
Refs #670
Verification
go build ./...,go vet ./...,golangci-lintover the touched trees (0 issues),make fmtproducing no diff, andgo build -tags e2e,integrationtest,storageboundary ./.... Full suite runs in CI.The eight deletion branches were reassembled onto the tooling commits and diffed against the original combined branch: byte-identical, so nothing was lost or duplicated in the split.