Skip to content

Commit

Permalink
Merge pull request #2083 from tonistiigi/v0.8-picks-20210421
Browse files Browse the repository at this point in the history
  • Loading branch information
AkihiroSuda authored Apr 22, 2021
2 parents 1428883 + 3f105f9 commit 81c2cbd
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 17 deletions.
20 changes: 11 additions & 9 deletions cache/contenthash/checksum.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,17 +406,19 @@ func (cc *cacheContext) ChecksumWildcard(ctx context.Context, mountable cache.Mo
return digest.FromBytes([]byte{}), nil
}

if len(wildcards) > 1 {
digester := digest.Canonical.Digester()
for i, w := range wildcards {
if i != 0 {
digester.Hash().Write([]byte{0})
}
digester.Hash().Write([]byte(w.Record.Digest))
if len(wildcards) == 1 && path.Base(p) == path.Base(wildcards[0].Path) {
return wildcards[0].Record.Digest, nil
}

digester := digest.Canonical.Digester()
for i, w := range wildcards {
if i != 0 {
digester.Hash().Write([]byte{0})
}
return digester.Digest(), nil
digester.Hash().Write([]byte(path.Base(w.Path)))
digester.Hash().Write([]byte(w.Record.Digest))
}
return wildcards[0].Record.Digest, nil
return digester.Digest(), nil
}

func (cc *cacheContext) Checksum(ctx context.Context, mountable cache.Mountable, p string, followLinks bool, s session.Group) (digest.Digest, error) {
Expand Down
10 changes: 6 additions & 4 deletions cache/contenthash/checksum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,25 +179,27 @@ func TestChecksumWildcard(t *testing.T) {

dgst, err := cc.ChecksumWildcard(context.TODO(), ref, "f*o", false, nil)
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgst)
require.Equal(t, digest.FromBytes(append([]byte("foo"), []byte(dgstFileData0)...)), dgst)

expFoos := digest.Digest("sha256:c9f914ad7ad8fe6092ce67484b43ca39c2087aabf9e4a1b223249b0f8b09b9f2")
expFoos := digest.Digest("sha256:7f51c821895cfc116d3f64231dfb438e87a237ecbbe027cd96b7ee5e763cc569")

dgst, err = cc.ChecksumWildcard(context.TODO(), ref, "f*", false, nil)
require.NoError(t, err)
require.Equal(t, expFoos, dgst)

dgst, err = cc.ChecksumWildcard(context.TODO(), ref, "x/d?", false, nil)
require.NoError(t, err)
require.Equal(t, dgstDirD0, dgst)
require.Equal(t, digest.FromBytes(append([]byte("d0"), []byte(dgstDirD0)...)), dgst)

dgst, err = cc.ChecksumWildcard(context.TODO(), ref, "x/d?/def", true, nil)
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgst)

expFoos2 := digest.Digest("sha256:8afc09c7018d65d5eb318a9ef55cb704dec1f06d288181d913fc27a571aa042d")

dgst, err = cc.ChecksumWildcard(context.TODO(), ref, "y*", true, nil)
require.NoError(t, err)
require.Equal(t, expFoos, dgst)
require.Equal(t, expFoos2, dgst)

err = ref.Release(context.TODO())
require.NoError(t, err)
Expand Down
28 changes: 28 additions & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ func TestIntegration(t *testing.T) {
testSharedCacheMounts,
testLockedCacheMounts,
testDuplicateCacheMount,
testRunCacheWithMounts,
testParallelLocalBuilds,
testSecretMounts,
testExtraHosts,
Expand Down Expand Up @@ -2759,6 +2760,33 @@ func testDuplicateCacheMount(t *testing.T, sb integration.Sandbox) {
require.NoError(t, err)
}

func testRunCacheWithMounts(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)
c, err := New(context.TODO(), sb.Address())
require.NoError(t, err)
defer c.Close()

busybox := llb.Image("busybox:latest")

out := busybox.Run(llb.Shlex(`sh -e -c "[[ -f /m1/sbin/apk ]]"`))
out.AddMount("/m1", llb.Image("alpine:latest"), llb.Readonly)

def, err := out.Marshal(context.TODO())
require.NoError(t, err)

_, err = c.Solve(context.TODO(), def, SolveOpt{}, nil)
require.NoError(t, err)

out = busybox.Run(llb.Shlex(`sh -e -c "[[ -f /m1/sbin/apk ]]"`))
out.AddMount("/m1", llb.Image("busybox:latest"), llb.Readonly)

def, err = out.Marshal(context.TODO())
require.NoError(t, err)

_, err = c.Solve(context.TODO(), def, SolveOpt{}, nil)
require.Error(t, err)
}

func testCacheMountNoCache(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)
c, err := New(context.TODO(), sb.Address())
Expand Down
1 change: 1 addition & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ coverage:
default:
target: auto # auto % coverage target
threshold: 1% # allow for 1% reduction of coverage without failing
patch: off
47 changes: 47 additions & 0 deletions frontend/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ var allTests = []integration.Test{
testDockefileCheckHostname,
testDefaultShellAndPath,
testDockerfileLowercase,
testWildcardRenameCache,
}

var fileOpTests = []integration.Test{
Expand Down Expand Up @@ -3752,6 +3753,52 @@ LABEL foo=bar
require.Equal(t, "baz", v)
}

// #2008
func testWildcardRenameCache(t *testing.T, sb integration.Sandbox) {
skipDockerd(t, sb)
f := getFrontend(t, sb)

dockerfile := []byte(`
FROM alpine
COPY file* /files/
RUN ls /files/file1
`)
dir, err := tmpdir(
fstest.CreateFile("Dockerfile", dockerfile, 0600),
fstest.CreateFile("file1", []byte("foo"), 0600),
)
require.NoError(t, err)
defer os.RemoveAll(dir)

c, err := client.New(context.TODO(), sb.Address())
require.NoError(t, err)
defer c.Close()

destDir, err := ioutil.TempDir("", "buildkit")
require.NoError(t, err)
defer os.RemoveAll(destDir)

_, err = f.Solve(context.TODO(), c, client.SolveOpt{
LocalDirs: map[string]string{
builder.DefaultLocalNameDockerfile: dir,
builder.DefaultLocalNameContext: dir,
},
}, nil)
require.NoError(t, err)

err = os.Rename(filepath.Join(dir, "file1"), filepath.Join(dir, "file2"))
require.NoError(t, err)

// cache should be invalidated and build should fail
_, err = f.Solve(context.TODO(), c, client.SolveOpt{
LocalDirs: map[string]string{
builder.DefaultLocalNameDockerfile: dir,
builder.DefaultLocalNameContext: dir,
},
}, nil)
require.Error(t, err)
}

func testOnBuildCleared(t *testing.T, sb integration.Sandbox) {
f := getFrontend(t, sb)

Expand Down
20 changes: 18 additions & 2 deletions solver/llbsolver/ops/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ func cloneExecOp(old *pb.ExecOp) pb.ExecOp {
}
n.Meta = &meta
n.Mounts = nil
for i := range n.Mounts {
m := *n.Mounts[i]
for i := range old.Mounts {
m := *old.Mounts[i]
n.Mounts = append(n.Mounts, &m)
}
return n
Expand All @@ -97,6 +97,22 @@ func (e *execOp) CacheMap(ctx context.Context, g session.Group, index int) (*sol
}
}

// Special case for cache compatibility with buggy versions that wrongly
// excluded Exec.Mounts: for the default case of one root mount (i.e. RUN
// inside a Dockerfile), do not include the mount when generating the cache
// map.
if len(op.Mounts) == 1 &&
op.Mounts[0].Dest == "/" &&
op.Mounts[0].Selector == "" &&
!op.Mounts[0].Readonly &&
op.Mounts[0].MountType == pb.MountType_BIND &&
op.Mounts[0].CacheOpt == nil &&
op.Mounts[0].SSHOpt == nil &&
op.Mounts[0].SecretOpt == nil &&
op.Mounts[0].ResultID == "" {
op.Mounts = nil
}

dt, err := json.Marshal(struct {
Type string
Exec *pb.ExecOp
Expand Down
2 changes: 1 addition & 1 deletion util/contentutil/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func CopyChain(ctx context.Context, ingester content.Ingester, provider content.
handlers := []images.Handler{
images.ChildrenHandler(provider),
filterHandler,
retryhandler.New(remotes.FetchHandler(ingester, &localFetcher{provider}), nil),
retryhandler.New(remotes.FetchHandler(ingester, &localFetcher{provider}), func(_ []byte) {}),
}

if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion util/imageutil/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func Config(ctx context.Context, str string, resolver remotes.Resolver, cache Co
children := childrenConfigHandler(cache, platform)

handlers := []images.Handler{
retryhandler.New(remotes.FetchHandler(cache, fetcher), nil),
retryhandler.New(remotes.FetchHandler(cache, fetcher), func(_ []byte) {}),
children,
}
if err := images.Dispatch(ctx, images.Handlers(handlers...), nil, desc); err != nil {
Expand Down
1 change: 1 addition & 0 deletions util/progress/logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ func (sw *streamWriter) Close() error {
func LoggerFromContext(ctx context.Context) func([]byte) {
return func(dt []byte) {
pw, _, _ := progress.FromContext(ctx)
defer pw.Close()
pw.Write(identity.NewID(), client.VertexLog{
Stream: stderr,
Data: []byte(dt),
Expand Down
9 changes: 9 additions & 0 deletions util/resolver/retryhandler/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/containerd/containerd/images"
remoteserrors "github.com/containerd/containerd/remotes/errors"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -47,6 +48,14 @@ func New(f images.HandlerFunc, logger func([]byte)) images.HandlerFunc {
}

func retryError(err error) bool {
// Retry on 5xx errors
var errUnexpectedStatus remoteserrors.ErrUnexpectedStatus
if errors.As(err, &errUnexpectedStatus) &&
errUnexpectedStatus.StatusCode >= 500 &&
errUnexpectedStatus.StatusCode <= 599 {
return true
}

if errors.Is(err, io.EOF) || errors.Is(err, syscall.ECONNRESET) || errors.Is(err, syscall.EPIPE) {
return true
}
Expand Down

0 comments on commit 81c2cbd

Please sign in to comment.