- 
                Notifications
    
You must be signed in to change notification settings  - Fork 32
 
[feat] Add Support for VPC only backends #803
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
| 
           I didn't realize we had added NB-VPC support in CAPL but with fixed /30 ranges and just needed to make it enabled by default. Do we need changes in machinecontroller to consume the new changes in machineTemplate? Not sure how they are being used based on the current state of the PR.  | 
    
          
 Yup. We added these changes few months back I think. As for machinecontroller, we don't need to update anything. Just setting privateIP to false and removing public interface does the job!  | 
    
          Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@            Coverage Diff             @@
##             main     #803      +/-   ##
==========================================
+ Coverage   63.20%   63.22%   +0.01%     
==========================================
  Files          71       71              
  Lines        7359     7363       +4     
==========================================
+ Hits         4651     4655       +4     
  Misses       2435     2435              
  Partials      273      273              ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
  | 
    
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 introduces VPC-only backend support for the Cluster API Provider Linode (CAPL), removing the requirement for public/private IPs on nodes. The changes enable cluster communication exclusively through VPC networks while supporting external traffic through VPC NAT with public IPs.
Key changes include:
- Addition of 
EnableVPCBackendsfield to toggle VPC-scoped NodeBalancer creation and VPC backend IP usage - Refactoring of IP selection logic to prioritize VPC IPs over traditional private IPs when VPC backends are enabled
 - Simplification of NodeBalancer VPC creation conditions to check VPC references before IPv4 range requirements
 
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description | 
|---|---|
api/v1alpha2/linodecluster_types.go | 
Adds EnableVPCBackends field to NetworkSpec with immutable validation | 
cloud/services/loadbalancers.go | 
Introduces helper functions and simplifies VPC NodeBalancer creation logic | 
internal/controller/linodecluster_controller_helpers.go | 
Refactors IP selection logic to prioritize VPC IPs when enabled | 
config/crd/bases/*.yaml | 
Updates CRDs with new EnableVPCBackends field and validation rules | 
cloud/services/loadbalancers_test.go | 
Updates test cases to include new EnableVPCBackends field | 
internal/controller/linodecluster_controller_test.go | 
Adds SubnetID to test data and mock call expectations | 
internal/controller/linodecluster_controller_helpers_test.go | 
Updates test cases with EnableVPCBackends field | 
docs/src/reference/out.md | 
Documents the new EnableVPCBackends field | 
hack/generate-flavors.sh | 
Updates flavor generation log message | 
Comments suppressed due to low confidence (1)
internal/controller/linodecluster_controller_helpers.go:139
- [nitpick] The variable name 
resultscould be more descriptive, such asipPortCombosorportCombinationsto better reflect its purpose. 
	results = append(results, fmt.Sprintf("%s:%d", ip, apiServerLBPort))
API: add spec.network.enableVPCBackends (default false, immutable). When true and VPCRef/VPCID is set, create the NodeBalancer in the target VPC and prefer VPC backend IPs. NodeBalancerBackendIPv4Range remains optional and is honored when provided. Services: introduce ShouldUseVPC(scope) and DetermineAPIServerLBPort(scope); EnsureNodeBalancer uses VPC SubnetID and optional IPv4Range; node registration prefers VPC internal IPs when enabled, otherwise falls back to Linode private IPs. Controller: refactor getIPPortCombo to select VPC IPs first, factor helpers findFirstVPCInternalIP/findFirstPrivateInternalIP and buildPortCombosForIP; reuse DetermineAPIServerLBPort for DNS endpoint. CRDs+Docs: extend LinodeCluster CRDs with enableVPCBackends (default false, immutable); update docs reference for the new field. Tests: update unit tests to set EnableVPCBackends=true in VPC scenarios and to use new helpers; keep behavior unchanged when flag is false.
db1bca2    to
    8fb3e11      
    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.
LGTM

Summary
Feature-flagged support for VPC-scoped NodeBalancer backends. When enabled, control plane backends use VPC internal IPs and the NodeBalancer is created inside the target VPC. External traffic continues to egress via VPC NAT with a public IP.
Key changes
spec.network.enableVPCBackends(defaultfalse, immutable). Whentrueand eitherspec.VPCReforspec.VPCIDis set, create the NodeBalancer in the VPC and prefer VPC backend IPs. Optionalspec.network.nodeBalancerBackendIPv4Rangeis honored if provided.ShouldUseVPC(scope)andDetermineAPIServerLBPort(scope).SubnetIDand optionalIPv4Range; backend registration prefers VPC internal IPs, falls back to Linode private IPs when the flag is disabled.getIPPortCombonow prefers VPC IPs when enabled; factored helpers to select IPs and compose port pairs; DNS endpoint now reusesDetermineAPIServerLBPort.enableVPCBackends(defaultfalse, immutable); docs updated accordingly.Decisions
enableVPCBackendsis explicitly set totrue.What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
TODOs: