From 0abe151fe8f9c84578d83ecb71bc0e7b3881bcaf Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Thu, 14 Nov 2019 09:09:15 -0700 Subject: [PATCH 01/27] Extract pkg/util/mount and drop BUILD Preserving git history. --- doc.go | 18 ++ fake_mounter.go | 204 ++++++++++++++ mount.go | 267 +++++++++++++++++++ mount_helper_common.go | 99 +++++++ mount_helper_test.go | 139 ++++++++++ mount_helper_unix.go | 140 ++++++++++ mount_helper_unix_test.go | 215 +++++++++++++++ mount_helper_windows.go | 98 +++++++ mount_helper_windows_test.go | 65 +++++ mount_linux.go | 489 ++++++++++++++++++++++++++++++++++ mount_linux_test.go | 439 ++++++++++++++++++++++++++++++ mount_test.go | 60 +++++ mount_unsupported.go | 72 +++++ mount_windows.go | 280 +++++++++++++++++++ mount_windows_test.go | 333 +++++++++++++++++++++++ safe_format_and_mount_test.go | 248 +++++++++++++++++ 16 files changed, 3166 insertions(+) create mode 100644 doc.go create mode 100644 fake_mounter.go create mode 100644 mount.go create mode 100644 mount_helper_common.go create mode 100644 mount_helper_test.go create mode 100644 mount_helper_unix.go create mode 100644 mount_helper_unix_test.go create mode 100644 mount_helper_windows.go create mode 100644 mount_helper_windows_test.go create mode 100644 mount_linux.go create mode 100644 mount_linux_test.go create mode 100644 mount_test.go create mode 100644 mount_unsupported.go create mode 100644 mount_windows.go create mode 100644 mount_windows_test.go create mode 100644 safe_format_and_mount_test.go diff --git a/doc.go b/doc.go new file mode 100644 index 00000000000..15179e53f2d --- /dev/null +++ b/doc.go @@ -0,0 +1,18 @@ +/* +Copyright 2014 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package mount defines an interface to mounting filesystems. +package mount // import "k8s.io/kubernetes/pkg/util/mount" diff --git a/fake_mounter.go b/fake_mounter.go new file mode 100644 index 00000000000..315bba6941f --- /dev/null +++ b/fake_mounter.go @@ -0,0 +1,204 @@ +/* +Copyright 2015 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "os" + "path/filepath" + "sync" + + "k8s.io/klog" +) + +// FakeMounter implements mount.Interface for tests. +type FakeMounter struct { + MountPoints []MountPoint + log []FakeAction + // Error to return for a path when calling IsLikelyNotMountPoint + MountCheckErrors map[string]error + // Some tests run things in parallel, make sure the mounter does not produce + // any golang's DATA RACE warnings. + mutex sync.Mutex + UnmountFunc UnmountFunc +} + +type UnmountFunc func(path string) error + +var _ Interface = &FakeMounter{} + +const ( + // FakeActionMount is the string for specifying mount as FakeAction.Action + FakeActionMount = "mount" + // FakeActionUnmount is the string for specifying unmount as FakeAction.Action + FakeActionUnmount = "unmount" +) + +// FakeAction objects are logged every time a fake mount or unmount is called. +type FakeAction struct { + Action string // "mount" or "unmount" + Target string // applies to both mount and unmount actions + Source string // applies only to "mount" actions + FSType string // applies only to "mount" actions +} + +func NewFakeMounter(mps []MountPoint) *FakeMounter { + return &FakeMounter{ + MountPoints: mps, + } +} + +// ResetLog clears all the log entries in FakeMounter +func (f *FakeMounter) ResetLog() { + f.mutex.Lock() + defer f.mutex.Unlock() + + f.log = []FakeAction{} +} + +// GetLog returns the slice of FakeActions taken by the mounter +func (f *FakeMounter) GetLog() []FakeAction { + f.mutex.Lock() + defer f.mutex.Unlock() + + return f.log +} + +// Mount records the mount event and updates the in-memory mount points for FakeMounter +func (f *FakeMounter) Mount(source string, target string, fstype string, options []string) error { + f.mutex.Lock() + defer f.mutex.Unlock() + + opts := []string{} + + for _, option := range options { + // find 'bind' option + if option == "bind" { + // This is a bind-mount. In order to mimic linux behaviour, we must + // use the original device of the bind-mount as the real source. + // E.g. when mounted /dev/sda like this: + // $ mount /dev/sda /mnt/test + // $ mount -o bind /mnt/test /mnt/bound + // then /proc/mount contains: + // /dev/sda /mnt/test + // /dev/sda /mnt/bound + // (and not /mnt/test /mnt/bound) + // I.e. we must use /dev/sda as source instead of /mnt/test in the + // bind mount. + for _, mnt := range f.MountPoints { + if source == mnt.Path { + source = mnt.Device + break + } + } + } + // reuse MountPoint.Opts field to mark mount as readonly + opts = append(opts, option) + } + + // If target is a symlink, get its absolute path + absTarget, err := filepath.EvalSymlinks(target) + if err != nil { + absTarget = target + } + f.MountPoints = append(f.MountPoints, MountPoint{Device: source, Path: absTarget, Type: fstype, Opts: opts}) + klog.V(5).Infof("Fake mounter: mounted %s to %s", source, absTarget) + f.log = append(f.log, FakeAction{Action: FakeActionMount, Target: absTarget, Source: source, FSType: fstype}) + return nil +} + +// Unmount records the unmount event and updates the in-memory mount points for FakeMounter +func (f *FakeMounter) Unmount(target string) error { + f.mutex.Lock() + defer f.mutex.Unlock() + + // If target is a symlink, get its absolute path + absTarget, err := filepath.EvalSymlinks(target) + if err != nil { + absTarget = target + } + + newMountpoints := []MountPoint{} + for _, mp := range f.MountPoints { + if mp.Path == absTarget { + if f.UnmountFunc != nil { + err := f.UnmountFunc(absTarget) + if err != nil { + return err + } + } + klog.V(5).Infof("Fake mounter: unmounted %s from %s", mp.Device, absTarget) + // Don't copy it to newMountpoints + continue + } + newMountpoints = append(newMountpoints, MountPoint{Device: mp.Device, Path: mp.Path, Type: mp.Type}) + } + f.MountPoints = newMountpoints + f.log = append(f.log, FakeAction{Action: FakeActionUnmount, Target: absTarget}) + delete(f.MountCheckErrors, target) + return nil +} + +// List returns all the in-memory mountpoints for FakeMounter +func (f *FakeMounter) List() ([]MountPoint, error) { + f.mutex.Lock() + defer f.mutex.Unlock() + + return f.MountPoints, nil +} + +// IsLikelyNotMountPoint determines whether a path is a mountpoint by checking +// if the absolute path to file is in the in-memory mountpoints +func (f *FakeMounter) IsLikelyNotMountPoint(file string) (bool, error) { + f.mutex.Lock() + defer f.mutex.Unlock() + + err := f.MountCheckErrors[file] + if err != nil { + return false, err + } + + _, err = os.Stat(file) + if err != nil { + return true, err + } + + // If file is a symlink, get its absolute path + absFile, err := filepath.EvalSymlinks(file) + if err != nil { + absFile = file + } + + for _, mp := range f.MountPoints { + if mp.Path == absFile { + klog.V(5).Infof("isLikelyNotMountPoint for %s: mounted %s, false", file, mp.Path) + return false, nil + } + } + klog.V(5).Infof("isLikelyNotMountPoint for %s: true", file) + return true, nil +} + +// GetMountRefs finds all mount references to the path, returns a +// list of paths. +func (f *FakeMounter) GetMountRefs(pathname string) ([]string, error) { + realpath, err := filepath.EvalSymlinks(pathname) + if err != nil { + // Ignore error in FakeMounter, because we actually didn't create files. + realpath = pathname + } + return getMountRefsByDev(f, realpath) +} diff --git a/mount.go b/mount.go new file mode 100644 index 00000000000..821cc875345 --- /dev/null +++ b/mount.go @@ -0,0 +1,267 @@ +/* +Copyright 2014 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// TODO(thockin): This whole pkg is pretty linux-centric. As soon as we have +// an alternate platform, we will need to abstract further. + +package mount + +import ( + "os" + "path/filepath" + "strings" + + utilexec "k8s.io/utils/exec" +) + +const ( + // Default mount command if mounter path is not specified. + defaultMountCommand = "mount" +) + +// Interface defines the set of methods to allow for mount operations on a system. +type Interface interface { + // Mount mounts source to target as fstype with given options. + Mount(source string, target string, fstype string, options []string) error + // Unmount unmounts given target. + Unmount(target string) error + // List returns a list of all mounted filesystems. This can be large. + // On some platforms, reading mounts directly from the OS is not guaranteed + // consistent (i.e. it could change between chunked reads). This is guaranteed + // to be consistent. + List() ([]MountPoint, error) + // IsLikelyNotMountPoint uses heuristics to determine if a directory + // is not a mountpoint. + // It should return ErrNotExist when the directory does not exist. + // IsLikelyNotMountPoint does NOT properly detect all mountpoint types + // most notably linux bind mounts and symbolic link. For callers that do not + // care about such situations, this is a faster alternative to calling List() + // and scanning that output. + IsLikelyNotMountPoint(file string) (bool, error) + // GetMountRefs finds all mount references to pathname, returning a slice of + // paths. Pathname can be a mountpoint path or a normal directory + // (for bind mount). On Linux, pathname is excluded from the slice. + // For example, if /dev/sdc was mounted at /path/a and /path/b, + // GetMountRefs("/path/a") would return ["/path/b"] + // GetMountRefs("/path/b") would return ["/path/a"] + // On Windows there is no way to query all mount points; as long as pathname is + // a valid mount, it will be returned. + GetMountRefs(pathname string) ([]string, error) +} + +// Compile-time check to ensure all Mounter implementations satisfy +// the mount interface. +var _ Interface = &Mounter{} + +// MountPoint represents a single line in /proc/mounts or /etc/fstab. +type MountPoint struct { + Device string + Path string + Type string + Opts []string + Freq int + Pass int +} + +// SafeFormatAndMount probes a device to see if it is formatted. +// Namely it checks to see if a file system is present. If so it +// mounts it otherwise the device is formatted first then mounted. +type SafeFormatAndMount struct { + Interface + Exec utilexec.Interface +} + +// FormatAndMount formats the given disk, if needed, and mounts it. +// That is if the disk is not formatted and it is not being mounted as +// read-only it will format it first then mount it. Otherwise, if the +// disk is already formatted or it is being mounted as read-only, it +// will be mounted without formatting. +func (mounter *SafeFormatAndMount) FormatAndMount(source string, target string, fstype string, options []string) error { + return mounter.formatAndMount(source, target, fstype, options) +} + +// getMountRefsByDev finds all references to the device provided +// by mountPath; returns a list of paths. +// Note that mountPath should be path after the evaluation of any symblolic links. +func getMountRefsByDev(mounter Interface, mountPath string) ([]string, error) { + mps, err := mounter.List() + if err != nil { + return nil, err + } + + // Finding the device mounted to mountPath. + diskDev := "" + for i := range mps { + if mountPath == mps[i].Path { + diskDev = mps[i].Device + break + } + } + + // Find all references to the device. + var refs []string + for i := range mps { + if mps[i].Device == diskDev || mps[i].Device == mountPath { + if mps[i].Path != mountPath { + refs = append(refs, mps[i].Path) + } + } + } + return refs, nil +} + +// GetDeviceNameFromMount given a mnt point, find the device from /proc/mounts +// returns the device name, reference count, and error code. +func GetDeviceNameFromMount(mounter Interface, mountPath string) (string, int, error) { + mps, err := mounter.List() + if err != nil { + return "", 0, err + } + + // Find the device name. + // FIXME if multiple devices mounted on the same mount path, only the first one is returned. + device := "" + // If mountPath is symlink, need get its target path. + slTarget, err := filepath.EvalSymlinks(mountPath) + if err != nil { + slTarget = mountPath + } + for i := range mps { + if mps[i].Path == slTarget { + device = mps[i].Device + break + } + } + + // Find all references to the device. + refCount := 0 + for i := range mps { + if mps[i].Device == device { + refCount++ + } + } + return device, refCount, nil +} + +// IsNotMountPoint determines if a directory is a mountpoint. +// It should return ErrNotExist when the directory does not exist. +// IsNotMountPoint is more expensive than IsLikelyNotMountPoint. +// IsNotMountPoint detects bind mounts in linux. +// IsNotMountPoint enumerates all the mountpoints using List() and +// the list of mountpoints may be large, then it uses +// isMountPointMatch to evaluate whether the directory is a mountpoint. +func IsNotMountPoint(mounter Interface, file string) (bool, error) { + // IsLikelyNotMountPoint provides a quick check + // to determine whether file IS A mountpoint. + notMnt, notMntErr := mounter.IsLikelyNotMountPoint(file) + if notMntErr != nil && os.IsPermission(notMntErr) { + // We were not allowed to do the simple stat() check, e.g. on NFS with + // root_squash. Fall back to /proc/mounts check below. + notMnt = true + notMntErr = nil + } + if notMntErr != nil { + return notMnt, notMntErr + } + // identified as mountpoint, so return this fact. + if notMnt == false { + return notMnt, nil + } + + // Resolve any symlinks in file, kernel would do the same and use the resolved path in /proc/mounts. + resolvedFile, err := filepath.EvalSymlinks(file) + if err != nil { + return true, err + } + + // check all mountpoints since IsLikelyNotMountPoint + // is not reliable for some mountpoint types. + mountPoints, mountPointsErr := mounter.List() + if mountPointsErr != nil { + return notMnt, mountPointsErr + } + for _, mp := range mountPoints { + if isMountPointMatch(mp, resolvedFile) { + notMnt = false + break + } + } + return notMnt, nil +} + +// MakeBindOpts detects whether a bind mount is being requested and makes the remount options to +// use in case of bind mount, due to the fact that bind mount doesn't respect mount options. +// The list equals: +// options - 'bind' + 'remount' (no duplicate) +func MakeBindOpts(options []string) (bool, []string, []string) { + // Because we have an FD opened on the subpath bind mount, the "bind" option + // needs to be included, otherwise the mount target will error as busy if you + // remount as readonly. + // + // As a consequence, all read only bind mounts will no longer change the underlying + // volume mount to be read only. + bindRemountOpts := []string{"bind", "remount"} + bind := false + bindOpts := []string{"bind"} + + // _netdev is a userspace mount option and does not automatically get added when + // bind mount is created and hence we must carry it over. + if checkForNetDev(options) { + bindOpts = append(bindOpts, "_netdev") + } + + for _, option := range options { + switch option { + case "bind": + bind = true + break + case "remount": + break + default: + bindRemountOpts = append(bindRemountOpts, option) + } + } + + return bind, bindOpts, bindRemountOpts +} + +func checkForNetDev(options []string) bool { + for _, option := range options { + if option == "_netdev" { + return true + } + } + return false +} + +// PathWithinBase checks if give path is within given base directory. +func PathWithinBase(fullPath, basePath string) bool { + rel, err := filepath.Rel(basePath, fullPath) + if err != nil { + return false + } + if StartsWithBackstep(rel) { + // Needed to escape the base path. + return false + } + return true +} + +// StartsWithBackstep checks if the given path starts with a backstep segment. +func StartsWithBackstep(rel string) bool { + // normalize to / and check for ../ + return rel == ".." || strings.HasPrefix(filepath.ToSlash(rel), "../") +} diff --git a/mount_helper_common.go b/mount_helper_common.go new file mode 100644 index 00000000000..81f91a8be8d --- /dev/null +++ b/mount_helper_common.go @@ -0,0 +1,99 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "fmt" + "os" + + "k8s.io/klog" +) + +// CleanupMountPoint unmounts the given path and deletes the remaining directory +// if successful. If extensiveMountPointCheck is true IsNotMountPoint will be +// called instead of IsLikelyNotMountPoint. IsNotMountPoint is more expensive +// but properly handles bind mounts within the same fs. +func CleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) error { + pathExists, pathErr := PathExists(mountPath) + if !pathExists { + klog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath) + return nil + } + corruptedMnt := IsCorruptedMnt(pathErr) + if pathErr != nil && !corruptedMnt { + return fmt.Errorf("Error checking path: %v", pathErr) + } + return doCleanupMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt) +} + +// doCleanupMountPoint unmounts the given path and +// deletes the remaining directory if successful. +// if extensiveMountPointCheck is true +// IsNotMountPoint will be called instead of IsLikelyNotMountPoint. +// IsNotMountPoint is more expensive but properly handles bind mounts within the same fs. +// if corruptedMnt is true, it means that the mountPath is a corrupted mountpoint, and the mount point check +// will be skipped +func doCleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool, corruptedMnt bool) error { + if !corruptedMnt { + var notMnt bool + var err error + if extensiveMountPointCheck { + notMnt, err = IsNotMountPoint(mounter, mountPath) + } else { + notMnt, err = mounter.IsLikelyNotMountPoint(mountPath) + } + + if err != nil { + return err + } + + if notMnt { + klog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath) + return os.Remove(mountPath) + } + } + + // Unmount the mount path + klog.V(4).Infof("%q is a mountpoint, unmounting", mountPath) + if err := mounter.Unmount(mountPath); err != nil { + return err + } + + notMnt, mntErr := mounter.IsLikelyNotMountPoint(mountPath) + if mntErr != nil { + return mntErr + } + if notMnt { + klog.V(4).Infof("%q is unmounted, deleting the directory", mountPath) + return os.Remove(mountPath) + } + return fmt.Errorf("Failed to unmount path %v", mountPath) +} + +// PathExists returns true if the specified path exists. +// TODO: clean this up to use pkg/util/file/FileExists +func PathExists(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } else if os.IsNotExist(err) { + return false, nil + } else if IsCorruptedMnt(err) { + return true, err + } + return false, err +} diff --git a/mount_helper_test.go b/mount_helper_test.go new file mode 100644 index 00000000000..91be5cf956a --- /dev/null +++ b/mount_helper_test.go @@ -0,0 +1,139 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "runtime" + "syscall" + "testing" +) + +func TestDoCleanupMountPoint(t *testing.T) { + + if runtime.GOOS == "darwin" { + t.Skipf("not supported on GOOS=%s", runtime.GOOS) + } + + const testMount = "test-mount" + const defaultPerm = 0750 + + tests := map[string]struct { + corruptedMnt bool + // Function that prepares the directory structure for the test under + // the given base directory. + // Returns a fake MountPoint, a fake error for the mount point, + // and error if the prepare function encountered a fatal error. + prepare func(base string) (MountPoint, error, error) + expectErr bool + }{ + "mount-ok": { + prepare: func(base string) (MountPoint, error, error) { + path := filepath.Join(base, testMount) + if err := os.MkdirAll(path, defaultPerm); err != nil { + return MountPoint{}, nil, err + } + return MountPoint{Device: "/dev/sdb", Path: path}, nil, nil + }, + }, + "mount-corrupted": { + prepare: func(base string) (MountPoint, error, error) { + path := filepath.Join(base, testMount) + if err := os.MkdirAll(path, defaultPerm); err != nil { + return MountPoint{}, nil, err + } + return MountPoint{Device: "/dev/sdb", Path: path}, os.NewSyscallError("fake", syscall.ESTALE), nil + }, + corruptedMnt: true, + }, + "mount-err-not-corrupted": { + prepare: func(base string) (MountPoint, error, error) { + path := filepath.Join(base, testMount) + if err := os.MkdirAll(path, defaultPerm); err != nil { + return MountPoint{}, nil, err + } + return MountPoint{Device: "/dev/sdb", Path: path}, os.NewSyscallError("fake", syscall.ETIMEDOUT), nil + }, + expectErr: true, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + + tmpDir, err := ioutil.TempDir("", "unmount-mount-point-test") + if err != nil { + t.Fatalf("failed to create tmpdir: %v", err) + } + defer os.RemoveAll(tmpDir) + + if tt.prepare == nil { + t.Fatalf("prepare function required") + } + + mountPoint, mountError, err := tt.prepare(tmpDir) + if err != nil { + t.Fatalf("failed to prepare test: %v", err) + } + + fake := NewFakeMounter( + []MountPoint{mountPoint}, + ) + fake.MountCheckErrors = map[string]error{mountPoint.Path: mountError} + + err = doCleanupMountPoint(mountPoint.Path, fake, true, tt.corruptedMnt) + if tt.expectErr { + if err == nil { + t.Errorf("test %s failed, expected error, got none", name) + } + if err := validateDirExists(mountPoint.Path); err != nil { + t.Errorf("test %s failed, mount path doesn't exist: %v", name, err) + } + } + if !tt.expectErr { + if err != nil { + t.Errorf("test %s failed: %v", name, err) + } + if err := validateDirNotExists(mountPoint.Path); err != nil { + t.Errorf("test %s failed, mount path still exists: %v", name, err) + } + } + }) + } +} + +func validateDirExists(dir string) error { + _, err := ioutil.ReadDir(dir) + if err != nil { + return err + } + return nil +} + +func validateDirNotExists(dir string) error { + _, err := ioutil.ReadDir(dir) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + return fmt.Errorf("dir %q still exists", dir) +} diff --git a/mount_helper_unix.go b/mount_helper_unix.go new file mode 100644 index 00000000000..9c91e158264 --- /dev/null +++ b/mount_helper_unix.go @@ -0,0 +1,140 @@ +// +build !windows + +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "fmt" + "os" + "strconv" + "strings" + "syscall" + + utilio "k8s.io/utils/io" +) + +const ( + // At least number of fields per line in /proc//mountinfo. + expectedAtLeastNumFieldsPerMountInfo = 10 + // How many times to retry for a consistent read of /proc/mounts. + maxListTries = 3 +) + +// IsCorruptedMnt return true if err is about corrupted mount point +func IsCorruptedMnt(err error) bool { + if err == nil { + return false + } + var underlyingError error + switch pe := err.(type) { + case nil: + return false + case *os.PathError: + underlyingError = pe.Err + case *os.LinkError: + underlyingError = pe.Err + case *os.SyscallError: + underlyingError = pe.Err + } + + return underlyingError == syscall.ENOTCONN || underlyingError == syscall.ESTALE || underlyingError == syscall.EIO || underlyingError == syscall.EACCES +} + +// MountInfo represents a single line in /proc//mountinfo. +type MountInfo struct { + // Unique ID for the mount (maybe reused after umount). + ID int + // The ID of the parent mount (or of self for the root of this mount namespace's mount tree). + ParentID int + // The value of `st_dev` for files on this filesystem. + MajorMinor string + // The pathname of the directory in the filesystem which forms the root of this mount. + Root string + // Mount source, filesystem-specific information. e.g. device, tmpfs name. + Source string + // Mount point, the pathname of the mount point. + MountPoint string + // Optional fieds, zero or more fields of the form "tag[:value]". + OptionalFields []string + // The filesystem type in the form "type[.subtype]". + FsType string + // Per-mount options. + MountOptions []string + // Per-superblock options. + SuperOptions []string +} + +// ParseMountInfo parses /proc/xxx/mountinfo. +func ParseMountInfo(filename string) ([]MountInfo, error) { + content, err := utilio.ConsistentRead(filename, maxListTries) + if err != nil { + return []MountInfo{}, err + } + contentStr := string(content) + infos := []MountInfo{} + + for _, line := range strings.Split(contentStr, "\n") { + if line == "" { + // the last split() item is empty string following the last \n + continue + } + // See `man proc` for authoritative description of format of the file. + fields := strings.Fields(line) + if len(fields) < expectedAtLeastNumFieldsPerMountInfo { + return nil, fmt.Errorf("wrong number of fields in (expected at least %d, got %d): %s", expectedAtLeastNumFieldsPerMountInfo, len(fields), line) + } + id, err := strconv.Atoi(fields[0]) + if err != nil { + return nil, err + } + parentID, err := strconv.Atoi(fields[1]) + if err != nil { + return nil, err + } + info := MountInfo{ + ID: id, + ParentID: parentID, + MajorMinor: fields[2], + Root: fields[3], + MountPoint: fields[4], + MountOptions: strings.Split(fields[5], ","), + } + // All fields until "-" are "optional fields". + i := 6 + for ; i < len(fields) && fields[i] != "-"; i++ { + info.OptionalFields = append(info.OptionalFields, fields[i]) + } + // Parse the rest 3 fields. + i++ + if len(fields)-i < 3 { + return nil, fmt.Errorf("expect 3 fields in %s, got %d", line, len(fields)-i) + } + info.FsType = fields[i] + info.Source = fields[i+1] + info.SuperOptions = strings.Split(fields[i+2], ",") + infos = append(infos, info) + } + return infos, nil +} + +// isMountPointMatch returns true if the path in mp is the same as dir. +// Handles case where mountpoint dir has been renamed due to stale NFS mount. +func isMountPointMatch(mp MountPoint, dir string) bool { + deletedDir := fmt.Sprintf("%s\\040(deleted)", dir) + return ((mp.Path == dir) || (mp.Path == deletedDir)) +} diff --git a/mount_helper_unix_test.go b/mount_helper_unix_test.go new file mode 100644 index 00000000000..f364b02863c --- /dev/null +++ b/mount_helper_unix_test.go @@ -0,0 +1,215 @@ +// +build !windows + +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "io/ioutil" + "os" + "path/filepath" + "reflect" + "testing" +) + +func writeFile(content string) (string, string, error) { + tempDir, err := ioutil.TempDir("", "mounter_shared_test") + if err != nil { + return "", "", err + } + filename := filepath.Join(tempDir, "mountinfo") + err = ioutil.WriteFile(filename, []byte(content), 0600) + if err != nil { + os.RemoveAll(tempDir) + return "", "", err + } + return tempDir, filename, nil +} + +func TestParseMountInfo(t *testing.T) { + info := + `62 0 253:0 / / rw,relatime shared:1 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered +78 62 0:41 / /tmp rw,nosuid,nodev shared:30 - tmpfs tmpfs rw,seclabel +80 62 0:42 / /var/lib/nfs/rpc_pipefs rw,relatime shared:31 - rpc_pipefs sunrpc rw +82 62 0:43 / /var/lib/foo rw,relatime shared:32 - tmpfs tmpfs rw +83 63 0:44 / /var/lib/bar rw,relatime - tmpfs tmpfs rw +227 62 253:0 /var/lib/docker/devicemapper /var/lib/docker/devicemapper rw,relatime - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered +224 62 253:0 /var/lib/docker/devicemapper/test/shared /var/lib/docker/devicemapper/test/shared rw,relatime master:1 shared:44 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered +76 17 8:1 / /mnt/stateful_partition rw,nosuid,nodev,noexec,relatime - ext4 /dev/sda1 rw,commit=30,data=ordered +80 17 8:1 /var /var rw,nosuid,nodev,noexec,relatime shared:30 - ext4 /dev/sda1 rw,commit=30,data=ordered +189 80 8:1 /var/lib/kubelet /var/lib/kubelet rw,relatime shared:30 - ext4 /dev/sda1 rw,commit=30,data=ordered +818 77 8:40 / /var/lib/kubelet/pods/c25464af-e52e-11e7-ab4d-42010a800002/volumes/kubernetes.io~gce-pd/vol1 rw,relatime shared:290 - ext4 /dev/sdc rw,data=ordered +819 78 8:48 / /var/lib/kubelet/pods/c25464af-e52e-11e7-ab4d-42010a800002/volumes/kubernetes.io~gce-pd/vol1 rw,relatime shared:290 - ext4 /dev/sdd rw,data=ordered +900 100 8:48 /dir1 /var/lib/kubelet/pods/c25464af-e52e-11e7-ab4d-42010a800002/volume-subpaths/vol1/subpath1/0 rw,relatime shared:290 - ext4 /dev/sdd rw,data=ordered +901 101 8:1 /dir1 /var/lib/kubelet/pods/c25464af-e52e-11e7-ab4d-42010a800002/volume-subpaths/vol1/subpath1/1 rw,relatime shared:290 - ext4 /dev/sdd rw,data=ordered +902 102 8:1 /var/lib/kubelet/pods/d4076f24-e53a-11e7-ba15-42010a800002/volumes/kubernetes.io~empty-dir/vol1/dir1 /var/lib/kubelet/pods/d4076f24-e53a-11e7-ba15-42010a800002/volume-subpaths/vol1/subpath1/0 rw,relatime shared:30 - ext4 /dev/sda1 rw,commit=30,data=ordered +903 103 8:1 /var/lib/kubelet/pods/d4076f24-e53a-11e7-ba15-42010a800002/volumes/kubernetes.io~empty-dir/vol2/dir1 /var/lib/kubelet/pods/d4076f24-e53a-11e7-ba15-42010a800002/volume-subpaths/vol1/subpath1/1 rw,relatime shared:30 - ext4 /dev/sda1 rw,commit=30,data=ordered +178 25 253:0 /etc/bar /var/lib/kubelet/pods/12345/volume-subpaths/vol1/subpath1/0 rw,relatime shared:1 - ext4 /dev/sdb2 rw,errors=remount-ro,data=ordered +698 186 0:41 /tmp1/dir1 /var/lib/kubelet/pods/41135147-e697-11e7-9342-42010a800002/volume-subpaths/vol1/subpath1/0 rw shared:26 - tmpfs tmpfs rw +918 77 8:50 / /var/lib/kubelet/pods/2345/volumes/kubernetes.io~gce-pd/vol1 rw,relatime shared:290 - ext4 /dev/sdc rw,data=ordered +919 78 8:58 / /var/lib/kubelet/pods/2345/volumes/kubernetes.io~gce-pd/vol1 rw,relatime shared:290 - ext4 /dev/sdd rw,data=ordered +920 100 8:50 /dir1 /var/lib/kubelet/pods/2345/volume-subpaths/vol1/subpath1/0 rw,relatime shared:290 - ext4 /dev/sdc rw,data=ordered +150 23 1:58 / /media/nfs_vol rw,relatime shared:89 - nfs4 172.18.4.223:/srv/nfs rw,vers=4.0,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=172.18.4.223,local_lock=none,addr=172.18.4.223 +151 24 1:58 / /media/nfs_bindmount rw,relatime shared:89 - nfs4 172.18.4.223:/srv/nfs/foo rw,vers=4.0,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=172.18.4.223,local_lock=none,addr=172.18.4.223 +134 23 0:58 / /var/lib/kubelet/pods/43219158-e5e1-11e7-a392-0e858b8eaf40/volumes/kubernetes.io~nfs/nfs1 rw,relatime shared:89 - nfs4 172.18.4.223:/srv/nfs rw,vers=4.0,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=172.18.4.223,local_lock=none,addr=172.18.4.223 +187 23 0:58 / /var/lib/kubelet/pods/1fc5ea21-eff4-11e7-ac80-0e858b8eaf40/volumes/kubernetes.io~nfs/nfs2 rw,relatime shared:96 - nfs4 172.18.4.223:/srv/nfs2 rw,vers=4.0,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=172.18.4.223,local_lock=none,addr=172.18.4.223 +188 24 0:58 / /var/lib/kubelet/pods/43219158-e5e1-11e7-a392-0e858b8eaf40/volume-subpaths/nfs1/subpath1/0 rw,relatime shared:89 - nfs4 172.18.4.223:/srv/nfs/foo rw,vers=4.0,rsize=524288,wsize=524288,namlen=255,hard,proto=tcp,port=0,timeo=600,retrans=2,sec=sys,clientaddr=172.18.4.223,local_lock=none,addr=172.18.4.223 +347 60 0:71 / /var/lib/kubelet/pods/13195d46-f9fa-11e7-bbf1-5254007a695a/volumes/kubernetes.io~nfs/vol2 rw,relatime shared:170 - nfs 172.17.0.3:/exports/2 rw,vers=3,rsize=1048576,wsize=1048576,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,mountaddr=172.17.0.3,mountvers=3,mountport=20048,mountproto=udp,local_lock=none,addr=172.17.0.3 +222 24 253:0 /tmp/src /mnt/dst rw,relatime shared:1 - ext4 /dev/mapper/vagrant--vg-root rw,errors=remount-ro,data=ordered +28 18 0:24 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:9 - tmpfs tmpfs ro,mode=755 +29 28 0:25 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:10 - cgroup cgroup rw,xattr,release_agent=/lib/systemd/systemd-cgroups-agent,name=systemd +31 28 0:27 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:13 - cgroup cgroup rw,cpuset +32 28 0:28 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:14 - cgroup cgroup rw,cpu,cpuacct +33 28 0:29 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:15 - cgroup cgroup rw,freezer +34 28 0:30 / /sys/fs/cgroup/net_cls,net_prio rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,net_cls,net_prio +35 28 0:31 / /sys/fs/cgroup/pids rw,nosuid,nodev,noexec,relatime shared:17 - cgroup cgroup rw,pids +36 28 0:32 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,devices +37 28 0:33 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime shared:19 - cgroup cgroup rw,hugetlb +38 28 0:34 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:20 - cgroup cgroup rw,blkio +39 28 0:35 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:21 - cgroup cgroup rw,memory +40 28 0:36 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:22 - cgroup cgroup rw,perf_event +` + tempDir, filename, err := writeFile(info) + if err != nil { + t.Fatalf("cannot create temporary file: %v", err) + } + defer os.RemoveAll(tempDir) + + tests := []struct { + name string + id int + expectedInfo MountInfo + }{ + { + "simple bind mount", + 189, + MountInfo{ + ID: 189, + ParentID: 80, + MajorMinor: "8:1", + Root: "/var/lib/kubelet", + Source: "/dev/sda1", + MountPoint: "/var/lib/kubelet", + OptionalFields: []string{"shared:30"}, + FsType: "ext4", + MountOptions: []string{"rw", "relatime"}, + SuperOptions: []string{"rw", "commit=30", "data=ordered"}, + }, + }, + { + "bind mount a directory", + 222, + MountInfo{ + ID: 222, + ParentID: 24, + MajorMinor: "253:0", + Root: "/tmp/src", + Source: "/dev/mapper/vagrant--vg-root", + MountPoint: "/mnt/dst", + OptionalFields: []string{"shared:1"}, + FsType: "ext4", + MountOptions: []string{"rw", "relatime"}, + SuperOptions: []string{"rw", "errors=remount-ro", "data=ordered"}, + }, + }, + { + "more than one optional fields", + 224, + MountInfo{ + ID: 224, + ParentID: 62, + MajorMinor: "253:0", + Root: "/var/lib/docker/devicemapper/test/shared", + Source: "/dev/mapper/ssd-root", + MountPoint: "/var/lib/docker/devicemapper/test/shared", + OptionalFields: []string{"master:1", "shared:44"}, + FsType: "ext4", + MountOptions: []string{"rw", "relatime"}, + SuperOptions: []string{"rw", "seclabel", "data=ordered"}, + }, + }, + { + "cgroup-mountpoint", + 28, + MountInfo{ + ID: 28, + ParentID: 18, + MajorMinor: "0:24", + Root: "/", + Source: "tmpfs", + MountPoint: "/sys/fs/cgroup", + OptionalFields: []string{"shared:9"}, + FsType: "tmpfs", + MountOptions: []string{"ro", "nosuid", "nodev", "noexec"}, + SuperOptions: []string{"ro", "mode=755"}, + }, + }, + { + "cgroup-subsystem-systemd-mountpoint", + 29, + MountInfo{ + ID: 29, + ParentID: 28, + MajorMinor: "0:25", + Root: "/", + Source: "cgroup", + MountPoint: "/sys/fs/cgroup/systemd", + OptionalFields: []string{"shared:10"}, + FsType: "cgroup", + MountOptions: []string{"rw", "nosuid", "nodev", "noexec", "relatime"}, + SuperOptions: []string{"rw", "xattr", "release_agent=/lib/systemd/systemd-cgroups-agent", "name=systemd"}, + }, + }, + { + "cgroup-subsystem-cpuset-mountpoint", + 31, + MountInfo{ + ID: 31, + ParentID: 28, + MajorMinor: "0:27", + Root: "/", + Source: "cgroup", + MountPoint: "/sys/fs/cgroup/cpuset", + OptionalFields: []string{"shared:13"}, + FsType: "cgroup", + MountOptions: []string{"rw", "nosuid", "nodev", "noexec", "relatime"}, + SuperOptions: []string{"rw", "cpuset"}, + }, + }, + } + + infos, err := ParseMountInfo(filename) + if err != nil { + t.Fatalf("Cannot parse %s: %s", filename, err) + } + + for _, test := range tests { + found := false + for _, info := range infos { + if info.ID == test.id { + found = true + if !reflect.DeepEqual(info, test.expectedInfo) { + t.Errorf("Test case %q:\n expected: %+v\n got: %+v", test.name, test.expectedInfo, info) + } + break + } + } + if !found { + t.Errorf("Test case %q: mountPoint %d not found", test.name, test.id) + } + } +} diff --git a/mount_helper_windows.go b/mount_helper_windows.go new file mode 100644 index 00000000000..5e6b1dd9adc --- /dev/null +++ b/mount_helper_windows.go @@ -0,0 +1,98 @@ +// +build windows + +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "fmt" + "os" + "strconv" + "strings" + "syscall" + + "k8s.io/klog" +) + +// following failure codes are from https://docs.microsoft.com/en-us/windows/desktop/debug/system-error-codes--1300-1699- +// ERROR_BAD_NETPATH = 53 +// ERROR_NETWORK_BUSY = 54 +// ERROR_UNEXP_NET_ERR = 59 +// ERROR_NETNAME_DELETED = 64 +// ERROR_NETWORK_ACCESS_DENIED = 65 +// ERROR_BAD_DEV_TYPE = 66 +// ERROR_BAD_NET_NAME = 67 +// ERROR_SESSION_CREDENTIAL_CONFLICT = 1219 +// ERROR_LOGON_FAILURE = 1326 +var errorNoList = [...]int{53, 54, 59, 64, 65, 66, 67, 1219, 1326} + +// IsCorruptedMnt return true if err is about corrupted mount point +func IsCorruptedMnt(err error) bool { + if err == nil { + return false + } + + var underlyingError error + switch pe := err.(type) { + case nil: + return false + case *os.PathError: + underlyingError = pe.Err + case *os.LinkError: + underlyingError = pe.Err + case *os.SyscallError: + underlyingError = pe.Err + } + + if ee, ok := underlyingError.(syscall.Errno); ok { + for _, errno := range errorNoList { + if int(ee) == errno { + klog.Warningf("IsCorruptedMnt failed with error: %v, error code: %v", err, errno) + return true + } + } + } + + return false +} + +func NormalizeWindowsPath(path string) string { + normalizedPath := strings.Replace(path, "/", "\\", -1) + if strings.HasPrefix(normalizedPath, "\\") { + normalizedPath = "c:" + normalizedPath + } + return normalizedPath +} + +// ValidateDiskNumber : disk number should be a number in [0, 99] +func ValidateDiskNumber(disk string) error { + diskNum, err := strconv.Atoi(disk) + if err != nil { + return fmt.Errorf("wrong disk number format: %q, err:%v", disk, err) + } + + if diskNum < 0 || diskNum > 99 { + return fmt.Errorf("disk number out of range: %q", disk) + } + + return nil +} + +// isMountPointMatch determines if the mountpoint matches the dir +func isMountPointMatch(mp MountPoint, dir string) bool { + return mp.Path == dir +} diff --git a/mount_helper_windows_test.go b/mount_helper_windows_test.go new file mode 100644 index 00000000000..7f5e6eb1827 --- /dev/null +++ b/mount_helper_windows_test.go @@ -0,0 +1,65 @@ +// +build windows + +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "testing" +) + +func TestNormalizeWindowsPath(t *testing.T) { + path := `/var/lib/kubelet/pods/146f8428-83e7-11e7-8dd4-000d3a31dac4/volumes/kubernetes.io~azure-disk` + normalizedPath := NormalizeWindowsPath(path) + if normalizedPath != `c:\var\lib\kubelet\pods\146f8428-83e7-11e7-8dd4-000d3a31dac4\volumes\kubernetes.io~azure-disk` { + t.Errorf("normizeWindowsPath test failed, normalizedPath : %q", normalizedPath) + } + + path = `/var/lib/kubelet/pods/146f8428-83e7-11e7-8dd4-000d3a31dac4\volumes\kubernetes.io~azure-disk` + normalizedPath = NormalizeWindowsPath(path) + if normalizedPath != `c:\var\lib\kubelet\pods\146f8428-83e7-11e7-8dd4-000d3a31dac4\volumes\kubernetes.io~azure-disk` { + t.Errorf("normizeWindowsPath test failed, normalizedPath : %q", normalizedPath) + } + + path = `/` + normalizedPath = NormalizeWindowsPath(path) + if normalizedPath != `c:\` { + t.Errorf("normizeWindowsPath test failed, normalizedPath : %q", normalizedPath) + } +} + +func TestValidateDiskNumber(t *testing.T) { + diskNum := "0" + if err := ValidateDiskNumber(diskNum); err != nil { + t.Errorf("TestValidateDiskNumber test failed, disk number : %s", diskNum) + } + + diskNum = "99" + if err := ValidateDiskNumber(diskNum); err != nil { + t.Errorf("TestValidateDiskNumber test failed, disk number : %s", diskNum) + } + + diskNum = "ab" + if err := ValidateDiskNumber(diskNum); err == nil { + t.Errorf("TestValidateDiskNumber test failed, disk number : %s", diskNum) + } + + diskNum = "100" + if err := ValidateDiskNumber(diskNum); err == nil { + t.Errorf("TestValidateDiskNumber test failed, disk number : %s", diskNum) + } +} diff --git a/mount_linux.go b/mount_linux.go new file mode 100644 index 00000000000..2a910024297 --- /dev/null +++ b/mount_linux.go @@ -0,0 +1,489 @@ +// +build linux + +/* +Copyright 2014 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + + "k8s.io/klog" + utilexec "k8s.io/utils/exec" + utilio "k8s.io/utils/io" +) + +const ( + // Number of fields per line in /proc/mounts as per the fstab man page. + expectedNumFieldsPerLine = 6 + // Location of the mount file to use + procMountsPath = "/proc/mounts" + // Location of the mountinfo file + procMountInfoPath = "/proc/self/mountinfo" + // 'fsck' found errors and corrected them + fsckErrorsCorrected = 1 + // 'fsck' found errors but exited without correcting them + fsckErrorsUncorrected = 4 +) + +// Mounter provides the default implementation of mount.Interface +// for the linux platform. This implementation assumes that the +// kubelet is running in the host's root mount namespace. +type Mounter struct { + mounterPath string + withSystemd bool +} + +// New returns a mount.Interface for the current system. +// It provides options to override the default mounter behavior. +// mounterPath allows using an alternative to `/bin/mount` for mounting. +func New(mounterPath string) Interface { + return &Mounter{ + mounterPath: mounterPath, + withSystemd: detectSystemd(), + } +} + +// Mount mounts source to target as fstype with given options. 'source' and 'fstype' must +// be an empty string in case it's not required, e.g. for remount, or for auto filesystem +// type, where kernel handles fstype for you. The mount 'options' is a list of options, +// currently come from mount(8), e.g. "ro", "remount", "bind", etc. If no more option is +// required, call Mount with an empty string list or nil. +func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error { + // Path to mounter binary if containerized mounter is needed. Otherwise, it is set to empty. + // All Linux distros are expected to be shipped with a mount utility that a support bind mounts. + mounterPath := "" + bind, bindOpts, bindRemountOpts := MakeBindOpts(options) + if bind { + err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts) + if err != nil { + return err + } + return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts) + } + // The list of filesystems that require containerized mounter on GCI image cluster + fsTypesNeedMounter := map[string]struct{}{ + "nfs": {}, + "glusterfs": {}, + "ceph": {}, + "cifs": {}, + } + if _, ok := fsTypesNeedMounter[fstype]; ok { + mounterPath = mounter.mounterPath + } + return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, options) +} + +// doMount runs the mount command. mounterPath is the path to mounter binary if containerized mounter is used. +func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source string, target string, fstype string, options []string) error { + mountArgs := MakeMountArgs(source, target, fstype, options) + if len(mounterPath) > 0 { + mountArgs = append([]string{mountCmd}, mountArgs...) + mountCmd = mounterPath + } + + if mounter.withSystemd { + // Try to run mount via systemd-run --scope. This will escape the + // service where kubelet runs and any fuse daemons will be started in a + // specific scope. kubelet service than can be restarted without killing + // these fuse daemons. + // + // Complete command line (when mounterPath is not used): + // systemd-run --description=... --scope -- mount -t + // + // Expected flow: + // * systemd-run creates a transient scope (=~ cgroup) and executes its + // argument (/bin/mount) there. + // * mount does its job, forks a fuse daemon if necessary and finishes. + // (systemd-run --scope finishes at this point, returning mount's exit + // code and stdout/stderr - thats one of --scope benefits). + // * systemd keeps the fuse daemon running in the scope (i.e. in its own + // cgroup) until the fuse daemon dies (another --scope benefit). + // Kubelet service can be restarted and the fuse daemon survives. + // * When the fuse daemon dies (e.g. during unmount) systemd removes the + // scope automatically. + // + // systemd-mount is not used because it's too new for older distros + // (CentOS 7, Debian Jessie). + mountCmd, mountArgs = AddSystemdScope("systemd-run", target, mountCmd, mountArgs) + } else { + // No systemd-run on the host (or we failed to check it), assume kubelet + // does not run as a systemd service. + // No code here, mountCmd and mountArgs are already populated. + } + + klog.V(4).Infof("Mounting cmd (%s) with arguments (%s)", mountCmd, mountArgs) + command := exec.Command(mountCmd, mountArgs...) + output, err := command.CombinedOutput() + if err != nil { + args := strings.Join(mountArgs, " ") + klog.Errorf("Mount failed: %v\nMounting command: %s\nMounting arguments: %s\nOutput: %s\n", err, mountCmd, args, string(output)) + return fmt.Errorf("mount failed: %v\nMounting command: %s\nMounting arguments: %s\nOutput: %s", + err, mountCmd, args, string(output)) + } + return err +} + +// detectSystemd returns true if OS runs with systemd as init. When not sure +// (permission errors, ...), it returns false. +// There may be different ways how to detect systemd, this one makes sure that +// systemd-runs (needed by Mount()) works. +func detectSystemd() bool { + if _, err := exec.LookPath("systemd-run"); err != nil { + klog.V(2).Infof("Detected OS without systemd") + return false + } + // Try to run systemd-run --scope /bin/true, that should be enough + // to make sure that systemd is really running and not just installed, + // which happens when running in a container with a systemd-based image + // but with different pid 1. + cmd := exec.Command("systemd-run", "--description=Kubernetes systemd probe", "--scope", "true") + output, err := cmd.CombinedOutput() + if err != nil { + klog.V(2).Infof("Cannot run systemd-run, assuming non-systemd OS") + klog.V(4).Infof("systemd-run failed with: %v", err) + klog.V(4).Infof("systemd-run output: %s", string(output)) + return false + } + klog.V(2).Infof("Detected OS with systemd") + return true +} + +// MakeMountArgs makes the arguments to the mount(8) command. +// Implementation is shared with NsEnterMounter +func MakeMountArgs(source, target, fstype string, options []string) []string { + // Build mount command as follows: + // mount [-t $fstype] [-o $options] [$source] $target + mountArgs := []string{} + if len(fstype) > 0 { + mountArgs = append(mountArgs, "-t", fstype) + } + if len(options) > 0 { + mountArgs = append(mountArgs, "-o", strings.Join(options, ",")) + } + if len(source) > 0 { + mountArgs = append(mountArgs, source) + } + mountArgs = append(mountArgs, target) + + return mountArgs +} + +// AddSystemdScope adds "system-run --scope" to given command line +// implementation is shared with NsEnterMounter +func AddSystemdScope(systemdRunPath, mountName, command string, args []string) (string, []string) { + descriptionArg := fmt.Sprintf("--description=Kubernetes transient mount for %s", mountName) + systemdRunArgs := []string{descriptionArg, "--scope", "--", command} + return systemdRunPath, append(systemdRunArgs, args...) +} + +// Unmount unmounts the target. +func (mounter *Mounter) Unmount(target string) error { + klog.V(4).Infof("Unmounting %s", target) + command := exec.Command("umount", target) + output, err := command.CombinedOutput() + if err != nil { + return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", err, target, string(output)) + } + return nil +} + +// List returns a list of all mounted filesystems. +func (*Mounter) List() ([]MountPoint, error) { + return ListProcMounts(procMountsPath) +} + +// IsLikelyNotMountPoint determines if a directory is not a mountpoint. +// It is fast but not necessarily ALWAYS correct. If the path is in fact +// a bind mount from one part of a mount to another it will not be detected. +// It also can not distinguish between mountpoints and symbolic links. +// mkdir /tmp/a /tmp/b; mount --bind /tmp/a /tmp/b; IsLikelyNotMountPoint("/tmp/b") +// will return true. When in fact /tmp/b is a mount point. If this situation +// is of interest to you, don't use this function... +func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) { + stat, err := os.Stat(file) + if err != nil { + return true, err + } + rootStat, err := os.Stat(filepath.Dir(strings.TrimSuffix(file, "/"))) + if err != nil { + return true, err + } + // If the directory has a different device as parent, then it is a mountpoint. + if stat.Sys().(*syscall.Stat_t).Dev != rootStat.Sys().(*syscall.Stat_t).Dev { + return false, nil + } + + return true, nil +} + +// GetMountRefs finds all mount references to pathname, returns a +// list of paths. Path could be a mountpoint or a normal +// directory (for bind mount). +func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { + pathExists, pathErr := PathExists(pathname) + if !pathExists { + return []string{}, nil + } else if IsCorruptedMnt(pathErr) { + klog.Warningf("GetMountRefs found corrupted mount at %s, treating as unmounted path", pathname) + return []string{}, nil + } else if pathErr != nil { + return nil, fmt.Errorf("error checking path %s: %v", pathname, pathErr) + } + realpath, err := filepath.EvalSymlinks(pathname) + if err != nil { + return nil, err + } + return SearchMountPoints(realpath, procMountInfoPath) +} + +// formatAndMount uses unix utils to format and mount the given disk +func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error { + readOnly := false + for _, option := range options { + if option == "ro" { + readOnly = true + break + } + } + + options = append(options, "defaults") + + if !readOnly { + // Run fsck on the disk to fix repairable issues, only do this for volumes requested as rw. + klog.V(4).Infof("Checking for issues with fsck on disk: %s", source) + args := []string{"-a", source} + out, err := mounter.Exec.Command("fsck", args...).CombinedOutput() + if err != nil { + ee, isExitError := err.(utilexec.ExitError) + switch { + case err == utilexec.ErrExecutableNotFound: + klog.Warningf("'fsck' not found on system; continuing mount without running 'fsck'.") + case isExitError && ee.ExitStatus() == fsckErrorsCorrected: + klog.Infof("Device %s has errors which were corrected by fsck.", source) + case isExitError && ee.ExitStatus() == fsckErrorsUncorrected: + return fmt.Errorf("'fsck' found errors on device %s but could not correct them: %s", source, string(out)) + case isExitError && ee.ExitStatus() > fsckErrorsUncorrected: + klog.Infof("`fsck` error %s", string(out)) + } + } + } + + // Try to mount the disk + klog.V(4).Infof("Attempting to mount disk: %s %s %s", fstype, source, target) + mountErr := mounter.Interface.Mount(source, target, fstype, options) + if mountErr != nil { + // Mount failed. This indicates either that the disk is unformatted or + // it contains an unexpected filesystem. + existingFormat, err := mounter.GetDiskFormat(source) + if err != nil { + return err + } + if existingFormat == "" { + if readOnly { + // Don't attempt to format if mounting as readonly, return an error to reflect this. + return errors.New("failed to mount unformatted volume as read only") + } + + // Disk is unformatted so format it. + args := []string{source} + // Use 'ext4' as the default + if len(fstype) == 0 { + fstype = "ext4" + } + + if fstype == "ext4" || fstype == "ext3" { + args = []string{ + "-F", // Force flag + "-m0", // Zero blocks reserved for super-user + source, + } + } + klog.Infof("Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", source, fstype, args) + _, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() + if err == nil { + // the disk has been formatted successfully try to mount it again. + klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target) + return mounter.Interface.Mount(source, target, fstype, options) + } + klog.Errorf("format of disk %q failed: type:(%q) target:(%q) options:(%q)error:(%v)", source, fstype, target, options, err) + return err + } + // Disk is already formatted and failed to mount + if len(fstype) == 0 || fstype == existingFormat { + // This is mount error + return mountErr + } + // Block device is formatted with unexpected filesystem, let the user know + return fmt.Errorf("failed to mount the volume as %q, it already contains %s. Mount error: %v", fstype, existingFormat, mountErr) + } + return mountErr +} + +// GetDiskFormat uses 'blkid' to see if the given disk is unformatted +func (mounter *SafeFormatAndMount) GetDiskFormat(disk string) (string, error) { + args := []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", disk} + klog.V(4).Infof("Attempting to determine if disk %q is formatted using blkid with args: (%v)", disk, args) + dataOut, err := mounter.Exec.Command("blkid", args...).CombinedOutput() + output := string(dataOut) + klog.V(4).Infof("Output: %q, err: %v", output, err) + + if err != nil { + if exit, ok := err.(utilexec.ExitError); ok { + if exit.ExitStatus() == 2 { + // Disk device is unformatted. + // For `blkid`, if the specified token (TYPE/PTTYPE, etc) was + // not found, or no (specified) devices could be identified, an + // exit code of 2 is returned. + return "", nil + } + } + klog.Errorf("Could not determine if disk %q is formatted (%v)", disk, err) + return "", err + } + + var fstype, pttype string + + lines := strings.Split(output, "\n") + for _, l := range lines { + if len(l) <= 0 { + // Ignore empty line. + continue + } + cs := strings.Split(l, "=") + if len(cs) != 2 { + return "", fmt.Errorf("blkid returns invalid output: %s", output) + } + // TYPE is filesystem type, and PTTYPE is partition table type, according + // to https://www.kernel.org/pub/linux/utils/util-linux/v2.21/libblkid-docs/. + if cs[0] == "TYPE" { + fstype = cs[1] + } else if cs[0] == "PTTYPE" { + pttype = cs[1] + } + } + + if len(pttype) > 0 { + klog.V(4).Infof("Disk %s detected partition table type: %s", disk, pttype) + // Returns a special non-empty string as filesystem type, then kubelet + // will not format it. + return "unknown data, probably partitions", nil + } + + return fstype, nil +} + +// ListProcMounts is shared with NsEnterMounter +func ListProcMounts(mountFilePath string) ([]MountPoint, error) { + content, err := utilio.ConsistentRead(mountFilePath, maxListTries) + if err != nil { + return nil, err + } + return parseProcMounts(content) +} + +func parseProcMounts(content []byte) ([]MountPoint, error) { + out := []MountPoint{} + lines := strings.Split(string(content), "\n") + for _, line := range lines { + if line == "" { + // the last split() item is empty string following the last \n + continue + } + fields := strings.Fields(line) + if len(fields) != expectedNumFieldsPerLine { + return nil, fmt.Errorf("wrong number of fields (expected %d, got %d): %s", expectedNumFieldsPerLine, len(fields), line) + } + + mp := MountPoint{ + Device: fields[0], + Path: fields[1], + Type: fields[2], + Opts: strings.Split(fields[3], ","), + } + + freq, err := strconv.Atoi(fields[4]) + if err != nil { + return nil, err + } + mp.Freq = freq + + pass, err := strconv.Atoi(fields[5]) + if err != nil { + return nil, err + } + mp.Pass = pass + + out = append(out, mp) + } + return out, nil +} + +// SearchMountPoints finds all mount references to the source, returns a list of +// mountpoints. +// The source can be a mount point or a normal directory (bind mount). We +// didn't support device because there is no use case by now. +// Some filesystems may share a source name, e.g. tmpfs. And for bind mounting, +// it's possible to mount a non-root path of a filesystem, so we need to use +// root path and major:minor to represent mount source uniquely. +// This implementation is shared between Linux and NsEnterMounter +func SearchMountPoints(hostSource, mountInfoPath string) ([]string, error) { + mis, err := ParseMountInfo(mountInfoPath) + if err != nil { + return nil, err + } + + mountID := 0 + rootPath := "" + majorMinor := "" + + // Finding the underlying root path and major:minor if possible. + // We need search in backward order because it's possible for later mounts + // to overlap earlier mounts. + for i := len(mis) - 1; i >= 0; i-- { + if hostSource == mis[i].MountPoint || PathWithinBase(hostSource, mis[i].MountPoint) { + // If it's a mount point or path under a mount point. + mountID = mis[i].ID + rootPath = filepath.Join(mis[i].Root, strings.TrimPrefix(hostSource, mis[i].MountPoint)) + majorMinor = mis[i].MajorMinor + break + } + } + + if rootPath == "" || majorMinor == "" { + return nil, fmt.Errorf("failed to get root path and major:minor for %s", hostSource) + } + + var refs []string + for i := range mis { + if mis[i].ID == mountID { + // Ignore mount entry for mount source itself. + continue + } + if mis[i].Root == rootPath && mis[i].MajorMinor == majorMinor { + refs = append(refs, mis[i].MountPoint) + } + } + + return refs, nil +} diff --git a/mount_linux_test.go b/mount_linux_test.go new file mode 100644 index 00000000000..47b6e31a04c --- /dev/null +++ b/mount_linux_test.go @@ -0,0 +1,439 @@ +// +build linux + +/* +Copyright 2014 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "io/ioutil" + "os" + "reflect" + "testing" +) + +func TestReadProcMountsFrom(t *testing.T) { + successCase := + `/dev/0 /path/to/0 type0 flags 0 0 +/dev/1 /path/to/1 type1 flags 1 1 +/dev/2 /path/to/2 type2 flags,1,2=3 2 2 +` + // NOTE: readProcMountsFrom has been updated to using fnv.New32a() + mounts, err := parseProcMounts([]byte(successCase)) + if err != nil { + t.Errorf("expected success, got %v", err) + } + if len(mounts) != 3 { + t.Fatalf("expected 3 mounts, got %d", len(mounts)) + } + mp := MountPoint{"/dev/0", "/path/to/0", "type0", []string{"flags"}, 0, 0} + if !mountPointsEqual(&mounts[0], &mp) { + t.Errorf("got unexpected MountPoint[0]: %#v", mounts[0]) + } + mp = MountPoint{"/dev/1", "/path/to/1", "type1", []string{"flags"}, 1, 1} + if !mountPointsEqual(&mounts[1], &mp) { + t.Errorf("got unexpected MountPoint[1]: %#v", mounts[1]) + } + mp = MountPoint{"/dev/2", "/path/to/2", "type2", []string{"flags", "1", "2=3"}, 2, 2} + if !mountPointsEqual(&mounts[2], &mp) { + t.Errorf("got unexpected MountPoint[2]: %#v", mounts[2]) + } + + errorCases := []string{ + "/dev/0 /path/to/mount\n", + "/dev/1 /path/to/mount type flags a 0\n", + "/dev/2 /path/to/mount type flags 0 b\n", + } + for _, ec := range errorCases { + _, err := parseProcMounts([]byte(ec)) + if err == nil { + t.Errorf("expected error") + } + } +} + +func mountPointsEqual(a, b *MountPoint) bool { + if a.Device != b.Device || a.Path != b.Path || a.Type != b.Type || !reflect.DeepEqual(a.Opts, b.Opts) || a.Pass != b.Pass || a.Freq != b.Freq { + return false + } + return true +} + +func TestGetMountRefs(t *testing.T) { + fm := NewFakeMounter( + []MountPoint{ + {Device: "/dev/sdb", Path: "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd"}, + {Device: "/dev/sdb", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd-in-pod"}, + {Device: "/dev/sdc", Path: "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd2"}, + {Device: "/dev/sdc", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod1"}, + {Device: "/dev/sdc", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod2"}, + }) + + tests := []struct { + mountPath string + expectedRefs []string + }{ + { + "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd-in-pod", + []string{ + "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd", + }, + }, + { + "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod1", + []string{ + "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod2", + "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd2", + }, + }, + { + "/var/fake/directory/that/doesnt/exist", + []string{}, + }, + } + + for i, test := range tests { + if refs, err := fm.GetMountRefs(test.mountPath); err != nil || !setEquivalent(test.expectedRefs, refs) { + t.Errorf("%d. getMountRefs(%q) = %v, %v; expected %v, nil", i, test.mountPath, refs, err, test.expectedRefs) + } + } +} + +func setEquivalent(set1, set2 []string) bool { + map1 := make(map[string]bool) + map2 := make(map[string]bool) + for _, s := range set1 { + map1[s] = true + } + for _, s := range set2 { + map2[s] = true + } + + for s := range map1 { + if !map2[s] { + return false + } + } + for s := range map2 { + if !map1[s] { + return false + } + } + return true +} + +func TestGetDeviceNameFromMount(t *testing.T) { + fm := NewFakeMounter( + []MountPoint{ + {Device: "/dev/disk/by-path/prefix-lun-1", + Path: "/mnt/111"}, + {Device: "/dev/disk/by-path/prefix-lun-1", + Path: "/mnt/222"}, + }) + + tests := []struct { + mountPath string + expectedDevice string + expectedRefs int + }{ + { + "/mnt/222", + "/dev/disk/by-path/prefix-lun-1", + 2, + }, + } + + for i, test := range tests { + if device, refs, err := GetDeviceNameFromMount(fm, test.mountPath); err != nil || test.expectedRefs != refs || test.expectedDevice != device { + t.Errorf("%d. GetDeviceNameFromMount(%s) = (%s, %d), %v; expected (%s,%d), nil", i, test.mountPath, device, refs, err, test.expectedDevice, test.expectedRefs) + } + } +} + +func TestGetMountRefsByDev(t *testing.T) { + fm := NewFakeMounter( + []MountPoint{ + {Device: "/dev/sdb", Path: "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd"}, + {Device: "/dev/sdb", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd-in-pod"}, + {Device: "/dev/sdc", Path: "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd2"}, + {Device: "/dev/sdc", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod1"}, + {Device: "/dev/sdc", Path: "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod2"}, + }) + + tests := []struct { + mountPath string + expectedRefs []string + }{ + { + "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd", + []string{ + "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd-in-pod", + }, + }, + { + "/var/lib/kubelet/plugins/kubernetes.io/gce-pd/mounts/gce-pd2", + []string{ + "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod1", + "/var/lib/kubelet/pods/some-pod/volumes/kubernetes.io~gce-pd/gce-pd2-in-pod2", + }, + }, + } + + for i, test := range tests { + + if refs, err := getMountRefsByDev(fm, test.mountPath); err != nil || !setEquivalent(test.expectedRefs, refs) { + t.Errorf("%d. getMountRefsByDev(%q) = %v, %v; expected %v, nil", i, test.mountPath, refs, err, test.expectedRefs) + } + } +} + +func TestPathWithinBase(t *testing.T) { + tests := []struct { + name string + fullPath string + basePath string + expected bool + }{ + { + name: "good subpath", + fullPath: "/a/b/c", + basePath: "/a", + expected: true, + }, + { + name: "good subpath 2", + fullPath: "/a/b/c", + basePath: "/a/b", + expected: true, + }, + { + name: "good subpath end slash", + fullPath: "/a/b/c/", + basePath: "/a/b", + expected: true, + }, + { + name: "good subpath backticks", + fullPath: "/a/b/../c", + basePath: "/a", + expected: true, + }, + { + name: "good subpath equal", + fullPath: "/a/b/c", + basePath: "/a/b/c", + expected: true, + }, + { + name: "good subpath equal 2", + fullPath: "/a/b/c/", + basePath: "/a/b/c", + expected: true, + }, + { + name: "good subpath root", + fullPath: "/a", + basePath: "/", + expected: true, + }, + { + name: "bad subpath parent", + fullPath: "/a/b/c", + basePath: "/a/b/c/d", + expected: false, + }, + { + name: "bad subpath outside", + fullPath: "/b/c", + basePath: "/a/b/c", + expected: false, + }, + { + name: "bad subpath prefix", + fullPath: "/a/b/cd", + basePath: "/a/b/c", + expected: false, + }, + { + name: "bad subpath backticks", + fullPath: "/a/../b", + basePath: "/a", + expected: false, + }, + { + name: "configmap subpath", + fullPath: "/var/lib/kubelet/pods/uuid/volumes/kubernetes.io~configmap/config/..timestamp/file.txt", + basePath: "/var/lib/kubelet/pods/uuid/volumes/kubernetes.io~configmap/config", + expected: true, + }, + } + for _, test := range tests { + if PathWithinBase(test.fullPath, test.basePath) != test.expected { + t.Errorf("test %q failed: expected %v", test.name, test.expected) + } + + } +} + +func TestSearchMountPoints(t *testing.T) { + base := ` +19 25 0:18 / /sys rw,nosuid,nodev,noexec,relatime shared:7 - sysfs sysfs rw +20 25 0:4 / /proc rw,nosuid,nodev,noexec,relatime shared:12 - proc proc rw +21 25 0:6 / /dev rw,nosuid,relatime shared:2 - devtmpfs udev rw,size=4058156k,nr_inodes=1014539,mode=755 +22 21 0:14 / /dev/pts rw,nosuid,noexec,relatime shared:3 - devpts devpts rw,gid=5,mode=620,ptmxmode=000 +23 25 0:19 / /run rw,nosuid,noexec,relatime shared:5 - tmpfs tmpfs rw,size=815692k,mode=755 +25 0 252:0 / / rw,relatime shared:1 - ext4 /dev/mapper/ubuntu--vg-root rw,errors=remount-ro,data=ordered +26 19 0:12 / /sys/kernel/security rw,nosuid,nodev,noexec,relatime shared:8 - securityfs securityfs rw +27 21 0:21 / /dev/shm rw,nosuid,nodev shared:4 - tmpfs tmpfs rw +28 23 0:22 / /run/lock rw,nosuid,nodev,noexec,relatime shared:6 - tmpfs tmpfs rw,size=5120k +29 19 0:23 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:9 - tmpfs tmpfs ro,mode=755 +30 29 0:24 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:10 - cgroup cgroup rw,xattr,release_agent=/lib/systemd/systemd-cgroups-agent,name=systemd +31 19 0:25 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:11 - pstore pstore rw +32 29 0:26 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:13 - cgroup cgroup rw,devices +33 29 0:27 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:14 - cgroup cgroup rw,freezer +34 29 0:28 / /sys/fs/cgroup/pids rw,nosuid,nodev,noexec,relatime shared:15 - cgroup cgroup rw,pids +35 29 0:29 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,blkio +36 29 0:30 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:17 - cgroup cgroup rw,memory +37 29 0:31 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,perf_event +38 29 0:32 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime shared:19 - cgroup cgroup rw,hugetlb +39 29 0:33 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:20 - cgroup cgroup rw,cpu,cpuacct +40 29 0:34 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:21 - cgroup cgroup rw,cpuset +41 29 0:35 / /sys/fs/cgroup/net_cls,net_prio rw,nosuid,nodev,noexec,relatime shared:22 - cgroup cgroup rw,net_cls,net_prio +58 25 7:1 / /mnt/disks/blkvol1 rw,relatime shared:38 - ext4 /dev/loop1 rw,data=ordere +` + + testcases := []struct { + name string + source string + mountInfos string + expectedRefs []string + expectedErr error + }{ + { + "dir", + "/mnt/disks/vol1", + base, + nil, + nil, + }, + { + "dir-used", + "/mnt/disks/vol1", + base + ` +56 25 252:0 /mnt/disks/vol1 /var/lib/kubelet/pods/1890aef5-5a60-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test rw,relatime shared:1 - ext4 /dev/mapper/ubuntu--vg-root rw,errors=remount-ro,data=ordered +57 25 0:45 / /mnt/disks/vol rw,relatime shared:36 - tmpfs tmpfs rw +`, + []string{"/var/lib/kubelet/pods/1890aef5-5a60-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test"}, + nil, + }, + { + "tmpfs-vol", + "/mnt/disks/vol1", + base + `120 25 0:76 / /mnt/disks/vol1 rw,relatime shared:41 - tmpfs vol1 rw,size=10000k +`, + nil, + nil, + }, + { + "tmpfs-vol-used-by-two-pods", + "/mnt/disks/vol1", + base + `120 25 0:76 / /mnt/disks/vol1 rw,relatime shared:41 - tmpfs vol1 rw,size=10000k +196 25 0:76 / /var/lib/kubelet/pods/ade3ac21-5a5b-11e8-8559-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-8f263585 rw,relatime shared:41 - tmpfs vol1 rw,size=10000k +228 25 0:76 / /var/lib/kubelet/pods/ac60532d-5a5b-11e8-8559-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-8f263585 rw,relatime shared:41 - tmpfs vol1 rw,size=10000k +`, + []string{ + "/var/lib/kubelet/pods/ade3ac21-5a5b-11e8-8559-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-8f263585", + "/var/lib/kubelet/pods/ac60532d-5a5b-11e8-8559-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-8f263585", + }, + nil, + }, + { + "tmpfs-subdir-used-indirectly-via-bindmount-dir-by-one-pod", + "/mnt/vol1/foo", + base + `177 25 0:46 / /mnt/data rw,relatime shared:37 - tmpfs data rw +190 25 0:46 /vol1 /mnt/vol1 rw,relatime shared:37 - tmpfs data rw +191 25 0:46 /vol2 /mnt/vol2 rw,relatime shared:37 - tmpfs data rw +62 25 0:46 /vol1/foo /var/lib/kubelet/pods/e25f2f01-5b06-11e8-8694-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test rw,relatime shared:37 - tmpfs data rw +`, + []string{"/var/lib/kubelet/pods/e25f2f01-5b06-11e8-8694-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test"}, + nil, + }, + { + "dir-bindmounted", + "/mnt/disks/vol2", + base + `342 25 252:0 /mnt/disks/vol2 /mnt/disks/vol2 rw,relatime shared:1 - ext4 /dev/mapper/ubuntu--vg-root rw,errors=remount-ro,data=ordered +`, + nil, + nil, + }, + { + "dir-bindmounted-used-by-one-pod", + "/mnt/disks/vol2", + base + `342 25 252:0 /mnt/disks/vol2 /mnt/disks/vol2 rw,relatime shared:1 - ext4 /dev/mapper/ubuntu--vg-root rw,errors=remount-ro,data=ordered +77 25 252:0 /mnt/disks/vol2 /var/lib/kubelet/pods/f30dc360-5a5d-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-1fb30a1c rw,relatime shared:1 - ext4 /dev/mapper/ubuntu--vg-root rw,errors=remount-ro,data=ordered +`, + []string{"/var/lib/kubelet/pods/f30dc360-5a5d-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-1fb30a1c"}, + nil, + }, + { + "blockfs", + "/mnt/disks/blkvol1", + base + `58 25 7:1 / /mnt/disks/blkvol1 rw,relatime shared:38 - ext4 /dev/loop1 rw,data=ordered +`, + nil, + nil, + }, + { + "blockfs-used-by-one-pod", + "/mnt/disks/blkvol1", + base + `58 25 7:1 / /mnt/disks/blkvol1 rw,relatime shared:38 - ext4 /dev/loop1 rw,data=ordered +62 25 7:1 / /var/lib/kubelet/pods/f19fe4e2-5a63-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test rw,relatime shared:38 - ext4 /dev/loop1 rw,data=ordered +`, + []string{"/var/lib/kubelet/pods/f19fe4e2-5a63-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test"}, + nil, + }, + { + "blockfs-used-by-two-pods", + "/mnt/disks/blkvol1", + base + `58 25 7:1 / /mnt/disks/blkvol1 rw,relatime shared:38 - ext4 /dev/loop1 rw,data=ordered +62 25 7:1 / /var/lib/kubelet/pods/f19fe4e2-5a63-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test rw,relatime shared:38 - ext4 /dev/loop1 rw,data=ordered +95 25 7:1 / /var/lib/kubelet/pods/4854a48b-5a64-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test rw,relatime shared:38 - ext4 /dev/loop1 rw,data=ordered +`, + []string{"/var/lib/kubelet/pods/f19fe4e2-5a63-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test", + "/var/lib/kubelet/pods/4854a48b-5a64-11e8-962f-000c29bb0377/volumes/kubernetes.io~local-volume/local-pv-test"}, + nil, + }, + } + tmpFile, err := ioutil.TempFile("", "test-get-filetype") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmpFile.Name()) + defer tmpFile.Close() + for _, v := range testcases { + tmpFile.Truncate(0) + tmpFile.Seek(0, 0) + tmpFile.WriteString(v.mountInfos) + tmpFile.Sync() + refs, err := SearchMountPoints(v.source, tmpFile.Name()) + if !reflect.DeepEqual(refs, v.expectedRefs) { + t.Errorf("test %q: expected Refs: %#v, got %#v", v.name, v.expectedRefs, refs) + } + if !reflect.DeepEqual(err, v.expectedErr) { + t.Errorf("test %q: expected err: %v, got %v", v.name, v.expectedErr, err) + } + } +} diff --git a/mount_test.go b/mount_test.go new file mode 100644 index 00000000000..6b9d5d5fc4b --- /dev/null +++ b/mount_test.go @@ -0,0 +1,60 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "reflect" + "testing" +) + +func TestMakeBindOpts(t *testing.T) { + tests := []struct { + mountOption []string + isBind bool + expectedBindOpts []string + expectedRemountOpts []string + }{ + { + []string{"vers=2", "ro", "_netdev"}, + false, + []string{}, + []string{}, + }, + { + + []string{"bind", "vers=2", "ro", "_netdev"}, + true, + []string{"bind", "_netdev"}, + []string{"bind", "remount", "vers=2", "ro", "_netdev"}, + }, + } + for _, test := range tests { + bind, bindOpts, bindRemountOpts := MakeBindOpts(test.mountOption) + if bind != test.isBind { + t.Errorf("Expected bind to be %v but got %v", test.isBind, bind) + } + if test.isBind { + if !reflect.DeepEqual(test.expectedBindOpts, bindOpts) { + t.Errorf("Expected bind mount options to be %+v got %+v", test.expectedBindOpts, bindOpts) + } + if !reflect.DeepEqual(test.expectedRemountOpts, bindRemountOpts) { + t.Errorf("Expected remount options to be %+v got %+v", test.expectedRemountOpts, bindRemountOpts) + } + } + + } +} diff --git a/mount_unsupported.go b/mount_unsupported.go new file mode 100644 index 00000000000..d13ec5c34d5 --- /dev/null +++ b/mount_unsupported.go @@ -0,0 +1,72 @@ +// +build !linux,!windows + +/* +Copyright 2014 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "errors" +) + +// Mounter implements mount.Interface for unsupported platforms +type Mounter struct { + mounterPath string +} + +var errUnsupported = errors.New("util/mount on this platform is not supported") + +// New returns a mount.Interface for the current system. +// It provides options to override the default mounter behavior. +// mounterPath allows using an alternative to `/bin/mount` for mounting. +func New(mounterPath string) Interface { + return &Mounter{ + mounterPath: mounterPath, + } +} + +// Mount always returns an error on unsupported platforms +func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error { + return errUnsupported +} + +// Unmount always returns an error on unsupported platforms +func (mounter *Mounter) Unmount(target string) error { + return errUnsupported +} + +// List always returns an error on unsupported platforms +func (mounter *Mounter) List() ([]MountPoint, error) { + return []MountPoint{}, errUnsupported +} + +// IsLikelyNotMountPoint always returns an error on unsupported platforms +func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) { + return true, errUnsupported +} + +// GetMountRefs always returns an error on unsupported platforms +func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { + return nil, errUnsupported +} + +func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error { + return mounter.Interface.Mount(source, target, fstype, options) +} + +func (mounter *SafeFormatAndMount) diskLooksUnformatted(disk string) (bool, error) { + return true, errUnsupported +} diff --git a/mount_windows.go b/mount_windows.go new file mode 100644 index 00000000000..9371ff43551 --- /dev/null +++ b/mount_windows.go @@ -0,0 +1,280 @@ +// +build windows + +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + + "k8s.io/klog" + utilexec "k8s.io/utils/exec" + "k8s.io/utils/keymutex" + utilpath "k8s.io/utils/path" +) + +// Mounter provides the default implementation of mount.Interface +// for the windows platform. This implementation assumes that the +// kubelet is running in the host's root mount namespace. +type Mounter struct { + mounterPath string +} + +// New returns a mount.Interface for the current system. +// It provides options to override the default mounter behavior. +// mounterPath allows using an alternative to `/bin/mount` for mounting. +func New(mounterPath string) Interface { + return &Mounter{ + mounterPath: mounterPath, + } +} + +// acquire lock for smb mount +var getSMBMountMutex = keymutex.NewHashed(0) + +// Mount : mounts source to target with given options. +// currently only supports cifs(smb), bind mount(for disk) +func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error { + target = NormalizeWindowsPath(target) + + if source == "tmpfs" { + klog.V(3).Infof("mounting source (%q), target (%q), with options (%q)", source, target, options) + return os.MkdirAll(target, 0755) + } + + parentDir := filepath.Dir(target) + if err := os.MkdirAll(parentDir, 0755); err != nil { + return err + } + + klog.V(4).Infof("mount options(%q) source:%q, target:%q, fstype:%q, begin to mount", + options, source, target, fstype) + bindSource := source + + // tell it's going to mount azure disk or azure file according to options + if bind, _, _ := MakeBindOpts(options); bind { + // mount azure disk + bindSource = NormalizeWindowsPath(source) + } else { + if len(options) < 2 { + klog.Warningf("mount options(%q) command number(%d) less than 2, source:%q, target:%q, skip mounting", + options, len(options), source, target) + return nil + } + + // currently only cifs mount is supported + if strings.ToLower(fstype) != "cifs" { + return fmt.Errorf("only cifs mount is supported now, fstype: %q, mounting source (%q), target (%q), with options (%q)", fstype, source, target, options) + } + + // lock smb mount for the same source + getSMBMountMutex.LockKey(source) + defer getSMBMountMutex.UnlockKey(source) + + if output, err := newSMBMapping(options[0], options[1], source); err != nil { + if isSMBMappingExist(source) { + klog.V(2).Infof("SMB Mapping(%s) already exists, now begin to remove and remount", source) + if output, err := removeSMBMapping(source); err != nil { + return fmt.Errorf("Remove-SmbGlobalMapping failed: %v, output: %q", err, output) + } + if output, err := newSMBMapping(options[0], options[1], source); err != nil { + return fmt.Errorf("New-SmbGlobalMapping remount failed: %v, output: %q", err, output) + } + } else { + return fmt.Errorf("New-SmbGlobalMapping failed: %v, output: %q", err, output) + } + } + } + + if output, err := exec.Command("cmd", "/c", "mklink", "/D", target, bindSource).CombinedOutput(); err != nil { + klog.Errorf("mklink failed: %v, source(%q) target(%q) output: %q", err, bindSource, target, string(output)) + return err + } + + return nil +} + +// do the SMB mount with username, password, remotepath +// return (output, error) +func newSMBMapping(username, password, remotepath string) (string, error) { + if username == "" || password == "" || remotepath == "" { + return "", fmt.Errorf("invalid parameter(username: %s, password: %s, remoteapth: %s)", username, password, remotepath) + } + + // use PowerShell Environment Variables to store user input string to prevent command line injection + // https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables?view=powershell-5.1 + cmdLine := `$PWord = ConvertTo-SecureString -String $Env:smbpassword -AsPlainText -Force` + + `;$Credential = New-Object -TypeName System.Management.Automation.PSCredential -ArgumentList $Env:smbuser, $PWord` + + `;New-SmbGlobalMapping -RemotePath $Env:smbremotepath -Credential $Credential` + cmd := exec.Command("powershell", "/c", cmdLine) + cmd.Env = append(os.Environ(), + fmt.Sprintf("smbuser=%s", username), + fmt.Sprintf("smbpassword=%s", password), + fmt.Sprintf("smbremotepath=%s", remotepath)) + + output, err := cmd.CombinedOutput() + return string(output), err +} + +// check whether remotepath is already mounted +func isSMBMappingExist(remotepath string) bool { + cmd := exec.Command("powershell", "/c", `Get-SmbGlobalMapping -RemotePath $Env:smbremotepath`) + cmd.Env = append(os.Environ(), fmt.Sprintf("smbremotepath=%s", remotepath)) + _, err := cmd.CombinedOutput() + return err == nil +} + +// remove SMB mapping +func removeSMBMapping(remotepath string) (string, error) { + cmd := exec.Command("powershell", "/c", `Remove-SmbGlobalMapping -RemotePath $Env:smbremotepath -Force`) + cmd.Env = append(os.Environ(), fmt.Sprintf("smbremotepath=%s", remotepath)) + output, err := cmd.CombinedOutput() + return string(output), err +} + +// Unmount unmounts the target. +func (mounter *Mounter) Unmount(target string) error { + klog.V(4).Infof("azureMount: Unmount target (%q)", target) + target = NormalizeWindowsPath(target) + if output, err := exec.Command("cmd", "/c", "rmdir", target).CombinedOutput(); err != nil { + klog.Errorf("rmdir failed: %v, output: %q", err, string(output)) + return err + } + return nil +} + +// List returns a list of all mounted filesystems. todo +func (mounter *Mounter) List() ([]MountPoint, error) { + return []MountPoint{}, nil +} + +// IsLikelyNotMountPoint determines if a directory is not a mountpoint. +func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) { + stat, err := os.Lstat(file) + if err != nil { + return true, err + } + // If current file is a symlink, then it is a mountpoint. + if stat.Mode()&os.ModeSymlink != 0 { + target, err := os.Readlink(file) + if err != nil { + return true, fmt.Errorf("readlink error: %v", err) + } + exists, err := utilpath.Exists(utilpath.CheckFollowSymlink, target) + if err != nil { + return true, err + } + return !exists, nil + } + + return true, nil +} + +// GetMountRefs : empty implementation here since there is no place to query all mount points on Windows +func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { + windowsPath := NormalizeWindowsPath(pathname) + pathExists, pathErr := PathExists(windowsPath) + if !pathExists { + return []string{}, nil + } else if IsCorruptedMnt(pathErr) { + klog.Warningf("GetMountRefs found corrupted mount at %s, treating as unmounted path", windowsPath) + return []string{}, nil + } else if pathErr != nil { + return nil, fmt.Errorf("error checking path %s: %v", windowsPath, pathErr) + } + return []string{pathname}, nil +} + +func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error { + // Try to mount the disk + klog.V(4).Infof("Attempting to formatAndMount disk: %s %s %s", fstype, source, target) + + if err := ValidateDiskNumber(source); err != nil { + klog.Errorf("diskMount: formatAndMount failed, err: %v", err) + return err + } + + if len(fstype) == 0 { + // Use 'NTFS' as the default + fstype = "NTFS" + } + + // format disk if it is unformatted(raw) + cmd := fmt.Sprintf("Get-Disk -Number %s | Where partitionstyle -eq 'raw' | Initialize-Disk -PartitionStyle MBR -PassThru"+ + " | New-Partition -AssignDriveLetter -UseMaximumSize | Format-Volume -FileSystem %s -Confirm:$false", source, fstype) + if output, err := mounter.Exec.Command("powershell", "/c", cmd).CombinedOutput(); err != nil { + return fmt.Errorf("diskMount: format disk failed, error: %v, output: %q", err, string(output)) + } + klog.V(4).Infof("diskMount: Disk successfully formatted, disk: %q, fstype: %q", source, fstype) + + driveLetter, err := getDriveLetterByDiskNumber(source, mounter.Exec) + if err != nil { + return err + } + driverPath := driveLetter + ":" + target = NormalizeWindowsPath(target) + klog.V(4).Infof("Attempting to formatAndMount disk: %s %s %s", fstype, driverPath, target) + if output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, driverPath).CombinedOutput(); err != nil { + klog.Errorf("mklink failed: %v, output: %q", err, string(output)) + return err + } + return nil +} + +// Get drive letter according to windows disk number +func getDriveLetterByDiskNumber(diskNum string, exec utilexec.Interface) (string, error) { + cmd := fmt.Sprintf("(Get-Partition -DiskNumber %s).DriveLetter", diskNum) + output, err := exec.Command("powershell", "/c", cmd).CombinedOutput() + if err != nil { + return "", fmt.Errorf("azureMount: Get Drive Letter failed: %v, output: %q", err, string(output)) + } + if len(string(output)) < 1 { + return "", fmt.Errorf("azureMount: Get Drive Letter failed, output is empty") + } + return string(output)[:1], nil +} + +// getAllParentLinks walks all symbolic links and return all the parent targets recursively +func getAllParentLinks(path string) ([]string, error) { + const maxIter = 255 + links := []string{} + for { + links = append(links, path) + if len(links) > maxIter { + return links, fmt.Errorf("unexpected length of parent links: %v", links) + } + + fi, err := os.Lstat(path) + if err != nil { + return links, fmt.Errorf("Lstat: %v", err) + } + if fi.Mode()&os.ModeSymlink == 0 { + break + } + + path, err = os.Readlink(path) + if err != nil { + return links, fmt.Errorf("Readlink error: %v", err) + } + } + + return links, nil +} diff --git a/mount_windows_test.go b/mount_windows_test.go new file mode 100644 index 00000000000..288cc633518 --- /dev/null +++ b/mount_windows_test.go @@ -0,0 +1,333 @@ +// +build windows + +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "fmt" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/utils/exec/testing" +) + +func makeLink(link, target string) error { + if output, err := exec.Command("cmd", "/c", "mklink", "/D", link, target).CombinedOutput(); err != nil { + return fmt.Errorf("mklink failed: %v, link(%q) target(%q) output: %q", err, link, target, string(output)) + } + return nil +} + +func removeLink(link string) error { + if output, err := exec.Command("cmd", "/c", "rmdir", link).CombinedOutput(); err != nil { + return fmt.Errorf("rmdir failed: %v, output: %q", err, string(output)) + } + return nil +} + +func setEquivalent(set1, set2 []string) bool { + map1 := make(map[string]bool) + map2 := make(map[string]bool) + for _, s := range set1 { + map1[s] = true + } + for _, s := range set2 { + map2[s] = true + } + + for s := range map1 { + if !map2[s] { + return false + } + } + for s := range map2 { + if !map1[s] { + return false + } + } + return true +} + +// this func must run in admin mode, otherwise it will fail +func TestGetMountRefs(t *testing.T) { + tests := []struct { + mountPath string + expectedRefs []string + }{ + { + mountPath: `c:\windows`, + expectedRefs: []string{`c:\windows`}, + }, + { + mountPath: `c:\doesnotexist`, + expectedRefs: []string{}, + }, + } + + mounter := Mounter{"fake/path"} + + for _, test := range tests { + if refs, err := mounter.GetMountRefs(test.mountPath); err != nil || !setEquivalent(test.expectedRefs, refs) { + t.Errorf("getMountRefs(%q) = %v, error: %v; expected %v", test.mountPath, refs, err, test.expectedRefs) + } + } +} + +func TestPathWithinBase(t *testing.T) { + tests := []struct { + fullPath string + basePath string + expectedResult bool + }{ + { + fullPath: `c:\tmp\a\b\c`, + basePath: `c:\tmp`, + expectedResult: true, + }, + { + fullPath: `c:\tmp1`, + basePath: `c:\tmp2`, + expectedResult: false, + }, + { + fullPath: `c:\tmp`, + basePath: `c:\tmp`, + expectedResult: true, + }, + { + fullPath: `c:\tmp`, + basePath: `c:\tmp\a\b\c`, + expectedResult: false, + }, + { + fullPath: `c:\kubelet\pods\uuid\volumes\kubernetes.io~configmap\config\..timestamp\file.txt`, + basePath: `c:\kubelet\pods\uuid\volumes\kubernetes.io~configmap\config`, + expectedResult: true, + }, + } + + for _, test := range tests { + result := PathWithinBase(test.fullPath, test.basePath) + assert.Equal(t, result, test.expectedResult, "Expect result not equal with PathWithinBase(%s, %s) return: %q, expected: %q", + test.fullPath, test.basePath, result, test.expectedResult) + } +} + +func TestIsLikelyNotMountPoint(t *testing.T) { + mounter := Mounter{"fake/path"} + + tests := []struct { + fileName string + targetLinkName string + setUp func(base, fileName, targetLinkName string) error + expectedResult bool + expectError bool + }{ + { + "Dir", + "", + func(base, fileName, targetLinkName string) error { + return os.Mkdir(filepath.Join(base, fileName), 0750) + }, + true, + false, + }, + { + "InvalidDir", + "", + func(base, fileName, targetLinkName string) error { + return nil + }, + true, + true, + }, + { + "ValidSymLink", + "targetSymLink", + func(base, fileName, targetLinkName string) error { + targeLinkPath := filepath.Join(base, targetLinkName) + if err := os.Mkdir(targeLinkPath, 0750); err != nil { + return err + } + + filePath := filepath.Join(base, fileName) + if err := makeLink(filePath, targeLinkPath); err != nil { + return err + } + return nil + }, + false, + false, + }, + { + "InvalidSymLink", + "targetSymLink2", + func(base, fileName, targetLinkName string) error { + targeLinkPath := filepath.Join(base, targetLinkName) + if err := os.Mkdir(targeLinkPath, 0750); err != nil { + return err + } + + filePath := filepath.Join(base, fileName) + if err := makeLink(filePath, targeLinkPath); err != nil { + return err + } + return removeLink(targeLinkPath) + }, + true, + false, + }, + } + + for _, test := range tests { + base, err := ioutil.TempDir("", test.fileName) + if err != nil { + t.Fatalf(err.Error()) + } + + defer os.RemoveAll(base) + + if err := test.setUp(base, test.fileName, test.targetLinkName); err != nil { + t.Fatalf("unexpected error in setUp(%s, %s): %v", test.fileName, test.targetLinkName, err) + } + + filePath := filepath.Join(base, test.fileName) + result, err := mounter.IsLikelyNotMountPoint(filePath) + assert.Equal(t, result, test.expectedResult, "Expect result not equal with IsLikelyNotMountPoint(%s) return: %q, expected: %q", + filePath, result, test.expectedResult) + + if test.expectError { + assert.NotNil(t, err, "Expect error during IsLikelyNotMountPoint(%s)", filePath) + } else { + assert.Nil(t, err, "Expect error is nil during IsLikelyNotMountPoint(%s)", filePath) + } + } +} + +func TestFormatAndMount(t *testing.T) { + tests := []struct { + device string + target string + fstype string + execScripts []ExecArgs + mountOptions []string + expectError bool + }{ + { + device: "0", + target: "disk", + fstype: "NTFS", + execScripts: []ExecArgs{ + {"powershell", []string{"/c", "Get-Disk", "-Number"}, "0", nil}, + {"powershell", []string{"/c", "Get-Partition", "-DiskNumber"}, "0", nil}, + {"cmd", []string{"/c", "mklink", "/D"}, "", nil}, + }, + mountOptions: []string{}, + expectError: false, + }, + { + device: "0", + target: "disk", + fstype: "", + execScripts: []ExecArgs{ + {"powershell", []string{"/c", "Get-Disk", "-Number"}, "0", nil}, + {"powershell", []string{"/c", "Get-Partition", "-DiskNumber"}, "0", nil}, + {"cmd", []string{"/c", "mklink", "/D"}, "", nil}, + }, + mountOptions: []string{}, + expectError: false, + }, + { + device: "invalidDevice", + target: "disk", + fstype: "NTFS", + mountOptions: []string{}, + expectError: true, + }, + } + + for _, test := range tests { + fakeMounter := ErrorMounter{NewFakeMounter(nil), 0, nil} + fakeExec := &testingexec.FakeExec{} + for _, script := range test.execScripts { + fakeCmd := &testingexec.FakeCmd{} + cmdAction := makeFakeCmd(fakeCmd, script.command, script.args...) + outputAction := makeFakeOutput(script.output, script.err) + fakeCmd.CombinedOutputScript = append(fakeCmd.CombinedOutputScript, outputAction) + fakeExec.CommandScript = append(fakeExec.CommandScript, cmdAction) + } + mounter := SafeFormatAndMount{ + Interface: &fakeMounter, + Exec: fakeExec, + } + base, err := ioutil.TempDir("", test.device) + if err != nil { + t.Fatalf(err.Error()) + } + defer os.RemoveAll(base) + + target := filepath.Join(base, test.target) + err = mounter.FormatAndMount(test.device, target, test.fstype, test.mountOptions) + if test.expectError { + assert.NotNil(t, err, "Expect error during FormatAndMount(%s, %s, %s, %v)", test.device, test.target, test.fstype, test.mountOptions) + } else { + assert.Nil(t, err, "Expect error is nil during FormatAndMount(%s, %s, %s, %v)", test.device, test.target, test.fstype, test.mountOptions) + } + } +} + +func TestNewSMBMapping(t *testing.T) { + tests := []struct { + username string + password string + remotepath string + expectError bool + }{ + { + "", + "password", + `\\remotepath`, + true, + }, + { + "username", + "", + `\\remotepath`, + true, + }, + { + "username", + "password", + "", + true, + }, + } + + for _, test := range tests { + _, err := newSMBMapping(test.username, test.password, test.remotepath) + if test.expectError { + assert.NotNil(t, err, "Expect error during newSMBMapping(%s, %s, %s, %v)", test.username, test.password, test.remotepath) + } else { + assert.Nil(t, err, "Expect error is nil during newSMBMapping(%s, %s, %s, %v)", test.username, test.password, test.remotepath) + } + } +} diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go new file mode 100644 index 00000000000..d6cc06bce58 --- /dev/null +++ b/safe_format_and_mount_test.go @@ -0,0 +1,248 @@ +/* +Copyright 2014 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package mount + +import ( + "fmt" + "io/ioutil" + "os" + "runtime" + "strings" + "testing" + + "k8s.io/utils/exec" + "k8s.io/utils/exec/testing" +) + +type ErrorMounter struct { + *FakeMounter + errIndex int + err []error +} + +func (mounter *ErrorMounter) Mount(source string, target string, fstype string, options []string) error { + i := mounter.errIndex + mounter.errIndex++ + if mounter.err != nil && mounter.err[i] != nil { + return mounter.err[i] + } + return mounter.FakeMounter.Mount(source, target, fstype, options) +} + +type ExecArgs struct { + command string + args []string + output string + err error +} + +func TestSafeFormatAndMount(t *testing.T) { + if runtime.GOOS == "darwin" || runtime.GOOS == "windows" { + t.Skipf("not supported on GOOS=%s", runtime.GOOS) + } + mntDir, err := ioutil.TempDir(os.TempDir(), "mount") + if err != nil { + t.Fatalf("failed to create tmp dir: %v", err) + } + defer os.RemoveAll(mntDir) + tests := []struct { + description string + fstype string + mountOptions []string + execScripts []ExecArgs + mountErrs []error + expectedError error + }{ + { + description: "Test a read only mount", + fstype: "ext4", + mountOptions: []string{"ro"}, + }, + { + description: "Test a normal mount", + fstype: "ext4", + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + }, + }, + { + description: "Test 'fsck' fails with exit status 4", + fstype: "ext4", + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, + }, + expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them"), + }, + { + description: "Test 'fsck' fails with exit status 1 (errors found and corrected)", + fstype: "ext4", + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, + }, + }, + { + description: "Test 'fsck' fails with exit status other than 1 and 4 (likely unformatted device)", + fstype: "ext4", + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 8}}, + }, + }, + { + description: "Test that 'blkid' is called and fails", + fstype: "ext4", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + }, + expectedError: fmt.Errorf("unknown filesystem type '(null)'"), + }, + { + description: "Test that 'blkid' is called and confirms unformatted disk, format fails", + fstype: "ext4", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, + }, + expectedError: fmt.Errorf("formatting failed"), + }, + { + description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount fails", + fstype: "ext4", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), fmt.Errorf("Still cannot mount")}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, + }, + expectedError: fmt.Errorf("Still cannot mount"), + }, + { + description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount passes", + fstype: "ext4", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, + }, + expectedError: nil, + }, + { + description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount passes with ext3", + fstype: "ext3", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + {"mkfs.ext3", []string{"-F", "-m0", "/dev/foo"}, "", nil}, + }, + expectedError: nil, + }, + { + description: "test that none ext4 fs does not get called with ext4 options.", + fstype: "xfs", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, + }, + expectedError: nil, + }, + { + description: "Test that 'blkid' is called and reports ext4 partition", + fstype: "ext3", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, + }, + expectedError: fmt.Errorf("failed to mount the volume as \"ext3\", it already contains unknown data, probably partitions. Mount error: unknown filesystem type '(null)'"), + }, + { + description: "Test that 'blkid' is called but has some usage or other errors (an exit code of 4 is returned)", + fstype: "xfs", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, + {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, + }, + expectedError: fmt.Errorf("exit 4"), + }, + } + + for _, test := range tests { + fakeMounter := ErrorMounter{NewFakeMounter(nil), 0, test.mountErrs} + fakeExec := &testingexec.FakeExec{ExactOrder: true} + for _, script := range test.execScripts { + fakeCmd := &testingexec.FakeCmd{} + cmdAction := makeFakeCmd(fakeCmd, script.command, script.args...) + outputAction := makeFakeOutput(script.output, script.err) + fakeCmd.CombinedOutputScript = append(fakeCmd.CombinedOutputScript, outputAction) + fakeExec.CommandScript = append(fakeExec.CommandScript, cmdAction) + } + mounter := SafeFormatAndMount{ + Interface: &fakeMounter, + Exec: fakeExec, + } + + device := "/dev/foo" + dest := mntDir + err := mounter.FormatAndMount(device, dest, test.fstype, test.mountOptions) + if test.expectedError == nil { + if err != nil { + t.Errorf("test \"%s\" unexpected non-error: %v", test.description, err) + } + + // Check that something was mounted on the directory + isNotMountPoint, err := fakeMounter.IsLikelyNotMountPoint(dest) + if err != nil || isNotMountPoint { + t.Errorf("test \"%s\" the directory was not mounted", test.description) + } + + //check that the correct device was mounted + mountedDevice, _, err := GetDeviceNameFromMount(fakeMounter.FakeMounter, dest) + if err != nil || mountedDevice != device { + t.Errorf("test \"%s\" the correct device was not mounted", test.description) + } + } else { + if err == nil || !strings.HasPrefix(err.Error(), test.expectedError.Error()) { + t.Errorf("test \"%s\" unexpected error: \n [%v]. \nExpecting [%v]", test.description, err, test.expectedError) + } + } + } +} + +func makeFakeCmd(fakeCmd *testingexec.FakeCmd, cmd string, args ...string) testingexec.FakeCommandAction { + c := cmd + a := args + return func(cmd string, args ...string) exec.Cmd { + command := testingexec.InitFakeCmd(fakeCmd, c, a...) + return command + } +} + +func makeFakeOutput(output string, err error) testingexec.FakeCombinedOutputAction { + o := output + return func() ([]byte, error) { + return []byte(o), err + } +} From ac9fc1376194adb54b13069cb5bbebf1db90d598 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Thu, 14 Nov 2019 11:30:20 -0700 Subject: [PATCH 02/27] Update doc.go to show k8s.io/utils --- doc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc.go b/doc.go index 15179e53f2d..c81b426ce8c 100644 --- a/doc.go +++ b/doc.go @@ -15,4 +15,4 @@ limitations under the License. */ // Package mount defines an interface to mounting filesystems. -package mount // import "k8s.io/kubernetes/pkg/util/mount" +package mount // import "k8s.io/utils/mount" From b221620e597d0b99a92584f82efabeaa82856738 Mon Sep 17 00:00:00 2001 From: Travis Rhoden Date: Thu, 14 Nov 2019 11:30:40 -0700 Subject: [PATCH 03/27] Fix golint errors Add mount package to hack/.golint_failures to ignore stuttering errors. --- fake_mounter.go | 3 +++ mount_helper_windows.go | 3 +++ 2 files changed, 6 insertions(+) diff --git a/fake_mounter.go b/fake_mounter.go index 315bba6941f..d3a82f415c2 100644 --- a/fake_mounter.go +++ b/fake_mounter.go @@ -36,6 +36,7 @@ type FakeMounter struct { UnmountFunc UnmountFunc } +// UnmountFunc is a function callback to be executed during the Unmount() call. type UnmountFunc func(path string) error var _ Interface = &FakeMounter{} @@ -55,6 +56,8 @@ type FakeAction struct { FSType string // applies only to "mount" actions } +// NewFakeMounter returns a FakeMounter struct that implements Interface and is +// suitable for testing purposes. func NewFakeMounter(mps []MountPoint) *FakeMounter { return &FakeMounter{ MountPoints: mps, diff --git a/mount_helper_windows.go b/mount_helper_windows.go index 5e6b1dd9adc..516f970d282 100644 --- a/mount_helper_windows.go +++ b/mount_helper_windows.go @@ -70,6 +70,9 @@ func IsCorruptedMnt(err error) bool { return false } +// NormalizeWindowsPath makes sure the given path is a valid path on Windows +// systems by making sure all instances of `/` are replaced with `\\`, and the +// path beings with `c:` func NormalizeWindowsPath(path string) string { normalizedPath := strings.Replace(path, "/", "\\", -1) if strings.HasPrefix(normalizedPath, "\\") { From 82a590b2d57ad5829aefb88fb6e5a5ba72167c2f Mon Sep 17 00:00:00 2001 From: SataQiu <1527062125@qq.com> Date: Wed, 4 Dec 2019 22:43:29 +0800 Subject: [PATCH 04/27] feature: implement Output method for FakeCmd --- safe_format_and_mount_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go index d6cc06bce58..34bf92fbc25 100644 --- a/safe_format_and_mount_test.go +++ b/safe_format_and_mount_test.go @@ -240,9 +240,9 @@ func makeFakeCmd(fakeCmd *testingexec.FakeCmd, cmd string, args ...string) testi } } -func makeFakeOutput(output string, err error) testingexec.FakeCombinedOutputAction { +func makeFakeOutput(output string, err error) testingexec.FakeAction { o := output - return func() ([]byte, error) { - return []byte(o), err + return func() ([]byte, []byte, error) { + return []byte(o), nil, err } } From 0e8dbc2c1e8a2c708c0ba3279c9ea206be77bfe3 Mon Sep 17 00:00:00 2001 From: Maxime VISONNEAU Date: Tue, 17 Dec 2019 09:57:00 +0000 Subject: [PATCH 05/27] Validate the existence of filesystem before attempting to mount it (linux) --- mount_linux.go | 81 +++++++++++++++++------------------ safe_format_and_mount_test.go | 50 ++++++++++++++------- 2 files changed, 74 insertions(+), 57 deletions(-) diff --git a/mount_linux.go b/mount_linux.go index 2a910024297..2e87f7169fd 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -19,7 +19,6 @@ limitations under the License. package mount import ( - "errors" "fmt" "os" "os/exec" @@ -289,55 +288,53 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, } } - // Try to mount the disk - klog.V(4).Infof("Attempting to mount disk: %s %s %s", fstype, source, target) - mountErr := mounter.Interface.Mount(source, target, fstype, options) - if mountErr != nil { - // Mount failed. This indicates either that the disk is unformatted or - // it contains an unexpected filesystem. - existingFormat, err := mounter.GetDiskFormat(source) - if err != nil { - return err + // Check if the disk is already formatted + existingFormat, err := mounter.GetDiskFormat(source) + if err != nil { + return err + } + + // Use 'ext4' as the default + if len(fstype) == 0 { + fstype = "ext4" + } + + if existingFormat == "" { + // Do not attempt to format the disk if mounting as readonly, return an error to reflect this. + if readOnly { + return fmt.Errorf("cannot mount unformatted disk %s as we are manipulating it in read-only mode", source) } - if existingFormat == "" { - if readOnly { - // Don't attempt to format if mounting as readonly, return an error to reflect this. - return errors.New("failed to mount unformatted volume as read only") - } - // Disk is unformatted so format it. - args := []string{source} - // Use 'ext4' as the default - if len(fstype) == 0 { - fstype = "ext4" + // Disk is unformatted so format it. + args := []string{source} + if fstype == "ext4" || fstype == "ext3" { + args = []string{ + "-F", // Force flag + "-m0", // Zero blocks reserved for super-user + source, } + } - if fstype == "ext4" || fstype == "ext3" { - args = []string{ - "-F", // Force flag - "-m0", // Zero blocks reserved for super-user - source, - } - } - klog.Infof("Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", source, fstype, args) - _, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() - if err == nil { - // the disk has been formatted successfully try to mount it again. - klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target) - return mounter.Interface.Mount(source, target, fstype, options) - } + klog.Infof("Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", source, fstype, args) + _, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() + if err != nil { klog.Errorf("format of disk %q failed: type:(%q) target:(%q) options:(%q)error:(%v)", source, fstype, target, options, err) return err } - // Disk is already formatted and failed to mount - if len(fstype) == 0 || fstype == existingFormat { - // This is mount error - return mountErr - } - // Block device is formatted with unexpected filesystem, let the user know - return fmt.Errorf("failed to mount the volume as %q, it already contains %s. Mount error: %v", fstype, existingFormat, mountErr) + + klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target) + } else if fstype != existingFormat { + // Verify that the disk is formatted with filesystem type we are expecting + klog.Warningf("Configured to mount disk %s as %s but current format is %s, things might break", source, existingFormat, fstype) } - return mountErr + + // Mount the disk + klog.V(4).Infof("Attempting to mount disk %s in %s format at %s", source, fstype, target) + if err := mounter.Interface.Mount(source, target, fstype, options); err != nil { + return err + } + + return nil } // GetDiskFormat uses 'blkid' to see if the given disk is unformatted diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go index 34bf92fbc25..17de98b1d33 100644 --- a/safe_format_and_mount_test.go +++ b/safe_format_and_mount_test.go @@ -25,7 +25,7 @@ import ( "testing" "k8s.io/utils/exec" - "k8s.io/utils/exec/testing" + testingexec "k8s.io/utils/exec/testing" ) type ErrorMounter struct { @@ -68,15 +68,37 @@ func TestSafeFormatAndMount(t *testing.T) { expectedError error }{ { - description: "Test a read only mount", + description: "Test a read only mount of an already formatted device", fstype: "ext4", mountOptions: []string{"ro"}, + execScripts: []ExecArgs{ + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + }, }, { - description: "Test a normal mount", + description: "Test a normal mount of an already formatted device", fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + }, + }, + { + description: "Test a read only mount of unformatted device", + fstype: "ext4", + mountOptions: []string{"ro"}, + execScripts: []ExecArgs{ + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + }, + expectedError: fmt.Errorf("cannot mount unformatted disk /dev/foo as we are manipulating it in read-only mode"), + }, + { + description: "Test a normal mount of unformatted device", + fstype: "ext4", + execScripts: []ExecArgs{ + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, }, { @@ -84,6 +106,7 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them"), }, @@ -92,6 +115,7 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, }, { @@ -99,6 +123,7 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 8}}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, }, { @@ -107,7 +132,7 @@ func TestSafeFormatAndMount(t *testing.T) { mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, }, expectedError: fmt.Errorf("unknown filesystem type '(null)'"), }, @@ -125,18 +150,17 @@ func TestSafeFormatAndMount(t *testing.T) { { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount fails", fstype: "ext4", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), fmt.Errorf("Still cannot mount")}, + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, - expectedError: fmt.Errorf("Still cannot mount"), + expectedError: fmt.Errorf("unknown filesystem type '(null)'"), }, { - description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount passes", + description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes", fstype: "ext4", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, @@ -145,9 +169,8 @@ func TestSafeFormatAndMount(t *testing.T) { expectedError: nil, }, { - description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount passes with ext3", + description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes with ext3", fstype: "ext3", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, @@ -158,7 +181,6 @@ func TestSafeFormatAndMount(t *testing.T) { { description: "test that none ext4 fs does not get called with ext4 options.", fstype: "xfs", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, @@ -168,13 +190,11 @@ func TestSafeFormatAndMount(t *testing.T) { }, { description: "Test that 'blkid' is called and reports ext4 partition", - fstype: "ext3", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + fstype: "ext4", execScripts: []ExecArgs{ {"fsck", []string{"-a", "/dev/foo"}, "", nil}, - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, }, - expectedError: fmt.Errorf("failed to mount the volume as \"ext3\", it already contains unknown data, probably partitions. Mount error: unknown filesystem type '(null)'"), }, { description: "Test that 'blkid' is called but has some usage or other errors (an exit code of 4 is returned)", From 51e2ee9753f486883dc70e765c2f87dbe6aecaa2 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Tue, 17 Dec 2019 22:59:32 -0500 Subject: [PATCH 06/27] Return typed error when Mount Fails Mount can fail for a variety of reasons and caller might want to know why mount failed. Using untyped string based error does not provide enough granularity to make that verification. --- mount.go | 31 +++++++++++++++++++++++++++++++ mount_linux.go | 10 ++++++---- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/mount.go b/mount.go index 821cc875345..0e7d1087044 100644 --- a/mount.go +++ b/mount.go @@ -20,6 +20,7 @@ limitations under the License. package mount import ( + "fmt" "os" "path/filepath" "strings" @@ -76,6 +77,36 @@ type MountPoint struct { Pass int } +type MountErrorType string + +const ( + FilesystemMismatch MountErrorType = "FilesystemMismatch" + HasFilesystemErrors MountErrorType = "HasFilesystemErrors" + UnformattedReadOnly MountErrorType = "UnformattedReadOnly" + FormatFailed MountErrorType = "FormatFailed" +) + +type MountError struct { + Type MountErrorType + Message string +} + +func (mountError *MountError) String() string { + return mountError.Message +} + +func (mountError *MountError) Error() string { + return mountError.Message +} + +func NewMountError(mountErrorValue MountErrorType, format string, args ...interface{}) error { + mountError := &MountError{ + Type: mountErrorValue, + Message: fmt.Sprintf(format, args...), + } + return mountError +} + // SafeFormatAndMount probes a device to see if it is formatted. // Namely it checks to see if a file system is present. If so it // mounts it otherwise the device is formatted first then mounted. diff --git a/mount_linux.go b/mount_linux.go index 2e87f7169fd..77d01e78bd5 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -267,6 +267,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, } options = append(options, "defaults") + var mountErrorValue MountErrorType if !readOnly { // Run fsck on the disk to fix repairable issues, only do this for volumes requested as rw. @@ -281,7 +282,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, case isExitError && ee.ExitStatus() == fsckErrorsCorrected: klog.Infof("Device %s has errors which were corrected by fsck.", source) case isExitError && ee.ExitStatus() == fsckErrorsUncorrected: - return fmt.Errorf("'fsck' found errors on device %s but could not correct them: %s", source, string(out)) + return NewMountError(HasFilesystemErrors, "'fsck' found errors on device %s but could not correct them: %s", source, string(out)) case isExitError && ee.ExitStatus() > fsckErrorsUncorrected: klog.Infof("`fsck` error %s", string(out)) } @@ -302,7 +303,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, if existingFormat == "" { // Do not attempt to format the disk if mounting as readonly, return an error to reflect this. if readOnly { - return fmt.Errorf("cannot mount unformatted disk %s as we are manipulating it in read-only mode", source) + return NewMountError(UnformattedReadOnly, "cannot mount unformatted disk %s as we are manipulating it in read-only mode", source) } // Disk is unformatted so format it. @@ -319,19 +320,20 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, _, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() if err != nil { klog.Errorf("format of disk %q failed: type:(%q) target:(%q) options:(%q)error:(%v)", source, fstype, target, options, err) - return err + return NewMountError(FormatFailed, err.Error()) } klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target) } else if fstype != existingFormat { // Verify that the disk is formatted with filesystem type we are expecting + mountErrorValue = FilesystemMismatch klog.Warningf("Configured to mount disk %s as %s but current format is %s, things might break", source, existingFormat, fstype) } // Mount the disk klog.V(4).Infof("Attempting to mount disk %s in %s format at %s", source, fstype, target) if err := mounter.Interface.Mount(source, target, fstype, options); err != nil { - return err + return NewMountError(mountErrorValue, err.Error()) } return nil From 3651c2ef519ae2649f4803f7644ae402582eb0f0 Mon Sep 17 00:00:00 2001 From: Lou Date: Wed, 4 Dec 2019 16:51:52 +0800 Subject: [PATCH 07/27] use xfs_repair to check and repair xfs filesystem Signed-off-by: Lou --- mount_linux.go | 93 ++++++++++++++++++++++++++--------- safe_format_and_mount_test.go | 59 +++++++++++++++++----- 2 files changed, 115 insertions(+), 37 deletions(-) diff --git a/mount_linux.go b/mount_linux.go index 77d01e78bd5..88febbb0ad7 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -256,6 +256,54 @@ func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { return SearchMountPoints(realpath, procMountInfoPath) } +// checkAndRepairFileSystem checks and repairs filesystems using command fsck. +func (mounter *SafeFormatAndMount) checkAndRepairFilesystem(source string) error { + klog.V(4).Infof("Checking for issues with fsck on disk: %s", source) + args := []string{"-a", source} + out, err := mounter.Exec.Command("fsck", args...).CombinedOutput() + if err != nil { + ee, isExitError := err.(utilexec.ExitError) + switch { + case err == utilexec.ErrExecutableNotFound: + klog.Warningf("'fsck' not found on system; continuing mount without running 'fsck'.") + case isExitError && ee.ExitStatus() == fsckErrorsCorrected: + klog.Infof("Device %s has errors which were corrected by fsck.", source) + case isExitError && ee.ExitStatus() == fsckErrorsUncorrected: + return NewMountError(HasFilesystemErrors, "'fsck' found errors on device %s but could not correct them: %s", source, string(out)) + case isExitError && ee.ExitStatus() > fsckErrorsUncorrected: + klog.Infof("`fsck` error %s", string(out)) + } + } + return nil +} + +// checkAndRepairXfsFilesystem checks and repairs xfs filesystem using command xfs_repair. +func (mounter *SafeFormatAndMount) checkAndRepairXfsFilesystem(source string) error { + klog.V(4).Infof("Checking for issues with xfs_repair on disk: %s", source) + + args := []string{source} + checkArgs := []string{"-n", source} + + // check-only using "xfs_repair -n", if the exit status is not 0, perform a "xfs_repair" + _, err := mounter.Exec.Command("xfs_repair", checkArgs...).CombinedOutput() + if err != nil { + if err == utilexec.ErrExecutableNotFound { + klog.Warningf("'xfs_repair' not found on system; continuing mount without running 'xfs_repair'.") + return nil + } else { + klog.Warningf("Filesystem corruption was detected for %s, running xfs_repair to repair", source) + out, err := mounter.Exec.Command("xfs_repair", args...).CombinedOutput() + if err != nil { + return fmt.Errorf("'xfs_repair' found errors on device %s but could not correct them: %s\n", source, out) + } else { + klog.Infof("Device %s has errors which were corrected by xfs_repair.", source) + return nil + } + } + } + return nil +} + // formatAndMount uses unix utils to format and mount the given disk func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error { readOnly := false @@ -269,26 +317,6 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, options = append(options, "defaults") var mountErrorValue MountErrorType - if !readOnly { - // Run fsck on the disk to fix repairable issues, only do this for volumes requested as rw. - klog.V(4).Infof("Checking for issues with fsck on disk: %s", source) - args := []string{"-a", source} - out, err := mounter.Exec.Command("fsck", args...).CombinedOutput() - if err != nil { - ee, isExitError := err.(utilexec.ExitError) - switch { - case err == utilexec.ErrExecutableNotFound: - klog.Warningf("'fsck' not found on system; continuing mount without running 'fsck'.") - case isExitError && ee.ExitStatus() == fsckErrorsCorrected: - klog.Infof("Device %s has errors which were corrected by fsck.", source) - case isExitError && ee.ExitStatus() == fsckErrorsUncorrected: - return NewMountError(HasFilesystemErrors, "'fsck' found errors on device %s but could not correct them: %s", source, string(out)) - case isExitError && ee.ExitStatus() > fsckErrorsUncorrected: - klog.Infof("`fsck` error %s", string(out)) - } - } - } - // Check if the disk is already formatted existingFormat, err := mounter.GetDiskFormat(source) if err != nil { @@ -324,10 +352,27 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, } klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target) - } else if fstype != existingFormat { - // Verify that the disk is formatted with filesystem type we are expecting - mountErrorValue = FilesystemMismatch - klog.Warningf("Configured to mount disk %s as %s but current format is %s, things might break", source, existingFormat, fstype) + } else { + if fstype != existingFormat { + // Verify that the disk is formatted with filesystem type we are expecting + mountErrorValue = FilesystemMismatch + klog.Warningf("Configured to mount disk %s as %s but current format is %s, things might break", source, existingFormat, fstype) + } + + if !readOnly { + // Run check tools on the disk to fix repairable issues, only do this for formatted volumes requested as rw. + var err error + switch existingFormat { + case "xfs": + err = mounter.checkAndRepairXfsFilesystem(source) + default: + err = mounter.checkAndRepairFilesystem(source) + } + + if err != nil { + return err + } + } } // Mount the disk diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go index 17de98b1d33..5a8727134bd 100644 --- a/safe_format_and_mount_test.go +++ b/safe_format_and_mount_test.go @@ -79,8 +79,8 @@ func TestSafeFormatAndMount(t *testing.T) { description: "Test a normal mount of an already formatted device", fstype: "ext4", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, }, }, { @@ -96,7 +96,6 @@ func TestSafeFormatAndMount(t *testing.T) { description: "Test a normal mount of unformatted device", fstype: "ext4", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, @@ -105,8 +104,8 @@ func TestSafeFormatAndMount(t *testing.T) { description: "Test 'fsck' fails with exit status 4", fstype: "ext4", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, }, expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them"), }, @@ -114,16 +113,16 @@ func TestSafeFormatAndMount(t *testing.T) { description: "Test 'fsck' fails with exit status 1 (errors found and corrected)", fstype: "ext4", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, }, }, { description: "Test 'fsck' fails with exit status other than 1 and 4 (likely unformatted device)", fstype: "ext4", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 8}}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 8}}, }, }, { @@ -131,8 +130,8 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, }, expectedError: fmt.Errorf("unknown filesystem type '(null)'"), }, @@ -141,7 +140,6 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, }, @@ -152,7 +150,6 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "ext4", mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, @@ -162,7 +159,6 @@ func TestSafeFormatAndMount(t *testing.T) { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes", fstype: "ext4", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, @@ -172,7 +168,6 @@ func TestSafeFormatAndMount(t *testing.T) { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes with ext3", fstype: "ext3", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext3", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, @@ -182,7 +177,6 @@ func TestSafeFormatAndMount(t *testing.T) { description: "test that none ext4 fs does not get called with ext4 options.", fstype: "xfs", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, @@ -192,8 +186,8 @@ func TestSafeFormatAndMount(t *testing.T) { description: "Test that 'blkid' is called and reports ext4 partition", fstype: "ext4", execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, + {"fsck", []string{"-a", "/dev/foo"}, "", nil}, }, }, { @@ -201,12 +195,51 @@ func TestSafeFormatAndMount(t *testing.T) { fstype: "xfs", mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'"), nil}, execScripts: []ExecArgs{ - {"fsck", []string{"-a", "/dev/foo"}, "", nil}, {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, expectedError: fmt.Errorf("exit 4"), }, + { + description: "Test that 'xfs_repair' is called only once, no need to repair the filesystem", + fstype: "xfs", + execScripts: []ExecArgs{ + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, + {"xfs_repair", []string{"-n", "/dev/foo"}, "", nil}, + }, + expectedError: nil, + }, + { + description: "Test that 'xfs_repair' is called twice and repair the filesystem", + fstype: "xfs", + execScripts: []ExecArgs{ + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, + {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, + {"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil}, + }, + expectedError: nil, + }, + { + description: "Test that 'xfs_repair' is called twice and repair the filesystem, but mount failed", + fstype: "xfs", + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + execScripts: []ExecArgs{ + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, + {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, + {"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil}, + }, + expectedError: fmt.Errorf("unknown filesystem type '(null)'"), + }, + { + description: "Test that 'xfs_repair' is called twice but could not repair the filesystem", + fstype: "xfs", + execScripts: []ExecArgs{ + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, + {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, + {"xfs_repair", []string{"/dev/foo"}, "\nAn error occurred\n", &testingexec.FakeExitError{Status: 1}}, + }, + expectedError: fmt.Errorf("'xfs_repair' found errors on device %s but could not correct them: %v", "/dev/foo", "\nAn error occurred\n"), + }, } for _, test := range tests { From 241ff06ccc1edd2c4b80f1981c27bc28a32bd325 Mon Sep 17 00:00:00 2001 From: Lou Date: Thu, 9 Jan 2020 20:21:21 +0800 Subject: [PATCH 08/27] update after review Signed-off-by: Lou --- mount_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mount_linux.go b/mount_linux.go index 88febbb0ad7..88a46424bd7 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -294,7 +294,7 @@ func (mounter *SafeFormatAndMount) checkAndRepairXfsFilesystem(source string) er klog.Warningf("Filesystem corruption was detected for %s, running xfs_repair to repair", source) out, err := mounter.Exec.Command("xfs_repair", args...).CombinedOutput() if err != nil { - return fmt.Errorf("'xfs_repair' found errors on device %s but could not correct them: %s\n", source, out) + return NewMountError(HasFilesystemErrors, "'xfs_repair' found errors on device %s but could not correct them: %s\n", source, out) } else { klog.Infof("Device %s has errors which were corrected by xfs_repair.", source) return nil From 0f9a3f756b9adeb8326e0fe7ee7771b16003a019 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Tue, 21 Jan 2020 10:28:19 -0500 Subject: [PATCH 09/27] Split MajorMinor into two fields this reflects what is in the docker/docker/pkg/mount API. makes it easier to port additional code to out API. --- mount_helper_unix.go | 24 +++++++- mount_helper_unix_test.go | 113 ++++++++++++++++++++++++++++++++++++-- mount_linux.go | 10 ++-- 3 files changed, 134 insertions(+), 13 deletions(-) diff --git a/mount_helper_unix.go b/mount_helper_unix.go index 9c91e158264..1e14d8c96ca 100644 --- a/mount_helper_unix.go +++ b/mount_helper_unix.go @@ -61,8 +61,12 @@ type MountInfo struct { ID int // The ID of the parent mount (or of self for the root of this mount namespace's mount tree). ParentID int - // The value of `st_dev` for files on this filesystem. - MajorMinor string + // Major indicates one half of the device ID which identifies the device class + // (parsed from `st_dev` for files on this filesystem). + Major int + // Minor indicates one half of the device ID which identifies a specific + // instance of device (parsed from `st_dev` for files on this filesystem). + Minor int // The pathname of the directory in the filesystem which forms the root of this mount. Root string // Mount source, filesystem-specific information. e.g. device, tmpfs name. @@ -106,10 +110,24 @@ func ParseMountInfo(filename string) ([]MountInfo, error) { if err != nil { return nil, err } + mm := strings.Split(fields[2], ":") + if len(mm) != 2 { + return nil, fmt.Errorf("parsing '%s' failed: unexpected minor:major pair %s", line, mm) + } + major, err := strconv.Atoi(mm[0]) + if err != nil { + return nil, fmt.Errorf("parsing '%s' failed: unable to parse major device id, err:%v", mm[0], err) + } + minor, err := strconv.Atoi(mm[1]) + if err != nil { + return nil, fmt.Errorf("parsing '%s' failed: unable to parse minor device id, err:%v", mm[1], err) + } + info := MountInfo{ ID: id, ParentID: parentID, - MajorMinor: fields[2], + Major: major, + Minor: minor, Root: fields[3], MountPoint: fields[4], MountOptions: strings.Split(fields[5], ","), diff --git a/mount_helper_unix_test.go b/mount_helper_unix_test.go index f364b02863c..af263dbab30 100644 --- a/mount_helper_unix_test.go +++ b/mount_helper_unix_test.go @@ -100,7 +100,8 @@ func TestParseMountInfo(t *testing.T) { MountInfo{ ID: 189, ParentID: 80, - MajorMinor: "8:1", + Major: 8, + Minor: 1, Root: "/var/lib/kubelet", Source: "/dev/sda1", MountPoint: "/var/lib/kubelet", @@ -116,7 +117,8 @@ func TestParseMountInfo(t *testing.T) { MountInfo{ ID: 222, ParentID: 24, - MajorMinor: "253:0", + Major: 253, + Minor: 0, Root: "/tmp/src", Source: "/dev/mapper/vagrant--vg-root", MountPoint: "/mnt/dst", @@ -132,7 +134,8 @@ func TestParseMountInfo(t *testing.T) { MountInfo{ ID: 224, ParentID: 62, - MajorMinor: "253:0", + Major: 253, + Minor: 0, Root: "/var/lib/docker/devicemapper/test/shared", Source: "/dev/mapper/ssd-root", MountPoint: "/var/lib/docker/devicemapper/test/shared", @@ -148,7 +151,8 @@ func TestParseMountInfo(t *testing.T) { MountInfo{ ID: 28, ParentID: 18, - MajorMinor: "0:24", + Major: 0, + Minor: 24, Root: "/", Source: "tmpfs", MountPoint: "/sys/fs/cgroup", @@ -164,7 +168,8 @@ func TestParseMountInfo(t *testing.T) { MountInfo{ ID: 29, ParentID: 28, - MajorMinor: "0:25", + Major: 0, + Minor: 25, Root: "/", Source: "cgroup", MountPoint: "/sys/fs/cgroup/systemd", @@ -180,7 +185,8 @@ func TestParseMountInfo(t *testing.T) { MountInfo{ ID: 31, ParentID: 28, - MajorMinor: "0:27", + Major: 0, + Minor: 27, Root: "/", Source: "cgroup", MountPoint: "/sys/fs/cgroup/cpuset", @@ -213,3 +219,98 @@ func TestParseMountInfo(t *testing.T) { } } } + +func TestBadParseMountInfo(t *testing.T) { + tests := []struct { + info string + name string + id int + expectedInfo *MountInfo + error string + }{ + { + `224 62 253:0 /var/lib/docker/devicemapper/test/shared /var/lib/docker/devicemapper/test/shared rw,relatime master:1 shared:44 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered`, + "good major:minor field", + 224, + &MountInfo{ + ID: 224, + ParentID: 62, + Major: 253, + Minor: 0, + Root: "/var/lib/docker/devicemapper/test/shared", + Source: "/dev/mapper/ssd-root", + MountPoint: "/var/lib/docker/devicemapper/test/shared", + OptionalFields: []string{"master:1", "shared:44"}, + FsType: "ext4", + MountOptions: []string{"rw", "relatime"}, + SuperOptions: []string{"rw", "seclabel", "data=ordered"}, + }, + "", + }, + { + `224 62 /var/lib/docker/devicemapper/test/shared /var/lib/docker/devicemapper/test/shared rw,relatime master:1 shared:44 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered`, + "missing major:minor field", + 224, + nil, + `parsing '224 62 /var/lib/docker/devicemapper/test/shared /var/lib/docker/devicemapper/test/shared rw,relatime master:1 shared:44 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered' failed: unexpected minor:major pair [/var/lib/docker/devicemapper/test/shared]`, + }, + { + `224 62 :0 /var/lib/docker/devicemapper/test/shared /var/lib/docker/devicemapper/test/shared rw,relatime master:1 shared:44 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered`, + "empty major field", + 224, + nil, + `parsing '' failed: unable to parse major device id, err:strconv.Atoi: parsing "": invalid syntax`, + }, + { + `224 62 253: /var/lib/docker/devicemapper/test/shared /var/lib/docker/devicemapper/test/shared rw,relatime master:1 shared:44 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered`, + "empty minor field", + 224, + nil, + `parsing '' failed: unable to parse minor device id, err:strconv.Atoi: parsing "": invalid syntax`, + }, + { + `224 62 foo:0 /var/lib/docker/devicemapper/test/shared /var/lib/docker/devicemapper/test/shared rw,relatime master:1 shared:44 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered`, + "alphabet in major field", + 224, + nil, + `parsing 'foo' failed: unable to parse major device id, err:strconv.Atoi: parsing "foo": invalid syntax`, + }, + { + `224 62 253:bar /var/lib/docker/devicemapper/test/shared /var/lib/docker/devicemapper/test/shared rw,relatime master:1 shared:44 - ext4 /dev/mapper/ssd-root rw,seclabel,data=ordered`, + "alphabet in minor field", + 224, + nil, + `parsing 'bar' failed: unable to parse minor device id, err:strconv.Atoi: parsing "bar": invalid syntax`, + }, + } + + for _, test := range tests { + tempDir, filename, err := writeFile(test.info) + if err != nil { + t.Fatalf("cannot create temporary file: %v", err) + } + defer os.RemoveAll(tempDir) + + infos, err := ParseMountInfo(filename) + if err != nil { + if err.Error() != test.error { + t.Errorf("Test case %q:\n expected error: %+v\n got: %+v", test.name, test.error, err.Error()) + } + continue + } + + found := false + for _, info := range infos { + if info.ID == test.id { + found = true + if !reflect.DeepEqual(info, *test.expectedInfo) { + t.Errorf("Test case %q:\n expected: %+v\n got: %+v", test.name, test.expectedInfo, info) + } + break + } + } + if !found { + t.Errorf("Test case %q: mountPoint %d not found", test.name, test.id) + } + } +} diff --git a/mount_linux.go b/mount_linux.go index 88a46424bd7..b9405ba12e9 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -499,7 +499,8 @@ func SearchMountPoints(hostSource, mountInfoPath string) ([]string, error) { mountID := 0 rootPath := "" - majorMinor := "" + major := -1 + minor := -1 // Finding the underlying root path and major:minor if possible. // We need search in backward order because it's possible for later mounts @@ -509,12 +510,13 @@ func SearchMountPoints(hostSource, mountInfoPath string) ([]string, error) { // If it's a mount point or path under a mount point. mountID = mis[i].ID rootPath = filepath.Join(mis[i].Root, strings.TrimPrefix(hostSource, mis[i].MountPoint)) - majorMinor = mis[i].MajorMinor + major = mis[i].Major + minor = mis[i].Minor break } } - if rootPath == "" || majorMinor == "" { + if rootPath == "" || major == -1 || minor == -1 { return nil, fmt.Errorf("failed to get root path and major:minor for %s", hostSource) } @@ -524,7 +526,7 @@ func SearchMountPoints(hostSource, mountInfoPath string) ([]string, error) { // Ignore mount entry for mount source itself. continue } - if mis[i].Root == rootPath && mis[i].MajorMinor == majorMinor { + if mis[i].Root == rootPath && mis[i].Major == major && mis[i].Minor == minor { refs = append(refs, mis[i].MountPoint) } } From 99f567aa7af30ea69107ba195419553ac8f73429 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 23 Jan 2020 18:59:01 -0800 Subject: [PATCH 10/27] Add more detailed error output when disk formatting fails --- mount_linux.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mount_linux.go b/mount_linux.go index b9405ba12e9..c4485162987 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -345,10 +345,11 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, } klog.Infof("Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", source, fstype, args) - _, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() + output, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() if err != nil { - klog.Errorf("format of disk %q failed: type:(%q) target:(%q) options:(%q)error:(%v)", source, fstype, target, options, err) - return NewMountError(FormatFailed, err.Error()) + detailedErr := fmt.Sprintf("format of disk %q failed: type:(%q) target:(%q) options:(%q) errcode:(%v) output:(%v) ", source, fstype, target, options, err, string(output)) + klog.Error(detailedErr) + return NewMountError(FormatFailed, detailedErr) } klog.Infof("Disk successfully formatted (mkfs): %s - %s %s", fstype, source, target) From 5d34e5006ab85290ce8c820645c9ee91f3b2d2dd Mon Sep 17 00:00:00 2001 From: David Zhu Date: Thu, 23 Jan 2020 19:25:03 -0800 Subject: [PATCH 11/27] FormatAndMount unit test only checks for MountErrorValue now and closed gaps for some error values --- mount.go | 8 ++++--- mount_linux.go | 4 ++-- safe_format_and_mount_test.go | 44 +++++++++++++++++------------------ 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/mount.go b/mount.go index 0e7d1087044..46d77327fbf 100644 --- a/mount.go +++ b/mount.go @@ -84,6 +84,8 @@ const ( HasFilesystemErrors MountErrorType = "HasFilesystemErrors" UnformattedReadOnly MountErrorType = "UnformattedReadOnly" FormatFailed MountErrorType = "FormatFailed" + GetDiskFormatFailed MountErrorType = "GetDiskFormatFailed" + UnknownMountError MountErrorType = "UnknownMountError" ) type MountError struct { @@ -91,16 +93,16 @@ type MountError struct { Message string } -func (mountError *MountError) String() string { +func (mountError MountError) String() string { return mountError.Message } -func (mountError *MountError) Error() string { +func (mountError MountError) Error() string { return mountError.Message } func NewMountError(mountErrorValue MountErrorType, format string, args ...interface{}) error { - mountError := &MountError{ + mountError := MountError{ Type: mountErrorValue, Message: fmt.Sprintf(format, args...), } diff --git a/mount_linux.go b/mount_linux.go index c4485162987..4dc696d69b0 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -315,12 +315,12 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, } options = append(options, "defaults") - var mountErrorValue MountErrorType + mountErrorValue := UnknownMountError // Check if the disk is already formatted existingFormat, err := mounter.GetDiskFormat(source) if err != nil { - return err + return NewMountError(GetDiskFormatFailed, "failed to get disk format of disk %s: %v", source, err) } // Use 'ext4' as the default diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go index 5a8727134bd..a0869293656 100644 --- a/safe_format_and_mount_test.go +++ b/safe_format_and_mount_test.go @@ -21,7 +21,6 @@ import ( "io/ioutil" "os" "runtime" - "strings" "testing" "k8s.io/utils/exec" @@ -60,12 +59,12 @@ func TestSafeFormatAndMount(t *testing.T) { } defer os.RemoveAll(mntDir) tests := []struct { - description string - fstype string - mountOptions []string - execScripts []ExecArgs - mountErrs []error - expectedError error + description string + fstype string + mountOptions []string + execScripts []ExecArgs + mountErrs []error + expErrorType MountErrorType }{ { description: "Test a read only mount of an already formatted device", @@ -90,7 +89,7 @@ func TestSafeFormatAndMount(t *testing.T) { execScripts: []ExecArgs{ {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, }, - expectedError: fmt.Errorf("cannot mount unformatted disk /dev/foo as we are manipulating it in read-only mode"), + expErrorType: UnformattedReadOnly, }, { description: "Test a normal mount of unformatted device", @@ -107,7 +106,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=ext4\n", nil}, {"fsck", []string{"-a", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, }, - expectedError: fmt.Errorf("'fsck' found errors on device /dev/foo but could not correct them"), + expErrorType: HasFilesystemErrors, }, { description: "Test 'fsck' fails with exit status 1 (errors found and corrected)", @@ -133,7 +132,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nPTTYPE=dos\n", nil}, {"fsck", []string{"-a", "/dev/foo"}, "", nil}, }, - expectedError: fmt.Errorf("unknown filesystem type '(null)'"), + expErrorType: FilesystemMismatch, }, { description: "Test that 'blkid' is called and confirms unformatted disk, format fails", @@ -143,7 +142,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, }, - expectedError: fmt.Errorf("formatting failed"), + expErrorType: FormatFailed, }, { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, second mount fails", @@ -153,7 +152,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, - expectedError: fmt.Errorf("unknown filesystem type '(null)'"), + expErrorType: UnknownMountError, }, { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes", @@ -162,7 +161,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, - expectedError: nil, }, { description: "Test that 'blkid' is called and confirms unformatted disk, format passes, mount passes with ext3", @@ -171,7 +169,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.ext3", []string{"-F", "-m0", "/dev/foo"}, "", nil}, }, - expectedError: nil, }, { description: "test that none ext4 fs does not get called with ext4 options.", @@ -180,7 +177,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, - expectedError: nil, }, { description: "Test that 'blkid' is called and reports ext4 partition", @@ -198,7 +194,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 4}}, {"mkfs.xfs", []string{"/dev/foo"}, "", nil}, }, - expectedError: fmt.Errorf("exit 4"), + expErrorType: GetDiskFormatFailed, }, { description: "Test that 'xfs_repair' is called only once, no need to repair the filesystem", @@ -207,7 +203,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, {"xfs_repair", []string{"-n", "/dev/foo"}, "", nil}, }, - expectedError: nil, }, { description: "Test that 'xfs_repair' is called twice and repair the filesystem", @@ -217,7 +212,6 @@ func TestSafeFormatAndMount(t *testing.T) { {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, {"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil}, }, - expectedError: nil, }, { description: "Test that 'xfs_repair' is called twice and repair the filesystem, but mount failed", @@ -228,7 +222,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, {"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil}, }, - expectedError: fmt.Errorf("unknown filesystem type '(null)'"), + expErrorType: UnknownMountError, }, { description: "Test that 'xfs_repair' is called twice but could not repair the filesystem", @@ -238,7 +232,7 @@ func TestSafeFormatAndMount(t *testing.T) { {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, {"xfs_repair", []string{"/dev/foo"}, "\nAn error occurred\n", &testingexec.FakeExitError{Status: 1}}, }, - expectedError: fmt.Errorf("'xfs_repair' found errors on device %s but could not correct them: %v", "/dev/foo", "\nAn error occurred\n"), + expErrorType: HasFilesystemErrors, }, } @@ -260,7 +254,7 @@ func TestSafeFormatAndMount(t *testing.T) { device := "/dev/foo" dest := mntDir err := mounter.FormatAndMount(device, dest, test.fstype, test.mountOptions) - if test.expectedError == nil { + if len(test.expErrorType) == 0 { if err != nil { t.Errorf("test \"%s\" unexpected non-error: %v", test.description, err) } @@ -277,8 +271,12 @@ func TestSafeFormatAndMount(t *testing.T) { t.Errorf("test \"%s\" the correct device was not mounted", test.description) } } else { - if err == nil || !strings.HasPrefix(err.Error(), test.expectedError.Error()) { - t.Errorf("test \"%s\" unexpected error: \n [%v]. \nExpecting [%v]", test.description, err, test.expectedError) + mntErr, ok := err.(MountError) + if !ok { + t.Errorf("mount error not of mount error type: %v", err) + } + if mntErr.Type != test.expErrorType { + t.Errorf("test \"%s\" unexpected error: \n [%v]. \nExpecting err type[%v]", test.description, err, test.expErrorType) } } } From 8dd4fc79dc840e09afff303eb7b210adb7306168 Mon Sep 17 00:00:00 2001 From: saad-ali Date: Fri, 7 Feb 2020 17:43:22 -0800 Subject: [PATCH 12/27] Introduce paramater for sensitive mount options. Introduce optional sensitiveOptions parameter to allow sensitive mount options to be passed in a separate parameter from the normal mount options and ensures the sensitiveOptions are never logged. --- fake_mounter.go | 11 ++- mount.go | 80 ++++++++++++++++++++-- mount_linux.go | 113 ++++++++++++++++++++++--------- mount_linux_test.go | 77 +++++++++++++++++++++ mount_test.go | 124 ++++++++++++++++++++++++++++++++++ mount_unsupported.go | 7 +- mount_windows.go | 24 +++++-- safe_format_and_mount_test.go | 50 ++++++++++++-- 8 files changed, 433 insertions(+), 53 deletions(-) diff --git a/fake_mounter.go b/fake_mounter.go index d3a82f415c2..8e690bbfc28 100644 --- a/fake_mounter.go +++ b/fake_mounter.go @@ -82,6 +82,15 @@ func (f *FakeMounter) GetLog() []FakeAction { // Mount records the mount event and updates the in-memory mount points for FakeMounter func (f *FakeMounter) Mount(source string, target string, fstype string, options []string) error { + return f.MountSensitive(source, target, fstype, options, nil /* sensitiveOptions */) +} + +// Mount records the mount event and updates the in-memory mount points for FakeMounter +// sensitiveOptions to be passed in a separate parameter from the normal +// mount options and ensures the sensitiveOptions are never logged. This +// method should be used by callers that pass sensitive material (like +// passwords) as mount options. +func (f *FakeMounter) MountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { f.mutex.Lock() defer f.mutex.Unlock() @@ -117,7 +126,7 @@ func (f *FakeMounter) Mount(source string, target string, fstype string, options if err != nil { absTarget = target } - f.MountPoints = append(f.MountPoints, MountPoint{Device: source, Path: absTarget, Type: fstype, Opts: opts}) + f.MountPoints = append(f.MountPoints, MountPoint{Device: source, Path: absTarget, Type: fstype, Opts: append(opts, sensitiveOptions...)}) klog.V(5).Infof("Fake mounter: mounted %s to %s", source, absTarget) f.log = append(f.log, FakeAction{Action: FakeActionMount, Target: absTarget, Source: source, FSType: fstype}) return nil diff --git a/mount.go b/mount.go index 46d77327fbf..112d7ebc178 100644 --- a/mount.go +++ b/mount.go @@ -31,12 +31,21 @@ import ( const ( // Default mount command if mounter path is not specified. defaultMountCommand = "mount" + // Log message where sensitive mount options were removed + sensitiveOptionsRemoved = "" ) // Interface defines the set of methods to allow for mount operations on a system. type Interface interface { // Mount mounts source to target as fstype with given options. + // options MUST not contain sensitive material (like passwords). Mount(source string, target string, fstype string, options []string) error + // MountSensitive is the same as Mount() but this method allows + // sensitiveOptions to be passed in a separate parameter from the normal + // mount options and ensures the sensitiveOptions are never logged. This + // method should be used by callers that pass sensitive material (like + // passwords) as mount options. + MountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error // Unmount unmounts given target. Unmount(target string) error // List returns a list of all mounted filesystems. This can be large. @@ -72,7 +81,7 @@ type MountPoint struct { Device string Path string Type string - Opts []string + Opts []string // Opts may contain sensitive mount options (like passwords) and MUST be treated as such (e.g. not logged). Freq int Pass int } @@ -122,8 +131,18 @@ type SafeFormatAndMount struct { // read-only it will format it first then mount it. Otherwise, if the // disk is already formatted or it is being mounted as read-only, it // will be mounted without formatting. +// options MUST not contain sensitive material (like passwords). func (mounter *SafeFormatAndMount) FormatAndMount(source string, target string, fstype string, options []string) error { - return mounter.formatAndMount(source, target, fstype, options) + return mounter.FormatAndMountSensitive(source, target, fstype, options, nil /* sensitiveOptions */) +} + +// FormatAndMountSensitive is the same as FormatAndMount but this method allows +// sensitiveOptions to be passed in a separate parameter from the normal mount +// options and ensures the sensitiveOptions are never logged. This method should +// be used by callers that pass sensitive material (like passwords) as mount +// options. +func (mounter *SafeFormatAndMount) FormatAndMountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { + return mounter.formatAndMountSensitive(source, target, fstype, options, sensitiveOptions) } // getMountRefsByDev finds all references to the device provided @@ -240,6 +259,16 @@ func IsNotMountPoint(mounter Interface, file string) (bool, error) { // The list equals: // options - 'bind' + 'remount' (no duplicate) func MakeBindOpts(options []string) (bool, []string, []string) { + bind, bindOpts, bindRemountOpts, _ := MakeBindOptsSensitive(options, nil /* sensitiveOptions */) + return bind, bindOpts, bindRemountOpts +} + +// MakeBindOptsSensitive is the same as MakeBindOpts but this method allows +// sensitiveOptions to be passed in a separate parameter from the normal mount +// options and ensures the sensitiveOptions are never logged. This method should +// be used by callers that pass sensitive material (like passwords) as mount +// options. +func MakeBindOptsSensitive(options []string, sensitiveOptions []string) (bool, []string, []string, []string) { // Because we have an FD opened on the subpath bind mount, the "bind" option // needs to be included, otherwise the mount target will error as busy if you // remount as readonly. @@ -247,12 +276,13 @@ func MakeBindOpts(options []string) (bool, []string, []string) { // As a consequence, all read only bind mounts will no longer change the underlying // volume mount to be read only. bindRemountOpts := []string{"bind", "remount"} + bindRemountSensitiveOpts := []string{} bind := false bindOpts := []string{"bind"} // _netdev is a userspace mount option and does not automatically get added when // bind mount is created and hence we must carry it over. - if checkForNetDev(options) { + if checkForNetDev(options, sensitiveOptions) { bindOpts = append(bindOpts, "_netdev") } @@ -268,15 +298,32 @@ func MakeBindOpts(options []string) (bool, []string, []string) { } } - return bind, bindOpts, bindRemountOpts + for _, sensitiveOption := range sensitiveOptions { + switch sensitiveOption { + case "bind": + bind = true + break + case "remount": + break + default: + bindRemountSensitiveOpts = append(bindRemountSensitiveOpts, sensitiveOption) + } + } + + return bind, bindOpts, bindRemountOpts, bindRemountSensitiveOpts } -func checkForNetDev(options []string) bool { +func checkForNetDev(options []string, sensitiveOptions []string) bool { for _, option := range options { if option == "_netdev" { return true } } + for _, sensitiveOption := range sensitiveOptions { + if sensitiveOption == "_netdev" { + return true + } + } return false } @@ -298,3 +345,26 @@ func StartsWithBackstep(rel string) bool { // normalize to / and check for ../ return rel == ".." || strings.HasPrefix(filepath.ToSlash(rel), "../") } + +// sanitizedOptionsForLogging will return a comma seperated string containing +// options and sensitiveOptions. Each entry in sensitiveOptions will be +// replaced with the string sensitiveOptionsRemoved +// e.g. o1,o2,, +func sanitizedOptionsForLogging(options []string, sensitiveOptions []string) string { + seperator := "" + if len(options) > 0 && len(sensitiveOptions) > 0 { + seperator = "," + } + + sensitiveOptionsStart := "" + sensitiveOptionsEnd := "" + if len(sensitiveOptions) > 0 { + sensitiveOptionsStart = strings.Repeat(sensitiveOptionsRemoved+",", len(sensitiveOptions)-1) + sensitiveOptionsEnd = sensitiveOptionsRemoved + } + + return strings.Join(options, ",") + + seperator + + sensitiveOptionsStart + + sensitiveOptionsEnd +} diff --git a/mount_linux.go b/mount_linux.go index 4dc696d69b0..2d24af913e4 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -69,16 +69,25 @@ func New(mounterPath string) Interface { // currently come from mount(8), e.g. "ro", "remount", "bind", etc. If no more option is // required, call Mount with an empty string list or nil. func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error { + return mounter.MountSensitive(source, target, fstype, options, nil) +} + +// MountSensitive is the same as Mount() but this method allows +// sensitiveOptions to be passed in a separate parameter from the normal +// mount options and ensures the sensitiveOptions are never logged. This +// method should be used by callers that pass sensitive material (like +// passwords) as mount options. +func (mounter *Mounter) MountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { // Path to mounter binary if containerized mounter is needed. Otherwise, it is set to empty. // All Linux distros are expected to be shipped with a mount utility that a support bind mounts. mounterPath := "" - bind, bindOpts, bindRemountOpts := MakeBindOpts(options) + bind, bindOpts, bindRemountOpts, bindRemountOptsSensitive := MakeBindOptsSensitive(options, sensitiveOptions) if bind { - err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts) + err := mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindOpts, bindRemountOptsSensitive) if err != nil { return err } - return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts) + return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, bindRemountOpts, bindRemountOptsSensitive) } // The list of filesystems that require containerized mounter on GCI image cluster fsTypesNeedMounter := map[string]struct{}{ @@ -90,14 +99,16 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio if _, ok := fsTypesNeedMounter[fstype]; ok { mounterPath = mounter.mounterPath } - return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, options) + return mounter.doMount(mounterPath, defaultMountCommand, source, target, fstype, options, sensitiveOptions) } // doMount runs the mount command. mounterPath is the path to mounter binary if containerized mounter is used. -func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source string, target string, fstype string, options []string) error { - mountArgs := MakeMountArgs(source, target, fstype, options) +// sensitiveOptions is an extention of options except they will not be logged (because they may contain sensitive material) +func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source string, target string, fstype string, options []string, sensitiveOptions []string) error { + mountArgs, mountArgsLogStr := MakeMountArgsSensitive(source, target, fstype, options, sensitiveOptions) if len(mounterPath) > 0 { mountArgs = append([]string{mountCmd}, mountArgs...) + mountArgsLogStr = mountCmd + " " + mountArgsLogStr mountCmd = mounterPath } @@ -124,21 +135,21 @@ func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source stri // // systemd-mount is not used because it's too new for older distros // (CentOS 7, Debian Jessie). - mountCmd, mountArgs = AddSystemdScope("systemd-run", target, mountCmd, mountArgs) + mountCmd, mountArgs, mountArgsLogStr = AddSystemdScopeSensitive("systemd-run", target, mountCmd, mountArgs, mountArgsLogStr) } else { // No systemd-run on the host (or we failed to check it), assume kubelet // does not run as a systemd service. // No code here, mountCmd and mountArgs are already populated. } - klog.V(4).Infof("Mounting cmd (%s) with arguments (%s)", mountCmd, mountArgs) + // Logging with sensitive mount options removed. + klog.V(4).Infof("Mounting cmd (%s) with arguments (%s)", mountCmd, mountArgsLogStr) command := exec.Command(mountCmd, mountArgs...) output, err := command.CombinedOutput() if err != nil { - args := strings.Join(mountArgs, " ") - klog.Errorf("Mount failed: %v\nMounting command: %s\nMounting arguments: %s\nOutput: %s\n", err, mountCmd, args, string(output)) + klog.Errorf("Mount failed: %v\nMounting command: %s\nMounting arguments: %s\nOutput: %s\n", err, mountCmd, mountArgsLogStr, string(output)) return fmt.Errorf("mount failed: %v\nMounting command: %s\nMounting arguments: %s\nOutput: %s", - err, mountCmd, args, string(output)) + err, mountCmd, mountArgsLogStr, string(output)) } return err } @@ -169,33 +180,59 @@ func detectSystemd() bool { } // MakeMountArgs makes the arguments to the mount(8) command. -// Implementation is shared with NsEnterMounter -func MakeMountArgs(source, target, fstype string, options []string) []string { - // Build mount command as follows: - // mount [-t $fstype] [-o $options] [$source] $target - mountArgs := []string{} - if len(fstype) > 0 { - mountArgs = append(mountArgs, "-t", fstype) - } - if len(options) > 0 { - mountArgs = append(mountArgs, "-o", strings.Join(options, ",")) - } - if len(source) > 0 { - mountArgs = append(mountArgs, source) - } - mountArgs = append(mountArgs, target) - +// options MUST not contain sensitive material (like passwords). +func MakeMountArgs(source, target, fstype string, options []string) (mountArgs []string) { + mountArgs, _ = MakeMountArgsSensitive(source, target, fstype, options, nil /* sensitiveOptions */) return mountArgs } +// MakeMountArgsSensitive makes the arguments to the mount(8) command. +// sensitiveOptions is an extention of options except they will not be logged (because they may contain sensitive material) +func MakeMountArgsSensitive(source, target, fstype string, options []string, sensitiveOptions []string) (mountArgs []string, mountArgsLogStr string) { + // Build mount command as follows: + // mount [-t $fstype] [-o $options] [$source] $target + mountArgs = []string{} + mountArgsLogStr = "" + if len(fstype) > 0 { + mountArgs = append(mountArgs, "-t", fstype) + mountArgsLogStr += strings.Join(mountArgs, " ") + } + if len(options) > 0 || len(sensitiveOptions) > 0 { + combinedOptions := []string{} + combinedOptions = append(combinedOptions, options...) + combinedOptions = append(combinedOptions, sensitiveOptions...) + mountArgs = append(mountArgs, "-o", strings.Join(combinedOptions, ",")) + // exclude sensitiveOptions from log string + mountArgsLogStr += " -o " + sanitizedOptionsForLogging(options, sensitiveOptions) + } + if len(source) > 0 { + mountArgs = append(mountArgs, source) + mountArgsLogStr += " " + source + } + mountArgs = append(mountArgs, target) + mountArgsLogStr += " " + target + + return mountArgs, mountArgsLogStr +} + // AddSystemdScope adds "system-run --scope" to given command line -// implementation is shared with NsEnterMounter +// If args contains sensitive material, use AddSystemdScopeSensitive to construct +// a safe to log string. func AddSystemdScope(systemdRunPath, mountName, command string, args []string) (string, []string) { descriptionArg := fmt.Sprintf("--description=Kubernetes transient mount for %s", mountName) systemdRunArgs := []string{descriptionArg, "--scope", "--", command} return systemdRunPath, append(systemdRunArgs, args...) } +// AddSystemdScopeSensitive adds "system-run --scope" to given command line +// It also accepts takes a sanitized string containing mount arguments, mountArgsLogStr, +// and returns the string appended to the systemd command for logging. +func AddSystemdScopeSensitive(systemdRunPath, mountName, command string, args []string, mountArgsLogStr string) (string, []string, string) { + descriptionArg := fmt.Sprintf("--description=Kubernetes transient mount for %s", mountName) + systemdRunArgs := []string{descriptionArg, "--scope", "--", command} + return systemdRunPath, append(systemdRunArgs, args...), strings.Join(systemdRunArgs, " ") + " " + mountArgsLogStr +} + // Unmount unmounts the target. func (mounter *Mounter) Unmount(target string) error { klog.V(4).Infof("Unmounting %s", target) @@ -305,7 +342,7 @@ func (mounter *SafeFormatAndMount) checkAndRepairXfsFilesystem(source string) er } // formatAndMount uses unix utils to format and mount the given disk -func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error { +func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { readOnly := false for _, option := range options { if option == "ro" { @@ -313,6 +350,15 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, break } } + if !readOnly { + // Check sensitiveOptions for ro + for _, option := range sensitiveOptions { + if option == "ro" { + readOnly = true + break + } + } + } options = append(options, "defaults") mountErrorValue := UnknownMountError @@ -347,7 +393,9 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, klog.Infof("Disk %q appears to be unformatted, attempting to format as type: %q with options: %v", source, fstype, args) output, err := mounter.Exec.Command("mkfs."+fstype, args...).CombinedOutput() if err != nil { - detailedErr := fmt.Sprintf("format of disk %q failed: type:(%q) target:(%q) options:(%q) errcode:(%v) output:(%v) ", source, fstype, target, options, err, string(output)) + // Do not log sensitiveOptions only options + sensitiveOptionsLog := sanitizedOptionsForLogging(options, sensitiveOptions) + detailedErr := fmt.Sprintf("format of disk %q failed: type:(%q) target:(%q) options:(%q) errcode:(%v) output:(%v) ", source, fstype, target, sensitiveOptionsLog, err, string(output)) klog.Error(detailedErr) return NewMountError(FormatFailed, detailedErr) } @@ -378,7 +426,7 @@ func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, // Mount the disk klog.V(4).Infof("Attempting to mount disk %s in %s format at %s", source, fstype, target) - if err := mounter.Interface.Mount(source, target, fstype, options); err != nil { + if err := mounter.MountSensitive(source, target, fstype, options, sensitiveOptions); err != nil { return NewMountError(mountErrorValue, err.Error()) } @@ -457,7 +505,8 @@ func parseProcMounts(content []byte) ([]MountPoint, error) { } fields := strings.Fields(line) if len(fields) != expectedNumFieldsPerLine { - return nil, fmt.Errorf("wrong number of fields (expected %d, got %d): %s", expectedNumFieldsPerLine, len(fields), line) + // Do not log line in case it contains sensitive Mount options + return nil, fmt.Errorf("wrong number of fields (expected %d, got %d)", expectedNumFieldsPerLine, len(fields)) } mp := MountPoint{ diff --git a/mount_linux_test.go b/mount_linux_test.go index 47b6e31a04c..dc7d4fd882f 100644 --- a/mount_linux_test.go +++ b/mount_linux_test.go @@ -22,6 +22,7 @@ import ( "io/ioutil" "os" "reflect" + "strings" "testing" ) @@ -437,3 +438,79 @@ func TestSearchMountPoints(t *testing.T) { } } } + +func TestSensitiveMountOptions(t *testing.T) { + // Arrange + testcases := []struct { + source string + target string + fstype string + options []string + sensitiveOptions []string + }{ + { + + source: "mySrc", + target: "myTarget", + fstype: "myFS", + options: []string{"o1", "o2"}, + sensitiveOptions: []string{"s1", "s2"}, + }, + { + + source: "mySrc", + target: "myTarget", + fstype: "myFS", + options: []string{}, + sensitiveOptions: []string{"s1", "s2"}, + }, + { + + source: "mySrc", + target: "myTarget", + fstype: "myFS", + options: []string{"o1", "o2"}, + sensitiveOptions: []string{}, + }, + } + + for _, v := range testcases { + // Act + mountArgs, mountArgsLogStr := MakeMountArgsSensitive(v.source, v.target, v.fstype, v.options, v.sensitiveOptions) + + // Assert + t.Logf("\r\nmountArgs =%q\r\nmountArgsLogStr=%q", mountArgs, mountArgsLogStr) + for _, option := range v.options { + if found := contains(mountArgs, option, t); !found { + t.Errorf("Expected option (%q) to exist in returned mountArts (%q), but it does not", option, mountArgs) + } + if !strings.Contains(mountArgsLogStr, option) { + t.Errorf("Expected option (%q) to exist in returned mountArgsLogStr (%q), but it does", option, mountArgsLogStr) + } + } + for _, sensitiveOption := range v.sensitiveOptions { + if found := contains(mountArgs, sensitiveOption, t); !found { + t.Errorf("Expected sensitiveOption (%q) to exist in returned mountArts (%q), but it does not", sensitiveOption, mountArgs) + } + if strings.Contains(mountArgsLogStr, sensitiveOption) { + t.Errorf("Expected sensitiveOption (%q) to not exist in returned mountArgsLogStr (%q), but it does", sensitiveOption, mountArgsLogStr) + } + } + } +} + +func contains(slice []string, str string, t *testing.T) bool { + optionsIndex := -1 + for i, s := range slice { + if s == "-o" { + optionsIndex = i + 1 + break + } + } + + if optionsIndex < 0 || optionsIndex >= len(slice) { + return false + } + + return strings.Contains(slice[optionsIndex], str) +} diff --git a/mount_test.go b/mount_test.go index 6b9d5d5fc4b..041a28dae00 100644 --- a/mount_test.go +++ b/mount_test.go @@ -18,6 +18,7 @@ package mount import ( "reflect" + "strings" "testing" ) @@ -58,3 +59,126 @@ func TestMakeBindOpts(t *testing.T) { } } + +func TestMakeBindOptsSensitive(t *testing.T) { + tests := []struct { + mountOptions []string + sensitiveMountOptions []string + isBind bool + expectedBindOpts []string + expectedRemountOpts []string + expectedSensitiveRemountOpts []string + }{ + { + mountOptions: []string{"vers=2", "ro", "_netdev"}, + sensitiveMountOptions: []string{"user=foo", "pass=bar"}, + isBind: false, + expectedBindOpts: []string{}, + expectedRemountOpts: []string{}, + expectedSensitiveRemountOpts: []string{"user=foo", "pass=bar"}, + }, + { + + mountOptions: []string{"vers=2", "ro", "_netdev"}, + sensitiveMountOptions: []string{"user=foo", "pass=bar", "bind"}, + isBind: true, + expectedBindOpts: []string{"bind", "_netdev"}, + expectedRemountOpts: []string{"bind", "remount", "vers=2", "ro", "_netdev"}, + expectedSensitiveRemountOpts: []string{"user=foo", "pass=bar"}, + }, + { + mountOptions: []string{"vers=2", "remount", "ro", "_netdev"}, + sensitiveMountOptions: []string{"user=foo", "pass=bar"}, + isBind: false, + expectedBindOpts: []string{}, + expectedRemountOpts: []string{}, + expectedSensitiveRemountOpts: []string{"user=foo", "pass=bar"}, + }, + { + mountOptions: []string{"vers=2", "ro", "_netdev"}, + sensitiveMountOptions: []string{"user=foo", "pass=bar", "remount"}, + isBind: false, + expectedBindOpts: []string{}, + expectedRemountOpts: []string{}, + expectedSensitiveRemountOpts: []string{"user=foo", "pass=bar"}, + }, + { + + mountOptions: []string{"vers=2", "bind", "ro", "_netdev"}, + sensitiveMountOptions: []string{"user=foo", "remount", "pass=bar"}, + isBind: true, + expectedBindOpts: []string{"bind", "_netdev"}, + expectedRemountOpts: []string{"bind", "remount", "vers=2", "ro", "_netdev"}, + expectedSensitiveRemountOpts: []string{"user=foo", "pass=bar"}, + }, + { + + mountOptions: []string{"vers=2", "bind", "ro", "_netdev"}, + sensitiveMountOptions: []string{"user=foo", "remount", "pass=bar"}, + isBind: true, + expectedBindOpts: []string{"bind", "_netdev"}, + expectedRemountOpts: []string{"bind", "remount", "vers=2", "ro", "_netdev"}, + expectedSensitiveRemountOpts: []string{"user=foo", "pass=bar"}, + }, + } + for _, test := range tests { + bind, bindOpts, bindRemountOpts, bindRemountSensitiveOpts := MakeBindOptsSensitive(test.mountOptions, test.sensitiveMountOptions) + if bind != test.isBind { + t.Errorf("Expected bind to be %v but got %v", test.isBind, bind) + } + if test.isBind { + if !reflect.DeepEqual(test.expectedBindOpts, bindOpts) { + t.Errorf("Expected bind mount options to be %+v got %+v", test.expectedBindOpts, bindOpts) + } + if !reflect.DeepEqual(test.expectedRemountOpts, bindRemountOpts) { + t.Errorf("Expected remount options to be %+v got %+v", test.expectedRemountOpts, bindRemountOpts) + } + if !reflect.DeepEqual(test.expectedSensitiveRemountOpts, bindRemountSensitiveOpts) { + t.Errorf("Expected sensitive remount options to be %+v got %+v", test.expectedSensitiveRemountOpts, bindRemountSensitiveOpts) + } + } + + } +} + +func TestOptionsForLogging(t *testing.T) { + // Arrange + testcases := []struct { + options []string + sensitiveOptions []string + }{ + { + options: []string{"o1", "o2"}, + sensitiveOptions: []string{"s1"}, + }, + { + options: []string{"o1", "o2"}, + sensitiveOptions: []string{"s1", "s2"}, + }, + { + sensitiveOptions: []string{"s1", "s2"}, + }, + { + options: []string{"o1", "o2"}, + }, + {}, + } + + for _, v := range testcases { + // Act + maskedStr := sanitizedOptionsForLogging(v.options, v.sensitiveOptions) + + // Assert + for _, sensitiveOption := range v.sensitiveOptions { + if strings.Contains(maskedStr, sensitiveOption) { + t.Errorf("Found sensitive log option %q in %q", sensitiveOption, maskedStr) + } + } + + actualCount := strings.Count(maskedStr, sensitiveOptionsRemoved) + expectedCount := len(v.sensitiveOptions) + if actualCount != expectedCount { + t.Errorf("Found %v instances of %q in %q. Expected %v", actualCount, sensitiveOptionsRemoved, maskedStr, expectedCount) + } + } +} diff --git a/mount_unsupported.go b/mount_unsupported.go index d13ec5c34d5..985edbe3d56 100644 --- a/mount_unsupported.go +++ b/mount_unsupported.go @@ -43,6 +43,11 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio return errUnsupported } +// Mount always returns an error on unsupported platforms +func (mounter *Mounter) MountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { + return errUnsupported +} + // Unmount always returns an error on unsupported platforms func (mounter *Mounter) Unmount(target string) error { return errUnsupported @@ -63,7 +68,7 @@ func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { return nil, errUnsupported } -func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error { +func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { return mounter.Interface.Mount(source, target, fstype, options) } diff --git a/mount_windows.go b/mount_windows.go index 9371ff43551..e9fad2b47fa 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -53,10 +53,20 @@ var getSMBMountMutex = keymutex.NewHashed(0) // Mount : mounts source to target with given options. // currently only supports cifs(smb), bind mount(for disk) func (mounter *Mounter) Mount(source string, target string, fstype string, options []string) error { + return mounter.MountSensitive(source, target, fstype, options, nil /* sensitiveOptions */) +} + +// MountSensitive is the same as Mount() but this method allows +// sensitiveOptions to be passed in a separate parameter from the normal +// mount options and ensures the sensitiveOptions are never logged. This +// method should be used by callers that pass sensitive material (like +// passwords) as mount options. +func (mounter *Mounter) MountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { target = NormalizeWindowsPath(target) + sanitizedOptionsForLogging := sanitizedOptionsForLogging(options, sensitiveOptions) if source == "tmpfs" { - klog.V(3).Infof("mounting source (%q), target (%q), with options (%q)", source, target, options) + klog.V(3).Infof("mounting source (%q), target (%q), with options (%q)", source, target, sanitizedOptionsForLogging) return os.MkdirAll(target, 0755) } @@ -66,23 +76,23 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio } klog.V(4).Infof("mount options(%q) source:%q, target:%q, fstype:%q, begin to mount", - options, source, target, fstype) + sanitizedOptionsForLogging, source, target, fstype) bindSource := source // tell it's going to mount azure disk or azure file according to options - if bind, _, _ := MakeBindOpts(options); bind { + if bind, _, _, _ := MakeBindOpts(options, sensitiveOptions); bind { // mount azure disk bindSource = NormalizeWindowsPath(source) } else { if len(options) < 2 { klog.Warningf("mount options(%q) command number(%d) less than 2, source:%q, target:%q, skip mounting", - options, len(options), source, target) + sanitizedOptionsForLogging, len(options), source, target) return nil } // currently only cifs mount is supported if strings.ToLower(fstype) != "cifs" { - return fmt.Errorf("only cifs mount is supported now, fstype: %q, mounting source (%q), target (%q), with options (%q)", fstype, source, target, options) + return fmt.Errorf("only cifs mount is supported now, fstype: %q, mounting source (%q), target (%q), with options (%q)", fstype, source, target, sanitizedOptionsForLogging) } // lock smb mount for the same source @@ -116,7 +126,7 @@ func (mounter *Mounter) Mount(source string, target string, fstype string, optio // return (output, error) func newSMBMapping(username, password, remotepath string) (string, error) { if username == "" || password == "" || remotepath == "" { - return "", fmt.Errorf("invalid parameter(username: %s, password: %s, remoteapth: %s)", username, password, remotepath) + return "", fmt.Errorf("invalid parameter(username: %s, password: %s, remoteapth: %s)", username, sensitiveOptionsRemoved, remotepath) } // use PowerShell Environment Variables to store user input string to prevent command line injection @@ -203,7 +213,7 @@ func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { return []string{pathname}, nil } -func (mounter *SafeFormatAndMount) formatAndMount(source string, target string, fstype string, options []string) error { +func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { // Try to mount the disk klog.V(4).Infof("Attempting to formatAndMount disk: %s %s %s", fstype, source, target) diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go index a0869293656..782d3da67e7 100644 --- a/safe_format_and_mount_test.go +++ b/safe_format_and_mount_test.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "runtime" + "strings" "testing" "k8s.io/utils/exec" @@ -34,6 +35,10 @@ type ErrorMounter struct { } func (mounter *ErrorMounter) Mount(source string, target string, fstype string, options []string) error { + return mounter.MountSensitive(source, target, fstype, options, nil /* sensitiveOptions */) +} + +func (mounter *ErrorMounter) MountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { i := mounter.errIndex mounter.errIndex++ if mounter.err != nil && mounter.err[i] != nil { @@ -59,12 +64,13 @@ func TestSafeFormatAndMount(t *testing.T) { } defer os.RemoveAll(mntDir) tests := []struct { - description string - fstype string - mountOptions []string - execScripts []ExecArgs - mountErrs []error - expErrorType MountErrorType + description string + fstype string + mountOptions []string + sensitiveMountOptions []string + execScripts []ExecArgs + mountErrs []error + expErrorType MountErrorType }{ { description: "Test a read only mount of an already formatted device", @@ -234,6 +240,17 @@ func TestSafeFormatAndMount(t *testing.T) { }, expErrorType: HasFilesystemErrors, }, + { + description: "Test that 'blkid' is called and confirms unformatted disk, format fails with sensitive options", + fstype: "ext4", + sensitiveMountOptions: []string{"mySecret"}, + mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, + execScripts: []ExecArgs{ + {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 2}}, + {"mkfs.ext4", []string{"-F", "-m0", "/dev/foo"}, "", fmt.Errorf("formatting failed")}, + }, + expErrorType: FormatFailed, + }, } for _, test := range tests { @@ -253,7 +270,12 @@ func TestSafeFormatAndMount(t *testing.T) { device := "/dev/foo" dest := mntDir - err := mounter.FormatAndMount(device, dest, test.fstype, test.mountOptions) + var err error + if len(test.sensitiveMountOptions) == 0 { + err = mounter.FormatAndMount(device, dest, test.fstype, test.mountOptions) + } else { + err = mounter.FormatAndMountSensitive(device, dest, test.fstype, test.mountOptions, test.sensitiveMountOptions) + } if len(test.expErrorType) == 0 { if err != nil { t.Errorf("test \"%s\" unexpected non-error: %v", test.description, err) @@ -278,6 +300,20 @@ func TestSafeFormatAndMount(t *testing.T) { if mntErr.Type != test.expErrorType { t.Errorf("test \"%s\" unexpected error: \n [%v]. \nExpecting err type[%v]", test.description, err, test.expErrorType) } + if len(test.sensitiveMountOptions) == 0 { + if strings.Contains(mntErr.Error(), sensitiveOptionsRemoved) { + t.Errorf("test \"%s\" returned an error unexpectedly containing the string %q: %v", test.description, sensitiveOptionsRemoved, err) + } + } else { + if !strings.Contains(err.Error(), sensitiveOptionsRemoved) { + t.Errorf("test \"%s\" returned an error without the string %q: %v", test.description, sensitiveOptionsRemoved, err) + } + for _, sensitiveOption := range test.sensitiveMountOptions { + if strings.Contains(err.Error(), sensitiveOption) { + t.Errorf("test \"%s\" returned an error with a sensitive string (%q): %v", test.description, sensitiveOption, err) + } + } + } } } } From 6a662180ac11c980c2baaf571f73bda81ce7bff7 Mon Sep 17 00:00:00 2001 From: saad-ali Date: Fri, 28 Feb 2020 19:58:05 -0800 Subject: [PATCH 13/27] Fix mount_windows build error --- mount_windows.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mount_windows.go b/mount_windows.go index e9fad2b47fa..d9ae1116d68 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -80,7 +80,7 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri bindSource := source // tell it's going to mount azure disk or azure file according to options - if bind, _, _, _ := MakeBindOpts(options, sensitiveOptions); bind { + if bind, _, _, _ := MakeBindOptsSensitive(options, sensitiveOptions); bind { // mount azure disk bindSource = NormalizeWindowsPath(source) } else { From 3f4217bc69a579b543334b543b8d0e6a46a618da Mon Sep 17 00:00:00 2001 From: kvaps Date: Wed, 18 Mar 2020 09:15:17 +0100 Subject: [PATCH 14/27] Fix subPath mountpint check --- mount_helper_common.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/mount_helper_common.go b/mount_helper_common.go index 81f91a8be8d..36bb32b463d 100644 --- a/mount_helper_common.go +++ b/mount_helper_common.go @@ -48,9 +48,9 @@ func CleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointC // if corruptedMnt is true, it means that the mountPath is a corrupted mountpoint, and the mount point check // will be skipped func doCleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool, corruptedMnt bool) error { + var notMnt bool + var err error if !corruptedMnt { - var notMnt bool - var err error if extensiveMountPointCheck { notMnt, err = IsNotMountPoint(mounter, mountPath) } else { @@ -73,9 +73,13 @@ func doCleanupMountPoint(mountPath string, mounter Interface, extensiveMountPoin return err } - notMnt, mntErr := mounter.IsLikelyNotMountPoint(mountPath) - if mntErr != nil { - return mntErr + if extensiveMountPointCheck { + notMnt, err = IsNotMountPoint(mounter, mountPath) + } else { + notMnt, err = mounter.IsLikelyNotMountPoint(mountPath) + } + if err != nil { + return err } if notMnt { klog.V(4).Infof("%q is unmounted, deleting the directory", mountPath) From d37a1749ed53742361fd83b067022e64b53b6633 Mon Sep 17 00:00:00 2001 From: Matt Boersma Date: Thu, 19 Mar 2020 19:04:56 -0600 Subject: [PATCH 15/27] Fix windows MountSensitive error --- mount_windows.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/mount_windows.go b/mount_windows.go index d9ae1116d68..a99fdb9ce5b 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -79,14 +79,15 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri sanitizedOptionsForLogging, source, target, fstype) bindSource := source - // tell it's going to mount azure disk or azure file according to options if bind, _, _, _ := MakeBindOptsSensitive(options, sensitiveOptions); bind { - // mount azure disk bindSource = NormalizeWindowsPath(source) } else { - if len(options) < 2 { + allOptions := []string{} + allOptions = append(allOptions, options...) + allOptions = append(allOptions, sensitiveOptions...) + if len(allOptions) < 2 { klog.Warningf("mount options(%q) command number(%d) less than 2, source:%q, target:%q, skip mounting", - sanitizedOptionsForLogging, len(options), source, target) + sanitizedOptionsForLogging, len(allOptions), source, target) return nil } @@ -99,13 +100,13 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri getSMBMountMutex.LockKey(source) defer getSMBMountMutex.UnlockKey(source) - if output, err := newSMBMapping(options[0], options[1], source); err != nil { + if output, err := newSMBMapping(allOptions[0], allOptions[1], source); err != nil { if isSMBMappingExist(source) { klog.V(2).Infof("SMB Mapping(%s) already exists, now begin to remove and remount", source) if output, err := removeSMBMapping(source); err != nil { return fmt.Errorf("Remove-SmbGlobalMapping failed: %v, output: %q", err, output) } - if output, err := newSMBMapping(options[0], options[1], source); err != nil { + if output, err := newSMBMapping(allOptions[0], allOptions[1], source); err != nil { return fmt.Errorf("New-SmbGlobalMapping remount failed: %v, output: %q", err, output) } } else { From 5f0ba4923c503e0741bf02e9935c06061f3713e9 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Tue, 24 Mar 2020 13:38:13 -0400 Subject: [PATCH 16/27] Revert xfs_repair fix --- mount_linux.go | 36 +-------------------------------- safe_format_and_mount_test.go | 38 ----------------------------------- 2 files changed, 1 insertion(+), 73 deletions(-) diff --git a/mount_linux.go b/mount_linux.go index 2d24af913e4..41f69efe3f0 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -314,33 +314,6 @@ func (mounter *SafeFormatAndMount) checkAndRepairFilesystem(source string) error return nil } -// checkAndRepairXfsFilesystem checks and repairs xfs filesystem using command xfs_repair. -func (mounter *SafeFormatAndMount) checkAndRepairXfsFilesystem(source string) error { - klog.V(4).Infof("Checking for issues with xfs_repair on disk: %s", source) - - args := []string{source} - checkArgs := []string{"-n", source} - - // check-only using "xfs_repair -n", if the exit status is not 0, perform a "xfs_repair" - _, err := mounter.Exec.Command("xfs_repair", checkArgs...).CombinedOutput() - if err != nil { - if err == utilexec.ErrExecutableNotFound { - klog.Warningf("'xfs_repair' not found on system; continuing mount without running 'xfs_repair'.") - return nil - } else { - klog.Warningf("Filesystem corruption was detected for %s, running xfs_repair to repair", source) - out, err := mounter.Exec.Command("xfs_repair", args...).CombinedOutput() - if err != nil { - return NewMountError(HasFilesystemErrors, "'xfs_repair' found errors on device %s but could not correct them: %s\n", source, out) - } else { - klog.Infof("Device %s has errors which were corrected by xfs_repair.", source) - return nil - } - } - } - return nil -} - // formatAndMount uses unix utils to format and mount the given disk func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target string, fstype string, options []string, sensitiveOptions []string) error { readOnly := false @@ -410,14 +383,7 @@ func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target if !readOnly { // Run check tools on the disk to fix repairable issues, only do this for formatted volumes requested as rw. - var err error - switch existingFormat { - case "xfs": - err = mounter.checkAndRepairXfsFilesystem(source) - default: - err = mounter.checkAndRepairFilesystem(source) - } - + err := mounter.checkAndRepairFilesystem(source) if err != nil { return err } diff --git a/safe_format_and_mount_test.go b/safe_format_and_mount_test.go index 782d3da67e7..38c2c8f5b9f 100644 --- a/safe_format_and_mount_test.go +++ b/safe_format_and_mount_test.go @@ -202,44 +202,6 @@ func TestSafeFormatAndMount(t *testing.T) { }, expErrorType: GetDiskFormatFailed, }, - { - description: "Test that 'xfs_repair' is called only once, no need to repair the filesystem", - fstype: "xfs", - execScripts: []ExecArgs{ - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, - {"xfs_repair", []string{"-n", "/dev/foo"}, "", nil}, - }, - }, - { - description: "Test that 'xfs_repair' is called twice and repair the filesystem", - fstype: "xfs", - execScripts: []ExecArgs{ - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, - {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, - {"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil}, - }, - }, - { - description: "Test that 'xfs_repair' is called twice and repair the filesystem, but mount failed", - fstype: "xfs", - mountErrs: []error{fmt.Errorf("unknown filesystem type '(null)'")}, - execScripts: []ExecArgs{ - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, - {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, - {"xfs_repair", []string{"/dev/foo"}, "\ndone\n", nil}, - }, - expErrorType: UnknownMountError, - }, - { - description: "Test that 'xfs_repair' is called twice but could not repair the filesystem", - fstype: "xfs", - execScripts: []ExecArgs{ - {"blkid", []string{"-p", "-s", "TYPE", "-s", "PTTYPE", "-o", "export", "/dev/foo"}, "DEVNAME=/dev/foo\nTYPE=xfs\n", nil}, - {"xfs_repair", []string{"-n", "/dev/foo"}, "", &testingexec.FakeExitError{Status: 1}}, - {"xfs_repair", []string{"/dev/foo"}, "\nAn error occurred\n", &testingexec.FakeExitError{Status: 1}}, - }, - expErrorType: HasFilesystemErrors, - }, { description: "Test that 'blkid' is called and confirms unformatted disk, format fails with sensitive options", fstype: "ext4", From e2ef310046165befe3fc024f698833d227d2e4aa Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Fri, 10 Apr 2020 07:15:16 -0400 Subject: [PATCH 17/27] fix bad spelling Signed-off-by: Davanum Srinivas --- mount.go | 8 ++++---- mount_linux.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/mount.go b/mount.go index 112d7ebc178..77ec9c6d329 100644 --- a/mount.go +++ b/mount.go @@ -346,14 +346,14 @@ func StartsWithBackstep(rel string) bool { return rel == ".." || strings.HasPrefix(filepath.ToSlash(rel), "../") } -// sanitizedOptionsForLogging will return a comma seperated string containing +// sanitizedOptionsForLogging will return a comma separated string containing // options and sensitiveOptions. Each entry in sensitiveOptions will be // replaced with the string sensitiveOptionsRemoved // e.g. o1,o2,, func sanitizedOptionsForLogging(options []string, sensitiveOptions []string) string { - seperator := "" + separator := "" if len(options) > 0 && len(sensitiveOptions) > 0 { - seperator = "," + separator = "," } sensitiveOptionsStart := "" @@ -364,7 +364,7 @@ func sanitizedOptionsForLogging(options []string, sensitiveOptions []string) str } return strings.Join(options, ",") + - seperator + + separator + sensitiveOptionsStart + sensitiveOptionsEnd } diff --git a/mount_linux.go b/mount_linux.go index 41f69efe3f0..4de29ee322d 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -103,7 +103,7 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri } // doMount runs the mount command. mounterPath is the path to mounter binary if containerized mounter is used. -// sensitiveOptions is an extention of options except they will not be logged (because they may contain sensitive material) +// sensitiveOptions is an extension of options except they will not be logged (because they may contain sensitive material) func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source string, target string, fstype string, options []string, sensitiveOptions []string) error { mountArgs, mountArgsLogStr := MakeMountArgsSensitive(source, target, fstype, options, sensitiveOptions) if len(mounterPath) > 0 { @@ -187,7 +187,7 @@ func MakeMountArgs(source, target, fstype string, options []string) (mountArgs [ } // MakeMountArgsSensitive makes the arguments to the mount(8) command. -// sensitiveOptions is an extention of options except they will not be logged (because they may contain sensitive material) +// sensitiveOptions is an extension of options except they will not be logged (because they may contain sensitive material) func MakeMountArgsSensitive(source, target, fstype string, options []string, sensitiveOptions []string) (mountArgs []string, mountArgsLogStr string) { // Build mount command as follows: // mount [-t $fstype] [-o $options] [$source] $target From 93189a19bc4a0765f6cc983263e6ace760b64e06 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Fri, 10 Apr 2020 07:19:17 -0400 Subject: [PATCH 18/27] ignore golint for some stutter that we want to keep as-is Signed-off-by: Davanum Srinivas --- mount.go | 6 +++--- mount_helper_unix.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/mount.go b/mount.go index 77ec9c6d329..14997a75c9b 100644 --- a/mount.go +++ b/mount.go @@ -77,7 +77,7 @@ type Interface interface { var _ Interface = &Mounter{} // MountPoint represents a single line in /proc/mounts or /etc/fstab. -type MountPoint struct { +type MountPoint struct { // nolint: golint Device string Path string Type string @@ -86,7 +86,7 @@ type MountPoint struct { Pass int } -type MountErrorType string +type MountErrorType string // nolint: golint const ( FilesystemMismatch MountErrorType = "FilesystemMismatch" @@ -97,7 +97,7 @@ const ( UnknownMountError MountErrorType = "UnknownMountError" ) -type MountError struct { +type MountError struct { // nolint: golint Type MountErrorType Message string } diff --git a/mount_helper_unix.go b/mount_helper_unix.go index 1e14d8c96ca..11b70ebc298 100644 --- a/mount_helper_unix.go +++ b/mount_helper_unix.go @@ -56,7 +56,7 @@ func IsCorruptedMnt(err error) bool { } // MountInfo represents a single line in /proc//mountinfo. -type MountInfo struct { +type MountInfo struct { // nolint: golint // Unique ID for the mount (maybe reused after umount). ID int // The ID of the parent mount (or of self for the root of this mount namespace's mount tree). From f5bb1d9860fe35165db776ff58cf3dcb48972811 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Wed, 8 Apr 2020 15:01:33 -0400 Subject: [PATCH 19/27] Switch to klog v2 Signed-off-by: Davanum Srinivas --- fake_mounter.go | 2 +- mount_helper_common.go | 2 +- mount_helper_windows.go | 2 +- mount_linux.go | 2 +- mount_windows.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/fake_mounter.go b/fake_mounter.go index 8e690bbfc28..f48c2badba6 100644 --- a/fake_mounter.go +++ b/fake_mounter.go @@ -21,7 +21,7 @@ import ( "path/filepath" "sync" - "k8s.io/klog" + "k8s.io/klog/v2" ) // FakeMounter implements mount.Interface for tests. diff --git a/mount_helper_common.go b/mount_helper_common.go index 36bb32b463d..1d40549b5f4 100644 --- a/mount_helper_common.go +++ b/mount_helper_common.go @@ -20,7 +20,7 @@ import ( "fmt" "os" - "k8s.io/klog" + "k8s.io/klog/v2" ) // CleanupMountPoint unmounts the given path and deletes the remaining directory diff --git a/mount_helper_windows.go b/mount_helper_windows.go index 516f970d282..b308ce76d2f 100644 --- a/mount_helper_windows.go +++ b/mount_helper_windows.go @@ -25,7 +25,7 @@ import ( "strings" "syscall" - "k8s.io/klog" + "k8s.io/klog/v2" ) // following failure codes are from https://docs.microsoft.com/en-us/windows/desktop/debug/system-error-codes--1300-1699- diff --git a/mount_linux.go b/mount_linux.go index 4de29ee322d..b7a443fdf6c 100644 --- a/mount_linux.go +++ b/mount_linux.go @@ -27,7 +27,7 @@ import ( "strings" "syscall" - "k8s.io/klog" + "k8s.io/klog/v2" utilexec "k8s.io/utils/exec" utilio "k8s.io/utils/io" ) diff --git a/mount_windows.go b/mount_windows.go index a99fdb9ce5b..85d699696c9 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -25,7 +25,7 @@ import ( "path/filepath" "strings" - "k8s.io/klog" + "k8s.io/klog/v2" utilexec "k8s.io/utils/exec" "k8s.io/utils/keymutex" utilpath "k8s.io/utils/path" From eec07e05bac1478af0e4561941957a7e5cae8c08 Mon Sep 17 00:00:00 2001 From: Jing Xu Date: Mon, 18 May 2020 11:29:16 -0700 Subject: [PATCH 20/27] Remove driver letter assignment during volume format This PR removes the driver letter assignment during volume format and mount because driver letter might run out and cause issues during mount. Intead, it uses volume id to mount the target dir. --- mount_windows.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/mount_windows.go b/mount_windows.go index 85d699696c9..cf419b34942 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -26,7 +26,6 @@ import ( "strings" "k8s.io/klog/v2" - utilexec "k8s.io/utils/exec" "k8s.io/utils/keymutex" utilpath "k8s.io/utils/path" ) @@ -230,17 +229,17 @@ func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target // format disk if it is unformatted(raw) cmd := fmt.Sprintf("Get-Disk -Number %s | Where partitionstyle -eq 'raw' | Initialize-Disk -PartitionStyle MBR -PassThru"+ - " | New-Partition -AssignDriveLetter -UseMaximumSize | Format-Volume -FileSystem %s -Confirm:$false", source, fstype) + " | New-Partition -UseMaximumSize | Format-Volume -FileSystem %s -Confirm:$false", source, fstype) if output, err := mounter.Exec.Command("powershell", "/c", cmd).CombinedOutput(); err != nil { return fmt.Errorf("diskMount: format disk failed, error: %v, output: %q", err, string(output)) } klog.V(4).Infof("diskMount: Disk successfully formatted, disk: %q, fstype: %q", source, fstype) - driveLetter, err := getDriveLetterByDiskNumber(source, mounter.Exec) + volumeIds, err := listVolumesOnDisk(source) if err != nil { return err } - driverPath := driveLetter + ":" + driverPath := volumeIds[0] target = NormalizeWindowsPath(target) klog.V(4).Infof("Attempting to formatAndMount disk: %s %s %s", fstype, driverPath, target) if output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, driverPath).CombinedOutput(); err != nil { @@ -250,17 +249,17 @@ func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target return nil } -// Get drive letter according to windows disk number -func getDriveLetterByDiskNumber(diskNum string, exec utilexec.Interface) (string, error) { - cmd := fmt.Sprintf("(Get-Partition -DiskNumber %s).DriveLetter", diskNum) +// ListVolumesOnDisk - returns back list of volumes(volumeIDs) in the disk (requested in diskID). +func listVolumesOnDisk(diskID string) (volumeIDs []string, err error) { + cmd := fmt.Sprintf("(Get-Disk -DeviceId %s | Get-Partition | Get-Volume).UniqueId", diskID) output, err := exec.Command("powershell", "/c", cmd).CombinedOutput() + klog.V(4).Infof("listVolumesOnDisk id from %s: %s", diskID, string(output)) if err != nil { - return "", fmt.Errorf("azureMount: Get Drive Letter failed: %v, output: %q", err, string(output)) + return []string{}, fmt.Errorf("error list volumes on disk. cmd: %s, output: %s, error: %v", cmd, string(output), err) } - if len(string(output)) < 1 { - return "", fmt.Errorf("azureMount: Get Drive Letter failed, output is empty") - } - return string(output)[:1], nil + + volumeIds := strings.Split(strings.TrimSpace(string(output)), "\r\n") + return volumeIds, nil } // getAllParentLinks walks all symbolic links and return all the parent targets recursively From 91a87aa690abf9744099d522845a39012b6c1022 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Tue, 2 Jun 2020 13:02:26 +0000 Subject: [PATCH 21/27] fix: remove unnecessary readlink check in IsLikelyNotMountPoint on Windows fix build failure fix comments fix build failure fix comments --- mount_windows.go | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/mount_windows.go b/mount_windows.go index cf419b34942..ae3c52267de 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -27,7 +27,6 @@ import ( "k8s.io/klog/v2" "k8s.io/utils/keymutex" - utilpath "k8s.io/utils/path" ) // Mounter provides the default implementation of mount.Interface @@ -182,19 +181,10 @@ func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) { if err != nil { return true, err } - // If current file is a symlink, then it is a mountpoint. - if stat.Mode()&os.ModeSymlink != 0 { - target, err := os.Readlink(file) - if err != nil { - return true, fmt.Errorf("readlink error: %v", err) - } - exists, err := utilpath.Exists(utilpath.CheckFollowSymlink, target) - if err != nil { - return true, err - } - return !exists, nil - } + if stat.Mode()&os.ModeSymlink != 0 { + return false, err + } return true, nil } From 029074ef670ecaa57ee0dbed1b70bdc3b591eb64 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Wed, 3 Jun 2020 03:06:16 +0000 Subject: [PATCH 22/27] chore: add more logging for mklink on Windows use volumeID var --- mount_windows.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/mount_windows.go b/mount_windows.go index ae3c52267de..62c967d65e2 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -113,10 +113,12 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri } } - if output, err := exec.Command("cmd", "/c", "mklink", "/D", target, bindSource).CombinedOutput(); err != nil { + output, err := exec.Command("cmd", "/c", "mklink", "/D", target, bindSource).CombinedOutput() + if err != nil { klog.Errorf("mklink failed: %v, source(%q) target(%q) output: %q", err, bindSource, target, string(output)) return err } + klog.V(2).Infof("mklink source(%q) on target(%q) successfully, output: %q", bindSource, target, string(output)) return nil } @@ -229,13 +231,14 @@ func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target if err != nil { return err } - driverPath := volumeIds[0] + volumeID := volumeIds[0] target = NormalizeWindowsPath(target) - klog.V(4).Infof("Attempting to formatAndMount disk: %s %s %s", fstype, driverPath, target) - if output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, driverPath).CombinedOutput(); err != nil { - klog.Errorf("mklink failed: %v, output: %q", err, string(output)) + output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, volumeID).CombinedOutput() + if err != nil { + klog.Errorf("mklink(%s, %s) failed: %v, output: %q", target, volumeID, err, string(output)) return err } + klog.V(2).Infof("formatAndMount disk(%s) fstype(%s) on(%s) with output(%s) successfully", volumeID, fstype, target, string(output)) return nil } From a9ea36e54ee66ead3367d9c48884233329b787e1 Mon Sep 17 00:00:00 2001 From: Jing Xu Date: Thu, 18 Jun 2020 09:52:24 -0700 Subject: [PATCH 23/27] Revert "Merge pull request #166 from jingxu97/May/drivename" This reverts commit 278ece378a5054f16a07622b51ddaf82b328d6e6, reversing changes made to 2df71ebbae66f39338aed4cd0bb82d2212ee33cc. --- mount_windows.go | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/mount_windows.go b/mount_windows.go index 62c967d65e2..f8b5ca328a3 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -26,6 +26,7 @@ import ( "strings" "k8s.io/klog/v2" + utilexec "k8s.io/utils/exec" "k8s.io/utils/keymutex" ) @@ -221,38 +222,38 @@ func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target // format disk if it is unformatted(raw) cmd := fmt.Sprintf("Get-Disk -Number %s | Where partitionstyle -eq 'raw' | Initialize-Disk -PartitionStyle MBR -PassThru"+ - " | New-Partition -UseMaximumSize | Format-Volume -FileSystem %s -Confirm:$false", source, fstype) + " | New-Partition -AssignDriveLetter -UseMaximumSize | Format-Volume -FileSystem %s -Confirm:$false", source, fstype) if output, err := mounter.Exec.Command("powershell", "/c", cmd).CombinedOutput(); err != nil { return fmt.Errorf("diskMount: format disk failed, error: %v, output: %q", err, string(output)) } klog.V(4).Infof("diskMount: Disk successfully formatted, disk: %q, fstype: %q", source, fstype) - volumeIds, err := listVolumesOnDisk(source) + driveLetter, err := getDriveLetterByDiskNumber(source, mounter.Exec) if err != nil { return err } - volumeID := volumeIds[0] + driverPath := driveLetter + ":" target = NormalizeWindowsPath(target) - output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, volumeID).CombinedOutput() + output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, driverPath).CombinedOutput() if err != nil { - klog.Errorf("mklink(%s, %s) failed: %v, output: %q", target, volumeID, err, string(output)) + klog.Errorf("mklink(%s, %s) failed: %v, output: %q", target, driverPath, err, string(output)) return err } - klog.V(2).Infof("formatAndMount disk(%s) fstype(%s) on(%s) with output(%s) successfully", volumeID, fstype, target, string(output)) + klog.V(2).Infof("formatAndMount disk(%s) fstype(%s) on(%s) with output(%s) successfully", driverPath, fstype, target, string(output)) return nil } -// ListVolumesOnDisk - returns back list of volumes(volumeIDs) in the disk (requested in diskID). -func listVolumesOnDisk(diskID string) (volumeIDs []string, err error) { - cmd := fmt.Sprintf("(Get-Disk -DeviceId %s | Get-Partition | Get-Volume).UniqueId", diskID) +// Get drive letter according to windows disk number +func getDriveLetterByDiskNumber(diskNum string, exec utilexec.Interface) (string, error) { + cmd := fmt.Sprintf("(Get-Partition -DiskNumber %s).DriveLetter", diskNum) output, err := exec.Command("powershell", "/c", cmd).CombinedOutput() - klog.V(4).Infof("listVolumesOnDisk id from %s: %s", diskID, string(output)) if err != nil { - return []string{}, fmt.Errorf("error list volumes on disk. cmd: %s, output: %s, error: %v", cmd, string(output), err) + return "", fmt.Errorf("azureMount: Get Drive Letter failed: %v, output: %q", err, string(output)) } - - volumeIds := strings.Split(strings.TrimSpace(string(output)), "\r\n") - return volumeIds, nil + if len(string(output)) < 1 { + return "", fmt.Errorf("azureMount: Get Drive Letter failed, output is empty") + } + return string(output)[:1], nil } // getAllParentLinks walks all symbolic links and return all the parent targets recursively From 37cdee17ee1afb94e73eaec485b8cec7051c58ed Mon Sep 17 00:00:00 2001 From: Jing Xu Date: Mon, 18 May 2020 11:29:16 -0700 Subject: [PATCH 24/27] Remove driver letter assignment during volume format This PR removes the driver letter assignment during volume format and mount because driver letter might run out and cause issues during mount. Intead, it uses volume id to mount the target dir. --- mount_windows.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/mount_windows.go b/mount_windows.go index f8b5ca328a3..0534649c062 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -26,7 +26,6 @@ import ( "strings" "k8s.io/klog/v2" - utilexec "k8s.io/utils/exec" "k8s.io/utils/keymutex" ) @@ -222,17 +221,17 @@ func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target // format disk if it is unformatted(raw) cmd := fmt.Sprintf("Get-Disk -Number %s | Where partitionstyle -eq 'raw' | Initialize-Disk -PartitionStyle MBR -PassThru"+ - " | New-Partition -AssignDriveLetter -UseMaximumSize | Format-Volume -FileSystem %s -Confirm:$false", source, fstype) + " | New-Partition -UseMaximumSize | Format-Volume -FileSystem %s -Confirm:$false", source, fstype) if output, err := mounter.Exec.Command("powershell", "/c", cmd).CombinedOutput(); err != nil { return fmt.Errorf("diskMount: format disk failed, error: %v, output: %q", err, string(output)) } klog.V(4).Infof("diskMount: Disk successfully formatted, disk: %q, fstype: %q", source, fstype) - driveLetter, err := getDriveLetterByDiskNumber(source, mounter.Exec) + volumeIds, err := listVolumesOnDisk(source) if err != nil { return err } - driverPath := driveLetter + ":" + driverPath := volumeIds[0] target = NormalizeWindowsPath(target) output, err := mounter.Exec.Command("cmd", "/c", "mklink", "/D", target, driverPath).CombinedOutput() if err != nil { @@ -243,17 +242,17 @@ func (mounter *SafeFormatAndMount) formatAndMountSensitive(source string, target return nil } -// Get drive letter according to windows disk number -func getDriveLetterByDiskNumber(diskNum string, exec utilexec.Interface) (string, error) { - cmd := fmt.Sprintf("(Get-Partition -DiskNumber %s).DriveLetter", diskNum) +// ListVolumesOnDisk - returns back list of volumes(volumeIDs) in the disk (requested in diskID). +func listVolumesOnDisk(diskID string) (volumeIDs []string, err error) { + cmd := fmt.Sprintf("(Get-Disk -DeviceId %s | Get-Partition | Get-Volume).UniqueId", diskID) output, err := exec.Command("powershell", "/c", cmd).CombinedOutput() + klog.V(4).Infof("listVolumesOnDisk id from %s: %s", diskID, string(output)) if err != nil { - return "", fmt.Errorf("azureMount: Get Drive Letter failed: %v, output: %q", err, string(output)) + return []string{}, fmt.Errorf("error list volumes on disk. cmd: %s, output: %s, error: %v", cmd, string(output), err) } - if len(string(output)) < 1 { - return "", fmt.Errorf("azureMount: Get Drive Letter failed, output is empty") - } - return string(output)[:1], nil + + volumeIds := strings.Split(strings.TrimSpace(string(output)), "\r\n") + return volumeIds, nil } // getAllParentLinks walks all symbolic links and return all the parent targets recursively From 89fac93b43da347f7770fcba810235e357fbe75b Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sun, 9 Aug 2020 14:11:07 +0000 Subject: [PATCH 25/27] fix: smb remount issue typo typo add one logging --- mount_windows.go | 42 ++++++++++++++++++++++++++++++++++-------- mount_windows_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/mount_windows.go b/mount_windows.go index 0534649c062..b2793d76231 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -98,19 +98,32 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri getSMBMountMutex.LockKey(source) defer getSMBMountMutex.UnlockKey(source) - if output, err := newSMBMapping(allOptions[0], allOptions[1], source); err != nil { + var output string + var err error + username := allOptions[0] + password := allOptions[1] + if output, err = newSMBMapping(username, password, source); err != nil { + klog.Warningf("SMB Mapping(%s) returned with error(%v), output(%s)", source, err, string(output)) if isSMBMappingExist(source) { - klog.V(2).Infof("SMB Mapping(%s) already exists, now begin to remove and remount", source) - if output, err := removeSMBMapping(source); err != nil { - return fmt.Errorf("Remove-SmbGlobalMapping failed: %v, output: %q", err, output) + valid, errPath := isValidPath(source) + if errPath != nil { + return errPath } - if output, err := newSMBMapping(allOptions[0], allOptions[1], source); err != nil { - return fmt.Errorf("New-SmbGlobalMapping remount failed: %v, output: %q", err, output) + if valid { + err = nil + klog.V(2).Infof("SMB Mapping(%s) already exists and is still valid, skip error", source) + } else { + klog.V(2).Infof("SMB Mapping(%s) already exists while it's not valid, now begin to remove and remount", source) + if output, err = removeSMBMapping(source); err != nil { + return fmt.Errorf("Remove-SmbGlobalMapping failed: %v, output: %q", err, output) + } + output, err = newSMBMapping(username, password, source) } - } else { - return fmt.Errorf("New-SmbGlobalMapping failed: %v, output: %q", err, output) } } + if err != nil { + return fmt.Errorf("New-SmbGlobalMapping(%s) failed: %v, output: %q", source, err, output) + } } output, err := exec.Command("cmd", "/c", "mklink", "/D", target, bindSource).CombinedOutput() @@ -153,6 +166,19 @@ func isSMBMappingExist(remotepath string) bool { return err == nil } +// check whether remotepath is valid +// return (true, nil) if remotepath is valid +func isValidPath(remotepath string) (bool, error) { + cmd := exec.Command("powershell", "/c", `Test-Path $Env:remoteapth`) + cmd.Env = append(os.Environ(), fmt.Sprintf("remoteapth=%s", remotepath)) + output, err := cmd.CombinedOutput() + if err != nil { + return false, fmt.Errorf("returned output: %s, error: %v", string(output), err) + } + + return strings.HasPrefix(strings.ToLower(string(output)), "true"), nil +} + // remove SMB mapping func removeSMBMapping(remotepath string) (string, error) { cmd := exec.Command("powershell", "/c", `Remove-SmbGlobalMapping -RemotePath $Env:smbremotepath -Force`) diff --git a/mount_windows_test.go b/mount_windows_test.go index 288cc633518..584a5e7e591 100644 --- a/mount_windows_test.go +++ b/mount_windows_test.go @@ -331,3 +331,33 @@ func TestNewSMBMapping(t *testing.T) { } } } + +func TestIsValidPath(t *testing.T) { + tests := []struct { + remotepath string + expectedResult bool + expectError bool + }{ + { + "c:", + true, + false, + }, + { + "invalid-path", + false, + false, + }, + } + + for _, test := range tests { + result, err := isValidPath(test.remotepath) + assert.Equal(t, result, test.expectedResult, "Expect result not equal with isValidPath(%s) return: %q, expected: %q, error: %v", + test.remotepath, result, test.expectedResult, err) + if test.expectError { + assert.NotNil(t, err, "Expect error during isValidPath(%s)", test.remotepath) + } else { + assert.Nil(t, err, "Expect error is nil during isValidPath(%s)", test.remotepath) + } + } +} From 6f59f9db45effbd113efd9ba736b460b0d2ed418 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sat, 15 Aug 2020 14:16:42 +0000 Subject: [PATCH 26/27] fix: return error with fewer mount options on Windows --- mount_windows.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mount_windows.go b/mount_windows.go index b2793d76231..02df7099172 100644 --- a/mount_windows.go +++ b/mount_windows.go @@ -84,9 +84,8 @@ func (mounter *Mounter) MountSensitive(source string, target string, fstype stri allOptions = append(allOptions, options...) allOptions = append(allOptions, sensitiveOptions...) if len(allOptions) < 2 { - klog.Warningf("mount options(%q) command number(%d) less than 2, source:%q, target:%q, skip mounting", + return fmt.Errorf("mount options(%q) should have at least 2 options, current number:%d, source:%q, target:%q", sanitizedOptionsForLogging, len(allOptions), source, target) - return nil } // currently only cifs mount is supported From 5db0ae548fb5e7f2f5ce54351b6926f2ea6758d6 Mon Sep 17 00:00:00 2001 From: Srini Brahmaroutu Date: Tue, 1 Sep 2020 22:42:43 -0700 Subject: [PATCH 27/27] moving files from k8s.io/util/mount into staging/src/k8s.io/mount-utils --- go.mod | 1 + hack/.golint_failures | 1 + staging/publishing/rules.yaml | 8 + staging/repos_generated.bzl | 1 + staging/src/BUILD | 1 + .../.github/PULL_REQUEST_TEMPLATE.md | 2 + staging/src/k8s.io/mount-utils/BUILD | 107 ++++++++++ staging/src/k8s.io/mount-utils/LICENSE | 201 ++++++++++++++++++ staging/src/k8s.io/mount-utils/OWNERS | 14 ++ staging/src/k8s.io/mount-utils/README.md | 30 +++ .../src/k8s.io/mount-utils/SECURITY_CONTACTS | 18 ++ .../src/k8s.io/mount-utils/code-of-conduct.md | 3 + .../src/k8s.io/mount-utils/doc.go | 2 +- .../src/k8s.io/mount-utils/fake_mounter.go | 0 staging/src/k8s.io/mount-utils/go.mod | 16 ++ staging/src/k8s.io/mount-utils/go.sum | 33 +++ .../src/k8s.io/mount-utils/mount.go | 4 - .../k8s.io/mount-utils/mount_helper_common.go | 0 .../k8s.io/mount-utils/mount_helper_test.go | 0 .../k8s.io/mount-utils/mount_helper_unix.go | 0 .../mount-utils/mount_helper_unix_test.go | 0 .../mount-utils/mount_helper_windows.go | 0 .../mount-utils/mount_helper_windows_test.go | 0 .../src/k8s.io/mount-utils/mount_linux.go | 2 +- .../k8s.io/mount-utils/mount_linux_test.go | 0 .../src/k8s.io/mount-utils/mount_test.go | 0 .../k8s.io/mount-utils/mount_unsupported.go | 0 .../src/k8s.io/mount-utils/mount_windows.go | 0 .../k8s.io/mount-utils/mount_windows_test.go | 0 .../mount-utils/safe_format_and_mount_test.go | 0 vendor/k8s.io/mount-utils | 1 + vendor/modules.txt | 1 + 32 files changed, 440 insertions(+), 6 deletions(-) create mode 100644 staging/src/k8s.io/mount-utils/.github/PULL_REQUEST_TEMPLATE.md create mode 100644 staging/src/k8s.io/mount-utils/BUILD create mode 100644 staging/src/k8s.io/mount-utils/LICENSE create mode 100644 staging/src/k8s.io/mount-utils/OWNERS create mode 100644 staging/src/k8s.io/mount-utils/README.md create mode 100644 staging/src/k8s.io/mount-utils/SECURITY_CONTACTS create mode 100644 staging/src/k8s.io/mount-utils/code-of-conduct.md rename doc.go => staging/src/k8s.io/mount-utils/doc.go (93%) rename fake_mounter.go => staging/src/k8s.io/mount-utils/fake_mounter.go (100%) create mode 100644 staging/src/k8s.io/mount-utils/go.mod create mode 100644 staging/src/k8s.io/mount-utils/go.sum rename mount.go => staging/src/k8s.io/mount-utils/mount.go (99%) rename mount_helper_common.go => staging/src/k8s.io/mount-utils/mount_helper_common.go (100%) rename mount_helper_test.go => staging/src/k8s.io/mount-utils/mount_helper_test.go (100%) rename mount_helper_unix.go => staging/src/k8s.io/mount-utils/mount_helper_unix.go (100%) rename mount_helper_unix_test.go => staging/src/k8s.io/mount-utils/mount_helper_unix_test.go (100%) rename mount_helper_windows.go => staging/src/k8s.io/mount-utils/mount_helper_windows.go (100%) rename mount_helper_windows_test.go => staging/src/k8s.io/mount-utils/mount_helper_windows_test.go (100%) rename mount_linux.go => staging/src/k8s.io/mount-utils/mount_linux.go (99%) rename mount_linux_test.go => staging/src/k8s.io/mount-utils/mount_linux_test.go (100%) rename mount_test.go => staging/src/k8s.io/mount-utils/mount_test.go (100%) rename mount_unsupported.go => staging/src/k8s.io/mount-utils/mount_unsupported.go (100%) rename mount_windows.go => staging/src/k8s.io/mount-utils/mount_windows.go (100%) rename mount_windows_test.go => staging/src/k8s.io/mount-utils/mount_windows_test.go (100%) rename safe_format_and_mount_test.go => staging/src/k8s.io/mount-utils/safe_format_and_mount_test.go (100%) create mode 120000 vendor/k8s.io/mount-utils diff --git a/go.mod b/go.mod index d62c0676c31..b93122e0fa4 100644 --- a/go.mod +++ b/go.mod @@ -484,6 +484,7 @@ replace ( k8s.io/kubelet => ./staging/src/k8s.io/kubelet k8s.io/legacy-cloud-providers => ./staging/src/k8s.io/legacy-cloud-providers k8s.io/metrics => ./staging/src/k8s.io/metrics + k8s.io/mount-utils => ./staging/src/k8s.io/mount-utils k8s.io/sample-apiserver => ./staging/src/k8s.io/sample-apiserver k8s.io/sample-cli-plugin => ./staging/src/k8s.io/sample-cli-plugin k8s.io/sample-controller => ./staging/src/k8s.io/sample-controller diff --git a/hack/.golint_failures b/hack/.golint_failures index 9fba7988f66..13b08705294 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -487,6 +487,7 @@ staging/src/k8s.io/metrics/pkg/client/custom_metrics/fake staging/src/k8s.io/metrics/pkg/client/custom_metrics/scheme staging/src/k8s.io/metrics/pkg/client/external_metrics staging/src/k8s.io/metrics/pkg/client/external_metrics/fake +staging/src/k8s.io/mount-utils staging/src/k8s.io/sample-apiserver/pkg/admission/wardleinitializer staging/src/k8s.io/sample-apiserver/pkg/apis/wardle/v1alpha1 staging/src/k8s.io/sample-apiserver/pkg/registry/wardle/fischer diff --git a/staging/publishing/rules.yaml b/staging/publishing/rules.yaml index b61fa32c075..d07711ae7e0 100644 --- a/staging/publishing/rules.yaml +++ b/staging/publishing/rules.yaml @@ -1645,3 +1645,11 @@ rules: dir: staging/src/k8s.io/controller-manager name: release-1.19 go: 1.15 + +- destination: mount-utils + library: true + branches: + - source: + branch: master + dir: staging/src/k8s.io/mount-utils + name: master diff --git a/staging/repos_generated.bzl b/staging/repos_generated.bzl index 8adfb85f514..212032e6c4e 100644 --- a/staging/repos_generated.bzl +++ b/staging/repos_generated.bzl @@ -37,6 +37,7 @@ staging_repos = [ "k8s.io/kubelet", "k8s.io/legacy-cloud-providers", "k8s.io/metrics", + "k8s.io/mount-utils", "k8s.io/sample-apiserver", "k8s.io/sample-cli-plugin", "k8s.io/sample-controller", diff --git a/staging/src/BUILD b/staging/src/BUILD index 680dedeb077..b8b0597c81e 100644 --- a/staging/src/BUILD +++ b/staging/src/BUILD @@ -36,6 +36,7 @@ filegroup( "//staging/src/k8s.io/kubelet:all-srcs", "//staging/src/k8s.io/legacy-cloud-providers:all-srcs", "//staging/src/k8s.io/metrics:all-srcs", + "//staging/src/k8s.io/mount-utils:all-srcs", "//staging/src/k8s.io/sample-apiserver:all-srcs", "//staging/src/k8s.io/sample-cli-plugin:all-srcs", "//staging/src/k8s.io/sample-controller:all-srcs", diff --git a/staging/src/k8s.io/mount-utils/.github/PULL_REQUEST_TEMPLATE.md b/staging/src/k8s.io/mount-utils/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 00000000000..e7e5eb834b2 --- /dev/null +++ b/staging/src/k8s.io/mount-utils/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,2 @@ +Sorry, we do not accept changes directly against this repository. Please see +CONTRIBUTING.md for information on where and how to contribute instead. diff --git a/staging/src/k8s.io/mount-utils/BUILD b/staging/src/k8s.io/mount-utils/BUILD new file mode 100644 index 00000000000..d57b710fe82 --- /dev/null +++ b/staging/src/k8s.io/mount-utils/BUILD @@ -0,0 +1,107 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = [ + "doc.go", + "fake_mounter.go", + "mount.go", + "mount_helper_common.go", + "mount_helper_unix.go", + "mount_helper_windows.go", + "mount_linux.go", + "mount_unsupported.go", + "mount_windows.go", + ], + importmap = "k8s.io/kubernetes/vendor/k8s.io/mount-utils", + importpath = "k8s.io/mount-utils", + visibility = ["//visibility:public"], + deps = [ + "//vendor/k8s.io/klog/v2:go_default_library", + "//vendor/k8s.io/utils/exec:go_default_library", + ] + select({ + "@io_bazel_rules_go//go/platform:aix": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:android": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:darwin": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:dragonfly": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:freebsd": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:illumos": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:ios": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:js": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:linux": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:nacl": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:netbsd": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:openbsd": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:plan9": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:solaris": [ + "//vendor/k8s.io/utils/io:go_default_library", + ], + "@io_bazel_rules_go//go/platform:windows": [ + "//vendor/k8s.io/utils/keymutex:go_default_library", + ], + "//conditions:default": [], + }), +) + +go_test( + name = "go_default_test", + srcs = [ + "mount_helper_test.go", + "mount_helper_unix_test.go", + "mount_helper_windows_test.go", + "mount_linux_test.go", + "mount_test.go", + "mount_windows_test.go", + "safe_format_and_mount_test.go", + ], + embed = [":go_default_library"], + deps = [ + "//vendor/k8s.io/utils/exec:go_default_library", + "//vendor/k8s.io/utils/exec/testing:go_default_library", + ] + select({ + "@io_bazel_rules_go//go/platform:windows": [ + "//vendor/github.com/stretchr/testify/assert:go_default_library", + ], + "//conditions:default": [], + }), +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/mount-utils/LICENSE b/staging/src/k8s.io/mount-utils/LICENSE new file mode 100644 index 00000000000..8dada3edaf5 --- /dev/null +++ b/staging/src/k8s.io/mount-utils/LICENSE @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "{}" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright {yyyy} {name of copyright owner} + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/staging/src/k8s.io/mount-utils/OWNERS b/staging/src/k8s.io/mount-utils/OWNERS new file mode 100644 index 00000000000..5fa5cc24c37 --- /dev/null +++ b/staging/src/k8s.io/mount-utils/OWNERS @@ -0,0 +1,14 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +reviewers: + - jingxu97 + - saad-ali + - jsafrane + - msau42 + - andyzhangx + - gnufied +approvers: + - jingxu97 + - saad-ali + - jsafrane + diff --git a/staging/src/k8s.io/mount-utils/README.md b/staging/src/k8s.io/mount-utils/README.md new file mode 100644 index 00000000000..5510e99bc4f --- /dev/null +++ b/staging/src/k8s.io/mount-utils/README.md @@ -0,0 +1,30 @@ +## Purpose + +This repository defines an interface to mounting filesystems to be consumed by +various Kubernetes and out-of-tree CSI components. + +Consumers of this repository can make use of functions like 'Mount' to mount +source to target as fstype with given options, 'Unmount' to unmount a target. +Other useful functions include 'List' all mounted file systems and find all +mount references to a path using 'GetMountRefs' + +## Community, discussion, contribution, and support + +Learn how to engage with the Kubernetes community on the [community +page](http://kubernetes.io/community/). + +You can reach the maintainers of this repository at: + +- Slack: #sig-storage (on https://kubernetes.slack.com -- get an + invite at slack.kubernetes.io) +- Mailing List: + https://groups.google.com/forum/#!forum/kubernetes-sig-storage + +### Code of Conduct + +Participation in the Kubernetes community is governed by the [Kubernetes +Code of Conduct](code-of-conduct.md). + +### Contibution Guidelines + +See [CONTRIBUTING.md](CONTRIBUTING.md) for more information. diff --git a/staging/src/k8s.io/mount-utils/SECURITY_CONTACTS b/staging/src/k8s.io/mount-utils/SECURITY_CONTACTS new file mode 100644 index 00000000000..14fe23e186d --- /dev/null +++ b/staging/src/k8s.io/mount-utils/SECURITY_CONTACTS @@ -0,0 +1,18 @@ +# Defined below are the security contacts for this repo. +# +# They are the contact point for the Product Security Committee to reach out +# to for triaging and handling of incoming issues. +# +# The below names agree to abide by the +# [Embargo Policy](https://git.k8s.io/security/private-distributors-list.md#embargo-policy) +# and will be removed and replaced if they violate that agreement. +# +# DO NOT REPORT SECURITY VULNERABILITIES DIRECTLY TO THESE NAMES, FOLLOW THE +# INSTRUCTIONS AT https://kubernetes.io/security/ + +saad-ali +cjcullen +joelsmith +liggitt +philips +tallclair diff --git a/staging/src/k8s.io/mount-utils/code-of-conduct.md b/staging/src/k8s.io/mount-utils/code-of-conduct.md new file mode 100644 index 00000000000..0d15c00cf32 --- /dev/null +++ b/staging/src/k8s.io/mount-utils/code-of-conduct.md @@ -0,0 +1,3 @@ +# Kubernetes Community Code of Conduct + +Please refer to our [Kubernetes Community Code of Conduct](https://git.k8s.io/community/code-of-conduct.md) diff --git a/doc.go b/staging/src/k8s.io/mount-utils/doc.go similarity index 93% rename from doc.go rename to staging/src/k8s.io/mount-utils/doc.go index c81b426ce8c..b7cac03a52e 100644 --- a/doc.go +++ b/staging/src/k8s.io/mount-utils/doc.go @@ -15,4 +15,4 @@ limitations under the License. */ // Package mount defines an interface to mounting filesystems. -package mount // import "k8s.io/utils/mount" +package mount // import "k8s.io/mount-utils" diff --git a/fake_mounter.go b/staging/src/k8s.io/mount-utils/fake_mounter.go similarity index 100% rename from fake_mounter.go rename to staging/src/k8s.io/mount-utils/fake_mounter.go diff --git a/staging/src/k8s.io/mount-utils/go.mod b/staging/src/k8s.io/mount-utils/go.mod new file mode 100644 index 00000000000..5bc95b701db --- /dev/null +++ b/staging/src/k8s.io/mount-utils/go.mod @@ -0,0 +1,16 @@ +// This is a generated file. Do not edit directly. + +module k8s.io/mount-utils + +go 1.15 + +require ( + github.com/kr/pretty v0.2.0 // indirect + github.com/stretchr/testify v1.4.0 + gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect + gopkg.in/yaml.v2 v2.2.8 // indirect + k8s.io/klog/v2 v2.2.0 + k8s.io/utils v0.0.0-20200729134348-d5654de09c73 +) + +replace k8s.io/mount-utils => ../mount-utils diff --git a/staging/src/k8s.io/mount-utils/go.sum b/staging/src/k8s.io/mount-utils/go.sum new file mode 100644 index 00000000000..5c7f2cfb15f --- /dev/null +++ b/staging/src/k8s.io/mount-utils/go.sum @@ -0,0 +1,33 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/go-logr/logr v0.1.0 h1:M1Tv3VzNlEHg6uyACnRdtrploV2P7wZqH8BoQMtz0cg= +github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas= +github.com/go-logr/logr v0.2.0 h1:QvGt2nLcHH0WK9orKa+ppBPAxREcH364nPUedEpK0TY= +github.com/go-logr/logr v0.2.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU= +github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs= +github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= +github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/spf13/afero v1.2.2 h1:5jhuqJyZCZf2JRofRvN/nIFgIWNzPa3/Vz8mYylgbWc= +github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= +github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= +gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= +gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +k8s.io/klog/v2 v2.0.0 h1:Foj74zO6RbjjP4hBEKjnYtjjAhGg4jNynUdYF6fJrok= +k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= +k8s.io/klog/v2 v2.2.0 h1:XRvcwJozkgZ1UQJmfMGpvRthQHOvihEhYtDfAaxMz/A= +k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= +k8s.io/utils v0.0.0-20200729134348-d5654de09c73 h1:uJmqzgNWG7XyClnU/mLPBWwfKKF1K8Hf8whTseBgJcg= +k8s.io/utils v0.0.0-20200729134348-d5654de09c73/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA= diff --git a/mount.go b/staging/src/k8s.io/mount-utils/mount.go similarity index 99% rename from mount.go rename to staging/src/k8s.io/mount-utils/mount.go index 14997a75c9b..2847da46bcc 100644 --- a/mount.go +++ b/staging/src/k8s.io/mount-utils/mount.go @@ -290,9 +290,7 @@ func MakeBindOptsSensitive(options []string, sensitiveOptions []string) (bool, [ switch option { case "bind": bind = true - break case "remount": - break default: bindRemountOpts = append(bindRemountOpts, option) } @@ -302,9 +300,7 @@ func MakeBindOptsSensitive(options []string, sensitiveOptions []string) (bool, [ switch sensitiveOption { case "bind": bind = true - break case "remount": - break default: bindRemountSensitiveOpts = append(bindRemountSensitiveOpts, sensitiveOption) } diff --git a/mount_helper_common.go b/staging/src/k8s.io/mount-utils/mount_helper_common.go similarity index 100% rename from mount_helper_common.go rename to staging/src/k8s.io/mount-utils/mount_helper_common.go diff --git a/mount_helper_test.go b/staging/src/k8s.io/mount-utils/mount_helper_test.go similarity index 100% rename from mount_helper_test.go rename to staging/src/k8s.io/mount-utils/mount_helper_test.go diff --git a/mount_helper_unix.go b/staging/src/k8s.io/mount-utils/mount_helper_unix.go similarity index 100% rename from mount_helper_unix.go rename to staging/src/k8s.io/mount-utils/mount_helper_unix.go diff --git a/mount_helper_unix_test.go b/staging/src/k8s.io/mount-utils/mount_helper_unix_test.go similarity index 100% rename from mount_helper_unix_test.go rename to staging/src/k8s.io/mount-utils/mount_helper_unix_test.go diff --git a/mount_helper_windows.go b/staging/src/k8s.io/mount-utils/mount_helper_windows.go similarity index 100% rename from mount_helper_windows.go rename to staging/src/k8s.io/mount-utils/mount_helper_windows.go diff --git a/mount_helper_windows_test.go b/staging/src/k8s.io/mount-utils/mount_helper_windows_test.go similarity index 100% rename from mount_helper_windows_test.go rename to staging/src/k8s.io/mount-utils/mount_helper_windows_test.go diff --git a/mount_linux.go b/staging/src/k8s.io/mount-utils/mount_linux.go similarity index 99% rename from mount_linux.go rename to staging/src/k8s.io/mount-utils/mount_linux.go index b7a443fdf6c..10f046e86ba 100644 --- a/mount_linux.go +++ b/staging/src/k8s.io/mount-utils/mount_linux.go @@ -136,7 +136,7 @@ func (mounter *Mounter) doMount(mounterPath string, mountCmd string, source stri // systemd-mount is not used because it's too new for older distros // (CentOS 7, Debian Jessie). mountCmd, mountArgs, mountArgsLogStr = AddSystemdScopeSensitive("systemd-run", target, mountCmd, mountArgs, mountArgsLogStr) - } else { + // } else { // No systemd-run on the host (or we failed to check it), assume kubelet // does not run as a systemd service. // No code here, mountCmd and mountArgs are already populated. diff --git a/mount_linux_test.go b/staging/src/k8s.io/mount-utils/mount_linux_test.go similarity index 100% rename from mount_linux_test.go rename to staging/src/k8s.io/mount-utils/mount_linux_test.go diff --git a/mount_test.go b/staging/src/k8s.io/mount-utils/mount_test.go similarity index 100% rename from mount_test.go rename to staging/src/k8s.io/mount-utils/mount_test.go diff --git a/mount_unsupported.go b/staging/src/k8s.io/mount-utils/mount_unsupported.go similarity index 100% rename from mount_unsupported.go rename to staging/src/k8s.io/mount-utils/mount_unsupported.go diff --git a/mount_windows.go b/staging/src/k8s.io/mount-utils/mount_windows.go similarity index 100% rename from mount_windows.go rename to staging/src/k8s.io/mount-utils/mount_windows.go diff --git a/mount_windows_test.go b/staging/src/k8s.io/mount-utils/mount_windows_test.go similarity index 100% rename from mount_windows_test.go rename to staging/src/k8s.io/mount-utils/mount_windows_test.go diff --git a/safe_format_and_mount_test.go b/staging/src/k8s.io/mount-utils/safe_format_and_mount_test.go similarity index 100% rename from safe_format_and_mount_test.go rename to staging/src/k8s.io/mount-utils/safe_format_and_mount_test.go diff --git a/vendor/k8s.io/mount-utils b/vendor/k8s.io/mount-utils new file mode 120000 index 00000000000..a5a8937b13a --- /dev/null +++ b/vendor/k8s.io/mount-utils @@ -0,0 +1 @@ +../../staging/src/k8s.io/mount-utils \ No newline at end of file diff --git a/vendor/modules.txt b/vendor/modules.txt index 1182c1c8a08..f33338bbc77 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -2414,6 +2414,7 @@ k8s.io/metrics/pkg/client/custom_metrics/fake k8s.io/metrics/pkg/client/custom_metrics/scheme k8s.io/metrics/pkg/client/external_metrics k8s.io/metrics/pkg/client/external_metrics/fake +# k8s.io/mount-utils => ./staging/src/k8s.io/mount-utils # k8s.io/sample-apiserver v0.0.0 => ./staging/src/k8s.io/sample-apiserver ## explicit # k8s.io/sample-apiserver => ./staging/src/k8s.io/sample-apiserver