From 9ff169ccf421c00f3106481e43c5d86f77403b06 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Mon, 5 Jun 2023 18:30:30 -0400 Subject: [PATCH] daemon: modernize oci_linux_test.go Switch to using t.TempDir() instead of rolling our own. Clean up mounts leaked by the tests as otherwise the tests fail due to the leaked mounts because unlike the old cleanup code, t.TempDir() cleanup does not ignore errors from os.RemoveAll. Signed-off-by: Cory Snider --- daemon/oci_linux_test.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/daemon/oci_linux_test.go b/daemon/oci_linux_test.go index 072c0bfba1..ed9fd50d00 100644 --- a/daemon/oci_linux_test.go +++ b/daemon/oci_linux_test.go @@ -11,17 +11,18 @@ import ( "github.com/docker/docker/daemon/config" "github.com/docker/docker/daemon/network" "github.com/docker/docker/libnetwork" + "golang.org/x/sys/unix" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" "gotest.tools/v3/skip" ) func setupFakeDaemon(t *testing.T, c *container.Container) *Daemon { - root, err := os.MkdirTemp("", "oci_linux_test-root") - assert.NilError(t, err) + t.Helper() + root := t.TempDir() rootfs := filepath.Join(root, "rootfs") - err = os.MkdirAll(rootfs, 0755) + err := os.MkdirAll(rootfs, 0755) assert.NilError(t, err) netController, err := libnetwork.New() @@ -48,6 +49,18 @@ func setupFakeDaemon(t *testing.T, c *container.Container) *Daemon { c.NetworkSettings = &network.Settings{Networks: make(map[string]*network.EndpointSettings)} } + // HORRIBLE HACK: clean up shm mounts leaked by some tests. Otherwise the + // offending tests would fail due to the mounts blocking the temporary + // directory from being cleaned up. + t.Cleanup(func() { + if c.ShmPath != "" { + var err error + for err == nil { // Some tests over-mount over the same path multiple times. + err = unix.Unmount(c.ShmPath, unix.MNT_DETACH) + } + } + }) + return d } @@ -59,10 +72,6 @@ func (i *fakeImageService) StorageDriver() string { return "overlay" } -func cleanupFakeContainer(c *container.Container) { - _ = os.RemoveAll(c.Root) -} - // TestTmpfsDevShmNoDupMount checks that a user-specified /dev/shm tmpfs // mount (as in "docker run --tmpfs /dev/shm:rw,size=NNN") does not result // in "Duplicate mount point" error from the engine. @@ -80,7 +89,6 @@ func TestTmpfsDevShmNoDupMount(t *testing.T) { }, } d := setupFakeDaemon(t, c) - defer cleanupFakeContainer(c) _, err := d.createSpec(context.TODO(), &configStore{}, c) assert.Check(t, err) @@ -99,7 +107,6 @@ func TestIpcPrivateVsReadonly(t *testing.T) { }, } d := setupFakeDaemon(t, c) - defer cleanupFakeContainer(c) s, err := d.createSpec(context.TODO(), &configStore{}, c) assert.Check(t, err) @@ -128,7 +135,6 @@ func TestSysctlOverride(t *testing.T) { }, } d := setupFakeDaemon(t, c) - defer cleanupFakeContainer(c) // Ensure that the implicit sysctl is set correctly. s, err := d.createSpec(context.TODO(), &configStore{}, c) @@ -179,7 +185,6 @@ func TestSysctlOverrideHost(t *testing.T) { }, } d := setupFakeDaemon(t, c) - defer cleanupFakeContainer(c) // Ensure that the implicit sysctl is not set s, err := d.createSpec(context.TODO(), &configStore{}, c)