API docs don't include invariants for CSI volume delete

[Vickysomtee]

Nomad version

Nomad v1.6.0
BuildDate 2023-07-18T18:51:11Z
Revision 87d411f

Operating system and Environment details

Ubuntu

Issue

I am using the go sdk to successfully create Hetzner volumes for Postgres Databases on Nomad but I am unable to delete the volume from the external storage provider (Hetzner) after deregistering it from nomad.

I use the info func to get the volume

	res, _, err := client.CSIVolumes().Info(id, &nomad.QueryOptions{
		Namespace: "default",
		AuthToken: "xxxxxxxxxxxxxxxxxxxxx",
	})

Then pass the externalID gotten from the above res to the DeleteOpts func to delete the volume from the external storage provider (Hetzner)

	errr := client.CSIVolumes().DeleteOpts(&nomad.CSIVolumeDeleteRequest{
		ExternalVolumeID: res.ExternalID,
	}, &nomad.WriteOptions{
		Namespace: "default",
		AuthToken: "xxxxxxxxxxxxxxxxxxxxxxxxx",
	})

But I get this error

Unexpected response code: 500 (rpc error: volume not found: xxxxxxxxx)

[tgross]

I am unable to delete the volume from the external storage provider (Hetzner) after deregistering it from nomad.

Once you've deregistered the volume from Nomad, Nomad no longer has any ability to manipulate the volume. You'd need to re-register it before making the DeleteOpts API call. The equivalent action in the CLI is volume delete which has the following note:

The volume must still be registered with Nomad in order to be deleted. Deleting will fail if the volume is still in use by an allocation or in the process of being unpublished. If the volume no longer exists, this command will silently return without an error.

We should probably expand the API documentation to make sure that invariant is described. I'll mark this as a documentation issue but we'd also welcome a PR to improve the API docs if you're interested.

[Vickysomtee]

Once you've deregistered the volume from Nomad, Nomad no longer has any ability to manipulate the volume. You'd need to re-register it before making the DeleteOpts API call. The equivalent action in the CLI is volume delete which has the following note:

@tgross I just tried Deleting the volume from the external provider while it's still registered in nomad, and I am still caught with the same error

[tgross]

@Vickysomtee I did a little more digging and it looks like there's a bug in the API here, but not one we can easily fix without breaking backwards compatibility:

The architectural problem is this: even with the external storage provider's ID, we can't delete the volume unless we can lookup the plugin for that volume, and to do that, we need Nomad's ID for the volume. The error you're seeing comes only one place on the server csi_endpoint.go#L570, which you'll only get if the volume isn't registered. But the DeleteOpts API says:

The ID passed in the request is for the storage provider's ID, so a volume that's already been deregistered can be deleted.

But as we just saw, that's not possible because we can't lookup the plugin to send the delete RPC. So the ExternalID field we're passing from commands like nomad volume delete is not the external ID but the Nomad ID, which is why commands like volume delete work if you pass the ID Nomad gave you.

So the docstring for the DeleteOpts method is wrong, but so is the name of the field we're passing in. We should probably update that to add a new (correct) field name and deprecated (but not yet remove) the old field.

[Vickysomtee]

@tgross It took a while for me to understand your explaination so I decided to run a test and I understand it now. So I passed the volume ID for nomad instead of the external storage id and it worked. The volume got deleted from the the storage provider and deregistred from nomad.

You are right. What needs to be done is to add a new field name. Any chance I pick this up?