Start snapshot upgrade testing

snapshot/sink.go

@@ -6,8 +6,6 @@ import (
 	"log"
 	"os"
 	"path/filepath"
-	"runtime"
-	"strings"
 )
 
 // Sink is a sink for writing snapshot data to a Snapshot store.
@@ -223,31 +221,3 @@ func writeMeta(dir string, meta *Meta) error {
 	}
 	return fh.Close()
 }
-
-func parentDir(dir string) string {
-	return filepath.Dir(dir)
-}
-
-func tmpName(path string) string {
-	return path + tmpSuffix
-}
-
-func nonTmpName(path string) string {
-	return strings.TrimSuffix(path, tmpSuffix)
-}
-
-func moveFromTmpSync(src string) (string, error) {
-	dst := nonTmpName(src)
-	if err := os.Rename(src, dst); err != nil {
-		return "", err
-	}
-
-	// Sync parent directory to ensure snapshot is visible, but it's only
-	// needed on *nix style file systems.
-	if runtime.GOOS != "windows" {
-		if err := syncDir(parentDir(dst)); err != nil {
-			return "", err
-		}
-	}
-	return dst, nil
-}

snapshot/store.go

@@ -11,6 +11,7 @@ import (
 	"runtime"
 	"sort"
 	"strconv"
+	"strings"
 	sync "sync"
 	"time"
 
@@ -665,16 +666,31 @@ func copyFileSync(src, dst string) error {
 	return dstFd.Sync()
 }
 
+func parentDir(dir string) string {
+	return filepath.Dir(dir)
+}
+
+func tmpName(path string) string {
+	return path + tmpSuffix
+}
+
+func nonTmpName(path string) string {
+	return strings.TrimSuffix(path, tmpSuffix)
+}
+
+func moveFromTmpSync(src string) (string, error) {
+	dst := nonTmpName(src)
+	if err := os.Rename(src, dst); err != nil {
+		return "", err
+	}
+	return dst, syncDirParentMaybe(dst)
+}
+
 func removeDirSync(dir string) error {
 	if err := os.RemoveAll(dir); err != nil {
 		return err
 	}
-	if runtime.GOOS != "windows" {
-		if err := syncDir(filepath.Dir(dir)); err != nil {
-			return err
-		}
-	}
-	return nil
+	return syncDirParentMaybe(dir)
 }
 
 func syncDir(dir string) error {
@@ -686,6 +702,15 @@ func syncDir(dir string) error {
 	return fh.Sync()
 }
 
+// syncDirParentMaybe syncs the parent directory of the given
+// directory, but only on non-Windows platforms.
+func syncDirParentMaybe(dir string) error {
+	if runtime.GOOS == "windows" {
+		return nil
+	}
+	return syncDir(parentDir(dir))
+}
+
 // snapshotName generates a name for the snapshot.
 func snapshotName(term, index uint64) string {
 	now := time.Now()

snapshot/upgrader.go

@@ -14,6 +14,7 @@ import (
 // 'old' directory is removed before the function returns.
 func Upgrade(old, new string, logger *log.Logger) error {
 	if !dirExists(old) {
+		logger.Printf("old snapshot directory %s does not exist, nothing to upgrade", old)
 		return nil
 	}
 
@@ -42,7 +43,7 @@ func Upgrade(old, new string, logger *log.Logger) error {
 		if err := os.RemoveAll(newTmpDir); err != nil {
 			return fmt.Errorf("failed to remove temporary upgraded snapshot directory %s: %s", newTmpDir, err)
 		}
-		logger.Printf("removed temporary upgraded snapshot directory %s", tmpName(new))
+		logger.Println("detected temporary upgraded snapshot directory, removing")
 	}
 
 	// Start the upgrade process.
@@ -85,9 +86,12 @@ func Upgrade(old, new string, logger *log.Logger) error {
 	if err := os.Rename(newTmpDir, new); err != nil {
 		return fmt.Errorf("failed to move temporary snapshot directory %s to %s: %s", newTmpDir, new, err)
 	}
+	if err := syncDirParentMaybe(new); err != nil {
+		return fmt.Errorf("failed to sync parent directory of new snapshot directory %s: %s", new, err)
+	}
 
 	// We're done! Remove old.
-	if err := os.RemoveAll(old); err != nil {
+	if err := removeDirSync(old); err != nil {
 		return fmt.Errorf("failed to remove old snapshot directory %s: %s", old, err)
 	}
 	return nil

snapshot/upgrader_test.go

@@ -0,0 +1,20 @@
+package snapshot
+
+import (
+	"log"
+	"os"
+	"testing"
+)
+
+func Test_Upgrade_NothingToDo(t *testing.T) {
+	logger := log.New(os.Stderr, "[snapshot-store-upgrader] ", 0)
+	if err := Upgrade("/does/not/exist", "/does/not/exist/either", logger); err != nil {
+		t.Fatalf("failed to upgrade non-existent directories: %s", err)
+	}
+
+	oldEmpty := t.TempDir()
+	newEmpty := t.TempDir()
+	if err := Upgrade(oldEmpty, newEmpty, logger); err != nil {
+		t.Fatalf("failed to upgrade empty directories: %s", err)
+	}
+}