Docker: pass Options.platform to inspect and default local platform to linux/$GOARCH#302
Conversation
aac0f96 to
8458ab0
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #302 +/- ##
==========================================
+ Coverage 33.79% 34.06% +0.28%
==========================================
Files 39 39
Lines 3283 3301 +18
==========================================
+ Hits 1109 1124 +15
- Misses 1980 1982 +2
- Partials 194 195 +1 🚀 New features to boost your workflow:
|
jabrown85
left a comment
There was a problem hiding this comment.
This looks like a good idea to me! Left a couple comments to address
| if err != nil { | ||
| return imgutil.Platform{}, err | ||
| } | ||
| if daemonInfo.Os == "linux" { |
There was a problem hiding this comment.
We have some tests here around not having a platform defined. I was surprised there weren't any correlated test changes. Should there be a test to capture this new behavior?
There was a problem hiding this comment.
If I'm reading #283 correctly, in the test environment, both the Docker daemon and the imgutil tests run on the same architecture, amd64, hence this ends up causing no change.
A new test would be nice, but I'm not very sure how to introduce it.
8458ab0 to
d30fa9b
Compare
Signed-off-by: Bozhidar Marinov <bozhidar.marinov1@mail.schwarz>
d30fa9b to
e1e1453
Compare
|
Comments should be handled now; unsure how to add a new test for mismatched architecture between |
Fixes buildpacks/lifecycle#1599 in my testing.
Possible issues:
imgutils.localto match the current binary's architecture is a partial revert of Use daemon or runtime provided arch instead of amd64 #232. I'd rather change it to default tolinux/$GOARCHalways (and not just whendaemonInfo.Os == "linux"), for parity withremote, but I don't know if that will break other packages depending onimgutils. 😅If the base image's architecture does not match the architecture of the wanted platform, the old code used to return the base image anyway. With this change, it would instead error out.Fixed!