Skip to content

Commit dd4c370

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1786 from abays/osprh_25851
[OSPRH-25851] Ignore empty node sets during data plane OVN minor update
2 parents 65d6885 + 1e8a126 commit dd4c370

3 files changed

Lines changed: 86 additions & 57 deletions

File tree

‎internal/openstack/dataplane.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,10 @@ func DataplaneNodesetsOVNControllerImagesMatch(version *corev1beta1.OpenStackVer
4141
if nodeset.Generation != nodeset.Status.ObservedGeneration {
4242
return false
4343
}
44-
// we only check nodesets if they deploy OVN
45-
if nodeset.Status.ContainerImages["OvnControllerImage"] != "" {
44+
// After considering generation (to make sure reconciliation has quiesced for
45+
// the current nodeset spec), we only check nodesets if they have any nodes
46+
// and have deployed OVN
47+
if len(nodeset.Spec.Nodes) > 0 && nodeset.Status.ContainerImages["OvnControllerImage"] != "" {
4648
if !nodeset.IsReady() {
4749
return false
4850
}
@@ -60,6 +62,12 @@ func DataplaneNodesetsDeployed(version *corev1beta1.OpenStackVersion, dataplaneN
6062
if nodeset.Generation != nodeset.Status.ObservedGeneration {
6163
return false
6264
}
65+
// After considering generation (to make sure reconciliation has quiesced for
66+
// the current nodeset spec), we only care about deployed status if the nodeset
67+
// has nodes
68+
if len(nodeset.Spec.Nodes) == 0 {
69+
continue
70+
}
6371
if !nodeset.IsReady() {
6472
return false
6573
}

‎test/functional/ctlplane/base_test.go‎

Lines changed: 61 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -46,65 +46,70 @@ import (
4646
)
4747

4848
type Names struct {
49-
Namespace string
50-
OpenStackControlplaneName types.NamespacedName
51-
OpenStackVersionName types.NamespacedName
52-
OpenStackVersionName2 types.NamespacedName
53-
KeystoneAPIName types.NamespacedName
54-
MemcachedName types.NamespacedName
55-
MemcachedCertName types.NamespacedName
56-
CinderName types.NamespacedName
57-
ManilaName types.NamespacedName
58-
GlanceName types.NamespacedName
59-
NeutronName types.NamespacedName
60-
HorizonName types.NamespacedName
61-
HeatName types.NamespacedName
62-
NovaName types.NamespacedName
63-
TelemetryName types.NamespacedName
64-
WatcherName types.NamespacedName
65-
DBName types.NamespacedName
66-
DBCertName types.NamespacedName
67-
DBCell1Name types.NamespacedName
68-
DBCell1CertName types.NamespacedName
69-
RabbitMQName types.NamespacedName
70-
RabbitMQCertName types.NamespacedName
71-
RabbitMQCell1Name types.NamespacedName
72-
RabbitMQCell1CertName types.NamespacedName
73-
RabbitMQNotificationsName types.NamespacedName
74-
RabbitMQNotificationsCertName types.NamespacedName
75-
NoVNCProxyCell1CertPublicRouteName types.NamespacedName
76-
NoVNCProxyCell1CertPublicSvcName types.NamespacedName
77-
NoVNCProxyCell1CertVencryptName types.NamespacedName
78-
ServiceAccountName types.NamespacedName
79-
RoleName types.NamespacedName
80-
RoleBindingName types.NamespacedName
81-
RootCAPublicName types.NamespacedName
82-
RootCAInternalName types.NamespacedName
83-
RootCAOvnName types.NamespacedName
84-
RootCALibvirtName types.NamespacedName
85-
SelfSignedIssuerName types.NamespacedName
86-
CustomIssuerName types.NamespacedName
87-
CustomServiceCertSecretName types.NamespacedName
88-
CABundleName types.NamespacedName
89-
OpenStackClientName types.NamespacedName
90-
OVNNorthdName types.NamespacedName
91-
OVNNorthdCertName types.NamespacedName
92-
OVNControllerName types.NamespacedName
93-
OVNControllerCertName types.NamespacedName
94-
OVNDbServerNBName types.NamespacedName
95-
OVNDbServerSBName types.NamespacedName
96-
OVNMetricsCertName types.NamespacedName
97-
NeutronOVNCertName types.NamespacedName
98-
OpenStackTopology []types.NamespacedName
99-
WatcherCertPublicRouteName types.NamespacedName
100-
WatcherCertPublicSvcName types.NamespacedName
101-
WatcherCertInternalName types.NamespacedName
49+
Namespace string
50+
OpenStackControlplaneName types.NamespacedName
51+
OpenStackDataplaneNodeSetNoNodesName types.NamespacedName
52+
OpenStackVersionName types.NamespacedName
53+
OpenStackVersionName2 types.NamespacedName
54+
KeystoneAPIName types.NamespacedName
55+
MemcachedName types.NamespacedName
56+
MemcachedCertName types.NamespacedName
57+
CinderName types.NamespacedName
58+
ManilaName types.NamespacedName
59+
GlanceName types.NamespacedName
60+
NeutronName types.NamespacedName
61+
HorizonName types.NamespacedName
62+
HeatName types.NamespacedName
63+
NovaName types.NamespacedName
64+
TelemetryName types.NamespacedName
65+
WatcherName types.NamespacedName
66+
DBName types.NamespacedName
67+
DBCertName types.NamespacedName
68+
DBCell1Name types.NamespacedName
69+
DBCell1CertName types.NamespacedName
70+
RabbitMQName types.NamespacedName
71+
RabbitMQCertName types.NamespacedName
72+
RabbitMQCell1Name types.NamespacedName
73+
RabbitMQCell1CertName types.NamespacedName
74+
RabbitMQNotificationsName types.NamespacedName
75+
RabbitMQNotificationsCertName types.NamespacedName
76+
NoVNCProxyCell1CertPublicRouteName types.NamespacedName
77+
NoVNCProxyCell1CertPublicSvcName types.NamespacedName
78+
NoVNCProxyCell1CertVencryptName types.NamespacedName
79+
ServiceAccountName types.NamespacedName
80+
RoleName types.NamespacedName
81+
RoleBindingName types.NamespacedName
82+
RootCAPublicName types.NamespacedName
83+
RootCAInternalName types.NamespacedName
84+
RootCAOvnName types.NamespacedName
85+
RootCALibvirtName types.NamespacedName
86+
SelfSignedIssuerName types.NamespacedName
87+
CustomIssuerName types.NamespacedName
88+
CustomServiceCertSecretName types.NamespacedName
89+
CABundleName types.NamespacedName
90+
OpenStackClientName types.NamespacedName
91+
OVNNorthdName types.NamespacedName
92+
OVNNorthdCertName types.NamespacedName
93+
OVNControllerName types.NamespacedName
94+
OVNControllerCertName types.NamespacedName
95+
OVNDbServerNBName types.NamespacedName
96+
OVNDbServerSBName types.NamespacedName
97+
OVNMetricsCertName types.NamespacedName
98+
NeutronOVNCertName types.NamespacedName
99+
OpenStackTopology []types.NamespacedName
100+
WatcherCertPublicRouteName types.NamespacedName
101+
WatcherCertPublicSvcName types.NamespacedName
102+
WatcherCertInternalName types.NamespacedName
102103
}
103104

104105
func CreateNames(openstackControlplaneName types.NamespacedName) Names {
105106
return Names{
106107
Namespace: openstackControlplaneName.Namespace,
107108
OpenStackControlplaneName: openstackControlplaneName,
109+
OpenStackDataplaneNodeSetNoNodesName: types.NamespacedName{
110+
Namespace: openstackControlplaneName.Namespace,
111+
Name: "openstack-dataplane-nodeset-no-nodes",
112+
},
108113
OpenStackVersionName: types.NamespacedName{
109114
Namespace: openstackControlplaneName.Namespace,
110115
Name: openstackControlplaneName.Name, // same name as controlplane
@@ -392,6 +397,8 @@ func CreateOpenStackControlPlane(name types.NamespacedName, spec map[string]inte
392397
}
393398

394399
// Build OpenStackDataPlaneNodeSetSpec struct with empty `Nodes` list
400+
// NOTE: The function seems improperly named, given that the nodeset is
401+
// in fact given a node in the spec.
395402
func DefaultDataPlaneNoNodeSetSpec(tlsEnabled bool) map[string]interface{} {
396403
spec := map[string]interface{}{
397404
"preProvisioned": true,
@@ -401,8 +408,7 @@ func DefaultDataPlaneNoNodeSetSpec(tlsEnabled bool) map[string]interface{} {
401408
},
402409
"ansibleSSHPrivateKeySecret": "dataplane-ansible-ssh-private-key-secret",
403410
},
404-
"nodes": map[string]interface{}{},
405-
"servicesOverride": []string{},
411+
"nodes": map[string]interface{}{},
406412
}
407413
if tlsEnabled {
408414
spec["tlsEnabled"] = true

‎test/functional/ctlplane/openstackversion_controller_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,16 @@ var _ = Describe("OpenStackOperator controller", func() {
422422
// 5) simulate 1 more dataplanenodeset update to finish the minor update workflow
423423
It("updating targetVersion triggers a minor update workflow", Serial, func() {
424424

425+
// Also create an empty dataplanenodeset with no nodes. This allows the test to
426+
// verify that the minor update workflow works even with dataplane nodesets
427+
// present that also happen to have no nodes
428+
dataplanenodesetSpec := DefaultDataPlaneNoNodeSetSpec(false)
429+
dataplanenodesetSpec["nodes"] = map[string]interface{}{}
430+
DeferCleanup(
431+
th.DeleteInstance,
432+
CreateDataplaneNodeSet(names.OpenStackDataplaneNodeSetNoNodesName, dataplanenodesetSpec),
433+
)
434+
425435
// 1) switch to version 0.0.1, this triggers a minor update
426436
osversion := GetOpenStackVersion(names.OpenStackVersionName)
427437

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

529+
// Simulate the empty dataplanenodeset generation status as well
530+
dataplanenodeset = GetDataplaneNodeset(names.OpenStackDataplaneNodeSetNoNodesName)
531+
dataplanenodeset.Status.ObservedGeneration = dataplanenodeset.Generation
532+
Expect(th.K8sClient.Status().Update(th.Ctx, dataplanenodeset)).To(Succeed())
533+
519534
// and now finally we verify that OpenStackVersion is in the correct state (data plane OVN got updated)
520535
Eventually(func(g Gomega) {
521536

0 commit comments

Comments
 (0)