diff --git a/components/engine/api/server/backend/build/backend.go b/components/engine/api/server/backend/build/backend.go index bf171441ca..00aa7c3126 100644 --- a/components/engine/api/server/backend/build/backend.go +++ b/components/engine/api/server/backend/build/backend.go @@ -8,6 +8,7 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/builder/dockerfile" "github.com/docker/docker/image" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stringid" "github.com/pkg/errors" "golang.org/x/net/context" @@ -26,8 +27,8 @@ type Backend struct { } // NewBackend creates a new build backend from components -func NewBackend(components ImageComponent, builderBackend builder.Backend) *Backend { - manager := dockerfile.NewBuildManager(builderBackend) +func NewBackend(components ImageComponent, builderBackend builder.Backend, idMappings *idtools.IDMappings) *Backend { + manager := dockerfile.NewBuildManager(builderBackend, idMappings) return &Backend{imageComponent: components, manager: manager} } diff --git a/components/engine/builder/builder.go b/components/engine/builder/builder.go index 1aea99ea90..5cd6c838f9 100644 --- a/components/engine/builder/builder.go +++ b/components/engine/builder/builder.go @@ -11,6 +11,7 @@ import ( "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" containerpkg "github.com/docker/docker/container" + "github.com/docker/docker/layer" "golang.org/x/net/context" ) @@ -42,11 +43,7 @@ type Backend interface { // ContainerCreateWorkdir creates the workdir ContainerCreateWorkdir(containerID string) error - // ContainerCopy copies/extracts a source FileInfo to a destination path inside a container - // specified by a container object. - // TODO: extract in the builder instead of passing `decompress` - // TODO: use containerd/fs.changestream instead as a source - CopyOnBuild(containerID string, destPath string, srcRoot string, srcPath string, decompress bool) error + CreateImage(config []byte, parent string) (Image, error) ImageCacheBuilder } @@ -96,10 +93,13 @@ type ImageCache interface { type Image interface { ImageID() string RunConfig() *container.Config + MarshalJSON() ([]byte, error) } // ReleaseableLayer is an image layer that can be mounted and released type ReleaseableLayer interface { Release() error Mount() (string, error) + Commit() (ReleaseableLayer, error) + DiffID() layer.DiffID } diff --git a/components/engine/builder/dockerfile/builder.go b/components/engine/builder/dockerfile/builder.go index 52b540ab5d..d35ed9b91a 100644 --- a/components/engine/builder/dockerfile/builder.go +++ b/components/engine/builder/dockerfile/builder.go @@ -15,6 +15,9 @@ import ( "github.com/docker/docker/builder/dockerfile/command" "github.com/docker/docker/builder/dockerfile/parser" "github.com/docker/docker/builder/remotecontext" + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/chrootarchive" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/streamformatter" "github.com/docker/docker/pkg/stringid" "github.com/pkg/errors" @@ -37,15 +40,17 @@ var validCommitCommands = map[string]bool{ // BuildManager is shared across all Builder objects type BuildManager struct { + archiver *archive.Archiver backend builder.Backend pathCache pathCache // TODO: make this persistent } // NewBuildManager creates a BuildManager -func NewBuildManager(b builder.Backend) *BuildManager { +func NewBuildManager(b builder.Backend, idMappings *idtools.IDMappings) *BuildManager { return &BuildManager{ backend: b, pathCache: &syncmap.Map{}, + archiver: chrootarchive.NewArchiver(idMappings), } } @@ -73,6 +78,7 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) ( ProgressWriter: config.ProgressWriter, Backend: bm.backend, PathCache: bm.pathCache, + Archiver: bm.archiver, } return newBuilder(ctx, builderOptions).build(source, dockerfile) } @@ -83,6 +89,7 @@ type builderOptions struct { Backend builder.Backend ProgressWriter backend.ProgressWriter PathCache pathCache + Archiver *archive.Archiver } // Builder is a Dockerfile builder @@ -98,6 +105,7 @@ type Builder struct { docker builder.Backend clientCtx context.Context + archiver *archive.Archiver buildStages *buildStages disableCommit bool buildArgs *buildArgs @@ -121,6 +129,7 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder { Aux: options.ProgressWriter.AuxFormatter, Output: options.ProgressWriter.Output, docker: options.Backend, + archiver: options.Archiver, buildArgs: newBuildArgs(config.BuildArgs), buildStages: newBuildStages(), imageSources: newImageSources(clientCtx, options), diff --git a/components/engine/builder/dockerfile/copy.go b/components/engine/builder/dockerfile/copy.go index db98eb92e1..0026bdc158 100644 --- a/components/engine/builder/dockerfile/copy.go +++ b/components/engine/builder/dockerfile/copy.go @@ -13,9 +13,12 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/builder/remotecontext" + "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/ioutils" "github.com/docker/docker/pkg/progress" "github.com/docker/docker/pkg/streamformatter" + "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" "github.com/docker/docker/pkg/urlutil" "github.com/pkg/errors" @@ -34,6 +37,10 @@ type copyInfo struct { hash string } +func (c copyInfo) fullPath() (string, error) { + return symlink.FollowSymlinkInScope(filepath.Join(c.root, c.path), c.root) +} + func newCopyInfoFromSource(source builder.Source, path string, hash string) copyInfo { return copyInfo{root: source.Root(), path: path, hash: hash} } @@ -148,7 +155,7 @@ func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo, var err error o.source, err = imageSource.Source() if err != nil { - return nil, errors.Wrapf(err, "failed to copy") + return nil, errors.Wrapf(err, "failed to copy from %s", imageSource.ImageID()) } } @@ -352,6 +359,83 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b return } - lc, err := remotecontext.NewLazyContext(tmpDir) + lc, err := remotecontext.NewLazySource(tmpDir) return lc, filename, err } + +type copyFileOptions struct { + decompress bool + archiver *archive.Archiver +} + +func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions) error { + srcPath, err := source.fullPath() + if err != nil { + return err + } + destPath, err := dest.fullPath() + if err != nil { + return err + } + + archiver := options.archiver + + src, err := os.Stat(srcPath) + if err != nil { + return errors.Wrapf(err, "source path not found") + } + if src.IsDir() { + return copyDirectory(archiver, srcPath, destPath) + } + if options.decompress && archive.IsArchivePath(srcPath) { + return archiver.UntarPath(srcPath, destPath) + } + + destExistsAsDir, err := isExistingDirectory(destPath) + if err != nil { + return err + } + // dest.path must be used because destPath has already been cleaned of any + // trailing slash + if endsInSlash(dest.path) || destExistsAsDir { + // source.path must be used to get the correct filename when the source + // is a symlink + destPath = filepath.Join(destPath, filepath.Base(source.path)) + } + return copyFile(archiver, srcPath, destPath) +} + +func copyDirectory(archiver *archive.Archiver, source, dest string) error { + if err := archiver.CopyWithTar(source, dest); err != nil { + return errors.Wrapf(err, "failed to copy directory") + } + return fixPermissions(source, dest, archiver.IDMappings.RootPair()) +} + +func copyFile(archiver *archive.Archiver, source, dest string) error { + rootIDs := archiver.IDMappings.RootPair() + + if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest), 0755, rootIDs); err != nil { + return errors.Wrapf(err, "failed to create new directory") + } + if err := archiver.CopyFileWithTar(source, dest); err != nil { + return errors.Wrapf(err, "failed to copy file") + } + return fixPermissions(source, dest, rootIDs) +} + +func endsInSlash(path string) bool { + return strings.HasSuffix(path, string(os.PathSeparator)) +} + +// isExistingDirectory returns true if the path exists and is a directory +func isExistingDirectory(path string) (bool, error) { + destStat, err := os.Stat(path) + switch { + case os.IsNotExist(err): + return false, nil + case err != nil: + return false, err + } + return destStat.IsDir(), nil +} diff --git a/components/engine/builder/dockerfile/copy_test.go b/components/engine/builder/dockerfile/copy_test.go new file mode 100644 index 0000000000..aee225b5ff --- /dev/null +++ b/components/engine/builder/dockerfile/copy_test.go @@ -0,0 +1,45 @@ +package dockerfile + +import ( + "testing" + + "github.com/docker/docker/pkg/testutil/tempfile" + "github.com/stretchr/testify/assert" +) + +func TestIsExistingDirectory(t *testing.T) { + tmpfile := tempfile.NewTempFile(t, "file-exists-test", "something") + defer tmpfile.Remove() + tmpdir := tempfile.NewTempDir(t, "dir-exists-test") + defer tmpdir.Remove() + + var testcases = []struct { + doc string + path string + expected bool + }{ + { + doc: "directory exists", + path: tmpdir.Path, + expected: true, + }, + { + doc: "path doesn't exist", + path: "/bogus/path/does/not/exist", + expected: false, + }, + { + doc: "file exists", + path: tmpfile.Name(), + expected: false, + }, + } + + for _, testcase := range testcases { + result, err := isExistingDirectory(testcase.path) + if !assert.NoError(t, err) { + continue + } + assert.Equal(t, testcase.expected, result, testcase.doc) + } +} diff --git a/components/engine/builder/dockerfile/copy_unix.go b/components/engine/builder/dockerfile/copy_unix.go new file mode 100644 index 0000000000..326d95bb39 --- /dev/null +++ b/components/engine/builder/dockerfile/copy_unix.go @@ -0,0 +1,36 @@ +// +build !windows + +package dockerfile + +import ( + "os" + "path/filepath" + + "github.com/docker/docker/pkg/idtools" +) + +func fixPermissions(source, destination string, rootIDs idtools.IDPair) error { + skipChownRoot, err := isExistingDirectory(destination) + if err != nil { + return err + } + + // We Walk on the source rather than on the destination because we don't + // want to change permissions on things we haven't created or modified. + return filepath.Walk(source, func(fullpath string, info os.FileInfo, err error) error { + // Do not alter the walk root iff. it existed before, as it doesn't fall under + // the domain of "things we should chown". + if skipChownRoot && source == fullpath { + return nil + } + + // Path is prefixed by source: substitute with destination instead. + cleaned, err := filepath.Rel(source, fullpath) + if err != nil { + return err + } + + fullpath = filepath.Join(destination, cleaned) + return os.Lchown(fullpath, rootIDs.UID, rootIDs.GID) + }) +} diff --git a/components/engine/builder/dockerfile/copy_windows.go b/components/engine/builder/dockerfile/copy_windows.go new file mode 100644 index 0000000000..78f5b09457 --- /dev/null +++ b/components/engine/builder/dockerfile/copy_windows.go @@ -0,0 +1,8 @@ +package dockerfile + +import "github.com/docker/docker/pkg/idtools" + +func fixPermissions(source, destination string, rootIDs idtools.IDPair) error { + // chown is not supported on Windows + return nil +} diff --git a/components/engine/builder/dockerfile/dispatchers.go b/components/engine/builder/dockerfile/dispatchers.go index ca67eb7f30..1935ac85c5 100644 --- a/components/engine/builder/dockerfile/dispatchers.go +++ b/components/engine/builder/dockerfile/dispatchers.go @@ -23,6 +23,7 @@ import ( "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/builder" "github.com/docker/docker/builder/dockerfile/parser" + "github.com/docker/docker/image" "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/pkg/signal" "github.com/docker/go-connections/nat" @@ -251,10 +252,8 @@ func parseBuildStageName(args []string) (string, error) { return stageName, nil } -// scratchImage is used as a token for the empty base image. It uses buildStage -// as a convenient implementation of builder.Image, but is not actually a -// buildStage. -var scratchImage builder.Image = &buildStage{} +// scratchImage is used as a token for the empty base image. +var scratchImage builder.Image = &image.Image{} func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, error) { substitutionArgs := []string{} @@ -267,8 +266,8 @@ func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, err return nil, err } - if im, ok := b.buildStages.getByName(name); ok { - return im, nil + if stage, ok := b.buildStages.getByName(name); ok { + name = stage.ImageID() } // Windows cannot support a container with no base image. diff --git a/components/engine/builder/dockerfile/imagecontext.go b/components/engine/builder/dockerfile/imagecontext.go index f89cc8d9ce..2a5b6e2579 100644 --- a/components/engine/builder/dockerfile/imagecontext.go +++ b/components/engine/builder/dockerfile/imagecontext.go @@ -6,37 +6,29 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/api/types/backend" - "github.com/docker/docker/api/types/container" "github.com/docker/docker/builder" "github.com/docker/docker/builder/remotecontext" + dockerimage "github.com/docker/docker/image" "github.com/pkg/errors" "golang.org/x/net/context" ) type buildStage struct { - id string - config *container.Config + id string } -func newBuildStageFromImage(image builder.Image) *buildStage { - return &buildStage{id: image.ImageID(), config: image.RunConfig()} +func newBuildStage(imageID string) *buildStage { + return &buildStage{id: imageID} } func (b *buildStage) ImageID() string { return b.id } -func (b *buildStage) RunConfig() *container.Config { - return b.config -} - -func (b *buildStage) update(imageID string, runConfig *container.Config) { +func (b *buildStage) update(imageID string) { b.id = imageID - b.config = runConfig } -var _ builder.Image = &buildStage{} - // buildStages tracks each stage of a build so they can be retrieved by index // or by name. type buildStages struct { @@ -48,12 +40,12 @@ func newBuildStages() *buildStages { return &buildStages{byName: make(map[string]*buildStage)} } -func (s *buildStages) getByName(name string) (builder.Image, bool) { +func (s *buildStages) getByName(name string) (*buildStage, bool) { stage, ok := s.byName[strings.ToLower(name)] return stage, ok } -func (s *buildStages) get(indexOrName string) (builder.Image, error) { +func (s *buildStages) get(indexOrName string) (*buildStage, error) { index, err := strconv.Atoi(indexOrName) if err == nil { if err := s.validateIndex(index); err != nil { @@ -78,7 +70,7 @@ func (s *buildStages) validateIndex(i int) error { } func (s *buildStages) add(name string, image builder.Image) error { - stage := newBuildStageFromImage(image) + stage := newBuildStage(image.ImageID()) name = strings.ToLower(name) if len(name) > 0 { if _, ok := s.byName[name]; ok { @@ -90,8 +82,8 @@ func (s *buildStages) add(name string, image builder.Image) error { return nil } -func (s *buildStages) update(imageID string, runConfig *container.Config) { - s.sequence[len(s.sequence)-1].update(imageID, runConfig) +func (s *buildStages) update(imageID string) { + s.sequence[len(s.sequence)-1].update(imageID) } type getAndMountFunc func(string) (builder.Image, builder.ReleaseableLayer, error) @@ -100,6 +92,7 @@ type getAndMountFunc func(string) (builder.Image, builder.ReleaseableLayer, erro // all images so they can be unmounted at the end of the build. type imageSources struct { byImageID map[string]*imageMount + withoutID []*imageMount getImage getAndMountFunc cache pathCache // TODO: remove } @@ -129,7 +122,7 @@ func (m *imageSources) Get(idOrRef string) (*imageMount, error) { return nil, err } im := newImageMount(image, layer) - m.byImageID[image.ImageID()] = im + m.Add(im) return im, nil } @@ -140,9 +133,25 @@ func (m *imageSources) Unmount() (retErr error) { retErr = err } } + for _, im := range m.withoutID { + if err := im.unmount(); err != nil { + logrus.Error(err) + retErr = err + } + } return } +func (m *imageSources) Add(im *imageMount) { + switch im.image { + case nil: + im.image = &dockerimage.Image{} + m.withoutID = append(m.withoutID, im) + default: + m.byImageID[im.image.ImageID()] = im + } +} + // imageMount is a reference to an image that can be used as a builder.Source type imageMount struct { image builder.Image @@ -164,7 +173,7 @@ func (im *imageMount) Source() (builder.Source, error) { if err != nil { return nil, errors.Wrapf(err, "failed to mount %s", im.image.ImageID()) } - source, err := remotecontext.NewLazyContext(mountPath) + source, err := remotecontext.NewLazySource(mountPath) if err != nil { return nil, errors.Wrapf(err, "failed to create lazycontext for %s", mountPath) } @@ -180,6 +189,7 @@ func (im *imageMount) unmount() error { if err := im.layer.Release(); err != nil { return errors.Wrapf(err, "failed to unmount previous build image %s", im.image.ImageID()) } + im.layer = nil return nil } @@ -187,6 +197,10 @@ func (im *imageMount) Image() builder.Image { return im.image } +func (im *imageMount) Layer() builder.ReleaseableLayer { + return im.layer +} + func (im *imageMount) ImageID() string { return im.image.ImageID() } diff --git a/components/engine/builder/dockerfile/internals.go b/components/engine/builder/dockerfile/internals.go index 2d02e57a69..213e64a52e 100644 --- a/components/engine/builder/dockerfile/internals.go +++ b/components/engine/builder/dockerfile/internals.go @@ -12,6 +12,7 @@ import ( "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/backend" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/image" "github.com/docker/docker/pkg/stringid" "github.com/pkg/errors" ) @@ -37,7 +38,6 @@ func (b *Builder) commit(dispatchState *dispatchState, comment string) error { return b.commitContainer(dispatchState, id, runConfigWithCommentCmd) } -// TODO: see if any args can be dropped func (b *Builder) commitContainer(dispatchState *dispatchState, id string, containerConfig *container.Config) error { if b.disableCommit { return nil @@ -60,7 +60,47 @@ func (b *Builder) commitContainer(dispatchState *dispatchState, id string, conta } dispatchState.imageID = imageID - b.buildStages.update(imageID, dispatchState.runConfig) + b.buildStages.update(imageID) + return nil +} + +func (b *Builder) exportImage(state *dispatchState, imageMount *imageMount, runConfig *container.Config) error { + newLayer, err := imageMount.Layer().Commit() + if err != nil { + return err + } + + // add an image mount without an image so the layer is properly unmounted + // if there is an error before we can add the full mount with image + b.imageSources.Add(newImageMount(nil, newLayer)) + + parentImage, ok := imageMount.Image().(*image.Image) + if !ok { + return errors.Errorf("unexpected image type") + } + + newImage := image.NewChildImage(parentImage, image.ChildConfig{ + Author: state.maintainer, + ContainerConfig: runConfig, + DiffID: newLayer.DiffID(), + Config: copyRunConfig(state.runConfig), + }) + + // TODO: it seems strange to marshal this here instead of just passing in the + // image struct + config, err := newImage.MarshalJSON() + if err != nil { + return errors.Wrap(err, "failed to encode image config") + } + + exportedImage, err := b.docker.CreateImage(config, state.imageID) + if err != nil { + return errors.Wrapf(err, "failed to export image") + } + + state.imageID = exportedImage.ImageID() + b.imageSources.Add(newImageMount(exportedImage, newLayer)) + b.buildStages.update(state.imageID) return nil } @@ -71,24 +111,46 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error runConfigWithCommentCmd := copyRunConfig( state.runConfig, withCmdCommentString(fmt.Sprintf("%s %s in %s ", inst.cmdName, srcHash, inst.dest))) - containerID, err := b.probeAndCreate(state, runConfigWithCommentCmd) - if err != nil || containerID == "" { + hit, err := b.probeCache(state, runConfigWithCommentCmd) + if err != nil || hit { return err } - // Twiddle the destination when it's a relative path - meaning, make it - // relative to the WORKINGDIR - dest, err := normaliseDest(inst.cmdName, state.runConfig.WorkingDir, inst.dest) + imageMount, err := b.imageSources.Get(state.imageID) + if err != nil { + return errors.Wrapf(err, "failed to get destination image %q", state.imageID) + } + destInfo, err := createDestInfo(state.runConfig.WorkingDir, inst, imageMount) if err != nil { return err } + opts := copyFileOptions{ + decompress: inst.allowLocalDecompression, + archiver: b.archiver, + } for _, info := range inst.infos { - if err := b.docker.CopyOnBuild(containerID, dest, info.root, info.path, inst.allowLocalDecompression); err != nil { - return err + if err := performCopyForInfo(destInfo, info, opts); err != nil { + return errors.Wrapf(err, "failed to copy files") } } - return b.commitContainer(state, containerID, runConfigWithCommentCmd) + return b.exportImage(state, imageMount, runConfigWithCommentCmd) +} + +func createDestInfo(workingDir string, inst copyInstruction, imageMount *imageMount) (copyInfo, error) { + // Twiddle the destination when it's a relative path - meaning, make it + // relative to the WORKINGDIR + dest, err := normaliseDest(workingDir, inst.dest) + if err != nil { + return copyInfo{}, errors.Wrapf(err, "invalid %s", inst.cmdName) + } + + destMount, err := imageMount.Source() + if err != nil { + return copyInfo{}, errors.Wrapf(err, "failed to mount copy source") + } + + return newCopyInfoFromSource(destMount, dest, ""), nil } // For backwards compat, if there's just one info then use it as the @@ -182,7 +244,7 @@ func (b *Builder) probeCache(dispatchState *dispatchState, runConfig *container. fmt.Fprint(b.Stdout, " ---> Using cache\n") dispatchState.imageID = string(cachedID) - b.buildStages.update(dispatchState.imageID, runConfig) + b.buildStages.update(dispatchState.imageID) return true, nil } diff --git a/components/engine/builder/dockerfile/internals_unix.go b/components/engine/builder/dockerfile/internals_unix.go index 17102cb443..f4784e1cca 100644 --- a/components/engine/builder/dockerfile/internals_unix.go +++ b/components/engine/builder/dockerfile/internals_unix.go @@ -12,7 +12,7 @@ import ( // normaliseDest normalises the destination of a COPY/ADD command in a // platform semantically consistent way. -func normaliseDest(cmdName, workingDir, requested string) (string, error) { +func normaliseDest(workingDir, requested string) (string, error) { dest := filepath.FromSlash(requested) endsInSlash := strings.HasSuffix(requested, string(os.PathSeparator)) if !system.IsAbs(requested) { diff --git a/components/engine/builder/dockerfile/internals_windows.go b/components/engine/builder/dockerfile/internals_windows.go index 81a0386251..bb3285925f 100644 --- a/components/engine/builder/dockerfile/internals_windows.go +++ b/components/engine/builder/dockerfile/internals_windows.go @@ -12,7 +12,7 @@ import ( // normaliseDest normalises the destination of a COPY/ADD command in a // platform semantically consistent way. -func normaliseDest(cmdName, workingDir, requested string) (string, error) { +func normaliseDest(workingDir, requested string) (string, error) { dest := filepath.FromSlash(requested) endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator)) @@ -32,7 +32,7 @@ func normaliseDest(cmdName, workingDir, requested string) (string, error) { // we only want to validate where the DriveColon part has been supplied. if filepath.IsAbs(dest) { if strings.ToUpper(string(dest[0])) != "C" { - return "", fmt.Errorf("Windows does not support %s with a destinations not on the system drive (C:)", cmdName) + return "", fmt.Errorf("Windows does not support destinations not on the system drive (C:)") } dest = dest[2:] // Strip the drive letter } @@ -44,7 +44,7 @@ func normaliseDest(cmdName, workingDir, requested string) (string, error) { } if !system.IsAbs(dest) { if string(workingDir[0]) != "C" { - return "", fmt.Errorf("Windows does not support %s with relative paths when WORKDIR is not the system drive", cmdName) + return "", fmt.Errorf("Windows does not support relative paths when WORKDIR is not the system drive") } dest = filepath.Join(string(os.PathSeparator), workingDir[2:], dest) // Make sure we preserve any trailing slash diff --git a/components/engine/builder/dockerfile/internals_windows_test.go b/components/engine/builder/dockerfile/internals_windows_test.go index 868a6671a3..b4c8d4b3cb 100644 --- a/components/engine/builder/dockerfile/internals_windows_test.go +++ b/components/engine/builder/dockerfile/internals_windows_test.go @@ -2,16 +2,22 @@ package dockerfile -import "testing" +import ( + "fmt" + "testing" + + "github.com/docker/docker/pkg/testutil" + "github.com/stretchr/testify/assert" +) func TestNormaliseDest(t *testing.T) { tests := []struct{ current, requested, expected, etext string }{ - {``, `D:\`, ``, `Windows does not support TEST with a destinations not on the system drive (C:)`}, - {``, `e:/`, ``, `Windows does not support TEST with a destinations not on the system drive (C:)`}, + {``, `D:\`, ``, `Windows does not support destinations not on the system drive (C:)`}, + {``, `e:/`, ``, `Windows does not support destinations not on the system drive (C:)`}, {`invalid`, `./c1`, ``, `Current WorkingDir invalid is not platform consistent`}, {`C:`, ``, ``, `Current WorkingDir C: is not platform consistent`}, {`C`, ``, ``, `Current WorkingDir C is not platform consistent`}, - {`D:\`, `.`, ``, "Windows does not support TEST with relative paths when WORKDIR is not the system drive"}, + {`D:\`, `.`, ``, "Windows does not support relative paths when WORKDIR is not the system drive"}, {``, `D`, `D`, ``}, {``, `./a1`, `.\a1`, ``}, {``, `.\b1`, `.\b1`, ``}, @@ -32,20 +38,16 @@ func TestNormaliseDest(t *testing.T) { {`C:\wdm`, `foo/bar/`, `\wdm\foo\bar\`, ``}, {`C:\wdn`, `foo\bar/`, `\wdn\foo\bar\`, ``}, } - for _, i := range tests { - got, err := normaliseDest("TEST", i.current, i.requested) - if err != nil && i.etext == "" { - t.Fatalf("TestNormaliseDest Got unexpected error %q for %s %s. ", err.Error(), i.current, i.requested) - } - if i.etext != "" && ((err == nil) || (err != nil && err.Error() != i.etext)) { - if err == nil { - t.Fatalf("TestNormaliseDest Expected an error for %s %s but didn't get one", i.current, i.requested) - } else { - t.Fatalf("TestNormaliseDest Wrong error text for %s %s - %s", i.current, i.requested, err.Error()) + for _, testcase := range tests { + msg := fmt.Sprintf("Input: %s, %s", testcase.current, testcase.requested) + actual, err := normaliseDest(testcase.current, testcase.requested) + if testcase.etext == "" { + if !assert.NoError(t, err, msg) { + continue } - } - if i.etext == "" && got != i.expected { - t.Fatalf("TestNormaliseDest Expected %q for %q and %q. Got %q", i.expected, i.current, i.requested, got) + assert.Equal(t, testcase.expected, actual, msg) + } else { + testutil.ErrorContains(t, err, testcase.etext) } } } diff --git a/components/engine/builder/dockerfile/mockbackend_test.go b/components/engine/builder/dockerfile/mockbackend_test.go index 08ce18c2e8..876bb91f1b 100644 --- a/components/engine/builder/dockerfile/mockbackend_test.go +++ b/components/engine/builder/dockerfile/mockbackend_test.go @@ -1,6 +1,7 @@ package dockerfile import ( + "encoding/json" "io" "github.com/docker/docker/api/types" @@ -8,6 +9,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/builder" containerpkg "github.com/docker/docker/container" + "github.com/docker/docker/layer" "golang.org/x/net/context" ) @@ -76,6 +78,10 @@ func (m *MockBackend) MakeImageCache(cacheFrom []string) builder.ImageCache { return nil } +func (m *MockBackend) CreateImage(config []byte, parent string) (builder.Image, error) { + return nil, nil +} + type mockImage struct { id string config *container.Config @@ -89,6 +95,11 @@ func (i *mockImage) RunConfig() *container.Config { return i.config } +func (i *mockImage) MarshalJSON() ([]byte, error) { + type rawImage mockImage + return json.Marshal(rawImage(*i)) +} + type mockImageCache struct { getCacheFunc func(parentID string, cfg *container.Config) (string, error) } @@ -109,3 +120,11 @@ func (l *mockLayer) Release() error { func (l *mockLayer) Mount() (string, error) { return "mountPath", nil } + +func (l *mockLayer) Commit() (builder.ReleaseableLayer, error) { + return nil, nil +} + +func (l *mockLayer) DiffID() layer.DiffID { + return layer.DiffID("abcdef") +} diff --git a/components/engine/builder/remotecontext/detect.go b/components/engine/builder/remotecontext/detect.go index efac27f7ff..99385698cf 100644 --- a/components/engine/builder/remotecontext/detect.go +++ b/components/engine/builder/remotecontext/detect.go @@ -81,7 +81,7 @@ func withDockerfileFromContext(c modifiableContext, dockerfilePath string) (buil } func newGitRemote(gitURL string, dockerfilePath string) (builder.Source, *parser.Result, error) { - c, err := MakeGitContext(gitURL) // TODO: change this to NewLazyContext + c, err := MakeGitContext(gitURL) // TODO: change this to NewLazySource if err != nil { return nil, nil, err } diff --git a/components/engine/builder/remotecontext/lazycontext.go b/components/engine/builder/remotecontext/lazycontext.go index 283d82a7b1..41351a3583 100644 --- a/components/engine/builder/remotecontext/lazycontext.go +++ b/components/engine/builder/remotecontext/lazycontext.go @@ -12,30 +12,30 @@ import ( "github.com/pkg/errors" ) -// NewLazyContext creates a new LazyContext. LazyContext defines a hashed build +// NewLazySource creates a new LazyContext. LazyContext defines a hashed build // context based on a root directory. Individual files are hashed first time // they are asked. It is not safe to call methods of LazyContext concurrently. -func NewLazyContext(root string) (builder.Source, error) { - return &lazyContext{ +func NewLazySource(root string) (builder.Source, error) { + return &lazySource{ root: root, sums: make(map[string]string), }, nil } -type lazyContext struct { +type lazySource struct { root string sums map[string]string } -func (c *lazyContext) Root() string { +func (c *lazySource) Root() string { return c.root } -func (c *lazyContext) Close() error { +func (c *lazySource) Close() error { return nil } -func (c *lazyContext) Hash(path string) (string, error) { +func (c *lazySource) Hash(path string) (string, error) { cleanPath, fullPath, err := normalize(path, c.root) if err != nil { return "", err @@ -62,7 +62,7 @@ func (c *lazyContext) Hash(path string) (string, error) { return sum, nil } -func (c *lazyContext) prepareHash(relPath string, fi os.FileInfo) (string, error) { +func (c *lazySource) prepareHash(relPath string, fi os.FileInfo) (string, error) { p := filepath.Join(c.root, relPath) h, err := NewFileHash(p, relPath, fi) if err != nil { diff --git a/components/engine/cmd/dockerd/daemon.go b/components/engine/cmd/dockerd/daemon.go index ec2f477c48..b8b4762340 100644 --- a/components/engine/cmd/dockerd/daemon.go +++ b/components/engine/cmd/dockerd/daemon.go @@ -453,7 +453,7 @@ func initRouter(s *apiserver.Server, d *daemon.Daemon, c *cluster.Cluster) { image.NewRouter(d, decoder), systemrouter.NewRouter(d, c), volume.NewRouter(d), - build.NewRouter(buildbackend.NewBackend(d, d), d), + build.NewRouter(buildbackend.NewBackend(d, d, d.IDMappings()), d), swarmrouter.NewRouter(c), pluginrouter.NewRouter(d.PluginManager()), distributionrouter.NewRouter(d), diff --git a/components/engine/daemon/archive.go b/components/engine/daemon/archive.go index 3fb75bbb22..bd00daca53 100644 --- a/components/engine/daemon/archive.go +++ b/components/engine/daemon/archive.go @@ -10,9 +10,7 @@ import ( "github.com/docker/docker/container" "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/chrootarchive" - "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/ioutils" - "github.com/docker/docker/pkg/symlink" "github.com/docker/docker/pkg/system" "github.com/pkg/errors" ) @@ -362,103 +360,3 @@ func (daemon *Daemon) containerCopy(container *container.Container, resource str daemon.LogContainerEvent(container, "copy") return reader, nil } - -// CopyOnBuild copies/extracts a source FileInfo to a destination path inside a container -// specified by a container object. -// TODO: make sure callers don't unnecessarily convert destPath with filepath.FromSlash (Copy does it already). -// CopyOnBuild should take in abstract paths (with slashes) and the implementation should convert it to OS-specific paths. -func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decompress bool) error { - fullSrcPath, err := symlink.FollowSymlinkInScope(filepath.Join(srcRoot, srcPath), srcRoot) - if err != nil { - return err - } - - destExists := true - destDir := false - rootIDs := daemon.idMappings.RootPair() - - // Work in daemon-local OS specific file paths - destPath = filepath.FromSlash(destPath) - - c, err := daemon.GetContainer(cID) - if err != nil { - return err - } - err = daemon.Mount(c) - if err != nil { - return err - } - defer daemon.Unmount(c) - - dest, err := c.GetResourcePath(destPath) - if err != nil { - return err - } - - // Preserve the trailing slash - // TODO: why are we appending another path separator if there was already one? - if strings.HasSuffix(destPath, string(os.PathSeparator)) || destPath == "." { - destDir = true - dest += string(os.PathSeparator) - } - - destPath = dest - - destStat, err := os.Stat(destPath) - if err != nil { - if !os.IsNotExist(err) { - //logrus.Errorf("Error performing os.Stat on %s. %s", destPath, err) - return err - } - destExists = false - } - - archiver := chrootarchive.NewArchiver(daemon.idMappings) - src, err := os.Stat(fullSrcPath) - if err != nil { - return err - } - - if src.IsDir() { - // copy as directory - if err := archiver.CopyWithTar(fullSrcPath, destPath); err != nil { - return err - } - return fixPermissions(fullSrcPath, destPath, rootIDs.UID, rootIDs.GID, destExists) - } - if decompress && archive.IsArchivePath(fullSrcPath) { - // Only try to untar if it is a file and that we've been told to decompress (when ADD-ing a remote file) - - // First try to unpack the source as an archive - // to support the untar feature we need to clean up the path a little bit - // because tar is very forgiving. First we need to strip off the archive's - // filename from the path but this is only added if it does not end in slash - tarDest := destPath - if strings.HasSuffix(tarDest, string(os.PathSeparator)) { - tarDest = filepath.Dir(destPath) - } - - // try to successfully untar the orig - err := archiver.UntarPath(fullSrcPath, tarDest) - /* - if err != nil { - logrus.Errorf("Couldn't untar to %s: %v", tarDest, err) - } - */ - return err - } - - // only needed for fixPermissions, but might as well put it before CopyFileWithTar - if destDir || (destExists && destStat.IsDir()) { - destPath = filepath.Join(destPath, filepath.Base(srcPath)) - } - - if err := idtools.MkdirAllAndChownNew(filepath.Dir(destPath), 0755, rootIDs); err != nil { - return err - } - if err := archiver.CopyFileWithTar(fullSrcPath, destPath); err != nil { - return err - } - - return fixPermissions(fullSrcPath, destPath, rootIDs.UID, rootIDs.GID, destExists) -} diff --git a/components/engine/daemon/archive_unix.go b/components/engine/daemon/archive_unix.go index 8806e2e198..d5dfad78cb 100644 --- a/components/engine/daemon/archive_unix.go +++ b/components/engine/daemon/archive_unix.go @@ -3,9 +3,6 @@ package daemon import ( - "os" - "path/filepath" - "github.com/docker/docker/container" ) @@ -25,38 +22,6 @@ func checkIfPathIsInAVolume(container *container.Container, absPath string) (boo return toVolume, nil } -func fixPermissions(source, destination string, uid, gid int, destExisted bool) error { - // If the destination didn't already exist, or the destination isn't a - // directory, then we should Lchown the destination. Otherwise, we shouldn't - // Lchown the destination. - destStat, err := os.Stat(destination) - if err != nil { - // This should *never* be reached, because the destination must've already - // been created while untar-ing the context. - return err - } - doChownDestination := !destExisted || !destStat.IsDir() - - // We Walk on the source rather than on the destination because we don't - // want to change permissions on things we haven't created or modified. - return filepath.Walk(source, func(fullpath string, info os.FileInfo, err error) error { - // Do not alter the walk root iff. it existed before, as it doesn't fall under - // the domain of "things we should chown". - if !doChownDestination && (source == fullpath) { - return nil - } - - // Path is prefixed by source: substitute with destination instead. - cleaned, err := filepath.Rel(source, fullpath) - if err != nil { - return err - } - - fullpath = filepath.Join(destination, cleaned) - return os.Lchown(fullpath, uid, gid) - }) -} - // isOnlineFSOperationPermitted returns an error if an online filesystem operation // is not permitted. func (daemon *Daemon) isOnlineFSOperationPermitted(container *container.Container) error { diff --git a/components/engine/daemon/archive_windows.go b/components/engine/daemon/archive_windows.go index 1c5a894f7c..ab105607d5 100644 --- a/components/engine/daemon/archive_windows.go +++ b/components/engine/daemon/archive_windows.go @@ -17,11 +17,6 @@ func checkIfPathIsInAVolume(container *container.Container, absPath string) (boo return false, nil } -func fixPermissions(source, destination string, uid, gid int, destExisted bool) error { - // chown is not supported on Windows - return nil -} - // isOnlineFSOperationPermitted returns an error if an online filesystem operation // is not permitted (such as stat or for copying). Running Hyper-V containers // cannot have their file-system interrogated from the host as the filter is diff --git a/components/engine/daemon/build.go b/components/engine/daemon/build.go index 1bf05ded2c..3faa2affd4 100644 --- a/components/engine/daemon/build.go +++ b/components/engine/daemon/build.go @@ -1,6 +1,8 @@ package daemon import ( + "io" + "github.com/Sirupsen/logrus" "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" @@ -8,11 +10,11 @@ import ( "github.com/docker/docker/builder" "github.com/docker/docker/image" "github.com/docker/docker/layer" + "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/registry" "github.com/pkg/errors" "golang.org/x/net/context" - "io" ) type releaseableLayer struct { @@ -22,12 +24,14 @@ type releaseableLayer struct { } func (rl *releaseableLayer) Mount() (string, error) { - if rl.roLayer == nil { - return "", errors.New("can not mount an image with no root FS") - } var err error + var chainID layer.ChainID + if rl.roLayer != nil { + chainID = rl.roLayer.ChainID() + } + mountID := stringid.GenerateRandomID() - rl.rwLayer, err = rl.layerStore.CreateRWLayer(mountID, rl.roLayer.ChainID(), nil) + rl.rwLayer, err = rl.layerStore.CreateRWLayer(mountID, chainID, nil) if err != nil { return "", errors.Wrap(err, "failed to create rwlayer") } @@ -35,6 +39,36 @@ func (rl *releaseableLayer) Mount() (string, error) { return rl.rwLayer.Mount("") } +func (rl *releaseableLayer) Commit() (builder.ReleaseableLayer, error) { + var chainID layer.ChainID + if rl.roLayer != nil { + chainID = rl.roLayer.ChainID() + } + + stream, err := rl.rwLayer.TarStream() + if err != nil { + return nil, err + } + + newLayer, err := rl.layerStore.Register(stream, chainID) + if err != nil { + return nil, err + } + + if layer.IsEmpty(newLayer.DiffID()) { + _, err := rl.layerStore.Release(newLayer) + return &releaseableLayer{layerStore: rl.layerStore}, err + } + return &releaseableLayer{layerStore: rl.layerStore, roLayer: newLayer}, nil +} + +func (rl *releaseableLayer) DiffID() layer.DiffID { + if rl.roLayer == nil { + return layer.DigestSHA256EmptyTar + } + return rl.roLayer.DiffID() +} + func (rl *releaseableLayer) Release() error { rl.releaseRWLayer() return rl.releaseROLayer() @@ -62,8 +96,8 @@ func (rl *releaseableLayer) releaseROLayer() error { } func newReleasableLayerForImage(img *image.Image, layerStore layer.Store) (builder.ReleaseableLayer, error) { - if img.RootFS.ChainID() == "" { - return nil, nil + if img == nil || img.RootFS.ChainID() == "" { + return &releaseableLayer{layerStore: layerStore}, nil } // Hold a reference to the image layer so that it can't be removed before // it is released @@ -104,6 +138,11 @@ func (daemon *Daemon) pullForBuilder(ctx context.Context, name string, authConfi // Every call to GetImageAndReleasableLayer MUST call releasableLayer.Release() to prevent // leaking of layers. func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ReleaseableLayer, error) { + if refOrID == "" { + layer, err := newReleasableLayerForImage(nil, daemon.layerStore) + return nil, layer, err + } + if !opts.ForcePull { image, _ := daemon.GetImage(refOrID) // TODO: shouldn't we error out if error is different from "not found" ? @@ -120,3 +159,26 @@ func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID st layer, err := newReleasableLayerForImage(image, daemon.layerStore) return image, layer, err } + +// CreateImage creates a new image by adding a config and ID to the image store. +// This is similar to LoadImage() except that it receives JSON encoded bytes of +// an image instead of a tar archive. +func (daemon *Daemon) CreateImage(config []byte, parent string) (builder.Image, error) { + id, err := daemon.imageStore.Create(config) + if err != nil { + return nil, errors.Wrapf(err, "failed to create image") + } + + if parent != "" { + if err := daemon.imageStore.SetParent(id, image.ID(parent)); err != nil { + return nil, errors.Wrapf(err, "failed to set parent %s", parent) + } + } + + return daemon.imageStore.Get(id) +} + +// IDMappings returns uid/gid mappings for the builder +func (daemon *Daemon) IDMappings() *idtools.IDMappings { + return daemon.idMappings +} diff --git a/components/engine/daemon/commit.go b/components/engine/daemon/commit.go index e64c7d3333..6011628092 100644 --- a/components/engine/daemon/commit.go +++ b/components/engine/daemon/commit.go @@ -12,7 +12,6 @@ import ( containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/builder/dockerfile" "github.com/docker/docker/container" - "github.com/docker/docker/dockerversion" "github.com/docker/docker/image" "github.com/docker/docker/layer" "github.com/docker/docker/pkg/ioutils" @@ -129,11 +128,6 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str return "", err } - containerConfig := c.ContainerConfig - if containerConfig == nil { - containerConfig = container.Config - } - // It is not possible to commit a running container on Windows and on Solaris. if (runtime.GOOS == "windows" || runtime.GOOS == "solaris") && container.IsRunning() { return "", errors.Errorf("%+v does not support commit of a running container", runtime.GOOS) @@ -165,60 +159,36 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str } }() - var history []image.History - rootFS := image.NewRootFS() - osVersion := "" - var osFeatures []string - - if container.ImageID != "" { - img, err := daemon.imageStore.Get(container.ImageID) + var parent *image.Image + if container.ImageID == "" { + parent = new(image.Image) + parent.RootFS = image.NewRootFS() + } else { + parent, err = daemon.imageStore.Get(container.ImageID) if err != nil { return "", err } - history = img.History - rootFS = img.RootFS - osVersion = img.OSVersion - osFeatures = img.OSFeatures } - l, err := daemon.layerStore.Register(rwTar, rootFS.ChainID()) + l, err := daemon.layerStore.Register(rwTar, parent.RootFS.ChainID()) if err != nil { return "", err } defer layer.ReleaseAndLog(daemon.layerStore, l) - h := image.History{ - Author: c.Author, - Created: time.Now().UTC(), - CreatedBy: strings.Join(containerConfig.Cmd, " "), - Comment: c.Comment, - EmptyLayer: true, + containerConfig := c.ContainerConfig + if containerConfig == nil { + containerConfig = container.Config } - - if diffID := l.DiffID(); layer.DigestSHA256EmptyTar != diffID { - h.EmptyLayer = false - rootFS.Append(diffID) + cc := image.ChildConfig{ + ContainerID: container.ID, + Author: c.Author, + Comment: c.Comment, + ContainerConfig: containerConfig, + Config: newConfig, + DiffID: l.DiffID(), } - - history = append(history, h) - - config, err := json.Marshal(&image.Image{ - V1Image: image.V1Image{ - DockerVersion: dockerversion.Version, - Config: newConfig, - Architecture: runtime.GOARCH, - OS: runtime.GOOS, - Container: container.ID, - ContainerConfig: *containerConfig, - Author: c.Author, - Created: h.Created, - }, - RootFS: rootFS, - History: history, - OSFeatures: osFeatures, - OSVersion: osVersion, - }) - + config, err := json.Marshal(image.NewChildImage(parent, cc)) if err != nil { return "", err } diff --git a/components/engine/image/image.go b/components/engine/image/image.go index 17935ac234..9f892d298a 100644 --- a/components/engine/image/image.go +++ b/components/engine/image/image.go @@ -4,9 +4,13 @@ import ( "encoding/json" "errors" "io" + "runtime" + "strings" "time" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/dockerversion" + "github.com/docker/docker/layer" "github.com/opencontainers/go-digest" ) @@ -110,6 +114,51 @@ func (img *Image) MarshalJSON() ([]byte, error) { return json.Marshal(c) } +// ChildConfig is the configuration to apply to an Image to create a new +// Child image. Other properties of the image are copied from the parent. +type ChildConfig struct { + ContainerID string + Author string + Comment string + DiffID layer.DiffID + ContainerConfig *container.Config + Config *container.Config +} + +// NewChildImage creates a new Image as a child of this image. +func NewChildImage(img *Image, child ChildConfig) *Image { + isEmptyLayer := layer.IsEmpty(child.DiffID) + rootFS := img.RootFS + if rootFS == nil { + rootFS = NewRootFS() + } + if !isEmptyLayer { + rootFS.Append(child.DiffID) + } + imgHistory := NewHistory( + child.Author, + child.Comment, + strings.Join(child.ContainerConfig.Cmd, " "), + isEmptyLayer) + + return &Image{ + V1Image: V1Image{ + DockerVersion: dockerversion.Version, + Config: child.Config, + Architecture: runtime.GOARCH, + OS: runtime.GOOS, + Container: child.ContainerID, + ContainerConfig: *child.ContainerConfig, + Author: child.Author, + Created: imgHistory.Created, + }, + RootFS: rootFS, + History: append(img.History, imgHistory), + OSFeatures: img.OSFeatures, + OSVersion: img.OSVersion, + } +} + // History stores build commands that were used to create an image type History struct { // Created is the timestamp at which the image was created @@ -126,6 +175,18 @@ type History struct { EmptyLayer bool `json:"empty_layer,omitempty"` } +// NewHistory creates a new history struct from arguments, and sets the created +// time to the current time in UTC +func NewHistory(author, comment, createdBy string, isEmptyLayer bool) History { + return History{ + Author: author, + Created: time.Now().UTC(), + CreatedBy: createdBy, + Comment: comment, + EmptyLayer: isEmptyLayer, + } +} + // Exporter provides interface for loading and saving images type Exporter interface { Load(io.ReadCloser, io.Writer, bool) error diff --git a/components/engine/image/store.go b/components/engine/image/store.go index 26ae109a06..17a3b7880a 100644 --- a/components/engine/image/store.go +++ b/components/engine/image/store.go @@ -2,7 +2,6 @@ package image import ( "encoding/json" - "errors" "fmt" "sync" @@ -10,6 +9,7 @@ import ( "github.com/docker/distribution/digestset" "github.com/docker/docker/layer" "github.com/opencontainers/go-digest" + "github.com/pkg/errors" ) // Store is an interface for creating and accessing images @@ -147,7 +147,7 @@ func (is *store) Create(config []byte) (ID, error) { if layerID != "" { l, err = is.ls.Get(layerID) if err != nil { - return "", err + return "", errors.Wrapf(err, "failed to get layer %s", layerID) } } diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index 482de38b6c..b9a5dff789 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -5870,15 +5870,17 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFS(c *check.C) { dockerfile := ` FROM busybox AS first COPY foo bar + FROM busybox - %s - COPY baz baz - RUN echo mno > baz/cc + %s + COPY baz baz + RUN echo mno > baz/cc + FROM busybox - COPY bar / - COPY --from=1 baz sub/ - COPY --from=0 bar baz - COPY --from=first bar bay` + COPY bar / + COPY --from=1 baz sub/ + COPY --from=0 bar baz + COPY --from=first bar bay` ctx := fakecontext.New(c, "", fakecontext.WithDockerfile(fmt.Sprintf(dockerfile, "")), @@ -5892,32 +5894,25 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFS(c *check.C) { cli.BuildCmd(c, "build1", build.WithExternalBuildContext(ctx)) - out := cli.DockerCmd(c, "run", "build1", "cat", "bar").Combined() - c.Assert(strings.TrimSpace(out), check.Equals, "def") - out = cli.DockerCmd(c, "run", "build1", "cat", "sub/aa").Combined() - c.Assert(strings.TrimSpace(out), check.Equals, "ghi") - out = cli.DockerCmd(c, "run", "build1", "cat", "sub/cc").Combined() - c.Assert(strings.TrimSpace(out), check.Equals, "mno") - out = cli.DockerCmd(c, "run", "build1", "cat", "baz").Combined() - c.Assert(strings.TrimSpace(out), check.Equals, "abc") - out = cli.DockerCmd(c, "run", "build1", "cat", "bay").Combined() - c.Assert(strings.TrimSpace(out), check.Equals, "abc") + cli.DockerCmd(c, "run", "build1", "cat", "bar").Assert(c, icmd.Expected{Out: "def"}) + cli.DockerCmd(c, "run", "build1", "cat", "sub/aa").Assert(c, icmd.Expected{Out: "ghi"}) + cli.DockerCmd(c, "run", "build1", "cat", "sub/cc").Assert(c, icmd.Expected{Out: "mno"}) + cli.DockerCmd(c, "run", "build1", "cat", "baz").Assert(c, icmd.Expected{Out: "abc"}) + cli.DockerCmd(c, "run", "build1", "cat", "bay").Assert(c, icmd.Expected{Out: "abc"}) result := cli.BuildCmd(c, "build2", build.WithExternalBuildContext(ctx)) // all commands should be cached c.Assert(strings.Count(result.Combined(), "Using cache"), checker.Equals, 7) + c.Assert(getIDByName(c, "build1"), checker.Equals, getIDByName(c, "build2")) err := ioutil.WriteFile(filepath.Join(ctx.Dir, "Dockerfile"), []byte(fmt.Sprintf(dockerfile, "COPY baz/aa foo")), 0644) c.Assert(err, checker.IsNil) // changing file in parent block should not affect last block result = cli.BuildCmd(c, "build3", build.WithExternalBuildContext(ctx)) - c.Assert(strings.Count(result.Combined(), "Using cache"), checker.Equals, 5) - c.Assert(getIDByName(c, "build1"), checker.Equals, getIDByName(c, "build2")) - err = ioutil.WriteFile(filepath.Join(ctx.Dir, "foo"), []byte("pqr"), 0644) c.Assert(err, checker.IsNil) @@ -5925,10 +5920,8 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFS(c *check.C) { result = cli.BuildCmd(c, "build4", build.WithExternalBuildContext(ctx)) c.Assert(strings.Count(result.Combined(), "Using cache"), checker.Equals, 5) - out = cli.DockerCmd(c, "run", "build4", "cat", "bay").Combined() - c.Assert(strings.TrimSpace(out), check.Equals, "pqr") - out = cli.DockerCmd(c, "run", "build4", "cat", "baz").Combined() - c.Assert(strings.TrimSpace(out), check.Equals, "pqr") + cli.DockerCmd(c, "run", "build4", "cat", "bay").Assert(c, icmd.Expected{Out: "pqr"}) + cli.DockerCmd(c, "run", "build4", "cat", "baz").Assert(c, icmd.Expected{Out: "pqr"}) } func (s *DockerSuite) TestBuildCopyFromPreviousRootFSErrors(c *check.C) { diff --git a/components/engine/layer/empty.go b/components/engine/layer/empty.go index 3b6ffc82f7..80f2c12439 100644 --- a/components/engine/layer/empty.go +++ b/components/engine/layer/empty.go @@ -54,3 +54,8 @@ func (el *emptyLayer) DiffSize() (size int64, err error) { func (el *emptyLayer) Metadata() (map[string]string, error) { return make(map[string]string), nil } + +// IsEmpty returns true if the layer is an EmptyLayer +func IsEmpty(diffID DiffID) bool { + return diffID == DigestSHA256EmptyTar +} diff --git a/components/engine/pkg/testutil/tempfile/tempfile.go b/components/engine/pkg/testutil/tempfile/tempfile.go index b375cb9561..01474babff 100644 --- a/components/engine/pkg/testutil/tempfile/tempfile.go +++ b/components/engine/pkg/testutil/tempfile/tempfile.go @@ -34,3 +34,23 @@ func (f *TempFile) Name() string { func (f *TempFile) Remove() { os.Remove(f.Name()) } + +// TempDir is a temporary directory that can be used with unit tests. TempDir +// reduces the boilerplate setup required in each test case by handling +// setup errors. +type TempDir struct { + Path string +} + +// NewTempDir returns a new temp file with contents +func NewTempDir(t require.TestingT, prefix string) *TempDir { + path, err := ioutil.TempDir("", prefix+"-") + require.NoError(t, err) + + return &TempDir{Path: path} +} + +// Remove removes the file +func (f *TempDir) Remove() { + os.Remove(f.Path) +}