kotovalexarian-likes-github
/
moby--moby
mirror of https://github.com/moby/moby.git
1
0
Fork 0
Code Releases Activity

builder: fix layer lifecycle leak 

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
This commit is contained in:
Tonis Tiigi 2018-02-16 13:50:57 -08:00
parent 733ed2ddd3
commit 7ad41d53df
8 changed files with 186 additions and 136 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
builder/builder.go
View File

@ -53,7 +53,7 @@ type Backend interface {
// ImageBackend are the interface methods required from an image component
type ImageBackend interface {
GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (Image, ReleaseableLayer, error)
GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (Image, ROLayer, error)
}
// ExecBackend contains the interface methods required for executing containers
@ -100,10 +100,16 @@ type Image interface {
OperatingSystem() string
}
// ReleaseableLayer is an image layer that can be mounted and released
type ReleaseableLayer interface {
// ROLayer is a reference to image rootfs layer
type ROLayer interface {
Release() error
Mount() (containerfs.ContainerFS, error)
Commit() (ReleaseableLayer, error)
NewRWLayer() (RWLayer, error)
DiffID() layer.DiffID
}
// RWLayer is active layer that can be read/modified
type RWLayer interface {
Release() error
Root() containerfs.ContainerFS
Commit() (ROLayer, error)
}

20
builder/dockerfile/copy.go
View File

@ -72,8 +72,12 @@ type copier struct {
source builder.Source
pathCache pathCache
download sourceDownloader
tmpPaths []string
platform string
// for cleanup. TODO: having copier.cleanup() is error prone and hard to
// follow. Code calling performCopy should manage the lifecycle of its params.
// Copier should take override source as input, not imageMount.
activeLayer builder.RWLayer
tmpPaths []string
}
func copierFromDispatchRequest(req dispatchRequest, download sourceDownloader, imageSource *imageMount) copier {
@ -155,6 +159,10 @@ func (o *copier) Cleanup() {
os.RemoveAll(path)
}
o.tmpPaths = []string{}
if o.activeLayer != nil {
o.activeLayer.Release()
o.activeLayer = nil
}
}
// TODO: allowWildcards can probably be removed by refactoring this function further.
@ -166,9 +174,15 @@ func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo,
// done on image Source?
if imageSource != nil {
var err error
o.source, err = imageSource.Source()
rwLayer, err := imageSource.NewRWLayer()
if err != nil {
return nil, errors.Wrapf(err, "failed to copy from %s", imageSource.ImageID())
return nil, err
}
o.activeLayer = rwLayer
o.source, err = remotecontext.NewLazySource(rwLayer.Root())
if err != nil {
return nil, errors.Wrapf(err, "failed to create context for copy from %s", rwLayer.Root().Path())
}
}

6
builder/dockerfile/dispatchers_test.go
View File

@ -127,7 +127,7 @@ func TestFromScratch(t *testing.T) {
func TestFromWithArg(t *testing.T) {
tag, expected := ":sometag", "expectedthisid"
getImage := func(name string) (builder.Image, builder.ReleaseableLayer, error) {
getImage := func(name string) (builder.Image, builder.ROLayer, error) {
assert.Equal(t, "alpine"+tag, name)
return &mockImage{id: "expectedthisid"}, nil, nil
}
@ -159,7 +159,7 @@ func TestFromWithArg(t *testing.T) {
func TestFromWithUndefinedArg(t *testing.T) {
tag, expected := "sometag", "expectedthisid"
getImage := func(name string) (builder.Image, builder.ReleaseableLayer, error) {
getImage := func(name string) (builder.Image, builder.ROLayer, error) {
assert.Equal(t, "alpine", name)
return &mockImage{id: "expectedthisid"}, nil, nil
}
@ -433,7 +433,7 @@ func TestRunWithBuildArgs(t *testing.T) {
return imageCache
}
b.imageProber = newImageProber(mockBackend, nil, false)
mockBackend.getImageFunc = func(_ string) (builder.Image, builder.ReleaseableLayer, error) {
mockBackend.getImageFunc = func(_ string) (builder.Image, builder.ROLayer, error) {
return &mockImage{
id: "abcdef",
config: &container.Config{Cmd: origCmd},

31
builder/dockerfile/imagecontext.go
View File

@ -5,7 +5,6 @@ import (
"github.com/docker/docker/api/types/backend"
"github.com/docker/docker/builder"
"github.com/docker/docker/builder/remotecontext"
dockerimage "github.com/docker/docker/image"
"github.com/docker/docker/pkg/system"
"github.com/pkg/errors"
@ -13,7 +12,7 @@ import (
"golang.org/x/net/context"
)
type getAndMountFunc func(string, bool) (builder.Image, builder.ReleaseableLayer, error)
type getAndMountFunc func(string, bool) (builder.Image, builder.ROLayer, error)
// imageSources mounts images and provides a cache for mounted images. It tracks
// all images so they can be unmounted at the end of the build.
@ -24,7 +23,7 @@ type imageSources struct {
}
func newImageSources(ctx context.Context, options builderOptions) *imageSources {
getAndMount := func(idOrRef string, localOnly bool) (builder.Image, builder.ReleaseableLayer, error) {
getAndMount := func(idOrRef string, localOnly bool) (builder.Image, builder.ROLayer, error) {
pullOption := backend.PullOptionNoPull
if !localOnly {
if options.Options.PullParent {
@ -92,32 +91,14 @@ func (m *imageSources) Add(im *imageMount) {
type imageMount struct {
image builder.Image
source builder.Source
layer builder.ReleaseableLayer
layer builder.ROLayer
}
func newImageMount(image builder.Image, layer builder.ReleaseableLayer) *imageMount {
func newImageMount(image builder.Image, layer builder.ROLayer) *imageMount {
im := &imageMount{image: image, layer: layer}
return im
}
func (im *imageMount) Source() (builder.Source, error) {
if im.source == nil {
if im.layer == nil {
return nil, errors.Errorf("empty context")
}
mountPath, err := im.layer.Mount()
if err != nil {
return nil, errors.Wrapf(err, "failed to mount %s", im.image.ImageID())
}
source, err := remotecontext.NewLazySource(mountPath)
if err != nil {
return nil, errors.Wrapf(err, "failed to create lazycontext for %s", mountPath)
}
im.source = source
}
return im.source, nil
}
func (im *imageMount) unmount() error {
if im.layer == nil {
return nil
@ -133,8 +114,8 @@ func (im *imageMount) Image() builder.Image {
return im.image
}
func (im *imageMount) Layer() builder.ReleaseableLayer {
return im.layer
func (im *imageMount) NewRWLayer() (builder.RWLayer, error) {
return im.layer.NewRWLayer()
}
func (im *imageMount) ImageID() string {

26
builder/dockerfile/internals.go
View File

@ -17,6 +17,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/builder"
"github.com/docker/docker/image"
"github.com/docker/docker/pkg/archive"
"github.com/docker/docker/pkg/chrootarchive"
@ -114,8 +115,8 @@ func (b *Builder) commitContainer(dispatchState *dispatchState, id string, conta
return err
}
func (b *Builder) exportImage(state *dispatchState, imageMount *imageMount, runConfig *container.Config) error {
newLayer, err := imageMount.Layer().Commit()
func (b *Builder) exportImage(state *dispatchState, layer builder.RWLayer, parent builder.Image, runConfig *container.Config) error {
newLayer, err := layer.Commit()
if err != nil {
return err
}
@ -124,7 +125,7 @@ func (b *Builder) exportImage(state *dispatchState, imageMount *imageMount, runC
// 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)
parentImage, ok := parent.(*image.Image)
if !ok {
return errors.Errorf("unexpected image type")
}
@ -177,7 +178,13 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error
return errors.Wrapf(err, "failed to get destination image %q", state.imageID)
}
destInfo, err := createDestInfo(state.runConfig.WorkingDir, inst, imageMount, b.options.Platform)
rwLayer, err := imageMount.NewRWLayer()
if err != nil {
return err
}
defer rwLayer.Release()
destInfo, err := createDestInfo(state.runConfig.WorkingDir, inst, rwLayer, b.options.Platform)
if err != nil {
return err
}
@ -203,10 +210,10 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error
return errors.Wrapf(err, "failed to copy files")
}
}
return b.exportImage(state, imageMount, runConfigWithCommentCmd)
return b.exportImage(state, rwLayer, imageMount.Image(), runConfigWithCommentCmd)
}
func createDestInfo(workingDir string, inst copyInstruction, imageMount *imageMount, platform string) (copyInfo, error) {
func createDestInfo(workingDir string, inst copyInstruction, rwLayer builder.RWLayer, platform string) (copyInfo, error) {
// Twiddle the destination when it's a relative path - meaning, make it
// relative to the WORKINGDIR
dest, err := normalizeDest(workingDir, inst.dest, platform)
@ -214,12 +221,7 @@ func createDestInfo(workingDir string, inst copyInstruction, imageMount *imageMo
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
return copyInfo{root: rwLayer.Root(), path: dest}, nil
}
// normalizeDest normalises the destination of a COPY/ADD command in a

27
builder/dockerfile/mockbackend_test.go
View File

@ -20,7 +20,7 @@ import (
type MockBackend struct {
containerCreateFunc func(config types.ContainerCreateConfig) (container.ContainerCreateCreatedBody, error)
commitFunc func(backend.CommitConfig) (image.ID, error)
getImageFunc func(string) (builder.Image, builder.ReleaseableLayer, error)
getImageFunc func(string) (builder.Image, builder.ROLayer, error)
makeImageCacheFunc func(cacheFrom []string) builder.ImageCache
}
@ -66,7 +66,7 @@ func (m *MockBackend) CopyOnBuild(containerID string, destPath string, srcRoot s
return nil
}
func (m *MockBackend) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ReleaseableLayer, error) {
func (m *MockBackend) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ROLayer, error) {
if m.getImageFunc != nil {
return m.getImageFunc(refOrID)
}
@ -124,14 +124,25 @@ func (l *mockLayer) Release() error {
return nil
}
func (l *mockLayer) Mount() (containerfs.ContainerFS, error) {
return containerfs.NewLocalContainerFS("mountPath"), nil
}
func (l *mockLayer) Commit() (builder.ReleaseableLayer, error) {
return nil, nil
func (l *mockLayer) NewRWLayer() (builder.RWLayer, error) {
return &mockRWLayer{}, nil
}
func (l *mockLayer) DiffID() layer.DiffID {
return layer.DiffID("abcdef")
}
type mockRWLayer struct {
}
func (l *mockRWLayer) Release() error {
return nil
}
func (l *mockRWLayer) Commit() (builder.ROLayer, error) {
return nil, nil
}
func (l *mockRWLayer) Root() containerfs.ContainerFS {
return nil
}

156
daemon/build.go
View File

@ -15,130 +15,126 @@ import (
"github.com/docker/docker/pkg/system"
"github.com/docker/docker/registry"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/net/context"
)
type releaseableLayer struct {
type roLayer struct {
released bool
layerStore layer.Store
roLayer layer.Layer
rwLayer layer.RWLayer
}
func (rl *releaseableLayer) Mount() (containerfs.ContainerFS, error) {
var err error
var mountPath containerfs.ContainerFS
func (l *roLayer) DiffID() layer.DiffID {
if l.roLayer == nil {
return layer.DigestSHA256EmptyTar
}
return l.roLayer.DiffID()
}
func (l *roLayer) Release() error {
if l.released {
return nil
}
if l.roLayer != nil {
metadata, err := l.layerStore.Release(l.roLayer)
layer.LogReleaseMetadata(metadata)
if err != nil {
return errors.Wrap(err, "failed to release ROLayer")
}
}
l.roLayer = nil
l.released = true
return nil
}
func (l *roLayer) NewRWLayer() (builder.RWLayer, error) {
var chainID layer.ChainID
if rl.roLayer != nil {
chainID = rl.roLayer.ChainID()
if l.roLayer != nil {
chainID = l.roLayer.ChainID()
}
mountID := stringid.GenerateRandomID()
rl.rwLayer, err = rl.layerStore.CreateRWLayer(mountID, chainID, nil)
newLayer, err := l.layerStore.CreateRWLayer(mountID, chainID, nil)
if err != nil {
return nil, errors.Wrap(err, "failed to create rwlayer")
}
mountPath, err = rl.rwLayer.Mount("")
rwLayer := &rwLayer{layerStore: l.layerStore, rwLayer: newLayer}
fs, err := newLayer.Mount("")
if err != nil {
// Clean up the layer if we fail to mount it here.
metadata, err := rl.layerStore.ReleaseRWLayer(rl.rwLayer)
layer.LogReleaseMetadata(metadata)
if err != nil {
logrus.Errorf("Failed to release RWLayer: %s", err)
}
rl.rwLayer = nil
rwLayer.Release()
return nil, err
}
return mountPath, nil
rwLayer.fs = fs
return rwLayer, nil
}
func (rl *releaseableLayer) Commit() (builder.ReleaseableLayer, error) {
var chainID layer.ChainID
if rl.roLayer != nil {
chainID = rl.roLayer.ChainID()
}
type rwLayer struct {
released bool
layerStore layer.Store
rwLayer layer.RWLayer
fs containerfs.ContainerFS
}
stream, err := rl.rwLayer.TarStream()
func (l *rwLayer) Root() containerfs.ContainerFS {
return l.fs
}
func (l *rwLayer) Commit() (builder.ROLayer, error) {
stream, err := l.rwLayer.TarStream()
if err != nil {
return nil, err
}
defer stream.Close()
newLayer, err := rl.layerStore.Register(stream, chainID)
var chainID layer.ChainID
if parent := l.rwLayer.Parent(); parent != nil {
chainID = parent.ChainID()
}
newLayer, err := l.layerStore.Register(stream, chainID)
if err != nil {
return nil, err
}
// TODO: An optimization would be to handle empty layers before returning
return &releaseableLayer{layerStore: rl.layerStore, roLayer: newLayer}, nil
return &roLayer{layerStore: l.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 {
if rl.released {
func (l *rwLayer) Release() error {
if l.released {
return nil
}
if err := rl.releaseRWLayer(); err != nil {
// Best effort attempt at releasing read-only layer before returning original error.
rl.releaseROLayer()
return err
if l.fs != nil {
if err := l.rwLayer.Unmount(); err != nil {
return errors.Wrap(err, "failed to unmount RWLayer")
}
l.fs = nil
}
if err := rl.releaseROLayer(); err != nil {
return err
metadata, err := l.layerStore.ReleaseRWLayer(l.rwLayer)
layer.LogReleaseMetadata(metadata)
if err != nil {
return errors.Wrap(err, "failed to release RWLayer")
}
rl.released = true
l.released = true
return nil
}
func (rl *releaseableLayer) releaseRWLayer() error {
if rl.rwLayer == nil {
return nil
}
if err := rl.rwLayer.Unmount(); err != nil {
logrus.Errorf("Failed to unmount RWLayer: %s", err)
return err
}
metadata, err := rl.layerStore.ReleaseRWLayer(rl.rwLayer)
layer.LogReleaseMetadata(metadata)
if err != nil {
logrus.Errorf("Failed to release RWLayer: %s", err)
}
rl.rwLayer = nil
return err
}
func (rl *releaseableLayer) releaseROLayer() error {
if rl.roLayer == nil {
return nil
}
metadata, err := rl.layerStore.Release(rl.roLayer)
layer.LogReleaseMetadata(metadata)
if err != nil {
logrus.Errorf("Failed to release ROLayer: %s", err)
}
rl.roLayer = nil
return err
}
func newReleasableLayerForImage(img *image.Image, layerStore layer.Store) (builder.ReleaseableLayer, error) {
func newROLayerForImage(img *image.Image, layerStore layer.Store) (builder.ROLayer, error) {
if img == nil || img.RootFS.ChainID() == "" {
return &releaseableLayer{layerStore: layerStore}, nil
return &roLayer{layerStore: layerStore}, nil
}
// Hold a reference to the image layer so that it can't be removed before
// it is released
roLayer, err := layerStore.Get(img.RootFS.ChainID())
layer, err := layerStore.Get(img.RootFS.ChainID())
if err != nil {
return nil, errors.Wrapf(err, "failed to get layer for image %s", img.ImageID())
}
return &releaseableLayer{layerStore: layerStore, roLayer: roLayer}, nil
return &roLayer{layerStore: layerStore, roLayer: layer}, nil
}
// TODO: could this use the regular daemon PullImage ?
@ -170,12 +166,12 @@ func (daemon *Daemon) pullForBuilder(ctx context.Context, name string, authConfi
// GetImageAndReleasableLayer returns an image and releaseable layer for a reference or ID.
// 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) {
func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ROLayer, error) {
if refOrID == "" {
if !system.IsOSSupported(opts.OS) {
return nil, nil, system.ErrNotSupportedOperatingSystem
}
layer, err := newReleasableLayerForImage(nil, daemon.layerStores[opts.OS])
layer, err := newROLayerForImage(nil, daemon.layerStores[opts.OS])
return nil, layer, err
}
@ -189,7 +185,7 @@ func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID st
if !system.IsOSSupported(image.OperatingSystem()) {
return nil, nil, system.ErrNotSupportedOperatingSystem
}
layer, err := newReleasableLayerForImage(image, daemon.layerStores[image.OperatingSystem()])
layer, err := newROLayerForImage(image, daemon.layerStores[image.OperatingSystem()])
return image, layer, err
}
}
@ -201,7 +197,7 @@ func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID st
if !system.IsOSSupported(image.OperatingSystem()) {
return nil, nil, system.ErrNotSupportedOperatingSystem
}
layer, err := newReleasableLayerForImage(image, daemon.layerStores[image.OperatingSystem()])
layer, err := newROLayerForImage(image, daemon.layerStores[image.OperatingSystem()])
return image, layer, err
}

40
integration/build/build_test.go
View File

@ -301,6 +301,46 @@ COPY bar /`
require.NotContains(t, out.String(), "Using cache")
}
// docker/for-linux#135
// #35641
func TestBuildMultiStageLayerLeak(t *testing.T) {
ctx := context.TODO()
defer setupTest(t)()
// all commands need to match until COPY
dockerfile := `FROM busybox
WORKDIR /foo
COPY foo .
FROM busybox
WORKDIR /foo
COPY bar .
RUN [ -f bar ]
RUN [ ! -f foo ]
`
source := fakecontext.New(t, "",
fakecontext.WithFile("foo", "0"),
fakecontext.WithFile("bar", "1"),
fakecontext.WithDockerfile(dockerfile))
defer source.Close()
apiclient := testEnv.APIClient()
resp, err := apiclient.ImageBuild(ctx,
source.AsTarReader(t),
types.ImageBuildOptions{
Remove: true,
ForceRemove: true,
})
out := bytes.NewBuffer(nil)
require.NoError(t, err)
_, err = io.Copy(out, resp.Body)
resp.Body.Close()
require.NoError(t, err)
assert.Contains(t, out.String(), "Successfully built")
}
func writeTarRecord(t *testing.T, w *tar.Writer, fn, contents string) {
err := w.WriteHeader(&tar.Header{
Name: fn,