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

CSI: enforce usage at claim time #12112

Merged
merged 4 commits into from
Feb 24, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/12112.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
csi: Fixed a bug where the maximum number of volume claims was incorrectly enforced when an allocation claims a volume
```
8 changes: 5 additions & 3 deletions nomad/csi_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,11 @@ func TestCSIVolumeEndpoint_Claim(t *testing.T) {
// Create an initial volume claim request; we expect it to fail
// because there's no such volume yet.
claimReq := &structs.CSIVolumeClaimRequest{
VolumeID: id0,
AllocationID: alloc.ID,
Claim: structs.CSIVolumeClaimWrite,
VolumeID: id0,
AllocationID: alloc.ID,
Claim: structs.CSIVolumeClaimWrite,
AccessMode: structs.CSIVolumeAccessModeMultiNodeSingleWriter,
AttachmentMode: structs.CSIVolumeAttachmentModeFilesystem,
WriteRequest: structs.WriteRequest{
Region: "global",
Namespace: structs.DefaultNamespace,
Expand Down
54 changes: 31 additions & 23 deletions nomad/structs/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,26 @@ func (v *CSIVolume) WriteSchedulable() bool {
return false
}

// HasFreeReadClaims determines if there are any free read claims available
func (v *CSIVolume) HasFreeReadClaims() bool {
switch v.AccessMode {
case CSIVolumeAccessModeSingleNodeReader:
return len(v.ReadClaims) == 0
case CSIVolumeAccessModeSingleNodeWriter:
return len(v.ReadClaims) == 0 && len(v.WriteClaims) == 0
case CSIVolumeAccessModeUnknown:
// This volume was created but not yet claimed, so its
// capabilities have been checked in ReadSchedulable
return true
default:
// For multi-node AccessModes, the CSI spec doesn't allow for
// setting a max number of readers we track node resource
// exhaustion through v.ResourceExhausted which is checked in
// ReadSchedulable
return true
}
}

// HasFreeWriteClaims determines if there are any free write claims available
func (v *CSIVolume) HasFreeWriteClaims() bool {
switch v.AccessMode {
Expand All @@ -422,24 +442,13 @@ func (v *CSIVolume) HasFreeWriteClaims() bool {
// which is checked in WriteSchedulable
return true
case CSIVolumeAccessModeUnknown:
// this volume was created but not yet claimed, so we check what it's
// capable of, not what it's been assigned
if len(v.RequestedCapabilities) == 0 {
// COMPAT: a volume that was registered before 1.1.0 and has not
// had a change in claims could have no requested caps. It will
// get corrected on the first claim.
return true
}
for _, cap := range v.RequestedCapabilities {
switch cap.AccessMode {
case CSIVolumeAccessModeSingleNodeWriter, CSIVolumeAccessModeMultiNodeSingleWriter:
return len(v.WriteClaims) == 0
case CSIVolumeAccessModeMultiNodeMultiWriter:
return true
}
}
// This volume was created but not yet claimed, so its
// capabilities have been checked in WriteSchedulable
return true
default:
// Reader modes never have free write claims
return false
}
return false
}

// InUse tests whether any allocations are actively using the volume
Expand Down Expand Up @@ -538,6 +547,10 @@ func (v *CSIVolume) claimRead(claim *CSIVolumeClaim, alloc *Allocation) error {
return ErrCSIVolumeUnschedulable
}

if !v.HasFreeReadClaims() {
return ErrCSIVolumeMaxClaims
}

// Allocations are copy on write, so we want to keep the id but don't need the
// pointer. We'll get it from the db in denormalize.
v.ReadAllocs[claim.AllocationID] = nil
Expand Down Expand Up @@ -565,12 +578,7 @@ func (v *CSIVolume) claimWrite(claim *CSIVolumeClaim, alloc *Allocation) error {
}

if !v.HasFreeWriteClaims() {
// Check the blocking allocations to see if they belong to this job
for _, a := range v.WriteAllocs {
if a != nil && (a.Namespace != alloc.Namespace || a.JobID != alloc.JobID) {
return ErrCSIVolumeMaxClaims
}
}
return ErrCSIVolumeMaxClaims
}

// Allocations are copy on write, so we want to keep the id but don't need the
Expand Down