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

Add the ability to trash an image as part of namespace deletion #953

Closed
paulrobe opened this issue Nov 15, 2024 · 12 comments · Fixed by #1036
Closed

Add the ability to trash an image as part of namespace deletion #953

paulrobe opened this issue Nov 15, 2024 · 12 comments · Fixed by #1036
Assignees

Comments

@paulrobe
Copy link

As per discussion with Aviv Caro the Vsphere Plugin team are requesting the addition of a --rbd-trash-image flag to the delete namespace grpc/cli call as a counterpart to the equivalent --rbd-create-image that we currently use in the create namespace call.

@github-project-automation github-project-automation bot moved this to 🆕 New in NVMe-oF Nov 15, 2024
@gbregman gbregman self-assigned this Nov 20, 2024
@gbregman
Copy link
Collaborator

gbregman commented Jan 9, 2025

@paulrobe the question is should the gateway delete all RBD images when a namespace is deleted or just ones created by it? While it's quite safe for the gateway to create RBD images it's a little risky to delete them. As it is now there is no way for the gateway to distinguish between images created by itself and images created externally by the user.

@paulrobe
Copy link
Author

paulrobe commented Jan 9, 2025

Just the image with the same name as the namespace that was initially created when we created the datastore/namespace. Maybe I'm ignorant but my understanding was when we create a namespace with the option "create_image" that creates an RBD image with the same name so we're just looking to add the converse functionality where we call Delete /api/nvmeof/subsystem/{nqn}/namespace/{nsid} with an equivalent"trash_image = true"which is passed on to you via a grpc call with --rbd-trash-image and then said image is moved to the trash.

Does that make sense or am I mistaken about something? (certainly possible)

@gbregman
Copy link
Collaborator

gbregman commented Jan 9, 2025

@paulrobe the image name is passed by the user so we don't have any control on it. Anyway, I'll add a record of these image names to the gateway's state so it will know to only delete them and not externally created images.

@paulrobe
Copy link
Author

paulrobe commented Jan 9, 2025

In this case the user will be the RESTAPI right so we just need to make sure that's passing the image name right?

@gbregman
Copy link
Collaborator

gbregman commented Jan 9, 2025

Yes. I guess we can add a feature to automatically generate an image name as well. In any case we'll need to at least get a pool name.

@gbregman
Copy link
Collaborator

gbregman commented Jan 9, 2025

Adding the flag in the delete command is problematic. The way the gateway update works we can't tell which flags were passed to the delete command in case the delete was a result of an update triggered by an OMAP change. We could add such information but this will complicate the code. A much simpler approach is to pass the flag when creating the namespace. The question is do we want two separate flags? One to create the image and another one trash it on delete? We could just use one flag and automatically trash any image which was created by the gateway. We could add a flag to the configuration which will either enable or disable this behavior.

@caroav
Copy link
Collaborator

caroav commented Jan 10, 2025

@gbregman in case we go with your suggestion. In Ceph, when deleting data, the user must type something like "yes I'm sure" (need to find the exact phrase being used). We need to do the same.

@paulrobe is it ok for your use case, that images that were created by the GW will always be deleted when calling the delete ns?

@gbregman
Copy link
Collaborator

@caroav we are not interactive so we can't ask the user's approval. The closest thing we have is the "--force" parameter.

@caroav
Copy link
Collaborator

caroav commented Jan 10, 2025

No need to be interactive. Just add a flag which takes a string, and this string must be "yes-I'm sure" or something like that.

@paulrobe
Copy link
Author

For us, AFAIK, the only image created by the gateway is during a namespace creation and so if we're deleting that namespace we're happy for the related image to be deleted. @caroav Is that what you mean? Only delete the image that is tied to that namespace? Or am I missing some other possibilities?

All the other images we manipulate are done via the RBD APIs not the nvmeof ones

@caroav
Copy link
Collaborator

caroav commented Jan 10, 2025

Yes that's what I mean.

gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 12, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 12, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 12, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 12, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 12, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 12, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 12, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
@gbregman
Copy link
Collaborator

See ISCE-1742

gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 13, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 14, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 14, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 14, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 14, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 14, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 14, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 14, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 15, 2025
gbregman added a commit to gbregman/ceph-nvmeof that referenced this issue Jan 15, 2025
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in NVMe-oF Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants