Merge pull request #6605 from tstromberg/copy-confusion

sync: Fix path confusion and directory creation bugs

pkg/minikube/machine/filesync.go

@@ -19,6 +19,7 @@ package machine
 import (
 	"fmt"
 	"os"
+	"os/exec"
 	"path"
 	"path/filepath"
 
@@ -30,6 +31,15 @@ import (
 	"k8s.io/minikube/pkg/minikube/vmpath"
 )
 
+// guaranteed are directories we don't need to attempt recreation of
+var guaranteed = map[string]bool{
+	"/":    true,
+	"":     true,
+	"/etc": true,
+	"/var": true,
+	"/tmp": true,
+}
+
 // syncLocalAssets syncs files from MINIKUBE_HOME into the cluster
 func syncLocalAssets(cr command.Runner) error {
 	fs, err := localAssets()
@@ -37,6 +47,30 @@ func syncLocalAssets(cr command.Runner) error {
 		return err
 	}
 
+	if len(fs) == 0 {
+		return nil
+	}
+
+	// Deduplicate the list of directories to create
+	seen := map[string]bool{}
+	create := []string{}
+	for _, f := range fs {
+		dir := f.GetTargetDir()
+		if guaranteed[dir] || seen[dir] {
+			continue
+		}
+		create = append(create, dir)
+	}
+
+	// Create directories that are not guaranteed to exist
+	if len(create) > 0 {
+		args := append([]string{"mkdir", "-p"}, create...)
+		if _, err := cr.RunCmd(exec.Command("sudo", args...)); err != nil {
+			return err
+		}
+	}
+
+	// Copy the files into place
 	for _, f := range fs {
 		err := cr.Copy(f)
 		if err != nil {
@@ -62,7 +96,24 @@ func localAssets() ([]assets.CopyableFile, error) {
 	return fs, nil
 }
 
-// AssetsFromDir generates assets from a local filepath, with/without a flattened hierarchy
+// syncDest returns the path within a VM for a local asset
+func syncDest(localRoot string, localPath string, destRoot string, flatten bool) (string, error) {
+	rel, err := filepath.Rel(localRoot, localPath)
+	if err != nil {
+		return "", err
+	}
+
+	// On Windows, rel will be separated by \, which is not correct inside the VM
+	rel = filepath.ToSlash(rel)
+
+	// If flatten is set, dump everything into the same destination directory
+	if flatten {
+		return path.Join(destRoot, filepath.Base(localPath)), nil
+	}
+	return path.Join(destRoot, rel), nil
+}
+
+// assetsFromDir generates assets from a local filepath, with/without a flattened hierarchy
 func assetsFromDir(localRoot string, destRoot string, flatten bool) ([]assets.CopyableFile, error) {
 	glog.Infof("Scanning %s for local assets ...", localRoot)
 	fs := []assets.CopyableFile{}
@@ -74,23 +125,19 @@ func assetsFromDir(localRoot string, destRoot string, flatten bool) ([]assets.Co
 			return nil
 		}
 
-		rel, err := filepath.Rel(localRoot, localPath)
-		if err != nil {
-			return err
-		}
-
 		// The conversion will strip the leading 0 if present, so add it back if necessary
 		ps := fmt.Sprintf("%o", fi.Mode().Perm())
 		if len(ps) == 3 {
 			ps = fmt.Sprintf("0%s", ps)
 		}
 
-		dest := path.Join(destRoot, rel)
+		dest, err := syncDest(localRoot, localPath, destRoot, flatten)
+		if err != nil {
+			return err
+		}
 		targetDir := path.Dir(dest)
 		targetName := path.Base(dest)
-		if flatten {
-			targetDir = destRoot
-		}
+
 		glog.Infof("local asset: %s -> %s in %s", localPath, targetName, targetDir)
 		f, err := assets.NewFileAsset(localPath, targetDir, targetName, ps)
 		if err != nil {

pkg/minikube/machine/filesync_test.go

@@ -158,3 +158,33 @@ func TestAssetsFromDir(t *testing.T) {
 	}
 
 }
+
+func TestSyncDest(t *testing.T) {
+	tests := []struct {
+		description string
+		localParts  []string
+		destRoot    string
+		flatten     bool
+		want        string
+	}{
+		{"simple", []string{"etc", "hosts"}, "/", false, "/etc/hosts"},
+		{"nested", []string{"etc", "nested", "hosts"}, "/", false, "/etc/nested/hosts"},
+		{"flat", []string{"etc", "nested", "hosts"}, "/test", true, "/test/hosts"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			// Generate paths using filepath to mimic OS-specific issues
+			localRoot := localpath.MakeMiniPath("sync")
+			localParts := append([]string{localRoot}, test.localParts...)
+			localPath := filepath.Join(localParts...)
+			got, err := syncDest(localRoot, localPath, test.destRoot, test.flatten)
+			if err != nil {
+				t.Fatalf("syncDest(%s, %s, %v) unexpected err: %v", localRoot, localPath, test.flatten, err)
+			}
+			if got != test.want {
+				t.Errorf("syncDest(%s, %s, %v) = %s, want: %s", localRoot, localPath, test.flatten, got, test.want)
+			}
+		})
+	}
+}

test/integration/functional_test.go

@@ -56,7 +56,13 @@ func TestFunctional(t *testing.T) {
 
 	profile := UniqueProfileName("functional")
 	ctx, cancel := context.WithTimeout(context.Background(), 40*time.Minute)
-	defer CleanupWithLogs(t, profile, cancel)
+	defer func() {
+		p := localSyncTestPath()
+		if err := os.Remove(p); err != nil {
+			t.Logf("unable to remove %s: %v", p, err)
+		}
+		CleanupWithLogs(t, profile, cancel)
+	}()
 
 	// Serial tests
 	t.Run("serial", func(t *testing.T) {
@@ -657,11 +663,21 @@ func validateMySQL(ctx context.Context, t *testing.T, profile string) {
 	}
 }
 
+// vmSyncTestPath is where the test file will be synced into the VM
+func vmSyncTestPath() string {
+	return fmt.Sprintf("/etc/test/nested/copy/%d/hosts", os.Getpid())
+}
+
+// localSyncTestPath is where the test file will be synced into the VM
+func localSyncTestPath() string {
+	return filepath.Join(localpath.MiniPath(), "/files", vmSyncTestPath())
+}
+
 // Copy extra file into minikube home folder for file sync test
 func setupFileSync(ctx context.Context, t *testing.T, profile string) {
-	// 1. copy random file to MINIKUBE_HOME/files/etc
-	f := filepath.Join(localpath.MiniPath(), "/files/etc/sync.test")
-	err := copy.Copy("./testdata/sync.test", f)
+	p := localSyncTestPath()
+	t.Logf("local sync path: %s", p)
+	err := copy.Copy("./testdata/sync.test", p)
 	if err != nil {
 		t.Fatalf("copy: %v", err)
 	}
@@ -672,18 +688,22 @@ func validateFileSync(ctx context.Context, t *testing.T, profile string) {
 	if NoneDriver() {
 		t.Skipf("skipping: ssh unsupported by none")
 	}
-	// check file existence
-	rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "ssh", "cat /etc/sync.test"))
+
+	vp := vmSyncTestPath()
+	t.Logf("Checking for existence of %s within VM", vp)
+	rr, err := Run(t, exec.CommandContext(ctx, Target(), "-p", profile, "ssh", fmt.Sprintf("cat %s", vp)))
 	if err != nil {
 		t.Errorf("%s failed: %v", rr.Args, err)
 	}
+	got := rr.Stdout.String()
+	t.Logf("file sync test content: %s", got)
 
 	expected, err := ioutil.ReadFile("./testdata/sync.test")
 	if err != nil {
 		t.Errorf("test file not found: %v", err)
 	}
 
-	if diff := cmp.Diff(string(expected), rr.Stdout.String()); diff != "" {
+	if diff := cmp.Diff(string(expected), got); diff != "" {
 		t.Errorf("/etc/sync.test content mismatch (-want +got):\n%s", diff)
 	}
 }