)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Fix: Fix `ceph orch` command with quadruple arg `ceph orch apply rgw default default default default` (2 are supported)."},{"line_number":10,"context_line":"- Fix: Make the script correctly use overriden Ceph release version."},{"line_number":11,"context_line":"- Fix: Add ceph dependent args to make installation work with all supported ceph versions (suspport v15 to v20)."},{"line_number":12,"context_line":"- Fix: Compatibility for ceph octopus - symlink python to /usr/libexec/platform-python."},{"line_number":13,"context_line":"- Fix: Correct the swift endpoint URL to include `AUTH_\u003cproject_id\u003e` in path (required when `rgw_swift_account_in_url` is `true` which it is)."},{"line_number":14,"context_line":"- Fix: Configure `rgw_keystone_api_version` only for octopus where it is supported."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"dd64d11d_8d6dc792","line":11,"range":{"start_line":11,"start_character":100,"end_line":11,"end_character":112},"updated":"2026-05-19 22:53:22.000000000","message":"Hmmm, \n\n\nOctopus (v15) was EOL on 2022-08-09. There\u0027s no need to add new code\nto support it. I would drop all octopus-specific logic from this\nchange: https://docs.ceph.com/en/latest/releases/#active-releases","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Fix: Fix `ceph orch` command with quadruple arg `ceph orch apply rgw default default default default` (2 are supported)."},{"line_number":10,"context_line":"- Fix: Make the script correctly use overriden Ceph release version."},{"line_number":11,"context_line":"- Fix: Add ceph dependent args to make installation work with all supported ceph versions (suspport v15 to v20)."},{"line_number":12,"context_line":"- Fix: Compatibility for ceph octopus - symlink python to /usr/libexec/platform-python."},{"line_number":13,"context_line":"- Fix: Correct the swift endpoint URL to include `AUTH_\u003cproject_id\u003e` in path (required when `rgw_swift_account_in_url` is `true` which it is)."},{"line_number":14,"context_line":"- Fix: Configure `rgw_keystone_api_version` only for octopus where it is supported."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"77cfb1a6_f06b68c5","line":11,"range":{"start_line":11,"start_character":91,"end_line":11,"end_character":99},"updated":"2026-05-19 22:53:22.000000000","message":"nit: typo","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Fix: Fix `ceph orch` command with quadruple arg `ceph orch apply rgw default default default default` (2 are supported)."},{"line_number":10,"context_line":"- Fix: Make the script correctly use overriden Ceph release version."},{"line_number":11,"context_line":"- Fix: Add ceph dependent args to make installation work with all supported ceph versions (suspport v15 to v20)."},{"line_number":12,"context_line":"- Fix: Compatibility for ceph octopus - symlink python to /usr/libexec/platform-python."},{"line_number":13,"context_line":"- Fix: Correct the swift endpoint URL to include `AUTH_\u003cproject_id\u003e` in path (required when `rgw_swift_account_in_url` is `true` which it is)."},{"line_number":14,"context_line":"- Fix: Configure `rgw_keystone_api_version` only for octopus where it is supported."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"40acd3d5_3c029738","line":11,"range":{"start_line":11,"start_character":91,"end_line":11,"end_character":99},"in_reply_to":"77cfb1a6_f06b68c5","updated":"2026-05-30 14:31:18.000000000","message":"Thank you, fixed.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Fix: Fix `ceph orch` command with quadruple arg `ceph orch apply rgw default default default default` (2 are supported)."},{"line_number":10,"context_line":"- Fix: Make the script correctly use overriden Ceph release version."},{"line_number":11,"context_line":"- Fix: Add ceph dependent args to make installation work with all supported ceph versions (suspport v15 to v20)."},{"line_number":12,"context_line":"- Fix: Compatibility for ceph octopus - symlink python to /usr/libexec/platform-python."},{"line_number":13,"context_line":"- Fix: Correct the swift endpoint URL to include `AUTH_\u003cproject_id\u003e` in path (required when `rgw_swift_account_in_url` is `true` which it is)."},{"line_number":14,"context_line":"- Fix: Configure `rgw_keystone_api_version` only for octopus where it is supported."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"e3989685_6ec95708","line":11,"range":{"start_line":11,"start_character":100,"end_line":11,"end_character":112},"in_reply_to":"dd64d11d_8d6dc792","updated":"2026-05-30 14:31:18.000000000","message":"Hello, I agree. \n\nI edited the PR to support last 3 versions as documented in active releases. I removed all workarounds for older versions and updated all parts related to that. From my side resolved.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d3592b834c119d43250866e85d6bb2166a30960f","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Fix: Fix `ceph orch` command with quadruple arg `ceph orch apply rgw default default default default` (2 are supported)."},{"line_number":10,"context_line":"- Fix: Make the script correctly use overriden Ceph release version."},{"line_number":11,"context_line":"- Fix: Add ceph dependent args to make installation work with all supported ceph versions (suspport v15 to v20)."},{"line_number":12,"context_line":"- Fix: Compatibility for ceph octopus - symlink python to /usr/libexec/platform-python."},{"line_number":13,"context_line":"- Fix: Correct the swift endpoint URL to include `AUTH_\u003cproject_id\u003e` in path (required when `rgw_swift_account_in_url` is `true` which it is)."},{"line_number":14,"context_line":"- Fix: Configure `rgw_keystone_api_version` only for octopus where it is supported."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"ec735d9d_ade96116","line":11,"range":{"start_line":11,"start_character":100,"end_line":11,"end_character":112},"in_reply_to":"e3989685_6ec95708","updated":"2026-06-04 04:24:34.000000000","message":"Done","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"9ec64f792712e8d208ca2371ed7819a8752efcae","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"22cb96d7_e3c53244","updated":"2026-03-05 10:58:57.000000000","message":"Hello,\n\nthis is my first PR for Openstack project.\n\nI am maintainer of Velero plugin for Openstack. I wanted to set up a pipeline that would test the backups against Openstack with Ceph backend that includes Rados Gateway. You can see how it runs here - https://github.com/Lirt/velero-plugin-for-openstack/blob/master/.github/workflows/tests.yaml#L209.\n\nTo test this PR, you can use example config file with my Github fork by using this:\n\n```bash\nGIT_BASE\u003dhttps://github.com\nenable_plugin devstack-plugin-ceph https://github.com/Lirt/devstack-plugin-ceph.git\n```\n\nAfter I tried the devstack-plugin-ceph I realized there are critical bugs and that the RGW code path was probably not maintained for some time.\n\nMy code changes were tested in Ubuntu 24.04 VM with success.\n\nI would personally suggest to create more example configs, either smaller ones that bootstrap only critical ceph components, then one with RGW, Manila and test all of them with multiple Ceph versions.\n\nI noted in the documentation that versions v15 up to v20 are supported. I made that myself but I actually had to test with v15 and this one works as well.\n\nI don\u0027t know how the tests are being done for this project or how to run them at the moment. Although when I look now probably the `.zuul.yaml` is the source.\n\nI can help with this in another PRs if you let me know how and what is expected.","commit_id":"8ad6e202a503df689ebba9a68d7383a80ddbb906"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"202ab024_230bcab0","updated":"2026-05-19 22:53:22.000000000","message":"Thanks for working on this; several comments inline","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"aa00faff_6c9a566f","updated":"2026-05-30 14:31:18.000000000","message":"Hi, thank you very much for review. I addressed all the comments and retested with all 3 ceph versions confirming my example config works well.\n\nI dropped legacy ceph version support and kept only 3 supported versions.\n\nLet me know what do you think.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"8c0d54f47ab3372c0a15f3d913f4a494726fa84f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"df9aad00_2186bfd7","updated":"2026-06-05 12:40:46.000000000","message":"I think this is a good improvement, thanks for that!\nThis patch needs a rebase and I have a few comments associated to some code optimization we can do to make it more solid.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"8681837aed897a2033c0641927b7b3cfee353746","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"808d12be_465c4860","updated":"2026-06-04 04:34:33.000000000","message":"recheck\n\nvoting job timed out","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"ee842dfb0ea8f1edb65494dd6f6b6d3b160a641d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0a6cecf9_759070fe","in_reply_to":"df06b504_a1ec3e4d","updated":"2026-06-18 13:01:05.000000000","message":"I rebased the PR.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"2dd2054012703509b0d67f667f659225ba2db01f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"df06b504_a1ec3e4d","in_reply_to":"df9aad00_2186bfd7","updated":"2026-06-18 12:50:12.000000000","message":"Thank you for another review and sorry for my delay.\n\nI replied to all questions. I will need more feedback to know which way to go.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"96e95aca0cd1879da81b07774f18f16d6f54c173","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"59acdffb_6a1c7d2e","updated":"2026-06-25 05:24:12.000000000","message":"It took a bit for me to recall what we discussed in this patch. There are two\nmain discussions: \n1. how we get cephadm: this part can definitely be improved, but we can keep as it is because it worked for year, so not a real concern \n2. there\u0027s a comment I left that remains not addressed, and it would avoid an additional restart of the rgw daemon.  Because the concern is not a blocker, I\u0027m going to land this patch and we can eventually follow up to optimize/improve/address other parts.","commit_id":"0e147a8d715fd2e0a496003801c16e1a4b8d7a1c"}],"README.rst":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":80,"context_line":"    ENABLE_CEPH_RGW\u003dTrue"},{"line_number":81,"context_line":"    CEPH_RGW_PORT\u003d\"8080\""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Ceph requires passwordless SSH access to the root user on the machine to deploy Rados Gateway using ``ceph orch apply rgw`` command."},{"line_number":84,"context_line":"After you created stack user using `sudo ./tools/create-stack-user.sh` you can run following to create passwordless SSH access for the root user on localhost."},{"line_number":85,"context_line":"Be aware that this will allow root login over SSH which is considered unsafe if your machine is reachable from the Internet or other public networks."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"d870f3ce_234b2910","line":83,"range":{"start_line":83,"start_character":0,"end_line":83,"end_character":132},"updated":"2026-05-19 22:53:22.000000000","message":"This isn\u0027t isolated to RGW.. most \"ceph orch\" commands need passwordless ssh access. The bootstrap commands should take care of this. We generate a keypair and install it in the authorized_keys file. \n\nThe steps below would be needed only if  needed if the system has `PermitRootLogin no` in sshd_config. If that\u0027s the case, all aspects of the ceph deployment would fail, not just RGW. \n\nI suggest dropping this entire section, or, adding a generic note in the Known issues section: `PermitRootLogin prohibit-password` (the default on most distros) is sufficient.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d3592b834c119d43250866e85d6bb2166a30960f","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    ENABLE_CEPH_RGW\u003dTrue"},{"line_number":81,"context_line":"    CEPH_RGW_PORT\u003d\"8080\""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Ceph requires passwordless SSH access to the root user on the machine to deploy Rados Gateway using ``ceph orch apply rgw`` command."},{"line_number":84,"context_line":"After you created stack user using `sudo ./tools/create-stack-user.sh` you can run following to create passwordless SSH access for the root user on localhost."},{"line_number":85,"context_line":"Be aware that this will allow root login over SSH which is considered unsafe if your machine is reachable from the Internet or other public networks."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"d282b6a3_6066b2c4","line":83,"range":{"start_line":83,"start_character":0,"end_line":83,"end_character":132},"in_reply_to":"69602413_2b733fcd","updated":"2026-06-04 04:24:34.000000000","message":"Done","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":true,"context_lines":[{"line_number":80,"context_line":"    ENABLE_CEPH_RGW\u003dTrue"},{"line_number":81,"context_line":"    CEPH_RGW_PORT\u003d\"8080\""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"Ceph requires passwordless SSH access to the root user on the machine to deploy Rados Gateway using ``ceph orch apply rgw`` command."},{"line_number":84,"context_line":"After you created stack user using `sudo ./tools/create-stack-user.sh` you can run following to create passwordless SSH access for the root user on localhost."},{"line_number":85,"context_line":"Be aware that this will allow root login over SSH which is considered unsafe if your machine is reachable from the Internet or other public networks."},{"line_number":86,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"69602413_2b733fcd","line":83,"range":{"start_line":83,"start_character":0,"end_line":83,"end_character":132},"in_reply_to":"d870f3ce_234b2910","updated":"2026-05-30 14:31:18.000000000","message":"Updated as you suggested.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"}],"devstack/lib/cephadm":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"# DEFAULT OPTIONS"},{"line_number":30,"context_line":"ATTEMPTS\u003d30"},{"line_number":31,"context_line":"CEPH_CONTAINER_IMAGE\u003d${CEPH_CONTAINER_IMAGE:-\u0027quay.io/ceph/ceph:v20\u0027}"},{"line_number":32,"context_line":"DEVICES\u003d()"},{"line_number":33,"context_line":"if [[ \"$REMOTE_CEPH\" \u003d \"False\" ]]; then"},{"line_number":34,"context_line":"    FSID\u003d$(uuidgen)"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"1c04c84e_a8d0a370","line":31,"range":{"start_line":31,"start_character":0,"end_line":31,"end_character":20},"updated":"2026-05-19 22:53:22.000000000","message":"I\u0027m okay with the renaming here, but, you\u0027ll break anyone that\u0027s using \"CONTAINER_IMAGE\" at the moment. I suggest supporting CONTAINER_IMAGE as a fall back.\n\n\n```\nCEPH_CONTAINER_IMAGE\u003d${CEPH_CONTAINER_IMAGE:-${CONTAINER_IMAGE:-\u0027quay.io/ceph/ceph:v20\u0027}}\n```","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":true,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"# DEFAULT OPTIONS"},{"line_number":30,"context_line":"ATTEMPTS\u003d30"},{"line_number":31,"context_line":"CEPH_CONTAINER_IMAGE\u003d${CEPH_CONTAINER_IMAGE:-\u0027quay.io/ceph/ceph:v20\u0027}"},{"line_number":32,"context_line":"DEVICES\u003d()"},{"line_number":33,"context_line":"if [[ \"$REMOTE_CEPH\" \u003d \"False\" ]]; then"},{"line_number":34,"context_line":"    FSID\u003d$(uuidgen)"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"e346604d_d239a902","line":31,"range":{"start_line":31,"start_character":0,"end_line":31,"end_character":20},"in_reply_to":"1c04c84e_a8d0a370","updated":"2026-05-30 14:31:18.000000000","message":"True. I decided to rollback breaking changes (this and rename of CEPH_RGW_PORT) in order to keep this PR focused on fixing the RGW path plus allow to install 3 supported releases. The rename of the variable is not important, I just felt it is more explicit. It can be renamed in next PR. Let me know what do you think.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d3592b834c119d43250866e85d6bb2166a30960f","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"# DEFAULT OPTIONS"},{"line_number":30,"context_line":"ATTEMPTS\u003d30"},{"line_number":31,"context_line":"CEPH_CONTAINER_IMAGE\u003d${CEPH_CONTAINER_IMAGE:-\u0027quay.io/ceph/ceph:v20\u0027}"},{"line_number":32,"context_line":"DEVICES\u003d()"},{"line_number":33,"context_line":"if [[ \"$REMOTE_CEPH\" \u003d \"False\" ]]; then"},{"line_number":34,"context_line":"    FSID\u003d$(uuidgen)"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"8f7de823_c7e2d457","line":31,"range":{"start_line":31,"start_character":0,"end_line":31,"end_character":20},"in_reply_to":"e346604d_d239a902","updated":"2026-06-04 04:24:34.000000000","message":"Done","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":42,"context_line":"SLEEP\u003d5"},{"line_number":43,"context_line":"CEPHADM_DEV_OSD\u003d${CEPHADM_DEV_OSD:-\"True\"}"},{"line_number":44,"context_line":"CEPH_LOOPBACK_DISK_SIZE\u003d${CEPH_LOOPBACK_DISK_SIZE:-30G}"},{"line_number":45,"context_line":"CEPH_RGW_PORT\u003d${CEPH_RGW_PORT:-}"},{"line_number":46,"context_line":"TARGET_DEV_OSD_DIR\u003d${TARGET_DEV_OSD_DIR:-\"/opt/stack\"}"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"# POOLS"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"dc4f79c3_289b80a2","line":45,"range":{"start_line":45,"start_character":31,"end_line":45,"end_character":32},"updated":"2026-05-19 22:53:22.000000000","message":"Would 8080 be a good default?","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d3592b834c119d43250866e85d6bb2166a30960f","unresolved":false,"context_lines":[{"line_number":42,"context_line":"SLEEP\u003d5"},{"line_number":43,"context_line":"CEPHADM_DEV_OSD\u003d${CEPHADM_DEV_OSD:-\"True\"}"},{"line_number":44,"context_line":"CEPH_LOOPBACK_DISK_SIZE\u003d${CEPH_LOOPBACK_DISK_SIZE:-30G}"},{"line_number":45,"context_line":"CEPH_RGW_PORT\u003d${CEPH_RGW_PORT:-}"},{"line_number":46,"context_line":"TARGET_DEV_OSD_DIR\u003d${TARGET_DEV_OSD_DIR:-\"/opt/stack\"}"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"# POOLS"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"ff2d8009_689bfae7","line":45,"range":{"start_line":45,"start_character":31,"end_line":45,"end_character":32},"in_reply_to":"2a1aad7b_fb68c39e","updated":"2026-06-04 04:24:34.000000000","message":"Done","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":true,"context_lines":[{"line_number":42,"context_line":"SLEEP\u003d5"},{"line_number":43,"context_line":"CEPHADM_DEV_OSD\u003d${CEPHADM_DEV_OSD:-\"True\"}"},{"line_number":44,"context_line":"CEPH_LOOPBACK_DISK_SIZE\u003d${CEPH_LOOPBACK_DISK_SIZE:-30G}"},{"line_number":45,"context_line":"CEPH_RGW_PORT\u003d${CEPH_RGW_PORT:-}"},{"line_number":46,"context_line":"TARGET_DEV_OSD_DIR\u003d${TARGET_DEV_OSD_DIR:-\"/opt/stack\"}"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"# POOLS"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"2a1aad7b_fb68c39e","line":45,"range":{"start_line":45,"start_character":31,"end_line":45,"end_character":32},"in_reply_to":"dc4f79c3_289b80a2","updated":"2026-05-30 14:31:18.000000000","message":"I removed this variable. Similar reason as for the CEPH_CONTAINER_IMAGE. This was only to make the variable name more explicit but it\u0027s not important. \n\nAdditionally I let the `RGW_PORT` to be overridable from the configuration which will allow to fix potential port clashes.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    local version_dependent_args\u003d\"\""},{"line_number":213,"context_line":"    CEPH_RELEASE\u003d$(_get_ceph_codename)"},{"line_number":214,"context_line":"    if [ \"${CEPH_RELEASE}\" \u003d\u003d \"octopus\" ]; then"},{"line_number":215,"context_line":"        # Older ceph versions use shebang /usr/libexec/platform-python which"},{"line_number":216,"context_line":"        # doesn\u0027t exist in the modern operating systems anymore."},{"line_number":217,"context_line":"        sudo ln -sf /usr/bin/python3 /usr/libexec/platform-python"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"decc3baa_2caa34a9","line":214,"range":{"start_line":214,"start_character":4,"end_line":214,"end_character":47},"updated":"2026-05-19 22:53:22.000000000","message":"octopus is too old, hoping we can drop this..","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"fae8f22a3785d0735d8ff9d86a21136ad90b5766","unresolved":false,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    local version_dependent_args\u003d\"\""},{"line_number":213,"context_line":"    CEPH_RELEASE\u003d$(_get_ceph_codename)"},{"line_number":214,"context_line":"    if [ \"${CEPH_RELEASE}\" \u003d\u003d \"octopus\" ]; then"},{"line_number":215,"context_line":"        # Older ceph versions use shebang /usr/libexec/platform-python which"},{"line_number":216,"context_line":"        # doesn\u0027t exist in the modern operating systems anymore."},{"line_number":217,"context_line":"        sudo ln -sf /usr/bin/python3 /usr/libexec/platform-python"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"9f745180_b32e20f1","line":214,"range":{"start_line":214,"start_character":4,"end_line":214,"end_character":47},"in_reply_to":"2bf39eab_b64dc21b","updated":"2026-06-19 02:28:13.000000000","message":"Done","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":true,"context_lines":[{"line_number":211,"context_line":""},{"line_number":212,"context_line":"    local version_dependent_args\u003d\"\""},{"line_number":213,"context_line":"    CEPH_RELEASE\u003d$(_get_ceph_codename)"},{"line_number":214,"context_line":"    if [ \"${CEPH_RELEASE}\" \u003d\u003d \"octopus\" ]; then"},{"line_number":215,"context_line":"        # Older ceph versions use shebang /usr/libexec/platform-python which"},{"line_number":216,"context_line":"        # doesn\u0027t exist in the modern operating systems anymore."},{"line_number":217,"context_line":"        sudo ln -sf /usr/bin/python3 /usr/libexec/platform-python"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"2bf39eab_b64dc21b","line":214,"range":{"start_line":214,"start_character":4,"end_line":214,"end_character":47},"in_reply_to":"decc3baa_2caa34a9","updated":"2026-05-30 14:31:18.000000000","message":"Dropped.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":222,"context_line":"    if [ -z \"$cluster\" ]; then"},{"line_number":223,"context_line":"        local boot_attempts\u003d3"},{"line_number":224,"context_line":"        while [ \"$boot_attempts\" -ne 0 ]; do"},{"line_number":225,"context_line":"            if $SUDO \"$CEPHADM\" --image \"$CONTAINER_IMAGE\" \\"},{"line_number":226,"context_line":"                bootstrap \\"},{"line_number":227,"context_line":"                --fsid $FSID \\"},{"line_number":228,"context_line":"                --config \"$BOOTSTRAP_CONFIG\" \\"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"270113c0_9bb4fbc1","line":225,"range":{"start_line":225,"start_character":42,"end_line":225,"end_character":57},"updated":"2026-05-19 22:53:22.000000000","message":"See this usage?","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":true,"context_lines":[{"line_number":222,"context_line":"    if [ -z \"$cluster\" ]; then"},{"line_number":223,"context_line":"        local boot_attempts\u003d3"},{"line_number":224,"context_line":"        while [ \"$boot_attempts\" -ne 0 ]; do"},{"line_number":225,"context_line":"            if $SUDO \"$CEPHADM\" --image \"$CONTAINER_IMAGE\" \\"},{"line_number":226,"context_line":"                bootstrap \\"},{"line_number":227,"context_line":"                --fsid $FSID \\"},{"line_number":228,"context_line":"                --config \"$BOOTSTRAP_CONFIG\" \\"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"5ed2504a_01853c75","line":225,"range":{"start_line":225,"start_character":42,"end_line":225,"end_character":57},"in_reply_to":"270113c0_9bb4fbc1","updated":"2026-05-30 14:31:18.000000000","message":"Yes, related to previous comments I kept the variable name `CONTAINER_IMAGE` the same.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"fae8f22a3785d0735d8ff9d86a21136ad90b5766","unresolved":false,"context_lines":[{"line_number":222,"context_line":"    if [ -z \"$cluster\" ]; then"},{"line_number":223,"context_line":"        local boot_attempts\u003d3"},{"line_number":224,"context_line":"        while [ \"$boot_attempts\" -ne 0 ]; do"},{"line_number":225,"context_line":"            if $SUDO \"$CEPHADM\" --image \"$CONTAINER_IMAGE\" \\"},{"line_number":226,"context_line":"                bootstrap \\"},{"line_number":227,"context_line":"                --fsid $FSID \\"},{"line_number":228,"context_line":"                --config \"$BOOTSTRAP_CONFIG\" \\"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"21d97473_cdb07989","line":225,"range":{"start_line":225,"start_character":42,"end_line":225,"end_character":57},"in_reply_to":"5ed2504a_01853c75","updated":"2026-06-19 02:28:13.000000000","message":"Done","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":380,"context_line":"        --keyring $CEPH_KEYRING -- ceph fs volume create \"$FSNAME\""},{"line_number":381,"context_line":"}"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"# Get Ceph version"},{"line_number":384,"context_line":"function _get_ceph_version {"},{"line_number":385,"context_line":"    local ceph_version_str"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CEPH_CONTAINER_IMAGE \\"},{"line_number":388,"context_line":"        --version | awk \u0027{ print $3 }\u0027)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    echo $ceph_version_str"},{"line_number":391,"context_line":"}"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"# Get Ceph release codename"},{"line_number":394,"context_line":"function _get_ceph_codename {"},{"line_number":395,"context_line":"    local ceph_version_str"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CEPH_CONTAINER_IMAGE \\"},{"line_number":398,"context_line":"        --version | awk \u0027{ print $5 }\u0027)"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"    echo $ceph_version_str"},{"line_number":401,"context_line":"}"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"function _install_and_configure_clustered_nfs {"},{"line_number":404,"context_line":"    echo \"[CEPHADM] Deploy nfs.$FSNAME backend\""}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"7eb86262_5ea0fbe8","line":401,"range":{"start_line":383,"start_character":0,"end_line":401,"end_character":1},"updated":"2026-05-19 22:53:22.000000000","message":"Feels like we\u0027re calling these too many times. Maybe we can optimize and save the return value into a variable. \n\nJust an optimization if you care to make it","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"2dd2054012703509b0d67f667f659225ba2db01f","unresolved":true,"context_lines":[{"line_number":380,"context_line":"        --keyring $CEPH_KEYRING -- ceph fs volume create \"$FSNAME\""},{"line_number":381,"context_line":"}"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"# Get Ceph version"},{"line_number":384,"context_line":"function _get_ceph_version {"},{"line_number":385,"context_line":"    local ceph_version_str"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CEPH_CONTAINER_IMAGE \\"},{"line_number":388,"context_line":"        --version | awk \u0027{ print $3 }\u0027)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    echo $ceph_version_str"},{"line_number":391,"context_line":"}"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"# Get Ceph release codename"},{"line_number":394,"context_line":"function _get_ceph_codename {"},{"line_number":395,"context_line":"    local ceph_version_str"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CEPH_CONTAINER_IMAGE \\"},{"line_number":398,"context_line":"        --version | awk \u0027{ print $5 }\u0027)"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"    echo $ceph_version_str"},{"line_number":401,"context_line":"}"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"function _install_and_configure_clustered_nfs {"},{"line_number":404,"context_line":"    echo \"[CEPHADM] Deploy nfs.$FSNAME backend\""}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"eff2454e_ecf6fe9c","line":401,"range":{"start_line":383,"start_character":0,"end_line":401,"end_character":1},"in_reply_to":"649a4e7b_72acd37d","updated":"2026-06-18 12:50:12.000000000","message":"(Similar as in previous comment) I think we shouldn\u0027t rely on EL repo or operating system version as here we only download `cephadm` and then `cephadm` should ensure compatibility or throw error if it installs ceph on unsupported system (etc.). The ceph version is already constrained by the `/rpm-\u003ccodename\u003e/` part and I believe we should use only the latest repository which would be better/longer maintained.\n\nI checked 2 `cephadm` files and while they have different sizes, they claim the same version:\n```\n$ ./cephadm version  \ncephadm version 20.2.2 (0fcffee29411e3a38036764817b6e1afc59741cc) tentacle (stable)\n```\n\n\nI\u0027m Ok with optimization, but generally this shifts some logic to consumer where consumer must split/parse the codename and version (or throw one part away if only one thing is needed). So I see some tradeoff and the current way is more friendly for consumer side.\n\nUp to you to decide. I am new here.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":true,"context_lines":[{"line_number":380,"context_line":"        --keyring $CEPH_KEYRING -- ceph fs volume create \"$FSNAME\""},{"line_number":381,"context_line":"}"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"# Get Ceph version"},{"line_number":384,"context_line":"function _get_ceph_version {"},{"line_number":385,"context_line":"    local ceph_version_str"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CEPH_CONTAINER_IMAGE \\"},{"line_number":388,"context_line":"        --version | awk \u0027{ print $3 }\u0027)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    echo $ceph_version_str"},{"line_number":391,"context_line":"}"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"# Get Ceph release codename"},{"line_number":394,"context_line":"function _get_ceph_codename {"},{"line_number":395,"context_line":"    local ceph_version_str"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CEPH_CONTAINER_IMAGE \\"},{"line_number":398,"context_line":"        --version | awk \u0027{ print $5 }\u0027)"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"    echo $ceph_version_str"},{"line_number":401,"context_line":"}"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"function _install_and_configure_clustered_nfs {"},{"line_number":404,"context_line":"    echo \"[CEPHADM] Deploy nfs.$FSNAME backend\""}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"a61e76e0_d2c5a9c3","line":401,"range":{"start_line":383,"start_character":0,"end_line":401,"end_character":1},"in_reply_to":"7eb86262_5ea0fbe8","updated":"2026-05-30 14:31:18.000000000","message":"I hadn\u0027t luck with putting `CEPH_VERSION\u003d\"$(_get_ceph_codename)\"` at the end of the script. The variable was empty when running `./stack.sh`. I think the way this is ran doesn\u0027t allow the variable to be sourced or something.\n\nAnyway I because I dropped support for old ceph releases, the `_get_ceph_version` and `_get_ceph_codename` are used only in few places.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"8c0d54f47ab3372c0a15f3d913f4a494726fa84f","unresolved":true,"context_lines":[{"line_number":380,"context_line":"        --keyring $CEPH_KEYRING -- ceph fs volume create \"$FSNAME\""},{"line_number":381,"context_line":"}"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"# Get Ceph version"},{"line_number":384,"context_line":"function _get_ceph_version {"},{"line_number":385,"context_line":"    local ceph_version_str"},{"line_number":386,"context_line":""},{"line_number":387,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CEPH_CONTAINER_IMAGE \\"},{"line_number":388,"context_line":"        --version | awk \u0027{ print $3 }\u0027)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    echo $ceph_version_str"},{"line_number":391,"context_line":"}"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"# Get Ceph release codename"},{"line_number":394,"context_line":"function _get_ceph_codename {"},{"line_number":395,"context_line":"    local ceph_version_str"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CEPH_CONTAINER_IMAGE \\"},{"line_number":398,"context_line":"        --version | awk \u0027{ print $5 }\u0027)"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":"    echo $ceph_version_str"},{"line_number":401,"context_line":"}"},{"line_number":402,"context_line":""},{"line_number":403,"context_line":"function _install_and_configure_clustered_nfs {"},{"line_number":404,"context_line":"    echo \"[CEPHADM] Deploy nfs.$FSNAME backend\""}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"649a4e7b_72acd37d","line":401,"range":{"start_line":383,"start_character":0,"end_line":401,"end_character":1},"in_reply_to":"a61e76e0_d2c5a9c3","updated":"2026-06-05 12:40:46.000000000","message":"while I\u0027m fine to have a _get_ceph_codename, as mentioned in other parts of the review we need to also think about future versions, and switching from el9 to el10 when getting cephadm would be the affected piece of code.\nI also see that both `_codename` and `_version` return a awk \"{print $\u003cposition\u003e}\", so I was wondering if we want to optimize this part by having a single function that is just called once (e.g. at the beginning of this execution), and returns a string in the form of `$codename:$version`, so you can populate the two variables used in the rest of the execution. This way we can have a single function that \"inspects the container\" and prepare the input variables required for the rest of the script.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3bd1f2a34c921b770c797558c7fbdc8bbffeb3da","unresolved":true,"context_lines":[{"line_number":512,"context_line":""},{"line_number":513,"context_line":"    # Restart rgw to apply the new config"},{"line_number":514,"context_line":"    rgw_orch_name\u003d$(sudo ceph orch ls | grep rgw | awk \u0027{print $1}\u0027)"},{"line_number":515,"context_line":"    sudo ceph orch restart \"${rgw_orch_name}\""},{"line_number":516,"context_line":"    # Wait for the rgw to be up and running before returning"},{"line_number":517,"context_line":"    sleep \"10\""},{"line_number":518,"context_line":"}"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"38a1df67_b5e55757","line":515,"range":{"start_line":515,"start_character":4,"end_line":515,"end_character":45},"updated":"2026-05-19 22:53:22.000000000","message":"If RGW isn\u0027t deployed, `grep rgw` returns empty and `ceph orch restart \"\"` will fail. Add a guard for empty `rgw_orch_name`.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"e2e234503c693c47633d3d9cd214982c1652d0ea","unresolved":true,"context_lines":[{"line_number":512,"context_line":""},{"line_number":513,"context_line":"    # Restart rgw to apply the new config"},{"line_number":514,"context_line":"    rgw_orch_name\u003d$(sudo ceph orch ls | grep rgw | awk \u0027{print $1}\u0027)"},{"line_number":515,"context_line":"    sudo ceph orch restart \"${rgw_orch_name}\""},{"line_number":516,"context_line":"    # Wait for the rgw to be up and running before returning"},{"line_number":517,"context_line":"    sleep \"10\""},{"line_number":518,"context_line":"}"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"e53f922a_ab4cd12d","line":515,"range":{"start_line":515,"start_character":4,"end_line":515,"end_character":45},"in_reply_to":"38a1df67_b5e55757","updated":"2026-05-30 14:31:18.000000000","message":"Added a simple guard.","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d3592b834c119d43250866e85d6bb2166a30960f","unresolved":false,"context_lines":[{"line_number":512,"context_line":""},{"line_number":513,"context_line":"    # Restart rgw to apply the new config"},{"line_number":514,"context_line":"    rgw_orch_name\u003d$(sudo ceph orch ls | grep rgw | awk \u0027{print $1}\u0027)"},{"line_number":515,"context_line":"    sudo ceph orch restart \"${rgw_orch_name}\""},{"line_number":516,"context_line":"    # Wait for the rgw to be up and running before returning"},{"line_number":517,"context_line":"    sleep \"10\""},{"line_number":518,"context_line":"}"}],"source_content_type":"application/x-shellscript","patch_set":3,"id":"d4197330_190ea75a","line":515,"range":{"start_line":515,"start_character":4,"end_line":515,"end_character":45},"in_reply_to":"e53f922a_ab4cd12d","updated":"2026-06-04 04:24:34.000000000","message":"Done","commit_id":"d19bd4ed46571b687110f9b79e1e02e7633304b5"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"8c0d54f47ab3372c0a15f3d913f4a494726fa84f","unresolved":true,"context_lines":[{"line_number":168,"context_line":"    # $os_PACKAGE (\"rpm\") doesn\u0027t really matter. There is no ubuntu/debian"},{"line_number":169,"context_line":"    # equivalent being published by the ceph community."},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    os_release\u003d\"el9\""},{"line_number":172,"context_line":"    ceph_version\u003d$(_get_ceph_version)"},{"line_number":173,"context_line":"    curl -f -O https://download.ceph.com/rpm-${ceph_version}/${os_release}/noarch/cephadm"},{"line_number":174,"context_line":"    $SUDO mv cephadm $TARGET_BIN/cephadm"}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"7bfae8d1_c7d6f6f3","line":171,"updated":"2026-06-05 12:40:46.000000000","message":"I\u0027m not sure this is good. We\u0027re going to have `el10` at some point and then we need to reintroduce the code we\u0027re removing.\nWe can use `get_codename` that is based on the container, +1 for that, but we might want to call it beforehand and then resolve the release name here and take decisions based on the output.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"2dd2054012703509b0d67f667f659225ba2db01f","unresolved":true,"context_lines":[{"line_number":168,"context_line":"    # $os_PACKAGE (\"rpm\") doesn\u0027t really matter. There is no ubuntu/debian"},{"line_number":169,"context_line":"    # equivalent being published by the ceph community."},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    os_release\u003d\"el9\""},{"line_number":172,"context_line":"    ceph_version\u003d$(_get_ceph_version)"},{"line_number":173,"context_line":"    curl -f -O https://download.ceph.com/rpm-${ceph_version}/${os_release}/noarch/cephadm"},{"line_number":174,"context_line":"    $SUDO mv cephadm $TARGET_BIN/cephadm"}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"f80716d9_f63ac660","line":171,"in_reply_to":"7bfae8d1_c7d6f6f3","updated":"2026-06-18 12:50:12.000000000","message":"I have to say this part of script in general doesn\u0027t make sense 😀.\n\nWe are only downloading one binary/script for `cephadm` which doesn\u0027t depend on operating system (I\u0027m actually testing on Ubuntu). So the most logical thing imho would be if the `cephadm` was uploaded at `https://download.ceph.com/rpm-${ceph_version}/cephadm`. I don\u0027t know what is the difference really, but both files claim they are of same version and commit sha.\n\nI think we should just respect how the repo is structured. So I would generally not optimize here at all, just hardcode the `os_release` to latest `elX` (`el10`).\n\nI don\u0027t see other curl, yum, dnf, rpm installation dependency in this script, so I think this would be reasonable (I can leave comment with the explanation why it\u0027s hardcoded).\n\nLet me know what do you think.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"fae8f22a3785d0735d8ff9d86a21136ad90b5766","unresolved":true,"context_lines":[{"line_number":168,"context_line":"    # $os_PACKAGE (\"rpm\") doesn\u0027t really matter. There is no ubuntu/debian"},{"line_number":169,"context_line":"    # equivalent being published by the ceph community."},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    os_release\u003d\"el9\""},{"line_number":172,"context_line":"    ceph_version\u003d$(_get_ceph_version)"},{"line_number":173,"context_line":"    curl -f -O https://download.ceph.com/rpm-${ceph_version}/${os_release}/noarch/cephadm"},{"line_number":174,"context_line":"    $SUDO mv cephadm $TARGET_BIN/cephadm"}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"e2f49fd6_ff303e84","line":171,"in_reply_to":"f80716d9_f63ac660","updated":"2026-06-19 02:28:13.000000000","message":"It\u0027s not good, but, this part of the code didn\u0027t need to change for a couple of years :D The problem is the way cephadm is provided via https://download.ceph.com. This is a distro agnostic script.. I remember asking ceph folks if we can just source it from the source repo, but, it was somehow unstable to do that..","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"8c0d54f47ab3372c0a15f3d913f4a494726fa84f","unresolved":true,"context_lines":[{"line_number":378,"context_line":"    local ceph_version_str"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CONTAINER_IMAGE \\"},{"line_number":381,"context_line":"        --version | awk \u0027{ print $5 }\u0027)"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"    echo $ceph_version_str"},{"line_number":384,"context_line":"}"}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"b82396bd_c3c41352","line":381,"updated":"2026-06-05 12:40:46.000000000","message":"Because this is technically used in place of `CEPH_RELEASE`, and only limited to get cephadm and rgw, it\u0027s fine, but still I think this pattern is fragile.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"2dd2054012703509b0d67f667f659225ba2db01f","unresolved":true,"context_lines":[{"line_number":378,"context_line":"    local ceph_version_str"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"    ceph_version_str\u003d$(sudo podman run --rm --entrypoint ceph $CONTAINER_IMAGE \\"},{"line_number":381,"context_line":"        --version | awk \u0027{ print $5 }\u0027)"},{"line_number":382,"context_line":""},{"line_number":383,"context_line":"    echo $ceph_version_str"},{"line_number":384,"context_line":"}"}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"18bee5bb_5ae054e8","line":381,"in_reply_to":"b82396bd_c3c41352","updated":"2026-06-18 12:50:12.000000000","message":"Well I haven\u0027t change anything about how the function works. The user decides which version to use by overriding `CONTAINER_IMAGE` (CONTAINER_IMAGE\u003d${CONTAINER_IMAGE:-\u0027quay.io/ceph/ceph:v20\u0027})\n\nThen `_get_ceph_codename` and `_get_ceph_version` only return codename and full semver. So source of truth is in CONTAINER_IMAGE (as it was previously, I\u0027d generally define it with different variable, but it would be too much on top of existing changes in this MR imo).","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"8c0d54f47ab3372c0a15f3d913f4a494726fa84f","unresolved":true,"context_lines":[{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    # Ceph 20 doesn\u0027t recognize the config option \"rgw_keystone_api_version\" anymore."},{"line_number":487,"context_line":"    # Ceph 18 and 19 require \"rgw_keystone_api_version\" option to be set to 3 to work with keystone v3."},{"line_number":488,"context_line":"    ceph_release\u003d$(_get_ceph_codename)"},{"line_number":489,"context_line":"    if [[ \"${ceph_release}\" \u003d\u003d \"squid\" || \"${ceph_release}\" \u003d\u003d \"reef\" ]]; then"},{"line_number":490,"context_line":"        RGW_CONFIG_KEYS[\u0027rgw_keystone_api_version\u0027]\u003d3"},{"line_number":491,"context_line":"    fi"}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"37f4d733_bea55ed7","line":488,"updated":"2026-06-05 12:40:46.000000000","message":"iirc having variables for `container` and `release` was a design choice to meet some requirements of testing jobs.\nNow, I\u0027m not against a `_get_ceph_codename`, because I personally use that form too when I inspect a ceph container locally, but IMO the pattern is fragile. Perhaps, under this assumption, we can use a single `_container_inspect` function that returns both the `codename` and the `version`.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"2dd2054012703509b0d67f667f659225ba2db01f","unresolved":true,"context_lines":[{"line_number":485,"context_line":""},{"line_number":486,"context_line":"    # Ceph 20 doesn\u0027t recognize the config option \"rgw_keystone_api_version\" anymore."},{"line_number":487,"context_line":"    # Ceph 18 and 19 require \"rgw_keystone_api_version\" option to be set to 3 to work with keystone v3."},{"line_number":488,"context_line":"    ceph_release\u003d$(_get_ceph_codename)"},{"line_number":489,"context_line":"    if [[ \"${ceph_release}\" \u003d\u003d \"squid\" || \"${ceph_release}\" \u003d\u003d \"reef\" ]]; then"},{"line_number":490,"context_line":"        RGW_CONFIG_KEYS[\u0027rgw_keystone_api_version\u0027]\u003d3"},{"line_number":491,"context_line":"    fi"}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"c901a57f_6473c302","line":488,"in_reply_to":"37f4d733_bea55ed7","updated":"2026-06-18 12:50:12.000000000","message":"I think similar as for previous comment. I\u0027m leaving up to you what to do here.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"8c0d54f47ab3372c0a15f3d913f4a494726fa84f","unresolved":true,"context_lines":[{"line_number":494,"context_line":"        set_config_key \"global\" ${k} ${RGW_CONFIG_KEYS[$k]}"},{"line_number":495,"context_line":"    done"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"    # Restart rgw to apply the new config"},{"line_number":498,"context_line":"    rgw_orch_name\u003d$(sudo ceph orch ls | grep rgw | awk \u0027{print $1}\u0027)"},{"line_number":499,"context_line":"    if [[ \"${rgw_orch_name}\" \u003d\u003d \"\" ]]; then"},{"line_number":500,"context_line":"        echo \"[CEPHADM] error searching for RGW daemon\""}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"b0161c96_af1dc10d","line":497,"updated":"2026-06-05 12:40:46.000000000","message":"I\u0027m wondering if L497-L505 can go away by simply calling this function before deploying RGW (L536 should be moved to L531). This way we don\u0027t have to overcomplicate the logic with a restart and we have a smaller failure surface.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":25402,"name":"Francesco Pantano","email":"fpantano@redhat.com","username":"fmount"},"change_message_id":"96e95aca0cd1879da81b07774f18f16d6f54c173","unresolved":true,"context_lines":[{"line_number":494,"context_line":"        set_config_key \"global\" ${k} ${RGW_CONFIG_KEYS[$k]}"},{"line_number":495,"context_line":"    done"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"    # Restart rgw to apply the new config"},{"line_number":498,"context_line":"    rgw_orch_name\u003d$(sudo ceph orch ls | grep rgw | awk \u0027{print $1}\u0027)"},{"line_number":499,"context_line":"    if [[ \"${rgw_orch_name}\" \u003d\u003d \"\" ]]; then"},{"line_number":500,"context_line":"        echo \"[CEPHADM] error searching for RGW daemon\""}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"80c0cb79_de2b31bb","line":497,"in_reply_to":"96a64d34_7599f9e5","updated":"2026-06-25 05:24:12.000000000","message":"I\u0027m going to approve this patch because you\u0027re testing and I think we could benefit of having RGW deployed.\nStill this part is the main concern I had for two reason:\n1. I usually prefer to avoid chaining grep and awk: awk \u0027/rgw/ {print $1}\u0027 would achieve the same goal\n2. I think that L497-L505 can go away by simply calling this function before deploying RGW (L536 should be moved to L531)\n\nBecause this patch is here for a bit at this point it wouldn\u0027t hurt landing this code and follow up on this part.","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"},{"author":{"_account_id":38880,"name":"Ondrej Vasko","email":"ondrej.vaskoo@gmail.com","username":"o.vasko"},"change_message_id":"2dd2054012703509b0d67f667f659225ba2db01f","unresolved":true,"context_lines":[{"line_number":494,"context_line":"        set_config_key \"global\" ${k} ${RGW_CONFIG_KEYS[$k]}"},{"line_number":495,"context_line":"    done"},{"line_number":496,"context_line":""},{"line_number":497,"context_line":"    # Restart rgw to apply the new config"},{"line_number":498,"context_line":"    rgw_orch_name\u003d$(sudo ceph orch ls | grep rgw | awk \u0027{print $1}\u0027)"},{"line_number":499,"context_line":"    if [[ \"${rgw_orch_name}\" \u003d\u003d \"\" ]]; then"},{"line_number":500,"context_line":"        echo \"[CEPHADM] error searching for RGW daemon\""}],"source_content_type":"application/x-shellscript","patch_set":5,"id":"96a64d34_7599f9e5","line":497,"in_reply_to":"b0161c96_af1dc10d","updated":"2026-06-18 12:50:12.000000000","message":"I need to check that. It\u0027s possible original author wanted this function to be callable from anywhere (but forgot about reload/restart). Keeping the restart would make the function more resilient (e.g. to from where it is invoked).","commit_id":"01e2f06a0a96a60915cc875cc30c5a0c88507e02"}]}