-
Notifications
You must be signed in to change notification settings - Fork 41
Fix ConfigMap glitches and doc registry caching #58
Conversation
This fixes a few corner cases in how we handle ConfigMaps for the buildkitd toml config file. It adds integration coverage for those scenarios. It also adds a brief example showing how to configure a local registry for caching purposes to speed up incremental builds on a multi-node cluster.
c176ed6
to
9438b36
Compare
Codecov Report
@@ Coverage Diff @@
## main #58 +/- ##
==========================================
+ Coverage 50.30% 53.11% +2.81%
==========================================
Files 38 39 +1
Lines 2465 2517 +52
==========================================
+ Hits 1240 1337 +97
+ Misses 1073 1019 -54
- Partials 152 161 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
0b1a1f0
to
f8bc761
Compare
@@ -35,6 +35,11 @@ Assuming you have a valid kube configuration pointed at a cluster, you can run t | |||
make integration | |||
``` | |||
|
|||
If you want to run a single suite of tests while working on a specific area of the tests or main code, use something like this: | |||
``` | |||
make integration EXTRA_GO_TEST_FLAGS="-run TestConfigMapSuite -v" |
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.
Maybe include a list of the current suites and what/when it's useful to run those tests? I'm worried the people wouldn't know what to do here.
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.
I'll add a hint to find them
README.md
Outdated
kubectl buildkit create --config ./examples/local-registry-buildkitd.toml | ||
``` | ||
|
||
You can then build using the registry cache with something like |
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.
maybe:
"... using the registry cache with the command:"
Just to be a little more assertive on how to perform the action.
pkg/driver/kubernetes/driver.go
Outdated
_, err := d.configMapClient.Get(ctx, d.configMap.Name, metav1.GetOptions{}) | ||
|
||
if err != nil { |
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.
It's a little unclear to me how this works. What happens if a serious error is passed back here but userSpecifiedConfig is set?
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.
Good point - it would probably be masked - let me refine this a tad more to validate the error specifically.
f8bc761
to
4993b42
Compare
This fixes a few corner cases in how we handle ConfigMaps for the
buildkitd toml config file. It adds integration coverage for those
scenarios. It also adds a brief example showing how to configure
a local registry for caching purposes to speed up incremental
builds on a multi-node cluster.
Closes #31