Fix MinorUpdateOVNDataplane stuck when reconciles race

When the OVN image is unchanged between versions, the version
controller uses a condition-based state machine to track whether an
OVN dataplane deployment was observed running. When multiple reconciles
race at the same timestamp, the condition state from the previous
reconcile may not have persisted yet, causing the state machine to
reset and loop indefinitely on "Waiting for OVN Dataplane deployment
to be created".

Add IsDataplaneDeploymentCompletedForServiceType() to check for a
completed deployment with the target version for a given service type.
Use it as a fallback when the saved condition state doesn't show a
previously observed deployment — if a completed deployment exists,
proceed instead of waiting forever.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Martin Schuppert <mschuppert@redhat.com>

Author: stuggi

internal/controller/core/openstackversion_controller.go

@@ -26,10 +26,13 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/go-logr/logr"
@@ -82,6 +85,8 @@ func (r *OpenStackVersionReconciler) GetLogger(ctx context.Context) logr.Logger
 // +kubebuilder:rbac:groups=core.openstack.org,resources=openstackversions/finalizers,verbs=update;patch
 // +kubebuilder:rbac:groups=core.openstack.org,resources=openstackcontrolplanes,verbs=get;list;watch
 // +kubebuilder:rbac:groups=dataplane.openstack.org,resources=openstackdataplanenodesets,verbs=get;list;watch
+// +kubebuilder:rbac:groups=dataplane.openstack.org,resources=openstackdataplanedeployments,verbs=get;list;watch
+// +kubebuilder:rbac:groups=dataplane.openstack.org,resources=openstackdataplaneservices,verbs=get;list;watch
 
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
@@ -344,16 +349,24 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 				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
+					// The saved condition doesn't show we observed a running
+					// deployment (may be due to reconcile race). Check if a
+					// completed deployment already exists for the target version.
+					ovnDeploymentCompleted, err := openstack.IsDataplaneDeploymentCompletedForServiceType(
+						ctx, versionHelper, instance.Namespace, dataplaneNodesets, "ovn", instance.Spec.TargetVersion)
+					if err != nil {
+						return ctrl.Result{}, err
+					}
+					if !ovnDeploymentCompleted {
+						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
+					}
+					Log.Info("OVN Dataplane deployment completed (found completed deployment for target version)")
 				}
 				// Previously saw a running OVN deployment (condition was
 				// False/RequestedReason), now no OVN deployment is running
@@ -526,9 +539,38 @@ func (r *OpenStackVersionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		return nil
 	})
 
+	// Only reconcile when a deployment's Deployed status changes,
+	// not on every status update during deployment execution.
+	// The dataplane controller sets Deployed and DeployedVersion
+	// in the same status update, so checking Deployed alone is sufficient.
+	deploymentStatusPredicate := predicate.Funcs{
+		CreateFunc: func(_ event.CreateEvent) bool {
+			return true
+		},
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			oldDepl, ok := e.ObjectOld.(*dataplanev1.OpenStackDataPlaneDeployment)
+			if !ok {
+				return false
+			}
+			newDepl, ok := e.ObjectNew.(*dataplanev1.OpenStackDataPlaneDeployment)
+			if !ok {
+				return false
+			}
+			return oldDepl.Status.Deployed != newDepl.Status.Deployed
+		},
+		DeleteFunc: func(_ event.DeleteEvent) bool {
+			return false
+		},
+		GenericFunc: func(_ event.GenericEvent) bool {
+			return false
+		},
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		Watches(&corev1beta1.OpenStackControlPlane{}, versionFunc).
 		Watches(&dataplanev1.OpenStackDataPlaneNodeSet{}, versionFunc).
+		Watches(&dataplanev1.OpenStackDataPlaneDeployment{}, versionFunc,
+			builder.WithPredicates(deploymentStatusPredicate)).
 		For(&corev1beta1.OpenStackVersion{}).
 		Complete(r)
 }

internal/openstack/dataplane.go

@@ -60,17 +60,18 @@ 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(
+// deploymentFilter is a predicate that selects which deployments to consider.
+type deploymentFilter func(*dataplanev1.OpenStackDataPlaneDeployment) bool
+
+// findDataplaneDeploymentForServiceType checks if any deployment matching the
+// filter includes the given service type for at least one nodeset with nodes.
+func findDataplaneDeploymentForServiceType(
 	ctx context.Context,
 	h *helper.Helper,
 	namespace string,
 	dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList,
 	serviceType string,
+	filter deploymentFilter,
 ) (bool, error) {
 	// List all deployments in the namespace
 	deployments := &dataplanev1.OpenStackDataPlaneDeploymentList{}
@@ -92,8 +93,7 @@ func IsDataplaneDeploymentRunningForServiceType(
 	serviceCache := make(map[string]*dataplanev1.OpenStackDataPlaneService)
 
 	for _, deployment := range deployments.Items {
-		// Skip completed deployments
-		if deployment.Status.Deployed {
+		if !filter(&deployment) {
 			continue
 		}
 
@@ -139,6 +139,39 @@ func IsDataplaneDeploymentRunningForServiceType(
 	return false, nil
 }
 
+// IsDataplaneDeploymentRunningForServiceType checks if a non-completed
+// deployment exists that includes the given service type.
+func IsDataplaneDeploymentRunningForServiceType(
+	ctx context.Context,
+	h *helper.Helper,
+	namespace string,
+	dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList,
+	serviceType string,
+) (bool, error) {
+	return findDataplaneDeploymentForServiceType(ctx, h, namespace, dataplaneNodesets, serviceType,
+		func(d *dataplanev1.OpenStackDataPlaneDeployment) bool {
+			return !d.Status.Deployed
+		})
+}
+
+// IsDataplaneDeploymentCompletedForServiceType checks if a completed deployment
+// exists for the given service type with the target version. This is used during
+// minor updates when the OVN image is unchanged between versions — the image
+// match alone is not sufficient, so we check for a completed deployment instead.
+func IsDataplaneDeploymentCompletedForServiceType(
+	ctx context.Context,
+	h *helper.Helper,
+	namespace string,
+	dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList,
+	serviceType string,
+	targetVersion string,
+) (bool, error) {
+	return findDataplaneDeploymentForServiceType(ctx, h, namespace, dataplaneNodesets, serviceType,
+		func(d *dataplanev1.OpenStackDataPlaneDeployment) bool {
+			return d.Status.Deployed && d.Status.DeployedVersion == targetVersion
+		})
+}
+
 // 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 {