From 6950f1b2006e701f97247c47c2122b546b402ccf Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 24 Mar 2021 14:03:39 -0400 Subject: [PATCH 1/7] CSI: add missing fields to api.CSIVolume --- api/csi.go | 15 +++++++++++++++ vendor/github.com/hashicorp/nomad/api/csi.go | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/api/csi.go b/api/csi.go index 5cb04f3a7b9..cf85034b4f7 100644 --- a/api/csi.go +++ b/api/csi.go @@ -143,6 +143,14 @@ type CSIVolume struct { Secrets CSISecrets `hcl:"secrets"` Parameters map[string]string `hcl:"parameters"` Context map[string]string `hcl:"context"` + Capacity int64 `hcl:"-"` + + // These fields are used as part of the volume creation request + RequestedCapacityMin int64 `hcl:"capacity_min"` + RequestedCapacityMax int64 `hcl:"capacity_max"` + RequestedCapabilities []*CSIVolumeCapability `hcl:"capability"` + CloneID string `hcl:"clone_id"` + SnapshotID string `hcl:"snapshot_id"` // ReadAllocs is a map of allocation IDs for tracking reader claim status. // The Allocation value will always be nil; clients can populate this data @@ -176,6 +184,13 @@ type CSIVolume struct { ExtraKeysHCL []string `hcl1:",unusedKeys" json:"-"` } +// CSIVolumeCapability is a requested attachment and access mode for a +// volume +type CSIVolumeCapability struct { + AccessMode CSIVolumeAccessMode `hcl:"access_mode"` + AttachmentMode CSIVolumeAttachmentMode `hcl:"attachment_mode"` +} + type CSIVolumeIndexSort []*CSIVolumeListStub func (v CSIVolumeIndexSort) Len() int { diff --git a/vendor/github.com/hashicorp/nomad/api/csi.go b/vendor/github.com/hashicorp/nomad/api/csi.go index 5cb04f3a7b9..cf85034b4f7 100644 --- a/vendor/github.com/hashicorp/nomad/api/csi.go +++ b/vendor/github.com/hashicorp/nomad/api/csi.go @@ -143,6 +143,14 @@ type CSIVolume struct { Secrets CSISecrets `hcl:"secrets"` Parameters map[string]string `hcl:"parameters"` Context map[string]string `hcl:"context"` + Capacity int64 `hcl:"-"` + + // These fields are used as part of the volume creation request + RequestedCapacityMin int64 `hcl:"capacity_min"` + RequestedCapacityMax int64 `hcl:"capacity_max"` + RequestedCapabilities []*CSIVolumeCapability `hcl:"capability"` + CloneID string `hcl:"clone_id"` + SnapshotID string `hcl:"snapshot_id"` // ReadAllocs is a map of allocation IDs for tracking reader claim status. // The Allocation value will always be nil; clients can populate this data @@ -176,6 +184,13 @@ type CSIVolume struct { ExtraKeysHCL []string `hcl1:",unusedKeys" json:"-"` } +// CSIVolumeCapability is a requested attachment and access mode for a +// volume +type CSIVolumeCapability struct { + AccessMode CSIVolumeAccessMode `hcl:"access_mode"` + AttachmentMode CSIVolumeAttachmentMode `hcl:"attachment_mode"` +} + type CSIVolumeIndexSort []*CSIVolumeListStub func (v CSIVolumeIndexSort) Len() int { From 9734a80ff22c01e157540c4e1685bd76040f7ef0 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 24 Mar 2021 14:06:48 -0400 Subject: [PATCH 2/7] CSI: volume creation/registration should not validate attachment The CSI specification requires that we validate a list of `Capability` (access mode + accessibility) when we create volume, but the existing volume registration workflow incorrectly validates a single capability. The specific capability required by a volume claim is checked at the time we make the claim, so remove the check for `AttachmentMode`/`AcccessMode`. --- nomad/csi_endpoint_test.go | 5 ----- nomad/structs/csi.go | 16 ---------------- 2 files changed, 21 deletions(-) diff --git a/nomad/csi_endpoint_test.go b/nomad/csi_endpoint_test.go index 0c2c4427889..bf7debc59ad 100644 --- a/nomad/csi_endpoint_test.go +++ b/nomad/csi_endpoint_test.go @@ -162,11 +162,6 @@ func TestCSIVolumeEndpoint_Register(t *testing.T) { } resp1 := &structs.CSIVolumeRegisterResponse{} err := msgpackrpc.CallWithCodec(codec, "CSIVolume.Register", req1, resp1) - require.Error(t, err, "expected validation error") - - // Fix the registration so that it passes validation - vols[0].AttachmentMode = structs.CSIVolumeAttachmentModeFilesystem - err = msgpackrpc.CallWithCodec(codec, "CSIVolume.Register", req1, resp1) require.NoError(t, err) require.NotEqual(t, uint64(0), resp1.Index) diff --git a/nomad/structs/csi.go b/nomad/structs/csi.go index 2abac42d405..d4ea7bde5c4 100644 --- a/nomad/structs/csi.go +++ b/nomad/structs/csi.go @@ -590,22 +590,6 @@ func (v *CSIVolume) Validate() error { if v.Namespace == "" { errs = append(errs, "missing namespace") } - if v.AccessMode == "" { - errs = append(errs, "missing access mode") - } - if v.AttachmentMode == "" { - errs = append(errs, "missing attachment mode") - } - if v.AttachmentMode == CSIVolumeAttachmentModeBlockDevice { - if v.MountOptions != nil { - if v.MountOptions.FSType != "" { - errs = append(errs, "mount options not allowed for block-device") - } - if v.MountOptions.MountFlags != nil && len(v.MountOptions.MountFlags) != 0 { - errs = append(errs, "mount options not allowed for block-device") - } - } - } if v.SnapshotID != "" && v.CloneID != "" { errs = append(errs, "only one of snapshot_id and clone_id is allowed") } From 74e37b65f7f1d144a449a6e71176ffe10e7ea6ce Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 24 Mar 2021 14:10:44 -0400 Subject: [PATCH 3/7] CSI: fingerprint detailed controller capabilities In order to support new controller RPCs, we need to fingerprint volume capabilities in more detail and perform controller RPCs only when the specific capability is present. This fixes a bug in Ceph support where the plugin can only suport create/delete but we assume that it also supports attach/detach. --- .../pluginmanager/csimanager/fingerprint.go | 10 ++- nomad/csi_endpoint.go | 25 +++++--- nomad/csi_endpoint_test.go | 1 + nomad/structs/csi.go | 61 +++++++++++++++++-- nomad/structs/node.go | 49 +++++++++++---- plugins/csi/client_test.go | 2 +- plugins/csi/plugin.go | 30 ++++++++- 7 files changed, 150 insertions(+), 28 deletions(-) diff --git a/client/pluginmanager/csimanager/fingerprint.go b/client/pluginmanager/csimanager/fingerprint.go index b0da9c8fcf5..c35131a83c5 100644 --- a/client/pluginmanager/csimanager/fingerprint.go +++ b/client/pluginmanager/csimanager/fingerprint.go @@ -122,10 +122,18 @@ func (p *pluginFingerprinter) buildBasicFingerprint(ctx context.Context) (*struc } func applyCapabilitySetToControllerInfo(cs *csi.ControllerCapabilitySet, info *structs.CSIControllerInfo) { - info.SupportsReadOnlyAttach = cs.HasPublishReadonly + info.SupportsCreateDelete = cs.HasCreateDeleteVolume info.SupportsAttachDetach = cs.HasPublishUnpublishVolume info.SupportsListVolumes = cs.HasListVolumes + info.SupportsGetCapacity = cs.HasGetCapacity + info.SupportsCreateDeleteSnapshot = cs.HasCreateDeleteSnapshot + info.SupportsListSnapshots = cs.HasListSnapshots + info.SupportsClone = cs.HasCloneVolume + info.SupportsReadOnlyAttach = cs.HasPublishReadonly + info.SupportsExpand = cs.HasExpandVolume info.SupportsListVolumesAttachedNodes = cs.HasListVolumesPublishedNodes + info.SupportsCondition = cs.HasVolumeCondition + info.SupportsGet = cs.HasGetVolume } func (p *pluginFingerprinter) buildControllerFingerprint(ctx context.Context, base *structs.CSIInfo) (*structs.CSIInfo, error) { diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go index ac94296d1b8..403975a5497 100644 --- a/nomad/csi_endpoint.go +++ b/nomad/csi_endpoint.go @@ -441,9 +441,9 @@ func (v *CSIVolume) controllerPublishVolume(req *structs.CSIVolumeClaimRequest, return fmt.Errorf("%s: %s", structs.ErrUnknownAllocationPrefix, req.AllocationID) } - // if no plugin was returned then controller validation is not required. - // Here we can return nil. - if plug == nil { + // if no plugin was returned or the plugin doesn't attach volumes, then + // controller validation is not required and we can safely return nil. + if plug == nil || !plug.HasControllerCapability(structs.CSIControllerSupportsAttachDetach) { return nil } @@ -679,11 +679,21 @@ func (v *CSIVolume) controllerUnpublishVolume(vol *structs.CSIVolume, claim *str return nil } + state := v.srv.fsm.State() + ws := memdb.NewWatchSet() + + plugin, err := state.CSIPluginByID(ws, vol.PluginID) + if err != nil || plugin == nil { + return fmt.Errorf("could not query plugin: %v", err) + } + if !plugin.HasControllerCapability(structs.CSIControllerSupportsAttachDetach) { + return nil + } + // we only send a controller detach if a Nomad client no longer has // any claim to the volume, so we need to check the status of claimed // allocations - state := v.srv.fsm.State() - vol, err := state.CSIVolumeDenormalize(memdb.NewWatchSet(), vol) + vol, err = state.CSIVolumeDenormalize(ws, vol) if err != nil { return err } @@ -834,9 +844,8 @@ func (v *CSIVolume) Create(args *structs.CSIVolumeCreateRequest, reply *structs. if !plugin.ControllerRequired { return fmt.Errorf("plugin has no controller") } - - if err := v.controllerValidateVolume(regArgs, vol, plugin); err != nil { - return err + if !plugin.HasControllerCapability(structs.CSIControllerSupportsCreateDelete) { + return fmt.Errorf("plugin does not support creating volumes") } validatedVols = append(validatedVols, validated{vol, plugin}) diff --git a/nomad/csi_endpoint_test.go b/nomad/csi_endpoint_test.go index bf7debc59ad..5443bbc6d52 100644 --- a/nomad/csi_endpoint_test.go +++ b/nomad/csi_endpoint_test.go @@ -708,6 +708,7 @@ func TestCSIVolumeEndpoint_Create(t *testing.T) { Healthy: true, ControllerInfo: &structs.CSIControllerInfo{ SupportsAttachDetach: true, + SupportsCreateDelete: true, }, RequiresControllerPlugin: true, }, diff --git a/nomad/structs/csi.go b/nomad/structs/csi.go index d4ea7bde5c4..69ededeeb85 100644 --- a/nomad/structs/csi.go +++ b/nomad/structs/csi.go @@ -846,14 +846,67 @@ func (p *CSIPlugin) Copy() *CSIPlugin { return out } +type CSIControllerCapability int + +const ( + CSIControllerSupportsCreateDelete CSIControllerCapability = iota + CSIControllerSupportsAttachDetach + CSIControllerSupportsListVolumes + CSIControllerSupportsGetCapacity + CSIControllerSupportsCreateDeleteSnapshot + CSIControllerSupportsListSnapshots + CSIControllerSupportsClone + CSIControllerSupportsReadOnlyAttach + CSIControllerSupportsExpand + CSIControllerSupportsListVolumesAttachedNodes + CSIControllerSupportsCondition + CSIControllerSupportsGet +) + +func (p *CSIPlugin) HasControllerCapability(cap CSIControllerCapability) bool { + if len(p.Controllers) < 1 { + return false + } + // we're picking the first controller because they should be uniform + // across the same version of the plugin + for _, c := range p.Controllers { + switch cap { + case CSIControllerSupportsCreateDelete: + return c.ControllerInfo.SupportsCreateDelete + case CSIControllerSupportsAttachDetach: + return c.ControllerInfo.SupportsAttachDetach + case CSIControllerSupportsListVolumes: + return c.ControllerInfo.SupportsListVolumes + case CSIControllerSupportsGetCapacity: + return c.ControllerInfo.SupportsGetCapacity + case CSIControllerSupportsCreateDeleteSnapshot: + return c.ControllerInfo.SupportsCreateDeleteSnapshot + case CSIControllerSupportsListSnapshots: + return c.ControllerInfo.SupportsListSnapshots + case CSIControllerSupportsClone: + return c.ControllerInfo.SupportsClone + case CSIControllerSupportsReadOnlyAttach: + return c.ControllerInfo.SupportsReadOnlyAttach + case CSIControllerSupportsExpand: + return c.ControllerInfo.SupportsExpand + case CSIControllerSupportsListVolumesAttachedNodes: + return c.ControllerInfo.SupportsListVolumesAttachedNodes + case CSIControllerSupportsCondition: + return c.ControllerInfo.SupportsCondition + case CSIControllerSupportsGet: + return c.ControllerInfo.SupportsGet + default: + return false + } + } + return false +} + // AddPlugin adds a single plugin running on the node. Called from state.NodeUpdate in a // transaction func (p *CSIPlugin) AddPlugin(nodeID string, info *CSIInfo) error { if info.ControllerInfo != nil { - p.ControllerRequired = info.RequiresControllerPlugin && - (info.ControllerInfo.SupportsAttachDetach || - info.ControllerInfo.SupportsReadOnlyAttach) - + p.ControllerRequired = info.RequiresControllerPlugin prev, ok := p.Controllers[nodeID] if ok { if prev == nil { diff --git a/nomad/structs/node.go b/nomad/structs/node.go index a49cda0310b..c8631ebff40 100644 --- a/nomad/structs/node.go +++ b/nomad/structs/node.go @@ -112,23 +112,50 @@ func (n *CSINodeInfo) Copy() *CSINodeInfo { // CSIControllerInfo is the fingerprinted data from a CSI Plugin that is specific to // the Controller API. type CSIControllerInfo struct { - // SupportsReadOnlyAttach is set to true when the controller returns the - // ATTACH_READONLY capability. - SupportsReadOnlyAttach bool - // SupportsPublishVolume is true when the controller implements the methods - // required to attach and detach volumes. If this is false Nomad should skip - // the controller attachment flow. + // SupportsCreateDelete indicates plugin support for CREATE_DELETE_VOLUME + SupportsCreateDelete bool + + // SupportsPublishVolume is true when the controller implements the + // methods required to attach and detach volumes. If this is false Nomad + // should skip the controller attachment flow. SupportsAttachDetach bool - // SupportsListVolumes is true when the controller implements the ListVolumes - // RPC. NOTE: This does not guaruntee that attached nodes will be returned - // unless SupportsListVolumesAttachedNodes is also true. + // SupportsListVolumes is true when the controller implements the + // ListVolumes RPC. NOTE: This does not guarantee that attached nodes will + // be returned unless SupportsListVolumesAttachedNodes is also true. SupportsListVolumes bool - // SupportsListVolumesAttachedNodes indicates whether the plugin will return - // attached nodes data when making ListVolume RPCs + // SupportsGetCapacity indicates plugin support for GET_CAPACITY + SupportsGetCapacity bool + + // SupportsCreateDeleteSnapshot indicates plugin support for + // CREATE_DELETE_SNAPSHOT + SupportsCreateDeleteSnapshot bool + + // SupportsListSnapshots indicates plugin support for LIST_SNAPSHOTS + SupportsListSnapshots bool + + // SupportsClone indicates plugin support for CLONE_VOLUME + SupportsClone bool + + // SupportsReadOnlyAttach is set to true when the controller returns the + // ATTACH_READONLY capability. + SupportsReadOnlyAttach bool + + // SupportsExpand indicates plugin support for EXPAND_VOLUME + SupportsExpand bool + + // SupportsListVolumesAttachedNodes indicates whether the plugin will + // return attached nodes data when making ListVolume RPCs (plugin support + // for LIST_VOLUMES_PUBLISHED_NODES) SupportsListVolumesAttachedNodes bool + + // SupportsCondition indicates plugin support for VOLUME_CONDITION + SupportsCondition bool + + // SupportsGet indicates plugin support for GET_VOLUME + SupportsGet bool } func (c *CSIControllerInfo) Copy() *CSIControllerInfo { diff --git a/plugins/csi/client_test.go b/plugins/csi/client_test.go index a2b99487522..c2423e3ae16 100644 --- a/plugins/csi/client_test.go +++ b/plugins/csi/client_test.go @@ -222,7 +222,7 @@ func TestClient_RPC_ControllerGetCapabilities(t *testing.T) { { Type: &csipbv1.ControllerServiceCapability_Rpc{ Rpc: &csipbv1.ControllerServiceCapability_RPC{ - Type: csipbv1.ControllerServiceCapability_RPC_GET_CAPACITY, + Type: csipbv1.ControllerServiceCapability_RPC_UNKNOWN, }, }, }, diff --git a/plugins/csi/plugin.go b/plugins/csi/plugin.go index 4bf4f322c04..8e6cb713187 100644 --- a/plugins/csi/plugin.go +++ b/plugins/csi/plugin.go @@ -280,10 +280,18 @@ func NewPluginCapabilitySet(capabilities *csipbv1.GetPluginCapabilitiesResponse) } type ControllerCapabilitySet struct { + HasCreateDeleteVolume bool HasPublishUnpublishVolume bool - HasPublishReadonly bool HasListVolumes bool + HasGetCapacity bool + HasCreateDeleteSnapshot bool + HasListSnapshots bool + HasCloneVolume bool + HasPublishReadonly bool + HasExpandVolume bool HasListVolumesPublishedNodes bool + HasVolumeCondition bool + HasGetVolume bool } func NewControllerCapabilitySet(resp *csipbv1.ControllerGetCapabilitiesResponse) *ControllerCapabilitySet { @@ -293,14 +301,30 @@ func NewControllerCapabilitySet(resp *csipbv1.ControllerGetCapabilitiesResponse) for _, pcap := range pluginCapabilities { if c := pcap.GetRpc(); c != nil { switch c.Type { + case csipbv1.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME: + cs.HasCreateDeleteVolume = true case csipbv1.ControllerServiceCapability_RPC_PUBLISH_UNPUBLISH_VOLUME: cs.HasPublishUnpublishVolume = true - case csipbv1.ControllerServiceCapability_RPC_PUBLISH_READONLY: - cs.HasPublishReadonly = true case csipbv1.ControllerServiceCapability_RPC_LIST_VOLUMES: cs.HasListVolumes = true + case csipbv1.ControllerServiceCapability_RPC_GET_CAPACITY: + cs.HasGetCapacity = true + case csipbv1.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT: + cs.HasCreateDeleteSnapshot = true + case csipbv1.ControllerServiceCapability_RPC_LIST_SNAPSHOTS: + cs.HasListSnapshots = true + case csipbv1.ControllerServiceCapability_RPC_CLONE_VOLUME: + cs.HasCloneVolume = true + case csipbv1.ControllerServiceCapability_RPC_PUBLISH_READONLY: + cs.HasPublishReadonly = true + case csipbv1.ControllerServiceCapability_RPC_EXPAND_VOLUME: + cs.HasExpandVolume = true case csipbv1.ControllerServiceCapability_RPC_LIST_VOLUMES_PUBLISHED_NODES: cs.HasListVolumesPublishedNodes = true + case csipbv1.ControllerServiceCapability_RPC_VOLUME_CONDITION: + cs.HasVolumeCondition = true + case csipbv1.ControllerServiceCapability_RPC_GET_VOLUME: + cs.HasGetVolume = true default: continue } From 2f155b1ddca16f9dae24a143e4c734427c785536 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 29 Mar 2021 16:04:53 -0400 Subject: [PATCH 4/7] CSI: fix misleading HTTP test The HTTP test to create CSI volumes depends on having a controller plugin to talk to, but the test was using a node-only plugin, which allows it to silently ignore the missing controller. --- command/agent/csi_endpoint_test.go | 24 ++++++++++++++++-------- nomad/state/testing.go | 2 ++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/command/agent/csi_endpoint_test.go b/command/agent/csi_endpoint_test.go index 8b2ace2e197..bff28369780 100644 --- a/command/agent/csi_endpoint_test.go +++ b/command/agent/csi_endpoint_test.go @@ -58,7 +58,7 @@ func TestHTTP_CSIEndpointUtils(t *testing.T) { require.Equal(t, "bar", tops[0].Segments["foo"]) } -func TestHTTP_CSIEndpointVolume(t *testing.T) { +func TestHTTP_CSIEndpointRegisterVolume(t *testing.T) { t.Parallel() httpTest(t, nil, func(s *TestAgent) { server := s.Agent.Server() @@ -95,8 +95,17 @@ func TestHTTP_CSIEndpointVolume(t *testing.T) { resp = httptest.NewRecorder() _, err = s.Server.CSIVolumeSpecificRequest(resp, req) require.Equal(t, CodedError(400, "detach requires node ID"), err) + }) +} - cArgs := structs.CSIVolumeCreateRequest{ +func TestHTTP_CSIEndpointCreateVolume(t *testing.T) { + t.Parallel() + httpTest(t, nil, func(s *TestAgent) { + server := s.Agent.Server() + cleanup := state.CreateTestCSIPlugin(server.State(), "foo") + defer cleanup() + + args := structs.CSIVolumeCreateRequest{ Volumes: []*structs.CSIVolume{{ ID: "baz", PluginID: "foo", @@ -104,19 +113,18 @@ func TestHTTP_CSIEndpointVolume(t *testing.T) { AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem, }}, } - body = encodeReq(cArgs) - req, err = http.NewRequest("PUT", "/v1/volumes/create", body) + body := encodeReq(args) + req, err := http.NewRequest("PUT", "/v1/volumes/create", body) require.NoError(t, err) - resp = httptest.NewRecorder() + resp := httptest.NewRecorder() _, err = s.Server.CSIVolumesRequest(resp, req) - require.NoError(t, err, "put error") + require.Error(t, err, "controller validate volume: No path to node") req, err = http.NewRequest("DELETE", "/v1/volume/csi/baz", nil) require.NoError(t, err) resp = httptest.NewRecorder() _, err = s.Server.CSIVolumeSpecificRequest(resp, req) - require.NoError(t, err, "delete error") - + require.Error(t, err, "volume not found: baz") }) } diff --git a/nomad/state/testing.go b/nomad/state/testing.go index 39072010b02..554d21fb567 100644 --- a/nomad/state/testing.go +++ b/nomad/state/testing.go @@ -81,6 +81,8 @@ func createTestCSIPlugin(s *StateStore, id string, requiresController bool) func SupportsAttachDetach: true, SupportsListVolumes: true, SupportsListVolumesAttachedNodes: false, + SupportsCreateDeleteSnapshot: true, + SupportsListSnapshots: true, }, }, } From e9329a7e23e12a3d7a27f6016f3dc92ca76dc468 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 30 Mar 2021 11:37:35 -0400 Subject: [PATCH 5/7] address comments from code review --- nomad/csi_endpoint.go | 4 +++- nomad/structs/csi.go | 26 +++++++++++++------------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go index 403975a5497..09c7531e5b6 100644 --- a/nomad/csi_endpoint.go +++ b/nomad/csi_endpoint.go @@ -683,8 +683,10 @@ func (v *CSIVolume) controllerUnpublishVolume(vol *structs.CSIVolume, claim *str ws := memdb.NewWatchSet() plugin, err := state.CSIPluginByID(ws, vol.PluginID) - if err != nil || plugin == nil { + if err != nil { return fmt.Errorf("could not query plugin: %v", err) + } else if plugin == nil { + return fmt.Errorf("no such plugin: %q", vol.PluginID) } if !plugin.HasControllerCapability(structs.CSIControllerSupportsAttachDetach) { return nil diff --git a/nomad/structs/csi.go b/nomad/structs/csi.go index 69ededeeb85..5161bd9ebb1 100644 --- a/nomad/structs/csi.go +++ b/nomad/structs/csi.go @@ -846,21 +846,21 @@ func (p *CSIPlugin) Copy() *CSIPlugin { return out } -type CSIControllerCapability int +type CSIControllerCapability byte const ( - CSIControllerSupportsCreateDelete CSIControllerCapability = iota - CSIControllerSupportsAttachDetach - CSIControllerSupportsListVolumes - CSIControllerSupportsGetCapacity - CSIControllerSupportsCreateDeleteSnapshot - CSIControllerSupportsListSnapshots - CSIControllerSupportsClone - CSIControllerSupportsReadOnlyAttach - CSIControllerSupportsExpand - CSIControllerSupportsListVolumesAttachedNodes - CSIControllerSupportsCondition - CSIControllerSupportsGet + CSIControllerSupportsCreateDelete CSIControllerCapability = 0 + CSIControllerSupportsAttachDetach CSIControllerCapability = 1 + CSIControllerSupportsListVolumes CSIControllerCapability = 2 + CSIControllerSupportsGetCapacity CSIControllerCapability = 3 + CSIControllerSupportsCreateDeleteSnapshot CSIControllerCapability = 4 + CSIControllerSupportsListSnapshots CSIControllerCapability = 5 + CSIControllerSupportsClone CSIControllerCapability = 6 + CSIControllerSupportsReadOnlyAttach CSIControllerCapability = 7 + CSIControllerSupportsExpand CSIControllerCapability = 8 + CSIControllerSupportsListVolumesAttachedNodes CSIControllerCapability = 9 + CSIControllerSupportsCondition CSIControllerCapability = 10 + CSIControllerSupportsGet CSIControllerCapability = 11 ) func (p *CSIPlugin) HasControllerCapability(cap CSIControllerCapability) bool { From 1056a97af113318748d3ff5abd41c74070ba0610 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 30 Mar 2021 11:52:39 -0400 Subject: [PATCH 6/7] add comments to CSIControllerSupports* consts --- nomad/structs/csi.go | 57 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/nomad/structs/csi.go b/nomad/structs/csi.go index 5161bd9ebb1..1c99242c1cd 100644 --- a/nomad/structs/csi.go +++ b/nomad/structs/csi.go @@ -849,18 +849,53 @@ func (p *CSIPlugin) Copy() *CSIPlugin { type CSIControllerCapability byte const ( - CSIControllerSupportsCreateDelete CSIControllerCapability = 0 - CSIControllerSupportsAttachDetach CSIControllerCapability = 1 - CSIControllerSupportsListVolumes CSIControllerCapability = 2 - CSIControllerSupportsGetCapacity CSIControllerCapability = 3 - CSIControllerSupportsCreateDeleteSnapshot CSIControllerCapability = 4 - CSIControllerSupportsListSnapshots CSIControllerCapability = 5 - CSIControllerSupportsClone CSIControllerCapability = 6 - CSIControllerSupportsReadOnlyAttach CSIControllerCapability = 7 - CSIControllerSupportsExpand CSIControllerCapability = 8 + // CSIControllerSupportsCreateDelete indicates plugin support for + // CREATE_DELETE_VOLUME + CSIControllerSupportsCreateDelete CSIControllerCapability = 0 + + // CSIControllerSupportsAttachDetach is true when the controller + // implements the methods required to attach and detach volumes. If this + // is false Nomad should skip the controller attachment flow. + CSIControllerSupportsAttachDetach CSIControllerCapability = 1 + + // CSIControllerSupportsListVolumes is true when the controller implements + // the ListVolumes RPC. NOTE: This does not guarantee that attached nodes + // will be returned unless SupportsListVolumesAttachedNodes is also true. + CSIControllerSupportsListVolumes CSIControllerCapability = 2 + + // CSIControllerSupportsGetCapacity indicates plugin support for + // GET_CAPACITY + CSIControllerSupportsGetCapacity CSIControllerCapability = 3 + + // CSIControllerSupportsCreateDeleteSnapshot indicates plugin support for + // CREATE_DELETE_SNAPSHOT + CSIControllerSupportsCreateDeleteSnapshot CSIControllerCapability = 4 + + // CSIControllerSupportsListSnapshots indicates plugin support for + // LIST_SNAPSHOTS + CSIControllerSupportsListSnapshots CSIControllerCapability = 5 + + // CSIControllerSupportsClone indicates plugin support for CLONE_VOLUME + CSIControllerSupportsClone CSIControllerCapability = 6 + + // CSIControllerSupportsReadOnlyAttach is set to true when the controller + // returns the ATTACH_READONLY capability. + CSIControllerSupportsReadOnlyAttach CSIControllerCapability = 7 + + // CSIControllerSupportsExpand indicates plugin support for EXPAND_VOLUME + CSIControllerSupportsExpand CSIControllerCapability = 8 + + // CSIControllerSupportsListVolumesAttachedNodes indicates whether the + // plugin will return attached nodes data when making ListVolume RPCs + // (plugin support for LIST_VOLUMES_PUBLISHED_NODES) CSIControllerSupportsListVolumesAttachedNodes CSIControllerCapability = 9 - CSIControllerSupportsCondition CSIControllerCapability = 10 - CSIControllerSupportsGet CSIControllerCapability = 11 + + // CSIControllerSupportsCondition indicates plugin support for + // VOLUME_CONDITION + CSIControllerSupportsCondition CSIControllerCapability = 10 + + // CSIControllerSupportsGet indicates plugin support for GET_VOLUME + CSIControllerSupportsGet CSIControllerCapability = 11 ) func (p *CSIPlugin) HasControllerCapability(cap CSIControllerCapability) bool { From 32277fc8c638d57688a868e85411cd9b151b0c18 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 30 Mar 2021 11:56:18 -0400 Subject: [PATCH 7/7] improve comment for controller caps check --- nomad/csi_endpoint.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nomad/csi_endpoint.go b/nomad/csi_endpoint.go index 09c7531e5b6..22529165c35 100644 --- a/nomad/csi_endpoint.go +++ b/nomad/csi_endpoint.go @@ -441,8 +441,9 @@ func (v *CSIVolume) controllerPublishVolume(req *structs.CSIVolumeClaimRequest, return fmt.Errorf("%s: %s", structs.ErrUnknownAllocationPrefix, req.AllocationID) } - // if no plugin was returned or the plugin doesn't attach volumes, then - // controller validation is not required and we can safely return nil. + // Some plugins support controllers for create/snapshot but not attach. So + // if there's no plugin or the plugin doesn't attach volumes, then we can + // skip the controller publish workflow and return nil. if plug == nil || !plug.HasControllerCapability(structs.CSIControllerSupportsAttachDetach) { return nil }