Add Kubernetes Controller #563

micahhausler · 2021-12-02T17:24:08Z

Signed-off-by: Micah Hausler [email protected]

Description

This PR adds a new Kubernetes controller binary for the types merged in #542.

Adds a new cmd/tink-controller package with a Dockerfile and a main.go
Removes the status.data field on Workflows. This was just a string of the interpolated template, and was redundant given the strongly typed template actions already on the .status of Workflows.
Adds tests to the methods on the workflow API structure
Adds a new pkg/controllers package. This introduces a Manager abstraction that wraps sigs.k8s.io/controller-runtime's Reconciler type
- Controllers for individual types (workflows, templates, hardware) will reside in individual sub-packages
- Controllers patch incoming objects even in the event of an error: this is intentional as a controller in the Kubernetes Resource Model attempts to converge on desired state
Adds a new pkg/conversion package for converting protobuf types to Kubernetes CRD types and vice versa
Adds an internal pkg/internal/tests package with internal testing utilities.
- The first utility added is a FrozenTime interface with Before/After connivence methods for time.Time, as well as protobuf and K8s metav1 package time formats.

How Has This Been Tested?

I added unit tests for nearly all new branching logic
I added a CI target for building the tink-controller

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

There is no impact to existing users. Nothing form this change is called in any existing code path.

Checklist:

I have:

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

codecov · 2021-12-02T17:34:08Z

Codecov Report

Merging #563 (c2694a4) into main (bb45990) will increase coverage by 2.54%.
The diff coverage is 93.77%.

❗ Current head c2694a4 differs from pull request most recent head 38af099. Consider uploading reports for the commit 38af099 to get more accurate results

@@            Coverage Diff             @@
##             main     #563      +/-   ##
==========================================
+ Coverage   36.01%   38.56%   +2.54%     
==========================================
  Files          44       54      +10     
  Lines        3207     3563     +356     
==========================================
+ Hits         1155     1374     +219     
- Misses       1959     2092     +133     
- Partials       93       97       +4

Impacted Files	Coverage Δ
pkg/apis/core/v1alpha1/workflow_types.go	`100.00% <ø> (ø)`
pkg/controllers/workflow/controller.go	`76.78% <76.78%> (ø)`
pkg/apis/core/v1alpha1/hardware_methods.go	`100.00% <100.00%> (ø)`
pkg/apis/core/v1alpha1/template_methods.go	`100.00% <100.00%> (ø)`
pkg/apis/core/v1alpha1/workflow_methods.go	`100.00% <100.00%> (ø)`
pkg/convert/template.go	`100.00% <100.00%> (ø)`
pkg/convert/workflow.go	`100.00% <100.00%> (ø)`
workflow/template_validator.go	`95.29% <100.00%> (+10.67%)`	⬆️
http-server/http_handlers.go	`6.64% <0.00%> (-0.16%)`	⬇️
pkg/apis/core/v1alpha1/hardware_types.go	`100.00% <0.00%> (ø)`
... and 5 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 bb45990...38af099. Read the comment docs.

jacobweinstock · 2021-12-02T18:02:15Z

This is really great. I'm very excited about this, thank you!
There is quite a bit of new code going in. Is it too early for docs? Would be really helpful to know about things like Kubernetes client/server version requirements, how to run/deploy the controller, how the controller pattern works in general, how the controller code here is laid out, etc.

mmlb

I only just skimmed the layout so far and the autogenerated file question popped up and I wanted to ask that before diving deeper here.

pkg/apis/core/v1alpha1/zz_generated.deepcopy.go

micahhausler · 2021-12-03T20:07:39Z

This is really great. I'm very excited about this, thank you! There is quite a bit of new code going in. Is it too early for docs? Would be really helpful to know about things like Kubernetes client/server version requirements, how the controller pattern works in general, how the controller code here is laid out, etc.

Thank you! I did want to hold on docs initially, since there is nothing to write about other than "there is a controller you can run that nothing else interacts with yet."

I'll add docs in the next PR, that includes changes to the Tink API.

micahhausler · 2021-12-03T20:48:10Z

I call BS on CodeCov saying we're 2% lower in coverage. I know there are specific tests and cases I added that cover files CC is showing as uncovered.

cmd/tink-controller/Dockerfile

pkg/apis/core/v1alpha1/hardware_methods.go

mmlb · 2021-12-03T20:58:20Z

I call BS on CodeCov saying we're 2% lower in coverage. I know there are specific tests and cases I added that cover files CC is showing as uncovered.

codecov is hard to understand some (most?) times. Don't sweat it too much imo.

Signed-off-by: Micah Hausler <[email protected]> ## Description Extracted repo config changes from #563 ## Why is this needed * Go version is out of date in `ci.yaml` * `.mk` files were missing from editor config * Coverage was counted against for generated files Fixes: # N/A ## How Has This Been Tested? Ran verify/tests locally ## How are existing users impacted? What migration steps/scripts do we need? N/A ## Checklist: I have: - [x] updated the documentation and/or roadmap (if required) - [x] added unit or e2e tests - [x] provided instructions on how to upgrade

mmlb

