-
Notifications
You must be signed in to change notification settings - Fork 69
[feat] : add ipv6 ipam to CCM #369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #369 +/- ##
==========================================
- Coverage 75.39% 72.10% -3.29%
==========================================
Files 16 19 +3
Lines 2983 3402 +419
==========================================
+ Hits 2249 2453 +204
- Misses 545 722 +177
- Partials 189 227 +38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
47b9f3b
to
477a5a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- go.mod: Language not supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- go.mod: Language not supported
Comments suppressed due to low confidence (1)
cloud/linode/nodeipamcontroller.go:45
- The default IPv6 node CIDR mask size has been changed from 64 to 112. Please confirm that this new value aligns with the desired network design and update any related documentation if necessary.
defaultNodeMaskCIDRIPv6 = 112
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- go.mod: Language not supported
35a5cc4
to
ee7a97c
Compare
3c9c16f
to
9a369d3
Compare
9a369d3
to
3d7f16a
Compare
3d7ba04
to
4f49c34
Compare
4f49c34
to
bbf17ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds IPv6 IPAM support to the Linode cloud controller manager by introducing a new node IPAM controller and extending the existing Linode client interfaces to retrieve IPv6 configuration.
- Introduce
cloud/nodeipam
package with a dual-stack allocator that assigns both IPv4 and IPv6 PodCIDRs. - Extend
Client
interface withListInstanceConfigs
, update mocks, metrics, andgo.mod
dependencies for IPv6 support. - Refactor
cloud/linode/nodeipamcontroller.go
to enforce a single IPv4 cluster CIDR, set default IPv6 mask to /112, and wire in the new allocator.
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
go.mod | Bumped indirect dependencies, added Gomega, replaced LinodeGo fork |
cloud/nodeipam/node_ipam_controller.go | New top-level IPAM controller coordinating IPv4/IPv6 allocators |
cloud/nodeipam/ipam/ | New IPAM package (allocators, utils, tests, docs) |
cloud/linode/nodeipamcontroller.go | Refactored startup, processCIDRs , default masks, signature change |
cloud/linode/client/client.go | Added ListInstanceConfigs to the Client interface |
cloud/linode/client/client_with_metrics.go | Instrumented ListInstanceConfigs |
cloud/linode/client/mocks/mock_client.go | Added mock methods for ListInstanceConfigs |
Makefile | Expanded test coverage to include IPAM packages |
Comments suppressed due to low confidence (3)
cloud/nodeipam/ipam/cloud_allocator.go:335
- [nitpick] Consider adding unit tests for allocateIPv6CIDR to verify correct IPv6 parsing and that each node receives a unique /112 range.
return nil, fmt.Errorf("node %s has invalid ProviderID %s, expected prefix '%s'", node.Name, node.Spec.ProviderID, providerIDPrefix)
cloud/linode/nodeipamcontroller.go:47
- [nitpick] The signature of startNodeIpamController changed from the generic cloudprovider.Interface to a concrete *linodeCloud type. This may reduce flexibility; consider keeping the interface abstraction if other clouds or unit tests rely on the original interface.
func startNodeIpamController(stopCh <-chan struct{}, cloud *linodeCloud, nodeInformer v1.NodeInformer, kubeclient kubernetes.Interface) error {
cloud/linode/nodeipamcontroller_test.go:33
- [nitpick] Tests for processCIDRs and setNodeCIDRMaskSizes no longer cover IPv6 or dual-stack scenarios. Consider adding tests to validate IPv6 parsing and ensure mask sizes are applied correctly in both single- and dual-stack configurations.
func Test_setNodeCIDRMaskSizes(t *testing.T) {
bbf17ad
to
4ca7bff
Compare
return fmt.Errorf("failed to allocate cidr from cluster cidr: %w", err) | ||
} | ||
allocatedCIDRs[0] = podCIDR | ||
if allocatedCIDRs[1], err = c.allocateIPv6CIDR(ctx, node); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add logic to enable/disable ipv6 cidr allocation via flag
General:
This PR enables ipv6 ipam allocation to nodes. If CCM is started with
--allocate-node-cidrs
, it will allocate both ipv4 and ipv6 pod CIDRs to nodes.This PR adds IPv6 IP Address Management (IPAM) support to the cloud controller manager by extending the node IPAM allocator to compute and assign unique /112 IPv6 ranges from each node’s /64, updating Linode client interfaces and tests accordingly.
Introduce a new cloud/nodeipam controller and ipam package that allocates both IPv4 and IPv6 PodCIDRs.
Extend Linode client API (ListInstanceConfigs) and update mocks, metrics, and go.mod deps to support IPv6 range retrieval.
Update the Linode-specific nodeipamcontroller to pass IPv6 mask defaults and enforce only one IPv4 cluster CIDR.
Note
In this PR, we are adding a function to calculate unique /112 ipv6 cidr range for each node from the /64 cidr allocated to it. Dependencies for starting CCM with
--allocate-node-cidrs
flag:Sample CAPL PR which does the same: linode/cluster-api-provider-linode#773
Pull Request Guidelines: