Browse code

pkg/containerfs: move to internal

The only external consumer are the `graphdriver` and `graphdriver/shim`
packages in github.com/docker/go-plugins-helpers, which depended on
[ContainerFS][1], which was removed in 9ce2b30b817ad6fe6da8d91e5ac0aa19fb64e0d1.

graphdriver-plugins were deprecated in 6da604aa6a74ab770207932b3cbc3d009c3ae25f,
and support for them removed in 555dac5e14ad4925e020f1a72e8c39b7a316b0dc,
so removing this should not be an issue.

Ideally this package would've been moved inside `daemon/internal`, but it's used
by the `daemon` (cleanupContainer), `plugin` package, and by `graphdrivers`,
so needs to be in the top-level `internal/` package.

[1]: https://github.com/docker/go-plugins-helpers/blob/6eecb7beb65124bb44a23848bb46e98b4f50ae18/graphdriver/api.go#L218

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Sebastiaan van Stijn authored on 2024/07/01 01:48:51
Showing 16 changed files
... ...
@@ -17,7 +17,7 @@ import (
17 17
 	"github.com/docker/docker/container"
18 18
 	"github.com/docker/docker/daemon/config"
19 19
 	"github.com/docker/docker/errdefs"
20
-	"github.com/docker/docker/pkg/containerfs"
20
+	"github.com/docker/docker/internal/containerfs"
21 21
 	"github.com/opencontainers/selinux/go-selinux"
22 22
 	"github.com/pkg/errors"
23 23
 )
... ...
@@ -161,6 +161,8 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, config ba
161 161
 	// so that other goroutines don't attempt to concurrently open files
162 162
 	// within it. Having any file open on Windows (without the
163 163
 	// FILE_SHARE_DELETE flag) will block it from being deleted.
164
+	//
165
+	// TODO(thaJeztah): should this be moved to the "container" itself, or possibly be delegated to the graphdriver or snapshotter?
164 166
 	container.Lock()
165 167
 	err := containerfs.EnsureRemoveAll(container.Root)
166 168
 	container.Unlock()
... ...
@@ -37,7 +37,7 @@ import (
37 37
 	"github.com/containerd/containerd/pkg/userns"
38 38
 	"github.com/containerd/log"
39 39
 	"github.com/docker/docker/daemon/graphdriver"
40
-	"github.com/docker/docker/pkg/containerfs"
40
+	"github.com/docker/docker/internal/containerfs"
41 41
 	"github.com/docker/docker/pkg/idtools"
42 42
 	"github.com/docker/docker/pkg/parsers"
43 43
 	units "github.com/docker/go-units"
... ...
@@ -17,9 +17,9 @@ import (
17 17
 	"github.com/containerd/log"
18 18
 	"github.com/docker/docker/daemon/graphdriver"
19 19
 	"github.com/docker/docker/daemon/graphdriver/overlayutils"
20
+	"github.com/docker/docker/internal/containerfs"
20 21
 	"github.com/docker/docker/pkg/archive"
21 22
 	"github.com/docker/docker/pkg/chrootarchive"
22
-	"github.com/docker/docker/pkg/containerfs"
23 23
 	"github.com/docker/docker/pkg/directory"
24 24
 	"github.com/docker/docker/pkg/idtools"
25 25
 	"github.com/docker/docker/pkg/parsers/kernel"
... ...
@@ -19,9 +19,9 @@ import (
19 19
 	"github.com/containerd/log"
20 20
 	"github.com/docker/docker/daemon/graphdriver"
21 21
 	"github.com/docker/docker/daemon/graphdriver/overlayutils"
22
+	"github.com/docker/docker/internal/containerfs"
22 23
 	"github.com/docker/docker/pkg/archive"
23 24
 	"github.com/docker/docker/pkg/chrootarchive"
24
-	"github.com/docker/docker/pkg/containerfs"
25 25
 	"github.com/docker/docker/pkg/directory"
26 26
 	"github.com/docker/docker/pkg/idtools"
27 27
 	"github.com/docker/docker/pkg/ioutils"
... ...
@@ -7,7 +7,7 @@ import (
7 7
 
8 8
 	"github.com/docker/docker/daemon/graphdriver"
9 9
 	"github.com/docker/docker/errdefs"
10
-	"github.com/docker/docker/pkg/containerfs"
10
+	"github.com/docker/docker/internal/containerfs"
11 11
 	"github.com/docker/docker/pkg/idtools"
12 12
 	"github.com/docker/docker/pkg/parsers"
13 13
 	"github.com/docker/docker/quota"
14 14
new file mode 100644
... ...
@@ -0,0 +1,78 @@
0
+//go:build !darwin && !windows
1
+
2
+package containerfs
3
+
4
+import (
5
+	"os"
6
+	"syscall"
7
+	"time"
8
+
9
+	"github.com/moby/sys/mount"
10
+	"github.com/pkg/errors"
11
+)
12
+
13
+// EnsureRemoveAll wraps [os.RemoveAll] to check for specific errors that can
14
+// often be remedied.
15
+// Only use [EnsureRemoveAll] if you really want to make every effort to remove
16
+// a directory.
17
+//
18
+// Because of the way [os.Remove] (and by extension [os.RemoveAll]) works, there
19
+// can be a race between reading directory entries and then actually attempting
20
+// to remove everything in the directory.
21
+// These types of errors do not need to be returned since it's ok for the dir to
22
+// be gone we can just retry the remove operation.
23
+//
24
+// This should not return a [os.ErrNotExist] kind of error under any circumstances.
25
+func EnsureRemoveAll(dir string) error {
26
+	notExistErr := make(map[string]bool)
27
+
28
+	// track retries
29
+	exitOnErr := make(map[string]int)
30
+	maxRetry := 50
31
+
32
+	// Attempt to unmount anything beneath this dir first
33
+	mount.RecursiveUnmount(dir)
34
+
35
+	for {
36
+		err := os.RemoveAll(dir)
37
+		if err == nil {
38
+			return nil
39
+		}
40
+
41
+		pe, ok := err.(*os.PathError)
42
+		if !ok {
43
+			return err
44
+		}
45
+
46
+		if os.IsNotExist(err) {
47
+			if notExistErr[pe.Path] {
48
+				return err
49
+			}
50
+			notExistErr[pe.Path] = true
51
+
52
+			// There is a race where some subdir can be removed but after the parent
53
+			//   dir entries have been read.
54
+			// So the path could be from `os.Remove(subdir)`
55
+			// If the reported non-existent path is not the passed in `dir` we
56
+			// should just retry, but otherwise return with no error.
57
+			if pe.Path == dir {
58
+				return nil
59
+			}
60
+			continue
61
+		}
62
+
63
+		if pe.Err != syscall.EBUSY {
64
+			return err
65
+		}
66
+
67
+		if e := mount.Unmount(pe.Path); e != nil {
68
+			return errors.Wrapf(e, "error while removing %s", dir)
69
+		}
70
+
71
+		if exitOnErr[pe.Path] == maxRetry {
72
+			return err
73
+		}
74
+		exitOnErr[pe.Path]++
75
+		time.Sleep(100 * time.Millisecond)
76
+	}
77
+}
0 78
new file mode 100644
... ...
@@ -0,0 +1,36 @@
0
+//go:build !darwin
1
+
2
+package containerfs
3
+
4
+import (
5
+	"os"
6
+	"testing"
7
+)
8
+
9
+func TestEnsureRemoveAllNotExist(t *testing.T) {
10
+	// should never return an error for a non-existent path
11
+	if err := EnsureRemoveAll("/non/existent/path"); err != nil {
12
+		t.Fatal(err)
13
+	}
14
+}
15
+
16
+func TestEnsureRemoveAllWithDir(t *testing.T) {
17
+	dir, err := os.MkdirTemp("", "test-ensure-removeall-with-dir")
18
+	if err != nil {
19
+		t.Fatal(err)
20
+	}
21
+	if err := EnsureRemoveAll(dir); err != nil {
22
+		t.Fatal(err)
23
+	}
24
+}
25
+
26
+func TestEnsureRemoveAllWithFile(t *testing.T) {
27
+	tmp, err := os.CreateTemp("", "test-ensure-removeall-with-dir")
28
+	if err != nil {
29
+		t.Fatal(err)
30
+	}
31
+	tmp.Close()
32
+	if err := EnsureRemoveAll(tmp.Name()); err != nil {
33
+		t.Fatal(err)
34
+	}
35
+}
0 36
new file mode 100644
... ...
@@ -0,0 +1,56 @@
0
+//go:build !darwin && !windows
1
+
2
+package containerfs
3
+
4
+import (
5
+	"os"
6
+	"path/filepath"
7
+	"testing"
8
+	"time"
9
+
10
+	"github.com/moby/sys/mount"
11
+)
12
+
13
+func TestEnsureRemoveAllWithMount(t *testing.T) {
14
+	if os.Getuid() != 0 {
15
+		t.Skip("skipping test that requires root")
16
+	}
17
+
18
+	dir1, err := os.MkdirTemp("", "test-ensure-removeall-with-dir1")
19
+	if err != nil {
20
+		t.Fatal(err)
21
+	}
22
+	dir2, err := os.MkdirTemp("", "test-ensure-removeall-with-dir2")
23
+	if err != nil {
24
+		t.Fatal(err)
25
+	}
26
+	defer os.RemoveAll(dir2)
27
+
28
+	bindDir := filepath.Join(dir1, "bind")
29
+	if err := os.MkdirAll(bindDir, 0o755); err != nil {
30
+		t.Fatal(err)
31
+	}
32
+
33
+	if err := mount.Mount(dir2, bindDir, "none", "bind"); err != nil {
34
+		t.Fatal(err)
35
+	}
36
+
37
+	done := make(chan struct{}, 1)
38
+	go func() {
39
+		err = EnsureRemoveAll(dir1)
40
+		close(done)
41
+	}()
42
+
43
+	select {
44
+	case <-done:
45
+		if err != nil {
46
+			t.Fatal(err)
47
+		}
48
+	case <-time.After(5 * time.Second):
49
+		t.Fatal("timeout waiting for EnsureRemoveAll to finish")
50
+	}
51
+
52
+	if _, err := os.Stat(dir1); !os.IsNotExist(err) {
53
+		t.Fatalf("expected %q to not exist", dir1)
54
+	}
55
+}
0 56
new file mode 100644
... ...
@@ -0,0 +1,8 @@
0
+package containerfs
1
+
2
+import "os"
3
+
4
+// EnsureRemoveAll is an alias to [os.RemoveAll] on Windows.
5
+func EnsureRemoveAll(path string) error {
6
+	return os.RemoveAll(path)
7
+}
0 8
deleted file mode 100644
... ...
@@ -1,78 +0,0 @@
1
-//go:build !darwin && !windows
2
-
3
-package containerfs
4
-
5
-import (
6
-	"os"
7
-	"syscall"
8
-	"time"
9
-
10
-	"github.com/moby/sys/mount"
11
-	"github.com/pkg/errors"
12
-)
13
-
14
-// EnsureRemoveAll wraps [os.RemoveAll] to check for specific errors that can
15
-// often be remedied.
16
-// Only use [EnsureRemoveAll] if you really want to make every effort to remove
17
-// a directory.
18
-//
19
-// Because of the way [os.Remove] (and by extension [os.RemoveAll]) works, there
20
-// can be a race between reading directory entries and then actually attempting
21
-// to remove everything in the directory.
22
-// These types of errors do not need to be returned since it's ok for the dir to
23
-// be gone we can just retry the remove operation.
24
-//
25
-// This should not return a [os.ErrNotExist] kind of error under any circumstances.
26
-func EnsureRemoveAll(dir string) error {
27
-	notExistErr := make(map[string]bool)
28
-
29
-	// track retries
30
-	exitOnErr := make(map[string]int)
31
-	maxRetry := 50
32
-
33
-	// Attempt to unmount anything beneath this dir first
34
-	mount.RecursiveUnmount(dir)
35
-
36
-	for {
37
-		err := os.RemoveAll(dir)
38
-		if err == nil {
39
-			return nil
40
-		}
41
-
42
-		pe, ok := err.(*os.PathError)
43
-		if !ok {
44
-			return err
45
-		}
46
-
47
-		if os.IsNotExist(err) {
48
-			if notExistErr[pe.Path] {
49
-				return err
50
-			}
51
-			notExistErr[pe.Path] = true
52
-
53
-			// There is a race where some subdir can be removed but after the parent
54
-			//   dir entries have been read.
55
-			// So the path could be from `os.Remove(subdir)`
56
-			// If the reported non-existent path is not the passed in `dir` we
57
-			// should just retry, but otherwise return with no error.
58
-			if pe.Path == dir {
59
-				return nil
60
-			}
61
-			continue
62
-		}
63
-
64
-		if pe.Err != syscall.EBUSY {
65
-			return err
66
-		}
67
-
68
-		if e := mount.Unmount(pe.Path); e != nil {
69
-			return errors.Wrapf(e, "error while removing %s", dir)
70
-		}
71
-
72
-		if exitOnErr[pe.Path] == maxRetry {
73
-			return err
74
-		}
75
-		exitOnErr[pe.Path]++
76
-		time.Sleep(100 * time.Millisecond)
77
-	}
78
-}
79 1
deleted file mode 100644
... ...
@@ -1,36 +0,0 @@
1
-//go:build !darwin
2
-
3
-package containerfs
4
-
5
-import (
6
-	"os"
7
-	"testing"
8
-)
9
-
10
-func TestEnsureRemoveAllNotExist(t *testing.T) {
11
-	// should never return an error for a non-existent path
12
-	if err := EnsureRemoveAll("/non/existent/path"); err != nil {
13
-		t.Fatal(err)
14
-	}
15
-}
16
-
17
-func TestEnsureRemoveAllWithDir(t *testing.T) {
18
-	dir, err := os.MkdirTemp("", "test-ensure-removeall-with-dir")
19
-	if err != nil {
20
-		t.Fatal(err)
21
-	}
22
-	if err := EnsureRemoveAll(dir); err != nil {
23
-		t.Fatal(err)
24
-	}
25
-}
26
-
27
-func TestEnsureRemoveAllWithFile(t *testing.T) {
28
-	tmp, err := os.CreateTemp("", "test-ensure-removeall-with-dir")
29
-	if err != nil {
30
-		t.Fatal(err)
31
-	}
32
-	tmp.Close()
33
-	if err := EnsureRemoveAll(tmp.Name()); err != nil {
34
-		t.Fatal(err)
35
-	}
36
-}
37 1
deleted file mode 100644
... ...
@@ -1,56 +0,0 @@
1
-//go:build !darwin && !windows
2
-
3
-package containerfs
4
-
5
-import (
6
-	"os"
7
-	"path/filepath"
8
-	"testing"
9
-	"time"
10
-
11
-	"github.com/moby/sys/mount"
12
-)
13
-
14
-func TestEnsureRemoveAllWithMount(t *testing.T) {
15
-	if os.Getuid() != 0 {
16
-		t.Skip("skipping test that requires root")
17
-	}
18
-
19
-	dir1, err := os.MkdirTemp("", "test-ensure-removeall-with-dir1")
20
-	if err != nil {
21
-		t.Fatal(err)
22
-	}
23
-	dir2, err := os.MkdirTemp("", "test-ensure-removeall-with-dir2")
24
-	if err != nil {
25
-		t.Fatal(err)
26
-	}
27
-	defer os.RemoveAll(dir2)
28
-
29
-	bindDir := filepath.Join(dir1, "bind")
30
-	if err := os.MkdirAll(bindDir, 0o755); err != nil {
31
-		t.Fatal(err)
32
-	}
33
-
34
-	if err := mount.Mount(dir2, bindDir, "none", "bind"); err != nil {
35
-		t.Fatal(err)
36
-	}
37
-
38
-	done := make(chan struct{}, 1)
39
-	go func() {
40
-		err = EnsureRemoveAll(dir1)
41
-		close(done)
42
-	}()
43
-
44
-	select {
45
-	case <-done:
46
-		if err != nil {
47
-			t.Fatal(err)
48
-		}
49
-	case <-time.After(5 * time.Second):
50
-		t.Fatal("timeout waiting for EnsureRemoveAll to finish")
51
-	}
52
-
53
-	if _, err := os.Stat(dir1); !os.IsNotExist(err) {
54
-		t.Fatalf("expected %q to not exist", dir1)
55
-	}
56
-}
57 1
deleted file mode 100644
... ...
@@ -1,8 +0,0 @@
1
-package containerfs
2
-
3
-import "os"
4
-
5
-// EnsureRemoveAll is an alias to [os.RemoveAll] on Windows.
6
-func EnsureRemoveAll(path string) error {
7
-	return os.RemoveAll(path)
8
-}
... ...
@@ -29,9 +29,9 @@ import (
29 29
 	"github.com/docker/docker/api/types/registry"
30 30
 	"github.com/docker/docker/dockerversion"
31 31
 	"github.com/docker/docker/errdefs"
32
+	"github.com/docker/docker/internal/containerfs"
32 33
 	"github.com/docker/docker/pkg/authorization"
33 34
 	"github.com/docker/docker/pkg/chrootarchive"
34
-	"github.com/docker/docker/pkg/containerfs"
35 35
 	"github.com/docker/docker/pkg/pools"
36 36
 	"github.com/docker/docker/pkg/progress"
37 37
 	"github.com/docker/docker/pkg/stringid"
... ...
@@ -18,8 +18,8 @@ import (
18 18
 	"github.com/containerd/log"
19 19
 	"github.com/docker/docker/api/types"
20 20
 	"github.com/docker/docker/api/types/events"
21
+	"github.com/docker/docker/internal/containerfs"
21 22
 	"github.com/docker/docker/pkg/authorization"
22
-	"github.com/docker/docker/pkg/containerfs"
23 23
 	"github.com/docker/docker/pkg/ioutils"
24 24
 	v2 "github.com/docker/docker/plugin/v2"
25 25
 	"github.com/docker/docker/registry"
... ...
@@ -11,7 +11,7 @@ import (
11 11
 	"github.com/docker/docker/api/types"
12 12
 	"github.com/docker/docker/api/types/backend"
13 13
 	"github.com/docker/docker/api/types/events"
14
-	"github.com/docker/docker/pkg/containerfs"
14
+	"github.com/docker/docker/internal/containerfs"
15 15
 	"github.com/docker/docker/pkg/stringid"
16 16
 	v2 "github.com/docker/docker/plugin/v2"
17 17
 	"github.com/moby/sys/mount"