From 6485a6cdc09f86ba6c34369e22db5036e1de8bfa Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Sep 2016 19:59:07 +0200 Subject: [PATCH 1/6] Simplify mount logic --- src/cmds/restic/cmd_mount.go | 38 ++++++++++++------------------------ 1 file changed, 13 insertions(+), 25 deletions(-) diff --git a/src/cmds/restic/cmd_mount.go b/src/cmds/restic/cmd_mount.go index e25306e31..5274533de 100644 --- a/src/cmds/restic/cmd_mount.go +++ b/src/cmds/restic/cmd_mount.go @@ -20,7 +20,6 @@ type CmdMount struct { global *GlobalOptions ready chan struct{} - done chan struct{} } func init() { @@ -29,8 +28,7 @@ func init() { "The mount command mounts a repository read-only to a given directory", &CmdMount{ global: &globalOpts, - ready: make(chan struct{}, 1), - done: make(chan struct{}), + ready: make(chan struct{}), }) if err != nil { panic(err) @@ -80,30 +78,20 @@ func (cmd CmdMount) Execute(args []string) error { cmd.global.Printf("Don't forget to umount after quitting!\n") AddCleanupHandler(func() error { - return systemFuse.Unmount(mountpoint) - }) - - cmd.ready <- struct{}{} - - errServe := make(chan error) - go func() { - err = fs.Serve(c, &root) - if err != nil { - errServe <- err - } - - <-c.Ready - errServe <- c.MountError - }() - - select { - case err := <-errServe: - return err - case <-cmd.done: err := systemFuse.Unmount(mountpoint) if err != nil { - cmd.global.Printf("Error umounting: %s\n", err) + cmd.global.Warnf("unable to umount (maybe already umounted?): %v\n", err) } - return c.Close() + return nil + }) + + close(cmd.ready) + + err = fs.Serve(c, &root) + if err != nil { + return err } + + <-c.Ready + return c.MountError } From a0f3e94655244cb691f38dfd5181e44034699d55 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Sep 2016 21:15:49 +0200 Subject: [PATCH 2/6] fuse: handle duplicate timestamps for snapshots This closes #606, which fails because several snapshots are created with exactly the same timestamp, and the code checks that for each snapshot there is a dir in the fuse mount. This fails for colliding timestamps, so we now add a suffix "-1", "-2" etc for each duplicate timestamp. --- src/restic/fuse/snapshot.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/restic/fuse/snapshot.go b/src/restic/fuse/snapshot.go index d71adbc79..9270e9adc 100644 --- a/src/restic/fuse/snapshot.go +++ b/src/restic/fuse/snapshot.go @@ -4,6 +4,7 @@ package fuse import ( + "fmt" "os" "sync" "time" @@ -65,11 +66,24 @@ func (sn *SnapshotsDir) updateCache(ctx context.Context) error { defer sn.Unlock() for id := range sn.repo.List(restic.SnapshotFile, ctx.Done()) { + debug.Log("SnapshotsDir.List", "found snapshot id %v", id.Str()) snapshot, err := restic.LoadSnapshot(sn.repo, id) if err != nil { return err } - sn.knownSnapshots[snapshot.Time.Format(time.RFC3339)] = SnapshotWithId{snapshot, id} + + timestamp := snapshot.Time.Format(time.RFC3339) + + for i := 1; ; i++ { + if _, ok := sn.knownSnapshots[timestamp]; !ok { + break + } + + timestamp = fmt.Sprintf("%s-%d", snapshot.Time.Format(time.RFC3339), i) + } + + debug.Log("SnapshotsDir.List", " add %v as dir %v", id.Str(), timestamp) + sn.knownSnapshots[timestamp] = SnapshotWithId{snapshot, id} } return nil } From 4ffca0f4b4ed1979d3a0a34d2e655f5da4d6e86a Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Sep 2016 21:17:20 +0200 Subject: [PATCH 3/6] Improve integration tests for fuse --- src/cmds/restic/cleanup.go | 4 + src/cmds/restic/cmd_mount.go | 51 +++++--- src/cmds/restic/integration_fuse_test.go | 157 +++++++++++------------ 3 files changed, 110 insertions(+), 102 deletions(-) diff --git a/src/cmds/restic/cleanup.go b/src/cmds/restic/cleanup.go index c2072650a..a4481e2e3 100644 --- a/src/cmds/restic/cleanup.go +++ b/src/cmds/restic/cleanup.go @@ -32,6 +32,9 @@ func AddCleanupHandler(f func() error) { cleanupHandlers.Lock() defer cleanupHandlers.Unlock() + // reset the done flag for integration tests + cleanupHandlers.done = false + cleanupHandlers.list = append(cleanupHandlers.list, f) } @@ -51,6 +54,7 @@ func RunCleanupHandlers() { fmt.Fprintf(stderr, "error in cleanup handler: %v\n", err) } } + cleanupHandlers.list = nil } // CleanupHandler handles the SIGINT signal. diff --git a/src/cmds/restic/cmd_mount.go b/src/cmds/restic/cmd_mount.go index 5274533de..2f41b01d0 100644 --- a/src/cmds/restic/cmd_mount.go +++ b/src/cmds/restic/cmd_mount.go @@ -6,6 +6,7 @@ package main import ( "os" + "restic/debug" "restic/errors" resticfs "restic/fs" @@ -19,7 +20,6 @@ type CmdMount struct { Root bool `long:"owner-root" description:"use 'root' as the owner of files and dirs" default:"false"` global *GlobalOptions - ready chan struct{} } func init() { @@ -28,7 +28,6 @@ func init() { "The mount command mounts a repository read-only to a given directory", &CmdMount{ global: &globalOpts, - ready: make(chan struct{}), }) if err != nil { panic(err) @@ -39,10 +38,9 @@ func (cmd CmdMount) Usage() string { return "MOUNTPOINT" } -func (cmd CmdMount) Execute(args []string) error { - if len(args) == 0 { - return errors.Fatalf("wrong number of parameters, Usage: %s", cmd.Usage()) - } +func (cmd CmdMount) Mount(mountpoint string) error { + debug.Log("mount", "start mount") + defer debug.Log("mount", "finish mount") repo, err := cmd.global.OpenRepository() if err != nil { @@ -54,7 +52,6 @@ func (cmd CmdMount) Execute(args []string) error { return err } - mountpoint := args[0] if _, err := resticfs.Stat(mountpoint); os.IsNotExist(errors.Cause(err)) { cmd.global.Verbosef("Mountpoint %s doesn't exist, creating it\n", mountpoint) err = resticfs.Mkdir(mountpoint, os.ModeDir|0700) @@ -74,19 +71,7 @@ func (cmd CmdMount) Execute(args []string) error { root := fs.Tree{} root.Add("snapshots", fuse.NewSnapshotsDir(repo, cmd.Root)) - cmd.global.Printf("Now serving %s at %s\n", repo.Backend().Location(), mountpoint) - cmd.global.Printf("Don't forget to umount after quitting!\n") - - AddCleanupHandler(func() error { - err := systemFuse.Unmount(mountpoint) - if err != nil { - cmd.global.Warnf("unable to umount (maybe already umounted?): %v\n", err) - } - return nil - }) - - close(cmd.ready) - + debug.Log("mount", "serving mount at %v", mountpoint) err = fs.Serve(c, &root) if err != nil { return err @@ -95,3 +80,29 @@ func (cmd CmdMount) Execute(args []string) error { <-c.Ready return c.MountError } + +func (cmd CmdMount) Umount(mountpoint string) error { + return systemFuse.Unmount(mountpoint) +} + +func (cmd CmdMount) Execute(args []string) error { + if len(args) == 0 { + return errors.Fatalf("wrong number of parameters, Usage: %s", cmd.Usage()) + } + + mountpoint := args[0] + + AddCleanupHandler(func() error { + debug.Log("mount", "running umount cleanup handler for mount at %v", mountpoint) + err := cmd.Umount(mountpoint) + if err != nil { + cmd.global.Warnf("unable to umount (maybe already umounted?): %v\n", err) + } + return nil + }) + + cmd.global.Printf("Now serving the repository at %s\n", mountpoint) + cmd.global.Printf("Don't forget to umount after quitting!\n") + + return cmd.Mount(mountpoint) +} diff --git a/src/cmds/restic/integration_fuse_test.go b/src/cmds/restic/integration_fuse_test.go index a106d035d..396b7d167 100644 --- a/src/cmds/restic/integration_fuse_test.go +++ b/src/cmds/restic/integration_fuse_test.go @@ -10,8 +10,6 @@ import ( "testing" "time" - "restic/errors" - "restic" "restic/repository" . "restic/test" @@ -23,46 +21,55 @@ const ( mountTestSubdir = "snapshots" ) +func snapshotsDirExists(t testing.TB, dir string) bool { + f, err := os.Open(filepath.Join(dir, mountTestSubdir)) + if err != nil && os.IsNotExist(err) { + return false + } + + if err != nil { + t.Error(err) + } + + if err := f.Close(); err != nil { + t.Error(err) + } + + return true +} + // waitForMount blocks (max mountWait * mountSleep) until the subdir // "snapshots" appears in the dir. -func waitForMount(dir string) error { +func waitForMount(t testing.TB, dir string) { for i := 0; i < mountWait; i++ { - f, err := os.Open(dir) - if err != nil { - return err - } - - names, err := f.Readdirnames(-1) - if err != nil { - return err - } - - if err = f.Close(); err != nil { - return err - } - - for _, name := range names { - if name == mountTestSubdir { - return nil - } + if snapshotsDirExists(t, dir) { + t.Log("mounted directory is ready") + return } time.Sleep(mountSleep) } - return errors.Fatalf("subdir %q of dir %s never appeared", mountTestSubdir, dir) + t.Errorf("subdir %q of dir %s never appeared", mountTestSubdir, dir) } -func cmdMount(t testing.TB, global GlobalOptions, dir string, ready, done chan struct{}) { - defer func() { - ready <- struct{}{} - }() +func mount(t testing.TB, global GlobalOptions, dir string) { + cmd := &CmdMount{global: &global} + OK(t, cmd.Mount(dir)) +} - cmd := &CmdMount{global: &global, ready: ready, done: done} - OK(t, cmd.Execute([]string{dir})) - if TestCleanupTempDirs { - RemoveAll(t, dir) - } +func umount(t testing.TB, global GlobalOptions, dir string) { + cmd := &CmdMount{global: &global} + OK(t, cmd.Umount(dir)) +} + +func listSnapshots(t testing.TB, dir string) []string { + snapshotsDir, err := os.Open(filepath.Join(dir, "snapshots")) + OK(t, err) + names, err := snapshotsDir.Readdirnames(-1) + OK(t, err) + OK(t, snapshotsDir.Close()) + return names } func TestMount(t *testing.T) { @@ -70,34 +77,43 @@ func TestMount(t *testing.T) { t.Skip("Skipping fuse tests") } - checkSnapshots := func(repo *repository.Repository, mountpoint string, snapshotIDs []restic.ID) { - snapshotsDir, err := os.Open(filepath.Join(mountpoint, "snapshots")) - OK(t, err) - namesInSnapshots, err := snapshotsDir.Readdirnames(-1) - OK(t, err) - Assert(t, - len(namesInSnapshots) == len(snapshotIDs), - "Invalid number of snapshots: expected %d, got %d", len(snapshotIDs), len(namesInSnapshots)) - - namesMap := make(map[string]bool) - for _, name := range namesInSnapshots { - namesMap[name] = false - } - - for _, id := range snapshotIDs { - snapshot, err := restic.LoadSnapshot(repo, id) - OK(t, err) - _, ok := namesMap[snapshot.Time.Format(time.RFC3339)] - Assert(t, ok, "Snapshot %s isn't present in fuse dir", snapshot.Time.Format(time.RFC3339)) - namesMap[snapshot.Time.Format(time.RFC3339)] = true - } - for name, present := range namesMap { - Assert(t, present, "Directory %s is present in fuse dir but is not a snapshot", name) - } - OK(t, snapshotsDir.Close()) - } - withTestEnvironment(t, func(env *testEnvironment, global GlobalOptions) { + checkSnapshots := func(repo *repository.Repository, mountpoint string, snapshotIDs restic.IDs) { + t.Logf("checking for %d snapshots: %v", len(snapshotIDs), snapshotIDs) + go mount(t, global, mountpoint) + waitForMount(t, mountpoint) + defer umount(t, global, mountpoint) + + if !snapshotsDirExists(t, mountpoint) { + t.Fatal(`virtual directory "snapshots" doesn't exist`) + } + + ids := listSnapshots(t, env.repo) + t.Logf("found %v snapshots in repo: %v", len(ids), ids) + + namesInSnapshots := listSnapshots(t, mountpoint) + t.Logf("found %v snapshots in fuse mount: %v", len(namesInSnapshots), namesInSnapshots) + Assert(t, + len(namesInSnapshots) == len(snapshotIDs), + "Invalid number of snapshots: expected %d, got %d", len(snapshotIDs), len(namesInSnapshots)) + + namesMap := make(map[string]bool) + for _, name := range namesInSnapshots { + namesMap[name] = false + } + + for _, id := range snapshotIDs { + snapshot, err := restic.LoadSnapshot(repo, id) + OK(t, err) + _, ok := namesMap[snapshot.Time.Format(time.RFC3339)] + Assert(t, ok, "Snapshot %s isn't present in fuse dir", snapshot.Time.Format(time.RFC3339)) + namesMap[snapshot.Time.Format(time.RFC3339)] = true + } + for name, present := range namesMap { + Assert(t, present, "Directory %s is present in fuse dir but is not a snapshot", name) + } + } + cmdInit(t, global) repo, err := global.OpenRepository() OK(t, err) @@ -108,32 +124,9 @@ func TestMount(t *testing.T) { // We remove the mountpoint now to check that cmdMount creates it RemoveAll(t, mountpoint) - ready := make(chan struct{}, 2) - done := make(chan struct{}) - go cmdMount(t, global, mountpoint, ready, done) - <-ready - defer close(done) - OK(t, waitForMount(mountpoint)) - - mountpointDir, err := os.Open(mountpoint) - OK(t, err) - names, err := mountpointDir.Readdirnames(-1) - OK(t, err) - Assert(t, len(names) == 1 && names[0] == "snapshots", `The fuse virtual directory "snapshots" doesn't exist`) - OK(t, mountpointDir.Close()) - checkSnapshots(repo, mountpoint, []restic.ID{}) - datafile := filepath.Join("testdata", "backup-data.tar.gz") - fd, err := os.Open(datafile) - if os.IsNotExist(errors.Cause(err)) { - t.Skipf("unable to find data file %q, skipping", datafile) - return - } - OK(t, err) - OK(t, fd.Close()) - - SetupTarTestFixture(t, env.testdata, datafile) + SetupTarTestFixture(t, env.testdata, filepath.Join("testdata", "backup-data.tar.gz")) // first backup cmdBackup(t, global, []string{env.testdata}, nil) From e7fc908ff1033c734460cb52d77544083c2db846 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Sep 2016 21:25:59 +0200 Subject: [PATCH 4/6] Run fuse tests on Linux --- .travis.yml | 13 ++++++++++++- run_integration_tests.go | 17 ++--------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index f73baec3d..2a1306ea4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,16 +3,27 @@ sudo: false go: - 1.6.3 - - 1.7 + - 1.7.1 os: - linux - osx +env: + matrix: + RESTIC_TEST_FUSE=0 + matrix: exclude: - os: osx go: 1.6.3 + include: + - os: linux + go: 1.7.1 + sudo: true + env: + RESTIC_TEST_FUSE=1 + notifications: irc: diff --git a/run_integration_tests.go b/run_integration_tests.go index 5f3ee9f6f..bfdddebe8 100644 --- a/run_integration_tests.go +++ b/run_integration_tests.go @@ -165,19 +165,6 @@ func (env *TravisEnvironment) Prepare() error { return err } - if runtime.GOOS == "darwin" { - // install the libraries necessary for fuse - for _, cmd := range [][]string{ - {"brew", "update"}, - {"brew", "tap", "caskroom/cask"}, - {"brew", "cask", "install", "osxfuse"}, - } { - if err := run(cmd[0], cmd[1:]...); err != nil { - return err - } - } - } - if *runCrossCompile && !(runtime.Version() < "go1.7") { // only test cross compilation on linux with Travis if err := run("go", "get", "github.com/mitchellh/gox"); err != nil { @@ -314,8 +301,8 @@ func StartBackgroundCommand(env map[string]string, cmd string, args ...string) ( // RunTests starts the tests for Travis. func (env *TravisEnvironment) RunTests() error { - // run fuse tests on darwin - if runtime.GOOS != "darwin" { + // do not run fuse tests on darwin + if runtime.GOOS == "darwin" { msg("skip fuse integration tests on %v\n", runtime.GOOS) os.Setenv("RESTIC_TEST_FUSE", "0") } From 9add72e9d60bdcfbd6350bd04d6cfaf74d1bb662 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Sep 2016 21:29:12 +0200 Subject: [PATCH 5/6] Exclude unneeded test run without fuse tests --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index 2a1306ea4..9fb471b47 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,6 +17,8 @@ matrix: exclude: - os: osx go: 1.6.3 + - os: linux + go: 1.7.1 include: - os: linux go: 1.7.1 From d4a2d7008920035676407d4ea879d3ccf18139a6 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Thu, 15 Sep 2016 21:32:15 +0200 Subject: [PATCH 6/6] Retry umount for integration tests --- src/cmds/restic/integration_fuse_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/cmds/restic/integration_fuse_test.go b/src/cmds/restic/integration_fuse_test.go index 396b7d167..bfaeaf0a3 100644 --- a/src/cmds/restic/integration_fuse_test.go +++ b/src/cmds/restic/integration_fuse_test.go @@ -60,7 +60,18 @@ func mount(t testing.TB, global GlobalOptions, dir string) { func umount(t testing.TB, global GlobalOptions, dir string) { cmd := &CmdMount{global: &global} - OK(t, cmd.Umount(dir)) + + var err error + for i := 0; i < mountWait; i++ { + if err = cmd.Umount(dir); err == nil { + t.Logf("directory %v umounted", dir) + return + } + + time.Sleep(mountSleep) + } + + t.Errorf("unable to umount dir %v, last error was: %v", dir, err) } func listSnapshots(t testing.TB, dir string) []string {