From 8c8bf878fcea7a394078e758a216c5fedb9052f4 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Mon, 15 Jul 2019 18:28:12 -0700 Subject: [PATCH 1/2] Add passthrough for MountOptions for NodeStageVolume for CSI --- pkg/volume/csi/csi_attacher.go | 8 +++++++- pkg/volume/csi/csi_attacher_test.go | 12 ++++++++++++ pkg/volume/csi/csi_block.go | 3 ++- pkg/volume/csi/csi_client.go | 14 ++++++++++---- pkg/volume/csi/csi_client_test.go | 8 ++++++-- pkg/volume/csi/csi_util_test.go | 6 ++++++ pkg/volume/csi/fake/fake_client.go | 11 +++++++---- 7 files changed, 50 insertions(+), 12 deletions(-) diff --git a/pkg/volume/csi/csi_attacher.go b/pkg/volume/csi/csi_attacher.go index 9b1bd2a58f..e694294402 100644 --- a/pkg/volume/csi/csi_attacher.go +++ b/pkg/volume/csi/csi_attacher.go @@ -383,6 +383,11 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo accessMode = spec.PersistentVolume.Spec.AccessModes[0] } + var mountOptions []string + if spec.PersistentVolume != nil && spec.PersistentVolume.Spec.MountOptions != nil { + mountOptions = spec.PersistentVolume.Spec.MountOptions + } + fsType := csiSource.FSType err = csi.NodeStageVolume(ctx, csiSource.VolumeHandle, @@ -391,7 +396,8 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo fsType, accessMode, nodeStageSecrets, - csiSource.VolumeAttributes) + csiSource.VolumeAttributes, + mountOptions) if err != nil { return err diff --git a/pkg/volume/csi/csi_attacher_test.go b/pkg/volume/csi/csi_attacher_test.go index bfbf077905..0d70469973 100644 --- a/pkg/volume/csi/csi_attacher_test.go +++ b/pkg/volume/csi/csi_attacher_test.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path/filepath" + "reflect" "sync" "testing" "time" @@ -1032,6 +1033,14 @@ func TestAttacherMountDevice(t *testing.T) { stageUnstageSet: true, spec: volume.NewSpecFromPersistentVolume(makeTestPV(pvName, 10, testDriver, "test-vol1"), false), }, + { + testName: "normal PV with mount options", + volName: "test-vol1", + devicePath: "path1", + deviceMountPath: "path2", + stageUnstageSet: true, + spec: volume.NewSpecFromPersistentVolume(makeTestPVWithMountOptions(pvName, 10, testDriver, "test-vol1", []string{"test-op"}), false), + }, { testName: "no vol name", volName: "", @@ -1141,6 +1150,9 @@ func TestAttacherMountDevice(t *testing.T) { if vol.Path != tc.deviceMountPath { t.Errorf("expected mount path: %s. got: %s", tc.deviceMountPath, vol.Path) } + if !reflect.DeepEqual(vol.MountFlags, tc.spec.PersistentVolume.Spec.MountOptions) { + t.Errorf("expected mount options: %v, got: %v", tc.spec.PersistentVolume.Spec.MountOptions, vol.MountFlags) + } } } } diff --git a/pkg/volume/csi/csi_block.go b/pkg/volume/csi/csi_block.go index 7e04767995..19823d6a06 100644 --- a/pkg/volume/csi/csi_block.go +++ b/pkg/volume/csi/csi_block.go @@ -133,7 +133,8 @@ func (m *csiBlockMapper) stageVolumeForBlock( fsTypeBlockName, accessMode, nodeStageSecrets, - csiSource.VolumeAttributes) + csiSource.VolumeAttributes, + nil /* MountOptions */) if err != nil { klog.Error(log("blockMapper.stageVolumeForBlock failed: %v", err)) diff --git a/pkg/volume/csi/csi_client.go b/pkg/volume/csi/csi_client.go index 55904193e4..e4cd2a3146 100644 --- a/pkg/volume/csi/csi_client.go +++ b/pkg/volume/csi/csi_client.go @@ -72,6 +72,7 @@ type csiClient interface { accessMode api.PersistentVolumeAccessMode, secrets map[string]string, volumeContext map[string]string, + mountOptions []string, ) error NodeGetVolumeStats( @@ -515,6 +516,7 @@ func (c *csiDriverClient) NodeStageVolume(ctx context.Context, accessMode api.PersistentVolumeAccessMode, secrets map[string]string, volumeContext map[string]string, + mountOptions []string, ) error { klog.V(4).Info(log("calling NodeStageVolume rpc [volid=%s,staging_target_path=%s]", volID, stagingTargetPath)) if volID == "" { @@ -525,9 +527,9 @@ func (c *csiDriverClient) NodeStageVolume(ctx context.Context, } if c.nodeV1ClientCreator != nil { - return c.nodeStageVolumeV1(ctx, volID, publishContext, stagingTargetPath, fsType, accessMode, secrets, volumeContext) + return c.nodeStageVolumeV1(ctx, volID, publishContext, stagingTargetPath, fsType, accessMode, secrets, volumeContext, mountOptions) } else if c.nodeV0ClientCreator != nil { - return c.nodeStageVolumeV0(ctx, volID, publishContext, stagingTargetPath, fsType, accessMode, secrets, volumeContext) + return c.nodeStageVolumeV0(ctx, volID, publishContext, stagingTargetPath, fsType, accessMode, secrets, volumeContext, mountOptions) } return fmt.Errorf("failed to call NodeStageVolume. Both nodeV1ClientCreator and nodeV0ClientCreator are nil") @@ -542,6 +544,7 @@ func (c *csiDriverClient) nodeStageVolumeV1( accessMode api.PersistentVolumeAccessMode, secrets map[string]string, volumeContext map[string]string, + mountOptions []string, ) error { nodeClient, closer, err := c.nodeV1ClientCreator(c.addr) if err != nil { @@ -569,7 +572,8 @@ func (c *csiDriverClient) nodeStageVolumeV1( } else { req.VolumeCapability.AccessType = &csipbv1.VolumeCapability_Mount{ Mount: &csipbv1.VolumeCapability_MountVolume{ - FsType: fsType, + FsType: fsType, + MountFlags: mountOptions, }, } } @@ -587,6 +591,7 @@ func (c *csiDriverClient) nodeStageVolumeV0( accessMode api.PersistentVolumeAccessMode, secrets map[string]string, volumeContext map[string]string, + mountOptions []string, ) error { nodeClient, closer, err := c.nodeV0ClientCreator(c.addr) if err != nil { @@ -614,7 +619,8 @@ func (c *csiDriverClient) nodeStageVolumeV0( } else { req.VolumeCapability.AccessType = &csipbv0.VolumeCapability_Mount{ Mount: &csipbv0.VolumeCapability_MountVolume{ - FsType: fsType, + FsType: fsType, + MountFlags: mountOptions, }, } } diff --git a/pkg/volume/csi/csi_client_test.go b/pkg/volume/csi/csi_client_test.go index b53fa24e58..8bac6b71e1 100644 --- a/pkg/volume/csi/csi_client_test.go +++ b/pkg/volume/csi/csi_client_test.go @@ -175,6 +175,7 @@ func (c *fakeCsiDriverClient) NodeStageVolume(ctx context.Context, accessMode api.PersistentVolumeAccessMode, secrets map[string]string, volumeContext map[string]string, + mountOptions []string, ) error { c.t.Log("calling fake.NodeStageVolume...") req := &csipbv1.NodeStageVolumeRequest{ @@ -187,7 +188,8 @@ func (c *fakeCsiDriverClient) NodeStageVolume(ctx context.Context, }, AccessType: &csipbv1.VolumeCapability_Mount{ Mount: &csipbv1.VolumeCapability_MountVolume{ - FsType: fsType, + FsType: fsType, + MountFlags: mountOptions, }, }, }, @@ -448,10 +450,11 @@ func TestClientNodeStageVolume(t *testing.T) { stagingTargetPath string fsType string secrets map[string]string + mountOptions []string mustFail bool err error }{ - {name: "test ok", volID: "vol-test", stagingTargetPath: "/test/path", fsType: "ext4"}, + {name: "test ok", volID: "vol-test", stagingTargetPath: "/test/path", fsType: "ext4", mountOptions: []string{"unvalidated"}}, {name: "missing volID", stagingTargetPath: "/test/path", mustFail: true}, {name: "missing target path", volID: "vol-test", mustFail: true}, {name: "bad fs", volID: "vol-test", stagingTargetPath: "/test/path", fsType: "badfs", mustFail: true}, @@ -479,6 +482,7 @@ func TestClientNodeStageVolume(t *testing.T) { api.ReadWriteOnce, tc.secrets, map[string]string{"attr0": "val0"}, + tc.mountOptions, ) checkErr(t, tc.mustFail, err) diff --git a/pkg/volume/csi/csi_util_test.go b/pkg/volume/csi/csi_util_test.go index ea0a80a6dd..6f2919f008 100644 --- a/pkg/volume/csi/csi_util_test.go +++ b/pkg/volume/csi/csi_util_test.go @@ -42,6 +42,12 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } +func makeTestPVWithMountOptions(name string, sizeGig int, driverName, volID string, mountOptions []string) *api.PersistentVolume { + pv := makeTestPV(name, sizeGig, driverName, volID) + pv.Spec.MountOptions = mountOptions + return pv +} + func makeTestPV(name string, sizeGig int, driverName, volID string) *api.PersistentVolume { return &api.PersistentVolume{ ObjectMeta: meta.ObjectMeta{ diff --git a/pkg/volume/csi/fake/fake_client.go b/pkg/volume/csi/fake/fake_client.go index 1f3d0b5cdb..1f287e42a0 100644 --- a/pkg/volume/csi/fake/fake_client.go +++ b/pkg/volume/csi/fake/fake_client.go @@ -189,20 +189,23 @@ func (f *NodeClient) NodeStageVolume(ctx context.Context, req *csipb.NodeStageVo return nil, errors.New("missing staging target path") } + csiVol := CSIVolume{ + Path: req.GetStagingTargetPath(), + VolumeContext: req.GetVolumeContext(), + } + fsType := "" fsTypes := "block|ext4|xfs|zfs" mounted := req.GetVolumeCapability().GetMount() if mounted != nil { fsType = mounted.GetFsType() + csiVol.MountFlags = mounted.GetMountFlags() } if !strings.Contains(fsTypes, fsType) { return nil, errors.New("invalid fstype") } - f.nodeStagedVolumes[req.GetVolumeId()] = CSIVolume{ - Path: req.GetStagingTargetPath(), - VolumeContext: req.GetVolumeContext(), - } + f.nodeStagedVolumes[req.GetVolumeId()] = csiVol return &csipb.NodeStageVolumeResponse{}, nil } From b8bec43635808dcfd5a7c1e3de788b4290eb9a55 Mon Sep 17 00:00:00 2001 From: David Zhu Date: Tue, 16 Jul 2019 14:42:53 -0700 Subject: [PATCH 2/2] Add supportedMountOptions for GCE PD CSI Driver tests --- test/e2e/storage/drivers/csi.go | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/storage/drivers/csi.go b/test/e2e/storage/drivers/csi.go index 12537856b2..393510c5dd 100644 --- a/test/e2e/storage/drivers/csi.go +++ b/test/e2e/storage/drivers/csi.go @@ -350,6 +350,7 @@ func InitGcePDCSIDriver() testsuites.TestDriver { "ext4", "xfs", ), + SupportedMountOption: sets.NewString("debug", "nouid32"), Capabilities: map[testsuites.Capability]bool{ testsuites.CapPersistence: true, testsuites.CapFsGroup: true,