)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6816,"name":"Jesse Pretorius","email":"jesse@odyssey4.me","username":"jesse-pretorius"},"change_message_id":"b7dc6d5af487f12a6b33703535f49014b68cd8bb","unresolved":true,"context_lines":[{"line_number":15,"context_line":"Remove the redis bundle at the very end of the"},{"line_number":16,"context_line":"openstack upgrade, to ensure pacemaker is up and"},{"line_number":17,"context_line":"runing."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ib51903746d0ec5fc981efecf3e68fc5399535fdb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"0b5dc21e_a8fc0e38","line":18,"updated":"2023-06-19 10:21:17.000000000","message":"Please refer to rhbz#2158159 in the commit msg.","commit_id":"11d438c2d3a16c0e3b2c4fddf1e15fbac3c375e7"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"4f33e2d1348f5a0f78968dc3654ed74435bc33ff","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"39cca64b_33dd74ba","updated":"2023-05-31 17:46:24.000000000","message":"Removing redis automatically can be risky because it can break the deployment in case any services dependent on redis is enabled in the environment and users do not update their template properly to enable redis. That is the exact reason why upgrade hard-fails now.\n\nIf we want to fully automate it then we probably can introduce a new tht parameter to enforce removal of redis during upgrade.","commit_id":"819df66963747a83906300037c316a959fa1f83b"},{"author":{"_account_id":22954,"name":"Juan Badia Payno","email":"jbadiapa@redhat.com","username":"jbadiapa"},"change_message_id":"289d9ae063fc06f945913862e888094993a66898","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"a418c3f5_899eb765","updated":"2023-05-31 17:39:50.000000000","message":"waiting for dciabrin and takashi comments","commit_id":"819df66963747a83906300037c316a959fa1f83b"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"ff9ce73e739ee6d71be68211d58b30173f838c85","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"7f57742b_ad528645","in_reply_to":"15c26fff_6fc15d47","updated":"2023-06-13 18:32:00.000000000","message":"I rechecked the template files in stable/train and I confirmed that these templates already include redis service enabled. Octavia and Designate require redis since wallaby but that requires using the new environment file and the problem is not related to this change... So we\u0027d be fine if users are using built-in environment files. I\u0027ve seen some creating their own templates from scratch in the wile and they might hit a problem if they expect redis is explicitly enabled though I\u0027m not sure how much popular it is.\n\nTo be most safe we can still keep the flag and make it fail by default but if we expect users rarely have their own templates then we can merge this change (with fix suggested by Damien).","commit_id":"819df66963747a83906300037c316a959fa1f83b"},{"author":{"_account_id":22954,"name":"Juan Badia Payno","email":"jbadiapa@redhat.com","username":"jbadiapa"},"change_message_id":"d520d97935898d9a53f6232e5df8412241729508","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6801cbed_92f27f9b","in_reply_to":"39cca64b_33dd74ba","updated":"2023-05-31 20:05:33.000000000","message":"From train to wallaby, first the openstack part is upgraded and then OS is upgraded.\n\nAt this point I do not see the difference between remove redis automatically or manually.\n\n\nI meant operator do the openstack upgrade, then remove redis manually, then os upgrade.\n\nWhile with this patch, the operator upgrade openstack, then os which remove redis first.\n\nAm I wrong? or is there anything that I am skipping?","commit_id":"819df66963747a83906300037c316a959fa1f83b"},{"author":{"_account_id":20778,"name":"Damien Ciabrini","email":"dciabrin@redhat.com","username":"dciabrin"},"change_message_id":"5cc80594f8a0a169b310e67bca5057f8fd43b6e4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"15c26fff_6fc15d47","in_reply_to":"42cc85e2_219aaa13","updated":"2023-06-13 15:06:49.000000000","message":"A quick note on why we added the warning in the first place. Back then, redis was disabled my default but still usable if telemetry or designate was explicitely used (not default as well). Knowing that a FFU to wallaby would include the OS upgrade step prior to recreating a new pacemaker cluster and deploying wallaby on top, it looked like a burden to try and remove redis automatically while keeping idempotency.\n\nNow the situation has changed a bit, in that the FFU now upgrade all components to wallaby prior to doing the OS upgrade, removing redis is a simpler task because we know that the cluster is still running when we want to do so.\n\nAlso re. Takashi\u0027s point on RHOSP 16 and telemetry enablement, if a user upgrades a wallaby without explicitely passing telemetry or designate if it uses it, IMHO he/she shouldn\u0027t expect the upgrade to pass succesfully. In that case, this review would remove redis behind the curtain, but having a passing upgrade is arguably worse because the stack would lack the proper yaml files.\n\nI have the feeling Juan\u0027s patch makes sense now and removing redis automatically would be achievable while keeping idempotency, without much risk for the FFU.","commit_id":"819df66963747a83906300037c316a959fa1f83b"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"5a9a371dae07022a440c1a1d8db9c7ddbbdfeafd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"42cc85e2_219aaa13","in_reply_to":"5bb928fe_b1125eb3","updated":"2023-06-01 00:56:23.000000000","message":"In downstream RHOSP16, we add environments/docker-ha.yaml even if user does not give it (you can find the downstream only patch). Because of this in RHOSP16 Redis was by default enabled. We later disabled Redis in docker-ha.yaml by [1]. See [2] for further details.\n\n[1] https://review.opendev.org/c/openstack/tripleo-heat-templates/+/825213\n[2] https://bugzilla.redhat.com/show_bug.cgi?id\u003d1823250\n\nImagine that a user is using RHOSP16 with telemetry enabled. In his/her templates Redis is not explicitly enabled. Then later he/she upgrade their deployment without noticing now Redis is no longer enabled. Then upgrade fails at a different place or it might complete with broken telemetry.\n\nThe message was added to catch such problem early and gives a message to the user to let them understand what should be fixed.\n\nThis is not be a problem in upstream, though. If we don\u0027t like exposing our tech-debt caused by downstream adjustment then we can remove it but we have to understand that problem we likely face in RHOSP.","commit_id":"819df66963747a83906300037c316a959fa1f83b"},{"author":{"_account_id":22954,"name":"Juan Badia Payno","email":"jbadiapa@redhat.com","username":"jbadiapa"},"change_message_id":"74a5fd7a92726e5b790cb7e4d55f494792748586","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5bb928fe_b1125eb3","in_reply_to":"6801cbed_92f27f9b","updated":"2023-05-31 20:07:19.000000000","message":"At this point, Redis service should be None because is no longer used or enabled.","commit_id":"819df66963747a83906300037c316a959fa1f83b"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"24ea0ac2e52f74fd7fa8b05556f94e0ad2042f8d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"08bc26e3_908e1efa","in_reply_to":"7f57742b_ad528645","updated":"2023-06-16 05:28:48.000000000","message":"I\u0027m convinced with the Damien\u0027s comment so am fine with this proposal as long as it passes CI.","commit_id":"819df66963747a83906300037c316a959fa1f83b"},{"author":{"_account_id":22954,"name":"Juan Badia Payno","email":"jbadiapa@redhat.com","username":"jbadiapa"},"change_message_id":"ec30fb9190da6daa6f2c5e188b57ba3546759a3a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c632943b_cf50dd9a","updated":"2023-06-13 16:10:56.000000000","message":"testing","commit_id":"8860b4aa72ee367827b42435884d5fdae5a0a7a4"},{"author":{"_account_id":22954,"name":"Juan Badia Payno","email":"jbadiapa@redhat.com","username":"jbadiapa"},"change_message_id":"d849b50a9ee3f0e494e90a1ab5a365580c66c471","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"561aca11_050cfee6","updated":"2023-06-19 07:10:41.000000000","message":"recheck","commit_id":"11d438c2d3a16c0e3b2c4fddf1e15fbac3c375e7"},{"author":{"_account_id":20778,"name":"Damien Ciabrini","email":"dciabrin@redhat.com","username":"dciabrin"},"change_message_id":"4a37fc4720256b4d0e6dc42d4f8070aa72427ae0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"99f3186e_19747c1c","updated":"2023-06-27 07:49:35.000000000","message":"LGTM","commit_id":"0e2b7a1d58d94816425a9b927b968123a4968c00"},{"author":{"_account_id":6816,"name":"Jesse Pretorius","email":"jesse@odyssey4.me","username":"jesse-pretorius"},"change_message_id":"920c0901160c0e83c2a7a72b15c87872aa7e363e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d4cd4dbe_cfe2c7ca","updated":"2023-06-27 13:03:17.000000000","message":"recheck due to timeout","commit_id":"0e2b7a1d58d94816425a9b927b968123a4968c00"},{"author":{"_account_id":22954,"name":"Juan Badia Payno","email":"jbadiapa@redhat.com","username":"jbadiapa"},"change_message_id":"891e4f5646ad6b46bf9f3491ce3e42247df551dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"56a2178f_b4526c7d","updated":"2023-06-20 16:08:19.000000000","message":"there is a follow up patch https://review.opendev.org/c/openstack/tripleo-heat-templates/+/884912","commit_id":"0e2b7a1d58d94816425a9b927b968123a4968c00"}],"deployment/pacemaker/pacemaker-baremetal-puppet.yaml":[{"author":{"_account_id":20778,"name":"Damien Ciabrini","email":"dciabrin@redhat.com","username":"dciabrin"},"change_message_id":"e258bba6cfeb0e4452bb2069e5a3150270665908","unresolved":true,"context_lines":[{"line_number":222,"context_line":"          run_once: true"},{"line_number":223,"context_line":"          become: true"},{"line_number":224,"context_line":"          # remove redis-bundle"},{"line_number":225,"context_line":"          # prior to OS upgrade, don\u0027t assume the cluster is running"},{"line_number":226,"context_line":"          # probe the CIB file directly instead"},{"line_number":227,"context_line":"          shell: |"},{"line_number":228,"context_line":"            export CIB_file\u003d/var/lib/pacemaker/cib/cib.xml"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"68892464_84728e31","line":225,"updated":"2023-06-13 15:10:33.000000000","message":"running pcs to remove a resource requires the cluster to be running. However at step 5, we know that the cluster is running and the OS upgrade hasn\u0027t run yet. So it\u0027s safe to remve that comment and get rid of \"export CIB_file\u003d/var/lib/pacemaker/cib/cib.xml\". The crm_resource and pcs calls will act on the live cluster\u0027s CIB and the removal will get replicated on the entire cluster.","commit_id":"ac43ee7604e0678b8f5ba4704ae327fbc570cb77"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"24ea0ac2e52f74fd7fa8b05556f94e0ad2042f8d","unresolved":true,"context_lines":[{"line_number":222,"context_line":"          run_once: true"},{"line_number":223,"context_line":"          become: true"},{"line_number":224,"context_line":"          pacemaker_resource:"},{"line_number":225,"context_line":"                resource: redis-bundle"},{"line_number":226,"context_line":"                state: delete"},{"line_number":227,"context_line":"                wait_for_resource: true"},{"line_number":228,"context_line":"        - name: upgrade step 0"}],"source_content_type":"text/x-yaml","patch_set":4,"id":"af0a29dc_bc3e4d1c","line":225,"range":{"start_line":225,"start_character":10,"end_line":225,"end_character":16},"updated":"2023-06-16 05:28:48.000000000","message":"this looks overly indented ?","commit_id":"3139446bf58c2d7be1eecc2752b7c4a26c7428d5"},{"author":{"_account_id":22954,"name":"Juan Badia Payno","email":"jbadiapa@redhat.com","username":"jbadiapa"},"change_message_id":"e87ed6dcb10e2e6784c4ffa816c3e2a24bffbdbe","unresolved":true,"context_lines":[{"line_number":221,"context_line":"            - \u0027\"redis\" not in enabled_services|list\u0027"},{"line_number":222,"context_line":"          run_once: true"},{"line_number":223,"context_line":"          become: true"},{"line_number":224,"context_line":"          pacemaker_resource:"},{"line_number":225,"context_line":"            resource: redis-bundle"},{"line_number":226,"context_line":"            state: delete"},{"line_number":227,"context_line":"            wait_for_resource: true"},{"line_number":228,"context_line":"        - name: upgrade step 0"},{"line_number":229,"context_line":"          when: step|int \u003d\u003d 0"},{"line_number":230,"context_line":"          block:"}],"source_content_type":"text/x-yaml","patch_set":5,"id":"4afd9e9f_8b303d97","line":227,"range":{"start_line":224,"start_character":0,"end_line":227,"end_character":35},"updated":"2023-06-16 07:18:08.000000000","message":"This lead us to the following error:\n\n2023-06-15 23:44:19 | 2023-06-15 23:44:19.230774 | 52540095-16e6-e3ef-3ceb-00000000531f |      FATAL | Ensure redis is removed | controller-0 | error\u003d{\"changed\": false, \"error\": \"Error: Unable to update cib\nCall cib_apply_diff failed (-203): Update does not conform to the configured schema","commit_id":"acb3ca30914bba1abe0c7f22b8dbbb29d97d4273"}]}