Add kubernetes-based backend #250

micahhausler · 2022-04-06T15:52:37Z

Signed-off-by: Micah Hausler mhausler@amazon.com

Description

The new backend uses the Kubernetes CRD types in Tinkerbell core. Boots could use a bit of a refactor/organization in how the backends work, but for now I've tried to fit in to the existing structure.

Why is this needed

This is the boots side of work for tinkerbell/proposals#46

Fixes: #

How Has This Been Tested?

Tested with real bare metal workers using an image built from this branch. I will add E2E tests in a follow up PR, or can add to this branch

How are existing users impacted? What migration steps/scripts do we need?

No impact to existing users

Checklist:

I have:

updated the documentation and/or roadmap (if required)
added unit or e2e tests
provided instructions on how to upgrade

codecov · 2022-04-11T14:59:21Z

Codecov Report

Merging #250 (785b2fa) into main (1138ed3) will decrease coverage by 1%.
The diff coverage is 14%.

❗ Current head 785b2fa differs from pull request most recent head 4d162dd. Consider uploading reports for the commit 4d162dd to get more accurate results

@@         Coverage Diff          @@
##           main   #250    +/-   ##
====================================
- Coverage    39%    37%    -2%     
====================================
  Files        37     41     +4     
  Lines      2424   3007   +583     
====================================
+ Hits        948   1133   +185     
- Misses     1413   1784   +371     
- Partials     63     90    +27

Impacted Files	Coverage Δ
cmd/boots/dhcp.go	`11% <0%> (+<1%)`	⬆️
cmd/boots/http.go	`21% <0%> (+<1%)`	⬆️
packet/client.go	`28% <0%> (ø)`
packet/endpoints.go	`27% <0%> (ø)`
packet/k8s_client.go	`0% <0%> (ø)`
packet/k8s_models.go	`19% <19%> (ø)`
cmd/boots/main.go	`20% <66%> (+4%)`	⬆️
conf/mirror.go	`62% <0%> (-23%)`	⬇️
installers/vmware/main.go	`62% <0%> (-12%)`	⬇️
job/ipxe.go	`0% <0%> (ø)`
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1138ed3...4d162dd. Read the comment docs.

.github/workflows/ci.yaml

packet/k8s_client.go

packet/client.go

packet/endpoints.go

packet/k8s_client.go

jacobweinstock

Looking good. Had a few questions and suggestions.

mmlb · 2022-05-03T21:53:32Z

@micahhausler can we wait until tinkerbell/tink#258 is merged to get this in? I'd be a ok with doing the actual rebase and handling the changes necessary. Having to rebase tinkerbell/tink#258 is quite the pain.

micahhausler · 2022-05-04T14:22:39Z

@micahhausler can we wait until tinkerbell/tink#258 is merged to get this in? I'd be a ok with doing the actual rebase and handling the changes necessary. Having to rebase tinkerbell/tink#258 is quite the pain.

Sure, thats fine with me. I'd like to get this in soon though, maybe by Friday this week?

mmlb · 2022-05-04T16:06:27Z

I'm here to make good on my promise @micahhausler want me to rebase and fixup?

micahhausler · 2022-05-04T16:11:58Z

@mmlb I'm good to do it, thanks though!

jacobweinstock

thanks @micahhausler , just two small requests

cmd/boots/main.go

jacobweinstock · 2022-05-04T17:38:55Z

oh, also would you be able to provide some really basic instructions for use? maybe just how (or that its needed) to generate the crds from the tink repo and apply them to a cluster? or a link maybe to a guide for deploying Tink server/controller?

client/kubernetes/hardware_finder.go

.gitignore

client/kubernetes/k8s_models_test.go

client/tinkerbell/hardware_finder.go

cmd/boots/dhcp.go

mmlb · 2022-05-04T20:58:50Z

cmd/boots/main.go

@@ -220,15 +225,17 @@ func main() {
 	}
 }

