)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"3f7a8e6b3b44af3468b4c4e9bcc089fe12e32059","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"79c17563_7ebf3f7e","updated":"2022-06-30 12:33:50.000000000","message":"recheck","commit_id":"c9155a29bc1877eb37b61acff18c51b54f4c3eb6"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"186df96def457e4a81e5725b151e1e9864c1e588","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e21a202b_50b31b8f","updated":"2022-06-30 17:11:02.000000000","message":"Hi, just my initial feedback...\n\nHow do we envision the handling of the 2 parameters working together? If *Image is not set, look at *Images, or vice-versa? Would we deprecate the *Image parameters?\n\nI\u0027d like to see the interface into the ansible role be as clean as possible. It could still be a single var for the ansible role, if we had t-h-t (or another decoupled ansible role) write out OS version specific variable files, and then load the correct one based on the OS version fact.\n\nAlso, how do we see this integrating with openstack tripleo container image prepare? Would that command need to generate this new parameter as well? What if we updated the CiP defaults based off of a consistent naming scheme for older OS version images or something?\n\nWhat if we used entirely separate parameters for 8 based images, and then updated container-images/tripleo_containers.yaml.j2 (tripleo-common) to include generating parameter values for:\n\n\n  - imagename: \"{{namespace}}/{{name_prefix}}ovn-controller{{name_suffix}}{{os_suffix}}:{{tag}}\"\n    image_source: tripleo\n    params:\n    - ContainerOvnControllerConfigImage8\n    - ContainerOvnControllerImage8\n    services:\n    - OS::TripleO::Services::OVNController\n\n\nContainerImagePrepare could then set os_suffix to -rhel8 or -centosstream8.\n\nWe could then have a t-h-t composable service template that defined the *Image8 parameters, and wrote a vars file and loaded the correct one based on the gathered OS distribution facts.\n\nJust some ideas. I think I\u0027d like to see more of the end to end approach laid out before deciding on the t-h-t or ansible role interface.\n","commit_id":"62d1f6f2e5ad4a41515b4131a3d9a8e322036555"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"dff2f34341419df8747a3fe964c544006925eb81","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b536fc1e_1c1d1257","in_reply_to":"1557be8d_5b754b43","updated":"2022-07-01 00:12:01.000000000","message":"If changing the container names themselves is an option we can consider. Then yeah, I agree using os_suffix would be a much better solution. We could be more confident in the names when users have their own registry in play.\n\nSo, you think having two Heat params would be a cleaner solution than a single Param as a dictionary? The interface in tripleo-ansible would need to lookup a variable that contained a number rather than just a key in a single variable?\n\nDefinitely agree on the os_suffix. It will be much easier to implement. Maybe a new flag multi_rhel: true or multi_rhel: false by default. To allow users to avoid downloading two copies of every container if they don\u0027t want to use this feature. Which I imagine will be 80% a users.","commit_id":"62d1f6f2e5ad4a41515b4131a3d9a8e322036555"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"19014de3f61bac7f92889a011465172339ce8e9c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"1557be8d_5b754b43","in_reply_to":"e21a202b_50b31b8f","updated":"2022-06-30 23:39:14.000000000","message":"Hey,\n\nSo, I think the overall solution depends on whether we want to do this for every single container or just the ones that require it. If it\u0027s only nova_virtqemud and ovn_controller that have host OS dependencies. We might be better off just handling them as edge cases and keeping consistency with the cluster for everything else. Like, logrotate containers, nova_compute, etc. They probably don\u0027t need to run a OS specific container and might be better off sticking with the rest of the cluster.\n\n\nDefinitely needs an interface in tripleo-common. I have outlined the changes I believe will be required on a internal Jira ticket. But essentially, if we want to do it for every container:\n\ntripleo-common:\n1. We can change the namespace: arg in CIP to `namespaces: [\u0027rhosp-rhel8\u0027, \u0027rhosp-rhel9\u0027`. \n1a. For containers that require multi-rhel, we can `for namespace in namespaces:` collect all of the images for both OS versions\n1b. Generate the new Heat parameter proposed here as a dictionary.\n\n\ntripleo-heat-templates:\nAs proposed here. Write out the *Images parameter to group_vars\n\ntripleo-ansible:\nUse `ansible_facts[\u0027distribution_major_version\u0027]` as the key to lookup the value from the dictionary. Which I have added to this change for example:\nhttps://review.opendev.org/c/openstack/tripleo-ansible/+/845653/31/tripleo_ansible/roles/tripleo_ovn/templates/ovn_controller.yaml.j2#2\n\n\nIf we have the ability to change the naming schema of the containers themselves, I didn\u0027t consider that as an option. My main concern is how we handle it for cases where the user is using a custom container registry and we don\u0027t have control over what they are calling things. So the interface in CIP is essential. But it was the more challenging of the list of problems, so I wanted to verify that this path would even be viable before tackling that one.","commit_id":"62d1f6f2e5ad4a41515b4131a3d9a8e322036555"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"0efeaff66266cd82e942d108293f13f550cfad44","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"2d802400_b2f0c36c","updated":"2022-07-18 05:08:28.000000000","message":"This will also depend on:\nhttps://review.opendev.org/c/openstack/tripleo-common/+/850131","commit_id":"f2acb3457acc04580d86550483dce5353d1f37b7"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"e71b9baebfae2de9f7bc17bedb89065a1026449a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"45fe7242_0f774107","in_reply_to":"2d802400_b2f0c36c","updated":"2022-07-19 04:12:32.000000000","message":"Sorted. I\u0027ll keep the existing param there in tripleo-common until we can deprecate and remove it properly","commit_id":"f2acb3457acc04580d86550483dce5353d1f37b7"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"c399bcc2a04fdbba13e2ad2fd9284ad032cf0ec0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"ec525d2e_52eb0e46","updated":"2022-07-19 05:34:47.000000000","message":"o/ question inline - should this be wallaby only?","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":6816,"name":"Jesse Pretorius","email":"jesse@odyssey4.me","username":"jesse-pretorius"},"change_message_id":"05a5d6d9b4f8d2edf9ac9e92c455cf02ca19e148","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":22,"id":"a86c3862_52b27010","updated":"2022-07-28 09:09:53.000000000","message":"Adding Lukas \u0026 Daniel (Juan is already added) to the mix for reviews given that this is in the Upgrade domain. This also affects Updates, so also adding Owen \u0026 Mikolaj (Sofer is already added).","commit_id":"4c2654515772dc81c0e24efca3eac3c0ed3deaf8"}],"deployment/ovn/ovn-controller-container-puppet.yaml":[{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"4fc71f5578c51ea8718af996c33770e766cfdd52","unresolved":true,"context_lines":[{"line_number":272,"context_line":"           - {equals: [{get_param: ContainerOvnControllerImages }, {}]}"},{"line_number":273,"context_line":"           - False"},{"line_number":274,"context_line":"           - True"},{"line_number":275,"context_line":"        tripleo_ovn_controller_image: {get_param: ContainerOvnControllerImage}"},{"line_number":276,"context_line":"        tripleo_ovn_controller_images: {get_param: ContainerOvnControllerImages}"},{"line_number":277,"context_line":"        tripleo_ovn_bridge: {get_param: OVNIntegrationBridge}"},{"line_number":278,"context_line":"        tripleo_ovn_bridge_mappings: {get_param: NeutronBridgeMappings}"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"1a5d83a4_c79498b8","line":275,"updated":"2022-07-06 09:01:33.000000000","message":"So we will drop this one entirely, and just use the *Images dict variable below. This would allow for handling different images if the tags aren\u0027t the same. For example, rhosp-rhel9/ovn-controller-rhel9:17.1.1-527 and rhosp-rhel8/ovn-controller-rhel8:17.1.1-897.","commit_id":"1f49f71ad0d8c0cfd73d271b119a531e681706f7"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"e71b9baebfae2de9f7bc17bedb89065a1026449a","unresolved":false,"context_lines":[{"line_number":272,"context_line":"           - {equals: [{get_param: ContainerOvnControllerImages }, {}]}"},{"line_number":273,"context_line":"           - False"},{"line_number":274,"context_line":"           - True"},{"line_number":275,"context_line":"        tripleo_ovn_controller_image: {get_param: ContainerOvnControllerImage}"},{"line_number":276,"context_line":"        tripleo_ovn_controller_images: {get_param: ContainerOvnControllerImages}"},{"line_number":277,"context_line":"        tripleo_ovn_bridge: {get_param: OVNIntegrationBridge}"},{"line_number":278,"context_line":"        tripleo_ovn_bridge_mappings: {get_param: NeutronBridgeMappings}"}],"source_content_type":"text/x-yaml","patch_set":7,"id":"cb84d039_fd45b9d0","line":275,"in_reply_to":"1a5d83a4_c79498b8","updated":"2022-07-19 04:12:32.000000000","message":"Done","commit_id":"1f49f71ad0d8c0cfd73d271b119a531e681706f7"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"4fc71f5578c51ea8718af996c33770e766cfdd52","unresolved":true,"context_lines":[{"line_number":358,"context_line":"            privileged: true"},{"line_number":359,"context_line":"            user: root"},{"line_number":360,"context_line":"            command: [\u0027/bin/bash\u0027, \u0027-c\u0027, \u0027CMS_OPTS\u003d$(hiera ovn::controller::ovn_cms_options -c /etc/puppet/hiera.yaml);  if [ X\"$CMS_OPTS\" !\u003d  X ]; then ovs-vsctl set open . external_ids:ovn-cms-options\u003d$CMS_OPTS;else ovs-vsctl remove open . external_ids ovn-cms-options; fi\u0027]"},{"line_number":361,"context_line":"            image: \"{{ tripleo_ovn_controller_images[ansible_facts[\u0027distribution_major_version\u0027]] | default(tripleo_ovn_controller_image) }}\""},{"line_number":362,"context_line":"            volumes:"},{"line_number":363,"context_line":"              list_concat:"},{"line_number":364,"context_line":"                 - {get_attr: [ContainersCommon, volumes]}"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"a7fa5e5f_cd8853e5","line":361,"updated":"2022-07-06 09:01:33.000000000","message":"While we decide on how to handle to triplet-common side. I\u0027ll just leave this with a default to the single image.","commit_id":"7f84708b9fced097add790c117f768a4a212302e"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"e71b9baebfae2de9f7bc17bedb89065a1026449a","unresolved":false,"context_lines":[{"line_number":358,"context_line":"            privileged: true"},{"line_number":359,"context_line":"            user: root"},{"line_number":360,"context_line":"            command: [\u0027/bin/bash\u0027, \u0027-c\u0027, \u0027CMS_OPTS\u003d$(hiera ovn::controller::ovn_cms_options -c /etc/puppet/hiera.yaml);  if [ X\"$CMS_OPTS\" !\u003d  X ]; then ovs-vsctl set open . external_ids:ovn-cms-options\u003d$CMS_OPTS;else ovs-vsctl remove open . external_ids ovn-cms-options; fi\u0027]"},{"line_number":361,"context_line":"            image: \"{{ tripleo_ovn_controller_images[ansible_facts[\u0027distribution_major_version\u0027]] | default(tripleo_ovn_controller_image) }}\""},{"line_number":362,"context_line":"            volumes:"},{"line_number":363,"context_line":"              list_concat:"},{"line_number":364,"context_line":"                 - {get_attr: [ContainersCommon, volumes]}"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"8536f17b_51807c77","line":361,"in_reply_to":"a7fa5e5f_cd8853e5","updated":"2022-07-19 04:12:32.000000000","message":"Done","commit_id":"7f84708b9fced097add790c117f768a4a212302e"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"e71b9baebfae2de9f7bc17bedb89065a1026449a","unresolved":true,"context_lines":[{"line_number":224,"context_line":"    type: OS::Heat::Value"},{"line_number":225,"context_line":"    properties:"},{"line_number":226,"context_line":"      type: json"},{"line_number":227,"context_line":"      value:"},{"line_number":228,"context_line":"        ContainerOvnControllerImage:"},{"line_number":229,"context_line":"          \u00279\u0027: {get_param: ContainerOvnControllerImage9}"},{"line_number":230,"context_line":"          \u00278\u0027: {get_param: ContainerOvnControllerImage8}"},{"line_number":231,"context_line":"        ContainerOvnControllerConfigImage:"},{"line_number":232,"context_line":"          \u00279\u0027: {get_param: ContainerOvnControllerConfigImage9}"},{"line_number":233,"context_line":"          \u00278\u0027: {get_param: ContainerOvnControllerConfigImage8}"},{"line_number":234,"context_line":"        map_replace:"},{"line_number":235,"context_line":"          - map_replace:"},{"line_number":236,"context_line":"            - ovn::controller::ovn_bridge_mappings: NeutronBridgeMappings"},{"line_number":237,"context_line":"              ovn::controller::ovn_cms_options:"},{"line_number":238,"context_line":"                if:"},{"line_number":239,"context_line":"                  - az_ovn_unset"},{"line_number":240,"context_line":"                  - OVNCMSOptions"},{"line_number":241,"context_line":"                  - list_join:"},{"line_number":242,"context_line":"                    - \u0027\u0027"},{"line_number":243,"context_line":"                    - - OVNCMSOptions"},{"line_number":244,"context_line":"                      - \",availability-zones\u003d\""},{"line_number":245,"context_line":"                      - {get_param: OVNAvailabilityZone}"},{"line_number":246,"context_line":"              vswitch::ovs::enable_hw_offload: OvsHwOffload"},{"line_number":247,"context_line":"              container_cpuset_cpus: OVNContainerCpusetCpus"},{"line_number":248,"context_line":"            - values: {get_param: [RoleParameters]}"},{"line_number":249,"context_line":"          - values:"},{"line_number":250,"context_line":"              NeutronBridgeMappings: {get_param: NeutronBridgeMappings}"},{"line_number":251,"context_line":"              OVNCMSOptions: {get_param: OVNCMSOptions}"},{"line_number":252,"context_line":"              OvsHwOffload: {get_param: OvsHwOffload}"},{"line_number":253,"context_line":"              OVNContainerCpusetCpus: {get_param: OVNContainerCpusetCpus}"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"  OVNBridgeMappingsValue:"},{"line_number":256,"context_line":"    type: OS::Heat::Value"}],"source_content_type":"text/x-yaml","patch_set":16,"id":"7bfbf9f7_7775f2e3","line":253,"range":{"start_line":227,"start_character":0,"end_line":253,"end_character":73},"updated":"2022-07-19 04:12:32.000000000","message":"This doesn\u0027t work actually. Need to review this and restructure","commit_id":"e9e0309ea81db81de1335989940be18135d8d1bb"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"6fe882dab8c9ba641bfa38aa76441a927410066a","unresolved":true,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"9016417e_248e7237","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"updated":"2022-07-19 07:53:19.000000000","message":"If we can assign different roles according to OS versions then we can leverage this existing capability to use different container images per role.\n\nDid we consider that approach ?\n\nIMO current implementation with multiple image parameters are quite tricky and is hard to maintain.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"a1eb99988c6534184993edaf31adda9e903fcbb3","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"4523699b_0f69ed2e","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"3b206331_df590ff4","updated":"2022-07-26 23:32:20.000000000","message":"Here\u0027s my objection to using composable roles without a mechanism to select the OS based on facts from the host.\n\n1. It requires that the customer needs to first upgrade their undercloud. Then they need to go and edit files to split specific nodes from their existing Compute role into NewCompute role. Do we suggest they just use a Hostname map to make sure the existing hostnames remain unchanged?\n\n```\nHostnameMap:\n  overcloud-newcompute-0: overcloud-compute-71\n  overcloud-newcompute-1: overcloud-compute-72\n```\n\n\nThey would need to selectively go through all of their hosts and add them to this hostname map to work with the new composable role. Which isn\u0027t a great customer experience. Which is my second point.\n\n2. I believe it will be a better user experience if this is handled by TripleO. Rather than relying on the user to manually edit files before running the upgrade. I feel like we\u0027re going to get into more trouble going this route, with more calls for support than if we just handle it for them with a solution like the proposed here.\n\nSo they run the upgrade with --limit to exclude the hosts they don\u0027t want upgraded. Then, we have the required logic to support day 2 ops with nothing additional required from the user.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"8deddd03c6ac01aba328d46ba367b862d2f1c5d7","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"3b206331_df590ff4","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"3f745446_281d4605","updated":"2022-07-26 23:00:32.000000000","message":"Yeah, I\u0027m hearing everyones feedback that we don\u0027t like handling this in THT. \n\nI\u0027m working on adding something to tripleo-common to handle this atm. I\u0027ll push a new patchiest soonish once I have had some time to test it. So we\u0027ll generate the param in CIP as a map of the two items I\u0027m setting up here. Then we can leave THT mostly unchanged except for the j2 variable in the docker_config file.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"d00b36971bb39a16ec2318d04db5cba34d534e89","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"8d177e37_3d0ad574","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"4523699b_0f69ed2e","updated":"2022-07-27 02:55:21.000000000","message":"\u003e Then they need to go and edit files to split specific nodes from their existing Compute role into NewCompute role.\n\nAs commented earlier, all these can automated with upgrade. We already generate baremetal_deployment.yaml with existing hostnames[1]. \n\n\u003e  I feel like we\u0027re going to get into more trouble going this route, with more calls for support than if we just handle it for them with a solution like the proposed here. \n\nI won\u0027t agree. With separate role, user decides the nodes (they always) they don\u0027t want to upgrade and puts them in separate role with no magic of auto detecting os and conditional hacks in templates etc. With magic there would be more support cases and very difficult to detect any issue related to mixed version, when there are large number of nodes.\n\n[1] https://github.com/openstack/python-tripleoclient/blob/stable/wallaby/tripleoclient/v1/overcloud_node.py#L558","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"4bbad44ff00270b58e784e36ed15fff340e1915a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"ff4c8a39_8dec4a4f","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"617624a9_7cfb9eee","updated":"2022-07-19 09:30:17.000000000","message":"Hmm. OK. That make sense.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"19dc54f4b0bbbf0f53ad7ddefbc1db4faba06130","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"e2ac2516_10e6d194","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"72e156e2_20f6d793","updated":"2022-07-20 03:47:40.000000000","message":"\u003e Seems more difficult to automate, maybe some manual steps required by the user\n\nI don\u0027t see why it would be difficult to automate. As I mentioned earlier node extract provisioned (or a new command) would/can put the nodes (with existing hostnames and ips, as  decided by the user) in the new roles of both baremetal_deployment.yaml  and modified role_data.yaml. Anyway we generate the stack and ansible content everytime, so the overall workflow would remain the same.\n\nAlso, I think this way there would be clear separation/identification in deployment for old/upgraded nodes in the future without looking at the nodes.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"d3a261b9f16841f70cdcc186bb02b885389dc0a2","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"99e6a5da_031754b9","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"8d177e37_3d0ad574","updated":"2022-07-28 03:49:17.000000000","message":"Given that the majority of this conversation concerns the upgrade process. I would also like the opinion of the upgrades team as well.\n\nAdding @Sofer for contribution to the discussion.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"5e1188f806563fa302861ccfa2da65c7e8646fcf","unresolved":true,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"617624a9_7cfb9eee","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"9016417e_248e7237","updated":"2022-07-19 08:55:54.000000000","message":"Hey Takashi,\n\nUnfortunately, no that isn\u0027t an option here. The expectation is that customers will have a pre-existing deployment and will only want to upgrade some of the Compute nodes. So we need to be able to handle this on a per node basis and we also need to assume that the mixed nodes will exist within pre-existing roles and deployments. So 10 out of 50 Compute nodes for example might need to stay on a old version of RHEL, while the remaining 40 will be upgraded to the new version of RHEL.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":7144,"name":"James Slagle","email":"jslagle@redhat.com","username":"slagle"},"change_message_id":"cdc85c359d9d4bbc54aeb76ebdcb8beec3079caf","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"3f745446_281d4605","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"b0ea8b6e_f85edcf4","updated":"2022-07-26 20:56:58.000000000","message":"Custom roles solves for the re-use of the parameter. There may be other corner cases that need to be explored though with re-rolling an existing node. It needs investigation.\n\nContainer image prepare is still an open question though. We need to prepare the image for both OS versions. As CiP is driven by the parameters, how would we signal to that code that there are actually multiple images to be prepared for a single parameter? We\u0027d likely need some additional special case handling like we do for ceph in container_image_prepare_defaults.yaml.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"435458a82efe3551966021b6ec1fe35d6224bad1","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"72e156e2_20f6d793","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"df66ba7b_e83d6456","updated":"2022-07-20 02:24:02.000000000","message":"We can explore this. So you\u0027re proposal for customers upgrading from OSP16.2 \u003e 17. Would be to do all of this in the baremetal_deployment.yaml file instead of having tripleo handle it during the deployment?\n\nSomething like, \n```\n- name: Compute\n  hostname_format: overcloud-compute-%index%\n  count: 1\n  defaults:\n    profile: compute\n    managed: false\n  instances:\n  - hostname: overcloud-compute-0\n    networks:\n    - network: ctlplane\n      subnet: ctlplane_subnet\n      fixed_ip: 192.168.1.6\n    - network: internal_api\n      subnet: internal_api_subnet\n      fixed_ip: 172.16.2.84\n    - network: tenant\n      subnet: tenant_subnet\n      fixed_ip: 172.16.0.243\n    - network: storage\n      subnet: storage_subnet\n      fixed_ip: 172.16.1.147\n    - network: external\n      subnet: external_subnet\n      fixed_ip: 172.20.12.67\n    network_config:\n      template: /home/stack/templates/nic-configs/bonds_vlans.j2\n      default_route_network:\n      - external\n- name: ComputeRHEL8\n  hostname_format: overcloud-computerhel8-%index%\n  count: 1\n  defaults:\n    profile: compute\n    managed: false\n  instances:\n  - hostname: overcloud-compute-70\n    networks:\n    - network: ctlplane\n      subnet: ctlplane_subnet\n      fixed_ip: 192.168.1.70\n    - network: internal_api\n      subnet: internal_api_subnet\n      fixed_ip: 172.16.2.85\n    - network: tenant\n      subnet: tenant_subnet\n      fixed_ip: 172.16.0.244\n    - network: storage\n      subnet: storage_subnet\n      fixed_ip: 172.16.1.148\n    - network: external\n      subnet: external_subnet\n      fixed_ip: 172.20.12.68\n    network_config:\n      template: /home/stack/templates/nic-configs/bonds_vlans.j2\n      default_route_network:\n      - external\n```\n\nFor example. So we want to keep overcloud-compute-70 on RHEL8, so now we\u0027re adding it to a new role. Maintaining the IP\u0027s so that Ansible can address it in the same way.\n\nSeems more difficult to automate, maybe some manual steps required by the user. But leaving nodes on an old OS should be a very intentional decision anyway. So it\u0027s not the worst solution if they need to manually select and define the nodes to remain on RHEL8 this way?","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":6926,"name":"Bogdan Dobrelya","email":"bdobreli@redhat.com","username":"bogdando"},"change_message_id":"7fff4064483c1c6f90519707164d691aa64324de","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"b0ea8b6e_f85edcf4","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"e2ac2516_10e6d194","updated":"2022-07-26 13:52:01.000000000","message":"Using custom roles to allow OS versions fall behind is a good solution that fits into composable roles greatly. My vote goes into that. Shall the spec become updated to reflect that move?","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"8475bd3fb2aed0725ec6da7cabb7a5f5d5e95ce7","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: string"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"},{"line_number":32,"context_line":"    default: \u0027\u0027"},{"line_number":33,"context_line":"    description: The container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"df66ba7b_e83d6456","line":30,"range":{"start_line":30,"start_character":8,"end_line":30,"end_character":21},"in_reply_to":"ff4c8a39_8dec4a4f","updated":"2022-07-19 17:35:39.000000000","message":"\u003e existing deployment and will only want to upgrade some of the Compute nodes\n\nNot sure why that could be an issue? Can\u0027t baremetal_deployment.yaml generated from extract provision command and role_data.yaml be tweaked to carve out new role with the nodes that would only be upgraded. node extract provision command can probably be changed to automate that. That way we can easily leverage existing role specific parameters feature in THT.\n\nI think templates containing version specific parameters is not a good idea and would make the templates unmanageable in the long run.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"c399bcc2a04fdbba13e2ad2fd9284ad032cf0ec0","unresolved":true,"context_lines":[{"line_number":46,"context_line":"    type: string"},{"line_number":47,"context_line":"    tags:"},{"line_number":48,"context_line":"      - role_specific"},{"line_number":49,"context_line":"  ContainerOvnControllerConfigImage8:"},{"line_number":50,"context_line":"    default: \u0027\u0027"},{"line_number":51,"context_line":"    description: The EL8 container image to use for the ovn_controller config_volume"},{"line_number":52,"context_line":"    type: string"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"2b546022_526773a9","line":49,"range":{"start_line":49,"start_character":35,"end_line":49,"end_character":36},"updated":"2022-07-19 05:34:47.000000000","message":"do we really need/want this on master? it can go straight to wallaby imo\n\nmutli OS support will be restricted to wallaby\n\nwe have no centos8 for master any more.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"bbc1998786e18f19eeac1b0a12f6fa882ff2c23b","unresolved":true,"context_lines":[{"line_number":46,"context_line":"    type: string"},{"line_number":47,"context_line":"    tags:"},{"line_number":48,"context_line":"      - role_specific"},{"line_number":49,"context_line":"  ContainerOvnControllerConfigImage8:"},{"line_number":50,"context_line":"    default: \u0027\u0027"},{"line_number":51,"context_line":"    description: The EL8 container image to use for the ovn_controller config_volume"},{"line_number":52,"context_line":"    type: string"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"bc56d920_43e85fc7","line":49,"range":{"start_line":49,"start_character":35,"end_line":49,"end_character":36},"in_reply_to":"2b546022_526773a9","updated":"2022-07-19 08:56:55.000000000","message":"There will be a fairly significant number of changes to the architecture here to handle the different container images. I don\u0027t think we want Wallaby to stray too far away from what is happening on master?","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"8c34efae26419880fa3db6d40af941ebdfc0901d","unresolved":true,"context_lines":[{"line_number":46,"context_line":"    type: string"},{"line_number":47,"context_line":"    tags:"},{"line_number":48,"context_line":"      - role_specific"},{"line_number":49,"context_line":"  ContainerOvnControllerConfigImage8:"},{"line_number":50,"context_line":"    default: \u0027\u0027"},{"line_number":51,"context_line":"    description: The EL8 container image to use for the ovn_controller config_volume"},{"line_number":52,"context_line":"    type: string"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"e4162ea2_94371199","line":49,"range":{"start_line":49,"start_character":35,"end_line":49,"end_character":36},"in_reply_to":"bc56d920_43e85fc7","updated":"2022-07-20 07:11:11.000000000","message":"understood. problem is we won\u0027t be able to CI this on master.\n\nor rather, we may need to reconsider and bring back some c8 for master in order to be able to CI this. \n\notherwise the c8/9 mixed-os job I\u0027m working on is for wallaby only at least currently.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"4bbad44ff00270b58e784e36ed15fff340e1915a","unresolved":true,"context_lines":[{"line_number":51,"context_line":"    description: The EL8 container image to use for the ovn_controller config_volume"},{"line_number":52,"context_line":"    type: string"},{"line_number":53,"context_line":"    tags:"},{"line_number":54,"context_line":"      - role_specific"},{"line_number":55,"context_line":"  ContainerOvnControllerConfigImage9:"},{"line_number":56,"context_line":"    default: \u0027\u0027"},{"line_number":57,"context_line":"    description: The EL9 container image to use for the ovn_controller config_volume"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"7216e289_093e9771","line":54,"range":{"start_line":54,"start_character":8,"end_line":54,"end_character":21},"updated":"2022-07-19 09:30:17.000000000","message":"If I read the implementation correctly, the current version does not handle role specific definitions correctly.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"4bbad44ff00270b58e784e36ed15fff340e1915a","unresolved":true,"context_lines":[{"line_number":199,"context_line":"parameter_groups:"},{"line_number":200,"context_line":"- label: deprecated"},{"line_number":201,"context_line":"  description: Do not use deprecated params, they will be removed."},{"line_number":202,"context_line":"  parameters:"},{"line_number":203,"context_line":"    - ContainerOvnControllerImage"},{"line_number":204,"context_line":"    - ContainerOvnControllerConfigImage"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"conditions:"},{"line_number":207,"context_line":"  auth_cloud_name_set:"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"166d4d91_80fce601","line":204,"range":{"start_line":202,"start_character":13,"end_line":204,"end_character":39},"updated":"2022-07-19 09:30:17.000000000","message":"I don\u0027t think this is a good direction. If we do this then we are forced to update these parameters when we update base OS again (for example when we adopt to CentOS 10). IMO adding only EL8 versions makes future maintenance much easier.\n\nYou can generate images map like\n\n\nOvncontrollerImages:\n  \u0027default\u0027: {get_param: ContainerOvnControllerImage}\n  \u00278\u0027: {get_param: ContainerOvnControllerImage8}\n\nand implement the logic to use the default one if the release version is not present as a key.\n\nAlternatively we might want to compose up the override dictionary which looks like\n\nContainerOvnControllerImageOverride:\n  \u00278\u0027: {get_param: ContainerOvnControllerImage8}\n\n\nbut at this moment its feasibility is unclear and I need to dig into tripleo-common for further observation.","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"00450174f91bb2cb18449f5a712fef753e2d29e5","unresolved":false,"context_lines":[{"line_number":199,"context_line":"parameter_groups:"},{"line_number":200,"context_line":"- label: deprecated"},{"line_number":201,"context_line":"  description: Do not use deprecated params, they will be removed."},{"line_number":202,"context_line":"  parameters:"},{"line_number":203,"context_line":"    - ContainerOvnControllerImage"},{"line_number":204,"context_line":"    - ContainerOvnControllerConfigImage"},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"conditions:"},{"line_number":207,"context_line":"  auth_cloud_name_set:"}],"source_content_type":"text/x-yaml","patch_set":19,"id":"c11257ab_121d0cd1","line":204,"range":{"start_line":202,"start_character":13,"end_line":204,"end_character":39},"in_reply_to":"166d4d91_80fce601","updated":"2022-07-19 12:39:36.000000000","message":"Ack","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"6e2f1e9c68b7250b02a8e7920d9571eba32ddcaf","unresolved":true,"context_lines":[{"line_number":34,"context_line":"    type: string"},{"line_number":35,"context_line":"    tags:"},{"line_number":36,"context_line":"      - role_specific"},{"line_number":37,"context_line":"  ContainerOvnControllerImage8:"},{"line_number":38,"context_line":"    default: \u0027\u0027"},{"line_number":39,"context_line":"    type: string"},{"line_number":40,"context_line":"    description: image"}],"source_content_type":"text/x-yaml","patch_set":21,"id":"ab20f493_fdc92092","line":37,"updated":"2022-07-21 11:24:25.000000000","message":"It\u0027s a little unfortunate that we have to base this on a heat parameter this way. I can see one of the big reasons for not simply having an OvnControllerImages heat parameter instead is that our \"what containers do I need to pull down\" logic based in the tripleo-common lists depends on heat parameters. I wonder if there is a way to extend *that* implementation and have container-images-prepare be able to define a map and the tripleoclient conditionally trigger it. Then the template parameters don\u0027t have to care about versions at all?","commit_id":"90f9615265f120e9e6a06079db7940381a44ff25"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"d2909018d5f0d2c73dae66310f771d485390acbc","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    type: string"},{"line_number":35,"context_line":"    tags:"},{"line_number":36,"context_line":"      - role_specific"},{"line_number":37,"context_line":"  ContainerOvnControllerImage8:"},{"line_number":38,"context_line":"    default: \u0027\u0027"},{"line_number":39,"context_line":"    type: string"},{"line_number":40,"context_line":"    description: image"}],"source_content_type":"text/x-yaml","patch_set":21,"id":"785ca483_6f156806","line":37,"in_reply_to":"76aad5e0_307c99b1","updated":"2022-08-07 23:12:30.000000000","message":"Done this now:\nhttps://review.opendev.org/c/openstack/tripleo-common/+/850131","commit_id":"90f9615265f120e9e6a06079db7940381a44ff25"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"81556cc48ec35aac0aa99e4a353460ad2839b025","unresolved":true,"context_lines":[{"line_number":34,"context_line":"    type: string"},{"line_number":35,"context_line":"    tags:"},{"line_number":36,"context_line":"      - role_specific"},{"line_number":37,"context_line":"  ContainerOvnControllerImage8:"},{"line_number":38,"context_line":"    default: \u0027\u0027"},{"line_number":39,"context_line":"    type: string"},{"line_number":40,"context_line":"    description: image"}],"source_content_type":"text/x-yaml","patch_set":21,"id":"76aad5e0_307c99b1","line":37,"in_reply_to":"ab20f493_fdc92092","updated":"2022-07-21 12:38:37.000000000","message":"I completely agree. We would ideally generate a map param with tripleo-common. Given the current architecture in tripleo-common, this was the easiest way to get something working for testing and validation purposes. But ultimately, if we choose to take this approach, it would definitely be best to have the single param that contained a map of image URL\u0027s and avoid creating this within the Heat template.","commit_id":"90f9615265f120e9e6a06079db7940381a44ff25"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"98ec562f9c6b38b1fa5af00244a2e4dbbd0633fc","unresolved":true,"context_lines":[{"line_number":40,"context_line":"    description: image"},{"line_number":41,"context_line":"    tags:"},{"line_number":42,"context_line":"      - role_specific"},{"line_number":43,"context_line":"  ContainerOvnControllerConfigImage8:"},{"line_number":44,"context_line":"    default: \u0027\u0027"},{"line_number":45,"context_line":"    description: The EL8 container image to use for the ovn_controller config_volume"},{"line_number":46,"context_line":"    type: string"}],"source_content_type":"text/x-yaml","patch_set":21,"id":"809c9554_9021730a","line":43,"range":{"start_line":43,"start_character":35,"end_line":43,"end_character":36},"updated":"2022-07-19 14:46:02.000000000","message":"Why do You want to have \"hard coded\" version number (8) there? Wouldn\u0027t be better maybe to have something like \"Alt\" or \"Old\" instead? That way the same thing can be used later for e.g. RHEL9 and RHEL10","commit_id":"90f9615265f120e9e6a06079db7940381a44ff25"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"435458a82efe3551966021b6ec1fe35d6224bad1","unresolved":true,"context_lines":[{"line_number":40,"context_line":"    description: image"},{"line_number":41,"context_line":"    tags:"},{"line_number":42,"context_line":"      - role_specific"},{"line_number":43,"context_line":"  ContainerOvnControllerConfigImage8:"},{"line_number":44,"context_line":"    default: \u0027\u0027"},{"line_number":45,"context_line":"    description: The EL8 container image to use for the ovn_controller config_volume"},{"line_number":46,"context_line":"    type: string"}],"source_content_type":"text/x-yaml","patch_set":21,"id":"10fa9629_af63b75a","line":43,"range":{"start_line":43,"start_character":35,"end_line":43,"end_character":36},"in_reply_to":"809c9554_9021730a","updated":"2022-07-20 02:24:02.000000000","message":"Just because of the way I\u0027m using Ansible to lookup this value. But I guess that could be changed to do anything we want. In an effort to keep it simple though, I was using `ansible_facts[\u0027distribution_major_version\u0027]` to lookup the key from this dictionary.\n\nhttps://review.opendev.org/c/openstack/tripleo-heat-templates/+/848208/21/deployment/ovn/ovn-controller-container-puppet.yaml#373","commit_id":"90f9615265f120e9e6a06079db7940381a44ff25"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"57057979add1267f8040086800f74a11e4a9cd61","unresolved":true,"context_lines":[{"line_number":23,"context_line":"    description: Dictionary packing service data"},{"line_number":24,"context_line":"    type: json"},{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"6c84aadc_4f381a35","line":26,"updated":"2022-08-18 17:41:36.000000000","message":"If json then this should be {}.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"15f341a57987a9329558534dfa95b4a1e7634bc0","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"715281b7_ae7ad05d","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"updated":"2022-08-13 02:28:59.000000000","message":"Sorry not HEAT expert, but changing type is a breaking change right? If so we will need to provide backwards compatibility, right?","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":11166,"name":"Lukas Bezdicka","email":"social@v3.sk","username":"xbezdick"},"change_message_id":"3a570291f0639901c8fd3dd4c99a0146eed8d4ac","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"3a03d20e_9ed42fff","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"1dd8406f_7f7a4fb1","updated":"2022-08-23 22:33:56.000000000","message":"We are close to get passing CI with this approach while I can\u0027t even imagine how I\u0027d automate the role shuffling scenario. I think we should accept breaking change in the containers preparation - for example ContainerOvnControllerImage - which throws reasonable error right away.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"36caf49edc647f08f8d0655c6fa9b06489053f4b","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"fbbf76c3_ea45c0ee","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"328758e0_1e4acc5f","updated":"2022-08-23 04:22:15.000000000","message":"What are the exact issues and the corner cases with composable roles and leveraging role_specific container image parameters as suggested earlier. We can change container image prepare to cater to that? I\u0027ve not heard of any specific concern other than it\u0027s more work during upgrade. I believe that should not force us to use a hacky approach here or break all backward compatibility with the existing interfaces.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"62a9125c8998a885a7ed3f638fc9218e72483dec","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"31fae8c7_43702c4a","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"3a03d20e_9ed42fff","updated":"2022-08-24 04:38:00.000000000","message":"This current implementation even breaks non-multi-rhel scenario because ContainerOvnControllerImage is still getting a string value unless \"multil_rhel\" is not set.\n\nEven if we try implementing automated way to switch container images according to major release version, IMO we should not change this interface and adds a separate parameter for override(or somehow pass template and get it resolved in a lower layer).\n\nHaving inconsistent interface for all *Images parameter can make future maintenance quite difficult.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"57057979add1267f8040086800f74a11e4a9cd61","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"fb86e35f_e187cdd4","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"4e5c6761_6a2607cd","updated":"2022-08-18 17:41:36.000000000","message":"Yeah, I think this is safe as long as we understand tripleo generates the actual value, and so it should generate json for this service.\n\nBut where is this controlled? I\u0027m still trying to understand the big picture.\n\nBTW, if the user *is* trying to override the value, then if they supply a string Heat will throw an error. This, I guess, is a good side effect, as it helps alert the user they are doing something that they shouldn\u0027t.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"7116e0beb76275a8e7bd80f9a1c71b511338d4e7","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"328758e0_1e4acc5f","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"6f5cc7ee_4cd055cf","updated":"2022-08-23 03:58:38.000000000","message":"So, the easiest way around all of this is to just add a new parameter for these multi-rhel images. Leave the existing interface the same and deprecate it. That way we have both, if the single image param is defined, we will use that, else use the multi-rhel json param.\n\nThis is essentially what I had proposed we do in earlier patchsets, but I\u0027m handling the dictionary creation in tripleo-common now instead of as Heat values. Which will work better for all of the standalone roles work as well","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"30870aac7da1eed2d59a9ffd4bd58b4501826f69","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"4e5c6761_6a2607cd","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"715281b7_ae7ad05d","updated":"2022-08-14 10:25:08.000000000","message":"Since this parameter is generated by us on each update, I\u0027m not sure it should ever be an issue in this case. Is users were manually overriding it, chances are that they wouldn\u0027t be receiving the updated containers which wouldn\u0027t be ideal.\n\nIn earlier patchsets here, I was indeed using new parameters and leaving this existing functionality. Through the various iterations and suggestions on this review we had landed in a more simplified Heat template with the complexity moved into tripleo-common instead.\n\nBut if there is a good reason to keep this parameter and instead use a new one for the multi-reel stuff. I\u0027m happy to go back to that as well. I\u0027ll leave it to the group to vote.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"e9cd47817f118f59fbd1b100b13ba8fd7cdfd11f","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"1dd8406f_7f7a4fb1","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"8d41cd48_ad25119e","updated":"2022-08-23 08:11:00.000000000","message":"I do see where you\u0027re coming from though Rabi. Your argument is that the framework already supports this today, and none of these changes are really necessary. The user just needs a role_specific ContainerImagePrepare parameter, and role_specific Container*Image: parameters. Their custom HostnameMap to maintain hostnames and then it will work.\n\nMy proposal here implies that we will abstract the complexity and ambiguity and handle it for the user. \n\nWhereas composable roles would just be a documentation change outlining how the users can achieve it within the current framework.\n\nI was just hoping to automate more of it to avoid issues using this feature, and hopefully minimise escalations. \n\nWe can discuss tomorrow anyway.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":8833,"name":"Rabi Mishra","email":"ramishra@redhat.com","username":"rabi"},"change_message_id":"b7d3d7b404324b347a6a9a592e0b383fe3756f1d","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"6f5cc7ee_4cd055cf","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"fb86e35f_e187cdd4","updated":"2022-08-23 03:23:55.000000000","message":"Not really. I don\u0027t think we should change the parameter type as it would create incompatibility. This is a role_specific parameter and there can be instances users overriding them for a role in their deployment and it won\u0027t throw any error with parameter validation. We should make sure the existing interfaces won\u0027t break.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"8b0c89ff8d958d9992e9a7efafa93192a8ebd6bb","unresolved":true,"context_lines":[{"line_number":25,"context_line":"  ContainerOvnControllerImage:"},{"line_number":26,"context_line":"    default: \u0027\u0027"},{"line_number":27,"context_line":"    description: image"},{"line_number":28,"context_line":"    type: json"},{"line_number":29,"context_line":"    tags:"},{"line_number":30,"context_line":"      - role_specific"},{"line_number":31,"context_line":"  ContainerOvnControllerConfigImage:"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"8d41cd48_ad25119e","line":28,"range":{"start_line":28,"start_character":4,"end_line":28,"end_character":14},"in_reply_to":"fbbf76c3_ea45c0ee","updated":"2022-08-23 06:14:06.000000000","message":"We can do Composable roles. But here\u0027s the challenges I foresee;\n\n1. It means we need to implement a mechanism to take the nodes excluded from the LEAP upgrade, create a new composable role for them and assign the nodes to the role. So something like, external update run --limit !overcloud-compute-27,overcloud-compute-31,overcloud-compute-42\n\nSo 27, 31 and 42 would need to be noted and added to the new composable role.\n\n2. We would need to update the existing HostnameMap to ensure the hostnames of the above mentioned systems aren\u0027t changed. Keeping in mind that we are already using HostnameMap as part of the baremetal node provision process now. So we could have to find an arbitrarily named file in a potentially arbitrarily named directory and update the existing HostnameMap with our nodes that are to remain on RHEL8.\n\n3. Then we still need to generate the images anyway. We need to make changes to tripleo-common to pull the required images, and we would also need to template them out to a new composable role dictionary now. So \n\nComputeStayingInThePast:\n   ContainerOvnControllerImage: blah\n\n\nThe proposed method here avoids all of those steps except for the last one. Whereby we just pull the images for both OS versions, add them to a single dictionary for each container and use Ansible logic to deterministically pull the correct container.\n\nThe alternative is that we don\u0027t do steps 1 and 2 and instead put that back on the user. But it feels like that is likely to lead to more mistakes.\n\nAdditionally, the concept of Composable Roles might not need to exist in the standalone-roles world. So handling this in Ansible as much as possible provides a better integration path into the standalone-roles initiative. As they can simply defined a dictionary variable with 2 images and allow jinja2 to handle the rest during the execution.\n\n@Lukas has some thoughts on this from the upgrades perspective as well. But that\u0027s my thoughts on it after considering both approaches.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"57057979add1267f8040086800f74a11e4a9cd61","unresolved":true,"context_lines":[{"line_number":263,"context_line":"              jump:   \u0027NOTRACK\u0027"},{"line_number":264,"context_line":"              action: \u0027append\u0027"},{"line_number":265,"context_line":"              state: []"},{"line_number":266,"context_line":"      ansible_group_vars:"},{"line_number":267,"context_line":"        tripleo_ovn_controller_image: {get_param: ContainerOvnControllerImage}"},{"line_number":268,"context_line":"        tripleo_ovn_bridge: {get_param: OVNIntegrationBridge}"},{"line_number":269,"context_line":"        tripleo_ovn_bridge_mappings: {get_param: NeutronBridgeMappings}"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"b6d3b854_859fe251","line":266,"updated":"2022-08-18 17:41:36.000000000","message":"Why is this necessary? Maybe there\u0027s a tripleo-ansible patch I need to know about?","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"5c20424de693333f823c45d2336ac43865dda311","unresolved":true,"context_lines":[{"line_number":263,"context_line":"              jump:   \u0027NOTRACK\u0027"},{"line_number":264,"context_line":"              action: \u0027append\u0027"},{"line_number":265,"context_line":"              state: []"},{"line_number":266,"context_line":"      ansible_group_vars:"},{"line_number":267,"context_line":"        tripleo_ovn_controller_image: {get_param: ContainerOvnControllerImage}"},{"line_number":268,"context_line":"        tripleo_ovn_bridge: {get_param: OVNIntegrationBridge}"},{"line_number":269,"context_line":"        tripleo_ovn_bridge_mappings: {get_param: NeutronBridgeMappings}"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"d94bf952_b1bf4c41","line":266,"in_reply_to":"b6d3b854_859fe251","updated":"2022-08-18 18:12:47.000000000","message":"+1. @Brendan... would it be possible to make a quick flowchart how are these variables passed around to different components?","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":34271,"name":"Miro Tomaska","display_name":"Miro Tomaska","email":"mtomaska@redhat.com","username":"mtomaska"},"change_message_id":"15f341a57987a9329558534dfa95b4a1e7634bc0","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            privileged: true"},{"line_number":358,"context_line":"            user: root"},{"line_number":359,"context_line":"            command: [\u0027/bin/bash\u0027, \u0027-c\u0027, \u0027CMS_OPTS\u003d$(hiera ovn::controller::ovn_cms_options -c /etc/puppet/hiera.yaml);  if [ X\"$CMS_OPTS\" !\u003d  X ]; then ovs-vsctl set open . external_ids:ovn-cms-options\u003d$CMS_OPTS;else ovs-vsctl remove open . external_ids ovn-cms-options; fi\u0027]"},{"line_number":360,"context_line":"            image: \"{{ tripleo_ovn_controller_image[ansible_facts[\u0027distribution_major_version\u0027]] | default(tripleo_ovn_controller_image[\u00279\u0027]) }}\" "},{"line_number":361,"context_line":"            volumes:"},{"line_number":362,"context_line":"              list_concat:"},{"line_number":363,"context_line":"                 - {get_attr: [ContainersCommon, volumes]}"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"7b24469d_d45e9070","line":360,"range":{"start_line":360,"start_character":136,"end_line":360,"end_character":139},"updated":"2022-08-13 02:28:59.000000000","message":"Can we generalize this for the future? Like \"OS_Primary\" \"OS_Secondary\" or something similar such that we will not need to touch the code every time we want to support next version(and drop an old version)","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"27ffe39c9837fd932e87fd8f03c70a85a7abe4be","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            privileged: true"},{"line_number":358,"context_line":"            user: root"},{"line_number":359,"context_line":"            command: [\u0027/bin/bash\u0027, \u0027-c\u0027, \u0027CMS_OPTS\u003d$(hiera ovn::controller::ovn_cms_options -c /etc/puppet/hiera.yaml);  if [ X\"$CMS_OPTS\" !\u003d  X ]; then ovs-vsctl set open . external_ids:ovn-cms-options\u003d$CMS_OPTS;else ovs-vsctl remove open . external_ids ovn-cms-options; fi\u0027]"},{"line_number":360,"context_line":"            image: \"{{ tripleo_ovn_controller_image[ansible_facts[\u0027distribution_major_version\u0027]] | default(tripleo_ovn_controller_image[\u00279\u0027]) }}\" "},{"line_number":361,"context_line":"            volumes:"},{"line_number":362,"context_line":"              list_concat:"},{"line_number":363,"context_line":"                 - {get_attr: [ContainersCommon, volumes]}"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"12eb90e3_a61087d7","line":360,"updated":"2022-08-22 02:03:19.000000000","message":"Some of them are remnants of my earlier patchsets, where I was leveraging the standalone tripleo_ovn role. But we will use the tripleo_ovn_controller_image variable here:\n\nhttps://review.opendev.org/c/openstack/tripleo-heat-templates/+/848208/23/deployment/ovn/ovn-controller-container-puppet.yaml#360\n\nI\u0027ll remove the variables that I\u0027m no longer using to avoid confusion here.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":21129,"name":"Alan Bishop","email":"abishopsweng@gmail.com","username":"ASBishop","status":"ex Red Hat"},"change_message_id":"57057979add1267f8040086800f74a11e4a9cd61","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            privileged: true"},{"line_number":358,"context_line":"            user: root"},{"line_number":359,"context_line":"            command: [\u0027/bin/bash\u0027, \u0027-c\u0027, \u0027CMS_OPTS\u003d$(hiera ovn::controller::ovn_cms_options -c /etc/puppet/hiera.yaml);  if [ X\"$CMS_OPTS\" !\u003d  X ]; then ovs-vsctl set open . external_ids:ovn-cms-options\u003d$CMS_OPTS;else ovs-vsctl remove open . external_ids ovn-cms-options; fi\u0027]"},{"line_number":360,"context_line":"            image: \"{{ tripleo_ovn_controller_image[ansible_facts[\u0027distribution_major_version\u0027]] | default(tripleo_ovn_controller_image[\u00279\u0027]) }}\" "},{"line_number":361,"context_line":"            volumes:"},{"line_number":362,"context_line":"              list_concat:"},{"line_number":363,"context_line":"                 - {get_attr: [ContainersCommon, volumes]}"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"47d4aac1_02650a96","line":360,"range":{"start_line":360,"start_character":136,"end_line":360,"end_character":139},"in_reply_to":"7321656b_4d45511d","updated":"2022-08-18 17:41:36.000000000","message":"Is there a reason why this needs a \"| default\" filter? Shouldn\u0027t the value that\u0027s based on ansible_facts[\u0027distribution_major_version\u0027] always work?\n\nAs I note in another review, I really hope we find a way to minimize the places where these hard-coded values appear. And they\u0027ll proliferate as we identify more services that require multi-os support (nova, iscsid, multipath).\n\nTo that end, let\u0027s at least continue to use the \u0026ovn_controller_image anchor. That will eliminate duplicating the same on L372.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"},{"author":{"_account_id":30073,"name":"Brendan Shephard","email":"bshephar@bne-home.net","username":"bshephar"},"change_message_id":"30870aac7da1eed2d59a9ffd4bd58b4501826f69","unresolved":true,"context_lines":[{"line_number":357,"context_line":"            privileged: true"},{"line_number":358,"context_line":"            user: root"},{"line_number":359,"context_line":"            command: [\u0027/bin/bash\u0027, \u0027-c\u0027, \u0027CMS_OPTS\u003d$(hiera ovn::controller::ovn_cms_options -c /etc/puppet/hiera.yaml);  if [ X\"$CMS_OPTS\" !\u003d  X ]; then ovs-vsctl set open . external_ids:ovn-cms-options\u003d$CMS_OPTS;else ovs-vsctl remove open . external_ids ovn-cms-options; fi\u0027]"},{"line_number":360,"context_line":"            image: \"{{ tripleo_ovn_controller_image[ansible_facts[\u0027distribution_major_version\u0027]] | default(tripleo_ovn_controller_image[\u00279\u0027]) }}\" "},{"line_number":361,"context_line":"            volumes:"},{"line_number":362,"context_line":"              list_concat:"},{"line_number":363,"context_line":"                 - {get_attr: [ContainersCommon, volumes]}"}],"source_content_type":"text/x-yaml","patch_set":23,"id":"7321656b_4d45511d","line":360,"range":{"start_line":360,"start_character":136,"end_line":360,"end_character":139},"in_reply_to":"7b24469d_d45e9070","updated":"2022-08-14 10:25:08.000000000","message":"Yeah, I agree it would be best to not need to manually update this. I\u0027ll have a bit more of a think about it this week.","commit_id":"3f70b4c3db05a3574eb7f1e28de12553d691b8f3"}],"releasenotes/notes/deprecate-singular-ovn-controller-image-96ac70ccd7f795c2.yaml":[{"author":{"_account_id":8449,"name":"Marios Andreou","email":"marios.andreou@gmail.com","username":"marios"},"change_message_id":"c399bcc2a04fdbba13e2ad2fd9284ad032cf0ec0","unresolved":true,"context_lines":[{"line_number":17,"context_line":"    you define the namespaces dictionary in your ContainerImagePrepare parameters."},{"line_number":18,"context_line":"    Or, if overriding manually, ensure both of the above mentioned parameters"},{"line_number":19,"context_line":"    are defined for both EL8 and EL9. EL9 containers will be used for the Controllers."},{"line_number":20,"context_line":"    While EL8 will be used on nodes that are still running on EL8. "}],"source_content_type":"text/x-yaml","patch_set":19,"id":"9930626e_c95e279c","line":20,"updated":"2022-07-19 05:34:47.000000000","message":"whitespace nits","commit_id":"8ab36de4a9de119f0c1501f74f74b9883ac4ae37"},{"author":{"_account_id":22954,"name":"Juan Badia Payno","email":"jbadiapa@redhat.com","username":"jbadiapa"},"change_message_id":"260b3491b3b652a0fde14e2a10f02d66b62656f4","unresolved":true,"context_lines":[{"line_number":6,"context_line":"    use."},{"line_number":7,"context_line":"features:"},{"line_number":8,"context_line":"  - |"},{"line_number":9,"context_line":"    Instead of ContainerOvnControllerImage and ContainerOvnControllerConfigImage."},{"line_number":10,"context_line":"    These are now split into ContainerOvnControllerImage8 and "},{"line_number":11,"context_line":"    ContainerOvnControllerImage9 and ContainerOvnControllerConfigImage8"},{"line_number":12,"context_line":"    ContainerOvnControllerConfigImage9. These images are added to the"},{"line_number":13,"context_line":"    ContainerOvnController*Image dictionaries."},{"line_number":14,"context_line":"upgrade:"},{"line_number":15,"context_line":"  - |"},{"line_number":16,"context_line":"    If you are upgrading and plan to keep some Compute nodes on EL8. Ensure"}],"source_content_type":"text/x-yaml","patch_set":21,"id":"c4a155af_c8f253ae","line":13,"range":{"start_line":9,"start_character":4,"end_line":13,"end_character":46},"updated":"2022-07-20 07:11:07.000000000","message":"I can not see the split. The ContainerOvnControllerImage and ContainerOvnControllerConfigImage are kept and I can not see the ContainerOvnControllerImage9 neither ContainerOvnControllerConfigImage9 parameters.","commit_id":"90f9615265f120e9e6a06079db7940381a44ff25"}]}