)]}'
{"deployment/cinder/cinder-api-container-puppet.yaml":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"824fb4b070897a824bf3446fa7ee6da69b110ffd","unresolved":true,"context_lines":[{"line_number":410,"context_line":"            service_type: \"{{ item.service_type }}\""},{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"e45df225_0c3fd6fb","line":413,"range":{"start_line":413,"start_character":16,"end_line":413,"end_character":36},"updated":"2020-12-16 08:24:33.000000000","message":"so do you want to remove that from the loop ?","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"041e4c65fe15bc33e1cbe4d561e9b163edf0edcb","unresolved":true,"context_lines":[{"line_number":410,"context_line":"            service_type: \"{{ item.service_type }}\""},{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"df8537f1_8dfa776b","line":413,"range":{"start_line":413,"start_character":16,"end_line":413,"end_character":36},"in_reply_to":"8a3c382a_ccfad017","updated":"2020-12-16 15:54:30.000000000","message":"thanks for checking i read this task too quickly didn\u0027t realise it was about removal (state: absent :/) \n\nbut should this now be about removing cinder and cinderv2 ? i mean why are we removing cinderv3 here too?","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3b2b9f48595aeef6420c8c317b8e7651897a5314","unresolved":false,"context_lines":[{"line_number":410,"context_line":"            service_type: \"{{ item.service_type }}\""},{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"cde83f20_a5e25d7b","line":413,"range":{"start_line":413,"start_character":16,"end_line":413,"end_character":36},"in_reply_to":"df8537f1_8dfa776b","updated":"2020-12-16 16:28:06.000000000","message":"Answered below.","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"72de153109ebf4d53f4aee39ae5dc6af18d4288e","unresolved":true,"context_lines":[{"line_number":410,"context_line":"            service_type: \"{{ item.service_type }}\""},{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"8a3c382a_ccfad017","line":413,"range":{"start_line":413,"start_character":16,"end_line":413,"end_character":36},"in_reply_to":"e45df225_0c3fd6fb","updated":"2020-12-16 14:50:19.000000000","message":"The original code [1] removed the \"volume\" service (cinder\u0027s API v1). I converted that code into a loop, which now deletes both the v1 and v2 (\"volume2\") services. I\u0027m not sure if that answers you question.\n\n[1] Ic0eb72f78e2a19e2f40ab12631a872d828bab46a","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"041e4c65fe15bc33e1cbe4d561e9b163edf0edcb","unresolved":true,"context_lines":[{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"},{"line_number":417,"context_line":"          when:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"27dcb146_76005ed4","line":414,"range":{"start_line":414,"start_character":29,"end_line":414,"end_character":37},"updated":"2020-12-16 15:54:30.000000000","message":"so why do you want to remove cinderv3 here?","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":18002,"name":"John Fulton","email":"fulton@redhat.com","username":"fultonj"},"change_message_id":"3ddde1f8cf3ec48eaaf6033218bc3b3e1438822e","unresolved":true,"context_lines":[{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"},{"line_number":417,"context_line":"          when:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"3fa23c6d_18fd038e","line":414,"range":{"start_line":414,"start_character":29,"end_line":414,"end_character":37},"in_reply_to":"263fbf96_6ad0092a","updated":"2020-12-23 13:37:23.000000000","message":"Do you want to add a comment to the code itself on why service_name: cinderv3 is in the loop above? I realize you already explained it here in the review, but I\u0027m thinking of a person who thinks this is a bug. As the matter is unintuitive, the comment would make the code easier to maintain in the future. I suppose we could always refer to this review later in place of the comment, but this review vs a direct comment in the code is easier to see.","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"3b2b9f48595aeef6420c8c317b8e7651897a5314","unresolved":true,"context_lines":[{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"},{"line_number":417,"context_line":"          when:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"62231eb4_461bfa9d","line":414,"range":{"start_line":414,"start_character":29,"end_line":414,"end_character":37},"in_reply_to":"27dcb146_76005ed4","updated":"2020-12-16 16:28:06.000000000","message":"Yup, this can be confusing. Note the service_name includes the \"v3\" suffix but the service_name does not. L414 is cleaning up a workaround that was introduced way back in queens that was necessary for compatibility with tempest. The full history is long and boring, but the important patches are [1], which introduced the queens workaround, and [2] which required the workaround be removed because things broke again in train :-/\n\n[1] If1ef8b1ad60151c0dfd0a7804ba7e697fc4ede28\n[2] Ic0eb72f78e2a19e2f40ab12631a872d828bab46a\n\nCinder\u0027s v3 API is still present in the keystone catalog, with service_name \"cinderv3\" and service_type \"volumev3\" (both have \"v3\" suffix).","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"977e63d61f37f3edf5cac95cff3c1b5bcce65be7","unresolved":false,"context_lines":[{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"},{"line_number":417,"context_line":"          when:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"7a2aded0_2411fd89","line":414,"range":{"start_line":414,"start_character":29,"end_line":414,"end_character":37},"in_reply_to":"3fa23c6d_18fd038e","updated":"2020-12-23 16:38:04.000000000","message":"Done","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"d4035cfb460bda0d059c03822df761d930f8a4c6","unresolved":true,"context_lines":[{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"},{"line_number":417,"context_line":"          when:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"58761b91_e18e1a6d","line":414,"range":{"start_line":414,"start_character":29,"end_line":414,"end_character":37},"in_reply_to":"62231eb4_461bfa9d","updated":"2020-12-16 16:30:06.000000000","message":"Whoops, I meant the service_type on L414 doesn\u0027t have the \"v3\" suffix.","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"884b9165f717df5436bfed06257c4e9eb04478fd","unresolved":true,"context_lines":[{"line_number":411,"context_line":"            state: absent"},{"line_number":412,"context_line":"          loop:"},{"line_number":413,"context_line":"            - {service_name: cinderv2, service_type: volumev2}"},{"line_number":414,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":415,"context_line":"          loop_control:"},{"line_number":416,"context_line":"            index_var: cinder_api_service"},{"line_number":417,"context_line":"          when:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"263fbf96_6ad0092a","line":414,"range":{"start_line":414,"start_character":29,"end_line":414,"end_character":37},"in_reply_to":"62231eb4_461bfa9d","updated":"2020-12-16 16:30:38.000000000","message":"thank you for taking the time to explain makes more sense now","commit_id":"a9bde2d85e05be6f638b9b9ac8e5bb471cf0c3c0"},{"author":{"_account_id":3153,"name":"Emilien Macchi","email":"emilien@redhat.com","username":"emilienm"},"change_message_id":"fb6207771c228c29c1db300c4731d39600d92dc1","unresolved":true,"context_lines":[{"line_number":416,"context_line":"            # Ic0eb72f78e2a19e2f40ab12631a872d828bab46a for details."},{"line_number":417,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":418,"context_line":"          loop_control:"},{"line_number":419,"context_line":"            index_var: cinder_api_service"},{"line_number":420,"context_line":"          when:"},{"line_number":421,"context_line":"            - step|int \u003d\u003d 4"},{"line_number":422,"context_line":"        - name: Manage Cinder Volume Type"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"340b1374_0b5a643c","line":419,"updated":"2021-01-22 10:11:11.000000000","message":"FYI some clients rely on volume_api_version in clouds.yaml which is v2 by default sometimes (e.g. gophercloud).\nSo we should have modified https://github.com/openstack/tripleo-heat-templates/blob/master/deployment/keystone/keystone-container-puppet.yaml#L747 to force volume_api_version to be set to \"3\".\nI\u0027m on PTO but Martin André will fix it.","commit_id":"d0c9b6d0ce049a75cc8177a84ab67c3e659178c7"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"b7d48d17ab924e207517b8290b19d968e083ba25","unresolved":true,"context_lines":[{"line_number":416,"context_line":"            # Ic0eb72f78e2a19e2f40ab12631a872d828bab46a for details."},{"line_number":417,"context_line":"            - {service_name: cinderv3, service_type: volume}"},{"line_number":418,"context_line":"          loop_control:"},{"line_number":419,"context_line":"            index_var: cinder_api_service"},{"line_number":420,"context_line":"          when:"},{"line_number":421,"context_line":"            - step|int \u003d\u003d 4"},{"line_number":422,"context_line":"        - name: Manage Cinder Volume Type"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"4acd89f5_87e53d70","line":419,"in_reply_to":"340b1374_0b5a643c","updated":"2021-01-22 17:04:37.000000000","message":"Ah, I didn\u0027t realize, so thanks for pointing it out! I also could fix this, but don\u0027t want to swoop in if Martin already has it on his to-do list.","commit_id":"d0c9b6d0ce049a75cc8177a84ab67c3e659178c7"}]}
