Merge pull request #1864 from stuggi/ovn_update_cond

Fix MinorUpdateOVNDataplane race when OVN image unchanged between versions

Author: openshift-merge-bot[bot]

internal/controller/core/openstackversion_controller.go

@@ -291,6 +291,75 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 				Log.Info("Waiting on OVN Dataplane updates to complete")
 				return ctrl.Result{}, nil
 			}
+
+			// When the OVN controller image is the same between the deployed
+			// version and the target version, the image comparison above always
+			// passes because the nodeset already has the matching image from
+			// the previous update. In this case we need additional checks to
+			// confirm the OVN dataplane deployment for this update cycle has
+			// actually completed.
+			//
+			// We use the saved condition state (from before Init reset) to
+			// track whether we have observed a running OVN deployment during
+			// this update cycle:
+			// - If we see a running OVN deployment now: set condition False
+			//   (RequestedReason) to record that we observed one
+			// - If no running OVN deployment AND the previous condition was
+			//   False/RequestedReason: the deployment we saw previously has
+			//   completed → proceed (fall through to set True)
+			// - If no running OVN deployment AND the previous condition was
+			//   NOT False/RequestedReason (e.g. still Unknown from Init):
+			//   we haven't seen a deployment yet → keep waiting
+			//
+			// When the image differs between versions, the image match alone
+			// is sufficient proof that a deployment updated it, since the
+			// nodeset's ContainerImages are only set on successful completion.
+			deployedDefaults, hasDeployedDefaults := instance.Status.ContainerImageVersionDefaults[*instance.Status.DeployedVersion]
+			if hasDeployedDefaults &&
+				deployedDefaults.OvnControllerImage != nil &&
+				instance.Status.ContainerImages.OvnControllerImage != nil &&
+				*deployedDefaults.OvnControllerImage == *instance.Status.ContainerImages.OvnControllerImage {
+
+				ovnDeploymentRunning, err := openstack.IsDataplaneDeploymentRunningForServiceType(
+					ctx, versionHelper, instance.Namespace, dataplaneNodesets, "ovn")
+				if err != nil {
+					return ctrl.Result{}, err
+				}
+
+				if ovnDeploymentRunning {
+					// OVN deployment is actively running — record this in
+					// the condition so we can detect its completion later.
+					instance.Status.Conditions.Set(condition.FalseCondition(
+						corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
+						condition.RequestedReason,
+						condition.SeverityInfo,
+						corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
+					Log.Info("Waiting on OVN Dataplane deployment to complete (OVN image unchanged between versions)")
+					return ctrl.Result{}, nil
+				}
+
+				// No OVN deployment running. Check the saved condition state
+				// from the previous reconciliation to determine if we ever
+				// observed one running during this update cycle.
+				prevOvnDataplaneCond := savedConditions.Get(corev1beta1.OpenStackVersionMinorUpdateOVNDataplane)
+				if prevOvnDataplaneCond == nil ||
+					prevOvnDataplaneCond.Reason != condition.RequestedReason {
+					// We have never observed a running OVN deployment in
+					// this update cycle — the deployment has not been
+					// created yet. Keep waiting.
+					instance.Status.Conditions.Set(condition.FalseCondition(
+						corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
+						condition.InitReason,
+						condition.SeverityInfo,
+						corev1beta1.OpenStackVersionMinorUpdateReadyRunningMessage))
+					Log.Info("Waiting for OVN Dataplane deployment to be created (OVN image unchanged between versions)")
+					return ctrl.Result{}, nil
+				}
+				// Previously saw a running OVN deployment (condition was
+				// False/RequestedReason), now no OVN deployment is running
+				// → the deployment has completed. Fall through to set True.
+				Log.Info("OVN Dataplane deployment completed (OVN image unchanged between versions)")
+			}
 		}
 		instance.Status.Conditions.MarkTrue(
 			corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,

internal/openstack/dataplane.go

@@ -7,6 +7,8 @@ import (
 	corev1beta1 "github.com/openstack-k8s-operators/openstack-operator/api/core/v1beta1"
 
 	dataplanev1 "github.com/openstack-k8s-operators/openstack-operator/api/dataplane/v1beta1"
+	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -58,6 +60,85 @@ func DataplaneNodesetsOVNControllerImagesMatch(version *corev1beta1.OpenStackVer
 	return true
 }
 
+// IsDataplaneDeploymentRunningForServiceType checks whether any in-progress
+// OpenStackDataPlaneDeployment is deploying a service with the given
+// EDPMServiceType (e.g. "ovn"). It resolves which services each deployment
+// runs (from ServicesOverride or the nodeset's service list) and inspects
+// the service's EDPMServiceType to determine if it matches.
+func IsDataplaneDeploymentRunningForServiceType(
+	ctx context.Context,
+	h *helper.Helper,
+	namespace string,
+	dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList,
+	serviceType string,
+) (bool, error) {
+	// List all deployments in the namespace
+	deployments := &dataplanev1.OpenStackDataPlaneDeploymentList{}
+	opts := []client.ListOption{
+		client.InNamespace(namespace),
+	}
+	err := h.GetClient().List(ctx, deployments, opts...)
+	if err != nil {
+		return false, err
+	}
+
+	// Build a map of nodeset name -> nodeset for quick lookup
+	nodesetMap := make(map[string]*dataplanev1.OpenStackDataPlaneNodeSet, len(dataplaneNodesets.Items))
+	for i := range dataplaneNodesets.Items {
+		nodesetMap[dataplaneNodesets.Items[i].Name] = &dataplaneNodesets.Items[i]
+	}
+
+	// Cache service lookups to avoid repeated API calls
+	serviceCache := make(map[string]*dataplanev1.OpenStackDataPlaneService)
+
+	for _, deployment := range deployments.Items {
+		// Skip completed deployments
+		if deployment.Status.Deployed {
+			continue
+		}
+
+		// Determine which services this deployment runs for each of its nodesets
+		for _, nodesetName := range deployment.Spec.NodeSets {
+			nodeset, exists := nodesetMap[nodesetName]
+			if !exists || len(nodeset.Spec.Nodes) == 0 {
+				continue
+			}
+
+			var services []string
+			if len(deployment.Spec.ServicesOverride) != 0 {
+				services = deployment.Spec.ServicesOverride
+			} else {
+				services = nodeset.Spec.Services
+			}
+
+			for _, serviceName := range services {
+				svc, cached := serviceCache[serviceName]
+				if !cached {
+					foundService := &dataplanev1.OpenStackDataPlaneService{}
+					err := h.GetClient().Get(ctx, types.NamespacedName{
+						Name:      serviceName,
+						Namespace: namespace,
+					}, foundService)
+					if err != nil {
+						if k8s_errors.IsNotFound(err) {
+							continue
+						}
+						return false, err
+					}
+					svc = foundService
+					serviceCache[serviceName] = svc
+				}
+
+				if svc.Spec.EDPMServiceType == serviceType {
+					return true, nil
+				}
+			}
+		}
+	}
+
+	return false, nil
+}
+
 // DataplaneNodesetsDeployed returns true if all nodesets are deployed with the latest version
 func DataplaneNodesetsDeployed(version *corev1beta1.OpenStackVersion, dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList) bool {
 	for _, nodeset := range dataplaneNodesets.Items {