Skip to content

[OSPRH-25851] Ignore empty node sets during data plane OVN minor update #1786

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.

Sign up for GitHub

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

Open
abays wants to merge 1 commit into
base: main
Choose a base branch
from
+86 −57
Open

[OSPRH-25851] Ignore empty node sets during data plane OVN minor update #1786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions internal/openstack/dataplane.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ func DataplaneNodesetsOVNControllerImagesMatch(version *corev1beta1.OpenStackVer
if nodeset.Generation != nodeset.Status.ObservedGeneration {
return false
}
// we only check nodesets if they deploy OVN
if nodeset.Status.ContainerImages["OvnControllerImage"] != "" {
// After considering generation (to make sure reconciliation has quiesced for
// the current nodeset spec), we only check nodesets if they have any nodes
// and have deployed OVN
if len(nodeset.Spec.Nodes) > 0 && nodeset.Status.ContainerImages["OvnControllerImage"] != "" {
if !nodeset.IsReady() {
return false
}
Expand All @@ -60,6 +62,12 @@ func DataplaneNodesetsDeployed(version *corev1beta1.OpenStackVersion, dataplaneN
if nodeset.Generation != nodeset.Status.ObservedGeneration {
return false
}
// 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) == 0 {
continue
}
if !nodeset.IsReady() {
return false
}
Expand Down
116 changes: 61 additions & 55 deletions test/functional/ctlplane/base_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,65 +46,70 @@ import (
)

type Names struct {
Namespace string
OpenStackControlplaneName types.NamespacedName
OpenStackVersionName types.NamespacedName
OpenStackVersionName2 types.NamespacedName
KeystoneAPIName types.NamespacedName
MemcachedName types.NamespacedName
MemcachedCertName types.NamespacedName
CinderName types.NamespacedName
ManilaName types.NamespacedName
GlanceName types.NamespacedName
NeutronName types.NamespacedName
HorizonName types.NamespacedName
HeatName types.NamespacedName
NovaName types.NamespacedName
TelemetryName types.NamespacedName
WatcherName types.NamespacedName
DBName types.NamespacedName
DBCertName types.NamespacedName
DBCell1Name types.NamespacedName
DBCell1CertName types.NamespacedName
RabbitMQName types.NamespacedName
RabbitMQCertName types.NamespacedName
RabbitMQCell1Name types.NamespacedName
RabbitMQCell1CertName types.NamespacedName
RabbitMQNotificationsName types.NamespacedName
RabbitMQNotificationsCertName types.NamespacedName
NoVNCProxyCell1CertPublicRouteName types.NamespacedName
NoVNCProxyCell1CertPublicSvcName types.NamespacedName
NoVNCProxyCell1CertVencryptName types.NamespacedName
ServiceAccountName types.NamespacedName
RoleName types.NamespacedName
RoleBindingName types.NamespacedName
RootCAPublicName types.NamespacedName
RootCAInternalName types.NamespacedName
RootCAOvnName types.NamespacedName
RootCALibvirtName types.NamespacedName
SelfSignedIssuerName types.NamespacedName
CustomIssuerName types.NamespacedName
CustomServiceCertSecretName types.NamespacedName
CABundleName types.NamespacedName
OpenStackClientName types.NamespacedName
OVNNorthdName types.NamespacedName
OVNNorthdCertName types.NamespacedName
OVNControllerName types.NamespacedName
OVNControllerCertName types.NamespacedName
OVNDbServerNBName types.NamespacedName
OVNDbServerSBName types.NamespacedName
OVNMetricsCertName types.NamespacedName
NeutronOVNCertName types.NamespacedName
OpenStackTopology []types.NamespacedName
WatcherCertPublicRouteName types.NamespacedName
WatcherCertPublicSvcName types.NamespacedName
WatcherCertInternalName types.NamespacedName
Namespace string
OpenStackControlplaneName types.NamespacedName
OpenStackDataplaneNodeSetNoNodesName types.NamespacedName
OpenStackVersionName types.NamespacedName
OpenStackVersionName2 types.NamespacedName
KeystoneAPIName types.NamespacedName
MemcachedName types.NamespacedName
MemcachedCertName types.NamespacedName
CinderName types.NamespacedName
ManilaName types.NamespacedName
GlanceName types.NamespacedName
NeutronName types.NamespacedName
HorizonName types.NamespacedName
HeatName types.NamespacedName
NovaName types.NamespacedName
TelemetryName types.NamespacedName
WatcherName types.NamespacedName
DBName types.NamespacedName
DBCertName types.NamespacedName
DBCell1Name types.NamespacedName
DBCell1CertName types.NamespacedName
RabbitMQName types.NamespacedName
RabbitMQCertName types.NamespacedName
RabbitMQCell1Name types.NamespacedName
RabbitMQCell1CertName types.NamespacedName
RabbitMQNotificationsName types.NamespacedName
RabbitMQNotificationsCertName types.NamespacedName
NoVNCProxyCell1CertPublicRouteName types.NamespacedName
NoVNCProxyCell1CertPublicSvcName types.NamespacedName
NoVNCProxyCell1CertVencryptName types.NamespacedName
ServiceAccountName types.NamespacedName
RoleName types.NamespacedName
RoleBindingName types.NamespacedName
RootCAPublicName types.NamespacedName
RootCAInternalName types.NamespacedName
RootCAOvnName types.NamespacedName
RootCALibvirtName types.NamespacedName
SelfSignedIssuerName types.NamespacedName
CustomIssuerName types.NamespacedName
CustomServiceCertSecretName types.NamespacedName
CABundleName types.NamespacedName
OpenStackClientName types.NamespacedName
OVNNorthdName types.NamespacedName
OVNNorthdCertName types.NamespacedName
OVNControllerName types.NamespacedName
OVNControllerCertName types.NamespacedName
OVNDbServerNBName types.NamespacedName
OVNDbServerSBName types.NamespacedName
OVNMetricsCertName types.NamespacedName
NeutronOVNCertName types.NamespacedName
OpenStackTopology []types.NamespacedName
WatcherCertPublicRouteName types.NamespacedName
WatcherCertPublicSvcName types.NamespacedName
WatcherCertInternalName types.NamespacedName
}

func CreateNames(openstackControlplaneName types.NamespacedName) Names {
return Names{
Namespace: openstackControlplaneName.Namespace,
OpenStackControlplaneName: openstackControlplaneName,
OpenStackDataplaneNodeSetNoNodesName: types.NamespacedName{
Namespace: openstackControlplaneName.Namespace,
Name: "openstack-dataplane-nodeset-no-nodes",
},
OpenStackVersionName: types.NamespacedName{
Namespace: openstackControlplaneName.Namespace,
Name: openstackControlplaneName.Name, // same name as controlplane
Expand Down Expand Up @@ -392,6 +397,8 @@ func CreateOpenStackControlPlane(name types.NamespacedName, spec map[string]inte
}

// Build OpenStackDataPlaneNodeSetSpec struct with empty `Nodes` list
// NOTE: The function seems improperly named, given that the nodeset is
// in fact given a node in the spec.
func DefaultDataPlaneNoNodeSetSpec(tlsEnabled bool) map[string]interface{} {
spec := map[string]interface{}{
"preProvisioned": true,
Expand All @@ -401,8 +408,7 @@ func DefaultDataPlaneNoNodeSetSpec(tlsEnabled bool) map[string]interface{} {
},
"ansibleSSHPrivateKeySecret": "dataplane-ansible-ssh-private-key-secret",
},
"nodes": map[string]interface{}{},
"servicesOverride": []string{},
Copy link
Contributor Author

@abays abays Jan 30, 2026

Choose a reason for hiding this comment

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

I removed this because the servicesOverride field doesn't actually exist in OpenStackDataPlaneNodeSet. It's found in OpenStackDataPlaneDeployment instead, so this line is misleading.

"nodes": map[string]interface{}{},
}
if tlsEnabled {
spec["tlsEnabled"] = true
Expand Down
15 changes: 15 additions & 0 deletions test/functional/ctlplane/openstackversion_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,16 @@ var _ = Describe("OpenStackOperator controller", func() {
// 5) simulate 1 more dataplanenodeset update to finish the minor update workflow
It("updating targetVersion triggers a minor update workflow", Serial, func() {

// Also create an empty dataplanenodeset with no nodes. This allows the test to
// verify that the minor update workflow works even with dataplane nodesets
// present that also happen to have no nodes
dataplanenodesetSpec := DefaultDataPlaneNoNodeSetSpec(false)
dataplanenodesetSpec["nodes"] = map[string]interface{}{}
DeferCleanup(
th.DeleteInstance,
CreateDataplaneNodeSet(names.OpenStackDataplaneNodeSetNoNodesName, dataplanenodesetSpec),
)

// 1) switch to version 0.0.1, this triggers a minor update
osversion := GetOpenStackVersion(names.OpenStackVersionName)

Expand Down Expand Up @@ -516,6 +526,11 @@ var _ = Describe("OpenStackOperator controller", func() {
dataplanenodeset.Status.Conditions.MarkTrue(condition.ReadyCondition, dataplanev1.NodeSetReadyMessage)
Expect(th.K8sClient.Status().Update(th.Ctx, dataplanenodeset)).To(Succeed())

// Simulate the empty dataplanenodeset generation status as well
dataplanenodeset = GetDataplaneNodeset(names.OpenStackDataplaneNodeSetNoNodesName)
dataplanenodeset.Status.ObservedGeneration = dataplanenodeset.Generation
Expect(th.K8sClient.Status().Update(th.Ctx, dataplanenodeset)).To(Succeed())

// and now finally we verify that OpenStackVersion is in the correct state (data plane OVN got updated)
Eventually(func(g Gomega) {

Expand Down