Hi Christian,

Thanks for reporting the bug[1] and collecting all the relevant information related to this.
Following is my reply to your queries.

On Wed, Dec 20, 2023 at 11:11 PM Christian Rohmann <christian.rohmann@inovex.de> wrote:
On 14.12.23 13:16, Erno Kuvaja wrote:
On Thu, 14 Dec 2023 at 10:28, Eugen Block <eblock@nde.ag> wrote:
Interesting, I have a kolla-ansible one-node cluster with Antelope and 
there I see what you describe as well. So the behavior did indeed 
change. I guess the docs should be updated and contain read-only rbd 
profile for glance.

This sounds like regression to me.

Indeed this is a regression and it was a wild ride following the various strings along  ...


a) Commit https://github.com/openstack/glance_store/commit/3d221ec529862d43ab303644e74ee9ad6ce8cd40 introduced the method "_snapshot_has_external_reference" to the yoga
release to fix [1]. The commit message also briefly states:

NOTE: To check this dependency glance osd needs 'read' access to
cinder and nova side RBD pool.

but there is zero mention of this requirement in the release notes for Yoga [2].


The mention is in "glance store" release notes[2] and not glance since the rbd store in glance exists in the glance store project.
After this change, we do require the read access to the cinder "volumes" pool.

 

b) The mentioned method was removed again with [4] and this change was backported to the 2023.1 release.
There again was no mention of the change to operators via the release notes, who could now remove
the read access for volumes from the Glance user again.


The patch mentioned in (a) was a "workaround" to reject requests when we try to delete an image which has dependency on a volume since
that might corrupt the volume? Not sure about the behavior but it made sense to check the dependencies and reject the request.
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.

 

c) For none of the changes a and b there was any update to the actual documentation on how to configure the glance user ceph caps.



You are correct, we should be making appropriate changes to deployment documents/tools to reflect what is currently expected out of deployers.
However, these changes were made in the glance project so I will leave it upto the glance team to comment on it.
 

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.
 

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.
 

f) @Jonathan Overriding some managed code should really just be a temporary fix (it was for Stein if I read this correctly).
Could those openstack_keys in [6], once we figured out what the caps really should be, be converted into a PR against upstream of ceph-ansible [8] to fix things at the root?


g) I am still wondering what the caps to allow reading "rbd_children" prefixed rados objects is or was used for?
Especially with the managed profiles such as "rbd" or "rbd-readonly", things should be pretty well covered.



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

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 


Thanks
Rajat Dhasmana
 



My proposal still is .. to
  * determine the correct caps (least privileges, caps via profiles where possible, ...)

  * fix the documentation and code devstack as "upstreams" first
  * write an upgrade bullet point to release notes for Caracal for operators to check and align their caps from what they might have become over the various releases
  * distribute this as a reference to the deployment tools and also the Ceph docs




Regards


Christian


    
[1] https://bugs.launchpad.net/glance-store/+bug/1954883

      [2] https://docs.openstack.org/releasenotes/glance/yoga.html#

      [3] https://review.opendev.org/q/topic:%22bug/1954883%22

      [4]
      https://review.opendev.org/q/I34dcd90a09d43127ff2e8b477750c70f3cc01113

      [5]
https://opendev.org/openstack/glance_store/src/commit/054bd5ddf5d4d255076bd5f44296f2521e899394/glance_store/_drivers/rbd.py#L455

      [6]
https://opendev.org/openstack/openstack-ansible/commit/0f92985608c0f6ff941ea0445ae25eab20e94fb4

      [7]
https://opendev.org/openstack/devstack-plugin-ceph/src/commit/4c22c3d0905589d676bf4865ca5cf57994eb426d/devstack/lib/ceph#L712

      [8]
https://github.com/ceph/ceph-ansible/blob/b6102975549d8f870b0c20a01edda59d6ceac422/group_vars/all.yml.sample#L642