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.

Sign up for GitHub

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

Jump to bottom

Fix mountOptions in iSCSI and FC volume plugins #89172

Merged
merged 2 commits into from
Mar 26, 2020
Merged

Fix mountOptions in iSCSI and FC volume plugins #89172

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f60a0c8
Fix mount options in iSCSI volumes
jsafrane Mar 17, 2020
58129fd
Fix mount options in FibreChannel volumes
jsafrane Mar 17, 2020
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
6 changes: 1 addition & 5 deletions pkg/volume/fc/fc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"strings"
"testing"

"k8s.io/utils/exec/testing"
testingexec "k8s.io/utils/exec/testing"
"k8s.io/utils/mount"

v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -122,10 +122,6 @@ func (fake *fakeDiskManager) AttachDisk(b fcDiskMounter) (string, error) {
if err != nil {
return "", err
}
// Simulate the global mount so that the fakeMounter returns the
// expected number of mounts for the attached disk.
b.mounter.Mount(globalPath, globalPath, b.fsType, nil)

fake.attachCalled = true
return "", nil
}
Expand Down
30 changes: 1 addition & 29 deletions pkg/volume/fc/fc_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"k8s.io/klog"
"k8s.io/utils/mount"

v1 "k8s.io/api/core/v1"
"k8s.io/kubernetes/pkg/volume"
volumeutil "k8s.io/kubernetes/pkg/volume/util"
)
Expand Down Expand Up @@ -242,34 +241,7 @@ func (util *fcUtil) AttachDisk(b fcDiskMounter) (string, error) {
return "", err
}

// If the volumeMode is 'Block', plugin don't have to format the volume.
// The globalPDPath will be created by operationexecutor. Just return devicePath here.
klog.V(5).Infof("fc: AttachDisk volumeMode: %s, devicePath: %s", b.volumeMode, devicePath)
if b.volumeMode == v1.PersistentVolumeBlock {
return devicePath, nil
}

// mount it
globalPDPath := util.MakeGlobalPDName(*b.fcDisk)
if err := os.MkdirAll(globalPDPath, 0750); err != nil {
return devicePath, fmt.Errorf("fc: failed to mkdir %s, error", globalPDPath)
}

noMnt, err := b.mounter.IsLikelyNotMountPoint(globalPDPath)
if err != nil {
return devicePath, fmt.Errorf("Heuristic determination of mount point failed:%v", err)
}
if !noMnt {
klog.Infof("fc: %s already mounted", globalPDPath)
return devicePath, nil
}

err = b.mounter.FormatAndMount(devicePath, globalPDPath, b.fsType, b.mountOptions)
if err != nil {
return devicePath, fmt.Errorf("fc: failed to mount fc volume %s [%s] to %s, error %v", devicePath, b.fsType, globalPDPath, err)
}

return devicePath, err
return devicePath, nil
}
jsafrane marked this conversation as resolved.
Show resolved Hide resolved

// DetachDisk removes scsi device file such as /dev/sdX from the node.
Expand Down
1 change: 1 addition & 0 deletions pkg/volume/iscsi/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ go_library(
"//pkg/kubelet/config:go_default_library",
"//pkg/volume:go_default_library",
"//pkg/volume/util:go_default_library",
"//pkg/volume/util/types:go_default_library",
"//pkg/volume/util/volumepathhandler:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
Expand Down
72 changes: 23 additions & 49 deletions pkg/volume/iscsi/iscsi_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"k8s.io/kubernetes/pkg/kubelet/config"
"k8s.io/kubernetes/pkg/volume"
volumeutil "k8s.io/kubernetes/pkg/volume/util"
"k8s.io/kubernetes/pkg/volume/util/types"
)

