-
Notifications
You must be signed in to change notification settings - Fork 68
[feat] add support for using linode interfaces (beta-only) #435
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #435 +/- ##
==========================================
- Coverage 72.17% 71.87% -0.31%
==========================================
Files 19 19
Lines 3551 3598 +47
==========================================
+ Hits 2563 2586 +23
- Misses 751 773 +22
- Partials 237 239 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e52430a
to
9d26186
Compare
Unless there's some refactoring on the CCM so we can use the instanceCache outside of the linode package (running into cyclic dependencies), I think we’re going to need the flag for using new network interfaces. |
3d15db2
to
7e603c6
Compare
cb8ac5b
to
59887e9
Compare
59887e9
to
ae90c16
Compare
205654f
to
e79d121
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
This PR adds support for Linode Interfaces (currently in beta), enabling the CCM to work with instances using the new interface generation. The key change is implementing dual-path functionality that handles both legacy instance configuration interfaces and the new Linode interfaces based on the InterfaceGeneration
field.
Key changes:
- Added support for detecting and handling Linode interfaces vs legacy instance config interfaces
- Implemented IPv6 range extraction from the new interface format
- Updated route management to work with both interface types
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
cloud/nodeipam/ipam/cloud_allocator.go |
Core logic to detect interface generation and handle both Linode interfaces and legacy config interfaces for IPv6 CIDR allocation |
cloud/nodeipam/ipam/cloud_allocator_test.go |
Comprehensive test cases covering both interface types with mocking for the new interface APIs |
cloud/linode/route_controller.go |
Route management refactored to support both interface generations with a unified handler function |
cloud/linode/route_controller_test.go |
Test coverage for route operations using Linode interfaces |
cloud/linode/client/client.go |
Interface definition for new Linode interface API methods |
cloud/linode/client/client_with_metrics.go |
Prometheus metrics wrapper for the new interface methods |
cloud/linode/client/mocks/mock_client.go |
Generated mock implementations for testing the new interface methods |
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.
Looks good to me!
Linode Interfaces is in beta. This adds support so the CCM can work on Linodes using the new linode interfaces.
NOTE: If you want to test this locally you need to opt in to the Linode Interfaces beta AND the ability to use VPC-only backends (see linode/cluster-api-provider-linode#803).
Testing with linode interfaces
adumaine/linode-cloud-controller-manager:linode-interfaces
for the CCM repo and version, e.g.:LinodeMachineTemplate.Spec.Template.Spec.Interfaces
with:and adding the following to the
LinodeCluster.Spec.Network
:General:
Pull Request Guidelines: