Skip to content

Commit c5c4018

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1744 from dprince/ovn_minor_update_check
Skip OVN checks in minor update when OVN is disabled
2 parents be703f6 + f1b3dba commit c5c4018

File tree

2 files changed

+362
-17
lines changed

2 files changed

+362
-17
lines changed

‎internal/controller/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,

‎test/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)