From 1bef8b8879e06b2d412dfd4559cdd156576b1e3d Mon Sep 17 00:00:00 2001 From: Lang Martin Date: Mon, 23 Mar 2020 13:55:26 -0400 Subject: [PATCH] csi: add mount_options to volumes and volume requests (#7398) Add mount_options to both the volume definition on registration and to the volume block in the group where the volume is requested. If both are specified, the options provided in the request replace the options defined in the volume. They get passed to the NodePublishVolume, which causes the node plugin to actually mount the volume on the host. Individual tasks just mount bind into the host mounted volume (unchanged behavior). An operator can mount the same volume with different options by specifying it twice in the group context. closes #7007 * nomad/structs/volumes: add MountOptions to volume request * jobspec/test-fixtures/basic.hcl: add mount_options to volume block * jobspec/parse_test: add expected MountOptions * api/tasks: add mount_options * jobspec/parse_group: use hcl decode not mapstructure, mount_options * client/allocrunner/csi_hook: pass MountOptions through client/allocrunner/csi_hook: add a VolumeMountOptions client/allocrunner/csi_hook: drop Options client/allocrunner/csi_hook: use the structs options * client/pluginmanager/csimanager/interface: UsageOptions.MountOptions * client/pluginmanager/csimanager/volume: pass MountOptions in capabilities * plugins/csi/plugin: remove todo 7007 comment * nomad/structs/csi: MountOptions * api/csi: add options to the api for parsing, match structs * plugins/csi/plugin: move VolumeMountOptions to structs * api/csi: use specific type for mount_options * client/allocrunner/csi_hook: merge MountOptions here * rename CSIOptions to CSIMountOptions * client/allocrunner/csi_hook * client/pluginmanager/csimanager/volume * nomad/structs/csi * plugins/csi/fake/client: add PrevVolumeCapability * plugins/csi/plugin * client/pluginmanager/csimanager/volume_test: remove debugging * client/pluginmanager/csimanager/volume: fix odd merging logic * api: rename CSIOptions -> CSIMountOptions * nomad/csi_endpoint: remove a 7007 comment * command/alloc_status: show mount options in the volume list * nomad/structs/csi: include MountOptions in the volume stub * api/csi: add MountOptions to stub * command/volume_status_csi: clean up csiVolMountOption, add it * command/alloc_status: csiVolMountOption lives in volume_csi_status * command/node_status: display mount flags * nomad/structs/volumes: npe * plugins/csi/plugin: npe in ToCSIRepresentation * jobspec/parse_test: expand volume parse test cases * command/agent/job_endpoint: ApiTgToStructsTG needs MountOptions * command/volume_status_csi: copy paste error * jobspec/test-fixtures/basic: hclfmt * command/volume_status_csi: clean up csiVolMountOption --- api/csi.go | 18 +++-- api/tasks.go | 10 +-- client/allocrunner/csi_hook.go | 1 + client/pluginmanager/csimanager/interface.go | 1 + client/pluginmanager/csimanager/volume.go | 23 +++++- .../pluginmanager/csimanager/volume_test.go | 71 +++++++++++++++++-- command/agent/job_endpoint.go | 7 ++ command/alloc_status.go | 2 +- command/node_status.go | 12 ++-- command/volume_status_csi.go | 38 ++++++++++ jobspec/parse_group.go | 36 ++-------- jobspec/parse_test.go | 27 ++++++- jobspec/test-fixtures/basic.hcl | 21 +++++- nomad/csi_endpoint.go | 3 - nomad/structs/csi.go | 53 ++++++++++++++ nomad/structs/volumes.go | 15 ++-- plugins/csi/fake/client.go | 2 + plugins/csi/plugin.go | 56 ++++----------- 18 files changed, 287 insertions(+), 109 deletions(-) diff --git a/api/csi.go b/api/csi.go index df1c372d3..b78019659 100644 --- a/api/csi.go +++ b/api/csi.go @@ -81,15 +81,22 @@ const ( CSIVolumeAccessModeMultiNodeMultiWriter CSIVolumeAccessMode = "multi-node-multi-writer" ) +type CSIMountOptions struct { + FSType string `hcl:"fs_type"` + MountFlags []string `hcl:"mount_flags"` + ExtraKeysHCL []string `hcl:",unusedKeys" json:"-"` // report unexpected keys +} + // CSIVolume is used for serialization, see also nomad/structs/csi.go type CSIVolume struct { - ID string `hcl:"id"` - Name string `hcl:"name"` - ExternalID string `hcl:"external_id"` - Namespace string `hcl:"namespace"` - Topologies []*CSITopology `hcl:"topologies"` + ID string + Name string + ExternalID string `hcl:"external_id"` + Namespace string + Topologies []*CSITopology AccessMode CSIVolumeAccessMode `hcl:"access_mode"` AttachmentMode CSIVolumeAttachmentMode `hcl:"attachment_mode"` + MountOptions *CSIMountOptions `hcl:"mount_options"` // Allocations, tracking claim status ReadAllocs map[string]*Allocation @@ -151,6 +158,7 @@ type CSIVolumeListStub struct { Topologies []*CSITopology AccessMode CSIVolumeAccessMode AttachmentMode CSIVolumeAttachmentMode + MountOptions *CSIMountOptions Schedulable bool PluginID string Provider string diff --git a/api/tasks.go b/api/tasks.go index 7e68fea25..2e6e64d88 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -377,10 +377,12 @@ func (m *MigrateStrategy) Copy() *MigrateStrategy { // VolumeRequest is a representation of a storage volume that a TaskGroup wishes to use. type VolumeRequest struct { - Name string - Type string - Source string - ReadOnly bool `mapstructure:"read_only"` + Name string + Type string + Source string + ReadOnly bool `hcl:"read_only"` + MountOptions *CSIMountOptions `hcl:"mount_options"` + ExtraKeysHCL []string `hcl:",unusedKeys" json:"-"` } const ( diff --git a/client/allocrunner/csi_hook.go b/client/allocrunner/csi_hook.go index 02b0e4708..ac16cbbe5 100644 --- a/client/allocrunner/csi_hook.go +++ b/client/allocrunner/csi_hook.go @@ -48,6 +48,7 @@ func (c *csiHook) Prerun() error { ReadOnly: pair.request.ReadOnly, AttachmentMode: string(pair.volume.AttachmentMode), AccessMode: string(pair.volume.AccessMode), + MountOptions: pair.request.MountOptions, } mountInfo, err := mounter.MountVolume(ctx, pair.volume, c.alloc, usageOpts, pair.publishContext) diff --git a/client/pluginmanager/csimanager/interface.go b/client/pluginmanager/csimanager/interface.go index f2458fd11..c6f97cd69 100644 --- a/client/pluginmanager/csimanager/interface.go +++ b/client/pluginmanager/csimanager/interface.go @@ -22,6 +22,7 @@ type UsageOptions struct { ReadOnly bool AttachmentMode string AccessMode string + MountOptions *structs.CSIMountOptions } // ToFS is used by a VolumeManager to construct the path to where a volume diff --git a/client/pluginmanager/csimanager/volume.go b/client/pluginmanager/csimanager/volume.go index e76edae78..6243af4b6 100644 --- a/client/pluginmanager/csimanager/volume.go +++ b/client/pluginmanager/csimanager/volume.go @@ -117,6 +117,25 @@ func (v *volumeManager) ensureAllocDir(vol *structs.CSIVolume, alloc *structs.Al return allocPath, !isNotMount, nil } +func volumeCapability(vol *structs.CSIVolume, usage *UsageOptions) (*csi.VolumeCapability, error) { + capability, err := csi.VolumeCapabilityFromStructs(vol.AttachmentMode, vol.AccessMode) + if err != nil { + return nil, err + } + + var opts *structs.CSIMountOptions + if vol.MountOptions == nil { + opts = usage.MountOptions + } else { + opts = vol.MountOptions.Copy() + opts.Merge(usage.MountOptions) + } + + capability.MountVolume = opts + + return capability, nil +} + // stageVolume prepares a volume for use by allocations. When a plugin exposes // the STAGE_UNSTAGE_VOLUME capability it MUST be called once-per-volume for a // given usage mode before the volume can be NodePublish-ed. @@ -136,7 +155,7 @@ func (v *volumeManager) stageVolume(ctx context.Context, vol *structs.CSIVolume, return nil } - capability, err := csi.VolumeCapabilityFromStructs(vol.AttachmentMode, vol.AccessMode) + capability, err := volumeCapability(vol, usage) if err != nil { return err } @@ -175,7 +194,7 @@ func (v *volumeManager) publishVolume(ctx context.Context, vol *structs.CSIVolum return &MountInfo{Source: hostTargetPath}, nil } - capabilities, err := csi.VolumeCapabilityFromStructs(vol.AttachmentMode, vol.AccessMode) + capabilities, err := volumeCapability(vol, usage) if err != nil { return nil, err } diff --git a/client/pluginmanager/csimanager/volume_test.go b/client/pluginmanager/csimanager/volume_test.go index 148c55249..93aa2447a 100644 --- a/client/pluginmanager/csimanager/volume_test.go +++ b/client/pluginmanager/csimanager/volume_test.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/plugins/csi" csifake "github.com/hashicorp/nomad/plugins/csi/fake" "github.com/stretchr/testify/require" ) @@ -247,13 +248,14 @@ func TestVolumeManager_unstageVolume(t *testing.T) { func TestVolumeManager_publishVolume(t *testing.T) { t.Parallel() cases := []struct { - Name string - Allocation *structs.Allocation - Volume *structs.CSIVolume - UsageOptions *UsageOptions - PluginErr error - ExpectedErr error - ExpectedCSICallCount int64 + Name string + Allocation *structs.Allocation + Volume *structs.CSIVolume + UsageOptions *UsageOptions + PluginErr error + ExpectedErr error + ExpectedCSICallCount int64 + ExpectedVolumeCapability *csi.VolumeCapability }{ { Name: "Returns an error when the plugin returns an error", @@ -281,6 +283,56 @@ func TestVolumeManager_publishVolume(t *testing.T) { ExpectedErr: nil, ExpectedCSICallCount: 1, }, + { + Name: "Mount options in the volume", + Allocation: structs.MockAlloc(), + Volume: &structs.CSIVolume{ + ID: "foo", + AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem, + AccessMode: structs.CSIVolumeAccessModeMultiNodeMultiWriter, + MountOptions: &structs.CSIMountOptions{ + MountFlags: []string{"ro"}, + }, + }, + UsageOptions: &UsageOptions{}, + PluginErr: nil, + ExpectedErr: nil, + ExpectedCSICallCount: 1, + ExpectedVolumeCapability: &csi.VolumeCapability{ + AccessType: csi.VolumeAccessTypeMount, + AccessMode: csi.VolumeAccessModeMultiNodeMultiWriter, + MountVolume: &structs.CSIMountOptions{ + MountFlags: []string{"ro"}, + }, + }, + }, + { + Name: "Mount options override in the request", + Allocation: structs.MockAlloc(), + Volume: &structs.CSIVolume{ + ID: "foo", + AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem, + AccessMode: structs.CSIVolumeAccessModeMultiNodeMultiWriter, + MountOptions: &structs.CSIMountOptions{ + MountFlags: []string{"ro"}, + }, + }, + UsageOptions: &UsageOptions{ + MountOptions: &structs.CSIMountOptions{ + MountFlags: []string{"rw"}, + }, + }, + PluginErr: nil, + ExpectedErr: nil, + ExpectedCSICallCount: 1, + ExpectedVolumeCapability: &csi.VolumeCapability{ + AccessType: csi.VolumeAccessTypeMount, + AccessMode: csi.VolumeAccessModeMultiNodeMultiWriter, + MountVolume: &structs.CSIMountOptions{ + MountFlags: []string{"rw"}, + }, + }, + }, } for _, tc := range cases { @@ -303,6 +355,11 @@ func TestVolumeManager_publishVolume(t *testing.T) { } require.Equal(t, tc.ExpectedCSICallCount, csiFake.NodePublishVolumeCallCount) + + if tc.ExpectedVolumeCapability != nil { + require.Equal(t, tc.ExpectedVolumeCapability, csiFake.PrevVolumeCapability) + } + }) } } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index edba2a39c..b76329c56 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -762,6 +762,13 @@ func ApiTgToStructsTG(taskGroup *api.TaskGroup, tg *structs.TaskGroup) { Source: v.Source, } + if v.MountOptions != nil { + vol.MountOptions = &structs.CSIMountOptions{ + FSType: v.MountOptions.FSType, + MountFlags: v.MountOptions.MountFlags, + } + } + tg.Volumes[k] = vol } } diff --git a/command/alloc_status.go b/command/alloc_status.go index 666e468a3..b9479aefd 100644 --- a/command/alloc_status.go +++ b/command/alloc_status.go @@ -781,7 +781,7 @@ FOUND: vol.Provider, vol.Schedulable, volReq.ReadOnly, - "n/a", // TODO(tgross): https://github.com/hashicorp/nomad/issues/7007 + csiVolMountOption(vol.MountOptions, volReq.MountOptions), )) } else { csiVolumesOutput = append(csiVolumesOutput, diff --git a/command/node_status.go b/command/node_status.go index 307d55028..2651c4375 100644 --- a/command/node_status.go +++ b/command/node_status.go @@ -541,7 +541,7 @@ func (c *NodeStatusCommand) outputNodeCSIVolumeInfo(client *api.Client, node *ap // Duplicate nodeCSIVolumeNames to sort by name but also index volume names to ids var names []string - volNames := map[string]string{} + requests := map[string]*api.VolumeRequest{} for _, alloc := range runningAllocs { tg := alloc.GetTaskGroup() if tg == nil || len(tg.Volumes) == 0 { @@ -550,7 +550,7 @@ func (c *NodeStatusCommand) outputNodeCSIVolumeInfo(client *api.Client, node *ap for _, v := range tg.Volumes { names = append(names, v.Name) - volNames[v.Source] = v.Name + requests[v.Source] = v } } if len(names) == 0 { @@ -563,23 +563,25 @@ func (c *NodeStatusCommand) outputNodeCSIVolumeInfo(client *api.Client, node *ap volumes := map[string]*api.CSIVolumeListStub{} vs, _ := client.Nodes().CSIVolumes(node.ID, nil) for _, v := range vs { - n := volNames[v.ID] + n := requests[v.ID].Name volumes[n] = v } // Output the volumes in name order output := make([]string, 0, len(names)+1) - output = append(output, "ID|Name|Plugin ID|Schedulable|Provider|Access Mode") + output = append(output, "ID|Name|Plugin ID|Schedulable|Provider|Access Mode|Mount Options") for _, name := range names { v := volumes[name] + r := requests[v.ID] output = append(output, fmt.Sprintf( - "%s|%s|%s|%t|%s|%s", + "%s|%s|%s|%t|%s|%s|%s", v.ID, name, v.PluginID, v.Schedulable, v.Provider, v.AccessMode, + csiVolMountOption(v.MountOptions, r.MountOptions), )) } diff --git a/command/volume_status_csi.go b/command/volume_status_csi.go index 4838c009c..95f6883bf 100644 --- a/command/volume_status_csi.go +++ b/command/volume_status_csi.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/nomad/structs" ) func (c *VolumeStatusCommand) csiBanner() { @@ -105,6 +106,7 @@ func (c *VolumeStatusCommand) formatBasic(vol *api.CSIVolume) (string, error) { fmt.Sprintf("Access Mode|%s", vol.AccessMode), fmt.Sprintf("Attachment Mode|%s", vol.AttachmentMode), + fmt.Sprintf("Mount Options|%s", csiVolMountOption(vol.MountOptions, nil)), fmt.Sprintf("Namespace|%s", vol.Namespace), } @@ -151,3 +153,39 @@ func (c *VolumeStatusCommand) formatTopologies(vol *api.CSIVolume) string { return strings.Join(out, "\n") } + +func csiVolMountOption(volume, request *api.CSIMountOptions) string { + var req, opts *structs.CSIMountOptions + + if request != nil { + req = &structs.CSIMountOptions{ + FSType: request.FSType, + MountFlags: request.MountFlags, + } + } + + if volume == nil { + opts = req + } else { + opts = &structs.CSIMountOptions{ + FSType: volume.FSType, + MountFlags: volume.MountFlags, + } + opts.Merge(req) + } + + if opts == nil { + return "" + } + + var out string + if opts.FSType != "" { + out = fmt.Sprintf("fs_type: %s", opts.FSType) + } + + if len(opts.MountFlags) > 0 { + out = fmt.Sprintf("%s flags: %s", out, strings.Join(opts.MountFlags, ", ")) + } + + return out +} diff --git a/jobspec/parse_group.go b/jobspec/parse_group.go index 062c96074..86c078658 100644 --- a/jobspec/parse_group.go +++ b/jobspec/parse_group.go @@ -295,41 +295,17 @@ func parseRestartPolicy(final **api.RestartPolicy, list *ast.ObjectList) error { } func parseVolumes(out *map[string]*api.VolumeRequest, list *ast.ObjectList) error { - volumes := make(map[string]*api.VolumeRequest, len(list.Items)) + hcl.DecodeObject(out, list) - for _, item := range list.Items { - n := item.Keys[0].Token.Value().(string) - valid := []string{ - "type", - "read_only", - "hidden", - "source", - } - if err := helper.CheckHCLKeys(item.Val, valid); err != nil { - return err - } - - var m map[string]interface{} - if err := hcl.DecodeObject(&m, item.Val); err != nil { - return err - } - - var result api.VolumeRequest - dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - WeaklyTypedInput: true, - Result: &result, - }) + for k, v := range *out { + err := helper.UnusedKeys(v) if err != nil { return err } - if err := dec.Decode(m); err != nil { - return err - } - - result.Name = n - volumes[n] = &result + // This is supported by `hcl:",key"`, but that only works if we start at the + // parent ast.ObjectItem + v.Name = k } - *out = volumes return nil } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index ed66f05ae..924d3ab71 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -117,11 +117,32 @@ func TestParse(t *testing.T) { Operand: "=", }, }, - Volumes: map[string]*api.VolumeRequest{ "foo": { - Name: "foo", - Type: "host", + Name: "foo", + Type: "host", + Source: "/path", + ExtraKeysHCL: nil, + }, + "bar": { + Name: "bar", + Type: "csi", + Source: "bar-vol", + MountOptions: &api.CSIMountOptions{ + FSType: "ext4", + }, + ExtraKeysHCL: nil, + }, + "baz": { + Name: "baz", + Type: "csi", + Source: "bar-vol", + MountOptions: &api.CSIMountOptions{ + MountFlags: []string{ + "ro", + }, + }, + ExtraKeysHCL: nil, }, }, Affinities: []*api.Affinity{ diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index b5068a1c4..ba826bdc0 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -71,7 +71,26 @@ job "binstore-storagelocker" { count = 5 volume "foo" { - type = "host" + type = "host" + source = "/path" + } + + volume "bar" { + type = "csi" + source = "bar-vol" + + mount_options { + fs_type = "ext4" + } + } + + volume "baz" { + type = "csi" + source = "bar-vol" + + mount_options { + mount_flags = ["ro"] + } } restart { diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go index 15d106d2c..b0968c615 100644 --- a/nomad/csi_endpoint.go +++ b/nomad/csi_endpoint.go @@ -551,9 +551,6 @@ func (srv *Server) controllerPublishVolume(req *structs.CSIVolumeClaimRequest, r AttachmentMode: vol.AttachmentMode, AccessMode: vol.AccessMode, ReadOnly: req.Claim == structs.CSIVolumeClaimRead, - // TODO(tgross): we don't have a way of setting these yet. - // ref https://github.com/hashicorp/nomad/issues/7007 - // MountOptions: vol.MountOptions, } cReq.PluginID = plug.ID cReq.ControllerNodeID = nodeID diff --git a/nomad/structs/csi.go b/nomad/structs/csi.go index 704fece53..bbd19b99a 100644 --- a/nomad/structs/csi.go +++ b/nomad/structs/csi.go @@ -135,6 +135,56 @@ func ValidCSIVolumeWriteAccessMode(accessMode CSIVolumeAccessMode) bool { } } +// CSIMountOptions contain optional additional configuration that can be used +// when specifying that a Volume should be used with VolumeAccessTypeMount. +type CSIMountOptions struct { + // FSType is an optional field that allows an operator to specify the type + // of the filesystem. + FSType string + + // MountFlags contains additional options that may be used when mounting the + // volume by the plugin. This may contain sensitive data and should not be + // leaked. + MountFlags []string +} + +func (o *CSIMountOptions) Copy() *CSIMountOptions { + if o == nil { + return nil + } + return &(*o) +} + +func (o *CSIMountOptions) Merge(p *CSIMountOptions) { + if p == nil { + return + } + if p.FSType != "" { + o.FSType = p.FSType + } + if p.MountFlags != nil { + o.MountFlags = p.MountFlags + } +} + +// VolumeMountOptions implements the Stringer and GoStringer interfaces to prevent +// accidental leakage of sensitive mount flags via logs. +var _ fmt.Stringer = &CSIMountOptions{} +var _ fmt.GoStringer = &CSIMountOptions{} + +func (v *CSIMountOptions) String() string { + mountFlagsString := "nil" + if len(v.MountFlags) != 0 { + mountFlagsString = "[REDACTED]" + } + + return fmt.Sprintf("csi.CSIOptions(FSType: %s, MountFlags: %s)", v.FSType, mountFlagsString) +} + +func (v *CSIMountOptions) GoString() string { + return v.String() +} + // CSIVolume is the full representation of a CSI Volume type CSIVolume struct { // ID is a namespace unique URL safe identifier for the volume @@ -147,6 +197,7 @@ type CSIVolume struct { Topologies []*CSITopology AccessMode CSIVolumeAccessMode AttachmentMode CSIVolumeAttachmentMode + MountOptions *CSIMountOptions // Allocations, tracking claim status ReadAllocs map[string]*Allocation @@ -178,6 +229,7 @@ type CSIVolListStub struct { Topologies []*CSITopology AccessMode CSIVolumeAccessMode AttachmentMode CSIVolumeAttachmentMode + MountOptions *CSIMountOptions CurrentReaders int CurrentWriters int Schedulable bool @@ -228,6 +280,7 @@ func (v *CSIVolume) Stub() *CSIVolListStub { Topologies: v.Topologies, AccessMode: v.AccessMode, AttachmentMode: v.AttachmentMode, + MountOptions: v.MountOptions, CurrentReaders: len(v.ReadAllocs), CurrentWriters: len(v.WriteAllocs), Schedulable: v.Schedulable, diff --git a/nomad/structs/volumes.go b/nomad/structs/volumes.go index fe44e4830..e29d1c42b 100644 --- a/nomad/structs/volumes.go +++ b/nomad/structs/volumes.go @@ -86,10 +86,11 @@ func HostVolumeSliceMerge(a, b []*ClientHostVolumeConfig) []*ClientHostVolumeCon // VolumeRequest is a representation of a storage volume that a TaskGroup wishes to use. type VolumeRequest struct { - Name string - Type string - Source string - ReadOnly bool + Name string + Type string + Source string + ReadOnly bool + MountOptions *CSIMountOptions } func (v *VolumeRequest) Copy() *VolumeRequest { @@ -99,6 +100,12 @@ func (v *VolumeRequest) Copy() *VolumeRequest { nv := new(VolumeRequest) *nv = *v + if v.MountOptions == nil { + return nv + } + + nv.MountOptions = &(*v.MountOptions) + return nv } diff --git a/plugins/csi/fake/client.go b/plugins/csi/fake/client.go index 162b6bb73..b971ce260 100644 --- a/plugins/csi/fake/client.go +++ b/plugins/csi/fake/client.go @@ -67,6 +67,7 @@ type Client struct { NextNodeUnstageVolumeErr error NodeUnstageVolumeCallCount int64 + PrevVolumeCapability *csi.VolumeCapability NextNodePublishVolumeErr error NodePublishVolumeCallCount int64 @@ -217,6 +218,7 @@ func (c *Client) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolu c.Mu.Lock() defer c.Mu.Unlock() + c.PrevVolumeCapability = req.VolumeCapability c.NodePublishVolumeCallCount++ return c.NextNodePublishVolumeErr diff --git a/plugins/csi/plugin.go b/plugins/csi/plugin.go index 97e463dee..345b9f753 100644 --- a/plugins/csi/plugin.go +++ b/plugins/csi/plugin.go @@ -357,42 +357,13 @@ func (v VolumeAccessType) String() string { } } -// VolumeMountOptions contain optional additional configuration that can be used -// when specifying that a Volume should be used with VolumeAccessTypeMount. -type VolumeMountOptions struct { - // FSType is an optional field that allows an operator to specify the type - // of the filesystem. - FSType string - - // MountFlags contains additional options that may be used when mounting the - // volume by the plugin. This may contain sensitive data and should not be - // leaked. - MountFlags []string -} - -// VolumeMountOptions implements the Stringer and GoStringer interfaces to prevent -// accidental leakage of sensitive mount flags via logs. -var _ fmt.Stringer = &VolumeMountOptions{} -var _ fmt.GoStringer = &VolumeMountOptions{} - -func (v *VolumeMountOptions) String() string { - mountFlagsString := "nil" - if len(v.MountFlags) != 0 { - mountFlagsString = "[REDACTED]" - } - - return fmt.Sprintf("csi.VolumeMountOptions(FSType: %s, MountFlags: %s)", v.FSType, mountFlagsString) -} - -func (v *VolumeMountOptions) GoString() string { - return v.String() -} - // VolumeCapability describes the overall usage requirements for a given CSI Volume type VolumeCapability struct { - AccessType VolumeAccessType - AccessMode VolumeAccessMode - VolumeMountOptions *VolumeMountOptions + AccessType VolumeAccessType + AccessMode VolumeAccessMode + + // Indicate that the volume will be accessed via the filesystem API. + MountVolume *structs.CSIMountOptions } func VolumeCapabilityFromStructs(sAccessType structs.CSIVolumeAttachmentMode, sAccessMode structs.CSIVolumeAccessMode) (*VolumeCapability, error) { @@ -431,11 +402,8 @@ func VolumeCapabilityFromStructs(sAccessType structs.CSIVolumeAttachmentMode, sA } return &VolumeCapability{ - AccessType: accessType, - AccessMode: accessMode, - VolumeMountOptions: &VolumeMountOptions{ - // GH-7007: Currently we have no way to provide these - }, + AccessType: accessType, + AccessMode: accessMode, }, nil } @@ -451,12 +419,12 @@ func (c *VolumeCapability) ToCSIRepresentation() *csipbv1.VolumeCapability { } if c.AccessType == VolumeAccessTypeMount { - vc.AccessType = &csipbv1.VolumeCapability_Mount{ - Mount: &csipbv1.VolumeCapability_MountVolume{ - FsType: c.VolumeMountOptions.FSType, - MountFlags: c.VolumeMountOptions.MountFlags, - }, + opts := &csipbv1.VolumeCapability_MountVolume{} + if c.MountVolume != nil { + opts.FsType = c.MountVolume.FSType + opts.MountFlags = c.MountVolume.MountFlags } + vc.AccessType = &csipbv1.VolumeCapability_Mount{Mount: opts} } else { vc.AccessType = &csipbv1.VolumeCapability_Block{Block: &csipbv1.VolumeCapability_BlockVolume{}} }