-
Notifications
You must be signed in to change notification settings - Fork 80
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
Don't set BaremetalHosts in the nodeset spec #1258
Don't set BaremetalHosts in the nodeset spec #1258
Conversation
@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. |
/hold |
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. |
Instead use BaremetalSetTemplateSpec for baremetalSetTemplate. BaremetalHosts field would be populated from nodes. Jira: https://issues.redhat.com/browse/OSPRH-12709 Signed-off-by: rabi <[email protected]>
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/63fdc7ab10ea4ecd81ad4901fc8c9024 ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 12m 32s |
recheck |
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 |
I tested this by updating the operators (uninstalling and installing) and there were no issues. I also realized that we test the same in the kuttl job[1] as well. |
/unhold |
Good point and good to know. 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abays, 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 |
/test openstack-operator-build-deploy-kuttl |
f03ace0
into
openstack-k8s-operators:main
Jira: https://issues.redhat.com/browse/OSPRH-12709