add check that snapshotter supports the image platform when unpacking#3927
add check that snapshotter supports the image platform when unpacking#3927mxpv merged 1 commit intocontainerd:masterfrom
Conversation
|
Build succeeded.
|
katiewasnothere
force-pushed
the
snapshotter_check
branch
from
3285e26 to
1b51c72
Compare
|
Build succeeded.
|
katiewasnothere
force-pushed
the
snapshotter_check
branch
from
1b51c72 to
acd7c38
Compare
|
Build succeeded.
|
GOGC=75 golangci-lint run
�[1mclient.go:788�[0m:24: �[31m`platfroms` is a misspelling of `platforms`�[0m (misspell)
log.G(ctx).WithField("platfroms", snPlatforms).Info("snapshotter supported platforms")
�[33m^�[0m
Makefile:124: recipe for target 'check' failed |
|
@crosbymichael - Daemon seems to be panicking with this change. Do we have the panic.log anywhere to inspect? |
|
It could also be a deadlock I guess due to the client timeout on every test. |
katiewasnothere
force-pushed
the
snapshotter_check
branch
from
acd7c38 to
5d8265f
Compare
|
Build succeeded.
|
katiewasnothere
force-pushed
the
snapshotter_check
branch
from
5d8265f to
fb26547
Compare
|
Build succeeded.
|
katiewasnothere
force-pushed
the
snapshotter_check
branch
from
fb26547 to
85969e7
Compare
|
Build succeeded.
|
|
@crosbymichael could someone PTAL, sorry it's been a while 😁 |
|
Looks good to me. I'm not a reviewer though. |
|
Should we make this check optional (or up to client)? I can think of cross platform scenario: for instance if we use linux only dev mapper snapshotter to get a windows image and run it on a VM? |
@mxpv Yeah that seems like a fair thing to do. Do you have any suggestions on how we should expose this option to the client? |
|
FYI #2469 Buildkit uses a wrapper around differ/applier to support the migration between Linux layer tarballs and Windows layer tarballss. Ideally, containerd would do it itself. For buildkit use case, it is certainly important to unpack layers for a different Linux architecture. For cross-building you may just unpack and copy new files without the ability to run them or you can run them with an emulator. |
How do you feel about adding an
yup, another use case to have this as an optional check. |
katiewasnothere
force-pushed
the
snapshotter_check
branch
from
85969e7 to
c5f0d24
Compare
|
Build succeeded.
|
katiewasnothere
force-pushed
the
snapshotter_check
branch
from
c5f0d24 to
202af3c
Compare
|
Build succeeded.
|
|
@mxpv Done :) |
jterry75
left a comment
jterry75
left a comment
There was a problem hiding this comment.
This looks great just one small comment.
…unpacking Signed-off-by: Kathryn Baldauf <[email protected]>
katiewasnothere
force-pushed
the
snapshotter_check
branch
from
202af3c to
f8992f4
Compare
|
Build succeeded.
|
5 participants
This work is to prevent a snapshotter from attempting to unpack an image with a platform the snapshotter doesn't support.
For example, a windows snapshotter attempting to unpack an image with a target linux/amd64 platform. Previously, the snapshotter would attempt to unpack this image and fail with an obscure message. This work instead returns an error earlier on indicating the underlying issue that the snapshotter doesn't support the target image platform.
Additionally updated the target windows image used for testing.
Signed-off-by: Kathryn Baldauf [email protected]