Add Heat container image definitions - #76
Conversation
yazug
left a comment
There was a problem hiding this comment.
couple of nits but looks good to me nothing blocking it from my point of view.
rabi
left a comment
There was a problem hiding this comment.
Few questions as well,
Consolidating to one image means the three RELATED_IMAGE_HEAT_* references all have to point at the same pullspec, and needs changes in heat-operator/openstack-operator, right?
Since all three services would then share one image digest, any rebuild of that image would roll all three (api, api-cfn, engine) together, even for a fix that only affects one role (e.g. an httpd CVE bouncing number of heat-engine pods). Has this been taken into consideration?
|
Thanks for consolidating into a single one, that's great. New container name is openstack-heat which didn't exist in tcib. @yazug is that a problem? I think you mentioned to reuse an existing name when possible. |
Yes, the three RELATED_IMAGE_HEAT... should point to the same container image so the three values in OpenStackVersion would point to the same image. I don't know the details of heat-operator but i wouldn't expect this to require any change on it (we are doing the same in watcher-operator). About changes in openstack-operator to get default values, I'd say just setting the default values (https://github.com/search?q=repo%3Aopenstack-k8s-operators%2Fopenstack-operator+RELATED_IMAGE_HEAT_&type=code) . Given that this is different than tcib, we will need some mapping for transition for the mapping of container names to OpenStackVersion fields for CI which is what we just starting adding in https://github.com/openstack-k8s-operators/s2i-openstack-containers/blob/main/containers/image-mappings.yaml I may be missing something but from the impact in operators itself, i think consolidation should be easy to manage. @stuggi , any opinion on this?
Yes. I think it has been considered but it's probably good to wide the discussion and make sure all impact is considered. @yazug may give the opinion of reldel on that and give additional details about how the actual process works. Some considerations to keep in mind.
From the s2i-openstack-operators itself, reducing the number of images will help managing the content from different aspects so it's a fair goal but you are right that there may be other considerations. Do you think it'd be good to have some wider discussion and / or document these considerations somewhere? |
|
New changes are detected. LGTM label has been removed. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Use a consolidated single image for heat-api, heat-api-cfn, and heat-engine. The only differences between separate containers is that the api containers the 3 rpms: httpd, mod_ssl, python3-mod_wsgi. The image builds Heat from source upstream config files (api-paste.ini, environment.d, templates), and the distgit heat-dist.conf. Note that python3-cryptography is pinned to 43.0.0-4.el10, which matches the major version in upper-constraints.txt. Jira: OSPRH-34691 Jira: OSPRH-34692 Jira: OSPRH-34693 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: James Slagle <jslagle@redhat.com>
The additional surface area is httpd, mod_ssl, python3-mod_wsgi, since those are now included in the image that's used by heat-engine. If those deps do need to get updated frequently, we're talking about a lot of container restarts anyway across all the APIs. Adding in heat-engine to that list doesn't seem like it would make much difference. heat-engine restarts also shouldn't cause any disruption to any user stacks that might be in progress. That seems worth it for managing 1 less container image. |
Confirmed by @yazug we need the name for the new common container to be an existing one in tcib, heat-api, heat-base, heat-api-cfn or heat-engine. |
I don't see any reason we continue including those in heat-engine.
Well, heat-engine waits for in-progess stacks to complete or timeout before killing the engines. I don't think it's correct to assume that restarting heat-engines unnecessarily won't be an overhead.
I don't think we should name the common image any of the above as it would be misleading/confusing. I would prefer we split them into 2 images heat-api (includes heat-api-cfn) and heat-engine. |
Are we planning to keep the three RELATED_IMAGE_HEAT_* env vars (one per service) now that all three run the same image? If so, we've to guarantee they stay pinned to the same digest and don't diverge over time. Should we collapse to a single reference instead? |
That's TBD at this point. I thin keeping RELATED_IMAGE_HEAT_* on sync is doable, but we may also have a single env variable and consume the same to populate defaults for multiple openstackversion fields. We just need to manage the transition so that it's tied to moving all the CI jobs to new s2i master ones and make sure it's properly managed in the e2e process. |
Use a consolidated single image for heat-api, heat-api-cfn, and
heat-engine. The only differences between separate containers is that
the api containers the 3 rpms: httpd, mod_ssl, python3-mod_wsgi.
The image builds Heat from source with upstream config files (api-paste.ini, environment.d, templates), and the distgit heat-dist.conf.
Note that python3-cryptography is pinned to 43.0.0-4.el10, which matches
the major version in upper-constraints.txt.
Jira: OSPRH-34691
Jira: OSPRH-34692
Jira: OSPRH-34693
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Signed-off-by: James Slagle jslagle@redhat.com