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

Nomad CSI snapshot list: do not shorten snapshot ID #10660

Closed
ggriffiths opened this issue May 26, 2021 · 4 comments · Fixed by #10664
Closed

Nomad CSI snapshot list: do not shorten snapshot ID #10660

ggriffiths opened this issue May 26, 2021 · 4 comments · Fixed by #10664

Comments

@ggriffiths
Copy link
Contributor

ggriffiths commented May 26, 2021

Nomad version

[root@ip-70-0-14-127 demo]# nomad version
Nomad v1.1.0 (2678c3604bc9530014208bc167415e167fd440fc)

Operating system and Environment details

CentOS Linux 7 (Core)

Issue

"nomad volume snapshot delete" is documented to take a snapshot id:

[root@ip-70-0-14-127 demo]# nomad volume snapshot delete 
This command takes two arguments: <plugin id> <snapshot id>
For additional help try 'nomad volume snapshot delete -help'

However, after trying a few combinations, I noticed the delete only goes through when a snapshot name is provided.

Update: After looking into this more, I found that the snapshot id provided in nomad volume snapshot list is shortened to 12 characters. I was passing this shortened ID to nomad volume snapshot delete, which was not correct. Our snapshot ids are 18 characters. By coincidence, our CSI driver supports deleting snapshots by both name and ID, which explains the deletion succeeding via snapshot name.

Unless I'm missing something, there isn't a way to find the full snapshot id via nomad volume snapshot list. Perhaps a nomad volume snapshot list -json would be useful here? Similar to nomad volume status -json?

Reproduction steps

An example like the follow could allow users to become confused as to why their snapshots aren't being deleted:

[root@ip-70-0-14-127 demo]# nomad volume snapshot create pxvol1 snap2
Snapshot ID   Volume ID     Size    Create Time           Ready?
111599152205  430708984379  10 GiB  1970-01-01T00:00:01Z  true
[root@ip-70-0-14-127 demo]# nomad volume snapshot list
Snapshot ID   Volume ID     Size    Create Time  Ready?
111599152205  430708984379  10 GiB  <none>       true
729104150236  338826921232  10 GiB  <none>       true
[root@ip-70-0-14-127 demo]# nomad volume snapshot delete portworx 111599152205
[root@ip-70-0-14-127 demo]# nomad volume snapshot list
Snapshot ID   Volume ID     Size    Create Time  Ready?
111599152205  430708984379  10 GiB  <none>       true
729104150236  338826921232  10 GiB  <none>       true
[root@ip-70-0-14-127 demo]# nomad volume snapshot delete portworx snap2
[root@ip-70-0-14-127 demo]# nomad volume snapshot list
Snapshot ID   Volume ID     Size    Create Time  Ready?
729104150236  338826921232  10 GiB  <none>       true

Expected Result

nomad volume snapshot delete docs and CLI help should reflect that this command takes a snapshot name
OR
nomad volume snapshot delete implementation should instead take a snapshot_id as documented

nomad volume snapshot list should support a -json option.

Actual Result

nomad volume snapshot delete only works when a snapshot name is passed.

Option is not supported yet

Job file (if appropriate)

n/a

Nomad Server logs (if appropriate)

n/a

Nomad Client logs (if appropriate)

n/a

@ggriffiths ggriffiths changed the title Nomad CSI snapshot delete works with snapshot name, not snapshot ID Nomad CSI snapshot list: add -json option May 26, 2021
@tgross
Copy link
Member

tgross commented May 26, 2021

Hi @ggriffiths!

After looking into this more, I found that the snapshot id provided in nomad volume snapshot list is shortened to 12 characters. I was passing this shortened ID to nomad volume snapshot delete, which was not correct. Our snapshot ids are 18 characters. By coincidence, our CSI driver supports deleting snapshots by both name and ID, which explains the deletion succeeding via snapshot name.

Yeah this is an unfortunate bug. We typically shorten IDs that come back in the Nomad CLI because it's safe to pass prefixes to Nomad. So for example, an alloc ID might be c9add7a7-af32-40fd-9ce1-26bb4dae8221 but nomad alloc status c9add7a7 works fine because our internal datastore allows us to do prefix searches. But the snapshot ID is an external identifier and we have no control over whether the CSI plugin and external storage provider supports prefix searches. So we should not be shortening the snapshot ID. Will fix.

@ggriffiths ggriffiths changed the title Nomad CSI snapshot list: add -json option Nomad CSI snapshot list: do not shorten snapshot ID or volume ID May 27, 2021
@ggriffiths
Copy link
Contributor Author

Makes sense, I've updated this issue to reflect that fix.

@ggriffiths ggriffiths changed the title Nomad CSI snapshot list: do not shorten snapshot ID or volume ID Nomad CSI snapshot list: do not shorten snapshot ID May 27, 2021
@tgross
Copy link
Member

tgross commented May 27, 2021

This will ship in the next patch version of Nomad. Thanks @ggriffiths!

@tgross tgross added this to the 1.1.1 milestone May 27, 2021
@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 19, 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.

2 participants