)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"d0ae838d89bca6c2c9a8b31fa29e58ecdd4145a5","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Takashi Kajinami \u003ctkajinam@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-12-31 14:48:03 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cinder: Do not manage default volume type by puppet"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"... because we already have an ansible task to manage it."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"8b552708_af84560f","line":7,"updated":"2021-01-04 14:31:04.000000000","message":"nit: This patch is really about removing obsolete code that was disabled by [1]. Technically, [1] is where we stopped using puppet to manage the volume type.\n\n[1] I96a3351fca26cd8bb122a86cb4c3a58d5f88573e","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c14335aa035a2f96c58a7aea113b9f6ec9fe9825","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Takashi Kajinami \u003ctkajinam@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-12-31 14:48:03 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cinder: Do not manage default volume type by puppet"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"... because we already have an ansible task to manage it."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"d788363a_9fdd80a9","line":7,"in_reply_to":"2ce2bbc3_5220ce62","updated":"2021-01-05 04:21:47.000000000","message":"The parameter is used in keystone but it\u0027s not useful either. So I will cleaned up both.\n https://review.opendev.org/c/openstack/puppet-tripleo/+/768380","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"7d0afc3c4aaac1d4a6a9c9480f55192b42e7ec14","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Takashi Kajinami \u003ctkajinam@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-12-31 14:48:03 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cinder: Do not manage default volume type by puppet"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"... because we already have an ansible task to manage it."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ce49c222_9e4d64d4","line":7,"in_reply_to":"8b552708_af84560f","updated":"2021-01-05 01:52:42.000000000","message":"No. The task to manage volume type is still valid.\n\nThe keystone_resources_managed hieradata does nothing with volume type management in puppet-tripleo.","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"c81624e23d65a9c3bb05a87f2f8a4a6dcbf4bc19","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Takashi Kajinami \u003ctkajinam@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-12-31 14:48:03 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cinder: Do not manage default volume type by puppet"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"... because we already have an ansible task to manage it."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"2ce2bbc3_5220ce62","line":7,"in_reply_to":"b4f35c6d_14219924","updated":"2021-01-05 03:57:21.000000000","message":"I\u0027m guessing that parameter is still used elsewhere, so I think the cleanup should be limited to whatever is managing the cinder volume type.","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"48810129609442e34364d43b137a22d624b485d8","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Takashi Kajinami \u003ctkajinam@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-12-31 14:48:03 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cinder: Do not manage default volume type by puppet"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"... because we already have an ansible task to manage it."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"b4f35c6d_14219924","line":7,"in_reply_to":"c0a13ffb_984787e5","updated":"2021-01-05 03:51:11.000000000","message":"Sorry I missed that part... I thought it were coming from a different parameter. \nI have submitted a series of changes to cleanup the keystone_resources_managed parameter so let me squash this into the series.","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"664d5751169eb23ca818dd7e3f34a245036e17c4","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     Takashi Kajinami \u003ctkajinam@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2020-12-31 14:48:03 +0900"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Cinder: Do not manage default volume type by puppet"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"... because we already have an ansible task to manage it."},{"line_number":10,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"c0a13ffb_984787e5","line":7,"in_reply_to":"ce49c222_9e4d64d4","updated":"2021-01-05 03:05:44.000000000","message":"I\u0027m looking at these from I557d8f33c9c699aed14b3b6fc1d1c0407365cd08\n\n\nhttps://opendev.org/openstack/puppet-tripleo/src/branch/master/manifests/profile/base/cinder/api.pp#L75\nhttps://opendev.org/openstack/puppet-tripleo/src/branch/master/manifests/profile/base/cinder/api.pp#L79\nhttps://opendev.org/openstack/puppet-tripleo/src/branch/master/manifests/profile/base/cinder/api.pp#L111\n\nThe container_puppet_tasks in THT was effectively disabled by setting keystone_resources_managed to False.","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"}],"deployment/cinder/cinder-api-container-puppet.yaml":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"f0f26f0514e767688e47cb5c894421b03cdee5f7","unresolved":true,"context_lines":[{"line_number":350,"context_line":"                  - /var/log/containers/httpd/cinder-api:/var/log/httpd:z"},{"line_number":351,"context_line":"            environment:"},{"line_number":352,"context_line":"              KOLLA_CONFIG_STRATEGY: COPY_ALWAYS"},{"line_number":353,"context_line":"      container_puppet_tasks: {}"},{"line_number":354,"context_line":"      metadata_settings:"},{"line_number":355,"context_line":"        get_attr: [ApacheServiceBase, role_data, metadata_settings]"},{"line_number":356,"context_line":"      host_prep_tasks:"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"fef541f2_6e766c4e","line":353,"updated":"2020-12-31 05:45:33.000000000","message":"You should be able to delete this line entirely.\n\nBut, where is the code that replaces the functionality? If there\u0027s an ansible task that handles this, where is it invoked?","commit_id":"ea2446aa2a95ec7d19b1b3e44054fe51f83e30a3"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"6f579624ca21ab44033600c9fb5c257abc7363bc","unresolved":false,"context_lines":[{"line_number":350,"context_line":"                  - /var/log/containers/httpd/cinder-api:/var/log/httpd:z"},{"line_number":351,"context_line":"            environment:"},{"line_number":352,"context_line":"              KOLLA_CONFIG_STRATEGY: COPY_ALWAYS"},{"line_number":353,"context_line":"      container_puppet_tasks: {}"},{"line_number":354,"context_line":"      metadata_settings:"},{"line_number":355,"context_line":"        get_attr: [ApacheServiceBase, role_data, metadata_settings]"},{"line_number":356,"context_line":"      host_prep_tasks:"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"91302a02_56592da8","line":353,"in_reply_to":"fef541f2_6e766c4e","updated":"2020-12-31 05:50:06.000000000","message":"\u003e You should be able to delete this line entirely.\nThanks for the feedback. I removed this line as suggested.\n\n\u003e But, where is the code that replaces the functionality? If there\u0027s an ansible task that handles this, where is it invoked?\n\nIIUC we have another implementation to create the default volume type in external_deploy_tasks.","commit_id":"ea2446aa2a95ec7d19b1b3e44054fe51f83e30a3"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"d0ae838d89bca6c2c9a8b31fa29e58ecdd4145a5","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        map_merge:"},{"line_number":162,"context_line":"          - get_attr: [CinderBase, role_data, config_settings]"},{"line_number":163,"context_line":"          - get_attr: [ApacheServiceBase, role_data, config_settings]"},{"line_number":164,"context_line":"          - keystone_resources_managed: false"},{"line_number":165,"context_line":"          - cinder::keystone::authtoken::www_authenticate_uri: {get_param: [EndpointMap, KeystoneInternal, uri_no_suffix]}"},{"line_number":166,"context_line":"            cinder::keystone::authtoken::auth_url: {get_param: [EndpointMap, KeystoneInternal, uri_no_suffix]}"},{"line_number":167,"context_line":"            cinder::keystone::authtoken::password: {get_param: CinderPassword}"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"ab8a094d_15e7bf79","line":164,"updated":"2021-01-04 14:31:04.000000000","message":"This line was added by [1] and can be removed after cinder\u0027s puppet-tripleo code [2][3] gets cleaned up. With this in mind, I suggest removing the obsolete code from puppet-tripleo before this patch. That way it will be safe for this patch to remove L164.\n\n[1] I96a3351fca26cd8bb122a86cb4c3a58d5f88573e\n[2] I557d8f33c9c699aed14b3b6fc1d1c0407365cd08\n[3] Ia23996abefdd1410fb86f04ed84a314f4364339c","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c14335aa035a2f96c58a7aea113b9f6ec9fe9825","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        map_merge:"},{"line_number":162,"context_line":"          - get_attr: [CinderBase, role_data, config_settings]"},{"line_number":163,"context_line":"          - get_attr: [ApacheServiceBase, role_data, config_settings]"},{"line_number":164,"context_line":"          - keystone_resources_managed: false"},{"line_number":165,"context_line":"          - cinder::keystone::authtoken::www_authenticate_uri: {get_param: [EndpointMap, KeystoneInternal, uri_no_suffix]}"},{"line_number":166,"context_line":"            cinder::keystone::authtoken::auth_url: {get_param: [EndpointMap, KeystoneInternal, uri_no_suffix]}"},{"line_number":167,"context_line":"            cinder::keystone::authtoken::password: {get_param: CinderPassword}"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"aecd5021_5c3acf9d","line":164,"in_reply_to":"72d10ea0_9b16d436","updated":"2021-01-05 04:21:47.000000000","message":"I\u0027ve updated the patch to clean up the useless implementation and this parameter AFTER we remove the useless container_puppet_tasks.\n\nI think this is a backport candidate because we don\u0027t like to deploy useless container to do nothing, and with these separated patch we can backport this fix easily to older branches.\nOf cause we can squash this change into the subsequent change but in that case we also backport the cleanup part as well.","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"7d0afc3c4aaac1d4a6a9c9480f55192b42e7ec14","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        map_merge:"},{"line_number":162,"context_line":"          - get_attr: [CinderBase, role_data, config_settings]"},{"line_number":163,"context_line":"          - get_attr: [ApacheServiceBase, role_data, config_settings]"},{"line_number":164,"context_line":"          - keystone_resources_managed: false"},{"line_number":165,"context_line":"          - cinder::keystone::authtoken::www_authenticate_uri: {get_param: [EndpointMap, KeystoneInternal, uri_no_suffix]}"},{"line_number":166,"context_line":"            cinder::keystone::authtoken::auth_url: {get_param: [EndpointMap, KeystoneInternal, uri_no_suffix]}"},{"line_number":167,"context_line":"            cinder::keystone::authtoken::password: {get_param: CinderPassword}"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"72d10ea0_9b16d436","line":164,"in_reply_to":"ab8a094d_15e7bf79","updated":"2021-01-05 01:52:42.000000000","message":"As I mentioned in my previous comment the keystone_resources_managed parameter is not related to the code in puppet-tripleo which manages volume type, so this change should be considered separately.","commit_id":"b5d0bee7c628bd410f4fa18cdc3e8e8d3ee0875d"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"788625fbf2159359fa6d48a3b4766656883b6bb1","unresolved":true,"context_lines":[{"line_number":161,"context_line":"        map_merge:"},{"line_number":162,"context_line":"          - get_attr: [CinderBase, role_data, config_settings]"},{"line_number":163,"context_line":"          - get_attr: [ApacheServiceBase, role_data, config_settings]"},{"line_number":164,"context_line":"          - keystone_resources_managed: false"},{"line_number":165,"context_line":"          - cinder::keystone::authtoken::www_authenticate_uri: {get_param: [EndpointMap, KeystoneInternal, uri_no_suffix]}"},{"line_number":166,"context_line":"            cinder::keystone::authtoken::auth_url: {get_param: [EndpointMap, KeystoneInternal, uri_no_suffix]}"},{"line_number":167,"context_line":"            cinder::keystone::authtoken::password: {get_param: CinderPassword}"}],"source_content_type":"text/x-yaml","patch_set":6,"id":"901ac5c8_43ddf922","line":164,"updated":"2021-01-05 04:21:41.000000000","message":"Please see my earlier comment [1]. I believe this patch can remove L164, but only after the corresponding code is removed from puppet-tripleo\u0027s cinder/api.pp file. I was thinking the puppet-tripleo patch would come first, and this patch could depends-on it.\n\n[1] https://review.opendev.org/c/openstack/tripleo-heat-templates/+/768799/2/deployment/cinder/cinder-api-container-puppet.yaml#164\n\nBTW, deployment/keystone/keystone-container-puppet.yaml also sets keystone_resources_managed, but that one can be left alone. My focus is just on cinder\u0027s usage related to the volume type.","commit_id":"5a4f734e341be2aa8a42322776d3d63eb06ba628"}]}