Skip to content

Commit

Permalink
Merge pull request #1739 from tonistiigi/empty-layer
Browse files Browse the repository at this point in the history
clear file mount stubs and fix empty layer cases
  • Loading branch information
AkihiroSuda authored Oct 20, 2020
2 parents 48991bf + c8b8d6c commit dda009a
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 25 deletions.
5 changes: 5 additions & 0 deletions cache/remotecache/v1/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"sort"

"github.com/moby/buildkit/exporter/containerimage/exptypes"
"github.com/moby/buildkit/solver"
digest "github.com/opencontainers/go-digest"
"github.com/pkg/errors"
Expand Down Expand Up @@ -238,6 +239,10 @@ func marshalRemote(r *solver.Remote, state *marshalState) string {
}
desc := r.Descriptors[len(r.Descriptors)-1]

if desc.Digest == exptypes.EmptyGZLayer {
return parentID
}

state.descriptors[desc.Digest] = DescriptorProviderPair{
Descriptor: desc,
Provider: r.Provider,
Expand Down
25 changes: 14 additions & 11 deletions executor/containerdexecutor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/containerd/containerd"
"github.com/containerd/containerd/cio"
"github.com/containerd/containerd/mount"
containerdoci "github.com/containerd/containerd/oci"
"github.com/containerd/continuity/fs"
"github.com/docker/docker/pkg/idtools"
Expand Down Expand Up @@ -106,17 +107,19 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root cache.Moun
defer release()
}

lm := snapshot.LocalMounterWithMounts(rootMounts)
rootfsPath, err := lm.Mount()
if err != nil {
return err
}
defer lm.Unmount()
defer executor.MountStubsCleaner(rootfsPath, mounts)()

var sgids []uint32
uid, gid, err := oci.ParseUIDGID(meta.User)
if err != nil {
lm := snapshot.LocalMounterWithMounts(rootMounts)
rootfsPath, err := lm.Mount()
if err != nil {
return err
}
uid, gid, sgids, err = oci.GetUser(rootfsPath, meta.User)
if err != nil {
lm.Unmount()
return err
}

Expand All @@ -127,17 +130,13 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root cache.Moun

newp, err := fs.RootPath(rootfsPath, meta.Cwd)
if err != nil {
lm.Unmount()
return errors.Wrapf(err, "working dir %s points to invalid target", newp)
}
if _, err := os.Stat(newp); err != nil {
if err := idtools.MkdirAllAndChown(newp, 0755, identity); err != nil {
lm.Unmount()
return errors.Wrapf(err, "failed to create working directory %s", newp)
}
}

lm.Unmount()
}

provider, ok := w.networkProviders[meta.NetMode]
Expand Down Expand Up @@ -196,7 +195,11 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root cache.Moun
cioOpts = append(cioOpts, cio.WithTerminal)
}

task, err := container.NewTask(ctx, cio.NewCreator(cioOpts...), containerd.WithRootFS(rootMounts))
task, err := container.NewTask(ctx, cio.NewCreator(cioOpts...), containerd.WithRootFS([]mount.Mount{{
Source: rootfsPath,
Type: "bind",
Options: []string{"rbind"},
}}))
if err != nil {
return err
}
Expand Down
2 changes: 2 additions & 0 deletions executor/runcexecutor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ func (w *runcExecutor) Run(ctx context.Context, id string, root cache.Mountable,
}
defer mount.Unmount(rootFSPath, 0)

defer executor.MountStubsCleaner(rootFSPath, mounts)()

uid, gid, sgids, err := oci.GetUser(rootFSPath, meta.User)
if err != nil {
return err
Expand Down
49 changes: 49 additions & 0 deletions executor/stubs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package executor

import (
"errors"
"os"
"path/filepath"
"syscall"

"github.com/containerd/continuity/fs"
)

func MountStubsCleaner(dir string, mounts []Mount) func() {
names := []string{"/etc/resolv.conf", "/etc/hosts"}

for _, m := range mounts {
names = append(names, m.Dest)
}

paths := make([]string, 0, len(names))

for _, p := range names {
p = filepath.Join("/", p)
if p == "/" {
continue
}
realPath, err := fs.RootPath(dir, p)
if err != nil {
continue
}

_, err = os.Lstat(realPath)
if errors.Is(err, os.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) {
paths = append(paths, realPath)
}
}

return func() {
for _, p := range paths {
st, err := os.Lstat(p)
if err != nil {
continue
}
if st.Size() != 0 {
continue
}
os.Remove(p)
}
}
}
7 changes: 6 additions & 1 deletion exporter/containerimage/exptypes/types.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
package exptypes

import specs "github.com/opencontainers/image-spec/specs-go/v1"
import (
"github.com/opencontainers/go-digest"
specs "github.com/opencontainers/image-spec/specs-go/v1"
)

const ExporterImageConfigKey = "containerimage.config"
const ExporterInlineCache = "containerimage.inlinecache"
const ExporterPlatformsKey = "refs.platforms"

const EmptyGZLayer = digest.Digest("sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1")

type Platforms struct {
Platforms []Platform
}
Expand Down
12 changes: 4 additions & 8 deletions exporter/containerimage/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ import (
"golang.org/x/sync/errgroup"
)

const (
emptyGZLayer = digest.Digest("sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1")
)

type WriterOpt struct {
Snapshotter snapshot.Snapshotter
ContentStore content.Store
Expand Down Expand Up @@ -426,13 +422,13 @@ func normalizeLayersAndHistory(remote *solver.Remote, history []ocispec.History,
var layerIndex int
for i, h := range history {
if !h.EmptyLayer {
if h.Created == nil {
h.Created = refMeta[layerIndex].createdAt
}
if remote.Descriptors[layerIndex].Digest == emptyGZLayer {
if remote.Descriptors[layerIndex].Digest == exptypes.EmptyGZLayer {
h.EmptyLayer = true
remote.Descriptors = append(remote.Descriptors[:layerIndex], remote.Descriptors[layerIndex+1:]...)
} else {
if h.Created == nil {
h.Created = refMeta[layerIndex].createdAt
}
layerIndex++
}
}
Expand Down
1 change: 1 addition & 0 deletions frontend/dockerfile/dockerfile_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func testSecretFileParams(t *testing.T, sb integration.Sandbox) {
dockerfile := []byte(`
FROM busybox
RUN --mount=type=secret,required=false,mode=741,uid=100,gid=102,target=/mysecret [ "$(stat -c "%u %g %f" /mysecret)" = "100 102 81e1" ]
RUN [ ! -f /mysecret ] # check no stub left behind
`)

dir, err := tmpdir(
Expand Down
7 changes: 2 additions & 5 deletions snapshot/imagerefchecker/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,12 @@ import (
"github.com/containerd/containerd/content"
"github.com/containerd/containerd/images"
"github.com/moby/buildkit/cache"
"github.com/moby/buildkit/exporter/containerimage/exptypes"
digest "github.com/opencontainers/go-digest"
specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
)

const (
emptyGZLayer = digest.Digest("sha256:4f4fb700ef54461cfa02571ae0db9a0dc1e0cdb5577484a6d75e68dc38e8acc1")
)

type Opt struct {
ImageStore images.Store
ContentStore content.Store
Expand Down Expand Up @@ -93,7 +90,7 @@ func toDigests(layers []specs.Descriptor) []digest.Digest {
func layerKey(layers []digest.Digest) string {
b := &strings.Builder{}
for _, l := range layers {
if l != emptyGZLayer {
if l != exptypes.EmptyGZLayer {
b.Write([]byte(l))
}
}
Expand Down

0 comments on commit dda009a

Please sign in to comment.