here's what I've seen so far. Next up is reviewing the tests. About the import blocks, you can automatically fix everthing with:

sed -i '/^import (/,/^)/ { /^\s*$/ d }' $(git ls-files '*.go') && gofumpt -w -s .

I've been meaning to add this to ci-checks.sh but I thought gofumpt did that itself :(

pkg/controllers/workflow/controller.go

pkg/controllers/manager.go

pkg/controllers/workflow/controller_test.go

pkg/conversion/template.go

pkg/conversion/workflow.go

pkg/internal/tests/frozen_time.go

workflow/template_validator.go

micahhausler · 2021-12-21T21:43:11Z

@mmlb I removed go from the Nix install, it was using a different version than what GH Actions uses and caused CI failures

pkg/convert/workflow.go

micahhausler · 2022-01-07T20:09:34Z

@mmlb anything else you'd like to see here?

mmlb · 2022-01-07T20:24:44Z

@mmlb anything else you'd like to see here?

aahhg, sorry for dropping the ball on this. I think I was pretty happy with everything except for dropping go from shell.nix. I'll look again on Tuesday though if thats ok? If its just the go in shell.nix thing then we can merge and I'll sync up with you on why you needed to drop it so we can add it back in. Is that alright?

micahhausler · 2022-01-10T16:56:50Z

I think I was pretty happy with everything except for dropping go from shell.nix.

This is due to newer point releases of Go in the GH actions than nix has available. We set the Go version in GH actions to 1.17 in #565 which auto updated for two December point releases (including CVEs in 1.17.5) and one in Jan, but the latest Go 1.17 Nix commit is 2 months old pinning to 1.17.3 (32ef30c). This resulted in CI build failures when some tooling was built with 1.17.5 and Nix was 1.17.3.

I'll look again on Tuesday though if thats ok?

SGTM!

If its just the go in shell.nix thing then we can merge and I'll sync up with you on why you needed to drop it so we can add it back in. Is that alright?

SGTM!

micahhausler · 2022-01-14T15:57:34Z

@displague @mmlb I rebased on upstream main so it dismissed previous reviews

mmlb · 2022-01-18T20:27:23Z

pkg/convert/workflow.go

+				WorkerId: task.WorkerAddr,
+				Volumes:  append(task.Volumes, action.Volumes...),
+				// TODO: (micahhausler) Dedupe task volume targets overridden in the action volumes?
+				//   Also not sure how Docker handles nested mounts (ex: "/foo:/foo" and "/bar:/foo/bar")


fyi:

[manny@zennix:~/t/debug]$ tree . ├── bar │ └── from-bar └── foo └── from-foo 2 directories, 2 files [manny@zennix:~/t/debug]$ docker run --rm -ti -v $PWD/foo:/foo -v $PWD/bar:/foo/bar alpine / # echo ==foo==; ls -l /foo/; echo ==bar==; ls -l /foo/bar; ==foo== total 33 drwxr-xr-x 2 1000 users 3 Jan 18 20:23 bar -rw-r--r-- 1 1000 users 0 Jan 18 20:23 from-foo ==bar== total 1 -rw-r--r-- 1 1000 users 0 Jan 18 20:23 from-bar / # [manny@zennix:~/t/debug]$ tree . ├── bar │ └── from-bar └── foo ├── bar └── from-foo 3 directories, 2 files

note how foo/bar directory now exists

mmlb · 2022-01-18T20:47:22Z

Looks like mergify is getting stuck on docker-images, possibly due to the check being set up as a matrix job. I've opened up a chat with them to see what the fix/work-around is. I'd like to hold this here until tomorrow at most.

* Add new cmd/tink-controller package * Remove the `status.data` field on Workflows * Add tests for methods on API types * Add `pkg/controllers` package with controller logic * Controllers for individual types (workflows, templates, hardware) will reside in individual sub-packages * Add `pkg/convert` for type conversions * Add `pkg/internal/tests` for testing utilities * Added FrozenTime with Kubernetes and Protobuf methods Signed-off-by: Micah Hausler <[email protected]>

micahhausler · 2022-01-19T17:09:34Z

🎉

## Description This PR adds a Kubernetes API backend to Tinkerell as a replacement for Postgres. This PR builds on #563 and adds new API capabilities ## Why is this needed ## How Has This Been Tested? ## How are existing users impacted? What migration steps/scripts do we need? ## Checklist: I have: - [ ] updated the documentation and/or roadmap (if required) - [ ] added unit or e2e tests - [ ] provided instructions on how to upgrade

micahhausler requested review from jacobweinstock and mmlb December 2, 2021 17:26

micahhausler force-pushed the krm/controllers branch 3 times, most recently from 3414844 to a247c66 Compare December 2, 2021 17:58

mmlb reviewed Dec 2, 2021

View reviewed changes

pkg/apis/core/v1alpha1/zz_generated.deepcopy.go Outdated Show resolved Hide resolved

micahhausler force-pushed the krm/controllers branch from a247c66 to b29b9ea Compare December 3, 2021 20:05

micahhausler force-pushed the krm/controllers branch 5 times, most recently from 39cc132 to 56d3436 Compare December 3, 2021 20:42

