Skip to content

[multiple] Log patched container images for OpenStackVersion CR - #4152

Open
danpawlik wants to merge 1 commit into
openstack-k8s-operators:mainfrom
danpawlik:add-s2i-message
Open

[multiple] Log patched container images for OpenStackVersion CR#4152
danpawlik wants to merge 1 commit into
openstack-k8s-operators:mainfrom
danpawlik:add-s2i-message

Conversation

@danpawlik

Copy link
Copy Markdown
Contributor

When a job patches the OpenStackVersion controlplane CR with custom container images (e.g. s2i-built images from an operator PR), the resulting spec.customContainerImages is only visible by inspecting the must-gather openstackversions.core.openstack.org/controlplane.yaml. Jobs like s2i-speculative-deploy-test-* gave no in-log indication of which images were applied.

Add a debug task in both OpenStackVersion writers -- the update_containers role (template + oc apply) and the set_containers call in edpm_prepare -- that reads back the generated CR and prints the customContainerImages map, so the patched images are visible directly in the job output.

Assisted-By: Claude Opus 4.8 noreply@anthropic.com

@openshift-ci

openshift-ci Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci

openshift-ci Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign dasm for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@centosinfra-prod-github-app

Copy link
Copy Markdown

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/rdoproject.org/buildset/92b60f42348e45a0ba06614c851fa5f1

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 25m 59s
podified-multinode-edpm-deployment-crc POST_FAILURE in 25m 50s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 52m 09s
✔️ cifmw-crc-podified-edpm-baremetal-minor-update SUCCESS in 2h 13m 38s
✔️ cifmw-pod-zuul-files SUCCESS in 6m 52s
✔️ openstack-k8s-operators-content-provider-bootc SUCCESS in 2h 42m 10s
✔️ cifmw-crc-podified-edpm-baremetal-bootc SUCCESS in 1h 59m 27s
adoption-standalone-to-crc-ceph-provider POST_FAILURE in 3h 06m 59s
✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 11m 18s
✔️ cifmw-pod-pre-commit SUCCESS in 10m 44s
✔️ cifmw-molecule-edpm_prepare SUCCESS in 6m 59s
✔️ cifmw-molecule-update_containers SUCCESS in 4m 49s

@danpawlik
danpawlik marked this pull request as ready for review September 2, 2026 05:49
@danpawlik

Copy link
Copy Markdown
Contributor Author

Test done: openstack-k8s-operators/telemetry-operator#992

