Merge pull request #497 from olliewalsh/node_selectors

Ensure nodeSelector logic is consistent for all operators
openstack-k8s-operators · Nov 20, 2024 · 63d25e2 · 63d25e2
2 parents 5b4dcbe + ba329f5
commit 63d25e2
Show file tree

Hide file tree

Showing 13 changed files with 299 additions and 35 deletions.
diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go
@@ -32,7 +32,7 @@ type IronicServiceTemplate struct {
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this service. Setting here overrides
 	// any global NodeSelector settings within the Ironic CR
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// Resources - Compute Resources required by this service (Limits/Requests).

diff --git a/api/v1beta1/ironic_types.go b/api/v1beta1/ironic_types.go
@@ -142,7 +142,7 @@ type IronicSpecCore struct {
 	// NodeSelector to target subset of worker nodes running this service. Setting
 	// NodeSelector here acts as a default value and can be overridden by service
 	// specific NodeSelector Settings.
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// Storage class to host data. This is passed to IronicConductors unless
 	// storageClass is explicitly set for the conductor.

diff --git a/api/v1beta1/ironicinspector_types.go b/api/v1beta1/ironicinspector_types.go
@@ -62,7 +62,7 @@ type IronicInspectorTemplate struct {
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this service. Setting here overrides
 	// any global NodeSelector settings within the Ironic CR
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=true

diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go
diff --git a/controllers/ironic_controller.go b/controllers/ironic_controller.go
@@ -778,6 +778,11 @@ func (r *IronicReconciler) conductorDeploymentCreateOrUpdate(
 		KeystoneEndpoints:       *keystoneEndpoints,
 		TLS:                     instance.Spec.IronicAPI.TLS.Ca,
 	}
+
+	if IronicConductorSpec.NodeSelector == nil {
+		IronicConductorSpec.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	deployment := &ironicv1.IronicConductor{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -787,9 +792,6 @@ func (r *IronicReconciler) conductorDeploymentCreateOrUpdate(
 
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), r.Client, deployment, func() error {
 		deployment.Spec = IronicConductorSpec
-		if len(deployment.Spec.NodeSelector) == 0 {
-			deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
 		if deployment.Spec.StorageClass == "" {
 			deployment.Spec.StorageClass = instance.Spec.StorageClass
 		}
@@ -822,6 +824,10 @@ func (r *IronicReconciler) apiDeploymentCreateOrUpdate(
 		KeystoneEndpoints:  *keystoneEndpoints,
 	}
 
+	if IronicAPISpec.NodeSelector == nil {
+		IronicAPISpec.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	deployment := &ironicv1.IronicAPI{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      fmt.Sprintf("%s-api", instance.Name),
@@ -831,9 +837,6 @@ func (r *IronicReconciler) apiDeploymentCreateOrUpdate(
 
 	op, err := controllerutil.CreateOrUpdate(context.TODO(), r.Client, deployment, func() error {
 		deployment.Spec = IronicAPISpec
-		if len(deployment.Spec.NodeSelector) == 0 {
-			deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-		}
 		err := controllerutil.SetControllerReference(instance, deployment, r.Scheme)
 		if err != nil {
 			return err
@@ -977,6 +980,11 @@ func (r *IronicReconciler) inspectorDeploymentCreateOrUpdate(
 		RabbitMqClusterName:     instance.Spec.RabbitMqClusterName,
 		Secret:                  instance.Spec.Secret,
 	}
+
+	if IronicInspectorSpec.NodeSelector == nil {
+		IronicInspectorSpec.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	deployment := &ironicv1.IronicInspector{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      fmt.Sprintf("%s-inspector", instance.Name),
@@ -987,9 +995,6 @@ func (r *IronicReconciler) inspectorDeploymentCreateOrUpdate(
 	op, err := controllerutil.CreateOrUpdate(
 		context.TODO(), r.Client, deployment, func() error {
 			deployment.Spec = IronicInspectorSpec
-			if len(deployment.Spec.NodeSelector) == 0 {
-				deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-			}
 			err := controllerutil.SetControllerReference(
 				instance, deployment, r.Scheme)
 			if err != nil {
@@ -1046,6 +1051,11 @@ func (r *IronicReconciler) ironicNeutronAgentDeploymentCreateOrUpdate(
 		ServiceUser:                instance.Spec.ServiceUser,
 		TLS:                        instance.Spec.IronicAPI.TLS.Ca,
 	}
+
+	if IronicNeutronAgentSpec.NodeSelector == nil {
+		IronicNeutronAgentSpec.NodeSelector = instance.Spec.NodeSelector
+	}
+
 	deployment := &ironicv1.IronicNeutronAgent{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      fmt.Sprintf("%s-ironic-neutron-agent", instance.Name),
@@ -1056,9 +1066,6 @@ func (r *IronicReconciler) ironicNeutronAgentDeploymentCreateOrUpdate(
 	op, err := controllerutil.CreateOrUpdate(
 		context.TODO(), r.Client, deployment, func() error {
 			deployment.Spec = IronicNeutronAgentSpec
-			if len(deployment.Spec.NodeSelector) == 0 {
-				deployment.Spec.NodeSelector = instance.Spec.NodeSelector
-			}
 			err := controllerutil.SetControllerReference(
 				instance, deployment, r.Scheme)
 			if err != nil {

diff --git a/pkg/ironic/dbsync.go b/pkg/ironic/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/ironicapi/deployment.go b/pkg/ironicapi/deployment.go
@@ -187,8 +187,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 := ironic.APIDetails{

diff --git a/pkg/ironicconductor/statefulset.go b/pkg/ironicconductor/statefulset.go
@@ -316,8 +316,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
 	}
 
 	// init.sh needs to detect and set ProvisionNetworkIP

diff --git a/pkg/ironicinspector/dbsync.go b/pkg/ironicinspector/dbsync.go
@@ -96,5 +96,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/ironicinspector/statefulset.go b/pkg/ironicinspector/statefulset.go
@@ -327,8 +327,8 @@ func StatefulSet(
 		[]string{ironic.ServiceName},
 		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
 	}
 
 	// init.sh needs to detect and set InspectionNetworkIP

diff --git a/pkg/ironicneutronagent/deployment.go b/pkg/ironicneutronagent/deployment.go
@@ -128,8 +128,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 := APIDetails{

diff --git a/tests/functional/base_test.go b/tests/functional/base_test.go
@@ -234,13 +234,16 @@ func GetIronicNames(
 		},
 		InternalCertSecretName: types.NamespacedName{
 			Namespace: ironicName.Namespace,
-			Name:      "internal-tls-certs"},
+			Name:      "internal-tls-certs",
+		},
 		PublicCertSecretName: types.NamespacedName{
 			Namespace: ironicName.Namespace,
-			Name:      "public-tls-certs"},
+			Name:      "public-tls-certs",
+		},
 		CaBundleSecretName: types.NamespacedName{
 			Namespace: ironicName.Namespace,
-			Name:      "combined-ca-bundle"},
+			Name:      "combined-ca-bundle",
+		},
 	}
 }