-
Notifications
You must be signed in to change notification settings - Fork 421
[19.03 backport] Added garbage collector for image layers #268
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
// +build !windows | ||
|
||
package image // import "github.com/docker/docker/integration/image" | ||
|
||
import ( | ||
"context" | ||
"io" | ||
"io/ioutil" | ||
"os" | ||
"strconv" | ||
"strings" | ||
"syscall" | ||
"testing" | ||
"unsafe" | ||
|
||
"github.com/docker/docker/api/types" | ||
"github.com/docker/docker/internal/test/daemon" | ||
"github.com/docker/docker/internal/test/fakecontext" | ||
"gotest.tools/assert" | ||
"gotest.tools/skip" | ||
) | ||
|
||
// This is a regression test for #38488 | ||
// It ensures that orphan layers can be found and cleaned up | ||
// after unsuccessful image removal | ||
func TestRemoveImageGarbageCollector(t *testing.T) { | ||
// This test uses very platform specific way to prevent | ||
// daemon for remove image layer. | ||
skip.If(t, testEnv.DaemonInfo.OSType != "linux") | ||
skip.If(t, os.Getenv("DOCKER_ENGINE_GOARCH") != "amd64") | ||
|
||
// Create daemon with overlay2 graphdriver because vfs uses disk differently | ||
// and this test case would not work with it. | ||
d := daemon.New(t, daemon.WithStorageDriver("overlay2"), daemon.WithImageService) | ||
d.Start(t) | ||
defer d.Stop(t) | ||
|
||
ctx := context.Background() | ||
client := d.NewClientT(t) | ||
i := d.ImageService() | ||
|
||
img := "test-garbage-collector" | ||
|
||
// Build a image with multiple layers | ||
dockerfile := `FROM busybox | ||
RUN echo echo Running... > /run.sh` | ||
source := fakecontext.New(t, "", fakecontext.WithDockerfile(dockerfile)) | ||
defer source.Close() | ||
resp, err := client.ImageBuild(ctx, | ||
source.AsTarReader(t), | ||
types.ImageBuildOptions{ | ||
Remove: true, | ||
ForceRemove: true, | ||
Tags: []string{img}, | ||
}) | ||
assert.NilError(t, err) | ||
_, err = io.Copy(ioutil.Discard, resp.Body) | ||
resp.Body.Close() | ||
assert.NilError(t, err) | ||
image, _, err := client.ImageInspectWithRaw(ctx, img) | ||
assert.NilError(t, err) | ||
|
||
// Mark latest image layer to immutable | ||
data := image.GraphDriver.Data | ||
file, _ := os.Open(data["UpperDir"]) | ||
attr := 0x00000010 | ||
fsflags := uintptr(0x40086602) | ||
argp := uintptr(unsafe.Pointer(&attr)) | ||
_, _, errno := syscall.Syscall(syscall.SYS_IOCTL, file.Fd(), fsflags, argp) | ||
assert.Equal(t, "errno 0", errno.Error()) | ||
|
||
// Try to remove the image, it should generate error | ||
// but marking layer back to mutable before checking errors (so we don't break CI server) | ||
_, err = client.ImageRemove(ctx, img, types.ImageRemoveOptions{}) | ||
attr = 0x00000000 | ||
argp = uintptr(unsafe.Pointer(&attr)) | ||
_, _, errno = syscall.Syscall(syscall.SYS_IOCTL, file.Fd(), fsflags, argp) | ||
assert.Equal(t, "errno 0", errno.Error()) | ||
assert.Assert(t, err != nil) | ||
errStr := err.Error() | ||
if !(strings.Contains(errStr, "permission denied") || strings.Contains(errStr, "operation not permitted")) { | ||
t.Errorf("ImageRemove error not an permission error %s", errStr) | ||
} | ||
|
||
// Verify that layer remaining on disk | ||
dir, _ := os.Stat(data["UpperDir"]) | ||
assert.Equal(t, "true", strconv.FormatBool(dir.IsDir())) | ||
|
||
// Run imageService.Cleanup() and make sure that layer was removed from disk | ||
i.Cleanup() | ||
dir, err = os.Stat(data["UpperDir"]) | ||
assert.ErrorContains(t, err, "no such file or directory") | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
// +build !windows | ||
|
||
package daemon | ||
|
||
import ( | ||
"path/filepath" | ||
"runtime" | ||
|
||
"github.com/docker/docker/daemon/images" | ||
"github.com/docker/docker/layer" | ||
|
||
// register graph drivers | ||
_ "github.com/docker/docker/daemon/graphdriver/register" | ||
"github.com/docker/docker/pkg/idtools" | ||
) | ||
|
||
// WithImageService sets imageService options | ||
func WithImageService(d *Daemon) { | ||
layerStores := make(map[string]layer.Store) | ||
os := runtime.GOOS | ||
layerStores[os], _ = layer.NewStoreFromOptions(layer.StoreOptions{ | ||
Root: d.Root, | ||
MetadataStorePathTemplate: filepath.Join(d.RootDir(), "image", "%s", "layerdb"), | ||
GraphDriver: d.storageDriver, | ||
GraphDriverOptions: nil, | ||
IDMapping: &idtools.IdentityMapping{}, | ||
PluginGetter: nil, | ||
ExperimentalEnabled: false, | ||
OS: os, | ||
}) | ||
d.imageService = images.NewImageService(images.ImageServiceConfig{ | ||
LayerStores: layerStores, | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ import ( | |
|
||
"github.com/docker/distribution" | ||
"github.com/docker/docker/pkg/ioutils" | ||
"github.com/opencontainers/go-digest" | ||
digest "github.com/opencontainers/go-digest" | ||
"github.com/pkg/errors" | ||
"github.com/sirupsen/logrus" | ||
) | ||
|
@@ -305,6 +305,55 @@ func (fms *fileMetadataStore) GetMountParent(mount string) (ChainID, error) { | |
return ChainID(dgst), nil | ||
} | ||
|
||
func (fms *fileMetadataStore) getOrphan() ([]roLayer, error) { | ||
var orphanLayers []roLayer | ||
for _, algorithm := range supportedAlgorithms { | ||
fileInfos, err := ioutil.ReadDir(filepath.Join(fms.root, string(algorithm))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are there any scenarios where the paths here may be volume GUID prefixed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. Those layer metadata folders are on format: and when deleteLayer() function is called it will rename it to: |
||
if err != nil { | ||
if os.IsNotExist(err) { | ||
continue | ||
} | ||
return nil, err | ||
} | ||
|
||
for _, fi := range fileInfos { | ||
if !fi.IsDir() || !strings.HasSuffix(fi.Name(), "-removing") { | ||
continue | ||
} | ||
// At this stage, fi.Name value looks like <digest>-<random>-removing | ||
// Split on '-' to get the digest value. | ||
nameSplit := strings.Split(fi.Name(), "-") | ||
dgst := digest.NewDigestFromEncoded(algorithm, nameSplit[0]) | ||
if err := dgst.Validate(); err != nil { | ||
logrus.WithError(err).WithField("digest", string(algorithm)+":"+nameSplit[0]).Debug("ignoring invalid digest") | ||
continue | ||
} | ||
|
||
chainFile := filepath.Join(fms.root, string(algorithm), fi.Name(), "cache-id") | ||
contentBytes, err := ioutil.ReadFile(chainFile) | ||
if err != nil { | ||
if !os.IsNotExist(err) { | ||
logrus.WithError(err).WithField("digest", dgst).Error("failed to read cache ID") | ||
} | ||
continue | ||
} | ||
cacheID := strings.TrimSpace(string(contentBytes)) | ||
if cacheID == "" { | ||
logrus.Error("invalid cache ID") | ||
continue | ||
} | ||
|
||
l := &roLayer{ | ||
chainID: ChainID(dgst), | ||
cacheID: cacheID, | ||
} | ||
orphanLayers = append(orphanLayers, *l) | ||
} | ||
} | ||
|
||
return orphanLayers, nil | ||
} | ||
|
||
func (fms *fileMetadataStore) List() ([]ChainID, []string, error) { | ||
var ids []ChainID | ||
for _, algorithm := range supportedAlgorithms { | ||
|
@@ -346,8 +395,39 @@ func (fms *fileMetadataStore) List() ([]ChainID, []string, error) { | |
return ids, mounts, nil | ||
} | ||
|
||
func (fms *fileMetadataStore) Remove(layer ChainID) error { | ||
return os.RemoveAll(fms.getLayerDirectory(layer)) | ||
// Remove layerdb folder if that is marked for removal | ||
func (fms *fileMetadataStore) Remove(layer ChainID, cache string) error { | ||
dgst := digest.Digest(layer) | ||
files, err := ioutil.ReadDir(filepath.Join(fms.root, string(dgst.Algorithm()))) | ||
if err != nil { | ||
return err | ||
} | ||
for _, f := range files { | ||
if !strings.HasSuffix(f.Name(), "-removing") || !strings.HasPrefix(f.Name(), dgst.String()) { | ||
continue | ||
} | ||
|
||
// Make sure that we only remove layerdb folder which points to | ||
// requested cacheID | ||
dir := filepath.Join(fms.root, string(dgst.Algorithm()), f.Name()) | ||
chainFile := filepath.Join(dir, "cache-id") | ||
contentBytes, err := ioutil.ReadFile(chainFile) | ||
if err != nil { | ||
logrus.WithError(err).WithField("file", chainFile).Error("cannot get cache ID") | ||
continue | ||
} | ||
cacheID := strings.TrimSpace(string(contentBytes)) | ||
if cacheID != cache { | ||
continue | ||
} | ||
logrus.Debugf("Removing folder: %s", dir) | ||
err = os.RemoveAll(dir) | ||
if err != nil && !os.IsNotExist(err) { | ||
logrus.WithError(err).WithField("name", f.Name()).Error("cannot remove layer") | ||
continue | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func (fms *fileMetadataStore) RemoveMount(mount string) 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.
Is it possible to have a unit-test around the logic in getOrphan in filestore_test.go please?
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.
Sure. Implemented it on moby#39715