Add the --target flag as a parameter to the docker builder. #894
Conversation
Codecov Report
@@ Coverage Diff @@
## master #894 +/- ##
==========================================
+ Coverage 39.38% 39.52% +0.14%
==========================================
Files 61 61
Lines 2613 2616 +3
==========================================
+ Hits 1029 1034 +5
+ Misses 1471 1469 -2
Partials 113 113
Continue to review full report at Codecov.
|
dlorenc
force-pushed
the
dockertarget
branch
2 times, most recently
from
32de034 to
7d0f283
Compare
|
@dlorenc there’s much more required to support —target. Local builder has two modes, support needs to be added to gcb and kaniko too, and the computation of the build context should also be updated. |
|
Yeah sorry, this needs a WIP tag. I got sidetracked checking to see if kaniko supports the target flag yet. Can you explain the build context part a little more though? Is that an optimization because some of the build context might not be needed based on the target? |
|
@dgageot can you explain what you mean by the local builder having two modes? Are you referring to the docker API vs. docker binary? |
|
ping @dgageot I think I actually did this correctly by accident. There will be a small change in behavior, but I'm not sure what's correct. Previously if someone specified --cache-from with Kaniko, it would be ignored. Now it'll be an error. |
pkg/skaffold/docker/image_test.go
Outdated
| }{ | ||
| { | ||
| name: "build args", | ||
| args: args{ |
There was a problem hiding this comment.
WDYT of inlining args? Since it contains a single arguments, I think it makes sense.
Also, most tests use description instead of name to describe the test.
There was a problem hiding this comment.
Done, this is the pattern the vs-code test generator thing uses by default, but I agree it's overkill
|
@dlorenc you are right that you had it right the first time. I read your PR too quickly. Sorry |
It was definitely by accident :) I didn't realize the |
|
@dlorenc linter is failing |
Fixes #635