-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
dockerfile: add testing external dockerfile features #687
Conversation
@@ -74,7 +74,7 @@ FROM buildkit-base AS buildkitd.containerd_only | |||
ENV CGO_ENABLED=0 | |||
RUN go build -ldflags "$(cat .tmp/ldflags) -d" -o /usr/bin/buildkitd.containerd_only -tags no_oci_worker ./cmd/buildkitd | |||
|
|||
FROM registry:$REGISTRY_VERSION AS registry | |||
FROM tonistiigi/registry:$REGISTRY_VERSION AS registry |
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.
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.
A newer registry is needed because containerd imports with oci media types(that might be wrong).
9fe9002
to
5994dec
Compare
@@ -1,10 +1,15 @@ | |||
FROM --platform=$BUILDPLATFORM golang:1.11-alpine AS builder | |||
# syntax = tonistiigi/dockerfile:runmount20180925 |
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.
nit: runmount20181002 ?
ocispec "github.com/opencontainers/image-spec/specs-go/v1" | ||
) | ||
|
||
func providerFromBinary(fn string) (_ ocispec.Descriptor, _ content.Provider, _ func(), err error) { |
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.
nit: you don't need underscores?
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.
err
is named here
close() | ||
} | ||
}() | ||
// can't use contentutil.Buffer because ImportIndex has wrong dependency signature |
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.
could you add more context about "wrong dependency signature"
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 takes a content.Store
even though only Ingester/Provider
is required. Buffer
does not implement Store
.
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.
could you add that to the code comment?
hack/test
Outdated
@@ -105,8 +105,8 @@ if [ "$TEST_DOCKERFILE" == 1 ]; then | |||
esac | |||
|
|||
if [ -s $tarout ]; then | |||
cid=$(docker create -v /tmp --rm --privileged -e FRONTEND_GATEWAY_ONLY=local:/frontend.tar -e EXTERNAL_DF_FRONTEND=/dockerfile-frontend $iid go test -tags "$buildtags" ${TESTFLAGS:--v} ./frontend/dockerfile) | |||
docker cp $tarout $cid:/frontend.tar | |||
cid=$(docker create -v /tmp --rm --privileged -e BUILDKIT_WORKER_RANDOM=1 -e FRONTEND_GATEWAY_ONLY=local:/$release.tar -e EXTERNAL_DF_FRONTEND=/dockerfile-frontend $iid go test -tags "$buildtags" ${TESTFLAGS:--v} ./frontend/dockerfile) |
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.
why?
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
5994dec
to
2f8f019
Compare
LGTM, but I'm not sure about introducing non-determinism |
@AkihiroSuda It is unlikely that a gateway causes a problem with a specific worker while non-gateway(that is fully tested with every worker) didn't. But didn't want to just leave some workers completely untested here. We could add a variable that turns this off for local runs. |
depends on #683