search

vmware-archive / buildkit-cli-for-kubectl

BuildKit CLI for kubectl is a tool for building container images with your Kubernetes cluster
Other
499 stars 41 forks source link

Implement fast multi-node via an in-cluster proxy #106

Open dhiltgen opened 2 years ago

dhiltgen commented 2 years ago

Closes #26

This implements a proxy container inside the builder pod that can be used to handle image loading and transfer tasks at the end of a build.

I'm marking this WIP for the moment as it needs more documentation updates and CI work to publish the resulting image, but the main code should be ready for review. I would like to further improve/enhance native multi-arch support, but given we don't support that yet in the prior code, splitting that out as a follow on effort feels like the right approach (and this PR is already pretty massive)

dhiltgen commented 2 years ago

PR CI is now working.

Still needs post-merge CI (could come in a follow up PR possibly)

More docs are definitely a must-have before merging.

codecov-commenter commented 2 years ago

Codecov Report

Merging #106 (a611fb2) into main (c03728d) will increase coverage by 1.89%. The diff coverage is 66.94%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #106      +/-   ##
==========================================
+ Coverage   65.13%   67.02%   +1.89%     
==========================================
  Files          40       49       +9     
  Lines        2825     4552    +1727     
==========================================
+ Hits         1840     3051    +1211     
- Misses        761     1272     +511     
- Partials      224      229       +5
Flag Coverage Δ
integration-tests 47.62% <16.58%> (-14.33%) :arrow_down:
unit-tests 37.10% <59.76%> (+27.54%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/build/output.go 78.82% <0.00%> (-3.53%) :arrow_down:
pkg/cmd/create.go 70.96% <33.33%> (-7.83%) :arrow_down:
pkg/driver/kubernetes/creation.go 67.36% <38.46%> (+2.99%) :arrow_up:
pkg/build/build.go 44.84% <40.56%> (-3.43%) :arrow_down:
pkg/proxy/authproxy.go 45.45% <45.45%> (ø)
pkg/proxy/serverimageloader.go 51.87% <51.87%> (ø)
pkg/proxy/server.go 53.61% <53.61%> (ø)
pkg/proxy/dockerdloader.go 59.09% <59.09%> (ø)
cmd/buildkit-proxy/main.go 60.46% <60.46%> (ø)
pkg/proxy/containerdloader.go 61.11% <61.11%> (ø)
... and 52 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 c03728d...a611fb2. Read the comment docs.

dhiltgen commented 2 years ago

Added more coverage and updated the docs to reflect the new design.

Once we wire up native multi-arch support (to follow) we should be able to get much higher coverage on build.go (there's some ~dormant code in there right now)

dhiltgen commented 2 years ago

I wish the proxy implementation on this PR was simpler. I have some concerns this might turn into a maintenance challenge over time. What might make sense is to refine it a bit further so we have a "slow path fall-back" implemented where when/if the fast-path ever runs into problems or falls out of sync, users can still use the tool with the slow path until we update the proxy to match the new patterns.

Since this is already a patch-bomb, one possible thought is to put this on a feature branch, continue to iterate a bit more, maybe cut an RC build from that branch and let it mature a bit longer. (Open to suggestions)