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

WIP Don't set BaremetalHosts in the nodeset spec #1258

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rabi
Copy link
Contributor

@rabi rabi commented Jan 13, 2025

Copy link
Contributor

openshift-ci bot commented Jan 13, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rabi

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

The pull request process is described here

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

@rabi
Copy link
Contributor Author

rabi commented Jan 13, 2025

@abays Can you chime on what issues you see with this during update or we would need conversion webhook? I've disabled the check for field removal here for the crd-schema-checker to pass.

@rabi
Copy link
Contributor Author

rabi commented Jan 13, 2025

/hold

Copy link

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/openstack-operator for 1258,b5755f51554ffa1b23c20f43d58d71024a0ab12e

Instead use BaremetalSetTemplateSpec for baremetalSetTemplate.
BaremetalHosts field would be populated from nodes.

Jira: https://issues.redhat.com/browse/OSPRH-12709

Signed-off-by: rabi <ramishra@redhat.com>
Copy link

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/63fdc7ab10ea4ecd81ad4901fc8c9024

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 12m 32s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 21m 14s
cifmw-crc-podified-edpm-baremetal FAILURE in 1h 38m 51s
✔️ adoption-standalone-to-crc-ceph-provider SUCCESS in 3h 00m 55s
✔️ openstack-operator-tempest-multinode SUCCESS in 1h 40m 48s

@rabi
Copy link
Contributor Author

rabi commented Jan 13, 2025

recheck

@abays
Copy link
Contributor

abays commented Jan 13, 2025

@abays Can you chime on what issues you see with this during update or we would need conversion webhook? I've disabled the check for field removal here for the crd-schema-checker to pass.

I'm not entirely sure. But I think from an API perspective, if you're dropping fields in from the CRD, then existing CRs with the fields will just lose them the next time the CR is updated. That's the default behavior, at least, when you (as the user) add fields to a spec that don't actually exist in the underlying CRD. So my initial guess is that these fields will just disappear from the existing OpenStackDataPlaneNodeSet CRs and the API won't complain. But since the CRD schema checker wants to block removing fields without bumping the API version, it makes me suspect that I might be missing something. To be fair, I have never actually tried this.

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

Successfully merging this pull request may close these issues.

3 participants