Skip to content

Commit 8214146

Browse files
stuggiclaude
andcommitted
Fix MinorUpdateOVNDataplane race when OVN image unchanged between versions
Commit 63fd705 removed the nodeset.IsReady() check from DataplaneNodesetsOVNControllerImagesMatch to fix the minor update workflow getting stuck when unrelated deployments were running. That check was too strict — it blocked when any deployment was in progress, even if the OVN update had already completed. However, the remaining pure image comparison is too loose. When the OVN controller image does not change between two OpenStack versions, the nodeset already has the matching image from the previous update. The OpenStackVersion controller then sets MinorUpdateOVNDataplane=True immediately, before the edpm-ovn-update deployment finishes. This causes the subsequent minor update steps (controlplane update, edpm services update) to proceed while the OVN dataplane deployment is still running — resulting in both dataplane deployments running concurrently. Fix this by adding a new check before the image comparison: IsDataplaneDeploymentRunningForContainerImage lists all in-progress OpenStackDataPlaneDeployment resources, resolves which services each deploys (from ServicesOverride or the nodeset's service list), and inspects each service's ContainerImageFields to determine if it manages the specified container image (e.g. "OvnControllerImage"). This approach is generic and does not rely on deployment or service naming conventions. It correctly identifies OVN-related deployments even when using custom services, by checking what container image fields the service declares. The MinorUpdateOVNDataplane condition now requires both: - No in-progress deployment managing OvnControllerImage - OVN controller image on all nodesets matches the target version Related: OSPRH-25860 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Martin Schuppert <mschuppert@redhat.com>
1 parent ab21714 commit 8214146

2 files changed

Lines changed: 92 additions & 1 deletion

File tree

internal/controller/core/openstackversion_controller.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,18 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
282282
// minor update for Dataplane OVN
283283
// Only check OVN when enabled to avoid hanging on a removed condition
284284
if controlPlane.Spec.Ovn.Enabled {
285-
if !openstack.DataplaneNodesetsOVNControllerImagesMatch(instance, dataplaneNodesets) {
285+
// First check if any deployment that manages OvnControllerImage is
286+
// still running. This prevents a race condition where the OVN image
287+
// hasn't changed between versions: the nodeset already has the
288+
// matching image from a previous update, so the image comparison
289+
// alone would return true before the OVN deployment finishes.
290+
ovnDeploymentRunning, err := openstack.IsDataplaneDeploymentRunningForContainerImage(
291+
ctx, versionHelper, instance.Namespace, dataplaneNodesets, "OvnControllerImage")
292+
if err != nil {
293+
return ctrl.Result{}, err
294+
}
295+
if ovnDeploymentRunning ||
296+
!openstack.DataplaneNodesetsOVNControllerImagesMatch(instance, dataplaneNodesets) {
286297
instance.Status.Conditions.Set(condition.FalseCondition(
287298
corev1beta1.OpenStackVersionMinorUpdateOVNDataplane,
288299
condition.RequestedReason,

internal/openstack/dataplane.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package openstack
22

33
import (
44
"context"
5+
"slices"
56

67
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
78
corev1beta1 "github.com/openstack-k8s-operators/openstack-operator/api/core/v1beta1"
89

910
dataplanev1 "github.com/openstack-k8s-operators/openstack-operator/api/dataplane/v1beta1"
11+
"k8s.io/apimachinery/pkg/types"
1012
"sigs.k8s.io/controller-runtime/pkg/client"
1113
)
1214

@@ -58,6 +60,84 @@ func DataplaneNodesetsOVNControllerImagesMatch(version *corev1beta1.OpenStackVer
5860
return true
5961
}
6062

63+
// IsDataplaneDeploymentRunningForContainerImage checks whether any in-progress
64+
// OpenStackDataPlaneDeployment is deploying a service that manages the given
65+
// containerImageField (e.g. "OvnControllerImage"). It resolves which services
66+
// each deployment runs (from ServicesOverride or the nodeset's service list)
67+
// and inspects the service's ContainerImageFields to determine if it manages
68+
// the specified container image.
69+
func IsDataplaneDeploymentRunningForContainerImage(
70+
ctx context.Context,
71+
h *helper.Helper,
72+
namespace string,
73+
dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList,
74+
containerImageField string,
75+
) (bool, error) {
76+
// List all deployments in the namespace
77+
deployments := &dataplanev1.OpenStackDataPlaneDeploymentList{}
78+
opts := []client.ListOption{
79+
client.InNamespace(namespace),
80+
}
81+
err := h.GetClient().List(ctx, deployments, opts...)
82+
if err != nil {
83+
return false, err
84+
}
85+
86+
// Build a map of nodeset name -> nodeset for quick lookup
87+
nodesetMap := make(map[string]*dataplanev1.OpenStackDataPlaneNodeSet, len(dataplaneNodesets.Items))
88+
for i := range dataplaneNodesets.Items {
89+
nodesetMap[dataplaneNodesets.Items[i].Name] = &dataplaneNodesets.Items[i]
90+
}
91+
92+
// Cache service lookups to avoid repeated API calls
93+
serviceCache := make(map[string]*dataplanev1.OpenStackDataPlaneService)
94+
95+
for _, deployment := range deployments.Items {
96+
// Skip completed deployments
97+
if deployment.Status.Deployed {
98+
continue
99+
}
100+
101+
// Determine which services this deployment runs for each of its nodesets
102+
for _, nodesetName := range deployment.Spec.NodeSets {
103+
nodeset, exists := nodesetMap[nodesetName]
104+
if !exists {
105+
continue
106+
}
107+
108+
var services []string
109+
if len(deployment.Spec.ServicesOverride) != 0 {
110+
services = deployment.Spec.ServicesOverride
111+
} else {
112+
services = nodeset.Spec.Services
113+
}
114+
115+
for _, serviceName := range services {
116+
svc, cached := serviceCache[serviceName]
117+
if !cached {
118+
foundService := &dataplanev1.OpenStackDataPlaneService{}
119+
err := h.GetClient().Get(ctx, types.NamespacedName{
120+
Name: serviceName,
121+
Namespace: namespace,
122+
}, foundService)
123+
if err != nil {
124+
// Service not found — skip it
125+
continue
126+
}
127+
svc = foundService
128+
serviceCache[serviceName] = svc
129+
}
130+
131+
if slices.Contains(svc.Spec.ContainerImageFields, containerImageField) {
132+
return true, nil
133+
}
134+
}
135+
}
136+
}
137+
138+
return false, nil
139+
}
140+
61141
// DataplaneNodesetsDeployed returns true if all nodesets are deployed with the latest version
62142
func DataplaneNodesetsDeployed(version *corev1beta1.OpenStackVersion, dataplaneNodesets *dataplanev1.OpenStackDataPlaneNodeSetList) bool {
63143
for _, nodeset := range dataplaneNodesets.Items {

0 commit comments

Comments
 (0)