Merge pull request #252 from openshift-cherrypick-robot/cherry-pick-2…

…50-to-18.0-fr1 [18.0-fr1] Ensure nodeSelector logic is consistent for all operators
openstack-k8s-operators · Nov 22, 2024 · b998769 · b998769
2 parents ba17591 + 758e4a5
commit b998769
Show file tree

Hide file tree

Showing 12 changed files with 69 additions and 49 deletions.
diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go
@@ -86,7 +86,7 @@ type DesignateServiceTemplateCore struct {
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this service. Setting here overrides
 	// any global NodeSelector settings within the Designate CR.
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// CustomServiceConfig - customize the service config using this parameter to change service defaults,

diff --git a/api/v1beta1/designate_types.go b/api/v1beta1/designate_types.go
@@ -152,7 +152,7 @@ type DesignateSpecBase struct {
 
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this service
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=false

diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/controllers/designate_controller.go b/controllers/designate_controller.go
@@ -1394,6 +1394,10 @@ func (r *DesignateReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, i
 		},
 	}
 
+	if instance.Spec.DesignateAPI.NodeSelector == nil {
+		instance.Spec.DesignateAPI.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
 		deployment.Spec = instance.Spec.DesignateAPI
 		// Add in transfers from umbrella Designate (this instance) spec
@@ -1405,9 +1409,7 @@ func (r *DesignateReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, i
 		deployment.Spec.ServiceAccount = instance.RbacResourceName()
 		deployment.Spec.TLS = instance.Spec.DesignateAPI.TLS
 		deployment.Spec.TransportURLSecret = instance.Status.TransportURLSecret
-		if len(deployment.Spec.NodeSelector) == 0 {
-			deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
+		deployment.Spec.NodeSelector = instance.Spec.DesignateAPI.NodeSelector
 
 		err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
 		if err != nil {
@@ -1428,6 +1430,10 @@ func (r *DesignateReconciler) centralDeploymentCreateOrUpdate(ctx context.Contex
 		},
 	}
 
+	if instance.Spec.DesignateCentral.NodeSelector == nil {
+		instance.Spec.DesignateCentral.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
 		deployment.Spec = instance.Spec.DesignateCentral
 		// Add in transfers from umbrella Designate CR (this instance) spec
@@ -1439,9 +1445,7 @@ func (r *DesignateReconciler) centralDeploymentCreateOrUpdate(ctx context.Contex
 		deployment.Spec.TransportURLSecret = instance.Status.TransportURLSecret
 		deployment.Spec.ServiceAccount = instance.RbacResourceName()
 		deployment.Spec.TLS = instance.Spec.DesignateAPI.TLS.Ca
-		if len(deployment.Spec.NodeSelector) == 0 {
-			deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
+		deployment.Spec.NodeSelector = instance.Spec.DesignateProducer.NodeSelector
 
 		err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
 		if err != nil {
@@ -1462,6 +1466,10 @@ func (r *DesignateReconciler) workerDeploymentCreateOrUpdate(ctx context.Context
 		},
 	}
 
+	if instance.Spec.DesignateWorker.NodeSelector == nil {
+		instance.Spec.DesignateWorker.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
 		deployment.Spec = instance.Spec.DesignateWorker
 		// Add in transfers from umbrella Designate CR (this instance) spec
@@ -1473,9 +1481,7 @@ func (r *DesignateReconciler) workerDeploymentCreateOrUpdate(ctx context.Context
 		deployment.Spec.TransportURLSecret = instance.Status.TransportURLSecret
 		deployment.Spec.ServiceAccount = instance.RbacResourceName()
 		deployment.Spec.TLS = instance.Spec.DesignateAPI.TLS.Ca
-		if len(deployment.Spec.NodeSelector) == 0 {
-			deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
+		deployment.Spec.NodeSelector = instance.Spec.DesignateWorker.NodeSelector
 
 		err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
 		if err != nil {
@@ -1496,6 +1502,10 @@ func (r *DesignateReconciler) mdnsDaemonSetCreateOrUpdate(ctx context.Context, i
 		},
 	}
 
+	if instance.Spec.DesignateMdns.NodeSelector == nil {
+		instance.Spec.DesignateMdns.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, daemonset, func() error {
 		daemonset.Spec = instance.Spec.DesignateMdns
 		// Add in transfers from umbrella Designate CR (this instance) spec
@@ -1507,9 +1517,7 @@ func (r *DesignateReconciler) mdnsDaemonSetCreateOrUpdate(ctx context.Context, i
 		daemonset.Spec.TransportURLSecret = instance.Status.TransportURLSecret
 		daemonset.Spec.ServiceAccount = instance.RbacResourceName()
 		daemonset.Spec.TLS = instance.Spec.DesignateAPI.TLS.Ca
-		if len(daemonset.Spec.NodeSelector) == 0 {
-			daemonset.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
+		daemonset.Spec.NodeSelector = instance.Spec.DesignateMdns.NodeSelector
 
 		err := controllerutil.SetControllerReference(instance, daemonset, r.Scheme)
 		if err != nil {
@@ -1530,6 +1538,10 @@ func (r *DesignateReconciler) producerDeploymentCreateOrUpdate(ctx context.Conte
 		},
 	}
 
+	if instance.Spec.DesignateProducer.NodeSelector == nil {
+		instance.Spec.DesignateProducer.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
 		deployment.Spec = instance.Spec.DesignateProducer
 		// Add in transfers from umbrella Designate CR (this instance) spec
@@ -1541,9 +1553,7 @@ func (r *DesignateReconciler) producerDeploymentCreateOrUpdate(ctx context.Conte
 		deployment.Spec.TransportURLSecret = instance.Status.TransportURLSecret
 		deployment.Spec.ServiceAccount = instance.RbacResourceName()
 		deployment.Spec.TLS = instance.Spec.DesignateAPI.TLS.Ca
-		if len(deployment.Spec.NodeSelector) == 0 {
-			deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
+		deployment.Spec.NodeSelector = instance.Spec.DesignateCentral.NodeSelector
 
 		err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
 		if err != nil {
@@ -1564,6 +1574,10 @@ func (r *DesignateReconciler) backendbind9StatefulSetCreateOrUpdate(ctx context.
 		},
 	}
 
+	if instance.Spec.DesignateBackendbind9.NodeSelector == nil {
+		instance.Spec.DesignateBackendbind9.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, statefulSet, func() error {
 		statefulSet.Spec = instance.Spec.DesignateBackendbind9
 		// Add in transfers from umbrella Designate CR (this instance) spec
@@ -1572,9 +1586,7 @@ func (r *DesignateReconciler) backendbind9StatefulSetCreateOrUpdate(ctx context.
 		statefulSet.Spec.Secret = instance.Spec.Secret
 		statefulSet.Spec.PasswordSelectors = instance.Spec.PasswordSelectors
 		statefulSet.Spec.ServiceAccount = instance.RbacResourceName()
-		if len(statefulSet.Spec.NodeSelector) == 0 {
-			statefulSet.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
+		statefulSet.Spec.NodeSelector = instance.Spec.DesignateBackendbind9.NodeSelector
 
 		err := controllerutil.SetControllerReference(instance, statefulSet, r.Scheme)
 		if err != nil {
@@ -1598,14 +1610,16 @@ func (r *DesignateReconciler) unboundDeploymentCreateOrUpdate(
 		},
 	}
 
+	if instance.Spec.DesignateUnbound.NodeSelector == nil {
+		instance.Spec.DesignateUnbound.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error {
 		deployment.Spec = instance.Spec.DesignateUnbound
 		// Add in transfers from umbrella Designate CR (this instance) spec
 		// TODO: Add logic to determine when to set/overwrite, etc
 		deployment.Spec.ServiceAccount = instance.RbacResourceName()
-		if len(deployment.Spec.NodeSelector) == 0 {
-			deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
+		deployment.Spec.NodeSelector = instance.Spec.DesignateUnbound.NodeSelector
 
 		err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
 		if err != nil {

diff --git a/pkg/designate/dbsync.go b/pkg/designate/dbsync.go
@@ -95,5 +95,9 @@ func DbSyncJob(
 	}
 	job.Spec.Template.Spec.InitContainers = InitContainer(initContainerDetails)
 
+	if instance.Spec.NodeSelector != nil {
+		job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
+	}
+
 	return job
 }
diff --git a/pkg/designateapi/deployment.go b/pkg/designateapi/deployment.go
@@ -145,7 +145,6 @@ func Deployment(
 							LivenessProbe:  livenessProbe,
 						},
 					},
-					NodeSelector: instance.Spec.NodeSelector,
 				},
 			},
 		},
@@ -161,8 +160,8 @@ func Deployment(
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	initContainerDetails := designate.APIDetails{

diff --git a/pkg/designatebackendbind9/deployment.go b/pkg/designatebackendbind9/deployment.go
@@ -111,7 +111,6 @@ func StatefulSet(
 							ReadinessProbe: readinessProbe,
 						},
 					},
-					NodeSelector: instance.Spec.NodeSelector,
 				},
 			},
 		},
@@ -157,8 +156,8 @@ func StatefulSet(
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		statefulSet.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		statefulSet.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	// TODO: bind's init container doesn't need most of this stuff. It doesn't use rabbitmq, redis or access the

diff --git a/pkg/designatecentral/deployment.go b/pkg/designatecentral/deployment.go
@@ -122,7 +122,6 @@ func Deployment(
 							LivenessProbe: livenessProbe,
 						},
 					},
-					NodeSelector: instance.Spec.NodeSelector,
 				},
 			},
 		},
@@ -138,8 +137,8 @@ func Deployment(
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	initContainerDetails := designate.APIDetails{

diff --git a/pkg/designatemdns/daemonset.go b/pkg/designatemdns/daemonset.go
@@ -114,7 +114,6 @@ func DaemonSet(
 							LivenessProbe:  livenessProbe,
 						},
 					},
-					NodeSelector: instance.Spec.NodeSelector,
 				},
 			},
 		},
@@ -130,8 +129,8 @@ func DaemonSet(
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		daemonset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		daemonset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	initContainerDetails := designate.APIDetails{

diff --git a/pkg/designateproducer/deployment.go b/pkg/designateproducer/deployment.go
@@ -118,7 +118,6 @@ func Deployment(
 							LivenessProbe: livenessProbe,
 						},
 					},
-					NodeSelector: instance.Spec.NodeSelector,
 				},
 			},
 		},
@@ -134,8 +133,8 @@ func Deployment(
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	initContainerDetails := designate.APIDetails{

diff --git a/pkg/designateunbound/deployment.go b/pkg/designateunbound/deployment.go
@@ -146,8 +146,8 @@ func Deployment(instance *designatev1beta1.DesignateUnbound,
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	return deployment

diff --git a/pkg/designateworker/deployment.go b/pkg/designateworker/deployment.go
@@ -114,7 +114,6 @@ func Deployment(
 							LivenessProbe: livenessProbe,
 						},
 					},
-					NodeSelector: instance.Spec.NodeSelector,
 				},
 			},
 		},
@@ -130,8 +129,8 @@ func Deployment(
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	initContainerDetails := designate.APIDetails{