micahhausler force-pushed the krm/controllers branch from 463d8ab to d05b1d0 Compare December 3, 2021 20:56

mmlb reviewed Dec 3, 2021

View reviewed changes

cmd/tink-controller/Dockerfile Outdated Show resolved Hide resolved

pkg/apis/core/v1alpha1/hardware_methods.go Outdated Show resolved Hide resolved

micahhausler force-pushed the krm/controllers branch 3 times, most recently from 82e0761 to f9e14c5 Compare December 3, 2021 21:41

micahhausler mentioned this pull request Dec 3, 2021

Updated repository config files #565

Merged

3 tasks

micahhausler force-pushed the krm/controllers branch from f9e14c5 to ab6c6d5 Compare December 3, 2021 22:11

mmlb reviewed Dec 3, 2021

View reviewed changes

micahhausler force-pushed the krm/controllers branch from ab6c6d5 to 87164b5 Compare December 6, 2021 15:51

mmlb changed the title ~~Added Kubernetes Controller~~ Add Kubernetes Controller Dec 6, 2021

micahhausler force-pushed the krm/controllers branch 2 times, most recently from 8b1a799 to e905947 Compare December 9, 2021 20:37

micahhausler force-pushed the krm/controllers branch 2 times, most recently from 2b9de9d to cc44110 Compare December 19, 2021 21:11

micahhausler mentioned this pull request Dec 21, 2021

Make CI and local test workflows the same #572

Closed

micahhausler force-pushed the krm/controllers branch from 6dac4a5 to 73ca7c7 Compare December 21, 2021 21:26

micahhausler requested a review from mmlb December 22, 2021 19:10

micahhausler mentioned this pull request Dec 22, 2021

Krm/tink server #573

Merged

3 tasks

displague reviewed Jan 3, 2022

View reviewed changes

pkg/convert/workflow.go Outdated Show resolved Hide resolved

micahhausler force-pushed the krm/controllers branch from 73ca7c7 to 2464bee Compare January 4, 2022 15:31

displague previously approved these changes Jan 4, 2022

View reviewed changes

micahhausler dismissed displague’s stale review via fa8e816 January 14, 2022 15:57

micahhausler force-pushed the krm/controllers branch from 2464bee to fa8e816 Compare January 14, 2022 15:57

mmlb reviewed Jan 18, 2022

View reviewed changes

mmlb previously approved these changes Jan 18, 2022

View reviewed changes

mmlb added the ready-to-merge Signal to Mergify to merge the PR. label Jan 18, 2022

mmlb removed the request for review from jacobweinstock January 18, 2022 20:35

mmlb dismissed their stale review via a5ad5b7 January 19, 2022 16:45

mmlb force-pushed the krm/controllers branch from fa8e816 to a5ad5b7 Compare January 19, 2022 16:45

mmlb force-pushed the krm/controllers branch from a5ad5b7 to 38af099 Compare January 19, 2022 16:46

mmlb approved these changes Jan 19, 2022

View reviewed changes

mergify bot merged commit 41bf341 into tinkerbell:main Jan 19, 2022

displague added this to the 0.7.0 milestone Aug 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Kubernetes Controller #563

Add Kubernetes Controller #563

micahhausler commented Dec 2, 2021

codecov bot commented Dec 2, 2021 •

edited

Loading

jacobweinstock commented Dec 2, 2021 •

edited

Loading

mmlb left a comment •

edited

Loading

micahhausler commented Dec 3, 2021

micahhausler commented Dec 3, 2021

mmlb commented Dec 3, 2021

mmlb left a comment •

edited

Loading

micahhausler commented Dec 21, 2021

micahhausler commented Jan 7, 2022

mmlb commented Jan 7, 2022

micahhausler commented Jan 10, 2022

micahhausler commented Jan 14, 2022

mmlb Jan 18, 2022

mmlb commented Jan 18, 2022

micahhausler commented Jan 19, 2022

Add Kubernetes Controller #563

Add Kubernetes Controller #563

Conversation

micahhausler commented Dec 2, 2021

Description

How Has This Been Tested?

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

Checklist:

codecov bot commented Dec 2, 2021 • edited Loading

Codecov Report

jacobweinstock commented Dec 2, 2021 • edited Loading

mmlb left a comment • edited Loading

Choose a reason for hiding this comment

micahhausler commented Dec 3, 2021

micahhausler commented Dec 3, 2021

mmlb commented Dec 3, 2021

mmlb left a comment • edited Loading

Choose a reason for hiding this comment

micahhausler commented Dec 21, 2021

micahhausler commented Jan 7, 2022

mmlb commented Jan 7, 2022

micahhausler commented Jan 10, 2022

micahhausler commented Jan 14, 2022

mmlb Jan 18, 2022

Choose a reason for hiding this comment

mmlb commented Jan 18, 2022

micahhausler commented Jan 19, 2022

codecov bot commented Dec 2, 2021 •

edited

Loading

jacobweinstock commented Dec 2, 2021 •

edited

Loading

mmlb left a comment •

edited

Loading

mmlb left a comment •

edited

Loading