[OSPRH-25851] Ignore empty node sets during data plane OVN minor update#1786
Conversation
| "ansibleSSHPrivateKeySecret": "dataplane-ansible-ssh-private-key-secret", | ||
| }, | ||
| "nodes": map[string]interface{}{}, | ||
| "servicesOverride": []string{}, |
There was a problem hiding this comment.
I removed this because the servicesOverride field doesn't actually exist in OpenStackDataPlaneNodeSet. It's found in OpenStackDataPlaneDeployment instead, so this line is misleading.
|
looks good to me, but would let someone with better edpm knowledge approve to land it. |
rabi
left a comment
rabi
left a comment
There was a problem hiding this comment.
Sorry I missed it. LGTM. Did you face the issue with some testing. In real-life we don't expect users to have empty nodesets 😄
internal/openstack/dataplane.go
Outdated
| // After considering generation (to make sure reconciliation has quiesced for | ||
| // the current nodeset spec), we only care about deployed status if the nodeset | ||
| // has nodes | ||
| if len(nodeset.Spec.Nodes) < 1 { |
There was a problem hiding this comment.
I think general pattern is to use == 0 ?
A customer had run into this, which is what prompted the work. But yeah, the workaround is just to delete those empty |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test openstack-operator-build-deploy-kuttl |
/test openstack-operator-build-deploy-kuttl |
/test openstack-operator-build-deploy-kuttl |
Unrelated to PR. /test openstack-operator-build-deploy-kuttl |
|
/test openstack-operator-build-deploy-kuttl |
openshift-merge-bot
bot
merged commit dd4c370
into
openstack-k8s-operators:main
3 participants
During a minor update, we currently mistakenly consider empty
OpenStackDataPlaneNodeSets and wait for their non-existent OVN deployments to update. We should ignore them in this context instead, in order to prevent unnecessarily blocking the update and thus obviate the need for manual intervention to unblock it (which can be done by deleting the emptyOpenStackDataPlaneNodeSets).Tested locally in a CIFMW dev-scripts environment with a minor update of VA1:
OpenStackDataPlaneNodeSetafter initial deploymentJira: https://issues.redhat.com/browse/OSPRH-25851