const (
Expand Down Expand Up @@ -194,7 +195,9 @@ func (util *ISCSIUtil) MakeGlobalVDPDName(iscsi iscsiDisk) string {
return makeVDPDNameInternal(iscsi.plugin.host, iscsi.Portals[0], iscsi.Iqn, iscsi.Lun, iscsi.Iface)
}

func (util *ISCSIUtil) persistISCSI(conf iscsiDisk, mnt string) error {
// persistISCSIFile saves iSCSI volume configuration for DetachDisk
// into given directory.
func (util *ISCSIUtil) persistISCSIFile(conf iscsiDisk, mnt string) error {
file := filepath.Join(mnt, "iscsi.json")
fp, err := os.Create(file)
if err != nil {
Expand Down Expand Up @@ -457,61 +460,32 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) (string, error) {
}

klog.V(5).Infof("iscsi: AttachDisk devicePath: %s", devicePath)
// run global mount path related operations based on volumeMode
return globalPDPathOperation(b)(b, devicePath, util)

if err = util.persistISCSI(b); err != nil {
// Return uncertain error so kubelet calls Unmount / Unmap when the pod
// is deleted.
return "", types.NewUncertainProgressError(err.Error())
}
return devicePath, nil
}

// globalPDPathOperation returns global mount path related operations based on volumeMode.
// If the volumeMode is 'Filesystem' or not defined, plugin needs to create a dir, persist
// iscsi configurations, and then format/mount the volume.
// If the volumeMode is 'Block', plugin creates a dir and persists iscsi configurations.
// Since volume type is block, plugin doesn't need to format/mount the volume.
func globalPDPathOperation(b iscsiDiskMounter) func(iscsiDiskMounter, string, *ISCSIUtil) (string, error) {
// persistISCSI saves iSCSI volume configuration for DetachDisk into global
// mount / map directory.
func (util *ISCSIUtil) persistISCSI(b iscsiDiskMounter) error {
klog.V(5).Infof("iscsi: AttachDisk volumeMode: %s", b.volumeMode)
var globalPDPath string
if b.volumeMode == v1.PersistentVolumeBlock {
// If the volumeMode is 'Block', plugin don't need to format the volume.
return func(b iscsiDiskMounter, devicePath string, util *ISCSIUtil) (string, error) {
globalPDPath := b.manager.MakeGlobalVDPDName(*b.iscsiDisk)
// Create dir like /var/lib/kubelet/plugins/kubernetes.io/iscsi/volumeDevices/{ifaceName}/{portal-some_iqn-lun-lun_id}
if err := os.MkdirAll(globalPDPath, 0750); err != nil {
klog.Errorf("iscsi: failed to mkdir %s, error", globalPDPath)
return "", err
}
// Persist iscsi disk config to json file for DetachDisk path
util.persistISCSI(*(b.iscsiDisk), globalPDPath)

return devicePath, nil
}
globalPDPath = b.manager.MakeGlobalVDPDName(*b.iscsiDisk)
} else {
globalPDPath = b.manager.MakeGlobalPDName(*b.iscsiDisk)
}

// If the volumeMode is 'Filesystem', plugin needs to format the volume
// and mount it to globalPDPath.
return func(b iscsiDiskMounter, devicePath string, util *ISCSIUtil) (string, error) {
globalPDPath := b.manager.MakeGlobalPDName(*b.iscsiDisk)
notMnt, err := b.mounter.IsLikelyNotMountPoint(globalPDPath)
if err != nil && !os.IsNotExist(err) {
return "", fmt.Errorf("Heuristic determination of mount point failed:%v", err)
}
// Return confirmed devicePath to caller
if !notMnt {
klog.Infof("iscsi: %s already mounted", globalPDPath)
return devicePath, nil
}
// Create dir like /var/lib/kubelet/plugins/kubernetes.io/iscsi/{ifaceName}/{portal-some_iqn-lun-lun_id}
if err := os.MkdirAll(globalPDPath, 0750); err != nil {
klog.Errorf("iscsi: failed to mkdir %s, error", globalPDPath)
return "", err
}
// Persist iscsi disk config to json file for DetachDisk path
util.persistISCSI(*(b.iscsiDisk), globalPDPath)

err = b.mounter.FormatAndMount(devicePath, globalPDPath, b.fsType, nil)
if err != nil {
klog.Errorf("iscsi: failed to mount iscsi volume %s [%s] to %s, error %v", devicePath, b.fsType, globalPDPath, err)
}

return devicePath, nil
if err := os.MkdirAll(globalPDPath, 0750); err != nil {
klog.Errorf("iscsi: failed to mkdir %s, error", globalPDPath)
return err
}
// Persist iscsi disk config to json file for DetachDisk path
return util.persistISCSIFile(*(b.iscsiDisk), globalPDPath)
}

// Delete 1 block device of the form "sd*"
Expand Down