skaffold init --force supports cases with 1 image and multiple builders #4973
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4973 +/- ##
==========================================
- Coverage 72.27% 72.25% -0.02%
==========================================
Files 361 361
Lines 12603 12660 +57
==========================================
+ Hits 9109 9148 +39
- Misses 2821 2833 +12
- Partials 673 679 +6
Continue to review full report at Codecov.
|
|
Can we update the init docs with the order of preference? |
| } | ||
|
|
||
| d.builderImagePairs = append(d.builderImagePairs, BuilderImagePair{Builder: choice, ImageName: image}) | ||
| d.unresolvedImages = util.RemoveFromSlice(d.unresolvedImages, image) |
There was a problem hiding this comment.
should unresolvedImages be a map instead?
There was a problem hiding this comment.
perhaps! Could you explain your thoughts on why it should be?
There was a problem hiding this comment.
sorry I missed this comment. A map would have been more efficient for this RemoveFromSlice behavior.
@tejal29 how would you suggest documenting this without being too verbose? If we say something like "In the case of multiple builders being matched to a single image, the priority is Docker > Jib > Bazel > Buildpacks" I'm worried it might make things really long |
Ah not the output. I meant the Init docs https://skaffold.dev/docs/pipeline-stages/init/. |
| } | ||
|
|
||
| d.builderImagePairs = append(d.builderImagePairs, BuilderImagePair{Builder: choice, ImageName: image}) | ||
| d.unresolvedImages = util.RemoveFromSlice(d.unresolvedImages, image) |
There was a problem hiding this comment.
you could set d.unresolvedImages to empty array instead of calling RemoveFromSlice here.
|
@tejal29 Added a section to the docs, PTAL. I wasn't sure exactly where to place it within the init page |
Looks good! Ship it! |
Fixes #3661
Description
Allows for
skaffold init --forceto work in situations where it detects one image and multiple builders. Previously, the user would see the followingNow, a builder will be chosen with the following priority: Docker > Jib > Bazel > Buildpacks
This ordering was decided upon from input on both #3661 and https://kubernetes.slack.com/archives/CABQMSZA6/p1602786450111700.
Follow-up Work
It would be nice to do something in the case of multiple images and builders. I've opened #4912 to talk about this.