From d504256c68bbb58034b9881701aea04faa8b4615 Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Fri, 19 Dec 2025 08:19:31 -0500 Subject: [PATCH] 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 Jira: OSPRH-23103 --- .../core/openstackversion_controller.go | 40 ++- .../openstackversion_controller_test.go | 339 ++++++++++++++++++ 2 files changed, 362 insertions(+), 17 deletions(-) diff --git a/controllers/core/openstackversion_controller.go b/controllers/core/openstackversion_controller.go index 4b0b46bcd..a6ff43303 100644 --- a/controllers/core/openstackversion_controller.go +++ b/controllers/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, diff --git a/tests/functional/ctlplane/openstackversion_controller_test.go b/tests/functional/ctlplane/openstackversion_controller_test.go index 2d647986d..42db3781c 100644 --- a/tests/functional/ctlplane/openstackversion_controller_test.go +++ b/tests/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"