)]}'
{"deployment/glance/glance-api-container-puppet.yaml":[{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"8e1c62880d03f7790c3b495fdaac83ad9e2bb72c","unresolved":true,"context_lines":[{"line_number":264,"context_line":"          cinder_store:"},{"line_number":265,"context_line":"            GlanceBackend: cinder"},{"line_number":266,"context_line":"            GlanceStoreDescription: \u0027Cinder store\u0027"},{"line_number":267,"context_line":"  CinderVolumeType:"},{"line_number":268,"context_line":"    default: None"},{"line_number":269,"context_line":"    type: string"},{"line_number":270,"context_line":"    description: |"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"3e0b0b07_62d0f7f8","line":267,"updated":"2021-02-05 15:58:22.000000000","message":"-1: Please name this something like GlanceCinderVolumeType. THT parameters that begin with \"Cinder\" are presumed to be cinder settings, and this is a glance setting.","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"ec4bd219835edcae82c6a0f689fbb802a9088856","unresolved":true,"context_lines":[{"line_number":264,"context_line":"          cinder_store:"},{"line_number":265,"context_line":"            GlanceBackend: cinder"},{"line_number":266,"context_line":"            GlanceStoreDescription: \u0027Cinder store\u0027"},{"line_number":267,"context_line":"  CinderVolumeType:"},{"line_number":268,"context_line":"    default: None"},{"line_number":269,"context_line":"    type: string"},{"line_number":270,"context_line":"    description: |"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"2e478557_73d8ed50","line":267,"in_reply_to":"3e0b0b07_62d0f7f8","updated":"2021-02-08 06:57:12.000000000","message":"Sure, will modify this in next patch set.","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"8e1c62880d03f7790c3b495fdaac83ad9e2bb72c","unresolved":true,"context_lines":[{"line_number":266,"context_line":"            GlanceStoreDescription: \u0027Cinder store\u0027"},{"line_number":267,"context_line":"  CinderVolumeType:"},{"line_number":268,"context_line":"    default: None"},{"line_number":269,"context_line":"    type: string"},{"line_number":270,"context_line":"    description: |"},{"line_number":271,"context_line":"      The volume type must be configured in cinder already which needs to be pass"},{"line_number":272,"context_line":"      here for glance multiple cinder stores configuration. For single cinder store"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"221a998a_51289793","line":269,"updated":"2021-02-05 15:58:22.000000000","message":"\"None\" is misleading, as all cinder volumes have an associated type. Perhaps you want to allow cinder to choose the default volume type? If that\u0027s the case then I recommend a default THT value such as \u0027\u0027 (empty string).","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"ec4bd219835edcae82c6a0f689fbb802a9088856","unresolved":true,"context_lines":[{"line_number":266,"context_line":"            GlanceStoreDescription: \u0027Cinder store\u0027"},{"line_number":267,"context_line":"  CinderVolumeType:"},{"line_number":268,"context_line":"    default: None"},{"line_number":269,"context_line":"    type: string"},{"line_number":270,"context_line":"    description: |"},{"line_number":271,"context_line":"      The volume type must be configured in cinder already which needs to be pass"},{"line_number":272,"context_line":"      here for glance multiple cinder stores configuration. For single cinder store"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"ce7c3e7d_43ad5a6e","line":269,"in_reply_to":"221a998a_51289793","updated":"2021-02-08 06:57:12.000000000","message":"I will pass empty string here in next patch set.\n\nFor single cinder store, cinder_volume_type should be None, I will pass empty string here.\n\nFor multiple cinder store, I\u0027m thinking to provide below parameters values,\nparameter_defaults:\n  GlanceBackendID: cinder1\n  GlanceBackend: cinder\n  CinderVolumeType: lvmdriver-1\n  GlanceStoreDescription: \u0027Default Cinder store\u0027\n  GlanceMultistoreConfig:\n    cinder2:\n      GlanceBackend: cinder\n      CinderVolumeType: nfsdriver-1,\n      GlanceStoreDescription: \u0027Second cinder store\u0027\n    cinder3:\n      GlanceBackend: cinder\n      CinderVolumeType: cephdriver-1,\n      GlanceStoreDescription: \u0027Third Cinder store\u0027","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"5399c1a13fab7e28726fa34e36092798f128ae0e","unresolved":true,"context_lines":[{"line_number":266,"context_line":"            GlanceStoreDescription: \u0027Cinder store\u0027"},{"line_number":267,"context_line":"  CinderVolumeType:"},{"line_number":268,"context_line":"    default: None"},{"line_number":269,"context_line":"    type: string"},{"line_number":270,"context_line":"    description: |"},{"line_number":271,"context_line":"      The volume type must be configured in cinder already which needs to be pass"},{"line_number":272,"context_line":"      here for glance multiple cinder stores configuration. For single cinder store"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"0b233580_f7859853","line":269,"in_reply_to":"ce7c3e7d_43ad5a6e","updated":"2021-02-08 15:42:55.000000000","message":"Conceptually, yes, but I assume you mean to use \u0027GlanceCinderVolumeType\u0027 everywhere (including in the GlanceMultistoreConfig value.","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"8e1c62880d03f7790c3b495fdaac83ad9e2bb72c","unresolved":true,"context_lines":[{"line_number":268,"context_line":"    default: None"},{"line_number":269,"context_line":"    type: string"},{"line_number":270,"context_line":"    description: |"},{"line_number":271,"context_line":"      The volume type must be configured in cinder already which needs to be pass"},{"line_number":272,"context_line":"      here for glance multiple cinder stores configuration. For single cinder store"},{"line_number":273,"context_line":"      it should be None."},{"line_number":274,"context_line":"  CephClientUserName:"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"138b25d8_a0b9f0f6","line":271,"range":{"start_line":271,"start_character":6,"end_line":271,"end_character":58},"updated":"2021-02-05 15:58:22.000000000","message":"I find the entire description to be confusing, but this particular portion is troubling. If you\u0027re saying any volume type must already exist, then how will this work with fresh deployments? TripleO only provides support for specifying cinder\u0027s default volume type, and currently there\u0027s no way to specify additional volume types, or to have them associated with specific cinder backends. So I\u0027m not sure how this is supposed to work.","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":19138,"name":"Pranali Deore","email":"pdeore@redhat.com","username":"PranaliD"},"change_message_id":"ec4bd219835edcae82c6a0f689fbb802a9088856","unresolved":true,"context_lines":[{"line_number":268,"context_line":"    default: None"},{"line_number":269,"context_line":"    type: string"},{"line_number":270,"context_line":"    description: |"},{"line_number":271,"context_line":"      The volume type must be configured in cinder already which needs to be pass"},{"line_number":272,"context_line":"      here for glance multiple cinder stores configuration. For single cinder store"},{"line_number":273,"context_line":"      it should be None."},{"line_number":274,"context_line":"  CephClientUserName:"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"5badd432_d91768af","line":271,"range":{"start_line":271,"start_character":6,"end_line":271,"end_character":58},"in_reply_to":"138b25d8_a0b9f0f6","updated":"2021-02-08 06:57:12.000000000","message":"Yes, TripleO currently only supports specifying cinder\u0027s default volume type.\n\nSeems we would need to provide this support in cinder tripleo because for multiple cinder store support in glance, the volume types needs to be set for each cinder backend,\n\nEx, suppose we\u0027ve 3 backends in cinder, lvm, nfs and ceph and 3 volume types in cinder pointing to each backend, lvmdriver-1, nfsdriver-1, cephdriver-1, on glance side we will have to set cinder_volume_type\u003dlvmdriver-1 for store 1, cinder_volume_type\u003dnfsdriver-1 for store 2 and cinder_volume_type\u003dcephdriver-1 for store 3.\n\nTo make this work, we will require the OOO support for cinder volume_type\n\nPlease suggest if something else is in your mind, if the solution I\u0027ve proposed is not doable.","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"5399c1a13fab7e28726fa34e36092798f128ae0e","unresolved":true,"context_lines":[{"line_number":268,"context_line":"    default: None"},{"line_number":269,"context_line":"    type: string"},{"line_number":270,"context_line":"    description: |"},{"line_number":271,"context_line":"      The volume type must be configured in cinder already which needs to be pass"},{"line_number":272,"context_line":"      here for glance multiple cinder stores configuration. For single cinder store"},{"line_number":273,"context_line":"      it should be None."},{"line_number":274,"context_line":"  CephClientUserName:"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"b39604b1_f63301a9","line":271,"range":{"start_line":271,"start_character":6,"end_line":271,"end_character":58},"in_reply_to":"5badd432_d91768af","updated":"2021-02-08 15:42:55.000000000","message":"This is definitely the direction we need to go for this feature to work, but we\u0027ll want to carefully consider how to handle it in TripleO. The general trend is to limit the use of puppet to configuring conf files, and use ansible for other tasks. I think we need to do some design work, and I would like to participate.","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"c130098cc207746f1f5fa1a9a9b1cf51d6c94348","unresolved":true,"context_lines":[{"line_number":531,"context_line":"              glance::backend::cinder::cinder_store_project_name: \u0027service\u0027"},{"line_number":532,"context_line":"              glance::backend::cinder::cinder_store_user_name: \u0027glance\u0027"},{"line_number":533,"context_line":"              glance::backend::cinder::cinder_store_password: {get_param: GlancePassword}"},{"line_number":534,"context_line":"              tripleo::profile::base::glance::backend::cinder::cinder_volume_type: {get_param: CinderVolumeType}"},{"line_number":535,"context_line":"            - {}"},{"line_number":536,"context_line":"          -"},{"line_number":537,"context_line":"            if:"}],"source_content_type":"text/x-yaml","patch_set":1,"id":"788a72b0_5f323750","line":534,"range":{"start_line":534,"start_character":95,"end_line":534,"end_character":111},"updated":"2021-02-04 13:45:59.000000000","message":"please add release notes for CinderVolumeType","commit_id":"150e6596cd63098fc16a4191a721a23e37cfd1ca"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"d7e5a932e695d601198f739680be90cc5d896b44","unresolved":true,"context_lines":[{"line_number":241,"context_line":"    description: |"},{"line_number":242,"context_line":"      Dictionary of settings when configuring additional glance backends. The"},{"line_number":243,"context_line":"      hash key is the backend ID, and the value is a dictionary of parameter"},{"line_number":244,"context_line":"      values unique to that backend. Multiple rbd \u0026 cinder backends are allowed, but"},{"line_number":245,"context_line":"      file and swift backends are limited to one each. Example:"},{"line_number":246,"context_line":"        # Default glance store is rbd."},{"line_number":247,"context_line":"        GlanceBackend: rbd"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"c3e51729_08e49f2f","line":244,"range":{"start_line":244,"start_character":50,"end_line":244,"end_character":51},"updated":"2021-04-12 13:50:58.000000000","message":"Do not abbreviate \"and\"","commit_id":"253be7f0fd703e962b7ca8cbadfc6e00838d7e83"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"d7e5a932e695d601198f739680be90cc5d896b44","unresolved":true,"context_lines":[{"line_number":258,"context_line":"            CephClientUserName: client2"},{"line_number":259,"context_line":"          cinder1_store:"},{"line_number":260,"context_line":"            GlanceBackend: cinder"},{"line_number":261,"context_line":"            GlanceCinderVolumeType: \u0027lvm-driver1\u0027"},{"line_number":262,"context_line":"            GlanceStoreDescription: \u0027First cinder store\u0027"},{"line_number":263,"context_line":"          cinder2_store:"},{"line_number":264,"context_line":"            GlanceBackend: cinder"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"f8cf8b10_dd80600b","line":261,"range":{"start_line":261,"start_character":36,"end_line":261,"end_character":49},"updated":"2021-04-12 13:50:58.000000000","message":"Recommend something like \"volume-type-1\" and \"-2\" at L265. Mentioning both LVM and \"driver\" are potentially misleading because neither terms are associated with volume types.","commit_id":"253be7f0fd703e962b7ca8cbadfc6e00838d7e83"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"d7e5a932e695d601198f739680be90cc5d896b44","unresolved":true,"context_lines":[{"line_number":275,"context_line":"    type: string"},{"line_number":276,"context_line":"    description: |"},{"line_number":277,"context_line":"      A unique volume type required for each cinder store while configuring multiple cinder"},{"line_number":278,"context_line":"      stores as glance backends. The same volume types must be configured in Cinder as well"},{"line_number":279,"context_line":"      (Cinder side configuration can be done after glance service is up \u0026 running)."},{"line_number":280,"context_line":"      For single cinder store no need to set volume type."},{"line_number":281,"context_line":"  CephClientUserName:"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"abb219c1_0c453776","line":278,"range":{"start_line":278,"start_character":0,"end_line":278,"end_character":1},"updated":"2021-04-12 13:50:58.000000000","message":"L278 needs full-stop \u0027.\u0027 to terminate the sentence.\n\nL279 can probably be deleted, or maybe get reworded. You correctly mention the volume type must be configured in cinder, so maybe the point to make here is the volume type must exist prior to any attempt to add an image in the associated cinder store.\n\nI\u0027m still not sure of L280. There are circumstances where an admin may wish to specify a volume type for glance images even when there\u0027s just one cinder store. I think what you may want to say is specifying a volume type is optional.","commit_id":"253be7f0fd703e962b7ca8cbadfc6e00838d7e83"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"643a690591139d00bc599241f36513901e5c28aa","unresolved":true,"context_lines":[{"line_number":276,"context_line":"    description: |"},{"line_number":277,"context_line":"      A unique volume type required for each cinder store while configuring multiple cinder"},{"line_number":278,"context_line":"      stores as glance backends. The same volume types must be configured in Cinder as well."},{"line_number":279,"context_line":"      (The Volume type must exist in both glance and cinder prior to any attempt to add an"},{"line_number":280,"context_line":"      image in the associated cinder store.)"},{"line_number":281,"context_line":"  CephClientUserName:"},{"line_number":282,"context_line":"    default: openstack"}],"source_content_type":"text/x-yaml","patch_set":4,"id":"7403ab4b_8391611f","line":279,"range":{"start_line":279,"start_character":0,"end_line":279,"end_character":1},"updated":"2021-04-14 15:13:35.000000000","message":"Overall this patch is looking very good, but I find several nits with this last sentence:\n- The opening and closing parentheses () are distracting and should be removed.\n- No need to capitalize Volume, so lead with \"The volume type...\"\n- AFAIK the volume type doesn\u0027t \"exist\" in glance (it\u0027s just a cinder concept).\n\nIt also probably helps the user to know that if GlanceCinderVolumeType isn\u0027t specified then images will be stored using cinder\u0027s default volume type. How about replacing L279..280 with something like this?\n\n\"The volume type must exist in cinder prior to any attempt to add an image in the associated cinder store. If no volume type is specified then cinder\u0027s default volume type will be used.\"","commit_id":"0c7d9fc638c42bd5560f4bf720d796900eb6fbef"},{"author":{"_account_id":14985,"name":"Alex Schultz","email":"aschultz@next-development.com","username":"mwhahaha"},"change_message_id":"5c1e8fc4f8e11d01948b55f284138d4e0572064f","unresolved":true,"context_lines":[{"line_number":517,"context_line":"              tripleo::profile::base::glance::backend::cinder::cinder_mount_point_base: {get_param: GlanceCinderMountPointBase}"},{"line_number":518,"context_line":"              tripleo::profile::base::glance::backend::cinder::cinder_volume_type: {get_param: GlanceCinderVolumeType}"},{"line_number":519,"context_line":"          -"},{"line_number":520,"context_line":"            if:"},{"line_number":521,"context_line":"            - cinder_multipathd_enabled"},{"line_number":522,"context_line":"            - glance::backend::cinder::cinder_use_multipath: true"},{"line_number":523,"context_line":"          - if:"}],"source_content_type":"text/x-yaml","patch_set":5,"id":"be26eed9_d4eb7b4d","line":520,"updated":"2021-04-19 14:24:06.000000000","message":"nit: don\u0027t need the line break between the - and if","commit_id":"addcee106e2f0de09f785dbfde57d1185404c39c"}]}