-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
CSI: enforce single access mode at validation time (#12337)
A volume that has single-use access mode is feasibility checked during scheduling to ensure that only a single reader or writer claim exists. However, because feasibility checking is done one alloc at a time before the plan is written, a job that's misconfigured to have count > 1 that mounts one of these volumes will pass feasibility checking. Enforce the check at validation time instead to prevent us from even trying to evaluation a job that's misconfigured this way.
- Loading branch information
Showing
5 changed files
with
157 additions
and
36 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
```release-note:bug | ||
csi: Fixed a bug where single-use access modes were not enforced during validation | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
package structs | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/hashicorp/nomad/ci" | ||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
func TestVolumeRequest_Validate(t *testing.T) { | ||
ci.Parallel(t) | ||
|
||
testCases := []struct { | ||
name string | ||
expected []string | ||
canariesCount int | ||
taskGroupCount int | ||
req *VolumeRequest | ||
}{ | ||
{ | ||
name: "host volume with empty source", | ||
expected: []string{"volume has an empty source"}, | ||
req: &VolumeRequest{ | ||
Type: VolumeTypeHost, | ||
}, | ||
}, | ||
{ | ||
name: "host volume with CSI volume config", | ||
expected: []string{ | ||
"host volumes cannot have an access mode", | ||
"host volumes cannot have an attachment mode", | ||
"host volumes cannot have mount options", | ||
"host volumes do not support per_alloc", | ||
}, | ||
req: &VolumeRequest{ | ||
Type: VolumeTypeHost, | ||
ReadOnly: false, | ||
AccessMode: CSIVolumeAccessModeSingleNodeReader, | ||
AttachmentMode: CSIVolumeAttachmentModeBlockDevice, | ||
MountOptions: &CSIMountOptions{ | ||
FSType: "ext4", | ||
MountFlags: []string{"ro"}, | ||
}, | ||
PerAlloc: true, | ||
}, | ||
}, | ||
{ | ||
name: "CSI volume multi-reader-single-writer access mode", | ||
expected: []string{ | ||
"volume with multi-node-single-writer access mode allows only one writer", | ||
}, | ||
taskGroupCount: 2, | ||
req: &VolumeRequest{ | ||
Type: VolumeTypeCSI, | ||
AccessMode: CSIVolumeAccessModeMultiNodeSingleWriter, | ||
}, | ||
}, | ||
{ | ||
name: "CSI volume single reader access mode", | ||
expected: []string{ | ||
"volume with single-node-reader-only access mode allows only one reader", | ||
}, | ||
taskGroupCount: 2, | ||
req: &VolumeRequest{ | ||
Type: VolumeTypeCSI, | ||
AccessMode: CSIVolumeAccessModeSingleNodeReader, | ||
ReadOnly: true, | ||
}, | ||
}, | ||
{ | ||
name: "CSI volume per-alloc with canaries", | ||
expected: []string{"volume cannot be per_alloc when canaries are in use"}, | ||
canariesCount: 1, | ||
req: &VolumeRequest{ | ||
Type: VolumeTypeCSI, | ||
PerAlloc: true, | ||
}, | ||
}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
tc = tc | ||
t.Run(tc.name, func(t *testing.T) { | ||
err := tc.req.Validate(tc.taskGroupCount, tc.canariesCount) | ||
for _, expected := range tc.expected { | ||
require.Contains(t, err.Error(), expected) | ||
} | ||
}) | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters