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: create volume capability validation #10224

Merged
merged 7 commits into from
Mar 30, 2021

Conversation

tgross
Copy link
Member

@tgross tgross commented Mar 24, 2021

Fix a collection of validation-related issues for CSI volume creation that I discovered while getting things tested end-to-end.

  • CSI: add missing structs.CSIVolume fields to api.CSIVolume

  • 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/AccessMode.

  • 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 support create/delete but we assume that it also supports attach/detach.

  • CSI: 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.

Note for reviewers:

tgross added 4 commits March 29, 2021 16:11
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`.
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.
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.
Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; left minor suggestions

nomad/csi_endpoint.go Outdated Show resolved Hide resolved
nomad/structs/csi.go Outdated Show resolved Hide resolved
Copy link
Contributor

@notnoop notnoop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. A couple of minor questions but this looks great.

nomad/csi_endpoint.go Outdated Show resolved Hide resolved
nomad/csi_endpoint.go Show resolved Hide resolved
nomad/csi_endpoint.go Outdated Show resolved Hide resolved
nomad/structs/csi.go Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – nomad March 30, 2021 15:56 Inactive
@tgross tgross merged this pull request into csi-create-volume Mar 30, 2021
@tgross tgross deleted the csi-create-volume-capability-validaton branch March 30, 2021 17:23
@tgross tgross mentioned this pull request Mar 31, 2021
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants