From 82475a3897f9f22393e95265d9deb4f9d6be15e2 Mon Sep 17 00:00:00 2001 From: Blake Date: Thu, 16 Oct 2025 14:10:46 -0400 Subject: [PATCH 1/5] add NodeSelector support to config Signed-off-by: Blake --- .../api/v1alpha1/featurestore_types.go | 1 + .../api/v1alpha1/zz_generated.deepcopy.go | 11 ++ .../crd/bases/feast.dev_featurestores.yaml | 40 +++++ infra/feast-operator/dist/install.yaml | 40 +++++ infra/feast-operator/docs/api/markdown/ref.md | 5 + .../internal/controller/services/services.go | 35 +++++ .../controller/services/services_test.go | 141 ++++++++++++++++++ 7 files changed, 273 insertions(+) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 191505cc6a6..9250309b1cc 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -541,6 +541,7 @@ type OptionalCtrConfigs struct { EnvFrom *[]corev1.EnvFromSource `json:"envFrom,omitempty"` ImagePullPolicy *corev1.PullPolicy `json:"imagePullPolicy,omitempty"` Resources *corev1.ResourceRequirements `json:"resources,omitempty"` + NodeSelector *map[string]string `json:"nodeSelector,omitempty"` } // AuthzConfig defines the authorization settings for the deployed Feast services. diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index 1a893c82cf8..7ea04929b3d 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -768,6 +768,17 @@ func (in *OptionalCtrConfigs) DeepCopyInto(out *OptionalCtrConfigs) { *out = new(v1.ResourceRequirements) (*in).DeepCopyInto(*out) } + if in.NodeSelector != nil { + in, out := &in.NodeSelector, &out.NodeSelector + *out = new(map[string]string) + if **in != nil { + in, out := *in, *out + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OptionalCtrConfigs. diff --git a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml index 8debe3639f9..c964d46c27d 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -259,6 +259,10 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -1025,6 +1029,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -1480,6 +1488,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -1946,6 +1958,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -2441,6 +2457,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -4216,6 +4236,10 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -4994,6 +5018,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -5457,6 +5485,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -5935,6 +5967,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -6440,6 +6476,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 9efa8044a29..eb99da89cba 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -267,6 +267,10 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -1033,6 +1037,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -1488,6 +1496,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -1954,6 +1966,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -2449,6 +2465,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -4224,6 +4244,10 @@ spec: description: PullPolicy describes a policy for if/when to pull a container image type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -5002,6 +5026,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -5465,6 +5493,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -5943,6 +5975,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. @@ -6448,6 +6484,10 @@ spec: - error - critical type: string + nodeSelector: + additionalProperties: + type: string + type: object resources: description: ResourceRequirements describes the compute resource requirements. diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index 68978a08cf0..fac7ebfa784 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -46,6 +46,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | +| `nodeSelector` _map[string]string_ | | #### CronJobContainerConfigs @@ -64,6 +65,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | +| `nodeSelector` _map[string]string_ | | | `commands` _string array_ | Array of commands to be executed (in order) against a Feature Store deployment. Defaults to "feast apply" & "feast materialize-incremental $(date -u +'%Y-%m-%dT%H:%M:%S')" | @@ -566,6 +568,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | +| `nodeSelector` _map[string]string_ | | #### PvcConfig @@ -688,6 +691,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | +| `nodeSelector` _map[string]string_ | | | `tls` _[TlsConfigs](#tlsconfigs)_ | | | `logLevel` _string_ | LogLevel sets the logging level for the server Allowed values: "debug", "info", "warning", "error", "critical". | @@ -750,6 +754,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | +| `nodeSelector` _map[string]string_ | | | `tls` _[TlsConfigs](#tlsconfigs)_ | | | `logLevel` _string_ | LogLevel sets the logging level for the server Allowed values: "debug", "info", "warning", "error", "critical". | diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 5b70d6e1911..0ec8126d13a 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -412,6 +412,7 @@ func (feast *FeastServices) setPod(podSpec *corev1.PodSpec) error { feast.mountPvcConfigs(podSpec) feast.mountEmptyDirVolumes(podSpec) feast.mountUserDefinedVolumes(podSpec) + feast.applyNodeSelector(podSpec) return nil } @@ -793,6 +794,40 @@ func (feast *FeastServices) getLogLevelForType(feastType FeastServiceType) *stri return nil } +func (feast *FeastServices) getNodeSelectorForType(feastType FeastServiceType) *map[string]string { + if serviceConfigs := feast.getServerConfigs(feastType); serviceConfigs != nil { + return serviceConfigs.ContainerConfigs.OptionalCtrConfigs.NodeSelector + } + return nil +} + +func (feast *FeastServices) applyNodeSelector(podSpec *corev1.PodSpec) { + var selectedNodeSelector map[string]string + var selectedService FeastServiceType + + // Check all service types for node selector configuration + allServiceTypes := append(feastServerTypes, UIFeastType) + for _, feastType := range allServiceTypes { + if selector := feast.getNodeSelectorForType(feastType); selector != nil && len(*selector) > 0 { + selectedNodeSelector = *selector + selectedService = feastType + } + } + + // If no service has node selector configured, we're done + if len(selectedNodeSelector) == 0 { + return + } + + // Apply the simple node selector to the pod spec + podSpec.NodeSelector = selectedNodeSelector + + // Log which service's node selector was applied for debugging + log.FromContext(feast.Handler.Context).V(1).Info("Applied node selector", + "serviceType", selectedService, + "selector", selectedNodeSelector) +} + // GetObjectMeta returns the feast k8s object metadata with type func (feast *FeastServices) GetObjectMeta() metav1.ObjectMeta { return metav1.ObjectMeta{Name: GetFeastName(feast.Handler.FeatureStore), Namespace: feast.Handler.FeatureStore.Namespace} diff --git a/infra/feast-operator/internal/controller/services/services_test.go b/infra/feast-operator/internal/controller/services/services_test.go index 0c43aff5954..4d9e0260d2f 100644 --- a/infra/feast-operator/internal/controller/services/services_test.go +++ b/infra/feast-operator/internal/controller/services/services_test.go @@ -203,4 +203,145 @@ var _ = Describe("Registry Service", func() { Expect(ports[1].Name).To(Equal(string(RegistryFeastType) + "-rest")) }) }) + + Describe("NodeSelector Configuration", func() { + It("should apply NodeSelector to pod spec when configured", func() { + // Set NodeSelector for registry service + nodeSelector := map[string]string{ + "kubernetes.io/os": "linux", + "node-type": "compute", + } + featureStore.Spec.Services.Registry.Local.Server.ContainerConfigs.OptionalCtrConfigs.NodeSelector = &nodeSelector + Expect(k8sClient.Update(ctx, featureStore)).To(Succeed()) + Expect(feast.ApplyDefaults()).To(Succeed()) + applySpecToStatus(featureStore) + feast.refreshFeatureStore(ctx, typeNamespacedName) + + // Create deployment and verify NodeSelector is applied + deployment := feast.initFeastDeploy() + Expect(deployment).NotTo(BeNil()) + Expect(feast.setDeployment(deployment)).To(Succeed()) + + // Verify NodeSelector is applied to pod spec + expectedNodeSelector := map[string]string{ + "kubernetes.io/os": "linux", + "node-type": "compute", + } + Expect(deployment.Spec.Template.Spec.NodeSelector).To(Equal(expectedNodeSelector)) + }) + + It("should apply NodeSelector from last service when multiple services have selectors", func() { + // Set NodeSelector for registry service + registryNodeSelector := map[string]string{ + "kubernetes.io/os": "linux", + "node-type": "compute", + } + featureStore.Spec.Services.Registry.Local.Server.ContainerConfigs.OptionalCtrConfigs.NodeSelector = ®istryNodeSelector + + // Set NodeSelector for online store service + onlineNodeSelector := map[string]string{ + "node-type": "online", + "zone": "us-west-1a", + } + featureStore.Spec.Services.OnlineStore = &feastdevv1alpha1.OnlineStore{ + Server: &feastdevv1alpha1.ServerConfigs{ + ContainerConfigs: feastdevv1alpha1.ContainerConfigs{ + DefaultCtrConfigs: feastdevv1alpha1.DefaultCtrConfigs{ + Image: ptr("test-image"), + }, + OptionalCtrConfigs: feastdevv1alpha1.OptionalCtrConfigs{ + NodeSelector: &onlineNodeSelector, + }, + }, + }, + } + + Expect(k8sClient.Update(ctx, featureStore)).To(Succeed()) + Expect(feast.ApplyDefaults()).To(Succeed()) + applySpecToStatus(featureStore) + feast.refreshFeatureStore(ctx, typeNamespacedName) + + // Create deployment and verify merged NodeSelector is applied + deployment := feast.initFeastDeploy() + Expect(deployment).NotTo(BeNil()) + Expect(feast.setDeployment(deployment)).To(Succeed()) + + // Verify NodeSelector is applied with the last service's selector (Online service wins) + expectedNodeSelector := map[string]string{ + "node-type": "online", + "zone": "us-west-1a", + } + Expect(deployment.Spec.Template.Spec.NodeSelector).To(Equal(expectedNodeSelector)) + }) + + It("should apply UI service NodeSelector when UI has highest precedence", func() { + // Set NodeSelector for online service + onlineNodeSelector := map[string]string{ + "node-type": "online", + } + featureStore.Spec.Services.OnlineStore = &feastdevv1alpha1.OnlineStore{ + Server: &feastdevv1alpha1.ServerConfigs{ + ContainerConfigs: feastdevv1alpha1.ContainerConfigs{ + DefaultCtrConfigs: feastdevv1alpha1.DefaultCtrConfigs{ + Image: ptr("test-image"), + }, + OptionalCtrConfigs: feastdevv1alpha1.OptionalCtrConfigs{ + NodeSelector: &onlineNodeSelector, + }, + }, + }, + } + + // Set NodeSelector for UI service (should win) + uiNodeSelector := map[string]string{ + "node-type": "ui", + "zone": "us-east-1", + } + featureStore.Spec.Services.UI = &feastdevv1alpha1.ServerConfigs{ + ContainerConfigs: feastdevv1alpha1.ContainerConfigs{ + DefaultCtrConfigs: feastdevv1alpha1.DefaultCtrConfigs{ + Image: ptr("test-image"), + }, + OptionalCtrConfigs: feastdevv1alpha1.OptionalCtrConfigs{ + NodeSelector: &uiNodeSelector, + }, + }, + } + + Expect(k8sClient.Update(ctx, featureStore)).To(Succeed()) + Expect(feast.ApplyDefaults()).To(Succeed()) + applySpecToStatus(featureStore) + feast.refreshFeatureStore(ctx, typeNamespacedName) + + // Create deployment and verify UI service selector is applied + deployment := feast.initFeastDeploy() + Expect(deployment).NotTo(BeNil()) + Expect(feast.setDeployment(deployment)).To(Succeed()) + + // Verify NodeSelector is applied with UI service's selector (UI wins) + expectedNodeSelector := map[string]string{ + "node-type": "ui", + "zone": "us-east-1", + } + Expect(deployment.Spec.Template.Spec.NodeSelector).To(Equal(expectedNodeSelector)) + }) + + It("should handle empty NodeSelector gracefully", func() { + // Set empty NodeSelector + emptyNodeSelector := map[string]string{} + featureStore.Spec.Services.Registry.Local.Server.ContainerConfigs.OptionalCtrConfigs.NodeSelector = &emptyNodeSelector + Expect(k8sClient.Update(ctx, featureStore)).To(Succeed()) + Expect(feast.ApplyDefaults()).To(Succeed()) + applySpecToStatus(featureStore) + feast.refreshFeatureStore(ctx, typeNamespacedName) + + // Create deployment and verify no NodeSelector is applied (empty selector) + deployment := feast.initFeastDeploy() + Expect(deployment).NotTo(BeNil()) + Expect(feast.setDeployment(deployment)).To(Succeed()) + + // Verify no NodeSelector is applied (empty selector) + Expect(deployment.Spec.Template.Spec.NodeSelector).To(BeEmpty()) + }) + }) }) From 621ffeab329d7fcb342889d5dfc6f61bf72947aa Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 20 Oct 2025 10:30:57 -0400 Subject: [PATCH 2/5] code review updates Signed-off-by: Blake --- infra/feast-operator/docs/api/markdown/ref.md | 12 ++--- .../internal/controller/services/services.go | 38 ++++++++++---- .../controller/services/services_test.go | 51 +++++++++++++++++-- 3 files changed, 80 insertions(+), 21 deletions(-) diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index fac7ebfa784..f5bff35951b 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -46,7 +46,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | | +| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | #### CronJobContainerConfigs @@ -65,7 +65,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | | +| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | | `commands` _string array_ | Array of commands to be executed (in order) against a Feature Store deployment. Defaults to "feast apply" & "feast materialize-incremental $(date -u +'%Y-%m-%dT%H:%M:%S')" | @@ -568,7 +568,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | | +| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | #### PvcConfig @@ -657,7 +657,7 @@ _Appears in:_ | --- | --- | | `path` _string_ | | | `pvc` _[PvcConfig](#pvcconfig)_ | | -| `s3_additional_kwargs` _map[string]string_ | | +| `s3_additional_kwargs` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, the last service's configuration takes precedence. | #### RegistryPersistence @@ -691,7 +691,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | | +| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | | `tls` _[TlsConfigs](#tlsconfigs)_ | | | `logLevel` _string_ | LogLevel sets the logging level for the server Allowed values: "debug", "info", "warning", "error", "critical". | @@ -754,7 +754,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | | +| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | | `tls` _[TlsConfigs](#tlsconfigs)_ | | | `logLevel` _string_ | LogLevel sets the logging level for the server Allowed values: "debug", "info", "warning", "error", "critical". | diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 0ec8126d13a..3b14f9c49db 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -802,30 +802,46 @@ func (feast *FeastServices) getNodeSelectorForType(feastType FeastServiceType) * } func (feast *FeastServices) applyNodeSelector(podSpec *corev1.PodSpec) { - var selectedNodeSelector map[string]string - var selectedService FeastServiceType + // Merge node selectors from all services + mergedNodeSelector := make(map[string]string) // Check all service types for node selector configuration allServiceTypes := append(feastServerTypes, UIFeastType) for _, feastType := range allServiceTypes { if selector := feast.getNodeSelectorForType(feastType); selector != nil && len(*selector) > 0 { - selectedNodeSelector = *selector - selectedService = feastType + for k, v := range *selector { + mergedNodeSelector[k] = v + } } } // If no service has node selector configured, we're done - if len(selectedNodeSelector) == 0 { + if len(mergedNodeSelector) == 0 { return } - // Apply the simple node selector to the pod spec - podSpec.NodeSelector = selectedNodeSelector + // Merge with any existing node selectors (from ops team or other sources) + // This preserves pre-existing selectors while adding operator requirements + finalNodeSelector := feast.mergeNodeSelectors(podSpec.NodeSelector, mergedNodeSelector) + podSpec.NodeSelector = finalNodeSelector +} + +// mergeNodeSelectors merges existing and operator node selectors +// Existing selectors are preserved, operator selectors can override existing keys +func (feast *FeastServices) mergeNodeSelectors(existing, operator map[string]string) map[string]string { + merged := make(map[string]string) + + // Start with existing selectors (from ops team or other sources) + for k, v := range existing { + merged[k] = v + } + + // Add/override with operator selectors + for k, v := range operator { + merged[k] = v + } - // Log which service's node selector was applied for debugging - log.FromContext(feast.Handler.Context).V(1).Info("Applied node selector", - "serviceType", selectedService, - "selector", selectedNodeSelector) + return merged } // GetObjectMeta returns the feast k8s object metadata with type diff --git a/infra/feast-operator/internal/controller/services/services_test.go b/infra/feast-operator/internal/controller/services/services_test.go index 4d9e0260d2f..9b695e0d106 100644 --- a/infra/feast-operator/internal/controller/services/services_test.go +++ b/infra/feast-operator/internal/controller/services/services_test.go @@ -230,7 +230,7 @@ var _ = Describe("Registry Service", func() { Expect(deployment.Spec.Template.Spec.NodeSelector).To(Equal(expectedNodeSelector)) }) - It("should apply NodeSelector from last service when multiple services have selectors", func() { + It("should merge NodeSelectors from multiple services", func() { // Set NodeSelector for registry service registryNodeSelector := map[string]string{ "kubernetes.io/os": "linux", @@ -266,10 +266,53 @@ var _ = Describe("Registry Service", func() { Expect(deployment).NotTo(BeNil()) Expect(feast.setDeployment(deployment)).To(Succeed()) - // Verify NodeSelector is applied with the last service's selector (Online service wins) + // Verify NodeSelector merges all service selectors (online overrides registry for node-type) expectedNodeSelector := map[string]string{ - "node-type": "online", - "zone": "us-west-1a", + "kubernetes.io/os": "linux", + "node-type": "online", + "zone": "us-west-1a", + } + Expect(deployment.Spec.Template.Spec.NodeSelector).To(Equal(expectedNodeSelector)) + }) + + It("should merge operator NodeSelector with existing selectors (ops team scenario)", func() { + // Simulate existing node selector from ops team + existingNodeSelector := map[string]string{ + "team": "ml", + "environment": "prod", + } + + // Set NodeSelector for UI service + uiNodeSelector := map[string]string{ + "node-type": "ui", + } + featureStore.Spec.Services.UI = &feastdevv1alpha1.ServerConfigs{ + ContainerConfigs: feastdevv1alpha1.ContainerConfigs{ + DefaultCtrConfigs: feastdevv1alpha1.DefaultCtrConfigs{ + Image: ptr("test-image"), + }, + OptionalCtrConfigs: feastdevv1alpha1.OptionalCtrConfigs{ + NodeSelector: &uiNodeSelector, + }, + }, + } + + Expect(k8sClient.Update(ctx, featureStore)).To(Succeed()) + Expect(feast.ApplyDefaults()).To(Succeed()) + applySpecToStatus(featureStore) + feast.refreshFeatureStore(ctx, typeNamespacedName) + + // Create deployment and simulate existing node selector + deployment := feast.initFeastDeploy() + deployment.Spec.Template.Spec.NodeSelector = existingNodeSelector + Expect(deployment).NotTo(BeNil()) + Expect(feast.setDeployment(deployment)).To(Succeed()) + + // Verify NodeSelector merges existing and operator selectors + expectedNodeSelector := map[string]string{ + "team": "ml", + "environment": "prod", + "node-type": "ui", } Expect(deployment.Spec.Template.Spec.NodeSelector).To(Equal(expectedNodeSelector)) }) From 6a079506e17c6c38e1726918d61c40584e7ecc6e Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 20 Oct 2025 10:42:32 -0400 Subject: [PATCH 3/5] contrived example was actually wrong Signed-off-by: Blake --- .../controller/services/services_test.go | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services_test.go b/infra/feast-operator/internal/controller/services/services_test.go index 9b695e0d106..14509fe9933 100644 --- a/infra/feast-operator/internal/controller/services/services_test.go +++ b/infra/feast-operator/internal/controller/services/services_test.go @@ -275,13 +275,7 @@ var _ = Describe("Registry Service", func() { Expect(deployment.Spec.Template.Spec.NodeSelector).To(Equal(expectedNodeSelector)) }) - It("should merge operator NodeSelector with existing selectors (ops team scenario)", func() { - // Simulate existing node selector from ops team - existingNodeSelector := map[string]string{ - "team": "ml", - "environment": "prod", - } - + It("should merge operator NodeSelector with existing selectors (mutating webhook scenario)", func() { // Set NodeSelector for UI service uiNodeSelector := map[string]string{ "node-type": "ui", @@ -302,12 +296,23 @@ var _ = Describe("Registry Service", func() { applySpecToStatus(featureStore) feast.refreshFeatureStore(ctx, typeNamespacedName) - // Create deployment and simulate existing node selector + // Create deployment first deployment := feast.initFeastDeploy() - deployment.Spec.Template.Spec.NodeSelector = existingNodeSelector Expect(deployment).NotTo(BeNil()) Expect(feast.setDeployment(deployment)).To(Succeed()) + // Simulate a mutating webhook or admission controller adding node selectors + // This would happen after the operator creates the pod spec but before scheduling + existingNodeSelector := map[string]string{ + "team": "ml", + "environment": "prod", + } + deployment.Spec.Template.Spec.NodeSelector = existingNodeSelector + + // Apply the node selector logic again to test merging + // This simulates the operator reconciling and re-applying node selectors + feast.applyNodeSelector(&deployment.Spec.Template.Spec) + // Verify NodeSelector merges existing and operator selectors expectedNodeSelector := map[string]string{ "team": "ml", From 4ab65ff66c9f4513f56c582c98d92c792cf0c719 Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 20 Oct 2025 10:50:30 -0400 Subject: [PATCH 4/5] removing incorrect docs Signed-off-by: Blake --- infra/feast-operator/docs/api/markdown/ref.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index f5bff35951b..5c4fe5022c3 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -657,7 +657,7 @@ _Appears in:_ | --- | --- | | `path` _string_ | | | `pvc` _[PvcConfig](#pvcconfig)_ | | -| `s3_additional_kwargs` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, the last service's configuration takes precedence. | +| `s3_additional_kwargs` _map[string]string_ | | #### RegistryPersistence From 4fb5c8d961d2d8fc21972c7cf4427fcac9b6f56c Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 20 Oct 2025 11:06:19 -0400 Subject: [PATCH 5/5] docs are not working with description Signed-off-by: Blake --- infra/feast-operator/docs/api/markdown/ref.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index 5c4fe5022c3..fac7ebfa784 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -46,7 +46,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | +| `nodeSelector` _map[string]string_ | | #### CronJobContainerConfigs @@ -65,7 +65,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | +| `nodeSelector` _map[string]string_ | | | `commands` _string array_ | Array of commands to be executed (in order) against a Feature Store deployment. Defaults to "feast apply" & "feast materialize-incremental $(date -u +'%Y-%m-%dT%H:%M:%S')" | @@ -568,7 +568,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | +| `nodeSelector` _map[string]string_ | | #### PvcConfig @@ -691,7 +691,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | +| `nodeSelector` _map[string]string_ | | | `tls` _[TlsConfigs](#tlsconfigs)_ | | | `logLevel` _string_ | LogLevel sets the logging level for the server Allowed values: "debug", "info", "warning", "error", "critical". | @@ -754,7 +754,7 @@ _Appears in:_ | `envFrom` _[EnvFromSource](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envfromsource-v1-core)_ | | | `imagePullPolicy` _[PullPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#pullpolicy-v1-core)_ | | | `resources` _[ResourceRequirements](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#resourcerequirements-v1-core)_ | | -| `nodeSelector` _map[string]string_ | NodeSelector constrains pod scheduling to nodes with matching labels. When multiple services define nodeSelector, all selectors are merged together. Operator node selectors are merged with any existing selectors (e.g., from ops team), with operator values taking precedence for conflicting keys. | +| `nodeSelector` _map[string]string_ | | | `tls` _[TlsConfigs](#tlsconfigs)_ | | | `logLevel` _string_ | LogLevel sets the logging level for the server Allowed values: "debug", "info", "warning", "error", "critical". |