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

Volume snapshot snapshot_name seems to be mandatory in the code but optional in the cli #17828

Closed
yazdan opened this issue Jul 6, 2023 · 6 comments
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/cli theme/docs Documentation issues and enhancements theme/storage type/bug

Comments

@yazdan
Copy link
Contributor

yazdan commented Jul 6, 2023

I deployed nomad lab on my local machine with 2 node nomad cluster on vms and 3rd vm as Truenas scale.

Nomad version

Nomad v1.5.6
BuildDate 2023-05-19T18:26:13Z
Revision 8af70885c02ab921dedbdf6bc406a1e886866f80

Operating system and Environment details

  • Fedora 38 workstation as host os
  • Ubuntu server 22.04 as vm os
  • 2 node nomad cluster using kvm
  • 1 node storage using TrueNAS scale
  • Deployed and configured democratic-csi. Both controller and node jobs using nfs configuration, I followed this guide and used nfs configuration instead.
  • Created a volume
$ sudo docker images
REPOSITORY                     TAG            IMAGE ID       CREATED         SIZE
democraticcsi/democratic-csi   latest         fded1d6b6dca   3 weeks ago     319MB

$ nomad version
Nomad v1.5.6
BuildDate 2023-05-19T18:26:13Z
Revision 8af70885c02ab921dedbdf6bc406a1e886866f80

$ nomad plugin status
Container Storage Interface
ID       Provider                Controllers Healthy/Expected  Nodes Healthy/Expected
truenas  org.democratic-csi.nfs  1/1                           2/2

$ nomad volume status
Container Storage Interface
ID         Name        Plugin ID  Schedulable  Access Mode
mosquitto  mosquitto:  truenas    true         single-node-writer
test       test        truenas    true         single-node-writer

Issue

I you try to create volume snapshot without providing sanpshot_name it gives an error

$ nomad volume snapshot create test 
Error snapshotting volume: Unexpected response code: 500 (rpc error: 1 error occurred:
	* could not create snapshot: controller create snapshot: rpc error: controller create snapshot: CSI.ControllerCreateSnapshot: missing Name)

but if you provide the sanpshot_name it will create the

Reproduction steps

Enable democratic-csi and try to create a nameless snapshot

Expected Result

Snapshot is created as the sanpshot_name is optional

Actual Result

error

$ nomad volume snapshot create test 
Error snapshotting volume: Unexpected response code: 500 (rpc error: 1 error occurred:
	* could not create snapshot: controller create snapshot: rpc error: controller create snapshot: CSI.ControllerCreateSnapshot: missing Name)

Job file

not applicable

Nomad Server logs

Jul 06 19:39:20 nomad1 nomad[1613]:     2023-07-06T19:39:20.927Z [ERROR] http: request failed: method=PUT path=/v1/volumes/snapshot
Jul 06 19:39:20 nomad1 nomad[1613]:   error=
Jul 06 19:39:20 nomad1 nomad[1613]:   | rpc error: 1 error occurred:
Jul 06 19:39:20 nomad1 nomad[1613]:   | \t* could not create snapshot: controller create snapshot: CSI.ControllerCreateSnapshot: missing Name
Jul 06 19:39:20 nomad1 nomad[1613]:   |
Jul 06 19:39:20 nomad1 nomad[1613]:    code=500

PS: I checked the code and it seems that in the code name is not optional but mandatory

return errors.New("missing Name")

@yazdan
Copy link
Contributor Author

yazdan commented Jul 7, 2023

It seems in csi definition this field is not optional
https://github.com/container-storage-interface/spec/blob/master/csi.proto#L1131

@tgross
Copy link
Member

tgross commented Jul 7, 2023

Thanks for the follow-up @yazdan. This looks like a documentation miss and a bug in the CLI (it should validate the field is set before bothering to send the request).

@tgross tgross added theme/storage theme/docs Documentation issues and enhancements theme/cli labels Jul 7, 2023
@tgross tgross added this to Needs Triage in Nomad - Community Issues Triage via automation Jul 7, 2023
@tgross tgross moved this from Needs Triage to Needs Roadmapping in Nomad - Community Issues Triage Jul 7, 2023
@tgross tgross added the stage/accepted Confirmed, and intend to work on. No timeline committment though. label Jul 7, 2023
@yazdan
Copy link
Contributor Author

yazdan commented Jul 7, 2023

@tgross I can submit a PR to correct this. Please let me know

@tgross
Copy link
Member

tgross commented Jul 10, 2023

Sure, go for it!

@yazdan
Copy link
Contributor Author

yazdan commented Jul 16, 2023

PR created #17958

@tgross
Copy link
Member

tgross commented Dec 15, 2023

Doing some issue cleanup and this was fixed in #17958, which shipped in Nomad 1.6.2

@tgross tgross closed this as completed Dec 15, 2023
Nomad - Community Issues Triage automation moved this from Needs Roadmapping to Done Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/cli theme/docs Documentation issues and enhancements theme/storage type/bug
Projects
Development

No branches or pull requests

2 participants