Later a "fix"**was introduced to move the image into trash where it will get deleted eventually when the dependencies get deleted but we will succeed with the image delete operation which is the goal here. Yes that fix is the trash feature of [cr1]. In the commit message of [cr1] it's written "This trash must be purged by a scheduled rbd trash purge operation outside of Glance.", but the release notes
Rajat, thanks for diving into my wall of text. Honestly I thought we agreed during cinder weekly that the bug [1] was the best place to work on correcting any caps for the (potentially) affected projects. But be it as it may, discussions on the different aspects I brought up might more likely happen here in the ML anyways. On 30.01.24 19:10, Rajat Dhasmana wrote: then say this happens automatically. Which is correct? [cr1] https://review.opendev.org/c/openstack/glance_store/+/884524
e) The "_snapshot_has_external_reference" method is currently just dangling and unused [5].
Yes, I think we forgot to remove it in the patch that removes the "workaround" code and introduces the "fix" code. Looks like we can go ahead and remove that method. I pushed a change to: https://review.opendev.org/c/openstack/glance_store/+/907317
d) Adding to c, devstack very much is out of sync to what would currently be considered "correct" in regards to caps [7]. Too liberal caps / ACLs are not helpful when testing for regressions.
Correct again, devstack is giving out permissions too leniently which might not be desirable for an actual deployment. However, devstack setups are used for development and not production environments so I wouldn't be too inclined on devstack making any changes. I beg to disagree. Since devstack is used as target for all sorts of (integration) tests, the alignment of the access permissions makes sense. It's kinda like running everything as "root" and saying "it's only for testing"... how would you then notice any permission related issues? Using the same permission is devstack as (to be) noted in the documentation is crucial to them being "somewhat" correct.
From a cinder standpoint, I think the following permissions apply for OSD: (I'm not familiar with permissions required for monitor and manger)
cinder user -> for OSD: rwx in "volumes" pool, r in "images" pool, (I don't think we need any permissions in the "vms" pool but somehow the deployment tools configure it that way, cinder/nova folks can correct me here) cinder-backup user: for OSD: rwx in "backups" pool, r in "volumes" pool
This is one of my major drivers for starting all this fuzz in the first place .... Apart from the access levels "read" or "read-write" on the different pools, let me note again, that these plain caps are NOT recommended (anymore). Not using the managed "profiles" such as "rbd" or "rbd-readonly" instead of raw ACLs such das "rwx", does have side effects, see [cr3]. This makes a big difference as "such privileges include the ability to blocklist other client users.", required for lock of stale RBD clients to be removed from images, see [cr4]. We already ran into this with images having stale locks and that is also why deployment tools do use profiles. [cr3] https://docs.ceph.com/en/latest/rados/operations/user-management/#authorizat... [cr4] https://docs.ceph.com/en/latest/rbd/rbd-exclusive-locks/#rbd-exclusive-locks.
The reason requiring access to other pools is: 1. cinder user requires read access in the "images" pool since we perform COW cloning when we create a bootable volume from image 2. cinder-backup user requires read access in the "volumes" pool since creating a backup of a volume requires reading the volume from the "volumes" pool
If there are other permissions required or other cases where we need access to multiple pools, I'm happy to be corrected here.
[1] https://bugs.launchpad.net/nova/+bug/2051244 [2] https://docs.openstack.org/releasenotes/glance_store/yoga.html#upgrade-notes
There is https://review.opendev.org/c/openstack/cinder/+/809523 which tries to improve things. This might also have an impact on required caps? Regards Christian