From d3790db03acbe33145929cac4efe9f19097c0794 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Fri, 3 Nov 2017 22:19:10 -0400 Subject: [PATCH 1/3] The cp command must call Close() on files, and does not pass on Mac Path operations needed to be with filepath, not path (for cross platform compat), and TMPDIR is not guaranteed to have a trailing slash. --- pkg/kubectl/cmd/cp.go | 16 +++++++---- pkg/kubectl/cmd/cp_test.go | 54 +++++++++++++++++++------------------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/pkg/kubectl/cmd/cp.go b/pkg/kubectl/cmd/cp.go index 8b3a89096b6..a5d67de3f96 100644 --- a/pkg/kubectl/cmd/cp.go +++ b/pkg/kubectl/cmd/cp.go @@ -301,10 +301,12 @@ func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *tar.Writer) e if err != nil { return err } - defer f.Close() - _, err = io.Copy(tw, f) - return err + + if _, err := io.Copy(tw, f); err != nil { + return err + } + return f.Close() } return nil } @@ -330,7 +332,6 @@ func untarAll(reader io.Reader, destFile, prefix string) error { return err } if header.FileInfo().IsDir() { - if err := os.MkdirAll(outFileName, 0755); err != nil { return err } @@ -359,7 +360,12 @@ func untarAll(reader io.Reader, destFile, prefix string) error { return err } defer outFile.Close() - io.Copy(outFile, tarReader) + if _, err := io.Copy(outFile, tarReader); err != nil { + return err + } + if err := outFile.Close(); err != nil { + return err + } } } diff --git a/pkg/kubectl/cmd/cp_test.go b/pkg/kubectl/cmd/cp_test.go index b410bc694ba..8236fef7a37 100644 --- a/pkg/kubectl/cmd/cp_test.go +++ b/pkg/kubectl/cmd/cp_test.go @@ -109,8 +109,8 @@ func TestGetPrefix(t *testing.T) { } func TestTarUntar(t *testing.T) { - dir, err := ioutil.TempDir(os.TempDir(), "input") - dir2, err2 := ioutil.TempDir(os.TempDir(), "output") + dir, err := ioutil.TempDir("", "input") + dir2, err2 := ioutil.TempDir("", "output") if err != nil || err2 != nil { t.Errorf("unexpected error: %v | %v", err, err2) t.FailNow() @@ -160,74 +160,74 @@ func TestTarUntar(t *testing.T) { for _, file := range files { filepath := path.Join(dir, file.name) if err := os.MkdirAll(path.Dir(filepath), 0755); err != nil { - t.Errorf("unexpected error: %v", err) - t.FailNow() + t.Fatalf("unexpected error: %v", err) } if file.fileType == RegularFile { f, err := os.Create(filepath) if err != nil { - t.Errorf("unexpected error: %v", err) - t.FailNow() + t.Fatalf("unexpected error: %v", err) } defer f.Close() if _, err := io.Copy(f, bytes.NewBuffer([]byte(file.data))); err != nil { - t.Errorf("unexpected error: %v", err) - t.FailNow() + t.Fatalf("unexpected error: %v", err) + } + if err := f.Close(); err != nil { + t.Fatal(err) } } else if file.fileType == SymLink { err := os.Symlink(file.data, filepath) if err != nil { - t.Errorf("unexpected error: %v", err) - t.FailNow() + t.Fatalf("unexpected error: %v", err) } } else { - t.Errorf("unexpected file type: %v", file) - t.FailNow() + t.Fatalf("unexpected file type: %v", file) } } writer := &bytes.Buffer{} if err := makeTar(dir, dir, writer); err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } reader := bytes.NewBuffer(writer.Bytes()) if err := untarAll(reader, dir2, ""); err != nil { - t.Errorf("unexpected error: %v", err) - t.FailNow() + t.Fatalf("unexpected error: %v", err) } for _, file := range files { - absPath := dir2 + strings.TrimPrefix(dir, os.TempDir()) - filepath := path.Join(absPath, file.name) + absPath := filepath.Join(dir2, strings.TrimPrefix(dir, os.TempDir())) + filePath := filepath.Join(absPath, file.name) if file.fileType == RegularFile { - f, err := os.Open(filepath) + f, err := os.Open(filePath) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } defer f.Close() buff := &bytes.Buffer{} - io.Copy(buff, f) - + if _, err := io.Copy(buff, f); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } if file.data != string(buff.Bytes()) { - t.Errorf("expected: %s, saw: %s", file.data, string(buff.Bytes())) + t.Fatalf("expected: %s, saw: %s", file.data, string(buff.Bytes())) } } else if file.fileType == SymLink { - dest, err := os.Readlink(filepath) + dest, err := os.Readlink(filePath) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatalf("unexpected error: %v", err) } if file.data != dest { - t.Errorf("expected: %s, saw: %s", file.data, dest) + t.Fatalf("expected: %s, saw: %s", file.data, dest) } } else { - t.Errorf("unexpected file type: %v", file) - t.FailNow() + t.Fatalf("unexpected file type: %v", file) } } } From 66590d6f83ee1d3488228de021e18617366406ed Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Fri, 3 Nov 2017 22:19:58 -0400 Subject: [PATCH 2/3] Container manager has a bad fake interface --- .../cm/container_manager_unsupported_test.go | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/pkg/kubelet/cm/container_manager_unsupported_test.go b/pkg/kubelet/cm/container_manager_unsupported_test.go index ed8a4fb8701..592164b93f2 100644 --- a/pkg/kubelet/cm/container_manager_unsupported_test.go +++ b/pkg/kubelet/cm/container_manager_unsupported_test.go @@ -40,17 +40,20 @@ func (mi *fakeMountInterface) List() ([]mount.MountPoint, error) { return mi.mountPoints, nil } -func (f *fakeMountInterface) IsMountPointMatch(mp mount.MountPoint, dir string) bool { +func (mi *fakeMountInterface) IsMountPointMatch(mp mount.MountPoint, dir string) bool { return (mp.Path == dir) } -func (f *fakeMountInterface) IsNotMountPoint(dir string) (bool, error) { +func (mi *fakeMountInterface) IsNotMountPoint(dir string) (bool, error) { return false, fmt.Errorf("unsupported") } func (mi *fakeMountInterface) IsLikelyNotMountPoint(file string) (bool, error) { return false, fmt.Errorf("unsupported") } +func (mi *fakeMountInterface) GetDeviceNameFromMount(mountPath, pluginDir string) (string, error) { + return "", nil +} func (mi *fakeMountInterface) DeviceOpened(pathname string) (bool, error) { for _, mp := range mi.mountPoints { @@ -65,14 +68,26 @@ func (mi *fakeMountInterface) PathIsDevice(pathname string) (bool, error) { return true, nil } -func (mi *fakeMountInterface) GetDeviceNameFromMount(mountPath, pluginDir string) (string, error) { - return "", nil -} - func (mi *fakeMountInterface) MakeRShared(path string) error { return nil } +func (mi *fakeMountInterface) GetFileType(pathname string) (mount.FileType, error) { + return mount.FileType("fake"), nil +} + +func (mi *fakeMountInterface) MakeDir(pathname string) error { + return nil +} + +func (mi *fakeMountInterface) MakeFile(pathname string) error { + return nil +} + +func (mi *fakeMountInterface) ExistsPath(pathname string) bool { + return true +} + func fakeContainerMgrMountInt() mount.Interface { return &fakeMountInterface{ []mount.MountPoint{ From b844ac44f5e08d3a80094dd8dbbcc60cdeef0e9d Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Fri, 3 Nov 2017 22:20:58 -0400 Subject: [PATCH 3/3] Tmpdir can be a symlink, also fake mount needs to call nested mounter --- pkg/util/mount/mount_unsupported.go | 2 +- pkg/volume/rbd/rbd_test.go | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/util/mount/mount_unsupported.go b/pkg/util/mount/mount_unsupported.go index 244c22c04c7..87d1e374819 100644 --- a/pkg/util/mount/mount_unsupported.go +++ b/pkg/util/mount/mount_unsupported.go @@ -86,7 +86,7 @@ func (mounter *Mounter) MakeRShared(path string) error { } func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error { - return nil + return mounter.Interface.Mount(source, target, fstype, options) } func (mounter *SafeFormatAndMount) diskLooksUnformatted(disk string) (bool, error) { diff --git a/pkg/volume/rbd/rbd_test.go b/pkg/volume/rbd/rbd_test.go index 67fe5b702f3..a35a1a4e74d 100644 --- a/pkg/volume/rbd/rbd_test.go +++ b/pkg/volume/rbd/rbd_test.go @@ -19,6 +19,7 @@ package rbd import ( "fmt" "os" + "path/filepath" "reflect" "strings" "sync" @@ -149,7 +150,7 @@ func checkMounterLog(t *testing.T, fakeMounter *mount.FakeMounter, expected int, lastIndex := len(fakeMounter.Log) - 1 lastAction := fakeMounter.Log[lastIndex] if !reflect.DeepEqual(expectedAction, lastAction) { - t.Fatalf("fakeMounter.Log[%d] should be %v, not: %v", lastIndex, expectedAction, lastAction) + t.Fatalf("fakeMounter.Log[%d] should be %#v, not: %#v", lastIndex, expectedAction, lastAction) } } @@ -276,6 +277,10 @@ func TestPlugin(t *testing.T) { t.Fatalf("error creating temp dir: %v", err) } defer os.RemoveAll(tmpDir) + tmpDir, err = filepath.EvalSymlinks(tmpDir) + if err != nil { + t.Fatal(err) + } podUID := uuid.NewUUID() var cases []*testcase