Fix NodeSelector nil-vs-pointer reconcile loop in all service reconcilers

lmiccini · claude · lmiccini · commit 3cf0bfe496b4 · 2026-04-29T17:50:51.000+02:00
When the OSCP has no nodeSelector defined, instance.Spec.NodeSelector is
a nil map. Taking its address (&amp;instance.Spec.NodeSelector) creates a
non-nil pointer to a nil map, which is structurally different from a nil
pointer. controllerutil.CreateOrPatch detects this as a diff via
reflect.DeepEqual and sends an update patch on every reconcile, bumping
the sub-CR's Generation. The ObservedGeneration readiness check then
reports the sub-CR as "in progress", triggering another OSCP reconcile
and creating an infinite loop (~1 update/second).

Guard all 25 NodeSelector inheritance assignments with
len(instance.Spec.NodeSelector) &gt; 0 so the assignment is skipped when
the OSCP nodeSelector is nil or empty, avoiding the spurious diff.

Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/internal/openstack/barbican.go b/internal/openstack/barbican.go
@@ -149,7 +149,7 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr
 		instance.Spec.Barbican.Template.BarbicanAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)
 	}
 
-	if instance.Spec.Barbican.Template.NodeSelector == nil {
+	if instance.Spec.Barbican.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Barbican.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/cinder.go b/internal/openstack/cinder.go
@@ -172,7 +172,7 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl
 		instance.Spec.Cinder.Template.CinderAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)
 	}
 
-	if instance.Spec.Cinder.Template.NodeSelector == nil {
+	if instance.Spec.Cinder.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Cinder.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/designate.go b/internal/openstack/designate.go
@@ -156,7 +156,7 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont
 		instance.Spec.Designate.Template.DesignateAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)
 	}
 
