-
Notifications
You must be signed in to change notification settings - Fork 48
Add support for Tinkerbell as a platform #392
Conversation
a58e381
to
794b2af
Compare
Update:
Things which are missing in my opinion:
|
794b2af
to
4fdf684
Compare
I removed the symlinks mess and cleaned up the commit message. I also included some documentation. There are still several things missing, as mentioned in one of the commit messages and previous comment. Additionally, the code won't work with latest master of Tinkerbell, as they introduced breaking changes in tinkerbell/tink#88. |
4fdf684
to
7a66404
Compare
This is now resolved. |
f2f0ab7
to
f7065c4
Compare
f7065c4
to
9248f34
Compare
9248f34
to
f773ff4
Compare
Would be cool to get that merged, even if there is no tests for it. |
Rebased with #824 included, though it's not fully functional yet. |
09b8851
to
ac8c967
Compare
It seems the approach with Packet sandbox doesn't really work, as booting workers is not reliable for some reason and sometimes workers need to be rebooted from Packet console to boot using Tinkerbell. Also, if we switch nodes to use Tinkerbell for booting, then attaching public IP address to controller nodes and configuring them becomes non trivial, I start to doubt this is worth the effort. Perhaps we should use similar testbed to what we have on bare metal platform to test this. |
ac8c967
to
bcdb26f
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.
Awesome work @invidian 🥳 . Love how much I got to learn from it. I have suggested some minor changes, please have a look
assets/terraform-modules/platforms/tinkerbell/controllers/versions.tf
Outdated
Show resolved
Hide resolved
assets/terraform-modules/platforms/tinkerbell/workerpool/variables.tf
Outdated
Show resolved
Hide resolved
assets/terraform-modules/platforms/tinkerbell/workerpool/versions.tf
Outdated
Show resolved
Hide resolved
assets/terraform-modules/tinkerbell-sandbox/assets/deploy/tls/gencerts.sh
Show resolved
Hide resolved
b437839
to
0118018
Compare
I like how we cheery picked commit from ccm PR to another PR and then again to this PR. 😄 |
0118018
to
a3ae87e
Compare
a3ae87e
to
ec311b3
Compare
Addressed. Please have a look again. |
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.
Needs a rebase. LGTM after that.
ec311b3
to
f18ba1a
Compare
Rebased. |
This commit adds WorkerPoolNamesUnique method, which takes list of worker pool objects as an argument and checks, if all of them has unique names. This method allows to deduplicate the logic, which should be implemented for all platforms, to check if all defined worker pools have unique names. Signed-off-by: Mateusz Gozdek <[email protected]>
This commit adds 3 Terraform modules, which aims to eventually deduplicate Ignition configuration between all the platforms, by providing extendable base configuration. For now, only Tinkerbell platform will be consuming those modules. Eventually, we should migrate all platforms to use it, however this is not trivial task, as we lack tests on this level, so it must be done carefully. Signed-off-by: Mateusz Gozdek <[email protected]>
Instead of "any". Signed-off-by: Mateusz Gozdek <[email protected]>
Signed-off-by: Mateusz Gozdek <[email protected]>
674fd26
to
95685f5
Compare
This commit adds Tinkerbell as a supported platform by the Lokomotive. The Terraform code consumes newly introduced controller and worker Terraform modules, which reduces the amount of code required for introducing this new platform. Closes #382. Signed-off-by: Mateusz Gozdek <[email protected]>
To avoid weird race conditions which we saw in Tinkerbell pipeline. Signed-off-by: Mateusz Gozdek <[email protected]>
To make it more approachable on smaller machines. Signed-off-by: Mateusz Gozdek <[email protected]>
As word 'ether' triggers it. Signed-off-by: Mateusz Gozdek <[email protected]>
95685f5
to
7681aba
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
All requested changes has been applied.
Note: This PR is largely incomplete. I open it to provide a preview of PoC for having more centralized Terraform code, without 99% duplication between the platforms. It also introduces new structure of assets, without
lokomotive-kubernetes
paths.There are new Terraform modules introduced:
node
module, containing Ignition configuration to all kubernetes nodes, so for example kubelet configuration. This module is not being instantiated to to avoid nesting Terraform modules. Instead, files from this module are shared via symlinks to next modules.worker
module, which contains worker-related Ignition snippets and configuration values fornode
module templates. It includes default worker labels, extra worker mounds like iscsid daemon etc.controller
module, which contains controller-node generic Ignition snippets and configuration values fornode
module templates. It includes bootkube, etc, labels and tains for kubelet etc.Additionally, all modules attempt to re-use files like
variables.tf
via symlinks, so definitions of variables can be kept in one place, when some values needs to be passed down trough the stack. There might be duplications or unused variables, so this requires cleaning up.Go implementation needs to be added as well.
Closes #382