[multiple] Add bootc CP job and wire EDPM image outputs#3952
Conversation
|
[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 |
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider FAILURE in 21m 21s |
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider FAILURE in 27m 19s |
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider TIMED_OUT in 31m 24s |
|
recheck |
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider TIMED_OUT in 31m 24s |
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider TIMED_OUT in 31m 24s |
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider TIMED_OUT in 31m 24s |
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider TIMED_OUT in 31m 26s |
e2eb0c4 to
e786602
Compare
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider FAILURE in 21m 07s |
8fc6ca8 to
7a0b6f4
Compare
4287f7c to
9213830
Compare
|
Zuul encountered a syntax error while parsing its The project template "podified-multinode-edpm-baremetal-bootc- The problem appears in the "openstack-k8s-operators/edpm-ansible" project stanza: project: in "openstack-k8s-operators/edpm-ansible/zuul.d/projects.yaml@main", line 2 |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 39m 32s |
bc0dcda to
0180318
Compare
f2e9390 to
90494c0
Compare
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider TIMED_OUT in 31m 23s |
f39edc3 to
39bb2ae
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 4h 20m 06s |
|
Build failed (check pipeline). Post ❌ openstack-k8s-operators-content-provider FAILURE in 20m 21s |
Add a bootc-specific content provider job and wire the bootc baremetal pipeline to it. Also change cifmw-content-provider-build-imagesto build and push both bootc and non-bootc images. Change-Id: I2ef6b0d00640a663596e29034c885a70adcd051c Signed-off-by: rabi <ramishra@redhat.com>
evallesp
left a comment
There was a problem hiding this comment.
I've done a quick review. Please answer the threads and I'll continue.
Question: is it possible to split the code here in more than one commit and/or MRs? I see this is grouping a single functionality, but makes the review process much more slower.
| > {{ cifmw_edpm_build_images_basedir }}/logs/edpm_images/edpm_bootc_qcow2_build.log | ||
| 2> {{ cifmw_edpm_build_images_basedir }}/logs/edpm_images/edpm_bootc_qcow2_build_err.log | ||
|
|
||
| - name: Rename generated bootc qcow2 image |
There was a problem hiding this comment.
(blocking) suggestion: Let's move to use copy module.
There was a problem hiding this comment.
Not sure why? This is a large image and we should not duplicate it.
There was a problem hiding this comment.
Definetely sorry because my comment was too short.
I mean about the pipelines to the normal exit and the error exit.
There was a problem hiding this comment.
Oh you mean use copy for the logs, not the qcow2. register + copy would work but adds 4 tasks and buffers all build output in memory. Keeping > / 2>: we already get separate stdout/stderr log files streamed to disk, Ansible still fails on non-zero exit, and the error log is written even when the build fails. I did not see the pattern of register+copy elsewhere in the repo for logs.
| --volume {{ _cifmw_edpm_build_images_bootc_smoke_dir.path }}:/target:Z | ||
| localhost/edpm-bootc:{{ cifmw_edpm_build_images_tag }}-qcow2 | ||
|
|
||
| - name: List files extracted from bootc qcow2 image |
There was a problem hiding this comment.
(blocking) question: Is this a leftover?
There was a problem hiding this comment.
I'll remove it in the next update. added for debugging.
| - (cifmw_edpm_build_images_hardened_uefi | bool) or (cifmw_edpm_build_images_all | bool) | ||
| - cifmw_edpm_build_images_hardened_uefi_package | bool | ||
| ansible.builtin.command: | ||
| cmd: >- |
There was a problem hiding this comment.
(blocking) suggestion: I think these need to be move to use podman module.
There was a problem hiding this comment.
I was using podman_image for both uefi and bootc, noticed false positives at times with podman_image, hence switched to podman command.
There was a problem hiding this comment.
If so I'd like to see a docstring about the reason, so it's clear for the future if we want to remove them.
Did you find a way to reproduce that?
| environment: | ||
| KUBECONFIG: "{{ cifmw_openshift_kubeconfig }}" | ||
| PATH: "{{ cifmw_path }}" | ||
| ansible.builtin.command: |
There was a problem hiding this comment.
(blocking) suggestion: Let's move to use k8s_info
There was a problem hiding this comment.
I've kept it consistent with what we had before like in L229. Only reads can use k8s_info though, we can probably change all oc get to use k8s_info in a followup.
There was a problem hiding this comment.
I'd rather go to update them now. And in following let's fix the rest.
Hey @rabi, thanks for the PR! Before moving on to the analysis and code review, I definitely want to highlight @evallesp's comments about splitting this implementation into multiple PRs.
Of course, this is just a suggestion, not necessarily what I expect you to do. |
Single PR is intentional: bootc build/push, content-provider wiring, and Zuul/deploy changes belong together and are only fully tested by the new bootc pipeline jobs. Splitting would not add review or CI value, because intermediate PRs would land with incomplete, untested functionality. |
Add a bootc-specific content provider job alongside the existing UEFI content provider flow, and wire baremetal jobs to consume the returned EDPM OS container image outputs. Also gate EDPM image publishing on edpm-image-builder changes, with ci-framework overrides to validate the wiring here.