Skip to content

Commit d504256

Browse files
dprinceopenshift-cherrypick-robot
dprince
authored and
openshift-cherrypick-robot
committed
Skip OVN checks in minor update when OVN is disabled
Fixes a bug where the minor update sequence would hang when OVN is disabled. The controller was checking OVN conditions that don't exist when OVN is not enabled, causing the update process to stall. Changes: - Add Ovn.Enabled checks before OVN image and condition validation - Skip OVN controlplane and dataplane update checks when disabled - Add comprehensive functional test for minor updates with OVN disabled Co-Authored-By: Claude Sonnet 4.5 <[email protected]> Jira: OSPRH-23103
1 parent 8b80cad commit d504256

File tree

2 files changed

+362
-17
lines changed

2 files changed

+362
-17
lines changed

‎controllers/core/openstackversion_controller.go‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -262,29 +262,35 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
262262
// minor update in progress
263263
if instance.Status.DeployedVersion != nil && instance.Spec.TargetVersion != *instance.Status.DeployedVersion {
264264

265-
if !openstack.OVNControllerImageMatch(ctx, controlPlane, instance) ||
266-
!controlPlane.Status.Conditions.IsTrue(corev1beta1.OpenStackControlPlaneOVNReadyCondition) {
267-
instance.Status.Conditions.Set(condition.FalseCondition(
268-
corev1beta1.OpenStackVersionMinorUpdateOVNControlplane,
269-
condition.RequestedReason,
270-
condition.SeverityInfo,
271-
corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
272-
Log.Info("Minor update for OVN Controlplane in progress")
273-
return ctrl.Result{}, nil
265+
// Only check OVN when enabled to avoid hanging on a removed condition
266+
if controlPlane.Spec.Ovn.Enabled {
267+
if !openstack.OVNControllerImageMatch(ctx, controlPlane, instance) ||
268+
!controlPlane.Status.Conditions.IsTrue(corev1beta1.OpenStackControlPlaneOVNReadyCondition) {
269+
instance.Status.Conditions.Set(condition.FalseCondition(
270+
corev1beta1.OpenStackVersionMinorUpdateOVNControlplane,
271+
condition.RequestedReason,
272+
condition.SeverityInfo,
273+
corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
274+
Log.Info("Minor update for OVN Controlplane in progress")
275+
return ctrl.Result{}, nil
276+
}
274277
}
275278
instance.Status.Conditions.MarkTrue(
276279
corev1beta1.OpenStackVersionMinorUpdateOVNControlplane,
277280
corev1beta1.OpenStackVersionMinorUpdateReadyMessage)
278281

279282
// minor update for Dataplane OVN
280-
if !openstack.DataplaneNodesetsOVNControllerImagesMatch(instance, dataplaneNodesets) {
281-
instance.Status.Conditions.Set(condition.FalseCondition(
282-
corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
283-
condition.RequestedReason,
284-
condition.SeverityInfo,
285-
corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
286-
Log.Info("Waiting on OVN Dataplane updates to complete")
287-
return ctrl.Result{}, nil
283+
// Only check OVN when enabled to avoid hanging on a removed condition
284+
if controlPlane.Spec.Ovn.Enabled {
285+
if !openstack.DataplaneNodesetsOVNControllerImagesMatch(instance, dataplaneNodesets) {
286+
instance.Status.Conditions.Set(condition.FalseCondition(
287+
corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
288+
condition.RequestedReason,
289+
condition.SeverityInfo,
290+
corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
291+
Log.Info("Waiting on OVN Dataplane updates to complete")
292+
return ctrl.Result{}, nil
293+
}
288294
}
289295
instance.Status.Conditions.MarkTrue(
290296
corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,

‎tests/functional/ctlplane/openstackversion_controller_test.go‎

Lines changed: 339 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,345 @@ var _ = Describe("OpenStackOperator controller", func() {
686686

687687
})
688688

689+
// Test that minor updates don't hang when OVN is disabled
690+
When("Minor update with OVN disabled", func() {
691+
var (
692+
initialVersion = "old"
693+
updatedVersion = "0.0.1"
694+
targetRabbitMQVersion = ""
695+
targetMariaDBVersion = ""
696+
targetMemcachedVersion = ""
697+
targetKeystoneAPIVersion = ""
698+
testRabbitMQImage = "foo/rabbit:0.0.2"
699+
testMariaDBImage = "foo/maria:0.0.2"
700+
testMemcachedImage = "foo/memcached:0.0.2"
701+
testKeystoneAPIImage = "foo/keystone:0.0.2"
702+
)
703+
704+
BeforeEach(func() {
705+
// Lightweight controlplane spec with OVN DISABLED
706+
spec := GetDefaultOpenStackControlPlaneSpec()
707+
708+
// a single galera database
709+
galeraTemplate := map[string]interface{}{
710+
names.DBName.Name: map[string]interface{}{
711+
"storageRequest": "500M",
712+
},
713+
}
714+
spec["galera"] = map[string]interface{}{
715+
"enabled": true,
716+
"templates": galeraTemplate,
717+
}
718+
719+
// Disable non-essential services
720+
spec["horizon"] = map[string]interface{}{"enabled": false}
721+
spec["glance"] = map[string]interface{}{"enabled": false}
722+
spec["cinder"] = map[string]interface{}{"enabled": false}
723+
spec["neutron"] = map[string]interface{}{"enabled": false}
724+
spec["manila"] = map[string]interface{}{"enabled": false}
725+
spec["heat"] = map[string]interface{}{"enabled": false}
726+
spec["telemetry"] = map[string]interface{}{"enabled": false}
727+
spec["tls"] = GetTLSPublicSpec()
728+
729+
// CRITICAL: Disable OVN
730+
spec["ovn"] = map[string]interface{}{
731+
"enabled": false,
732+
}
733+
734+
DeferCleanup(
735+
th.DeleteInstance,
736+
CreateOpenStackVersion(names.OpenStackVersionName, GetDefaultOpenStackVersionSpec()),
737+
)
738+
739+
// create cert secrets for rabbitmq instances
740+
DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.RabbitMQCertName))
741+
DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.RabbitMQCell1CertName))
742+
743+
// create root CA secrets
744+
DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAPublicName))
745+
DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAInternalName))
746+
DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAOvnName))
747+
DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCALibvirtName))
748+
749+
// create cert secrets for galera instances
750+
DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.DBCertName))
751+
DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.DBCell1CertName))
752+
753+
// create cert secrets for memcached instance
754+
DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.MemcachedCertName))
755+
756+
// wait for initial version to be created (this gives us version 0.0.1)
757+
Eventually(func(g Gomega) {
758+
th.ExpectCondition(
759+
names.OpenStackVersionName,
760+
ConditionGetterFunc(OpenStackVersionConditionGetter),
761+
corev1.OpenStackVersionInitialized,
762+
k8s_corev1.ConditionTrue,
763+
)
764+
765+
version := GetOpenStackVersion(names.OpenStackVersionName)
766+
// capture target versions
767+
targetRabbitMQVersion = *version.Status.ContainerImages.RabbitmqImage
768+
targetMariaDBVersion = *version.Status.ContainerImages.MariadbImage
769+
targetMemcachedVersion = *version.Status.ContainerImages.InfraMemcachedImage
770+
targetKeystoneAPIVersion = *version.Status.ContainerImages.KeystoneAPIImage
771+
g.Expect(version).Should(Not(BeNil()))
772+
773+
g.Expect(*version.Status.AvailableVersion).Should(ContainSubstring("0.0.1"))
774+
g.Expect(version.Spec.TargetVersion).Should(ContainSubstring("0.0.1"))
775+
updatedVersion = *version.Status.AvailableVersion
776+
}, timeout, interval).Should(Succeed())
777+
778+
// inject an "old" version
779+
Eventually(func(g Gomega) {
780+
version := GetOpenStackVersion(names.OpenStackVersionName)
781+
version.Status.ContainerImageVersionDefaults[initialVersion] = version.Status.ContainerImageVersionDefaults[updatedVersion]
782+
version.Status.ContainerImageVersionDefaults[initialVersion].RabbitmqImage = &testRabbitMQImage
783+
version.Status.ContainerImageVersionDefaults[initialVersion].MariadbImage = &testMariaDBImage
784+
version.Status.ContainerImageVersionDefaults[initialVersion].InfraMemcachedImage = &testMemcachedImage
785+
version.Status.ContainerImageVersionDefaults[initialVersion].KeystoneAPIImage = &testKeystoneAPIImage
786+
g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed())
787+
}, timeout, interval).Should(Succeed())
788+
789+
Eventually(func(g Gomega) {
790+
version := GetOpenStackVersion(names.OpenStackVersionName)
791+
version.Spec.TargetVersion = initialVersion
792+
g.Expect(th.K8sClient.Update(th.Ctx, version)).To(Succeed())
793+
}, timeout, interval).Should(Succeed())
794+
795+
Eventually(func(g Gomega) {
796+
osversion := GetOpenStackVersion(names.OpenStackVersionName)
797+
g.Expect(osversion).Should(Not(BeNil()))
798+
g.Expect(osversion.Generation).Should(Equal(osversion.Status.ObservedGeneration))
799+
800+
th.ExpectCondition(
801+
names.OpenStackVersionName,
802+
ConditionGetterFunc(OpenStackVersionConditionGetter),
803+
corev1.OpenStackVersionInitialized,
804+
k8s_corev1.ConditionTrue,
805+
)
806+
807+
g.Expect(*osversion.Status.AvailableVersion).Should(Equal(updatedVersion))
808+
g.Expect(osversion.Spec.TargetVersion).Should(Equal(initialVersion))
809+
g.Expect(osversion.Status.DeployedVersion).Should(BeNil())
810+
}, timeout, interval).Should(Succeed())
811+
812+
DeferCleanup(
813+
th.DeleteInstance,
814+
CreateOpenStackControlPlane(names.OpenStackControlplaneName, spec),
815+
)
816+
817+
DeferCleanup(
818+
th.DeleteInstance,
819+
CreateDataplaneNodeSet(names.OpenStackVersionName, DefaultDataPlaneNoNodeSetSpec(false)),
820+
)
821+
822+
dataplanenodeset := GetDataplaneNodeset(names.OpenStackVersionName)
823+
dataplanenodeset.Status.DeployedVersion = initialVersion
824+
Expect(th.K8sClient.Status().Update(th.Ctx, dataplanenodeset)).To(Succeed())
825+
826+
th.CreateSecret(types.NamespacedName{Name: "openstack-config-secret", Namespace: namespace}, map[string][]byte{"secure.yaml": []byte("foo")})
827+
th.CreateConfigMap(types.NamespacedName{Name: "openstack-config", Namespace: namespace}, map[string]interface{}{"clouds.yaml": string("foo"), "OS_CLOUD": "default"})
828+
829+
// Verify OVN is disabled
830+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
831+
Expect(OSCtlplane.Spec.Ovn.Enabled).Should(BeFalse())
832+
833+
SimulateControlplaneReady()
834+
835+
// verify that DeployedVersion is set
836+
Eventually(func(g Gomega) {
837+
th.ExpectCondition(
838+
names.OpenStackControlplaneName,
839+
ConditionGetterFunc(OpenStackControlPlaneConditionGetter),
840+
condition.ReadyCondition,
841+
k8s_corev1.ConditionTrue,
842+
)
843+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
844+
g.Expect(OSCtlplane.Status.DeployedVersion).Should(Equal(&initialVersion))
845+
}, timeout, interval).Should(Succeed())
846+
847+
// verify DeployedVersion also gets set on the OpenStackVersion resource
848+
Eventually(func(g Gomega) {
849+
osversion := GetOpenStackVersion(names.OpenStackVersionName)
850+
g.Expect(osversion).Should(Not(BeNil()))
851+
g.Expect(osversion.Generation).Should(Equal(osversion.Status.ObservedGeneration))
852+
g.Expect(osversion.Status.DeployedVersion).Should(Equal(&initialVersion))
853+
}, timeout, interval).Should(Succeed())
854+
})
855+
856+
It("updating targetVersion should not hang on OVN checks", Serial, func() {
857+
// Trigger minor update by switching to updated version
858+
osversion := GetOpenStackVersion(names.OpenStackVersionName)
859+
860+
// should have a condition which reflects an update is available
861+
th.ExpectCondition(
862+
names.OpenStackVersionName,
863+
ConditionGetterFunc(OpenStackVersionConditionGetter),
864+
corev1.OpenStackVersionMinorUpdateAvailable,
865+
k8s_corev1.ConditionTrue,
866+
)
867+
868+
osversion.Spec.TargetVersion = updatedVersion
869+
Expect(k8sClient.Update(ctx, osversion)).Should(Succeed())
870+
871+
// Verify the OpenStackVersion gets re-initialized with new version
872+
Eventually(func(g Gomega) {
873+
osversion := GetOpenStackVersion(names.OpenStackVersionName)
874+
g.Expect(osversion).Should(Not(BeNil()))
875+
g.Expect(osversion.Generation).Should(Equal(osversion.Status.ObservedGeneration))
876+
877+
th.ExpectCondition(
878+
names.OpenStackVersionName,
879+
ConditionGetterFunc(OpenStackVersionConditionGetter),
880+
corev1.OpenStackVersionInitialized,
881+
k8s_corev1.ConditionTrue,
882+
)
883+
884+
g.Expect(*osversion.Status.AvailableVersion).Should(Equal(updatedVersion))
885+
g.Expect(osversion.Spec.TargetVersion).Should(Equal(updatedVersion))
886+
// target images should be set
887+
g.Expect(*osversion.Status.ContainerImages.RabbitmqImage).Should(Equal(targetRabbitMQVersion))
888+
g.Expect(*osversion.Status.ContainerImages.MariadbImage).Should(Equal(targetMariaDBVersion))
889+
g.Expect(*osversion.Status.ContainerImages.InfraMemcachedImage).Should(Equal(targetMemcachedVersion))
890+
g.Expect(*osversion.Status.ContainerImages.KeystoneAPIImage).Should(Equal(targetKeystoneAPIVersion))
891+
}, timeout, interval).Should(Succeed())
892+
893+
// CRITICAL: Verify that OVN controlplane update condition is immediately set to true (not hanging)
894+
// This is the key assertion that proves the bug is fixed
895+
Eventually(func(g Gomega) {
896+
osversion := GetOpenStackVersion(names.OpenStackVersionName)
897+
g.Expect(osversion).Should(Not(BeNil()))
898+
899+
// The OVN update condition should be true because OVN is disabled
900+
th.ExpectCondition(
901+
names.OpenStackVersionName,
902+
ConditionGetterFunc(OpenStackVersionConditionGetter),
903+
corev1.OpenStackVersionMinorUpdateOVNControlplane,
904+
k8s_corev1.ConditionTrue,
905+
)
906+
}, timeout, interval).Should(Succeed())
907+
908+
// Verify OVN dataplane update also proceeds
909+
Eventually(func(_ Gomega) {
910+
th.ExpectCondition(
911+
names.OpenStackVersionName,
912+
ConditionGetterFunc(OpenStackVersionConditionGetter),
913+
corev1.OpenStackVersionMinorUpdateOVNDataplane,
914+
k8s_corev1.ConditionTrue,
915+
)
916+
}, timeout, interval).Should(Succeed())
917+
918+
// Continue with infrastructure services
919+
th.ExpectCondition(
920+
names.OpenStackVersionName,
921+
ConditionGetterFunc(OpenStackVersionConditionGetter),
922+
corev1.OpenStackVersionMinorUpdateRabbitMQ,
923+
k8s_corev1.ConditionFalse,
924+
)
925+
926+
SimulateRabbitmqReady()
927+
Eventually(func(g Gomega) {
928+
th.ExpectCondition(
929+
names.OpenStackVersionName,
930+
ConditionGetterFunc(OpenStackVersionConditionGetter),
931+
corev1.OpenStackVersionMinorUpdateRabbitMQ,
932+
k8s_corev1.ConditionTrue,
933+
)
934+
935+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
936+
g.Expect(*OSCtlplane.Status.ContainerImages.RabbitmqImage).Should(Equal(targetRabbitMQVersion))
937+
}, timeout*4, interval).Should(Succeed())
938+
939+
SimulateGalaraReady()
940+
Eventually(func(g Gomega) {
941+
th.ExpectCondition(
942+
names.OpenStackVersionName,
943+
ConditionGetterFunc(OpenStackVersionConditionGetter),
944+
corev1.OpenStackVersionMinorUpdateMariaDB,
945+
k8s_corev1.ConditionTrue,
946+
)
947+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
948+
g.Expect(*OSCtlplane.Status.ContainerImages.MariadbImage).Should(Equal(targetMariaDBVersion))
949+
}, timeout, interval).Should(Succeed())
950+
951+
SimulateMemcachedReady()
952+
Eventually(func(g Gomega) {
953+
th.ExpectCondition(
954+
names.OpenStackVersionName,
955+
ConditionGetterFunc(OpenStackVersionConditionGetter),
956+
corev1.OpenStackVersionMinorUpdateMemcached,
957+
k8s_corev1.ConditionTrue,
958+
)
959+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
960+
g.Expect(*OSCtlplane.Status.ContainerImages.InfraMemcachedImage).Should(Equal(targetMemcachedVersion))
961+
}, timeout, interval).Should(Succeed())
962+
963+
keystone.SimulateKeystoneAPIReady(names.KeystoneAPIName)
964+
Eventually(func(g Gomega) {
965+
th.ExpectCondition(
966+
names.OpenStackVersionName,
967+
ConditionGetterFunc(OpenStackVersionConditionGetter),
968+
corev1.OpenStackVersionMinorUpdateKeystone,
969+
k8s_corev1.ConditionTrue,
970+
)
971+
972+
osversion := GetOpenStackVersion(names.OpenStackVersionName)
973+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
974+
g.Expect(OSCtlplane.Status.ContainerImages.KeystoneAPIImage).Should(Equal(osversion.Status.ContainerImages.KeystoneAPIImage))
975+
}, timeout, interval).Should(Succeed())
976+
977+
// Simulate controlplane ready
978+
SimulateControlplaneReady()
979+
Eventually(func(g Gomega) {
980+
th.ExpectCondition(
981+
names.OpenStackVersionName,
982+
ConditionGetterFunc(OpenStackVersionConditionGetter),
983+
corev1.OpenStackVersionMinorUpdateControlplane,
984+
k8s_corev1.ConditionTrue,
985+
)
986+
th.ExpectCondition(
987+
names.OpenStackControlplaneName,
988+
ConditionGetterFunc(OpenStackControlPlaneConditionGetter),
989+
condition.ReadyCondition,
990+
k8s_corev1.ConditionTrue,
991+
)
992+
993+
osversion := GetOpenStackVersion(names.OpenStackVersionName)
994+
OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
995+
// verify images match
996+
g.Expect(OSCtlplane.Status.ContainerImages.RabbitmqImage).Should(Equal(osversion.Status.ContainerImages.RabbitmqImage))
997+
g.Expect(OSCtlplane.Status.ContainerImages.MariadbImage).Should(Equal(osversion.Status.ContainerImages.MariadbImage))
998+
g.Expect(OSCtlplane.Status.ContainerImages.KeystoneAPIImage).Should(Equal(osversion.Status.ContainerImages.KeystoneAPIImage))
999+
g.Expect(OSCtlplane.Status.ContainerImages.InfraMemcachedImage).Should(Equal(osversion.Status.ContainerImages.InfraMemcachedImage))
1000+
}, timeout, interval).Should(Succeed())
1001+
1002+
// Simulate dataplane deployment complete
1003+
dataplanenodeset := GetDataplaneNodeset(names.OpenStackVersionName)
1004+
dataplanenodeset.Status.ObservedGeneration = dataplanenodeset.Generation
1005+
dataplanenodeset.Status.DeployedVersion = osversion.Spec.TargetVersion
1006+
dataplanenodeset.Status.Conditions.MarkTrue(condition.ReadyCondition, dataplanev1.NodeSetReadyMessage)
1007+
Expect(th.K8sClient.Status().Update(th.Ctx, dataplanenodeset)).To(Succeed())
1008+
1009+
// Verify minor update completes successfully
1010+
Eventually(func(g Gomega) {
1011+
osversion := GetOpenStackVersion(names.OpenStackVersionName)
1012+
g.Expect(osversion).Should(Not(BeNil()))
1013+
g.Expect(osversion.OwnerReferences).Should(HaveLen(1))
1014+
th.ExpectCondition(
1015+
names.OpenStackVersionName,
1016+
ConditionGetterFunc(OpenStackVersionConditionGetter),
1017+
condition.ReadyCondition,
1018+
k8s_corev1.ConditionTrue,
1019+
)
1020+
g.Expect(osversion.Status.DeployedVersion).Should(Equal(&updatedVersion))
1021+
// no condition which reflects an update is available
1022+
g.Expect(osversion.Status.Conditions.Has(corev1.OpenStackVersionMinorUpdateAvailable)).To(BeFalse())
1023+
}, timeout, interval).Should(Succeed())
1024+
})
1025+
1026+
})
1027+
6891028
When("CustomContainerImages are set", func() {
6901029
var (
6911030
initialVersion = "0.0.1"

0 commit comments

Comments
 (0)