Skip to content

[dataplane_adoption] Do not override images in the nodeset#1288

Open
karelyatin wants to merge 1 commit intoopenstack-k8s-operators:mainfrom
karelyatin:drop_image_overrides
Open

[dataplane_adoption] Do not override images in the nodeset#1288
karelyatin wants to merge 1 commit intoopenstack-k8s-operators:mainfrom
karelyatin:drop_image_overrides

Conversation

@karelyatin
Copy link
Contributor

@karelyatin karelyatin commented Mar 6, 2026 •
edited
Loading

OpenstackVersion CR have info for all the images, we should utilize that instead of overriding in the nodeset. Any image overrides in nodeset has highest priority so any handling/overrides with OpenstackVersion CR is not used.

Also added edpmNodeExporterImage which was missing in
OpenstackVersion template.

@softwarefactory-project-zuul
Copy link

softwarefactory-project-zuul bot commented Mar 6, 2026

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://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/589d0de0ac81470bb84a5ac1e0030271

✔️ noop SUCCESS in 0s
✔️ adoption-standalone-to-crc-ceph SUCCESS in 3h 00m 24s
❌ adoption-standalone-to-crc-no-ceph FAILURE in 2h 07m 17s

@karelyatin
Copy link
Contributor Author

karelyatin commented Mar 10, 2026

recheck barbican fixed

jistr
jistr approved these changes Mar 10, 2026
View reviewed changes
Copy link
Contributor

@jistr jistr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering whether this one should have a TP but on high level it seems correct to me to get rid of this logic.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Mar 10, 2026
@karelyatin
Copy link
Contributor Author

karelyatin commented Mar 10, 2026 •
edited
Loading

<< I'm wondering whether this one should have a TP but on high level it seems correct to me to get rid of this logic.
@jistr with TP you mean Test project? then this should be already validated in the jobs ran in this PR, also i validated it in downstream integration standalone and multinode jobs and also in unialpha adoption job and all went fine

ciecierski
ciecierski requested changes Mar 11, 2026
View reviewed changes
Copy link
Contributor

@ciecierski ciecierski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of edpm images removed in here are present as override in openstack_version.j2 template. Let's take edpm_frr_image as a first example, which is required for BGP, we not only ovveriding it for test run, but also pointing customer in docs to this override.

@karelyatin
Copy link
Contributor Author

karelyatin commented Mar 11, 2026

None of edpm images removed in here are present as override in openstack_version.j2 template. Let's take edpm_frr_image as a first example, which is required for BGP, we not only ovveriding it for test run, but also pointing customer in docs to this override.

@ciecierski but i see those are(including frr edpmFrrImage) in

data-plane-adoption/tests/roles/backend_services/templates/openstack_version.j2

Lines 22 to 30 in cd156bf

edpmFrrImage: {{ container_registry }}/{{ container_namespace }}/openstack-frr:{{ container_tag }}
edpmIscsidImage: {{ container_registry }}/{{ container_namespace }}/openstack-iscsid:{{ container_tag }}
edpmLogrotateCrondImage: {{ container_registry }}/{{ container_namespace }}/openstack-cron:{{ container_tag }}
edpmMultipathdImage: {{ container_registry }}/{{ container_namespace }}/openstack-multipathd:{{ container_tag }}
edpmNeutronDhcpAgentImage: {{ container_registry }}/{{ container_namespace }}/openstack-neutron-dhcp-agent:{{ container_tag }}
edpmNeutronMetadataAgentImage: {{ container_registry }}/{{ container_namespace }}/openstack-neutron-metadata-agent-ovn:{{ container_tag }}
edpmNeutronOvnAgentImage: {{ container_registry }}/{{ container_namespace }}/openstack-neutron-ovn-agent:{{ container_tag }}
edpmNeutronSriovAgentImage: {{ container_registry }}/{{ container_namespace }}/openstack-neutron-sriov-agent:{{ container_tag }}
edpmOvnBgpAgentImage: {{ container_registry }}/{{ container_namespace }}/openstack-ovn-bgp-agent:{{ container_tag }}
, and what's the reason to ask customers to explicitly define these images in nodeset ? if bundles and openstackversion already providing these for both connected and disconnected installations

@karelyatin
Copy link
Contributor Author

karelyatin commented Mar 11, 2026

But noticed altleast EdpmNodeExporterImage missing in openstack_version so will push an update for that atleast and check the same for other images apart from any other feedback

@karelyatin karelyatin marked this pull request as draft March 11, 2026 09:28
@karelyatin
[dataplane_adoption] Do not override images in the nodeset
e73f9f3 
OpenstackVersion CR have info for all the images, we
should utilize that instead of overriding in the nodeset.
Any image overrides in nodeset has highest priority so
any handling/overrides with OpenstackVersion CR is not
used.

Also added edpmNodeExporterImage which was missing in
OpenstackVersion template.

Signed-off-by: Yatin Karel <ykarel@redhat.com>
@karelyatin karelyatin force-pushed the drop_image_overrides branch from 13717f7 to e73f9f3 Compare March 11, 2026 11:54
@openshift-ci openshift-ci bot removed the lgtm label Mar 11, 2026
@openshift-ci
Copy link

openshift-ci bot commented Mar 11, 2026

New changes are detected. LGTM label has been removed.

@openshift-ci
Copy link

openshift-ci bot commented Mar 11, 2026

[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 ask for approval from jistr. 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

@karelyatin karelyatin marked this pull request as ready for review March 11, 2026 11:54
@softwarefactory-project-zuul
Copy link

softwarefactory-project-zuul bot commented Mar 11, 2026

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://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/4a631bd15779411184c2d0967dcc1bcf

✔️ noop SUCCESS in 0s
✔️ adoption-standalone-to-crc-ceph SUCCESS in 2h 56m 04s
❌ adoption-standalone-to-crc-no-ceph FAILURE in 1h 38m 38s

@karelyatin
Copy link
Contributor Author

karelyatin commented Mar 11, 2026

recheck openstackversion cleanup issue

@karelyatin
Copy link
Contributor Author

karelyatin commented Mar 11, 2026

recheck

1 similar comment
@karelyatin
Copy link
Contributor Author

karelyatin commented Mar 12, 2026

recheck

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

Reviewers

@ciecierski ciecierski ciecierski requested changes

@jistr jistr jistr approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

@jistr jistr

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@karelyatin @jistr @ciecierski