2026-09-01 18:38:16.766135 | controller | TASK [edpm_prepare : Display container images being patched msg=OpenStackVersion '{{ _ctlplane_name }}'
2026-09-01 18:38:16.766140 | controller | customContainerImages ({{ _cifmw_set_containers_images | length }}):
2026-09-01 18:38:16.766144 | controller | {% for name, uri in _cifmw_set_containers_images.items() %}
2026-09-01 18:38:16.766148 | controller | - {{ name }}: {{ uri }}
2026-09-01 18:38:16.766151 | controller | {% endfor %}
2026-09-01 18:38:16.766155 | controller | ] ***
2026-09-01 18:38:16.766159 | controller | Tuesday 01 September 2026  18:38:16 +0000 (0:00:00.183)       0:10:00.593 *****
2026-09-01 18:38:16.766163 | controller | Tuesday 01 September 2026  18:38:16 +0000 (0:00:00.183)       0:10:00.592 *****
2026-09-01 18:38:16.814170 | controller | ok: [localhost] =>
2026-09-01 18:38:16.814190 | controller |     msg: |
2026-09-01 18:38:16.814195 | controller |       OpenStackVersion 'controlplane'
2026-09-01 18:38:16.814200 | controller |       customContainerImages (8):
2026-09-01 18:38:16.814204 | controller |       - aodhAPIImage: 38.102.83.198:5001/openstack/openstack-aodh-api:992-22723563aa3d4d40d36e0167b6ba01fbc8ac3b02
2026-09-01 18:38:16.814208 | controller |       - aodhEvaluatorImage: 38.102.83.198:5001/openstack/openstack-aodh-api:992-22723563aa3d4d40d36e0167b6ba01fbc8ac3b02
2026-09-01 18:38:16.814212 | controller |       - aodhListenerImage: 38.102.83.198:5001/openstack/openstack-aodh-api:992-22723563aa3d4d40d36e0167b6ba01fbc8ac3b02
2026-09-01 18:38:16.814216 | controller |       - aodhNotifierImage: 38.102.83.198:5001/openstack/openstack-aodh-api:992-22723563aa3d4d40d36e0167b6ba01fbc8ac3b02
2026-09-01 18:38:16.814219 | controller |       - ceilometerCentralImage: 38.102.83.198:5001/openstack/openstack-ceilometer-central:992-22723563aa3d4d40d36e0167b6ba01fbc8ac3b02
2026-09-01 18:38:16.814223 | controller |       - ceilometerComputeImage: 38.102.83.198:5001/openstack/openstack-ceilometer-compute:992-22723563aa3d4d40d36e0167b6ba01fbc8ac3b02
2026-09-01 18:38:16.814227 | controller |       - ceilometerIpmiImage: 38.102.83.198:5001/openstack/openstack-ceilometer-compute:992-22723563aa3d4d40d36e0167b6ba01fbc8ac3b02
2026-09-01 18:38:16.814230 | controller |       - ceilometerNotificationImage: 38.102.83.198:5001/openstack/openstack-ceilometer-central:992-22723563aa3d4d40d36e0167b6ba01fbc8ac3b02

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Log OpenStackVersion custom container image overrides

✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Print generated OpenStackVersion customContainerImages maps in both container override workflows.
• Expose speculative s2i image selections directly in Ansible job logs.
Diagram

graph TD
  A["EDPM prepare"] --> B["Set module"] --> E["Generated CR"] --> F["Image extraction"] --> G["Job output"]
  C["Update role"] --> D["Template renderer"] --> E
Loading
High-Level Assessment

The current approach is appropriate because it logs the exact locally generated manifest for both writer paths, including preserved or rendered values. Querying the live cluster would not work reliably when application is disabled, while logging input variables could omit values added during generation or merging.

Files changed (2) +57 / -14

Enhancement (2) +57 / -14
kustomize_and_deploy.ymlLog images generated by set_containers +37/-14

Log images generated by set_containers

• Groups custom container generation into a block, registers the set_containers result, and reads its generated manifest. It parses spec.customContainerImages and prints each image override with the total count.

roles/edpm_prepare/tasks/kustomize_and_deploy.yml

main.ymlLog images rendered by the update_containers role +20/-0

Log images rendered by the update_containers role

• Reads the rendered OpenStackVersion manifest before the optional oc apply step. It safely defaults a missing image map to empty and prints all generated custom container image entries.

roles/update_containers/tasks/main.yml

@danpawlik
danpawlik requested a review from a team September 2, 2026 05:50
@danpawlik

Copy link
Copy Markdown
Contributor Author

recheck

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 📜 Skill insights (0)

Grey Divider


Action required

1. edpm_prepare variables misnamed 📘 Rule violation ⚙ Maintainability
Description
The new variables _cifmw_set_containers_result, _cifmw_set_containers_cr, and
_cifmw_set_containers_images begin with an underscore and omit the edpm_prepare role prefix.
They therefore violate the required cifmw_<role_name>_<variable_name> naming pattern.
Code

roles/edpm_prepare/tasks/kustomize_and_deploy.yml[73]

+      register: _cifmw_set_containers_result
Evidence
PR Compliance ID 1 requires every role variable to match ^cifmw_[a-z_][a-z0-9_]*$ and include the
role name. The added variables start with _cifmw_ and use set_containers rather than the
containing edpm_prepare role as their namespace.