-// getWorkflowFinder returns a no-op workflow finder if tinkerbell is not the backend
-func getWorkflowFinder() (client.WorkflowFinder, error) {
+// getWorkflowFinder returns a no-op workflow finder if tinkerbell or kubernetes is not the backend


hmm I think standalone should be allowed too right @jacobweinstock ?

ah yes. seems like standalone should be in there.

How does that apply? Standalone doesn't have a Tinkerbell backend right?

standalone only applies to the hardware fetcher, workflows can still happen with standalone.

With which Tinkerbell data model? Tink on K8s or Tink on postgres?

For backwards compatibility, I'll use Tink on Postgres. We'll have to figure how to transition this later

I wonder if standalone should also imply workflows from json too 🤔

@jacobweinstock I take it you use standalone w/ workflows, thoughts?

i use standalone mainly for testing, this seems fine regardless.

micahhausler · 2022-05-05T18:05:12Z

oh, also would you be able to provide some really basic instructions for use? maybe just how (or that its needed) to generate the crds from the tink repo and apply them to a cluster? or a link maybe to a guide for deploying Tink server/controller?

I'm going to hold of on E2E instructions for now, and do comprehensive documentation for all the components once the Hegel changes get in

jacobweinstock · 2022-05-05T18:13:15Z

Looks like a rebase of main is needed.

chrisdoherty4 · 2022-05-05T19:23:58Z

cmd/boots/main.go

+	var wf client.WorkflowFinder = &client.NoOpWorkflowFinder{}
+	var err error
+
+	switch os.Getenv("DATA_MODEL_VERSION") {


Have we a 'todo' somewhere, or on someones list, to figure out wiring of configuration in Boots generally?

@jacobweinstock @micahhausler

Created tinkerbell/boots#618

chrisdoherty4 · 2022-05-05T19:24:39Z

cmd/boots/main.go

@@ -351,6 +370,8 @@ func newCLI(cfg *config, fs *flag.FlagSet) *ffcli.Command {
 	fs.StringVar(&cfg.dhcpAddr, "dhcp-addr", conf.BOOTPBind, "IP and port to listen on for DHCP.")
 	fs.StringVar(&cfg.syslogAddr, "syslog-addr", conf.SyslogBind, "IP and port to listen on for syslog messages.")
 	fs.StringVar(&cfg.extraKernelArgs, "extra-kernel-args", "", "Extra set of kernel args (k=v k=v) that are appended to the kernel cmdline when booting via iPXE.")
+	fs.StringVar(&cfg.kubeconfig, "kubeconfig", "", "The Kubernetes config file location. Only applies if DATA_MODEL_VERSION=kubernetes.")
+	fs.StringVar(&cfg.kubeAPI, "kubernetes", "", "The Kubernetes API URL, used for in-cluster client construction. Only applies if DATA_MODEL_VERSION=kubernetes.")


I think this is different to Tink and Hegel. I think Tink and Hegel use --kubernetes-api?

Yes, I'll update tink to use --kubernetes to match the upstream argument names

Gotcha. I'll do the same on Hegel.

Updated tinkerbell/tink#619 with reasons

chrisdoherty4 · 2022-05-05T19:25:35Z

lgtm

mmlb · 2022-05-05T20:03:37Z

client/kubernetes/cluster.go

+func init() {
+	_ = clientgoscheme.AddToScheme(runtimescheme)
+	_ = v1alpha1.AddToScheme(runtimescheme)
+}


we're trying to minimize init()s can this be done some other way? sync.Once.Do() in NewCluster maybe?

Signed-off-by: Micah Hausler <mhausler@amazon.com>

mmlb

🎉

micahhausler requested a review from jacobweinstock April 6, 2022 15:52

micahhausler force-pushed the krm/client branch 4 times, most recently from 3b48776 to 9e015ec Compare April 11, 2022 14:55

micahhausler force-pushed the krm/client branch from 9e015ec to 7ac0299 Compare April 14, 2022 18:45

jacobweinstock reviewed Apr 15, 2022

View reviewed changes

micahhausler force-pushed the krm/client branch 3 times, most recently from 867dc93 to 4282254 Compare May 3, 2022 19:35

mmlb mentioned this pull request May 3, 2022

Spring cleaning #258

Merged

3 tasks

micahhausler force-pushed the krm/client branch from 4282254 to b55762c Compare May 4, 2022 16:18

jacobweinstock previously approved these changes May 4, 2022

View reviewed changes

cmd/boots/main.go Outdated Show resolved Hide resolved

mmlb reviewed May 4, 2022

View reviewed changes

micahhausler dismissed jacobweinstock’s stale review via 290e6b8 May 5, 2022 18:06

micahhausler force-pushed the krm/client branch from b55762c to 290e6b8 Compare May 5, 2022 18:06

jacobweinstock previously approved these changes May 5, 2022

View reviewed changes

micahhausler dismissed jacobweinstock’s stale review via 3982acb May 5, 2022 18:14

micahhausler force-pushed the krm/client branch 3 times, most recently from d490b4a to 8002dce Compare May 5, 2022 18:29

jacobweinstock previously approved these changes May 5, 2022

View reviewed changes

chrisdoherty4 reviewed May 5, 2022

View reviewed changes

chrisdoherty4 previously approved these changes May 5, 2022

View reviewed changes

mmlb reviewed May 5, 2022

View reviewed changes

Add kubernetes-based backend

4d162dd

Signed-off-by: Micah Hausler <mhausler@amazon.com>

micahhausler dismissed stale reviews from chrisdoherty4 and jacobweinstock via 4d162dd May 5, 2022 20:14

micahhausler force-pushed the krm/client branch from 8002dce to 4d162dd Compare May 5, 2022 20:14

jacobweinstock approved these changes May 5, 2022

View reviewed changes

mmlb suggested changes May 5, 2022

View reviewed changes

mmlb approved these changes May 5, 2022

View reviewed changes

mmlb added the ready-to-merge Signal to Mergify to merge the PR. label May 5, 2022

mergify bot merged commit 3f3b97c into tinkerbell:main May 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add kubernetes-based backend #250

Add kubernetes-based backend #250

micahhausler commented Apr 6, 2022 •

edited

Loading

codecov bot commented Apr 11, 2022 •

edited

Loading

jacobweinstock left a comment

mmlb commented May 3, 2022

micahhausler commented May 4, 2022

mmlb commented May 4, 2022

micahhausler commented May 4, 2022

jacobweinstock left a comment •

edited

Loading

jacobweinstock commented May 4, 2022 •

edited

Loading

mmlb May 4, 2022

jacobweinstock May 4, 2022

micahhausler May 5, 2022

mmlb May 5, 2022

micahhausler May 5, 2022

micahhausler May 5, 2022

mmlb May 5, 2022

mmlb May 5, 2022

jacobweinstock May 5, 2022

micahhausler commented May 5, 2022

jacobweinstock commented May 5, 2022

chrisdoherty4 May 5, 2022

micahhausler May 5, 2022 •

edited

Loading

chrisdoherty4 May 5, 2022

micahhausler May 5, 2022

chrisdoherty4 May 5, 2022

micahhausler May 5, 2022

chrisdoherty4 commented May 5, 2022

mmlb May 5, 2022

micahhausler May 5, 2022

mmlb left a comment

Add kubernetes-based backend #250

Add kubernetes-based backend #250

Conversation

micahhausler commented Apr 6, 2022 • edited Loading

Description

Why is this needed

How Has This Been Tested?

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

codecov bot commented Apr 11, 2022 • edited Loading

Codecov Report

jacobweinstock left a comment

Choose a reason for hiding this comment

mmlb commented May 3, 2022

micahhausler commented May 4, 2022

mmlb commented May 4, 2022

micahhausler commented May 4, 2022

jacobweinstock left a comment • edited Loading

Choose a reason for hiding this comment

jacobweinstock commented May 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

micahhausler commented May 5, 2022

jacobweinstock commented May 5, 2022

Choose a reason for hiding this comment

micahhausler May 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chrisdoherty4 commented May 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmlb left a comment

Choose a reason for hiding this comment

micahhausler commented Apr 6, 2022 •

edited

Loading

codecov bot commented Apr 11, 2022 •

edited

Loading

jacobweinstock left a comment •

edited

Loading

jacobweinstock commented May 4, 2022 •

edited

Loading

micahhausler May 5, 2022 •

edited

Loading