-	if instance.Spec.Designate.Template.NodeSelector == nil {
+	if instance.Spec.Designate.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Designate.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/dnsmasq.go b/internal/openstack/dnsmasq.go
@@ -40,7 +40,7 @@ func ReconcileDNSMasqs(ctx context.Context, instance *corev1beta1.OpenStackContr
 		instance.Spec.DNS.Template = &networkv1.DNSMasqSpecCore{}
 	}
 
-	if instance.Spec.DNS.Template.NodeSelector == nil {
+	if instance.Spec.DNS.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.DNS.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/galera.go b/internal/openstack/galera.go
@@ -266,7 +266,7 @@ func reconcileGalera(
 		return galeraReady, galera, nil
 	}
 
-	if spec.NodeSelector == nil {
+	if spec.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		spec.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/glance.go b/internal/openstack/glance.go
@@ -94,7 +94,7 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl
 		instance.Spec.Glance.Template.NotificationBusInstance = nil
 	}
 
-	if instance.Spec.Glance.Template.NodeSelector == nil {
+	if instance.Spec.Glance.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Glance.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/heat.go b/internal/openstack/heat.go
@@ -54,7 +54,7 @@ func ReconcileHeat(ctx context.Context, instance *corev1beta1.OpenStackControlPl
 	// Note: Migration from rabbitMqClusterName to messagingBus.cluster is handled by the webhook
 	// via annotation-based triggers. No direct spec mutation here to avoid GitOps conflicts.
 
-	if instance.Spec.Heat.Template.NodeSelector == nil {
+	if instance.Spec.Heat.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Heat.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/horizon.go b/internal/openstack/horizon.go
@@ -50,7 +50,7 @@ func ReconcileHorizon(ctx context.Context, instance *corev1beta1.OpenStackContro
 		instance.Spec.Horizon.Template = &horizonv1.HorizonSpecCore{}
 	}
 
-	if instance.Spec.Horizon.Template.NodeSelector == nil {
+	if instance.Spec.Horizon.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Horizon.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/ironic.go b/internal/openstack/ironic.go
@@ -58,7 +58,7 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl
 	// via annotation-based triggers. No direct spec mutation here to avoid GitOps conflicts.
 	// This applies to both Ironic main template and IronicNeutronAgent.
 
-	if instance.Spec.Ironic.Template.NodeSelector == nil {
+	if instance.Spec.Ironic.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Ironic.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/keystone.go b/internal/openstack/keystone.go
@@ -106,7 +106,7 @@ func ReconcileKeystoneAPI(ctx context.Context, instance *corev1beta1.OpenStackCo
 		instance.Spec.Keystone.Template.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)
 	}
 
-	if instance.Spec.Keystone.Template.NodeSelector == nil {
+	if instance.Spec.Keystone.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Keystone.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/manila.go b/internal/openstack/manila.go
@@ -152,7 +152,7 @@ func ReconcileManila(ctx context.Context, instance *corev1beta1.OpenStackControl
 		instance.Spec.Manila.Template.ManilaAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)
 	}
 
-	if instance.Spec.Manila.Template.NodeSelector == nil {
+	if instance.Spec.Manila.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Manila.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/memcached.go b/internal/openstack/memcached.go
@@ -273,7 +273,7 @@ func reconcileMemcached(
 		}
 	}
 
-	if spec.NodeSelector == nil {
+	if spec.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		spec.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/neutron.go b/internal/openstack/neutron.go
@@ -189,7 +189,7 @@ func ReconcileNeutron(ctx context.Context, instance *corev1beta1.OpenStackContro
 		instance.Spec.Neutron.Template.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)
 	}
 
-	if instance.Spec.Neutron.Template.NodeSelector == nil {
+	if instance.Spec.Neutron.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Neutron.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/nova.go b/internal/openstack/nova.go
@@ -75,7 +75,7 @@ func ReconcileNova(ctx context.Context, instance *corev1beta1.OpenStackControlPl
 	// Note: Migration from apiMessageBusInstance and cellMessageBusInstance to messagingBus.cluster
 	// is handled by the webhook via annotation-based triggers. No direct spec mutation here to avoid GitOps conflicts.
 
-	if instance.Spec.Nova.Template.NodeSelector == nil {
+	if instance.Spec.Nova.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Nova.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/octavia.go b/internal/openstack/octavia.go
@@ -73,7 +73,7 @@ func ReconcileOctavia(ctx context.Context, instance *corev1beta1.OpenStackContro
 	// Note: Migration from rabbitMqClusterName to messagingBus.cluster is handled by the webhook
 	// via annotation-based triggers. No direct spec mutation here to avoid GitOps conflicts.
 
-	if instance.Spec.Octavia.Template.NodeSelector == nil {
+	if instance.Spec.Octavia.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Octavia.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/openstackclient.go b/internal/openstack/openstackclient.go
@@ -47,7 +47,7 @@ func ReconcileOpenStackClient(ctx context.Context, instance *corev1.OpenStackCon
 		return ctrl.Result{}, nil
 	}
 
-	if instance.Spec.OpenStackClient.Template.NodeSelector == nil {
+	if instance.Spec.OpenStackClient.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.OpenStackClient.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/ovn.go b/internal/openstack/ovn.go
@@ -208,7 +208,7 @@ func ReconcileOVNDbClusters(ctx context.Context, instance *corev1beta1.OpenStack
 			dbcluster.MetricsTLS.CaBundleSecretName = instance.Status.TLS.CaBundleSecretName
 		}
 
-		if dbcluster.NodeSelector == nil {
+		if dbcluster.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 			dbcluster.NodeSelector = &instance.Spec.NodeSelector
 		}
 
@@ -349,7 +349,7 @@ func ReconcileOVNNorthd(ctx context.Context, instance *corev1beta1.OpenStackCont
 		ovnNorthdSpec.MetricsTLS.CaBundleSecretName = instance.Status.TLS.CaBundleSecretName
 	}
 
-	if ovnNorthdSpec.NodeSelector == nil {
+	if ovnNorthdSpec.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		ovnNorthdSpec.NodeSelector = &instance.Spec.NodeSelector
 	}
 
@@ -493,7 +493,7 @@ func ReconcileOVNController(ctx context.Context, instance *corev1beta1.OpenStack
 		ovnControllerSpec.MetricsTLS.CaBundleSecretName = instance.Status.TLS.CaBundleSecretName
 	}
 
-	if ovnControllerSpec.NodeSelector == nil {
+	if ovnControllerSpec.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		ovnControllerSpec.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/placement.go b/internal/openstack/placement.go
@@ -45,7 +45,7 @@ func ReconcilePlacementAPI(ctx context.Context, instance *corev1beta1.OpenStackC
 		instance.Spec.Placement.Template = &placementv1.PlacementAPISpecCore{}
 	}
 
-	if instance.Spec.Placement.Template.NodeSelector == nil {
+	if instance.Spec.Placement.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Placement.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/rabbitmq.go b/internal/openstack/rabbitmq.go
@@ -245,7 +245,7 @@ func reconcileRabbitMQ(
 		tlsCert = certSecret.Name
 	}
 
-	if spec.NodeSelector == nil {
+	if spec.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		spec.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/redis.go b/internal/openstack/redis.go
@@ -255,7 +255,7 @@ func reconcileRedis(
 		tlsCert = certSecret.Name
 	}
 
-	if spec.NodeSelector == nil {
+	if spec.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		spec.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/swift.go b/internal/openstack/swift.go
@@ -66,7 +66,7 @@ func ReconcileSwift(ctx context.Context, instance *corev1beta1.OpenStackControlP
 		instance.Spec.Swift.Template.SwiftProxy.RabbitMqClusterName = ""
 	}
 
-	if instance.Spec.Swift.Template.NodeSelector == nil {
+	if instance.Spec.Swift.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Swift.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/telemetry.go b/internal/openstack/telemetry.go
@@ -71,7 +71,7 @@ func ReconcileTelemetry(ctx context.Context, instance *corev1beta1.OpenStackCont
 	// notificationsBus.cluster (Aodh, Ceilometer) is handled by the webhook via annotation-based triggers.
 	// No direct spec mutation here to avoid GitOps conflicts.
 
-	if instance.Spec.Telemetry.Template.NodeSelector == nil {
+	if instance.Spec.Telemetry.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Telemetry.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 
diff --git a/internal/openstack/watcher.go b/internal/openstack/watcher.go
@@ -164,7 +164,7 @@ func ReconcileWatcher(ctx context.Context, instance *corev1beta1.OpenStackContro
 		instance.Spec.Watcher.Template.APIServiceTemplate.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)
 	}
 
-	if instance.Spec.Watcher.Template.NodeSelector == nil {
+	if instance.Spec.Watcher.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {
 		instance.Spec.Watcher.Template.NodeSelector = &instance.Spec.NodeSelector
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -149,7 +149,7 @@ func ReconcileBarbican(ctx context.Context, instance *corev1beta1.OpenStackContr`
`149`	`149`	`instance.Spec.Barbican.Template.BarbicanAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)`
`150`	`150`	`}`
`151`	`151`
`152`		`- if instance.Spec.Barbican.Template.NodeSelector == nil {`
	`152`	`+ if instance.Spec.Barbican.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`153`	`153`	`instance.Spec.Barbican.Template.NodeSelector = &instance.Spec.NodeSelector`
`154`	`154`	`}`
`155`	`155`
Original file line number	Diff line number	Diff line change
`@@ -172,7 +172,7 @@ func ReconcileCinder(ctx context.Context, instance *corev1beta1.OpenStackControl`
`172`	`172`	`instance.Spec.Cinder.Template.CinderAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)`
`173`	`173`	`}`
`174`	`174`
`175`		`- if instance.Spec.Cinder.Template.NodeSelector == nil {`
	`175`	`+ if instance.Spec.Cinder.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`176`	`176`	`instance.Spec.Cinder.Template.NodeSelector = &instance.Spec.NodeSelector`
`177`	`177`	`}`
`178`	`178`
Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ func ReconcileDesignate(ctx context.Context, instance *corev1beta1.OpenStackCont`
`156`	`156`	`instance.Spec.Designate.Template.DesignateAPI.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)`
`157`	`157`	`}`
`158`	`158`
`159`		`- if instance.Spec.Designate.Template.NodeSelector == nil {`
	`159`	`+ if instance.Spec.Designate.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`160`	`160`	`instance.Spec.Designate.Template.NodeSelector = &instance.Spec.NodeSelector`
`161`	`161`	`}`
`162`	`162`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ func ReconcileDNSMasqs(ctx context.Context, instance *corev1beta1.OpenStackContr`
`40`	`40`	`instance.Spec.DNS.Template = &networkv1.DNSMasqSpecCore{}`
`41`	`41`	`}`
`42`	`42`
`43`		`- if instance.Spec.DNS.Template.NodeSelector == nil {`
	`43`	`+ if instance.Spec.DNS.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`44`	`44`	`instance.Spec.DNS.Template.NodeSelector = &instance.Spec.NodeSelector`
`45`	`45`	`}`
`46`	`46`
Original file line number	Diff line number	Diff line change
`@@ -266,7 +266,7 @@ func reconcileGalera(`
`266`	`266`	`return galeraReady, galera, nil`
`267`	`267`	`}`
`268`	`268`
`269`		`- if spec.NodeSelector == nil {`
	`269`	`+ if spec.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`270`	`270`	`spec.NodeSelector = &instance.Spec.NodeSelector`
`271`	`271`	`}`
`272`	`272`
Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,7 @@ func ReconcileGlance(ctx context.Context, instance *corev1beta1.OpenStackControl`
`94`	`94`	`instance.Spec.Glance.Template.NotificationBusInstance = nil`
`95`	`95`	`}`
`96`	`96`
`97`		`- if instance.Spec.Glance.Template.NodeSelector == nil {`
	`97`	`+ if instance.Spec.Glance.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`98`	`98`	`instance.Spec.Glance.Template.NodeSelector = &instance.Spec.NodeSelector`
`99`	`99`	`}`
`100`	`100`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ func ReconcileHeat(ctx context.Context, instance *corev1beta1.OpenStackControlPl`
`54`	`54`	`// Note: Migration from rabbitMqClusterName to messagingBus.cluster is handled by the webhook`
`55`	`55`	`// via annotation-based triggers. No direct spec mutation here to avoid GitOps conflicts.`
`56`	`56`
`57`		`- if instance.Spec.Heat.Template.NodeSelector == nil {`
	`57`	`+ if instance.Spec.Heat.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`58`	`58`	`instance.Spec.Heat.Template.NodeSelector = &instance.Spec.NodeSelector`
`59`	`59`	`}`
`60`	`60`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ func ReconcileHorizon(ctx context.Context, instance *corev1beta1.OpenStackContro`
`50`	`50`	`instance.Spec.Horizon.Template = &horizonv1.HorizonSpecCore{}`
`51`	`51`	`}`
`52`	`52`
`53`		`- if instance.Spec.Horizon.Template.NodeSelector == nil {`
	`53`	`+ if instance.Spec.Horizon.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`54`	`54`	`instance.Spec.Horizon.Template.NodeSelector = &instance.Spec.NodeSelector`
`55`	`55`	`}`
`56`	`56`
Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ func ReconcileIronic(ctx context.Context, instance *corev1beta1.OpenStackControl`
`58`	`58`	`// via annotation-based triggers. No direct spec mutation here to avoid GitOps conflicts.`
`59`	`59`	`// This applies to both Ironic main template and IronicNeutronAgent.`
`60`	`60`
`61`		`- if instance.Spec.Ironic.Template.NodeSelector == nil {`
	`61`	`+ if instance.Spec.Ironic.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`62`	`62`	`instance.Spec.Ironic.Template.NodeSelector = &instance.Spec.NodeSelector`
`63`	`63`	`}`
`64`	`64`
Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ func ReconcileKeystoneAPI(ctx context.Context, instance *corev1beta1.OpenStackCo`
`106`	`106`	`instance.Spec.Keystone.Template.TLS.API.Internal.SecretName = endpointDetails.GetEndptCertSecret(service.EndpointInternal)`
`107`	`107`	`}`
`108`	`108`
`109`		`- if instance.Spec.Keystone.Template.NodeSelector == nil {`
	`109`	`+ if instance.Spec.Keystone.Template.NodeSelector == nil && len(instance.Spec.NodeSelector) > 0 {`
`110`	`110`	`instance.Spec.Keystone.Template.NodeSelector = &instance.Spec.NodeSelector`
`111`	`111`	`}`
`112`	`112`