AGENTS.md: Ansible Role Variables Must Follow the Required Naming Pattern
roles/edpm_prepare/tasks/kustomize_and_deploy.yml[73-86]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly registered and task-scoped variables do not match the required role-variable naming pattern.

## Issue Context
Rename them to begin with `cifmw_edpm_prepare_`, and update every reference consistently. Suitable names include `cifmw_edpm_prepare_set_containers_result`, `cifmw_edpm_prepare_set_containers_cr`, and `cifmw_edpm_prepare_set_containers_images`.

## Fix Focus Areas
- roles/edpm_prepare/tasks/kustomize_and_deploy.yml[73-86]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. update_containers variables misnamed 📘 Rule violation ⚙ Maintainability
Description
The new variables _cifmw_update_containers_cr and _cifmw_update_containers_images begin with an
underscore, so they do not match the required naming regex. This violates the role-variable
namespace requirement despite otherwise including the role name.
Code

roles/update_containers/tasks/main.yml[32]

+  register: _cifmw_update_containers_cr
Evidence
PR Compliance ID 1 requires role variables to match ^cifmw_[a-z_][a-z0-9_]*$. Both added names
begin with _, which the required expression does not permit.

AGENTS.md: Ansible Role Variables Must Follow the Required Naming Pattern
roles/update_containers/tasks/main.yml[32-46]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly registered and task-scoped variables fail the configured naming pattern because they begin with an underscore.

## Issue Context
Rename them to `cifmw_update_containers_cr` and `cifmw_update_containers_images`, updating all references in the read-back and debug tasks.

## Fix Focus Areas
- roles/update_containers/tasks/main.yml[32-46]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Check mode reads stale CR 🐞 Bug ☼ Reliability
Description
set_containers does not write the generated manifest in check mode, but the new task
unconditionally slurps its destination. A check-mode run therefore fails when the file is absent or
logs stale container overrides when an older file exists.
Code

roles/edpm_prepare/tasks/kustomize_and_deploy.yml[R75-77]

+    - name: Read back generated OpenStackVersion CR
+      ansible.builtin.slurp:
+        src: "{{ _cifmw_set_containers_result.dest_path }}"
Evidence
The role unconditionally slurps the registered destination and derives its log from that file. The
module supports check mode but guards the actual write with if not module.check_mode, while still
returning the destination path, so the path is not guaranteed to contain the CR just calculated.

roles/edpm_prepare/tasks/kustomize_and_deploy.yml[58-93]
plugins/modules/set_containers.py[591-600]
plugins/modules/set_containers.py[654-694]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new OpenStackVersion read-back runs during Ansible check mode even though `set_containers` does not write its generated manifest in that mode. This causes a missing-file failure or logs stale content.

## Issue Context
`set_containers` explicitly supports check mode and only writes `dest_path` outside check mode. Ensure both the read-back and its dependent debug task are skipped in check mode, or return and log the generated mapping directly from the module result.

## Fix Focus Areas
- roles/edpm_prepare/tasks/kustomize_and_deploy.yml[75-93]
- plugins/modules/set_containers.py[686-704]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Template readback breaks check mode 🐞 Bug ☼ Reliability
Description
The template task does not create or update its destination in check mode, but the newly added
slurp always reads that path. Check mode consequently fails if the generated artifact did not
already exist, or reports images from a stale artifact instead of the prospective template output.
Code

roles/update_containers/tasks/main.yml[R29-31]

+- name: Read back generated OpenStackVersion CR
+  ansible.builtin.slurp:
+    src: "{{ cifmw_update_containers_dest_path }}"
Evidence
The added slurp immediately reads the template destination without a check-mode condition. The
role defines this path as an output artifact, and its molecule scenario supplies a new temporary
destination that the role itself is expected to create, demonstrating that the file need not exist
before execution.

roles/update_containers/tasks/main.yml[23-47]
roles/update_containers/defaults/main.yml[21-32]
roles/update_containers/molecule/default/converge.yml[18-35]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The role unconditionally reads the generated OpenStackVersion file during Ansible check mode, although the preceding template task does not write files in that mode. The read can fail or display stale image values.

