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

Missing ability to specify parameters/secrets in CSI CreateSnapshot #12318

Closed
iSchluff opened this issue Mar 18, 2022 · 4 comments · Fixed by #12360
Closed

Missing ability to specify parameters/secrets in CSI CreateSnapshot #12318

iSchluff opened this issue Mar 18, 2022 · 4 comments · Fixed by #12360

Comments

@iSchluff
Copy link

Proposal

Currently nomad only supports setting the volume name and snapshot name in the snapshot request.

snaps, _, err := client.CSIVolumes().CreateSnapshot(&api.CSISnapshot{
SourceVolumeID: volID,
Name: snapshotName,
}, nil)

However some plugins like ceph-csi require additional parameters/secrets to be set for the request to be valid.
https://github.com/ceph/ceph-csi/blob/d357bebbc21895cd8d01bf0f04e2db425b4b95c9/internal/cephfs/store/volumeoptions.go#L147

So nomad cli would need to implement the ability to pass these to be able to make snapshots.

Use-cases

Ability to create snapshots in ceph-csi and possibly other CSI plugins.

@tgross
Copy link
Member

tgross commented Mar 18, 2022

Hi @iSchluff! In #10840 we added the ability to use the volume's secrets for creating snapshots (that shipped in Nomad 1.1.3). But I can definitely see wanting to have separate secrets for the delete call vs the create call. Most of the plumbing to do this was reworked in #12144 which will ship in Nomad 1.3.0, so it should be fairly straightforward to allow overriding the volume's secrets in the create request as well.

@iSchluff
Copy link
Author

iSchluff commented Mar 18, 2022

Hi @tgross, thanks for your feedback, however I think this would also need to be applied for parameters.
E.g. for ceph-csi you need to pass in a clusterID parameter with every request.
Optimally these could also be reused from the volume definition, but depending on the plugin it may be necessary to also specify parameters via the cli.

@tgross
Copy link
Member

tgross commented Mar 23, 2022

I'm starting implementation on this and expect to ship it shortly.

For what it's worth, the Ceph plugin should be using unique keys for each different secret it needs, all of which are passed along. So if there's a different secret for create vs snapshot, the Ceph plugin is supposed to handle that by having a different secret that we can store with the volume:

CO SHALL permit passing through the required secrets. A CO MAY pass the same secrets to all RPCs, therefore the keys for all unique secrets that an SP expects MUST be unique across all CSI operations.

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, 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 Oct 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants