Skip to content

Commit bb97d74

Browse files
olljanatolljanat
committed
Cherry-picked docker-archive#268 
1 parent 16e2c8a commit bb97d74

7 files changed

Lines changed: 309 additions & 6 deletions

File tree

‎integration/image/remove_unix_test.go‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// +build !windows
2+
3+
package image // import "github.com/docker/docker/integration/image"
4+
5+
import (
6+
"context"
7+
"io"
8+
"io/ioutil"
9+
"os"
10+
"strconv"
11+
"strings"
12+
"syscall"
13+
"testing"
14+
"unsafe"
15+
16+
"github.com/docker/docker/api/types"
17+
"github.com/docker/docker/internal/test/daemon"
18+
"github.com/docker/docker/internal/test/fakecontext"
19+
"gotest.tools/assert"
20+
"gotest.tools/skip"
21+
)
22+
23+
// This is a regression test for #38488
24+
// It ensures that orphan layers can be found and cleaned up
25+
// after unsuccessful image removal
26+
func TestRemoveImageGarbageCollector(t *testing.T) {
27+
// This test uses very platform specific way to prevent
28+
// daemon for remove image layer.
29+
skip.If(t, testEnv.DaemonInfo.OSType != "linux")
30+
skip.If(t, os.Getenv("DOCKER_ENGINE_GOARCH") != "amd64")
31+
32+
// Create daemon with overlay2 graphdriver because vfs uses disk differently
33+
// and this test case would not work with it.
34+
d := daemon.New(t, daemon.WithStorageDriver("overlay2"), daemon.WithImageService)
35+
d.Start(t)
36+
defer d.Stop(t)
37+
38+
ctx := context.Background()
39+
client := d.NewClientT(t)
40+
i := d.ImageService()
41+
42+
img := "test-garbage-collector"
43+
44+
// Build a image with multiple layers
45+
dockerfile := `FROM busybox
46+
RUN echo echo Running... > /run.sh`
47+
source := fakecontext.New(t, "", fakecontext.WithDockerfile(dockerfile))
48+
defer source.Close()
49+
resp, err := client.ImageBuild(ctx,
50+
source.AsTarReader(t),
51+
types.ImageBuildOptions{
52+
Remove: true,
53+
ForceRemove: true,
54+
Tags: []string{img},
55+
})
56+
assert.NilError(t, err)
57+
_, err = io.Copy(ioutil.Discard, resp.Body)
58+
resp.Body.Close()
59+
assert.NilError(t, err)
60+
image, _, err := client.ImageInspectWithRaw(ctx, img)
61+
assert.NilError(t, err)
62+
63+
// Mark latest image layer to immutable
64+
data := image.GraphDriver.Data
65+
file, _ := os.Open(data["UpperDir"])
66+
attr := 0x00000010
67+
fsflags := uintptr(0x40086602)
68+
argp := uintptr(unsafe.Pointer(&attr))
69+
_, _, errno := syscall.Syscall(syscall.SYS_IOCTL, file.Fd(), fsflags, argp)
70+
assert.Equal(t, "errno 0", errno.Error())
71+
72+
// Try to remove the image, it should generate error
73+
// but marking layer back to mutable before checking errors (so we don't break CI server)
74+
_, err = client.ImageRemove(ctx, img, types.ImageRemoveOptions{})
75+
attr = 0x00000000
76+
argp = uintptr(unsafe.Pointer(&attr))
77+
_, _, errno = syscall.Syscall(syscall.SYS_IOCTL, file.Fd(), fsflags, argp)
78+
assert.Equal(t, "errno 0", errno.Error())
79+
assert.Assert(t, err != nil)
80+
errStr := err.Error()
81+
if !(strings.Contains(errStr, "permission denied") || strings.Contains(errStr, "operation not permitted")) {
82+
t.Errorf("ImageRemove error not an permission error %s", errStr)
83+
}
84+
85+
// Verify that layer remaining on disk
86+
dir, _ := os.Stat(data["UpperDir"])
87+
assert.Equal(t, "true", strconv.FormatBool(dir.IsDir()))
88+
89+
// Run imageService.Cleanup() and make sure that layer was removed from disk
90+
i.Cleanup()
91+
dir, err = os.Stat(data["UpperDir"])
92+
assert.ErrorContains(t, err, "no such file or directory")
93+
}

