Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move MountsInGlobalPDPath from mount pkg to volume #74734

Merged
merged 2 commits into from
May 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/util/mount/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ func (f *FakeMounter) PathIsDevice(pathname string) (bool, error) {
return true, nil
}

func (f *FakeMounter) GetDeviceNameFromMount(mountPath, pluginDir string) (string, error) {
return getDeviceNameFromMount(f, mountPath, pluginDir)
func (f *FakeMounter) GetDeviceNameFromMount(mountPath, pluginMountDir string) (string, error) {
return getDeviceNameFromMount(f, mountPath, pluginMountDir)
}

func (f *FakeMounter) MakeRShared(path string) error {
Expand Down
17 changes: 8 additions & 9 deletions pkg/util/mount/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@ type FileType string

const (
// Default mount command if mounter path is not specified
defaultMountCommand = "mount"
MountsInGlobalPDPath = "mounts"
FileTypeDirectory FileType = "Directory"
FileTypeFile FileType = "File"
FileTypeSocket FileType = "Socket"
FileTypeCharDev FileType = "CharDevice"
FileTypeBlockDev FileType = "BlockDevice"
defaultMountCommand = "mount"
FileTypeDirectory FileType = "Directory"
FileTypeFile FileType = "File"
FileTypeSocket FileType = "Socket"
FileTypeCharDev FileType = "CharDevice"
FileTypeBlockDev FileType = "BlockDevice"
)

type Interface interface {
Expand All @@ -62,8 +61,8 @@ type Interface interface {
// PathIsDevice determines if a path is a device.
PathIsDevice(pathname string) (bool, error)
// GetDeviceNameFromMount finds the device name by checking the mount path
// to get the global mount path which matches its plugin directory
GetDeviceNameFromMount(mountPath, pluginDir string) (string, error)
// to get the global mount path within its plugin directory
GetDeviceNameFromMount(mountPath, pluginMountDir string) (string, error)
// MakeRShared checks that given path is on a mount with 'rshared' mount
// propagation. If not, it bind-mounts the path as rshared.
MakeRShared(path string) error
Expand Down
17 changes: 8 additions & 9 deletions pkg/util/mount/mount_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,19 +304,19 @@ func ExclusiveOpenFailsOnDevice(pathname string) (bool, error) {
}

//GetDeviceNameFromMount: given a mount point, find the device name from its global mount point
func (mounter *Mounter) GetDeviceNameFromMount(mountPath, pluginDir string) (string, error) {
return GetDeviceNameFromMountLinux(mounter, mountPath, pluginDir)
func (mounter *Mounter) GetDeviceNameFromMount(mountPath, pluginMountDir string) (string, error) {
return GetDeviceNameFromMountLinux(mounter, mountPath, pluginMountDir)
}

func getDeviceNameFromMount(mounter Interface, mountPath, pluginDir string) (string, error) {
return GetDeviceNameFromMountLinux(mounter, mountPath, pluginDir)
func getDeviceNameFromMount(mounter Interface, mountPath, pluginMountDir string) (string, error) {
return GetDeviceNameFromMountLinux(mounter, mountPath, pluginMountDir)
}

// GetDeviceNameFromMountLinux find the device name from /proc/mounts in which
// the mount path reference should match the given plugin directory. In case no mount path reference
// the mount path reference should match the given plugin mount directory. In case no mount path reference
// matches, returns the volume name taken from its given mountPath
// This implementation is shared with NsEnterMounter
func GetDeviceNameFromMountLinux(mounter Interface, mountPath, pluginDir string) (string, error) {
func GetDeviceNameFromMountLinux(mounter Interface, mountPath, pluginMountDir string) (string, error) {
refs, err := mounter.GetMountRefs(mountPath)
if err != nil {
klog.V(4).Infof("GetMountRefs failed for mount path %q: %v", mountPath, err)
Expand All @@ -326,10 +326,9 @@ func GetDeviceNameFromMountLinux(mounter Interface, mountPath, pluginDir string)
klog.V(4).Infof("Directory %s is not mounted", mountPath)
return "", fmt.Errorf("directory %s is not mounted", mountPath)
}
basemountPath := path.Join(pluginDir, MountsInGlobalPDPath)
for _, ref := range refs {
if strings.HasPrefix(ref, basemountPath) {
volumeID, err := filepath.Rel(basemountPath, ref)
if strings.HasPrefix(ref, pluginMountDir) {
volumeID, err := filepath.Rel(pluginMountDir, ref)
if err != nil {
klog.Errorf("Failed to get volume id from mount %s - %v", mountPath, err)
return "", err
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/mount/mount_unsupported.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) {
return true, unsupportedErr
}

func (mounter *Mounter) GetDeviceNameFromMount(mountPath, pluginDir string) (string, error) {
func (mounter *Mounter) GetDeviceNameFromMount(mountPath, pluginMountDir string) (string, error) {
return "", unsupportedErr
}

func getDeviceNameFromMount(mounter Interface, mountPath, pluginDir string) (string, error) {
func getDeviceNameFromMount(mounter Interface, mountPath, pluginMountDir string) (string, error) {
return "", unsupportedErr
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/util/mount/mount_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,14 @@ func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) {
}

// GetDeviceNameFromMount given a mnt point, find the device
func (mounter *Mounter) GetDeviceNameFromMount(mountPath, pluginDir string) (string, error) {
return getDeviceNameFromMount(mounter, mountPath, pluginDir)
func (mounter *Mounter) GetDeviceNameFromMount(mountPath, pluginMountDir string) (string, error) {
return getDeviceNameFromMount(mounter, mountPath, pluginMountDir)
}

// getDeviceNameFromMount find the device(drive) name in which
// the mount path reference should match the given plugin directory. In case no mount path reference
// the mount path reference should match the given plugin mount directory. In case no mount path reference
// matches, returns the volume name taken from its given mountPath
func getDeviceNameFromMount(mounter Interface, mountPath, pluginDir string) (string, error) {
func getDeviceNameFromMount(mounter Interface, mountPath, pluginMountDir string) (string, error) {
refs, err := mounter.GetMountRefs(mountPath)
if err != nil {
klog.V(4).Infof("GetMountRefs failed for mount path %q: %v", mountPath, err)
Expand All @@ -212,7 +212,7 @@ func getDeviceNameFromMount(mounter Interface, mountPath, pluginDir string) (str
if len(refs) == 0 {
return "", fmt.Errorf("directory %s is not mounted", mountPath)
}
basemountPath := normalizeWindowsPath(path.Join(pluginDir, MountsInGlobalPDPath))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to make sure pluginMountDir now has MountsInGlobalPDPath included?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, callers within K8s have to append MountsInGlobalPDPath before making this call now. Picture this code living at k8s.io/utils/mount instead of within k8s.io/kubernetes. It can't/won't know anything about MountsInGlobalPDPath.

I made sure to update all callers.

basemountPath := normalizeWindowsPath(pluginMountDir)
for _, ref := range refs {
if strings.Contains(ref, basemountPath) {
volumeID, err := filepath.Rel(normalizeWindowsPath(basemountPath), ref)
Expand Down
6 changes: 3 additions & 3 deletions pkg/volume/awsebs/aws_ebs.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ func getVolumeSource(

func (plugin *awsElasticBlockStorePlugin) ConstructVolumeSpec(volName, mountPath string) (*volume.Spec, error) {
mounter := plugin.host.GetMounter(plugin.GetPluginName())
pluginDir := plugin.host.GetPluginDir(plugin.GetPluginName())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the old plugin.host.GetPluginDir func removed now? If not, I would suggest keep the original code, make this PR as small as possible, easy for review.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may move the path.Join to filepath.Join code to a separate PR. I don't know what happened last night, but my late-night rebase squashed my two commits into one (and I know I didn't do that on purpose).

With that stuff taken out, it's pretty small.

To your specific question, no GetPluginDir is not removed. In fact is called from within the new makeGlobalPDPath().

volumeID, err := mounter.GetDeviceNameFromMount(mountPath, pluginDir)
pluginMntDir := util.GetPluginMountDir(plugin.host, plugin.GetPluginName())
volumeID, err := mounter.GetDeviceNameFromMount(mountPath, pluginMntDir)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -451,7 +451,7 @@ func makeGlobalPDPath(host volume.VolumeHost, volumeID aws.KubernetesVolumeID) s
// Clean up the URI to be more fs-friendly
name := string(volumeID)
name = strings.Replace(name, "://", "/", -1)
return filepath.Join(host.GetPluginDir(awsElasticBlockStorePluginName), mount.MountsInGlobalPDPath, name)
return filepath.Join(host.GetPluginDir(awsElasticBlockStorePluginName), util.MountsInGlobalPDPath, name)
}

func (ebs *awsElasticBlockStore) GetPath() string {
Expand Down
2 changes: 1 addition & 1 deletion pkg/volume/azure_dd/attacher.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (a *azureDiskAttacher) GetDeviceMountPath(spec *volume.Spec) (string, error
}

if volumeSource.Kind == nil { // this spec was constructed from info on the node
pdPath := filepath.Join(a.plugin.host.GetPluginDir(azureDataDiskPluginName), mount.MountsInGlobalPDPath, volumeSource.DataDiskURI)
pdPath := filepath.Join(a.plugin.host.GetPluginDir(azureDataDiskPluginName), util.MountsInGlobalPDPath, volumeSource.DataDiskURI)
return pdPath, nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/azure_dd/azure_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/sets"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/util/mount"
"k8s.io/kubernetes/pkg/volume"
"k8s.io/kubernetes/pkg/volume/util"
"k8s.io/legacy-cloud-providers/azure"
utilstrings "k8s.io/utils/strings"
)
Expand Down Expand Up @@ -80,7 +80,7 @@ func makeGlobalPDPath(host volume.VolumeHost, diskUri string, isManaged bool) (s
}
// "{m for managed b for blob}{hashed diskUri or DiskId depending on disk kind }"
diskName := fmt.Sprintf(uniqueDiskNameTemplate, prefix, hashedDiskUri)
pdPath := filepath.Join(host.GetPluginDir(azureDataDiskPluginName), mount.MountsInGlobalPDPath, diskName)
pdPath := filepath.Join(host.GetPluginDir(azureDataDiskPluginName), util.MountsInGlobalPDPath, diskName)
codenrhoden marked this conversation as resolved.
Show resolved Hide resolved

return pdPath, nil
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/azure_dd/azure_dd.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,8 @@ var _ volume.NodeExpandableVolumePlugin = &azureDataDiskPlugin{}

func (plugin *azureDataDiskPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
mounter := plugin.host.GetMounter(plugin.GetPluginName())
pluginDir := plugin.host.GetPluginDir(plugin.GetPluginName())
sourceName, err := mounter.GetDeviceNameFromMount(mountPath, pluginDir)
pluginMntDir := util.GetPluginMountDir(plugin.host, plugin.GetPluginName())
sourceName, err := mounter.GetDeviceNameFromMount(mountPath, pluginMntDir)

if err != nil {
return nil, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/cephfs/cephfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"fmt"
"os"
"os/exec"
"path"
"path/filepath"
"runtime"
"strings"

Expand Down Expand Up @@ -384,7 +384,7 @@ func (cephfsVolume *cephfs) execFuseMount(mountpoint string) error {
return err
}

keyringFile = path.Join(keyringPath, fileName)
keyringFile = filepath.Join(keyringPath, fileName)

} else {
keyringFile = cephfsVolume.secretFile
Expand Down
4 changes: 2 additions & 2 deletions pkg/volume/cephfs/cephfs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package cephfs

import (
"os"
"path"
"path/filepath"
"testing"

"k8s.io/api/core/v1"
Expand Down Expand Up @@ -83,7 +83,7 @@ func TestPlugin(t *testing.T) {
t.Errorf("Got a nil Mounter")
}
volumePath := mounter.GetPath()
volpath := path.Join(tmpDir, "pods/poduid/volumes/kubernetes.io~cephfs/vol1")
volpath := filepath.Join(tmpDir, "pods/poduid/volumes/kubernetes.io~cephfs/vol1")
if volumePath != volpath {
t.Errorf("Got unexpected path: %s", volumePath)
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/volume/cinder/cinder.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"os"
"path"
"path/filepath"

"k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -266,8 +267,8 @@ func (plugin *cinderPlugin) getCloudProvider() (BlockStorageProvider, error) {

func (plugin *cinderPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*volume.Spec, error) {
mounter := plugin.host.GetMounter(plugin.GetPluginName())
pluginDir := plugin.host.GetPluginDir(plugin.GetPluginName())
sourceName, err := mounter.GetDeviceNameFromMount(mountPath, pluginDir)
pluginMntDir := util.GetPluginMountDir(plugin.host, plugin.GetPluginName())
sourceName, err := mounter.GetDeviceNameFromMount(mountPath, pluginMntDir)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -449,7 +450,7 @@ func (b *cinderVolumeMounter) SetUpAt(dir string, fsGroup *int64) error {
}

func makeGlobalPDName(host volume.VolumeHost, devName string) string {
return path.Join(host.GetPluginDir(cinderVolumePluginName), mount.MountsInGlobalPDPath, devName)
return filepath.Join(host.GetPluginDir(cinderVolumePluginName), util.MountsInGlobalPDPath, devName)
}

func (cd *cinderVolume) GetPath() string {
Expand Down
8 changes: 4 additions & 4 deletions pkg/volume/cinder/cinder_block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package cinder

import (
"os"
"path"
"path/filepath"
"testing"

"k8s.io/api/core/v1"
Expand Down Expand Up @@ -47,7 +47,7 @@ func TestGetVolumeSpecFromGlobalMapPath(t *testing.T) {
//deferred clean up
defer os.RemoveAll(tmpVDir)

expectedGlobalPath := path.Join(tmpVDir, testGlobalPath)
expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath)

//Bad Path
badspec, err := getVolumeSpecFromGlobalMapPath("")
Expand Down Expand Up @@ -102,8 +102,8 @@ func TestGetPodAndPluginMapPaths(t *testing.T) {
//deferred clean up
defer os.RemoveAll(tmpVDir)

expectedGlobalPath := path.Join(tmpVDir, testGlobalPath)
expectedPodPath := path.Join(tmpVDir, testPodPath)
expectedGlobalPath := filepath.Join(tmpVDir, testGlobalPath)
expectedPodPath := filepath.Join(tmpVDir, testPodPath)

spec := getTestVolume(false, true /*isBlock*/)
plugMgr := volume.VolumePluginMgr{}
Expand Down
6 changes: 3 additions & 3 deletions pkg/volume/cinder/cinder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ package cinder
import (
"fmt"
"os"
"path"
"path/filepath"
"testing"
"time"

Expand Down Expand Up @@ -64,7 +64,7 @@ type fakePDManager struct {
}

func getFakeDeviceName(host volume.VolumeHost, pdName string) string {
return path.Join(host.GetPluginDir(cinderVolumePluginName), "device", pdName)
return filepath.Join(host.GetPluginDir(cinderVolumePluginName), "device", pdName)
}

// Real Cinder AttachDisk attaches a cinder volume. If it is not yet mounted,
Expand Down Expand Up @@ -160,7 +160,7 @@ func TestPlugin(t *testing.T) {
if mounter == nil {
t.Errorf("Got a nil Mounter")
}
volPath := path.Join(tmpDir, "pods/poduid/volumes/kubernetes.io~cinder/vol1")
volPath := filepath.Join(tmpDir, "pods/poduid/volumes/kubernetes.io~cinder/vol1")
path := mounter.GetPath()
if path != volPath {
t.Errorf("Got unexpected path: %s", path)
Expand Down
8 changes: 4 additions & 4 deletions pkg/volume/configmap/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -498,14 +498,14 @@ func TestPluginOptional(t *testing.T) {
}
}

datadirSymlink := path.Join(volumePath, "..data")
datadirSymlink := filepath.Join(volumePath, "..data")
datadir, err := os.Readlink(datadirSymlink)
if err != nil && os.IsNotExist(err) {
t.Fatalf("couldn't find volume path's data dir, %s", datadirSymlink)
} else if err != nil {
t.Fatalf("couldn't read symlink, %s", datadirSymlink)
}
datadirPath := path.Join(volumePath, datadir)
datadirPath := filepath.Join(volumePath, datadir)

infos, err := ioutil.ReadDir(volumePath)
if err != nil {
Expand Down Expand Up @@ -689,7 +689,7 @@ func configMap(namespace, name string) v1.ConfigMap {

func doTestConfigMapDataInVolume(volumePath string, configMap v1.ConfigMap, t *testing.T) {
for key, value := range configMap.Data {
configMapDataHostPath := path.Join(volumePath, key)
configMapDataHostPath := filepath.Join(volumePath, key)
if _, err := os.Stat(configMapDataHostPath); err != nil {
t.Fatalf("SetUp() failed, couldn't find configMap data on disk: %v", configMapDataHostPath)
} else {
Expand Down
3 changes: 1 addition & 2 deletions pkg/volume/csi/csi_attacher.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"errors"
"fmt"
"os"
"path"
"path/filepath"
"strings"
"time"
Expand Down Expand Up @@ -598,7 +597,7 @@ func makeDeviceMountPath(plugin *csiPlugin, spec *volume.Spec) (string, error) {
return "", fmt.Errorf("makeDeviceMountPath failed, pv name empty")
}

return path.Join(plugin.host.GetPluginDir(plugin.GetPluginName()), persistentVolumeInGlobalPath, pvName, globalMountInGlobalPath), nil
return filepath.Join(plugin.host.GetPluginDir(plugin.GetPluginName()), persistentVolumeInGlobalPath, pvName, globalMountInGlobalPath), nil
}

func getDriverAndVolNameFromDeviceMountPath(k8s kubernetes.Interface, deviceMountPath string) (string, string, error) {
Expand Down
5 changes: 2 additions & 3 deletions pkg/volume/csi/csi_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"errors"
"fmt"
"os"
"path"
"path/filepath"

"k8s.io/klog"
Expand Down Expand Up @@ -63,14 +62,14 @@ func (m *csiBlockMapper) GetGlobalMapPath(spec *volume.Spec) (string, error) {
// Example: plugins/kubernetes.io/csi/volumeDevices/staging/{pvname}
func (m *csiBlockMapper) getStagingPath() string {
sanitizedSpecVolID := utilstrings.EscapeQualifiedName(m.specName)
return path.Join(m.plugin.host.GetVolumeDevicePluginDir(CSIPluginName), "staging", sanitizedSpecVolID)
return filepath.Join(m.plugin.host.GetVolumeDevicePluginDir(CSIPluginName), "staging", sanitizedSpecVolID)
}

// getPublishPath returns a publish path for a file (on the node) that should be used on NodePublishVolume/NodeUnpublishVolume
// Example: plugins/kubernetes.io/csi/volumeDevices/publish/{pvname}
func (m *csiBlockMapper) getPublishPath() string {
sanitizedSpecVolID := utilstrings.EscapeQualifiedName(m.specName)
return path.Join(m.plugin.host.GetVolumeDevicePluginDir(CSIPluginName), "publish", sanitizedSpecVolID)
return filepath.Join(m.plugin.host.GetVolumeDevicePluginDir(CSIPluginName), "publish", sanitizedSpecVolID)
}

// GetPodDeviceMapPath returns pod's device file which will be mapped to a volume
Expand Down