)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"eab827a696f3fcae7d85c4917ac0e0dcf37dd28b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"0dd91ce7_0a61bfb5","updated":"2023-09-20 22:41:19.000000000","message":"Thanks for taking a look! Replied inline.","commit_id":"8e974c9d524f54a73a75b676a904d622257716de"}],"devstack/lib/ceph":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b9e7a93e8e2350c9da5e65af79f10821470047f2","unresolved":true,"context_lines":[{"line_number":126,"context_line":"# Set ``CINDER_CEPH_POOL_PER_BACKEND`` to True to configure Ceph to use a"},{"line_number":127,"context_line":"# separate pool per Ceph backend in CINDER_ENABLED_BACKENDS."},{"line_number":128,"context_line":"CINDER_CEPH_POOL_PER_BACKEND\u003d$(trueorfalse False CINDER_CEPH_POOL_PER_BACKEND)"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"# Rados gateway"},{"line_number":131,"context_line":"CEPH_RGW_PORT\u003d${CEPH_RGW_PORT:-8080}"},{"line_number":132,"context_line":"CEPH_RGW_IDENTITY_API_VERSION\u003d${CEPH_RGW_IDENTITY_API_VERSION:-3}"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"9af39871_fce4ef00","line":129,"updated":"2023-09-16 21:53:54.000000000","message":"Note to reviewers: I took this approach in an effort to minimize the potential for \"incorrect config\" and make it simple to understand for users.\n\nInitially I had instead something like `CINDER_CEPH_POOL_NUM` meaning how many Ceph pools to create, but that has the disadvantage of being about to go out of sync with the number of Ceph backends in `CINDER_ENABLED_BACKENDS`. I didn\u0027t want users to have to make sure `CINDER_ENABLED_BACKENDS` and `CINDER_CEPH_POOL_NUM` match.\n\nSo with this the user can indicate whether they would like a separate Ceph pool per Ceph backend in `CINDER_ENABLED_BACKENDS` or if they would prefer for all the backends to share the same `CINDER_CEPH_POOL` (current behavior).","commit_id":"8e974c9d524f54a73a75b676a904d622257716de"}],"devstack/lib/cephadm":[{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"47dac94a8658215268bd1101db8eace52a0e9fb9","unresolved":true,"context_lines":[{"line_number":646,"context_line":"    fi"},{"line_number":647,"context_line":""},{"line_number":648,"context_line":"    if is_ceph_enabled_for_service cinder; then"},{"line_number":649,"context_line":"        if [[ \"$CINDER_CEPH_POOL_PER_BACKEND\" \u003d\u003d \"False\" ]]; then"},{"line_number":650,"context_line":"            POOLS+\u003d($CINDER_CEPH_POOL)"},{"line_number":651,"context_line":"        else"},{"line_number":652,"context_line":"            local be be_name be_type"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"0c2c28ee_52f2f7fc","line":649,"updated":"2023-09-19 07:14:19.000000000","message":"I would have simplified this piece by having:\n1. a cinder pool that is always created by default (e.g. volumes)\n2. a list of additional pools defined as per the syntax you proposed\n\nThis way we wouldn\u0027t have defined a new variable for the purpose of providing this  behavior, and by just providing additional pools we can achieve the same result.\n\ne.g.\n\n```\nCINDER_CEPH_POOL\u003d\"volume\"\nCINDER_ADDITIONAL_BACKENDS\u003dceph:volume2,ceph:volume3\n\nif is_ceph_enabled_for_service cinder; then\n  POOLS+\u003d($CINDER_CEPH_POOL)\n  # here the logic of parsing additional pools\n  for be in ${CINDER_ENABLED_BACKENDS//,/ }; do\n    be_type\u003d${be%%:*}\n    be_name\u003d${be##*:}\n    if [[ \"$be_type\" \u003d\u003d \"ceph\" ]]; then\n      POOLS+\u003d($be_name)\n      iniset $CINDER_CONF $be_name rbd_pool $be_name\n    fi\n  done\nfi\n```\n\nHowever, I\u0027m not sure the variable `CINDER_ENABLED_BACKENDS` is used elsewhere, so what you did makes sense in case that variable is used by Cinder to provide the whole list of backends.\nNot blocking this change as long as it\u0027s tested and works as expected.","commit_id":"8e974c9d524f54a73a75b676a904d622257716de"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"eab827a696f3fcae7d85c4917ac0e0dcf37dd28b","unresolved":true,"context_lines":[{"line_number":646,"context_line":"    fi"},{"line_number":647,"context_line":""},{"line_number":648,"context_line":"    if is_ceph_enabled_for_service cinder; then"},{"line_number":649,"context_line":"        if [[ \"$CINDER_CEPH_POOL_PER_BACKEND\" \u003d\u003d \"False\" ]]; then"},{"line_number":650,"context_line":"            POOLS+\u003d($CINDER_CEPH_POOL)"},{"line_number":651,"context_line":"        else"},{"line_number":652,"context_line":"            local be be_name be_type"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"e1f99265_59d86c20","line":649,"in_reply_to":"0c2c28ee_52f2f7fc","updated":"2023-09-20 22:41:19.000000000","message":"You are correct, `CINDER_ENABLED_BACKENDS` is used by Cinder to provide the entire list of backends:\n\nhttps://github.com/openstack/devstack/blob/d3953db76641e825565390acc6f68501777c0f53/lib/cinder#L98\n\nand you set `CINDER_ENABLED_BACKENDS` in the Zuul job definition, this is the example from a tempest change I\u0027m working on:\n\nhttps://review.opendev.org/c/openstack/tempest/+/890360/19/zuul.d/project.yaml#6\n\nThis ^ is also how I have tested that it works.\n\nAnd these are a couple of examples from Cinder jobs:\n\nhttps://review.opendev.org/c/openstack/tempest/+/890360/19/zuul.d/project.yaml#6\n\nhttps://github.com/openstack/cinder/blob/8a9f9a7c36419ee503eb8f3ed4bed60767e9eb6b/.zuul.yaml#L358\n\nSo ultimately I\u0027m trying to add a way to get separate Ceph pools per Ceph backend in a way that would fit in with `CINDER_ENABLED_BACKENDS`.","commit_id":"8e974c9d524f54a73a75b676a904d622257716de"}]}