Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Add support for Tinkerbell as a platform #392

Merged
merged 8 commits into from
Dec 2, 2020
Merged

Add support for Tinkerbell as a platform #392

merged 8 commits into from
Dec 2, 2020

Conversation

invidian
Copy link
Member

@invidian invidian commented May 5, 2020 •
edited

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 for node 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 for node 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

@invidian invidian mentioned this pull request May 6, 2020
Closed
@invidian invidian force-pushed the invidian/tinkerbell-platform branch 3 times, most recently from a58e381 to 794b2af Compare May 13, 2020 13:58
@invidian
Copy link
Member Author

Update:

  • Go implementation is now added
  • Some feature parity like Flatcar channel and Flatcar version features are added
  • Idea with symlinks for variables.tf files ended up messy and it definitely needs to be reverted
  • Worker/Controller modules turns out to work pretty well

Things which are missing in my opinion:

  • Unit tests are missing
  • Configuration validation rules are missing
  • CI implementation is missing
  • Reference documentation is missing
  • Quick start guide is missing
  • Concept documentation on new Terraform code/modules is missing

@invidian invidian force-pushed the invidian/tinkerbell-platform branch from 794b2af to 4fdf684 Compare May 13, 2020 17:25
@invidian
Copy link
Member Author

invidian commented May 13, 2020 •
edited

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.

@invidian invidian force-pushed the invidian/tinkerbell-platform branch from 4fdf684 to 7a66404 Compare May 14, 2020 09:28
@invidian
Copy link
Member Author

Additionally, the code won't work with latest master of Tinkerbell, as they introduced breaking changes in tinkerbell/tink#88.

This is now resolved.

@invidian invidian marked this pull request as ready for review May 14, 2020 14:34
@invidian invidian force-pushed the invidian/tinkerbell-platform branch 3 times, most recently from f2f0ab7 to f7065c4 Compare May 15, 2020 09:02
@invidian invidian mentioned this pull request May 18, 2020
Merged
@invidian invidian force-pushed the invidian/tinkerbell-platform branch from f7065c4 to 9248f34 Compare May 19, 2020 06:19
@invidian invidian mentioned this pull request May 19, 2020
Open
@invidian invidian mentioned this pull request May 28, 2020
Open
@invidian invidian force-pushed the invidian/tinkerbell-platform branch from 9248f34 to f773ff4 Compare August 3, 2020 10:40
@invidian invidian requested review from iaguis and ipochi August 3, 2020 10:40
@invidian
Copy link
Member Author

invidian commented Aug 3, 2020

Would be cool to get that merged, even if there is no tests for it.

@invidian invidian mentioned this pull request Sep 4, 2020
Open
@invidian
Copy link
Member Author

invidian commented Oct 6, 2020

Rebased with #824 included, though it's not fully functional yet.

@invidian invidian force-pushed the invidian/tinkerbell-platform branch 2 times, most recently from 09b8851 to ac8c967 Compare October 6, 2020 18:48
@invidian
Copy link
Member Author

invidian commented Oct 8, 2020

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.

knrt10
knrt10 previously requested changes Nov 26, 2020
View reviewed changes
Copy link
Member

@knrt10 knrt10 left a 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/controller/variables.tf Outdated Show resolved Hide resolved
assets/terraform-modules/controller/versions.tf Outdated Show resolved Hide resolved
assets/terraform-modules/node/variables.tf Outdated Show resolved Hide resolved
assets/terraform-modules/node/versions.tf Outdated Show resolved Hide resolved
assets/terraform-modules/worker/variables.tf Outdated Show resolved Hide resolved
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
assets/terraform-modules/tinkerbell-sandbox/worker/versions.tf Outdated Show resolved Hide resolved
@knrt10
Copy link
Member

knrt10 commented Nov 27, 2020

I like how we cheery picked commit from ccm PR to another PR and then again to this PR. 😄

@invidian
Copy link
Member Author

I like how we cheery picked commit from ccm PR to another PR and then again to this PR. smile

@invidian invidian requested a review from knrt10 November 27, 2020 09:16
@invidian
Copy link
Member Author

Awesome work @invidian partying_face . Love how much I got to learn from it. I have suggested some minor changes, please have a look

Addressed. Please have a look again.

iaguis
iaguis reviewed Dec 1, 2020
View reviewed changes
Copy link
Contributor

@iaguis iaguis left a 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.

@iaguis iaguis removed this from the v0.6.0 milestone Dec 1, 2020
@invidian
Copy link
Member Author

invidian commented Dec 2, 2020

Rebased.

@invidian invidian requested a review from iaguis December 2, 2020 06:44
@invidian
pkg/platform: add WorkerPoolNamesUnique method and needed interface
0c6e0fa 
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 <mateusz@kinvolk.io>
@invidian
terraform: add reusable worker and controller modules
c4b4162 
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 <mateusz@kinvolk.io>
@invidian
bootkube: take bootstrap tokens as structure object
2cac6e9 
Instead of "any".

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@invidian
bootkube: simplify passing bootstrap tokens to template
08af393 
Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@invidian invidian force-pushed the invidian/tinkerbell-platform branch 2 times, most recently from 674fd26 to 95685f5 Compare December 2, 2020 07:49
@invidian
Add Tinkerbell platform
84821fd 
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 <mateusz@kinvolk.io>
@invidian
controllers: wait for etcd certificates before starting service
f7dd2ed 
To avoid weird race conditions which we saw in Tinkerbell pipeline.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@invidian
tinkerbell-sandbox: reduce provisioner resources
1e6a631 
To make it more approachable on smaller machines.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
@invidian
Ignore tinkerbell sandbox setup.sh from codespell
7681aba 
As word 'ether' triggers it.

Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>
iaguis
iaguis approved these changes Dec 2, 2020
View reviewed changes
Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@invidian invidian dismissed stale reviews from knrt10 and surajssd December 2, 2020 10:28

All requested changes has been applied.

@invidian invidian merged commit 20b3ea8 into master Dec 2, 2020
@invidian invidian deleted the invidian/tinkerbell-platform branch December 2, 2020 10:29
@invidian invidian mentioned this pull request Dec 7, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@karlskewes karlskewes karlskewes left review comments

@iaguis iaguis iaguis approved these changes

@johananl johananl Awaiting requested review from johananl

@ipochi ipochi Awaiting requested review from ipochi

@surajssd surajssd Awaiting requested review from surajssd

@knrt10 knrt10 Awaiting requested review from knrt10

Assignees
No one assigned
Labels
priority/P3 Low priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Tinkerbell as a platform
5 participants
@invidian @knrt10 @iaguis @surajssd @karlskewes