Skip OVN checks in minor update when OVN is disabled

internal/controller/core/openstackversion_controller.go

@@ -262,29 +262,35 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	// minor update in progress
 	if instance.Status.DeployedVersion != nil && instance.Spec.TargetVersion != *instance.Status.DeployedVersion {
 
-		if !openstack.OVNControllerImageMatch(ctx, controlPlane, instance) ||
-			!controlPlane.Status.Conditions.IsTrue(corev1beta1.OpenStackControlPlaneOVNReadyCondition) {
-			instance.Status.Conditions.Set(condition.FalseCondition(
-				corev1beta1.OpenStackVersionMinorUpdateOVNControlplane,
-				condition.RequestedReason,
-				condition.SeverityInfo,
-				corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
-			Log.Info("Minor update for OVN Controlplane in progress")
-			return ctrl.Result{}, nil
+		// Only check OVN when enabled to avoid hanging on a removed condition
+		if controlPlane.Spec.Ovn.Enabled {
+			if !openstack.OVNControllerImageMatch(ctx, controlPlane, instance) ||
+				!controlPlane.Status.Conditions.IsTrue(corev1beta1.OpenStackControlPlaneOVNReadyCondition) {
+				instance.Status.Conditions.Set(condition.FalseCondition(
+					corev1beta1.OpenStackVersionMinorUpdateOVNControlplane,
+					condition.RequestedReason,
+					condition.SeverityInfo,
+					corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
+				Log.Info("Minor update for OVN Controlplane in progress")
+				return ctrl.Result{}, nil
+			}
 		}
 		instance.Status.Conditions.MarkTrue(
 			corev1beta1.OpenStackVersionMinorUpdateOVNControlplane,
 			corev1beta1.OpenStackVersionMinorUpdateReadyMessage)
 
 		// minor update for Dataplane OVN
-		if !openstack.DataplaneNodesetsOVNControllerImagesMatch(instance, dataplaneNodesets) {
-			instance.Status.Conditions.Set(condition.FalseCondition(
-				corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
-				condition.RequestedReason,
-				condition.SeverityInfo,
-				corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
-			Log.Info("Waiting on OVN Dataplane updates to complete")
-			return ctrl.Result{}, nil
+		// Only check OVN when enabled to avoid hanging on a removed condition
+		if controlPlane.Spec.Ovn.Enabled {
+			if !openstack.DataplaneNodesetsOVNControllerImagesMatch(instance, dataplaneNodesets) {
+				instance.Status.Conditions.Set(condition.FalseCondition(
+					corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
+					condition.RequestedReason,
+					condition.SeverityInfo,
+					corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
+				Log.Info("Waiting on OVN Dataplane updates to complete")
+				return ctrl.Result{}, nil
+			}
 		}
 		instance.Status.Conditions.MarkTrue(
 			corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,

test/functional/ctlplane/openstackversion_controller_test.go

@@ -686,6 +686,345 @@ var _ = Describe("OpenStackOperator controller", func() {
 
 	})
 
+	// Test that minor updates don't hang when OVN is disabled
+	When("Minor update with OVN disabled", func() {
+		var (
+			initialVersion           = "old"
+			updatedVersion           = "0.0.1"
+			targetRabbitMQVersion    = ""
+			targetMariaDBVersion     = ""
+			targetMemcachedVersion   = ""
+			targetKeystoneAPIVersion = ""
+			testRabbitMQImage        = "foo/rabbit:0.0.2"
+			testMariaDBImage         = "foo/maria:0.0.2"
+			testMemcachedImage       = "foo/memcached:0.0.2"
+			testKeystoneAPIImage     = "foo/keystone:0.0.2"
+		)
+
+		BeforeEach(func() {
+			// Lightweight controlplane spec with OVN DISABLED
+			spec := GetDefaultOpenStackControlPlaneSpec()
+
+			// a single galera database
+			galeraTemplate := map[string]interface{}{
+				names.DBName.Name: map[string]interface{}{
+					"storageRequest": "500M",
+				},
+			}
+			spec["galera"] = map[string]interface{}{
+				"enabled":   true,
+				"templates": galeraTemplate,
+			}
+
+			// Disable non-essential services
+			spec["horizon"] = map[string]interface{}{"enabled": false}
+			spec["glance"] = map[string]interface{}{"enabled": false}
+			spec["cinder"] = map[string]interface{}{"enabled": false}
+			spec["neutron"] = map[string]interface{}{"enabled": false}
+			spec["manila"] = map[string]interface{}{"enabled": false}
+			spec["heat"] = map[string]interface{}{"enabled": false}
+			spec["telemetry"] = map[string]interface{}{"enabled": false}
+			spec["tls"] = GetTLSPublicSpec()
+
+			// CRITICAL: Disable OVN
+			spec["ovn"] = map[string]interface{}{
+				"enabled": false,
+			}
+
+			DeferCleanup(
+				th.DeleteInstance,
+				CreateOpenStackVersion(names.OpenStackVersionName, GetDefaultOpenStackVersionSpec()),
+			)
+
+			// create cert secrets for rabbitmq instances
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.RabbitMQCertName))
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.RabbitMQCell1CertName))
+
+			// create root CA secrets
+			DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAPublicName))
+			DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAInternalName))
+			DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCAOvnName))
+			DeferCleanup(k8sClient.Delete, ctx, CreateCertSecret(names.RootCALibvirtName))
+
+			// create cert secrets for galera instances
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.DBCertName))
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.DBCell1CertName))
+
+			// create cert secrets for memcached instance
+			DeferCleanup(k8sClient.Delete, ctx, th.CreateCertSecret(names.MemcachedCertName))
+
+			// wait for initial version to be created (this gives us version 0.0.1)
+			Eventually(func(g Gomega) {
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionInitialized,
+					k8s_corev1.ConditionTrue,
+				)
+
+				version := GetOpenStackVersion(names.OpenStackVersionName)
+				// capture target versions
+				targetRabbitMQVersion = *version.Status.ContainerImages.RabbitmqImage
+				targetMariaDBVersion = *version.Status.ContainerImages.MariadbImage
+				targetMemcachedVersion = *version.Status.ContainerImages.InfraMemcachedImage
+				targetKeystoneAPIVersion = *version.Status.ContainerImages.KeystoneAPIImage
+				g.Expect(version).Should(Not(BeNil()))
+
+				g.Expect(*version.Status.AvailableVersion).Should(ContainSubstring("0.0.1"))
+				g.Expect(version.Spec.TargetVersion).Should(ContainSubstring("0.0.1"))
+				updatedVersion = *version.Status.AvailableVersion
+			}, timeout, interval).Should(Succeed())
+
+			// inject an "old" version
+			Eventually(func(g Gomega) {
+				version := GetOpenStackVersion(names.OpenStackVersionName)
+				version.Status.ContainerImageVersionDefaults[initialVersion] = version.Status.ContainerImageVersionDefaults[updatedVersion]
+				version.Status.ContainerImageVersionDefaults[initialVersion].RabbitmqImage = &testRabbitMQImage
+				version.Status.ContainerImageVersionDefaults[initialVersion].MariadbImage = &testMariaDBImage
+				version.Status.ContainerImageVersionDefaults[initialVersion].InfraMemcachedImage = &testMemcachedImage
+				version.Status.ContainerImageVersionDefaults[initialVersion].KeystoneAPIImage = &testKeystoneAPIImage
+				g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				version := GetOpenStackVersion(names.OpenStackVersionName)
+				version.Spec.TargetVersion = initialVersion
+				g.Expect(th.K8sClient.Update(th.Ctx, version)).To(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				osversion := GetOpenStackVersion(names.OpenStackVersionName)
+				g.Expect(osversion).Should(Not(BeNil()))
+				g.Expect(osversion.Generation).Should(Equal(osversion.Status.ObservedGeneration))
+
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionInitialized,
+					k8s_corev1.ConditionTrue,
+				)
+
+				g.Expect(*osversion.Status.AvailableVersion).Should(Equal(updatedVersion))
+				g.Expect(osversion.Spec.TargetVersion).Should(Equal(initialVersion))
+				g.Expect(osversion.Status.DeployedVersion).Should(BeNil())
+			}, timeout, interval).Should(Succeed())
+
+			DeferCleanup(
+				th.DeleteInstance,
+				CreateOpenStackControlPlane(names.OpenStackControlplaneName, spec),
+			)
+
+			DeferCleanup(
+				th.DeleteInstance,
+				CreateDataplaneNodeSet(names.OpenStackVersionName, DefaultDataPlaneNoNodeSetSpec(false)),
+			)
+
+			dataplanenodeset := GetDataplaneNodeset(names.OpenStackVersionName)
+			dataplanenodeset.Status.DeployedVersion = initialVersion
+			Expect(th.K8sClient.Status().Update(th.Ctx, dataplanenodeset)).To(Succeed())
+
+			th.CreateSecret(types.NamespacedName{Name: "openstack-config-secret", Namespace: namespace}, map[string][]byte{"secure.yaml": []byte("foo")})
+			th.CreateConfigMap(types.NamespacedName{Name: "openstack-config", Namespace: namespace}, map[string]interface{}{"clouds.yaml": string("foo"), "OS_CLOUD": "default"})
+
+			// Verify OVN is disabled
+			OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
+			Expect(OSCtlplane.Spec.Ovn.Enabled).Should(BeFalse())
+
+			SimulateControlplaneReady()
+
+			// verify that DeployedVersion is set
+			Eventually(func(g Gomega) {
+				th.ExpectCondition(
+					names.OpenStackControlplaneName,
+					ConditionGetterFunc(OpenStackControlPlaneConditionGetter),
+					condition.ReadyCondition,
+					k8s_corev1.ConditionTrue,
+				)
+				OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
+				g.Expect(OSCtlplane.Status.DeployedVersion).Should(Equal(&initialVersion))
+			}, timeout, interval).Should(Succeed())
+
+			// verify DeployedVersion also gets set on the OpenStackVersion resource
+			Eventually(func(g Gomega) {
+				osversion := GetOpenStackVersion(names.OpenStackVersionName)
+				g.Expect(osversion).Should(Not(BeNil()))
+				g.Expect(osversion.Generation).Should(Equal(osversion.Status.ObservedGeneration))
+				g.Expect(osversion.Status.DeployedVersion).Should(Equal(&initialVersion))
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("updating targetVersion should not hang on OVN checks", Serial, func() {
+			// Trigger minor update by switching to updated version
+			osversion := GetOpenStackVersion(names.OpenStackVersionName)
+
+			// should have a condition which reflects an update is available
+			th.ExpectCondition(
+				names.OpenStackVersionName,
+				ConditionGetterFunc(OpenStackVersionConditionGetter),
+				corev1.OpenStackVersionMinorUpdateAvailable,
+				k8s_corev1.ConditionTrue,
+			)
+
+			osversion.Spec.TargetVersion = updatedVersion
+			Expect(k8sClient.Update(ctx, osversion)).Should(Succeed())
+
+			// Verify the OpenStackVersion gets re-initialized with new version
+			Eventually(func(g Gomega) {
+				osversion := GetOpenStackVersion(names.OpenStackVersionName)
+				g.Expect(osversion).Should(Not(BeNil()))
+				g.Expect(osversion.Generation).Should(Equal(osversion.Status.ObservedGeneration))
+
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionInitialized,
+					k8s_corev1.ConditionTrue,
+				)
+
+				g.Expect(*osversion.Status.AvailableVersion).Should(Equal(updatedVersion))
+				g.Expect(osversion.Spec.TargetVersion).Should(Equal(updatedVersion))
+				// target images should be set
+				g.Expect(*osversion.Status.ContainerImages.RabbitmqImage).Should(Equal(targetRabbitMQVersion))
+				g.Expect(*osversion.Status.ContainerImages.MariadbImage).Should(Equal(targetMariaDBVersion))
+				g.Expect(*osversion.Status.ContainerImages.InfraMemcachedImage).Should(Equal(targetMemcachedVersion))
+				g.Expect(*osversion.Status.ContainerImages.KeystoneAPIImage).Should(Equal(targetKeystoneAPIVersion))
+			}, timeout, interval).Should(Succeed())
+
+			// CRITICAL: Verify that OVN controlplane update condition is immediately set to true (not hanging)
+			// This is the key assertion that proves the bug is fixed
+			Eventually(func(g Gomega) {
+				osversion := GetOpenStackVersion(names.OpenStackVersionName)
+				g.Expect(osversion).Should(Not(BeNil()))
+
+				// The OVN update condition should be true because OVN is disabled
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionMinorUpdateOVNControlplane,
+					k8s_corev1.ConditionTrue,
+				)
+			}, timeout, interval).Should(Succeed())
+
+			// Verify OVN dataplane update also proceeds
+			Eventually(func(_ Gomega) {
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionMinorUpdateOVNDataplane,
+					k8s_corev1.ConditionTrue,
+				)
+			}, timeout, interval).Should(Succeed())
+
+			// Continue with infrastructure services
+			th.ExpectCondition(
+				names.OpenStackVersionName,
+				ConditionGetterFunc(OpenStackVersionConditionGetter),
+				corev1.OpenStackVersionMinorUpdateRabbitMQ,
+				k8s_corev1.ConditionFalse,
+			)
+
+			SimulateRabbitmqReady()
+			Eventually(func(g Gomega) {
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionMinorUpdateRabbitMQ,
+					k8s_corev1.ConditionTrue,
+				)
+
+				OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
+				g.Expect(*OSCtlplane.Status.ContainerImages.RabbitmqImage).Should(Equal(targetRabbitMQVersion))
+			}, timeout*4, interval).Should(Succeed())
+
+			SimulateGalaraReady()
+			Eventually(func(g Gomega) {
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionMinorUpdateMariaDB,
+					k8s_corev1.ConditionTrue,
+				)
+				OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
+				g.Expect(*OSCtlplane.Status.ContainerImages.MariadbImage).Should(Equal(targetMariaDBVersion))
+			}, timeout, interval).Should(Succeed())
+
+			SimulateMemcachedReady()
+			Eventually(func(g Gomega) {
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionMinorUpdateMemcached,
+					k8s_corev1.ConditionTrue,
+				)
+				OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
+				g.Expect(*OSCtlplane.Status.ContainerImages.InfraMemcachedImage).Should(Equal(targetMemcachedVersion))
+			}, timeout, interval).Should(Succeed())
+
+			keystone.SimulateKeystoneAPIReady(names.KeystoneAPIName)
+			Eventually(func(g Gomega) {
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionMinorUpdateKeystone,
+					k8s_corev1.ConditionTrue,
+				)
+
+				osversion := GetOpenStackVersion(names.OpenStackVersionName)
+				OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
+				g.Expect(OSCtlplane.Status.ContainerImages.KeystoneAPIImage).Should(Equal(osversion.Status.ContainerImages.KeystoneAPIImage))
+			}, timeout, interval).Should(Succeed())
+
+			// Simulate controlplane ready
+			SimulateControlplaneReady()
+			Eventually(func(g Gomega) {
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					corev1.OpenStackVersionMinorUpdateControlplane,
+					k8s_corev1.ConditionTrue,
+				)
+				th.ExpectCondition(
+					names.OpenStackControlplaneName,
+					ConditionGetterFunc(OpenStackControlPlaneConditionGetter),
+					condition.ReadyCondition,
+					k8s_corev1.ConditionTrue,
+				)
+
+				osversion := GetOpenStackVersion(names.OpenStackVersionName)
+				OSCtlplane := GetOpenStackControlPlane(names.OpenStackControlplaneName)
+				// verify images match
+				g.Expect(OSCtlplane.Status.ContainerImages.RabbitmqImage).Should(Equal(osversion.Status.ContainerImages.RabbitmqImage))
+				g.Expect(OSCtlplane.Status.ContainerImages.MariadbImage).Should(Equal(osversion.Status.ContainerImages.MariadbImage))
+				g.Expect(OSCtlplane.Status.ContainerImages.KeystoneAPIImage).Should(Equal(osversion.Status.ContainerImages.KeystoneAPIImage))
+				g.Expect(OSCtlplane.Status.ContainerImages.InfraMemcachedImage).Should(Equal(osversion.Status.ContainerImages.InfraMemcachedImage))
+			}, timeout, interval).Should(Succeed())
+
+			// Simulate dataplane deployment complete
+			dataplanenodeset := GetDataplaneNodeset(names.OpenStackVersionName)
+			dataplanenodeset.Status.ObservedGeneration = dataplanenodeset.Generation
+			dataplanenodeset.Status.DeployedVersion = osversion.Spec.TargetVersion
+			dataplanenodeset.Status.Conditions.MarkTrue(condition.ReadyCondition, dataplanev1.NodeSetReadyMessage)
+			Expect(th.K8sClient.Status().Update(th.Ctx, dataplanenodeset)).To(Succeed())
+
+			// Verify minor update completes successfully
+			Eventually(func(g Gomega) {
+				osversion := GetOpenStackVersion(names.OpenStackVersionName)
+				g.Expect(osversion).Should(Not(BeNil()))
+				g.Expect(osversion.OwnerReferences).Should(HaveLen(1))
+				th.ExpectCondition(
+					names.OpenStackVersionName,
+					ConditionGetterFunc(OpenStackVersionConditionGetter),
+					condition.ReadyCondition,
+					k8s_corev1.ConditionTrue,
+				)
+				g.Expect(osversion.Status.DeployedVersion).Should(Equal(&updatedVersion))
+				// no condition which reflects an update is available
+				g.Expect(osversion.Status.Conditions.Has(corev1.OpenStackVersionMinorUpdateAvailable)).To(BeFalse())
+			}, timeout, interval).Should(Succeed())
+		})
+
+	})
+
 	When("CustomContainerImages are set", func() {
 		var (
 			initialVersion = "0.0.1"