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.
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
[feat] VLAN support for CAPL clusters #525
[feat] VLAN support for CAPL clusters #525
Changes from 8 commits
f1962fa
1a3ebb7
3d23699
ca234c0
d70ff4a
8e6245f
4e0bb74
11e3c26
14bb6c1
62058c0
c37b625
acfce6d
a48857f
69e345d
e145140
789d49b
d62b229
e77147d
f1cd17b
82de6a2
f6ecb55
a366ba7
171a0c9
a8879f4
71b6180
5e6b217
24a121a
c9e8983
392b7e8
00c3dc3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Check warning on line 113 in api/v1alpha2/linodecluster_webhook.go
Codecov / codecov/patch
api/v1alpha2/linodecluster_webhook.go#L110-L113
Check failure on line 223 in controller/linodecluster_controller.go
GitHub Actions / go-analyze
Check warning on line 270 in controller/linodecluster_controller.go
Codecov / codecov/patch
controller/linodecluster_controller.go#L266-L270
Check warning on line 275 in controller/linodecluster_controller.go
Codecov / codecov/patch
controller/linodecluster_controller.go#L272-L275
Check warning on line 120 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L120
Check warning on line 122 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L122
Check warning on line 124 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L124
Check warning on line 153 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L150-L153
Check warning on line 155 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L155
Check warning on line 157 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L157
Check warning on line 232 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L229-L232
Check warning on line 382 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L380-L382
Check warning on line 387 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L384-L387
Check warning on line 390 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L389-L390
Check warning on line 392 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L392
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is a simple map of machineName -> IP address
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we just add this info to the status on each of the
LinodeMachines
in this case?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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this info is indeed saved into the
status
of linodeMachines - When XlinodeMachines
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 conflictsCheck warning on line 407 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L395-L407
Check warning on line 420 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L410-L420
Check warning on line 423 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L422-L423
Check warning on line 425 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L425
Check warning on line 429 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L428-L429
Check warning on line 434 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L432-L434
Check warning on line 440 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L437-L440
Check warning on line 442 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L442
Check warning on line 445 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L444-L445
Check warning on line 449 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L448-L449
Check warning on line 453 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L452-L453
Check warning on line 459 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L457-L459
Check warning on line 466 in controller/linodemachine_controller_helpers.go
Codecov / codecov/patch
controller/linodemachine_controller_helpers.go#L462-L466