Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[va-hci] Remove references to edpm images #204

Merged

Conversation

eduolivares
Copy link
Contributor

Default image versions obtained from the dataplane-operator will be used instead

Copy link
Contributor

@abays abays left a comment

Choose a reason for hiding this comment

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

Tested with VA1. Deployment succeeded and Tempest passed.

/lgtm

@karelyatin
Copy link
Contributor

/lgtm

The defaults for edpm images is managed in dataplane-operator, overriding just the edpm images and not the control plane(specifically OVN images atleast) can cause issues as mentioned in https://issues.redhat.com/browse/OSPRH-1504 . The new way for managing the both control plane/data plane is with OpenStackVersion CR.

@eduolivares please drop WIP from commit

@eduolivares eduolivares changed the title WIP [va-hci] Remove references to edpm images [va-hci] Remove references to edpm images Apr 29, 2024
@eduolivares
Copy link
Contributor Author

/lgtm

The defaults for edpm images is managed in dataplane-operator, overriding just the edpm images and not the control plane(specifically OVN images atleast) can cause issues as mentioned in https://issues.redhat.com/browse/OSPRH-1504 . The new way for managing the both control plane/data plane is with OpenStackVersion CR.

@eduolivares please drop WIP from commit

WIP removed from both commit and PR messages.

@karelyatin
Copy link
Contributor

/lgtm

@abays
Copy link
Contributor

abays commented Apr 29, 2024

Please wait to merge this until after #197 is merged, and then kindly rebase. We're splitting the EDPM deployment and the file modified in this PR will no longer exist under the current path.

Default image versions obtained from the dataplane-operator will be used
instead
Copy link
Contributor

@abays abays left a comment

Choose a reason for hiding this comment

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

/lgtm

@leifmadsen leifmadsen added bugfix Fixes an existing bug needs-info Information is requested of the reporter or reviewers triaged Issue has been initially reviewed and triaged labels Apr 29, 2024
@leifmadsen
Copy link
Contributor

Just throwing needs-info on this to make sure there is no issue across other VAs or DTs as they should all remain in sync.

@eduolivares
Copy link
Contributor Author

Just throwing needs-info on this to make sure there is no issue across other VAs or DTs as they should all remain in sync.

It seems no other VAs/DTs specify the EDPM images used:

architecture (main) $ git grep "edpm_.*_image"
examples/va/hci/edpm-pre-ceph/nodeset/values.yaml:        edpm_iscsid_image: '{{ registry_url }}/openstack-iscsid:{{ image_tag }}'
examples/va/hci/edpm-pre-ceph/nodeset/values.yaml:        edpm_logrotate_crond_image: '{{ registry_url }}/openstack-cron:{{ image_tag }}'
examples/va/hci/edpm-pre-ceph/nodeset/values.yaml:        edpm_neutron_metadata_agent_image: '{{ registry_url }}/openstack-neutron-metadata-agent-ovn:{{ image_tag }}'
examples/va/hci/edpm-pre-ceph/nodeset/values.yaml:        edpm_nova_compute_container_image: '{{ registry_url }}/openstack-nova-compute:{{ image_tag }}'
examples/va/hci/edpm-pre-ceph/nodeset/values.yaml:        edpm_nova_libvirt_container_image: '{{ registry_url }}/openstack-nova-libvirt:{{ image_tag }}'
examples/va/hci/edpm-pre-ceph/nodeset/values.yaml:        edpm_ovn_controller_agent_image: '{{ registry_url }}/openstack-ovn-controller:{{ image_tag }}'

I am working on a PR where EDPM images were specified too, but I removed them yesterday:
#193 (comment)

So, I think all VAs and DTs are synchronized.

@leifmadsen leifmadsen removed the needs-info Information is requested of the reporter or reviewers label Apr 30, 2024
@leifmadsen leifmadsen merged commit 7bf9d2b into openstack-k8s-operators:main Apr 30, 2024
2 checks passed
@karelyatin karelyatin mentioned this pull request May 1, 2024
@eduolivares eduolivares deleted the default-dp-images branch May 16, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes an existing bug triaged Issue has been initially reviewed and triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants