[feat] VLAN support for CAPL clusters #525
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #525 +/- ##
==========================================
- Coverage 64.73% 64.70% -0.03%
==========================================
Files 78 79 +1
Lines 4180 4236 +56
==========================================
+ Hits 2706 2741 +35
- Misses 1213 1232 +19
- Partials 261 263 +2 ☔ View full report in Codecov by Sentry. |
|
For this to work we will need this PR merged, a release for the CCM cut, and the CCM version bumped here, otherwise external IPs will not get set on the Nodes. |
AshleyDumaine
changed the title
| } | ||
|
|
||
| func createIPsConfigMap(ctx context.Context, machineScope *scope.MachineScope, ip string) error { | ||
| return machineScope.Client.Create(ctx, &corev1.ConfigMap{ |
There was a problem hiding this comment.
can you give an example of what is in this configmap? I wonder if we could use part of the LInodeCluster spec for this information instead
There was a problem hiding this comment.
it is a simple map of machineName -> IP address
apiVersion: v1
data:
schinmai-test-control-plane-f4g7c: 10.0.0.3
schinmai-test-control-plane-mfxh6: 10.0.0.2
schinmai-test-control-plane-mhczj: 10.0.0.1
kind: ConfigMap
metadata:
creationTimestamp: "2024-10-02T19:12:38Z"
labels:
clusterctl.cluster.x-k8s.io/move: "true"
name: schinmai-test-ips
namespace: default
resourceVersion: "7452"
uid: cf59bb0a-4340-4461-8317-c02748d72050
There was a problem hiding this comment.
I opted for a configmap so that we can have a record of what IP was assigned to which machine for updates (the linodeMachine spec doesn't have it)
There was a problem hiding this comment.
could we just add this info to the status on each of the LinodeMachines in this case?
There was a problem hiding this comment.
still not totally sure on the consumer of this information so I might be missing part of the context
There was a problem hiding this comment.
this info is indeed saved into the status of linodeMachines - When X linodeMachines are reconciled in parallel, the linodeMacine controller needs to pick an IP for each instance, independently. To avoid assigning the same IP to multiple machines, I'm using the configmap as a locking mechanism, and a record of the IPs assigned to each machine. IF the configmap was sucessfully updated by the reconcile loop, it guaranteees that no other machine can get that IP and we eliminate conflicts
tchinmai7
changed the title
| vlanIPsMu sync.RWMutex | ||
| // vlanIPsMap stores clusterName and a list of VlanIPs assigned to that cluster | ||
| vlanIPsMap = make(map[string]*ClusterIPs, 0) | ||
| vlanIPRange = "10.0.0.0/8" |
There was a problem hiding this comment.
We are using /11 and /8 both. Not sure if thats intentional or we wanted to use just one.
There was a problem hiding this comment.
this is used to inform netip.Prefix what the network prefix is, and is what makes nextIP work - the /11 is what gets assigned on the node - it could be much lower too
Co-authored-by: Rahul Sharma <[email protected]>
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: