From 47c79870ea529099cca635f53da870e0cea5652a Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Wed, 30 Apr 2014 11:16:06 +0200 Subject: [PATCH 1/5] devmapper: Properly restore mocked functions after test Currently the tests that mocks or denies functions leave this state around for the next test. This is no good if we want to actually test the devicemapper code in later tests. Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) --- daemon/graphdriver/devmapper/driver_test.go | 52 ++++++++++++++++++--- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/daemon/graphdriver/devmapper/driver_test.go b/daemon/graphdriver/devmapper/driver_test.go index e53dfaa753..54e75e2a09 100644 --- a/daemon/graphdriver/devmapper/driver_test.go +++ b/daemon/graphdriver/devmapper/driver_test.go @@ -20,8 +20,16 @@ func init() { DefaultBaseFsSize = 300 * 1024 * 1024 } +// We use assignment here to get the right type +var ( + oldMounted = Mounted + oldExecRun = execRun +) + // denyAllDevmapper mocks all calls to libdevmapper in the unit tests, and denies them by default func denyAllDevmapper() { + oldExecRun = execRun + // Hijack all calls to libdevmapper with default panics. // Authorized calls are selectively hijacked in each tests. DmTaskCreate = func(t int) *CDmTask { @@ -77,7 +85,29 @@ func denyAllDevmapper() { } } +func restoreAllDevmapper() { + DmGetLibraryVersion = dmGetLibraryVersionFct + DmGetNextTarget = dmGetNextTargetFct + DmLogInitVerbose = dmLogInitVerboseFct + DmSetDevDir = dmSetDevDirFct + DmTaskAddTarget = dmTaskAddTargetFct + DmTaskCreate = dmTaskCreateFct + DmTaskDestroy = dmTaskDestroyFct + DmTaskGetInfo = dmTaskGetInfoFct + DmTaskRun = dmTaskRunFct + DmTaskSetAddNode = dmTaskSetAddNodeFct + DmTaskSetCookie = dmTaskSetCookieFct + DmTaskSetMessage = dmTaskSetMessageFct + DmTaskSetName = dmTaskSetNameFct + DmTaskSetRo = dmTaskSetRoFct + DmTaskSetSector = dmTaskSetSectorFct + DmUdevWait = dmUdevWaitFct + LogWithErrnoInit = logWithErrnoInitFct + execRun = oldExecRun +} + func denyAllSyscall() { + oldMounted = Mounted sysMount = func(source, target, fstype string, flags uintptr, data string) (err error) { panic("sysMount: this method should not be called here") } @@ -110,6 +140,14 @@ func denyAllSyscall() { // } } +func restoreAllSyscall() { + sysMount = syscall.Mount + sysUnmount = syscall.Unmount + sysCloseOnExec = syscall.CloseOnExec + sysSyscall = syscall.Syscall + Mounted = oldMounted +} + func mkTestDirectory(t *testing.T) string { dir, err := ioutil.TempDir("", "docker-test-devmapper-") if err != nil { @@ -160,8 +198,10 @@ func TestInit(t *testing.T) { ) defer osRemoveAll(home) + denyAllDevmapper() + defer restoreAllDevmapper() + func() { - denyAllDevmapper() DmSetDevDir = func(dir string) int { calls["DmSetDevDir"] = true expectedDir := "/dev" @@ -398,7 +438,7 @@ func mockAllDevmapper(calls Set) { func TestDriverName(t *testing.T) { denyAllDevmapper() - defer denyAllDevmapper() + defer restoreAllDevmapper() oldInit := fakeInit() defer restoreInit(oldInit) @@ -412,8 +452,8 @@ func TestDriverName(t *testing.T) { func TestDriverCreate(t *testing.T) { denyAllDevmapper() denyAllSyscall() - defer denyAllSyscall() - defer denyAllDevmapper() + defer restoreAllSyscall() + defer restoreAllDevmapper() calls := make(Set) mockAllDevmapper(calls) @@ -524,8 +564,8 @@ func TestDriverCreate(t *testing.T) { func TestDriverRemove(t *testing.T) { denyAllDevmapper() denyAllSyscall() - defer denyAllSyscall() - defer denyAllDevmapper() + defer restoreAllSyscall() + defer restoreAllDevmapper() calls := make(Set) mockAllDevmapper(calls) From 84f19a09ac1bb6221aeafd858306b097203aa974 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Wed, 30 Apr 2014 11:59:26 +0200 Subject: [PATCH 2/5] vfs graphdriver: Make root dir mode 755 This matches the other backends. Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) --- daemon/graphdriver/vfs/driver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/graphdriver/vfs/driver.go b/daemon/graphdriver/vfs/driver.go index 765b21cded..7473aa659d 100644 --- a/daemon/graphdriver/vfs/driver.go +++ b/daemon/graphdriver/vfs/driver.go @@ -47,7 +47,7 @@ func (d *Driver) Create(id, parent string) error { if err := os.MkdirAll(path.Dir(dir), 0700); err != nil { return err } - if err := os.Mkdir(dir, 0700); err != nil { + if err := os.Mkdir(dir, 0755); err != nil { return err } if parent == "" { From 27744062aa96f5f16d615ed829bc0d06b7df381d Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Wed, 30 Apr 2014 14:43:51 +0200 Subject: [PATCH 3/5] graphdriver: Add generic test framework for graph drivers This adds daemon/graphdriver/graphtest/graphtest which has a few generic tests for all graph drivers, and then uses these from the btrs, devicemapper and vfs backends. I've not yet added the aufs backend, because i can't test that here atm. It should work though. Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) --- daemon/graphdriver/btrfs/btrfs_test.go | 28 +++ daemon/graphdriver/devmapper/driver_test.go | 23 ++ daemon/graphdriver/graphtest/graphtest.go | 225 ++++++++++++++++++++ daemon/graphdriver/vfs/vfs_test.go | 28 +++ 4 files changed, 304 insertions(+) create mode 100644 daemon/graphdriver/btrfs/btrfs_test.go create mode 100644 daemon/graphdriver/graphtest/graphtest.go create mode 100644 daemon/graphdriver/vfs/vfs_test.go diff --git a/daemon/graphdriver/btrfs/btrfs_test.go b/daemon/graphdriver/btrfs/btrfs_test.go new file mode 100644 index 0000000000..3069a98557 --- /dev/null +++ b/daemon/graphdriver/btrfs/btrfs_test.go @@ -0,0 +1,28 @@ +package btrfs + +import ( + "github.com/dotcloud/docker/daemon/graphdriver/graphtest" + "testing" +) + +// This avoids creating a new driver for each test if all tests are run +// Make sure to put new tests between TestBtrfsSetup and TestBtrfsTeardown +func TestBtrfsSetup(t *testing.T) { + graphtest.GetDriver(t, "btrfs") +} + +func TestBtrfsCreateEmpty(t *testing.T) { + graphtest.DriverTestCreateEmpty(t, "btrfs") +} + +func TestBtrfsCreateBase(t *testing.T) { + graphtest.DriverTestCreateBase(t, "btrfs") +} + +func TestBtrfsCreateSnap(t *testing.T) { + graphtest.DriverTestCreateSnap(t, "btrfs") +} + +func TestBtrfsTeardown(t *testing.T) { + graphtest.PutDriver(t) +} diff --git a/daemon/graphdriver/devmapper/driver_test.go b/daemon/graphdriver/devmapper/driver_test.go index 54e75e2a09..0602f123a0 100644 --- a/daemon/graphdriver/devmapper/driver_test.go +++ b/daemon/graphdriver/devmapper/driver_test.go @@ -5,6 +5,7 @@ package devmapper import ( "fmt" "github.com/dotcloud/docker/daemon/graphdriver" + "github.com/dotcloud/docker/daemon/graphdriver/graphtest" "io/ioutil" "path" "runtime" @@ -914,3 +915,25 @@ func assertMap(t *testing.T, m map[string]bool, keys ...string) { t.Fatalf("Unexpected keys: %v", m) } } + +// This avoids creating a new driver for each test if all tests are run +// Make sure to put new tests between TestDevmapperSetup and TestDevmapperTeardown +func TestDevmapperSetup(t *testing.T) { + graphtest.GetDriver(t, "devicemapper") +} + +func TestDevmapperCreateEmpty(t *testing.T) { + graphtest.DriverTestCreateEmpty(t, "devicemapper") +} + +func TestDevmapperCreateBase(t *testing.T) { + graphtest.DriverTestCreateBase(t, "devicemapper") +} + +func TestDevmapperCreateSnap(t *testing.T) { + graphtest.DriverTestCreateSnap(t, "devicemapper") +} + +func TestDevmapperTeardown(t *testing.T) { + graphtest.PutDriver(t) +} diff --git a/daemon/graphdriver/graphtest/graphtest.go b/daemon/graphdriver/graphtest/graphtest.go new file mode 100644 index 0000000000..c83c840bc3 --- /dev/null +++ b/daemon/graphdriver/graphtest/graphtest.go @@ -0,0 +1,225 @@ +package graphtest + +import ( + "github.com/dotcloud/docker/daemon/graphdriver" + "io/ioutil" + "os" + "path" + "syscall" + "testing" +) + +var ( + drv *Driver +) + +type Driver struct { + graphdriver.Driver + root string + refCount int +} + +func newDriver(t *testing.T, name string) *Driver { + root, err := ioutil.TempDir("/var/tmp", "docker-graphtest-") + if err != nil { + t.Fatal(err) + } + + if err := os.MkdirAll(root, 0755); err != nil { + t.Fatal(err) + } + + d, err := graphdriver.GetDriver(name, root) + if err != nil { + t.Fatal(err) + } + return &Driver{d, root, 1} +} + +func cleanup(t *testing.T, d *Driver) { + if err := drv.Cleanup(); err != nil { + t.Fatal(err) + } + os.RemoveAll(d.root) +} + +func GetDriver(t *testing.T, name string) graphdriver.Driver { + if drv == nil { + drv = newDriver(t, name) + } else { + drv.refCount++ + } + return drv +} + +func PutDriver(t *testing.T) { + if drv == nil { + t.Fatal("No driver to put!") + } + drv.refCount-- + if drv.refCount == 0 { + cleanup(t, drv) + drv = nil + } +} + +func verifyFile(t *testing.T, path string, mode os.FileMode, uid, gid uint32) { + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + + if fi.Mode()&os.ModeType != mode&os.ModeType { + t.Fatalf("Expected %s type 0x%x, got 0x%x", path, mode&os.ModeType, fi.Mode()&os.ModeType) + } + + if fi.Mode()&os.ModePerm != mode&os.ModePerm { + t.Fatalf("Expected %s mode %o, got %o", path, mode&os.ModePerm, fi.Mode()&os.ModePerm) + } + + if fi.Mode()&os.ModeSticky != mode&os.ModeSticky { + t.Fatalf("Expected %s sticky 0x%x, got 0x%x", path, mode&os.ModeSticky, fi.Mode()&os.ModeSticky) + } + + if fi.Mode()&os.ModeSetuid != mode&os.ModeSetuid { + t.Fatalf("Expected %s setuid 0x%x, got 0x%x", path, mode&os.ModeSetuid, fi.Mode()&os.ModeSetuid) + } + + if fi.Mode()&os.ModeSetgid != mode&os.ModeSetgid { + t.Fatalf("Expected %s setgid 0x%x, got 0x%x", path, mode&os.ModeSetgid, fi.Mode()&os.ModeSetgid) + } + + if stat, ok := fi.Sys().(*syscall.Stat_t); ok { + if stat.Uid != uid { + t.Fatal("%s no owned by uid %d", path, uid) + } + if stat.Gid != gid { + t.Fatal("%s not owned by gid %d", path, gid) + } + } + +} + +// Creates an new image and verifies it is empty and the right metadata +func DriverTestCreateEmpty(t *testing.T, drivername string) { + driver := GetDriver(t, drivername) + defer PutDriver(t) + + if err := driver.Create("empty", ""); err != nil { + t.Fatal(err) + } + + if !driver.Exists("empty") { + t.Fatal("Newly created image doesn't exist") + } + + dir, err := driver.Get("empty", "") + if err != nil { + t.Fatal(err) + } + + verifyFile(t, dir, 0755|os.ModeDir, 0, 0) + + // Verify that the directory is empty + fis, err := ioutil.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + + if len(fis) != 0 { + t.Fatal("New directory not empty") + } + + driver.Put("empty") + + if err := driver.Remove("empty"); err != nil { + t.Fatal(err) + } + +} + +func createBase(t *testing.T, driver graphdriver.Driver, name string) { + // We need to be able to set any perms + oldmask := syscall.Umask(0) + defer syscall.Umask(oldmask) + + if err := driver.Create(name, ""); err != nil { + t.Fatal(err) + } + + dir, err := driver.Get(name, "") + if err != nil { + t.Fatal(err) + } + defer driver.Put(name) + + subdir := path.Join(dir, "a subdir") + if err := os.Mkdir(subdir, 0705|os.ModeSticky); err != nil { + t.Fatal(err) + } + if err := os.Chown(subdir, 1, 2); err != nil { + t.Fatal(err) + } + + file := path.Join(dir, "a file") + if err := ioutil.WriteFile(file, []byte("Some data"), 0222|os.ModeSetuid); err != nil { + t.Fatal(err) + } +} + +func verifyBase(t *testing.T, driver graphdriver.Driver, name string) { + dir, err := driver.Get(name, "") + if err != nil { + t.Fatal(err) + } + defer driver.Put(name) + + subdir := path.Join(dir, "a subdir") + verifyFile(t, subdir, 0705|os.ModeDir|os.ModeSticky, 1, 2) + + file := path.Join(dir, "a file") + verifyFile(t, file, 0222|os.ModeSetuid, 0, 0) + + fis, err := ioutil.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + + if len(fis) != 2 { + t.Fatal("Unexpected files in base image") + } + +} + +func DriverTestCreateBase(t *testing.T, drivername string) { + driver := GetDriver(t, drivername) + defer PutDriver(t) + + createBase(t, driver, "Base") + verifyBase(t, driver, "Base") + + if err := driver.Remove("Base"); err != nil { + t.Fatal(err) + } +} + +func DriverTestCreateSnap(t *testing.T, drivername string) { + driver := GetDriver(t, drivername) + defer PutDriver(t) + + createBase(t, driver, "Base") + + if err := driver.Create("Snap", "Base"); err != nil { + t.Fatal(err) + } + + verifyBase(t, driver, "Snap") + + if err := driver.Remove("Snap"); err != nil { + t.Fatal(err) + } + + if err := driver.Remove("Base"); err != nil { + t.Fatal(err) + } +} diff --git a/daemon/graphdriver/vfs/vfs_test.go b/daemon/graphdriver/vfs/vfs_test.go new file mode 100644 index 0000000000..e79f93c91d --- /dev/null +++ b/daemon/graphdriver/vfs/vfs_test.go @@ -0,0 +1,28 @@ +package vfs + +import ( + "github.com/dotcloud/docker/daemon/graphdriver/graphtest" + "testing" +) + +// This avoids creating a new driver for each test if all tests are run +// Make sure to put new tests between TestVfsSetup and TestVfsTeardown +func TestVfsSetup(t *testing.T) { + graphtest.GetDriver(t, "vfs") +} + +func TestVfsCreateEmpty(t *testing.T) { + graphtest.DriverTestCreateEmpty(t, "vfs") +} + +func TestVfsCreateBase(t *testing.T) { + graphtest.DriverTestCreateBase(t, "vfs") +} + +func TestVfsCreateSnap(t *testing.T) { + graphtest.DriverTestCreateSnap(t, "vfs") +} + +func TestVfsTeardown(t *testing.T) { + graphtest.PutDriver(t) +} From 4bdb8c03fc9ac4c7c49fd9838d7eccdfd66e1c5b Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Thu, 27 Mar 2014 17:41:06 +0100 Subject: [PATCH 4/5] graphdriver: Fail initialization if supported but got error If a graphdriver fails initialization due to ErrNotSupported we ignore that and keep trying the next. But if some driver has a different error (for instance if you specified an unknown option for it) we fail the daemon startup, printing the error, rather than falling back to an unexected driver (typically vfs) which may not match what you have run earlier. Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) --- daemon/graphdriver/aufs/aufs.go | 2 +- daemon/graphdriver/aufs/aufs_test.go | 2 +- daemon/graphdriver/btrfs/btrfs.go | 2 +- daemon/graphdriver/driver.go | 22 +++++++++++++++------- 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index 12b7a77fb3..2b7aa1b68a 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/daemon/graphdriver/aufs/aufs.go @@ -54,7 +54,7 @@ type Driver struct { func Init(root string) (graphdriver.Driver, error) { // Try to load the aufs kernel module if err := supportsAufs(); err != nil { - return nil, err + return nil, graphdriver.ErrNotSupported } paths := []string{ "mnt", diff --git a/daemon/graphdriver/aufs/aufs_test.go b/daemon/graphdriver/aufs/aufs_test.go index 1ffa264aa1..dab5aecc41 100644 --- a/daemon/graphdriver/aufs/aufs_test.go +++ b/daemon/graphdriver/aufs/aufs_test.go @@ -19,7 +19,7 @@ var ( func testInit(dir string, t *testing.T) graphdriver.Driver { d, err := Init(dir) if err != nil { - if err == ErrAufsNotSupported { + if err == graphdriver.ErrNotSupported { t.Skip(err) } else { t.Fatal(err) diff --git a/daemon/graphdriver/btrfs/btrfs.go b/daemon/graphdriver/btrfs/btrfs.go index 4d195537eb..614dc1ff06 100644 --- a/daemon/graphdriver/btrfs/btrfs.go +++ b/daemon/graphdriver/btrfs/btrfs.go @@ -31,7 +31,7 @@ func Init(home string) (graphdriver.Driver, error) { } if buf.Type != 0x9123683E { - return nil, fmt.Errorf("%s is not a btrfs filesystem", rootdir) + return nil, graphdriver.ErrNotSupported } return &Driver{ diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index 80bf8a0143..96f8d3ab3e 100644 --- a/daemon/graphdriver/driver.go +++ b/daemon/graphdriver/driver.go @@ -1,9 +1,9 @@ package graphdriver import ( + "errors" "fmt" "github.com/dotcloud/docker/archive" - "github.com/dotcloud/docker/utils" "os" "path" ) @@ -43,6 +43,8 @@ var ( "devicemapper", "vfs", } + + ErrNotSupported = errors.New("driver not supported") ) func init() { @@ -62,7 +64,7 @@ func GetDriver(name, home string) (Driver, error) { if initFunc, exists := drivers[name]; exists { return initFunc(path.Join(home, name)) } - return nil, fmt.Errorf("No such driver: %s", name) + return nil, ErrNotSupported } func New(root string) (driver Driver, err error) { @@ -74,9 +76,12 @@ func New(root string) (driver Driver, err error) { // Check for priority drivers first for _, name := range priority { - if driver, err = GetDriver(name, root); err != nil { - utils.Debugf("Error loading driver %s: %s", name, err) - continue + driver, err = GetDriver(name, root) + if err != nil { + if err == ErrNotSupported { + continue + } + return nil, err } return driver, nil } @@ -84,9 +89,12 @@ func New(root string) (driver Driver, err error) { // Check all registered drivers if no priority driver is found for _, initFunc := range drivers { if driver, err = initFunc(root); err != nil { - continue + if err == ErrNotSupported { + continue + } + return nil, err } return driver, nil } - return nil, err + return nil, fmt.Errorf("No supported storage backend found") } From 55cd7dd7f90d19332464ac946727297de1969483 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 6 May 2014 14:44:42 +0200 Subject: [PATCH 5/5] grapdriver: Skip tests on non-supported backends For now this means the btrfs backend is skipped when run inside make test. You can however run it manually if you want. Docker-DCO-1.1-Signed-off-by: Alexander Larsson (github: alexlarsson) --- daemon/graphdriver/graphtest/graphtest.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/daemon/graphdriver/graphtest/graphtest.go b/daemon/graphdriver/graphtest/graphtest.go index c83c840bc3..f8ccb2ef33 100644 --- a/daemon/graphdriver/graphtest/graphtest.go +++ b/daemon/graphdriver/graphtest/graphtest.go @@ -31,6 +31,9 @@ func newDriver(t *testing.T, name string) *Driver { d, err := graphdriver.GetDriver(name, root) if err != nil { + if err == graphdriver.ErrNotSupported { + t.Skip("Driver %s not supported", name) + } t.Fatal(err) } return &Driver{d, root, 1} @@ -54,7 +57,7 @@ func GetDriver(t *testing.T, name string) graphdriver.Driver { func PutDriver(t *testing.T) { if drv == nil { - t.Fatal("No driver to put!") + t.Skip("No driver to put!") } drv.refCount-- if drv.refCount == 0 {