‎internal/test/daemon/daemon.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/docker/docker/api/types"
1717
"github.com/docker/docker/api/types/events"
1818
"github.com/docker/docker/client"
19+
"github.com/docker/docker/daemon/images"
1920
"github.com/docker/docker/internal/test"
2021
"github.com/docker/docker/internal/test/request"
2122
"github.com/docker/docker/opts"
@@ -77,6 +78,7 @@ type Daemon struct {
7778
init bool
7879
dockerdBinary string
7980
log logT
81+
imageService *images.ImageService
8082

8183
// swarm related field
8284
swarmListenAddr string
@@ -720,3 +722,8 @@ func cleanupRaftDir(t testingT, rootPath string) {
720722
}
721723
}
722724
}
725+
726+
// ImageService returns the Daemon's ImageService
727+
func (d *Daemon) ImageService() *images.ImageService {
728+
return d.imageService
729+
}

‎internal/test/daemon/ops.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,10 @@ func WithEnvironment(e environment.Execution) func(*Daemon) {
6363
}
6464
}
6565
}
66+
67+
// WithStorageDriver sets store driver option
68+
func WithStorageDriver(driver string) func(d *Daemon) {
69+
return func(d *Daemon) {
70+
d.storageDriver = driver
71+
}
72+
}

‎internal/test/daemon/ops_unix.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// +build !windows
2+
3+
package daemon
4+
5+
import (
6+
"path/filepath"
7+
"runtime"
8+
9+
"github.com/docker/docker/daemon/images"
10+
"github.com/docker/docker/layer"
11+
12+
// register graph drivers
13+
_ "github.com/docker/docker/daemon/graphdriver/register"
14+
"github.com/docker/docker/pkg/idtools"
15+
)
16+
17+
// WithImageService sets imageService options
18+
func WithImageService(d *Daemon) {
19+
layerStores := make(map[string]layer.Store)
20+
os := runtime.GOOS
21+
layerStores[os], _ = layer.NewStoreFromOptions(layer.StoreOptions{
22+
Root: d.Root,
23+
MetadataStorePathTemplate: filepath.Join(d.RootDir(), "image", "%s", "layerdb"),
24+
GraphDriver: d.storageDriver,
25+
GraphDriverOptions: nil,
26+
IDMapping: &idtools.IdentityMapping{},
27+
PluginGetter: nil,
28+
ExperimentalEnabled: false,
29+
OS: os,
30+
})
31+
d.imageService = images.NewImageService(images.ImageServiceConfig{
32+
LayerStores: layerStores,
33+
})
34+
}

‎layer/filestore.go‎

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414

1515
"github.com/docker/distribution"
1616
"github.com/docker/docker/pkg/ioutils"
17-
"github.com/opencontainers/go-digest"
17+
digest "github.com/opencontainers/go-digest"
1818
"github.com/pkg/errors"
1919
"github.com/sirupsen/logrus"
2020
)
@@ -305,6 +305,55 @@ func (fms *fileMetadataStore) GetMountParent(mount string) (ChainID, error) {
305305
return ChainID(dgst), nil
306306
}
307307

