From 6ee6a82d0a211d8503e0c5b383fb9a1ae4fb6b13 Mon Sep 17 00:00:00 2001 From: Hiroshi Nomura Date: Sat, 23 Sep 2017 19:08:35 +0900 Subject: [PATCH 1/2] Enable cache image for Windows --- pkg/minikube/machine/cache_images.go | 46 +++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go index 706240ae7f94..5b6e15f6c237 100644 --- a/pkg/minikube/machine/cache_images.go +++ b/pkg/minikube/machine/cache_images.go @@ -19,6 +19,7 @@ package machine import ( "io/ioutil" "os" + "os/exec" "path/filepath" "runtime" "strings" @@ -122,6 +123,32 @@ func hasWindowsDriveLetter(s string) bool { return false } +func getWindowsVolumeName(d string) (string, error) { + cmd := exec.Command("wmic", "volume", "where", "DriveLetter = '"+d+":'", "get", "DeviceID") + + stdout, err := cmd.Output() + if err != nil { + return "", err + } + + outs := strings.Split(strings.Replace(string(stdout), "\r", "", -1), "\n") + + var vname string + for _, l := range outs { + s := strings.TrimSpace(l) + if strings.HasPrefix(s, `\\?\Volume{`) && strings.HasSuffix(s, `}\`) { + vname = s + break + } + } + + if vname == "" { + return "", errors.New("failed to get a volume GUID") + } + + return vname, nil +} + func LoadFromCacheBlocking(cmd bootstrapper.CommandRunner, src string) error { glog.Infoln("Loading image from cache at ", src) filename := filepath.Base(src) @@ -162,6 +189,18 @@ func getSrcRef(image string) (types.ImageReference, error) { } func getDstRef(image, dst string) (types.ImageReference, error) { + if hasWindowsDriveLetter(dst) { + // ParseReference does not support a Windows drive letter. + // Therefore, will change the drive letter to a volume name. + vname, err := getWindowsVolumeName(dst[:1]) + if err != nil { + return nil, errors.Wrap(err, "parsing docker archive dst ref: get Windows volume name") + } + dst = vname + dst[3:] + if _, err := os.Stat(filepath.Dir(dst)); err != nil { + return nil, errors.Wrap(err, "parsing docker archive dst ref: get Windows volume name") + } + } dstRef, err := archive.ParseReference(dst + ":" + image) if err != nil { return nil, errors.Wrap(err, "parsing docker archive dst ref") @@ -179,13 +218,6 @@ func CacheImage(image, dst string) error { return errors.Wrapf(err, "making cache image directory: %s", dst) } - // TODO: support Windows drive letter. - // L:164 ParseReference does not support Windows drive letter. - // If contains Windows drive letter, it disable cache image for now. - if hasWindowsDriveLetter(dst) { - return nil - } - srcRef, err := getSrcRef(image) if err != nil { return errors.Wrap(err, "creating docker image src ref") From 7f8f3bb5d41d712fc2a1205404acbca6d4900ed7 Mon Sep 17 00:00:00 2001 From: Hiroshi Nomura Date: Sat, 30 Sep 2017 20:43:51 +0900 Subject: [PATCH 2/2] Add unit tests --- pkg/minikube/machine/cache_images.go | 42 +++++++++----- pkg/minikube/machine/cache_images_test.go | 70 +++++++++++++++++++++++ 2 files changed, 98 insertions(+), 14 deletions(-) diff --git a/pkg/minikube/machine/cache_images.go b/pkg/minikube/machine/cache_images.go index 5b6e15f6c237..78f81289750e 100644 --- a/pkg/minikube/machine/cache_images.go +++ b/pkg/minikube/machine/cache_images.go @@ -41,6 +41,8 @@ import ( const tempLoadDir = "/tmp" +var getWindowsVolumeName = getWindowsVolumeNameCmd + func CacheImagesForBootstrapper(version string, clusterBootstrapper string) error { images := bootstrapper.GetCachedImageList(version, clusterBootstrapper) @@ -98,7 +100,7 @@ func LoadImages(cmd bootstrapper.CommandRunner, images []string, cacheDir string // # ParseReference cannot have a : in the directory path func sanitizeCacheDir(image string) string { - if hasWindowsDriveLetter(image) { + if runtime.GOOS == "windows" && hasWindowsDriveLetter(image) { // not sanitize Windows drive letter. return image[:2] + strings.Replace(image[2:], ":", "_", -1) } @@ -106,9 +108,6 @@ func sanitizeCacheDir(image string) string { } func hasWindowsDriveLetter(s string) bool { - if runtime.GOOS != "windows" { - return false - } if len(s) < 3 { return false } @@ -123,7 +122,21 @@ func hasWindowsDriveLetter(s string) bool { return false } -func getWindowsVolumeName(d string) (string, error) { +// Replace a drive letter to a volume name. +func replaceWinDriveLetterToVolumeName(s string) (string, error) { + vname, err := getWindowsVolumeName(s[:1]) + if err != nil { + return "", err + } + path := vname + s[3:] + if _, err := os.Stat(filepath.Dir(path)); err != nil { + return "", err + } + + return path, nil +} + +func getWindowsVolumeNameCmd(d string) (string, error) { cmd := exec.Command("wmic", "volume", "where", "DriveLetter = '"+d+":'", "get", "DeviceID") stdout, err := cmd.Output() @@ -189,18 +202,19 @@ func getSrcRef(image string) (types.ImageReference, error) { } func getDstRef(image, dst string) (types.ImageReference, error) { - if hasWindowsDriveLetter(dst) { + if runtime.GOOS == "windows" && hasWindowsDriveLetter(dst) { // ParseReference does not support a Windows drive letter. - // Therefore, will change the drive letter to a volume name. - vname, err := getWindowsVolumeName(dst[:1]) - if err != nil { - return nil, errors.Wrap(err, "parsing docker archive dst ref: get Windows volume name") - } - dst = vname + dst[3:] - if _, err := os.Stat(filepath.Dir(dst)); err != nil { - return nil, errors.Wrap(err, "parsing docker archive dst ref: get Windows volume name") + // Therefore, will replace the drive letter to a volume name. + var err error + if dst, err = replaceWinDriveLetterToVolumeName(dst); err != nil { + return nil, errors.Wrap(err, "parsing docker archive dst ref: replace a Win drive letter to a volume name") } } + + return _getDstRef(image, dst) +} + +func _getDstRef(image, dst string) (types.ImageReference, error) { dstRef, err := archive.ParseReference(dst + ":" + image) if err != nil { return nil, errors.Wrap(err, "parsing docker archive dst ref") diff --git a/pkg/minikube/machine/cache_images_test.go b/pkg/minikube/machine/cache_images_test.go index 2ecc884feb4a..37c74904769c 100644 --- a/pkg/minikube/machine/cache_images_test.go +++ b/pkg/minikube/machine/cache_images_test.go @@ -17,6 +17,10 @@ limitations under the License. package machine import ( + "io/ioutil" + "os" + "runtime" + "strings" "testing" "k8s.io/minikube/pkg/minikube/constants" @@ -29,3 +33,69 @@ func TestGetSrcRef(t *testing.T) { } } } + +func TestGetDstRef(t *testing.T) { + paths := []struct { + path, separator string + }{ + {`/Users/foo/.minikube/cache/images`, `/`}, + {`/home/foo/.minikube/cache/images`, `/`}, + {`\\?\Volume{aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee}\Users\foo\.minikube\cache\images`, `\`}, + {`\\?\Volume{aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee}\minikube\.minikube\cache\images`, `\`}, + } + + cases := []struct { + image, dst string + }{} + for _, tp := range paths { + for _, image := range constants.LocalkubeCachedImages { + dst := strings.Join([]string{tp.path, strings.Replace(image, ":", "_", -1)}, tp.separator) + cases = append(cases, struct{ image, dst string }{image, dst}) + } + } + + for _, tc := range cases { + if _, err := _getDstRef(tc.image, tc.dst); err != nil { + t.Errorf("Error getting dst ref for %s: %s", tc.dst, err) + } + } +} + +func TestReplaceWinDriveLetterToVolumeName(t *testing.T) { + path, err := ioutil.TempDir("", "repwindl2vn") + if err != nil { + t.Fatalf("Error make tmp directory: %s", err) + } + defer os.RemoveAll(path) + + if runtime.GOOS != "windows" { + // Replace to fake func. + getWindowsVolumeName = func(d string) (string, error) { + return `/`, nil + } + // Add dummy Windows drive letter. + path = `C:` + path + } + + if _, err := replaceWinDriveLetterToVolumeName(path); err != nil { + t.Errorf("Error replace a Windows drive letter to a volume name: %s", err) + } +} + +func TestHasWindowsDriveLetter(t *testing.T) { + cases := []struct { + path string + want bool + }{ + {`C:\Users\Foo\.minikube`, true}, + {`D:\minikube\.minikube`, true}, + {`C\Foo\Bar\.minikube`, false}, + {`/home/foo/.minikube`, false}, + } + + for _, tc := range cases { + if hasWindowsDriveLetter(tc.path) != tc.want { + t.Errorf("%s have a Windows drive letter: %t", tc.path, tc.want) + } + } +}