From 769b79866aa645d4deeeb0a44120cde7b046f0d1 Mon Sep 17 00:00:00 2001 From: unclejack Date: Thu, 20 Nov 2014 19:33:15 +0200 Subject: [PATCH 1/6] pkg/system: fix cleanup in tests Docker-DCO-1.1-Signed-off-by: Cristian Staretu (github: unclejack) --- pkg/system/lstat_test.go | 4 +++- pkg/system/stat_test.go | 4 +++- pkg/system/utimes_test.go | 7 ++++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/system/lstat_test.go b/pkg/system/lstat_test.go index 7e271efea5..9bab4d7b0c 100644 --- a/pkg/system/lstat_test.go +++ b/pkg/system/lstat_test.go @@ -1,11 +1,13 @@ package system import ( + "os" "testing" ) func TestLstat(t *testing.T) { - file, invalid, _ := prepareFiles(t) + file, invalid, _, dir := prepareFiles(t) + defer os.RemoveAll(dir) statFile, err := Lstat(file) if err != nil { diff --git a/pkg/system/stat_test.go b/pkg/system/stat_test.go index 0dcb239ece..abcc8ea7a6 100644 --- a/pkg/system/stat_test.go +++ b/pkg/system/stat_test.go @@ -1,12 +1,14 @@ package system import ( + "os" "syscall" "testing" ) func TestFromStatT(t *testing.T) { - file, _, _ := prepareFiles(t) + file, _, _, dir := prepareFiles(t) + defer os.RemoveAll(dir) stat := &syscall.Stat_t{} err := syscall.Lstat(file, stat) diff --git a/pkg/system/utimes_test.go b/pkg/system/utimes_test.go index 38e4020cb5..1dea47cc15 100644 --- a/pkg/system/utimes_test.go +++ b/pkg/system/utimes_test.go @@ -8,7 +8,7 @@ import ( "testing" ) -func prepareFiles(t *testing.T) (string, string, string) { +func prepareFiles(t *testing.T) (string, string, string, string) { dir, err := ioutil.TempDir("", "docker-system-test") if err != nil { t.Fatal(err) @@ -26,11 +26,12 @@ func prepareFiles(t *testing.T) (string, string, string) { t.Fatal(err) } - return file, invalid, symlink + return file, invalid, symlink, dir } func TestLUtimesNano(t *testing.T) { - file, invalid, symlink := prepareFiles(t) + file, invalid, symlink, dir := prepareFiles(t) + defer os.RemoveAll(dir) before, err := os.Stat(file) if err != nil { From 32ba6ab83c7e47d627a2b971e7f6ca9b56e1be85 Mon Sep 17 00:00:00 2001 From: unclejack Date: Thu, 20 Nov 2014 19:34:35 +0200 Subject: [PATCH 2/6] pkg/archive: fix TempArchive cleanup w/ one read This fixes the removal of TempArchives which can read with only one read. Such archives weren't getting removed because EOF wasn't being triggered. Docker-DCO-1.1-Signed-off-by: Cristian Staretu (github: unclejack) --- pkg/archive/archive.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index 5a81223dbd..995668104d 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -742,17 +742,20 @@ func NewTempArchive(src Archive, dir string) (*TempArchive, error) { return nil, err } size := st.Size() - return &TempArchive{f, size}, nil + return &TempArchive{f, size, 0}, nil } type TempArchive struct { *os.File Size int64 // Pre-computed from Stat().Size() as a convenience + read int64 } func (archive *TempArchive) Read(data []byte) (int, error) { n, err := archive.File.Read(data) - if err != nil { + archive.read += int64(n) + if err != nil || archive.read == archive.Size { + archive.File.Close() os.Remove(archive.File.Name()) } return n, err From 4508bd94b0efd07a0ef48cd090786615e6b8cbb7 Mon Sep 17 00:00:00 2001 From: unclejack Date: Thu, 20 Nov 2014 19:36:54 +0200 Subject: [PATCH 3/6] pkg/symlink: fix cleanup for tests Docker-DCO-1.1-Signed-off-by: Cristian Staretu (github: unclejack) --- pkg/symlink/fs_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/symlink/fs_test.go b/pkg/symlink/fs_test.go index d85fd6da74..cc0d82d1a3 100644 --- a/pkg/symlink/fs_test.go +++ b/pkg/symlink/fs_test.go @@ -46,6 +46,7 @@ func TestFollowSymLinkUnderLinkedDir(t *testing.T) { if err != nil { t.Fatal(err) } + defer os.RemoveAll(dir) os.Mkdir(filepath.Join(dir, "realdir"), 0700) os.Symlink("realdir", filepath.Join(dir, "linkdir")) From 98307c8faefca5c4347288af18aee4dacbf8802c Mon Sep 17 00:00:00 2001 From: unclejack Date: Thu, 20 Nov 2014 19:37:46 +0200 Subject: [PATCH 4/6] integ-cli: fix cleanup in test which mounts tmpfs Docker-DCO-1.1-Signed-off-by: Cristian Staretu (github: unclejack) --- integration-cli/docker_cli_run_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index ca44aa3902..911861e8ac 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -1257,6 +1257,7 @@ func TestRunWithVolumesIsRecursive(t *testing.T) { if err := mount.Mount("tmpfs", tmpfsDir, "tmpfs", ""); err != nil { t.Fatalf("failed to create a tmpfs mount at %s - %s", tmpfsDir, err) } + defer mount.Unmount(tmpfsDir) f, err := ioutil.TempFile(tmpfsDir, "touch-me") if err != nil { From db7fded17fd984fc3c854d1e34bd8d656c3b3692 Mon Sep 17 00:00:00 2001 From: unclejack Date: Thu, 20 Nov 2014 19:38:41 +0200 Subject: [PATCH 5/6] integ-cli: fix cleanup in build tests Docker-DCO-1.1-Signed-off-by: Cristian Staretu (github: unclejack) --- integration-cli/docker_cli_build_test.go | 43 ++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 4e6fe63ae1..32b568b8c9 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -179,6 +179,7 @@ func TestBuildEnvironmentReplacementAddCopy(t *testing.T) { if err != nil { t.Fatal(err) } + defer ctx.Close() if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) @@ -632,6 +633,8 @@ func TestBuildSixtySteps(t *testing.T) { if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -656,6 +659,8 @@ RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -674,6 +679,8 @@ ADD test_file .`, if err != nil { t.Fatal(err) } + defer ctx.Close() + done := make(chan struct{}) go func() { if _, err := buildImageFromContext(name, ctx, true); err != nil { @@ -708,6 +715,8 @@ RUN [ $(ls -l /exists/exists_file | awk '{print $3":"$4}') = 'dockerio:dockerio' if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -947,6 +956,8 @@ RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -971,6 +982,8 @@ RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -996,6 +1009,8 @@ RUN [ $(ls -l /exists/test_file | awk '{print $3":"$4}') = 'root:root' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1022,6 +1037,8 @@ RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1040,6 +1057,8 @@ ADD . /`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1064,6 +1083,8 @@ RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1082,6 +1103,8 @@ COPY test_file .`, if err != nil { t.Fatal(err) } + defer ctx.Close() + done := make(chan struct{}) go func() { if _, err := buildImageFromContext(name, ctx, true); err != nil { @@ -1116,6 +1139,8 @@ RUN [ $(ls -l /exists/exists_file | awk '{print $3":"$4}') = 'dockerio:dockerio' if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1140,6 +1165,8 @@ RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1163,6 +1190,8 @@ RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1188,6 +1217,8 @@ RUN [ $(ls -l /exists/test_file | awk '{print $3":"$4}') = 'root:root' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1214,6 +1245,8 @@ RUN [ $(ls -l /exists | awk '{print $3":"$4}') = 'dockerio:dockerio' ]`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1231,6 +1264,8 @@ COPY . /`, if err != nil { t.Fatal(err) } + defer ctx.Close() + if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatal(err) } @@ -1858,6 +1893,7 @@ func TestBuildOnBuildLimitedInheritence(t *testing.T) { if err != nil { t.Fatal(err) } + defer ctx.Close() out1, _, err := dockerCmdInDir(t, ctx.Dir, "build", "-t", name1, ".") if err != nil { @@ -1874,6 +1910,7 @@ func TestBuildOnBuildLimitedInheritence(t *testing.T) { if err != nil { t.Fatal(err) } + defer ctx.Close() out2, _, err = dockerCmdInDir(t, ctx.Dir, "build", "-t", name2, ".") if err != nil { @@ -1890,6 +1927,7 @@ func TestBuildOnBuildLimitedInheritence(t *testing.T) { if err != nil { t.Fatal(err) } + defer ctx.Close() out3, _, err = dockerCmdInDir(t, ctx.Dir, "build", "-t", name3, ".") if err != nil { @@ -2984,6 +3022,8 @@ RUN [ "$(cat $TO)" = "hello" ] if err != nil { t.Fatal(err) } + defer ctx.Close() + _, err = buildImageFromContext(name, ctx, true) if err != nil { t.Fatal(err) @@ -3006,6 +3046,8 @@ RUN [ "$(cat /testfile)" = 'test!' ]` if err != nil { t.Fatal(err) } + defer ctx.Close() + _, err = buildImageFromContext(name, ctx, true) if err != nil { t.Fatal(err) @@ -3060,6 +3102,7 @@ RUN cat /existing-directory-trailing-slash/test/foo | grep Hi` } return &FakeContext{Dir: tmpDir} }() + defer ctx.Close() if _, err := buildImageFromContext(name, ctx, true); err != nil { t.Fatalf("build failed to complete for TestBuildAddTar: %v", err) From 4180579313e84ea7e3d85214521a815e95459a90 Mon Sep 17 00:00:00 2001 From: unclejack Date: Thu, 20 Nov 2014 19:39:08 +0200 Subject: [PATCH 6/6] graphdriver/aufs: fix tmp cleanup in tests Docker-DCO-1.1-Signed-off-by: Cristian Staretu (github: unclejack) --- daemon/graphdriver/aufs/aufs_test.go | 34 ++++++++++++++++------------ 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/daemon/graphdriver/aufs/aufs_test.go b/daemon/graphdriver/aufs/aufs_test.go index 971d448af8..e1ed64985f 100644 --- a/daemon/graphdriver/aufs/aufs_test.go +++ b/daemon/graphdriver/aufs/aufs_test.go @@ -4,16 +4,18 @@ import ( "crypto/sha256" "encoding/hex" "fmt" - "github.com/docker/docker/daemon/graphdriver" - "github.com/docker/docker/pkg/archive" "io/ioutil" "os" "path" "testing" + + "github.com/docker/docker/daemon/graphdriver" + "github.com/docker/docker/pkg/archive" ) var ( - tmp = path.Join(os.TempDir(), "aufs-tests", "aufs") + tmpOuter = path.Join(os.TempDir(), "aufs-tests") + tmp = path.Join(tmpOuter, "aufs") ) func testInit(dir string, t *testing.T) graphdriver.Driver { @@ -640,8 +642,8 @@ func testMountMoreThan42Layers(t *testing.T, mountPath string) { t.Fatal(err) } - d := testInit(mountPath, t).(*Driver) defer os.RemoveAll(mountPath) + d := testInit(mountPath, t).(*Driver) defer d.Cleanup() var last string var expected int @@ -662,24 +664,24 @@ func testMountMoreThan42Layers(t *testing.T, mountPath string) { if err := d.Create(current, parent); err != nil { t.Logf("Current layer %d", i) - t.Fatal(err) + t.Error(err) } point, err := d.Get(current, "") if err != nil { t.Logf("Current layer %d", i) - t.Fatal(err) + t.Error(err) } f, err := os.Create(path.Join(point, current)) if err != nil { t.Logf("Current layer %d", i) - t.Fatal(err) + t.Error(err) } f.Close() if i%10 == 0 { if err := os.Remove(path.Join(point, parent)); err != nil { t.Logf("Current layer %d", i) - t.Fatal(err) + t.Error(err) } expected-- } @@ -689,28 +691,30 @@ func testMountMoreThan42Layers(t *testing.T, mountPath string) { // Perform the actual mount for the top most image point, err := d.Get(last, "") if err != nil { - t.Fatal(err) + t.Error(err) } files, err := ioutil.ReadDir(point) if err != nil { - t.Fatal(err) + t.Error(err) } if len(files) != expected { - t.Fatalf("Expected %d got %d", expected, len(files)) + t.Errorf("Expected %d got %d", expected, len(files)) } } func TestMountMoreThan42Layers(t *testing.T) { + os.RemoveAll(tmpOuter) testMountMoreThan42Layers(t, tmp) } func TestMountMoreThan42LayersMatchingPathLength(t *testing.T) { - tmp := "aufs-tests" + defer os.RemoveAll(tmpOuter) + zeroes := "0" for { // This finds a mount path so that when combined into aufs mount options // 4096 byte boundary would be in between the paths or in permission - // section. For '/tmp' it will use '/tmp/aufs-tests00000000/aufs' - mountPath := path.Join(os.TempDir(), tmp, "aufs") + // section. For '/tmp' it will use '/tmp/aufs-tests/00000000/aufs' + mountPath := path.Join(tmpOuter, zeroes, "aufs") pathLength := 77 + len(mountPath) if mod := 4095 % pathLength; mod == 0 || mod > pathLength-2 { @@ -718,6 +722,6 @@ func TestMountMoreThan42LayersMatchingPathLength(t *testing.T) { testMountMoreThan42Layers(t, mountPath) return } - tmp += "0" + zeroes += "0" } }