308+
func (fms *fileMetadataStore) getOrphan() ([]roLayer, error) {
309+
var orphanLayers []roLayer
310+
for _, algorithm := range supportedAlgorithms {
311+
fileInfos, err := ioutil.ReadDir(filepath.Join(fms.root, string(algorithm)))
312+
if err != nil {
313+
if os.IsNotExist(err) {
314+
continue
315+
}
316+
return nil, err
317+
}
318+
319+
for _, fi := range fileInfos {
320+
if !fi.IsDir() || !strings.HasSuffix(fi.Name(), "-removing") {
321+
continue
322+
}
323+
// At this stage, fi.Name value looks like <digest>-<random>-removing
324+
// Split on '-' to get the digest value.
325+
nameSplit := strings.Split(fi.Name(), "-")
326+
dgst := digest.NewDigestFromEncoded(algorithm, nameSplit[0])
327+
if err := dgst.Validate(); err != nil {
328+
logrus.WithError(err).WithField("digest", string(algorithm)+":"+nameSplit[0]).Debug("ignoring invalid digest")
329+
continue
330+
}
331+
332+
chainFile := filepath.Join(fms.root, string(algorithm), fi.Name(), "cache-id")
333+
contentBytes, err := ioutil.ReadFile(chainFile)
334+
if err != nil {
335+
if !os.IsNotExist(err) {
336+
logrus.WithError(err).WithField("digest", dgst).Error("failed to read cache ID")
337+
}
338+
continue
339+
}
340+
cacheID := strings.TrimSpace(string(contentBytes))
341+
if cacheID == "" {
342+
logrus.Error("invalid cache ID")
343+
continue
344+
}
345+
346+
l := &roLayer{
347+
chainID: ChainID(dgst),
348+
cacheID: cacheID,
349+
}
350+
orphanLayers = append(orphanLayers, *l)
351+
}
352+
}
353+
354+
return orphanLayers, nil
355+
}
356+
308357
func (fms *fileMetadataStore) List() ([]ChainID, []string, error) {
309358
var ids []ChainID
310359
for _, algorithm := range supportedAlgorithms {
@@ -346,8 +395,39 @@ func (fms *fileMetadataStore) List() ([]ChainID, []string, error) {
346395
return ids, mounts, nil
347396
}
348397

349-
func (fms *fileMetadataStore) Remove(layer ChainID) error {
350-
return os.RemoveAll(fms.getLayerDirectory(layer))
398+
// Remove layerdb folder if that is marked for removal
399+
func (fms *fileMetadataStore) Remove(layer ChainID, cache string) error {
400+
dgst := digest.Digest(layer)
401+
files, err := ioutil.ReadDir(filepath.Join(fms.root, string(dgst.Algorithm())))
402+
if err != nil {
403+
return err
404+
}
405+
for _, f := range files {
406+
if !strings.HasSuffix(f.Name(), "-removing") || !strings.HasPrefix(f.Name(), dgst.String()) {
407+
continue
408+
}
409+
410+
// Make sure that we only remove layerdb folder which points to
411+
// requested cacheID
412+
dir := filepath.Join(fms.root, string(dgst.Algorithm()), f.Name())
413+
chainFile := filepath.Join(dir, "cache-id")
414+
contentBytes, err := ioutil.ReadFile(chainFile)
415+
if err != nil {
416+
logrus.WithError(err).WithField("file", chainFile).Error("cannot get cache ID")
417+
continue
418+
}
419+
cacheID := strings.TrimSpace(string(contentBytes))
420+
if cacheID != cache {
421+
continue
422+
}
423+
logrus.Debugf("Removing folder: %s", dir)
424+
err = os.RemoveAll(dir)
425+
if err != nil && !os.IsNotExist(err) {
426+
logrus.WithError(err).WithField("name", f.Name()).Error("cannot remove layer")
427+
continue
428+
}
429+
}
430+
return nil
351431
}
352432

353433
func (fms *fileMetadataStore) RemoveMount(mount string) error {

‎layer/filestore_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"syscall"
1111
"testing"
1212

13+
"github.com/docker/docker/pkg/stringid"
1314
"github.com/opencontainers/go-digest"
1415
)
1516

@@ -102,3 +103,50 @@ func TestStartTransactionFailure(t *testing.T) {
102103
t.Fatal(err)
103104
}
104105
}
106+
107+
func TestGetOrphan(t *testing.T) {
108+
fms, td, cleanup := newFileMetadataStore(t)
109+
defer cleanup()
110+
111+
layerRoot := filepath.Join(td, "sha256")
112+
if err := os.MkdirAll(layerRoot, 0755); err != nil {
113+
t.Fatal(err)
114+
}
115+
116+
tx, err := fms.StartTransaction()
117+
if err != nil {
118+
t.Fatal(err)
119+
}
120+
121+
layerid := randomLayerID(5)
122+
err = tx.Commit(layerid)
123+
if err != nil {
124+
t.Fatal(err)
125+
}
126+
layerPath := fms.getLayerDirectory(layerid)
127+
if err := ioutil.WriteFile(filepath.Join(layerPath, "cache-id"), []byte(stringid.GenerateRandomID()), 0644); err != nil {
128+
t.Fatal(err)
129+
}
130+
131+
orphanLayers, err := fms.getOrphan()
132+
if err != nil {
133+
t.Fatal(err)
134+
}
135+
if len(orphanLayers) != 0 {
136+
t.Fatalf("Expected to have zero orphan layers")
137+
}
138+
139+
layeridSplit := strings.Split(layerid.String(), ":")
140+
newPath := filepath.Join(layerRoot, fmt.Sprintf("%s-%s-removing", layeridSplit[1], stringid.GenerateRandomID()))
141+
err = os.Rename(layerPath, newPath)
142+
if err != nil {
143+
t.Fatal(err)
144+
}
145+
orphanLayers, err = fms.getOrphan()
146+
if err != nil {
147+
t.Fatal(err)
148+
}
149+
if len(orphanLayers) != 1 {
150+
t.Fatalf("Expected to have one orphan layer")
151+
}
152+
}

0 commit comments

Comments
 (0)