## Issue Context
The destination is a generated artifact and is not guaranteed to pre-exist. Guard both the `slurp` and dependent debug task with `not ansible_check_mode`, or render and parse the prospective template without relying on the destination file.

## Fix Focus Areas
- roles/update_containers/tasks/main.yml[23-47]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This modifies Ansible runtime workflows in two OpenStackVersion update paths, including CR file parsing and task-block behavior, so a careful review is warranted despite the localized scope.

Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread roles/edpm_prepare/tasks/kustomize_and_deploy.yml
Comment thread roles/update_containers/tasks/main.yml Outdated
Comment thread roles/edpm_prepare/tasks/kustomize_and_deploy.yml
Comment thread roles/update_containers/tasks/main.yml
@danpawlik
danpawlik force-pushed the add-s2i-message branch 2 times, most recently from 28dc47d to 336fbba Compare September 2, 2026 05:55
When a job patches the `OpenStackVersion` `controlplane` CR with custom
container images (e.g. s2i-built images from an operator PR), the resulting
`spec.customContainerImages` is only visible by inspecting the must-gather
`openstackversions.core.openstack.org/controlplane.yaml`. Jobs like
`s2i-speculative-deploy-test-*` gave no in-log indication of which images
were applied.

Add a debug task in both `OpenStackVersion` writers -- the `update_containers`
role (template + `oc apply`) and the `set_containers` call in `edpm_prepare`
-- that reads back the generated CR and prints it verbatim, so the patched
images are visible directly in the job output.

The rendered file is printed as raw lines rather than re-parsed with
`from_yaml`, which threw `yaml.scanner.ScannerError` on values containing a
colon-space sequence.

Assisted-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Daniel Pawlik <dpawlik@redhat.com>
@danpawlik

Copy link
Copy Markdown
Contributor Author

recheck

@centosinfra-prod-github-app

Copy link
Copy Markdown

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/rdoproject.org/buildset/f8f16022963446f99fe0c0cd5084719b

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 30m 15s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 37m 37s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 47m 56s
cifmw-crc-podified-edpm-baremetal-minor-update FAILURE in 50m 01s
✔️ cifmw-pod-zuul-files SUCCESS in 5m 55s
✔️ openstack-k8s-operators-content-provider-bootc SUCCESS in 2h 39m 41s
✔️ cifmw-crc-podified-edpm-baremetal-bootc SUCCESS in 2h 05m 41s
✔️ adoption-standalone-to-crc-ceph-provider SUCCESS in 3h 16m 46s
✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 9m 14s
✔️ cifmw-pod-pre-commit SUCCESS in 9m 51s
✔️ cifmw-molecule-edpm_prepare SUCCESS in 5m 59s
✔️ cifmw-molecule-update_containers SUCCESS in 4m 56s

@danpawlik

Copy link
Copy Markdown
Contributor Author

recheck

@centosinfra-prod-github-app

Copy link
Copy Markdown

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/zuul/t/rdoproject.org/buildset/413d375978b043978056478b91e0005b

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 34m 19s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 36m 41s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 2h 09m 51s
cifmw-crc-podified-edpm-baremetal-minor-update FAILURE in 56m 10s
✔️ cifmw-pod-zuul-files SUCCESS in 6m 12s
✔️ openstack-k8s-operators-content-provider-bootc SUCCESS in 2h 44m 25s
✔️ cifmw-crc-podified-edpm-baremetal-bootc SUCCESS in 1h 52m 30s
adoption-standalone-to-crc-ceph-provider POST_FAILURE in 3h 15m 04s
✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 10m 48s
✔️ cifmw-pod-pre-commit SUCCESS in 9m 59s
✔️ cifmw-molecule-edpm_prepare SUCCESS in 6m 36s
✔️ cifmw-molecule-update_containers SUCCESS in 5m 25s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant