From e67fc0dd0574b89eb2a37b5eff0226c023db31f2 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Fri, 16 May 2025 22:32:57 +0530 Subject: [PATCH 01/18] feat: Allow to set registry server rest/grpc mode in operator Signed-off-by: ntkathole --- .../api/v1alpha1/featurestore_types.go | 12 +++++-- .../api/v1alpha1/zz_generated.deepcopy.go | 18 +++++++++- .../crd/bases/feast.dev_featurestores.yaml | 11 ++++++ infra/feast-operator/dist/install.yaml | 11 ++++++ infra/feast-operator/docs/api/markdown/ref.md | 34 +++++++++++++++++-- .../featurestore_controller_loglevel_test.go | 10 ++++-- ...featurestore_controller_test_utils_test.go | 4 ++- .../featurestore_controller_tls_test.go | 8 +++-- .../controller/services/repo_config_test.go | 2 +- .../internal/controller/services/services.go | 13 +++++-- .../internal/controller/services/tls_test.go | 28 +++++++++------ 11 files changed, 126 insertions(+), 25 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 508f3d4bf7a..637099cdfb1 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -392,8 +392,8 @@ var ValidOnlineStoreDBStorePersistenceTypes = []string{ // LocalRegistryConfig configures the registry service type LocalRegistryConfig struct { // Creates a registry server container - Server *ServerConfigs `json:"server,omitempty"` - Persistence *RegistryPersistence `json:"persistence,omitempty"` + Server *RegistryServerConfigs `json:"server,omitempty"` + Persistence *RegistryPersistence `json:"persistence,omitempty"` } // RegistryPersistence configures the persistence settings for the registry service @@ -502,6 +502,14 @@ type ServerConfigs struct { VolumeMounts []corev1.VolumeMount `json:"volumeMounts,omitempty"` } +// RegistryServerConfigs creates a registry server for the feast service, with specified container configurations. +type RegistryServerConfigs struct { + ServerConfigs `json:",inline"` + // RestAPIEnabled determines if the registry should serve using the REST API instead of gRPC. + // +kubebuilder:default=true + RestAPIEnabled bool `json:"restAPIEnabled,omitempty"` +} + // CronJobContainerConfigs k8s container settings for the CronJob type CronJobContainerConfigs struct { ContainerConfigs `json:",inline"` diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index 6c0bc66d9df..81794a0217f 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -525,7 +525,7 @@ func (in *LocalRegistryConfig) DeepCopyInto(out *LocalRegistryConfig) { *out = *in if in.Server != nil { in, out := &in.Server, &out.Server - *out = new(ServerConfigs) + *out = new(RegistryServerConfigs) (*in).DeepCopyInto(*out) } if in.Persistence != nil { @@ -928,6 +928,22 @@ func (in *RegistryPersistence) DeepCopy() *RegistryPersistence { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RegistryServerConfigs) DeepCopyInto(out *RegistryServerConfigs) { + *out = *in + in.ServerConfigs.DeepCopyInto(&out.ServerConfigs) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RegistryServerConfigs. +func (in *RegistryServerConfigs) DeepCopy() *RegistryServerConfigs { + if in == nil { + return nil + } + out := new(RegistryServerConfigs) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RemoteRegistryConfig) DeepCopyInto(out *RemoteRegistryConfig) { *out = *in 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 f1928bfdeae..133f5f84363 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -1987,6 +1987,11 @@ spec: of compute resources required. type: object type: object + restAPIEnabled: + default: true + description: RestAPIEnabled determines if the registry + should serve using the REST API instead of gRPC. + type: boolean tls: description: TlsConfigs configures server TLS for a feast service. @@ -5965,6 +5970,12 @@ spec: amount of compute resources required. type: object type: object + restAPIEnabled: + default: true + description: RestAPIEnabled determines if the + registry should serve using the REST API instead + of gRPC. + type: boolean tls: description: TlsConfigs configures server TLS for a feast service. diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index c54ea39e5ab..7429b98f0bf 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -1995,6 +1995,11 @@ spec: of compute resources required. type: object type: object + restAPIEnabled: + default: true + description: RestAPIEnabled determines if the registry + should serve using the REST API instead of gRPC. + type: boolean tls: description: TlsConfigs configures server TLS for a feast service. @@ -5973,6 +5978,12 @@ spec: amount of compute resources required. type: object type: object + restAPIEnabled: + default: true + description: RestAPIEnabled determines if the + registry should serve using the REST API instead + of gRPC. + type: boolean tls: description: TlsConfigs configures server TLS for a feast service. diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index 2e90f0c7b91..a8d69260833 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -36,6 +36,7 @@ ContainerConfigs k8s container settings for the server _Appears in:_ - [CronJobContainerConfigs](#cronjobcontainerconfigs) +- [RegistryServerConfigs](#registryserverconfigs) - [ServerConfigs](#serverconfigs) | Field | Description | @@ -76,6 +77,7 @@ DefaultCtrConfigs k8s container settings that are applied by default _Appears in:_ - [ContainerConfigs](#containerconfigs) - [CronJobContainerConfigs](#cronjobcontainerconfigs) +- [RegistryServerConfigs](#registryserverconfigs) - [ServerConfigs](#serverconfigs) | Field | Description | @@ -405,7 +407,7 @@ _Appears in:_ | Field | Description | | --- | --- | -| `server` _[ServerConfigs](#serverconfigs)_ | Creates a registry server container | +| `server` _[RegistryServerConfigs](#registryserverconfigs)_ | Creates a registry server container | | `persistence` _[RegistryPersistence](#registrypersistence)_ | | @@ -555,6 +557,7 @@ OptionalCtrConfigs k8s container settings that are optional _Appears in:_ - [ContainerConfigs](#containerconfigs) - [CronJobContainerConfigs](#cronjobcontainerconfigs) +- [RegistryServerConfigs](#registryserverconfigs) - [ServerConfigs](#serverconfigs) | Field | Description | @@ -669,6 +672,32 @@ _Appears in:_ | `store` _[RegistryDBStorePersistence](#registrydbstorepersistence)_ | | +#### RegistryServerConfigs + + + +RegistryServerConfigs creates a registry server for the feast service, with specified container configurations. + +_Appears in:_ +- [LocalRegistryConfig](#localregistryconfig) + +| Field | Description | +| --- | --- | +| `image` _string_ | | +| `env` _[EnvVar](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#envvar-v1-core)_ | | +| `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)_ | | +| `tls` _[TlsConfigs](#tlsconfigs)_ | | +| `logLevel` _string_ | LogLevel sets the logging level for the server +Allowed values: "debug", "info", "warning", "error", "critical". | +| `volumeMounts` _[VolumeMount](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#volumemount-v1-core) array_ | VolumeMounts defines the list of volumes that should be mounted into the feast container. +This allows attaching persistent storage, config files, secrets, or other resources +required by the Feast components. Ensure that each volume mount has a corresponding +volume definition in the Volumes field. | +| `restAPIEnabled` _boolean_ | RestAPIEnabled determines if the registry should serve using the REST API instead of gRPC. | + + #### RemoteRegistryConfig @@ -709,9 +738,9 @@ ServerConfigs creates a server for the feast service, with specified container c _Appears in:_ - [FeatureStoreServices](#featurestoreservices) -- [LocalRegistryConfig](#localregistryconfig) - [OfflineStore](#offlinestore) - [OnlineStore](#onlinestore) +- [RegistryServerConfigs](#registryserverconfigs) | Field | Description | | --- | --- | @@ -753,6 +782,7 @@ _Appears in:_ TlsConfigs configures server TLS for a feast service. in an openshift cluster, this is configured by default using service serving certificates. _Appears in:_ +- [RegistryServerConfigs](#registryserverconfigs) - [ServerConfigs](#serverconfigs) | Field | Description | diff --git a/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go b/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go index 8b3832d46c2..a02d0894c8c 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go @@ -61,8 +61,10 @@ var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() { Services: &feastdevv1alpha1.FeatureStoreServices{ Registry: &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{ - LogLevel: strPtr("error"), + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{ + LogLevel: strPtr("error"), + }, }, }, }, @@ -196,7 +198,9 @@ var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() { resource.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ Registry: &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{}, + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{}, + }, }, }, OfflineStore: &feastdevv1alpha1.OfflineStore{}, diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test_utils_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test_utils_test.go index dcf684c7733..63df6d46f46 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test_utils_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test_utils_test.go @@ -130,7 +130,9 @@ func createFeatureStoreResource(resourceName string, image string, pullPolicy co }, Registry: &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{}, + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{}, + }, }, }, UI: &feastdevv1alpha1.ServerConfigs{ diff --git a/infra/feast-operator/internal/controller/featurestore_controller_tls_test.go b/infra/feast-operator/internal/controller/featurestore_controller_tls_test.go index e5b27f8ec56..b7aca319f05 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_tls_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_tls_test.go @@ -82,8 +82,10 @@ var _ = Describe("FeatureStore Controller - Feast service TLS", func() { }, Registry: &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{ - TLS: tlsConfigs, + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{ + TLS: tlsConfigs, + }, }, }, }, @@ -463,7 +465,7 @@ var _ = Describe("Test mountCustomCABundle functionality", func() { Spec: feastdevv1alpha1.FeatureStoreSpec{ FeastProject: feastProject, Services: &feastdevv1alpha1.FeatureStoreServices{ - Registry: &feastdevv1alpha1.Registry{Local: &feastdevv1alpha1.LocalRegistryConfig{Server: &feastdevv1alpha1.ServerConfigs{}}}, + Registry: &feastdevv1alpha1.Registry{Local: &feastdevv1alpha1.LocalRegistryConfig{Server: &feastdevv1alpha1.RegistryServerConfigs{ServerConfigs: feastdevv1alpha1.ServerConfigs{}}}}, OnlineStore: &feastdevv1alpha1.OnlineStore{Server: &feastdevv1alpha1.ServerConfigs{}}, OfflineStore: &feastdevv1alpha1.OfflineStore{Server: &feastdevv1alpha1.ServerConfigs{}}, UI: &feastdevv1alpha1.ServerConfigs{}, diff --git a/infra/feast-operator/internal/controller/services/repo_config_test.go b/infra/feast-operator/internal/controller/services/repo_config_test.go index a346ac72e80..bfe09c33e93 100644 --- a/infra/feast-operator/internal/controller/services/repo_config_test.go +++ b/infra/feast-operator/internal/controller/services/repo_config_test.go @@ -374,7 +374,7 @@ func minimalFeatureStoreWithAllServers() *feastdevv1alpha1.FeatureStore { }, Registry: &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{}, + Server: &feastdevv1alpha1.RegistryServerConfigs{}, }, }, UI: &feastdevv1alpha1.ServerConfigs{}, diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 768b4df74b1..384a3b90eb2 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -499,6 +499,13 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st deploySettings := FeastServiceConstants[feastType] targetPort := deploySettings.TargetHttpPort tls := feast.getTlsConfigs(feastType) + + if feastType == RegistryFeastType { + registry := feast.Handler.FeatureStore.Spec.Services.Registry + if registry != nil && registry.Local != nil && registry.Local.Server != nil && registry.Local.Server.RestAPIEnabled { + deploySettings.Args = append(deploySettings.Args, "--rest-api") + } + } if tls.IsTLS() { targetPort = deploySettings.TargetHttpsPort feastTlsPath := GetTlsPath(feastType) @@ -645,8 +652,10 @@ func (feast *FeastServices) getServerConfigs(feastType FeastServiceType) *feastd return appliedServices.OnlineStore.Server } case RegistryFeastType: - if feast.isLocalRegistry() { - return appliedServices.Registry.Local.Server + if feast.isLocalRegistry() && appliedServices.Registry != nil && + appliedServices.Registry.Local != nil && + appliedServices.Registry.Local.Server != nil { + return &appliedServices.Registry.Local.Server.ServerConfigs } case UIFeastType: return appliedServices.UI diff --git a/infra/feast-operator/internal/controller/services/tls_test.go b/infra/feast-operator/internal/controller/services/tls_test.go index caf694a2173..3bd34964619 100644 --- a/infra/feast-operator/internal/controller/services/tls_test.go +++ b/infra/feast-operator/internal/controller/services/tls_test.go @@ -56,7 +56,9 @@ var _ = Describe("TLS Config", func() { feast.Handler.FeatureStore.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ Registry: &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{}, + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{}, + }, }, }, } @@ -85,7 +87,9 @@ var _ = Describe("TLS Config", func() { feast.Handler.FeatureStore.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ Registry: &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{}, + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{}, + }, }, }, } @@ -190,11 +194,13 @@ var _ = Describe("TLS Config", func() { }, Registry: &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{ - TLS: &feastdevv1alpha1.TlsConfigs{ - SecretRef: &corev1.LocalObjectReference{}, - SecretKeyNames: feastdevv1alpha1.SecretKeyNames{ - TlsCrt: "test.crt", + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{ + TLS: &feastdevv1alpha1.TlsConfigs{ + SecretRef: &corev1.LocalObjectReference{}, + SecretKeyNames: feastdevv1alpha1.SecretKeyNames{ + TlsCrt: "test.crt", + }, }, }, }, @@ -244,9 +250,11 @@ var _ = Describe("TLS Config", func() { } feast.Handler.FeatureStore.Spec.Services.Registry = &feastdevv1alpha1.Registry{ Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.ServerConfigs{ - TLS: &feastdevv1alpha1.TlsConfigs{ - Disable: &disable, + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{ + TLS: &feastdevv1alpha1.TlsConfigs{ + Disable: &disable, + }, }, }, }, From ef5d3c4ac9d38709e5aefc63f133d2572c6caad6 Mon Sep 17 00:00:00 2001 From: Nikhil Kathole Date: Wed, 21 May 2025 10:12:44 +0530 Subject: [PATCH 02/18] Apply suggestions from code review Use isRegistryServer Co-authored-by: Tommy Hughes IV Signed-off-by: ntkathole --- .../internal/controller/services/services.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 384a3b90eb2..6060d88f105 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -500,9 +500,9 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st targetPort := deploySettings.TargetHttpPort tls := feast.getTlsConfigs(feastType) - if feastType == RegistryFeastType { - registry := feast.Handler.FeatureStore.Spec.Services.Registry - if registry != nil && registry.Local != nil && registry.Local.Server != nil && registry.Local.Server.RestAPIEnabled { + if feastType == RegistryFeastType && feast.isRegistryServer() { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + if registry.Local.Server.RestAPIEnabled { deploySettings.Args = append(deploySettings.Args, "--rest-api") } } @@ -652,9 +652,7 @@ func (feast *FeastServices) getServerConfigs(feastType FeastServiceType) *feastd return appliedServices.OnlineStore.Server } case RegistryFeastType: - if feast.isLocalRegistry() && appliedServices.Registry != nil && - appliedServices.Registry.Local != nil && - appliedServices.Registry.Local.Server != nil { + if feast.isRegistryServer() { return &appliedServices.Registry.Local.Server.ServerConfigs } case UIFeastType: From 7bcb68875ca441779d1995c8ab1946417a630098 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Wed, 21 May 2025 10:37:41 +0530 Subject: [PATCH 03/18] chore: rename RestAPIEnabled to RestAPI Signed-off-by: ntkathole --- .../api/v1alpha1/featurestore_types.go | 5 ++--- .../config/crd/bases/feast.dev_featurestores.yaml | 15 ++++++--------- infra/feast-operator/dist/install.yaml | 15 ++++++--------- infra/feast-operator/docs/api/markdown/ref.md | 2 +- .../internal/controller/services/services.go | 2 +- 5 files changed, 16 insertions(+), 23 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 637099cdfb1..cf639dd829e 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -505,9 +505,8 @@ type ServerConfigs struct { // RegistryServerConfigs creates a registry server for the feast service, with specified container configurations. type RegistryServerConfigs struct { ServerConfigs `json:",inline"` - // RestAPIEnabled determines if the registry should serve using the REST API instead of gRPC. - // +kubebuilder:default=true - RestAPIEnabled bool `json:"restAPIEnabled,omitempty"` + // RestAPI determines if the registry should serve using the REST API instead of gRPC. + RestAPI bool `json:"restAPI,omitempty"` } // CronJobContainerConfigs k8s container settings for the CronJob 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 133f5f84363..849428554e3 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -1987,10 +1987,9 @@ spec: of compute resources required. type: object type: object - restAPIEnabled: - default: true - description: RestAPIEnabled determines if the registry - should serve using the REST API instead of gRPC. + restAPI: + description: RestAPI determines if the registry should + serve using the REST API instead of gRPC. type: boolean tls: description: TlsConfigs configures server TLS for @@ -5970,11 +5969,9 @@ spec: amount of compute resources required. type: object type: object - restAPIEnabled: - default: true - description: RestAPIEnabled determines if the - registry should serve using the REST API instead - of gRPC. + restAPI: + description: RestAPI determines if the registry + should serve using the REST API instead of gRPC. type: boolean tls: description: TlsConfigs configures server TLS diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 7429b98f0bf..cf42a1cdb64 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -1995,10 +1995,9 @@ spec: of compute resources required. type: object type: object - restAPIEnabled: - default: true - description: RestAPIEnabled determines if the registry - should serve using the REST API instead of gRPC. + restAPI: + description: RestAPI determines if the registry should + serve using the REST API instead of gRPC. type: boolean tls: description: TlsConfigs configures server TLS for @@ -5978,11 +5977,9 @@ spec: amount of compute resources required. type: object type: object - restAPIEnabled: - default: true - description: RestAPIEnabled determines if the - registry should serve using the REST API instead - of gRPC. + restAPI: + description: RestAPI determines if the registry + should serve using the REST API instead of gRPC. type: boolean tls: description: TlsConfigs configures server TLS diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index a8d69260833..f36734b31ed 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -695,7 +695,7 @@ Allowed values: "debug", "info", "warning", "error", "critical". | This allows attaching persistent storage, config files, secrets, or other resources required by the Feast components. Ensure that each volume mount has a corresponding volume definition in the Volumes field. | -| `restAPIEnabled` _boolean_ | RestAPIEnabled determines if the registry should serve using the REST API instead of gRPC. | +| `restAPI` _boolean_ | RestAPI determines if the registry should serve using the REST API instead of gRPC. | #### RemoteRegistryConfig diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 6060d88f105..2e997509ff1 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -502,7 +502,7 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st if feastType == RegistryFeastType && feast.isRegistryServer() { registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.RestAPIEnabled { + if registry.Local.Server.RestAPI { deploySettings.Args = append(deploySettings.Args, "--rest-api") } } From 0cedd493f7e68a5d0c652178516d86666d261ccd Mon Sep 17 00:00:00 2001 From: ntkathole Date: Wed, 21 May 2025 12:35:17 +0530 Subject: [PATCH 04/18] test: Added test to verify registry with REST API enabled Signed-off-by: ntkathole --- .../featurestore_controller_test.go | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index efbd69c78e9..3b3b9293add 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -1213,6 +1213,51 @@ var _ = Describe("FeatureStore Controller", func() { Expect(cond.Message).To(Equal("Error: Remote feast registry of referenced FeatureStore '" + referencedRegistry.Name + "' is not ready")) }) + It("should handle local registry with REST API enabled", func() { + By("By properly setting RestAPI registry in status") + + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + name := "rest-registry-featurestore" + namespace := "default" + nsName := types.NamespacedName{ + Name: name, + Namespace: namespace, + } + + resource := &feastdevv1alpha1.FeatureStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: feastdevv1alpha1.FeatureStoreSpec{ + FeastProject: feastProject, + Services: &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + RestAPI: true, + }, + }, + }, + }, + }, + } + resource.SetGroupVersionKind(feastdevv1alpha1.GroupVersion.WithKind("FeatureStore")) + err := k8sClient.Create(ctx, resource) + Expect(err).NotTo(HaveOccurred()) + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: nsName, + }) + Expect(err).NotTo(HaveOccurred()) + err = k8sClient.Get(ctx, nsName, resource) + Expect(err).NotTo(HaveOccurred()) + Expect(resource.Status.Applied.Services.Registry.Local.Server.RestAPI).To(BeTrue()) + }) + It("should error on reconcile", func() { By("Trying to set the controller OwnerRef of a Deployment that already has a controller") controllerReconciler := &FeatureStoreReconciler{ From 38dc1f815e4f67d0b2a534fc98f938728b5924a2 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Wed, 21 May 2025 19:15:00 +0530 Subject: [PATCH 05/18] test: Extend test to assert rest-api container args Signed-off-by: ntkathole --- .../featurestore_controller_test.go | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index 3b3b9293add..04837c95be5 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -1256,6 +1256,29 @@ var _ = Describe("FeatureStore Controller", func() { err = k8sClient.Get(ctx, nsName, resource) Expect(err).NotTo(HaveOccurred()) Expect(resource.Status.Applied.Services.Registry.Local.Server.RestAPI).To(BeTrue()) + By("checking the registry container command include --rest-api") + feast := services.FeastServices{ + Handler: handler.FeastHandler{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + }, + } + + // check deployment + deploy := &appsv1.Deployment{} + objMeta := feast.GetObjectMeta() + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: objMeta.Name, + Namespace: objMeta.Namespace, + }, deploy) + Expect(err).NotTo(HaveOccurred()) + + registryContainer := services.GetRegistryContainer(*deploy) + Expect(registryContainer).NotTo(BeNil()) + Expect(registryContainer.Command).To(ContainElement("--rest-api"), + "expected --rest-api to be present in registry container command: %v", registryContainer.Command) }) It("should error on reconcile", func() { From f70ba9b253bb301725c64d4eacd62f974b2c4ad8 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Tue, 10 Jun 2025 09:00:52 +0530 Subject: [PATCH 06/18] feat: Added both grpc and rest flags Signed-off-by: ntkathole --- .../api/v1alpha1/featurestore_types.go | 8 +- .../api/v1alpha1/zz_generated.deepcopy.go | 10 ++ .../crd/bases/feast.dev_featurestores.yaml | 16 +- infra/feast-operator/dist/install.yaml | 16 +- infra/feast-operator/docs/api/markdown/ref.md | 3 +- .../featurestore_controller_test.go | 154 ++++++++++++------ .../internal/controller/services/services.go | 7 +- .../internal/controller/services/util.go | 11 ++ 8 files changed, 160 insertions(+), 65 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index cf639dd829e..8305581317e 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -505,8 +505,12 @@ type ServerConfigs struct { // RegistryServerConfigs creates a registry server for the feast service, with specified container configurations. type RegistryServerConfigs struct { ServerConfigs `json:",inline"` - // RestAPI determines if the registry should serve using the REST API instead of gRPC. - RestAPI bool `json:"restAPI,omitempty"` + + // Enable REST API registry server. Defaults to false if unset. + RestAPI *bool `json:"restAPI,omitempty"` + + // Enable gRPC registry server. Defaults to true if unset. + GRPC *bool `json:"grpc,omitempty"` } // CronJobContainerConfigs k8s container settings for the CronJob diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index 81794a0217f..1a893c82cf8 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -932,6 +932,16 @@ func (in *RegistryPersistence) DeepCopy() *RegistryPersistence { func (in *RegistryServerConfigs) DeepCopyInto(out *RegistryServerConfigs) { *out = *in in.ServerConfigs.DeepCopyInto(&out.ServerConfigs) + if in.RestAPI != nil { + in, out := &in.RestAPI, &out.RestAPI + *out = new(bool) + **out = **in + } + if in.GRPC != nil { + in, out := &in.GRPC, &out.GRPC + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RegistryServerConfigs. 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 849428554e3..7efdd9712d0 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -1924,6 +1924,10 @@ spec: x-kubernetes-map-type: atomic type: object type: array + grpc: + description: Enable gRPC registry server. Defaults + to true if unset. + type: boolean image: type: string imagePullPolicy: @@ -1988,8 +1992,8 @@ spec: type: object type: object restAPI: - description: RestAPI determines if the registry should - serve using the REST API instead of gRPC. + description: Enable REST API registry server. Defaults + to false if unset. type: boolean tls: description: TlsConfigs configures server TLS for @@ -5906,6 +5910,10 @@ spec: x-kubernetes-map-type: atomic type: object type: array + grpc: + description: Enable gRPC registry server. Defaults + to true if unset. + type: boolean image: type: string imagePullPolicy: @@ -5970,8 +5978,8 @@ spec: type: object type: object restAPI: - description: RestAPI determines if the registry - should serve using the REST API instead of gRPC. + description: Enable REST API registry server. + Defaults to false if unset. type: boolean tls: description: TlsConfigs configures server TLS diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index cf42a1cdb64..3790c3df855 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -1932,6 +1932,10 @@ spec: x-kubernetes-map-type: atomic type: object type: array + grpc: + description: Enable gRPC registry server. Defaults + to true if unset. + type: boolean image: type: string imagePullPolicy: @@ -1996,8 +2000,8 @@ spec: type: object type: object restAPI: - description: RestAPI determines if the registry should - serve using the REST API instead of gRPC. + description: Enable REST API registry server. Defaults + to false if unset. type: boolean tls: description: TlsConfigs configures server TLS for @@ -5914,6 +5918,10 @@ spec: x-kubernetes-map-type: atomic type: object type: array + grpc: + description: Enable gRPC registry server. Defaults + to true if unset. + type: boolean image: type: string imagePullPolicy: @@ -5978,8 +5986,8 @@ spec: type: object type: object restAPI: - description: RestAPI determines if the registry - should serve using the REST API instead of gRPC. + description: Enable REST API registry server. + Defaults to false if unset. type: boolean tls: description: TlsConfigs configures server TLS diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index f36734b31ed..f4752f9a7d0 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -695,7 +695,8 @@ Allowed values: "debug", "info", "warning", "error", "critical". | This allows attaching persistent storage, config files, secrets, or other resources required by the Feast components. Ensure that each volume mount has a corresponding volume definition in the Volumes field. | -| `restAPI` _boolean_ | RestAPI determines if the registry should serve using the REST API instead of gRPC. | +| `restAPI` _boolean_ | Enable REST API registry server. Defaults to false if unset. | +| `grpc` _boolean_ | Enable gRPC registry server. Defaults to true if unset. | #### RemoteRegistryConfig diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index 04837c95be5..ea8bb506ff2 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -19,7 +19,9 @@ package controller import ( "context" "encoding/base64" + "fmt" "reflect" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -49,6 +51,10 @@ const domainTls = ".svc.cluster.local:443" var image = "test:latest" +func ptr[T any](v T) *T { + return &v +} + var _ = Describe("FeatureStore Controller", func() { Context("When reconciling a resource", func() { const resourceName = "test-resource" @@ -1213,72 +1219,114 @@ var _ = Describe("FeatureStore Controller", func() { Expect(cond.Message).To(Equal("Error: Remote feast registry of referenced FeatureStore '" + referencedRegistry.Name + "' is not ready")) }) - It("should handle local registry with REST API enabled", func() { - By("By properly setting RestAPI registry in status") - + It("should correctly set container command args for grpc/rest modes", func() { controllerReconciler := &FeatureStoreReconciler{ Client: k8sClient, Scheme: k8sClient.Scheme(), } - name := "rest-registry-featurestore" - namespace := "default" - nsName := types.NamespacedName{ - Name: name, - Namespace: namespace, + cases := []struct { + name string + grpc *bool + restAPI *bool + expectedArgs []string + }{ + { + name: "default grpc only", + grpc: nil, + restAPI: nil, + expectedArgs: []string{"--grpc"}, + }, + { + name: "explicit grpc true only", + grpc: ptr(true), + restAPI: ptr(false), + expectedArgs: []string{"--grpc"}, + }, + { + name: "rest only", + grpc: ptr(false), + restAPI: ptr(true), + expectedArgs: []string{"--no-grpc", "--rest-api"}, + }, + { + name: "both grpc and rest", + grpc: ptr(true), + restAPI: ptr(true), + expectedArgs: []string{"--grpc", "--rest-api"}, + }, + { + name: "explicitly disable both", + grpc: ptr(false), + restAPI: ptr(false), + expectedArgs: []string{"--no-grpc"}, + }, } - resource := &feastdevv1alpha1.FeatureStore{ - ObjectMeta: metav1.ObjectMeta{ + for _, tc := range cases { + By(fmt.Sprintf("Testing: %s", tc.name)) + + name := strings.ReplaceAll(tc.name, " ", "-") + nsName := types.NamespacedName{ Name: name, - Namespace: namespace, - }, - Spec: feastdevv1alpha1.FeatureStoreSpec{ - FeastProject: feastProject, - Services: &feastdevv1alpha1.FeatureStoreServices{ - Registry: &feastdevv1alpha1.Registry{ - Local: &feastdevv1alpha1.LocalRegistryConfig{ - Server: &feastdevv1alpha1.RegistryServerConfigs{ - RestAPI: true, + Namespace: "default", + } + + resource := &feastdevv1alpha1.FeatureStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Spec: feastdevv1alpha1.FeatureStoreSpec{ + FeastProject: feastProject, + Services: &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + GRPC: tc.grpc, + RestAPI: tc.restAPI, + }, }, }, }, }, - }, - } - resource.SetGroupVersionKind(feastdevv1alpha1.GroupVersion.WithKind("FeatureStore")) - err := k8sClient.Create(ctx, resource) - Expect(err).NotTo(HaveOccurred()) - _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ - NamespacedName: nsName, - }) - Expect(err).NotTo(HaveOccurred()) - err = k8sClient.Get(ctx, nsName, resource) - Expect(err).NotTo(HaveOccurred()) - Expect(resource.Status.Applied.Services.Registry.Local.Server.RestAPI).To(BeTrue()) - By("checking the registry container command include --rest-api") - feast := services.FeastServices{ - Handler: handler.FeastHandler{ - Client: controllerReconciler.Client, - Context: ctx, - Scheme: controllerReconciler.Scheme, - FeatureStore: resource, - }, - } - - // check deployment - deploy := &appsv1.Deployment{} - objMeta := feast.GetObjectMeta() - err = k8sClient.Get(ctx, types.NamespacedName{ - Name: objMeta.Name, - Namespace: objMeta.Namespace, - }, deploy) - Expect(err).NotTo(HaveOccurred()) + } + resource.SetGroupVersionKind(feastdevv1alpha1.GroupVersion.WithKind("FeatureStore")) + err := k8sClient.Create(ctx, resource) + Expect(err).NotTo(HaveOccurred()) + + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nsName}) + Expect(err).NotTo(HaveOccurred()) + + err = k8sClient.Get(ctx, nsName, resource) + Expect(err).NotTo(HaveOccurred()) + + feast := services.FeastServices{ + Handler: handler.FeastHandler{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + }, + } - registryContainer := services.GetRegistryContainer(*deploy) - Expect(registryContainer).NotTo(BeNil()) - Expect(registryContainer.Command).To(ContainElement("--rest-api"), - "expected --rest-api to be present in registry container command: %v", registryContainer.Command) + deploy := &appsv1.Deployment{} + objMeta := feast.GetObjectMeta() + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: objMeta.Name, + Namespace: objMeta.Namespace, + }, deploy) + Expect(err).NotTo(HaveOccurred()) + + registryContainer := services.GetRegistryContainer(*deploy) + Expect(registryContainer).NotTo(BeNil()) + + for _, expectedArg := range tc.expectedArgs { + Expect(registryContainer.Command). + To(ContainElement(expectedArg), + "expected %s to be present in container command: %v", expectedArg, registryContainer.Command) + } + } }) It("should error on reconcile", func() { diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 2e997509ff1..57150ad5b74 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -502,7 +502,12 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st if feastType == RegistryFeastType && feast.isRegistryServer() { registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.RestAPI { + if registry.Local.Server.GRPC != nil && *registry.Local.Server.GRPC { + deploySettings.Args = append(deploySettings.Args, "--grpc") + } else if registry.Local.Server.GRPC != nil && !*registry.Local.Server.GRPC { + deploySettings.Args = append(deploySettings.Args, "--no-grpc") + } + if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { deploySettings.Args = append(deploySettings.Args, "--rest-api") } } diff --git a/infra/feast-operator/internal/controller/services/util.go b/infra/feast-operator/internal/controller/services/util.go index 41f961e557b..01498d2637c 100644 --- a/infra/feast-operator/internal/controller/services/util.go +++ b/infra/feast-operator/internal/controller/services/util.go @@ -125,6 +125,17 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { if services.Registry.Local.Server != nil { setDefaultCtrConfigs(&services.Registry.Local.Server.ContainerConfigs.DefaultCtrConfigs) + // Set default for GRPC: true if nil + if services.Registry.Local.Server.GRPC == nil { + defaultGRPC := true + services.Registry.Local.Server.GRPC = &defaultGRPC + } + + // Set default for RestAPI: false if nil + if services.Registry.Local.Server.RestAPI == nil { + defaultRestAPI := false + services.Registry.Local.Server.RestAPI = &defaultRestAPI + } } } else if services.Registry.Remote.FeastRef != nil && len(services.Registry.Remote.FeastRef.Namespace) == 0 { services.Registry.Remote.FeastRef.Namespace = cr.Namespace From 06f559c040d3d0ea0f7653a00375f81d76896f33 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Tue, 10 Jun 2025 19:29:30 +0530 Subject: [PATCH 07/18] feat: Support seperate ports and service for rest-api Signed-off-by: ntkathole --- .../api/v1alpha1/featurestore_types.go | 1 + .../crd/bases/feast.dev_featurestores.yaml | 2 + infra/feast-operator/dist/install.yaml | 2 + infra/feast-operator/docs/api/markdown/ref.md | 1 + .../internal/controller/services/services.go | 113 ++++++++++++++++++ .../controller/services/services_types.go | 16 ++- 6 files changed, 129 insertions(+), 6 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 8305581317e..3bef5b81e6b 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -624,6 +624,7 @@ type ServiceHostnames struct { OfflineStore string `json:"offlineStore,omitempty"` OnlineStore string `json:"onlineStore,omitempty"` Registry string `json:"registry,omitempty"` + RegistryRest string `json:"registryRest,omitempty"` UI string `json:"ui,omitempty"` } 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 7efdd9712d0..86a1b68bae3 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -8067,6 +8067,8 @@ spec: type: string registry: type: string + registryRest: + type: string ui: type: string type: object diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 3790c3df855..39cb0be505d 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -8075,6 +8075,8 @@ spec: type: string registry: type: string + registryRest: + type: string ui: type: string type: object diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index f4752f9a7d0..acf833338f8 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -773,6 +773,7 @@ _Appears in:_ | `offlineStore` _string_ | | | `onlineStore` _string_ | | | `registry` _string_ | | +| `registryRest` _string_ | | | `ui` _string_ | | diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 57150ad5b74..f5821e94483 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -224,8 +224,32 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) if err := feast.createService(feastType); err != nil { return feast.setFeastServiceCondition(err, feastType) } + // Create REST API service if needed + if feastType == RegistryFeastType && feast.isRegistryServer() { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { + if err := feast.createRestService(feastType); err != nil { + return feast.setFeastServiceCondition(err, feastType) + } + } else { + // Delete REST API service if REST API is disabled + _ = feast.Handler.DeleteOwnedFeastObj(&corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: feast.GetFeastServiceName(feastType) + "-rest", + Namespace: feast.Handler.FeatureStore.Namespace, + }, + }) + } + } } else { _ = feast.Handler.DeleteOwnedFeastObj(feast.initFeastSvc(feastType)) + // Delete REST API service if it exists + _ = feast.Handler.DeleteOwnedFeastObj(&corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: feast.GetFeastServiceName(feastType) + "-rest", + Namespace: feast.Handler.FeatureStore.Namespace, + }, + }) } return feast.setFeastServiceCondition(nil, feastType) } @@ -404,6 +428,16 @@ func (feast *FeastServices) setContainer(containers *[]corev1.Container, feastTy Protocol: corev1.ProtocolTCP, }, } + if feastType == RegistryFeastType && feast.isRegistryServer() { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { + container.Ports = append(container.Ports, corev1.ContainerPort{ + Name: name + "-rest", + ContainerPort: getTargetRestPort(feastType, tls), + Protocol: corev1.ProtocolTCP, + }) + } + } container.StartupProbe = &corev1.Probe{ ProbeHandler: probeHandler, PeriodSeconds: 3, @@ -509,6 +543,7 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st } if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { deploySettings.Args = append(deploySettings.Args, "--rest-api") + deploySettings.Args = append(deploySettings.Args, "--rest-port", strconv.Itoa(int(getTargetRestPort(feastType, tls)))) } } if tls.IsTLS() { @@ -627,6 +662,70 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi return controllerutil.SetControllerReference(feast.Handler.FeatureStore, svc, feast.Handler.Scheme) } +// createRestService creates a separate service for REST API +func (feast *FeastServices) createRestService(feastType FeastServiceType) error { + if feastType != RegistryFeastType || !feast.isRegistryServer() { + return nil + } + + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + if registry.Local.Server.RestAPI == nil || !*registry.Local.Server.RestAPI { + return nil + } + + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: feast.GetFeastServiceName(feastType) + "-rest", + Namespace: feast.Handler.FeatureStore.Namespace, + Labels: feast.getFeastTypeLabels(feastType), + }, + } + svc.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) + + if feast.isOpenShiftTls(feastType) { + if len(svc.Annotations) == 0 { + svc.Annotations = map[string]string{} + } + svc.Annotations["service.beta.openshift.io/serving-cert-secret-name"] = svc.Name + tlsNameSuffix + } + + var port int32 = HttpPort + scheme := HttpScheme + tls := feast.getTlsConfigs(feastType) + if tls.IsTLS() { + port = HttpsPort + scheme = HttpsScheme + } + + svc.Spec = corev1.ServiceSpec{ + Selector: feast.getLabels(), + Type: corev1.ServiceTypeClusterIP, + Ports: []corev1.ServicePort{ + { + Name: scheme, + Port: port, + Protocol: corev1.ProtocolTCP, + TargetPort: intstr.FromInt(int(getTargetRestPort(feastType, tls))), + }, + }, + } + + if err := controllerutil.SetControllerReference(feast.Handler.FeatureStore, svc, feast.Handler.Scheme); err != nil { + return err + } + + logger := log.FromContext(feast.Handler.Context) + if op, err := controllerutil.CreateOrUpdate(feast.Handler.Context, feast.Handler.Client, svc, controllerutil.MutateFn(func() error { + return nil // No need to mutate as we set everything above + })); err != nil { + return err + } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + logger.Info("Successfully reconciled", "Service", svc.Name, "operation", op) + } + + return nil +} + func (feast *FeastServices) setServiceAccount(sa *corev1.ServiceAccount) error { sa.Labels = feast.getLabels() return controllerutil.SetControllerReference(feast.Handler.FeatureStore, sa, feast.Handler.Scheme) @@ -730,8 +829,15 @@ func (feast *FeastServices) setServiceHostnames() error { } if feast.isRegistryServer() { objMeta := feast.initFeastSvc(RegistryFeastType) + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry feast.Handler.FeatureStore.Status.ServiceHostnames.Registry = objMeta.Name + "." + objMeta.Namespace + domain + getPortStr(feast.Handler.FeatureStore.Status.Applied.Services.Registry.Local.Server.TLS) + if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { + // Use the REST API service name + restSvcName := objMeta.Name + "-rest" + feast.Handler.FeatureStore.Status.ServiceHostnames.RegistryRest = restSvcName + "." + objMeta.Namespace + domain + + getPortStr(feast.Handler.FeatureStore.Status.Applied.Services.Registry.Local.Server.TLS) + } } else if feast.isRemoteRegistry() { return feast.setRemoteRegistryURL() } @@ -1003,6 +1109,13 @@ func getTargetPort(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfigs) return FeastServiceConstants[feastType].TargetHttpPort } +func getTargetRestPort(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfigs) int32 { + if tls.IsTLS() { + return FeastServiceConstants[feastType].TargetRestHttpsPort + } + return FeastServiceConstants[feastType].TargetRestHttpPort +} + func getProbeHandler(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfigs) corev1.ProbeHandler { targetPort := getTargetPort(feastType, tls) if feastType == OnlineFeastType { diff --git a/infra/feast-operator/internal/controller/services/services_types.go b/infra/feast-operator/internal/controller/services/services_types.go index 962132ace07..4a84e9532cd 100644 --- a/infra/feast-operator/internal/controller/services/services_types.go +++ b/infra/feast-operator/internal/controller/services/services_types.go @@ -108,9 +108,11 @@ var ( TargetHttpsPort: 6567, }, RegistryFeastType: { - Args: []string{"serve_registry"}, - TargetHttpPort: 6570, - TargetHttpsPort: 6571, + Args: []string{"serve_registry"}, + TargetHttpPort: 6570, + TargetHttpsPort: 6571, + TargetRestHttpPort: 6572, + TargetRestHttpsPort: 6573, }, UIFeastType: { Args: []string{"ui", "-h", "0.0.0.0"}, @@ -283,9 +285,11 @@ type AuthzConfig struct { } type deploymentSettings struct { - Args []string - TargetHttpPort int32 - TargetHttpsPort int32 + Args []string + TargetHttpPort int32 + TargetHttpsPort int32 + TargetRestHttpPort int32 + TargetRestHttpsPort int32 } // CustomCertificatesBundle represents a custom CA bundle configuration From b991093c779a7d155ba2705faf29cc9e115b6bea Mon Sep 17 00:00:00 2001 From: ntkathole Date: Tue, 10 Jun 2025 21:41:48 +0530 Subject: [PATCH 08/18] feat: Added check for remote registry of referenced FeatureStore Signed-off-by: ntkathole --- .../featurestore_controller_test.go | 53 +++++++++++++++++++ .../internal/controller/services/services.go | 5 ++ 2 files changed, 58 insertions(+) diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index ea8bb506ff2..86ff6adc010 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -1476,6 +1476,59 @@ var _ = Describe("FeatureStore Controller", func() { err = k8sClient.Update(ctx, resource) Expect(err).NotTo(HaveOccurred()) }) + + It("should error if referencing a remote registry without gRPC server enabled", func() { + const remoteStoreName = "remote-featurestore" + remoteNamespacedName := types.NamespacedName{ + Name: remoteStoreName, + Namespace: "default", + } + + // Create remote FeatureStore with gRPC disabled + remote := &feastdevv1alpha1.FeatureStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: remoteStoreName, + Namespace: "default", + }, + Spec: feastdevv1alpha1.FeatureStoreSpec{ + FeastProject: feastProject, + Services: &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + GRPC: ptr(false), + RestAPI: ptr(true), + }, + }, + }, + }, + }, + } + Expect(k8sClient.Create(ctx, remote)).To(Succeed()) + reconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + _, err := reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: remoteNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + // Update main FeatureStore to reference the remote registry + Expect(k8sClient.Get(ctx, typeNamespacedName, featurestore)).To(Succeed()) + featurestore.Spec.FeastProject = feastProject + featurestore.Spec.Services.Registry = &feastdevv1alpha1.Registry{ + Remote: &feastdevv1alpha1.RemoteRegistryConfig{ + FeastRef: &feastdevv1alpha1.FeatureStoreRef{Name: remoteStoreName}, + }, + } + Expect(k8sClient.Update(ctx, featurestore)).To(Succeed()) + _, err = reconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("must have gRPC server enabled")) + }) }) }) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index f5821e94483..92a0f9e937f 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -877,6 +877,11 @@ func (feast *FeastServices) setRemoteRegistryURL() error { remoteFeast.isRegistryServer() && apimeta.IsStatusConditionTrue(remoteFeast.Handler.FeatureStore.Status.Conditions, feastdevv1alpha1.RegistryReadyType) && len(remoteFeast.Handler.FeatureStore.Status.ServiceHostnames.Registry) > 0 { + // Check if gRPC server is enabled + registry := remoteFeast.Handler.FeatureStore.Status.Applied.Services.Registry + if registry.Local.Server.GRPC == nil || !*registry.Local.Server.GRPC { + return errors.New("Remote feast registry of referenced FeatureStore '" + remoteFeast.Handler.FeatureStore.Name + "' must have gRPC server enabled") + } feast.Handler.FeatureStore.Status.ServiceHostnames.Registry = remoteFeast.Handler.FeatureStore.Status.ServiceHostnames.Registry } else { return errors.New("Remote feast registry of referenced FeatureStore '" + remoteFeast.Handler.FeatureStore.Name + "' is not ready") From 4e06343dc4327d712b27c63dcd368c2f3b0a0b5b Mon Sep 17 00:00:00 2001 From: Nikhil Kathole Date: Wed, 11 Jun 2025 20:45:07 +0530 Subject: [PATCH 09/18] Apply suggestions from code review Co-authored-by: Tommy Hughes IV Signed-off-by: ntkathole --- .../internal/controller/services/services.go | 16 ++++++++-------- .../internal/controller/services/util.go | 5 ----- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 92a0f9e937f..52c68ccf934 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -228,7 +228,7 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) if feastType == RegistryFeastType && feast.isRegistryServer() { registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { - if err := feast.createRestService(feastType); err != nil { + if err := feast.createRestService(); err != nil { return feast.setFeastServiceCondition(err, feastType) } } else { @@ -536,10 +536,12 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st if feastType == RegistryFeastType && feast.isRegistryServer() { registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.GRPC != nil && *registry.Local.Server.GRPC { + if registry.Local.Server.GRPC != nil { + if *registry.Local.Server.GRPC { deploySettings.Args = append(deploySettings.Args, "--grpc") - } else if registry.Local.Server.GRPC != nil && !*registry.Local.Server.GRPC { + } else { deploySettings.Args = append(deploySettings.Args, "--no-grpc") + } } if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { deploySettings.Args = append(deploySettings.Args, "--rest-api") @@ -662,11 +664,9 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi return controllerutil.SetControllerReference(feast.Handler.FeatureStore, svc, feast.Handler.Scheme) } -// createRestService creates a separate service for REST API -func (feast *FeastServices) createRestService(feastType FeastServiceType) error { - if feastType != RegistryFeastType || !feast.isRegistryServer() { - return nil - } +// createRestService creates a separate service for the Registry REST API +func (feast *FeastServices) createRestService() error { + if feast.isRegistryServer() { registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry if registry.Local.Server.RestAPI == nil || !*registry.Local.Server.RestAPI { diff --git a/infra/feast-operator/internal/controller/services/util.go b/infra/feast-operator/internal/controller/services/util.go index 01498d2637c..662308056e2 100644 --- a/infra/feast-operator/internal/controller/services/util.go +++ b/infra/feast-operator/internal/controller/services/util.go @@ -131,11 +131,6 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) { services.Registry.Local.Server.GRPC = &defaultGRPC } - // Set default for RestAPI: false if nil - if services.Registry.Local.Server.RestAPI == nil { - defaultRestAPI := false - services.Registry.Local.Server.RestAPI = &defaultRestAPI - } } } else if services.Registry.Remote.FeastRef != nil && len(services.Registry.Remote.FeastRef.Namespace) == 0 { services.Registry.Remote.FeastRef.Namespace = cr.Namespace From e28a5789fb03dadd4e4544608f86250956d370e3 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Wed, 11 Jun 2025 20:06:04 +0530 Subject: [PATCH 10/18] fix: Created GetFeastRestServiceName method Signed-off-by: ntkathole --- .../internal/controller/services/services.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 52c68ccf934..d344c298edb 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -235,7 +235,7 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) // Delete REST API service if REST API is disabled _ = feast.Handler.DeleteOwnedFeastObj(&corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: feast.GetFeastServiceName(feastType) + "-rest", + Name: feast.GetFeastRestServiceName(feastType), Namespace: feast.Handler.FeatureStore.Namespace, }, }) @@ -246,7 +246,7 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) // Delete REST API service if it exists _ = feast.Handler.DeleteOwnedFeastObj(&corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: feast.GetFeastServiceName(feastType) + "-rest", + Name: feast.GetFeastRestServiceName(feastType), Namespace: feast.Handler.FeatureStore.Namespace, }, }) @@ -675,7 +675,7 @@ func (feast *FeastServices) createRestService() error { svc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ - Name: feast.GetFeastServiceName(feastType) + "-rest", + Name: feast.GetFeastRestServiceName(feastType), Namespace: feast.Handler.FeatureStore.Namespace, Labels: feast.getFeastTypeLabels(feastType), }, @@ -834,7 +834,7 @@ func (feast *FeastServices) setServiceHostnames() error { getPortStr(feast.Handler.FeatureStore.Status.Applied.Services.Registry.Local.Server.TLS) if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { // Use the REST API service name - restSvcName := objMeta.Name + "-rest" + restSvcName := feast.GetFeastRestServiceName(RegistryFeastType) feast.Handler.FeatureStore.Status.ServiceHostnames.RegistryRest = restSvcName + "." + objMeta.Namespace + domain + getPortStr(feast.Handler.FeatureStore.Status.Applied.Services.Registry.Local.Server.TLS) } @@ -1151,3 +1151,8 @@ func IsDeploymentAvailable(conditions []appsv1.DeploymentCondition) bool { return false } + +// GetFeastRestServiceName returns the feast REST service object name based on service type +func (feast *FeastServices) GetFeastRestServiceName(feastType FeastServiceType) string { + return feast.GetFeastServiceName(feastType) + "-rest" +} From 63af9dadd1584c1b288b24b0f06c783411c6face Mon Sep 17 00:00:00 2001 From: ntkathole Date: Wed, 11 Jun 2025 20:31:21 +0530 Subject: [PATCH 11/18] fix: Refactor createRestService to reuse setService Signed-off-by: ntkathole --- .../internal/controller/services/services.go | 101 +++++++----------- 1 file changed, 41 insertions(+), 60 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index d344c298edb..3278e6f74e1 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -228,7 +228,7 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) if feastType == RegistryFeastType && feast.isRegistryServer() { registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { - if err := feast.createRestService(); err != nil { + if err := feast.createRestService(feastType); err != nil { return feast.setFeastServiceCondition(err, feastType) } } else { @@ -537,11 +537,11 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st if feastType == RegistryFeastType && feast.isRegistryServer() { registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry if registry.Local.Server.GRPC != nil { - if *registry.Local.Server.GRPC { - deploySettings.Args = append(deploySettings.Args, "--grpc") - } else { - deploySettings.Args = append(deploySettings.Args, "--no-grpc") - } + if *registry.Local.Server.GRPC { + deploySettings.Args = append(deploySettings.Args, "--grpc") + } else { + deploySettings.Args = append(deploySettings.Args, "--no-grpc") + } } if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { deploySettings.Args = append(deploySettings.Args, "--rest-api") @@ -632,7 +632,7 @@ func (feast *FeastServices) setInitContainer(podSpec *corev1.PodSpec, fsYamlB64 } } -func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServiceType) error { +func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServiceType, isRestService ...bool) error { svc.Labels = feast.getFeastTypeLabels(feastType) if feast.isOpenShiftTls(feastType) { if len(svc.Annotations) == 0 { @@ -648,6 +648,14 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi port = HttpsPort scheme = HttpsScheme } + + var targetPort int32 + if len(isRestService) > 0 && isRestService[0] { + targetPort = getTargetRestPort(feastType, tls) + } else { + targetPort = getTargetPort(feastType, tls) + } + svc.Spec = corev1.ServiceSpec{ Selector: feast.getLabels(), Type: corev1.ServiceTypeClusterIP, @@ -656,7 +664,7 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi Name: scheme, Port: port, Protocol: corev1.ProtocolTCP, - TargetPort: intstr.FromInt(int(getTargetPort(feastType, tls))), + TargetPort: intstr.FromInt(int(targetPort)), }, }, } @@ -665,62 +673,23 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi } // createRestService creates a separate service for the Registry REST API -func (feast *FeastServices) createRestService() error { +func (feast *FeastServices) createRestService(feastType FeastServiceType) error { if feast.isRegistryServer() { - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.RestAPI == nil || !*registry.Local.Server.RestAPI { - return nil - } - - svc := &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: feast.GetFeastRestServiceName(feastType), - Namespace: feast.Handler.FeatureStore.Namespace, - Labels: feast.getFeastTypeLabels(feastType), - }, - } - svc.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) - - if feast.isOpenShiftTls(feastType) { - if len(svc.Annotations) == 0 { - svc.Annotations = map[string]string{} + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + if registry.Local.Server.RestAPI == nil || !*registry.Local.Server.RestAPI { + return nil } - svc.Annotations["service.beta.openshift.io/serving-cert-secret-name"] = svc.Name + tlsNameSuffix - } - var port int32 = HttpPort - scheme := HttpScheme - tls := feast.getTlsConfigs(feastType) - if tls.IsTLS() { - port = HttpsPort - scheme = HttpsScheme - } - - svc.Spec = corev1.ServiceSpec{ - Selector: feast.getLabels(), - Type: corev1.ServiceTypeClusterIP, - Ports: []corev1.ServicePort{ - { - Name: scheme, - Port: port, - Protocol: corev1.ProtocolTCP, - TargetPort: intstr.FromInt(int(getTargetRestPort(feastType, tls))), - }, - }, - } - - if err := controllerutil.SetControllerReference(feast.Handler.FeatureStore, svc, feast.Handler.Scheme); err != nil { - return err - } - - logger := log.FromContext(feast.Handler.Context) - if op, err := controllerutil.CreateOrUpdate(feast.Handler.Context, feast.Handler.Client, svc, controllerutil.MutateFn(func() error { - return nil // No need to mutate as we set everything above - })); err != nil { - return err - } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - logger.Info("Successfully reconciled", "Service", svc.Name, "operation", op) + logger := log.FromContext(feast.Handler.Context) + svc := feast.initFeastRestSvc(feastType) + if op, err := controllerutil.CreateOrUpdate(feast.Handler.Context, feast.Handler.Client, svc, controllerutil.MutateFn(func() error { + return feast.setService(svc, feastType, true) + })); err != nil { + return err + } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + logger.Info("Successfully reconciled", "Service", svc.Name, "operation", op) + } } return nil @@ -987,6 +956,18 @@ func (feast *FeastServices) initFeastSvc(feastType FeastServiceType) *corev1.Ser return svc } +func (feast *FeastServices) initFeastRestSvc(feastType FeastServiceType) *corev1.Service { + svc := &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: feast.GetFeastRestServiceName(feastType), + Namespace: feast.Handler.FeatureStore.Namespace, + Labels: feast.getFeastTypeLabels(feastType), + }, + } + svc.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) + return svc +} + func (feast *FeastServices) initFeastSA() *corev1.ServiceAccount { sa := &corev1.ServiceAccount{ ObjectMeta: feast.GetObjectMeta(), From dc176baf21ca0da55617e205cff268865341d651 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Wed, 11 Jun 2025 21:50:53 +0530 Subject: [PATCH 12/18] fix: Handle service and port on grpc disable Signed-off-by: ntkathole --- .../internal/controller/services/services.go | 88 ++++++++++++++----- .../internal/controller/services/tls_test.go | 6 +- 2 files changed, 70 insertions(+), 24 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 3278e6f74e1..f4ecafd3c3b 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -221,9 +221,24 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) _ = feast.Handler.DeleteOwnedFeastObj(feast.initPVC(feastType)) } if serviceConfig := feast.getServerConfigs(feastType); serviceConfig != nil { - if err := feast.createService(feastType); err != nil { - return feast.setFeastServiceCondition(err, feastType) + // For registry service, only create service if gRPC is enabled + if feastType == RegistryFeastType && feast.isRegistryServer() { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + if registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC { + if err := feast.createService(feastType); err != nil { + return feast.setFeastServiceCondition(err, feastType) + } + } else { + // Delete service if gRPC is disabled + _ = feast.Handler.DeleteOwnedFeastObj(feast.initFeastSvc(feastType)) + } + } else { + // For non-registry services, always create service + if err := feast.createService(feastType); err != nil { + return feast.setFeastServiceCondition(err, feastType) + } } + // Create REST API service if needed if feastType == RegistryFeastType && feast.isRegistryServer() { registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry @@ -280,7 +295,7 @@ func (feast *FeastServices) createService(feastType FeastServiceType) error { logger := log.FromContext(feast.Handler.Context) svc := feast.initFeastSvc(feastType) if op, err := controllerutil.CreateOrUpdate(feast.Handler.Context, feast.Handler.Client, svc, controllerutil.MutateFn(func() error { - return feast.setService(svc, feastType) + return feast.setService(svc, feastType, false) })); err != nil { return err } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { @@ -420,24 +435,29 @@ func (feast *FeastServices) setContainer(containers *[]corev1.Container, feastTy cmd := feast.getContainerCommand(feastType) container := getContainer(name, workingDir, cmd, serverConfigs.ContainerConfigs, fsYamlB64) tls := feast.getTlsConfigs(feastType) - probeHandler := getProbeHandler(feastType, tls) - container.Ports = []corev1.ContainerPort{ - { + probeHandler := feast.getProbeHandler(feastType, tls) + container.Ports = []corev1.ContainerPort{} + + isRegistry := feastType == RegistryFeastType && feast.isRegistryServer() + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + + grpcEnabled := !isRegistry || registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC + if grpcEnabled { + container.Ports = append(container.Ports, corev1.ContainerPort{ Name: name, ContainerPort: getTargetPort(feastType, tls), Protocol: corev1.ProtocolTCP, - }, + }) } - if feastType == RegistryFeastType && feast.isRegistryServer() { - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { - container.Ports = append(container.Ports, corev1.ContainerPort{ - Name: name + "-rest", - ContainerPort: getTargetRestPort(feastType, tls), - Protocol: corev1.ProtocolTCP, - }) - } + + if isRegistry && registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { + container.Ports = append(container.Ports, corev1.ContainerPort{ + Name: name + "-rest", + ContainerPort: getTargetRestPort(feastType, tls), + Protocol: corev1.ProtocolTCP, + }) } + container.StartupProbe = &corev1.Probe{ ProbeHandler: probeHandler, PeriodSeconds: 3, @@ -632,7 +652,7 @@ func (feast *FeastServices) setInitContainer(podSpec *corev1.PodSpec, fsYamlB64 } } -func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServiceType, isRestService ...bool) error { +func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServiceType, isRestService bool) error { svc.Labels = feast.getFeastTypeLabels(feastType) if feast.isOpenShiftTls(feastType) { if len(svc.Annotations) == 0 { @@ -650,7 +670,7 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi } var targetPort int32 - if len(isRestService) > 0 && isRestService[0] { + if isRestService { targetPort = getTargetRestPort(feastType, tls) } else { targetPort = getTargetPort(feastType, tls) @@ -1102,9 +1122,34 @@ func getTargetRestPort(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConf return FeastServiceConstants[feastType].TargetRestHttpPort } -func getProbeHandler(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfigs) corev1.ProbeHandler { - targetPort := getTargetPort(feastType, tls) - if feastType == OnlineFeastType { +func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfigs) corev1.ProbeHandler { + if feastType == RegistryFeastType { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + grpcEnabled := registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC + restEnabled := registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI + + if restEnabled { + targetPort := getTargetRestPort(feastType, tls) + probeHandler := corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/health", + Port: intstr.FromInt(int(targetPort)), + }, + } + if tls.IsTLS() { + probeHandler.HTTPGet.Scheme = corev1.URISchemeHTTPS + } + return probeHandler + } else if grpcEnabled { + targetPort := getTargetPort(feastType, tls) + return corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(int(targetPort)), + }, + } + } + } else if feastType == OnlineFeastType { + targetPort := getTargetPort(feastType, tls) probeHandler := corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Path: "/health", @@ -1116,6 +1161,7 @@ func getProbeHandler(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfig } return probeHandler } + targetPort := getTargetPort(feastType, tls) return corev1.ProbeHandler{ TCPSocket: &corev1.TCPSocketAction{ Port: intstr.FromInt(int(targetPort)), diff --git a/infra/feast-operator/internal/controller/services/tls_test.go b/infra/feast-operator/internal/controller/services/tls_test.go index 3bd34964619..cb1212811cd 100644 --- a/infra/feast-operator/internal/controller/services/tls_test.go +++ b/infra/feast-operator/internal/controller/services/tls_test.go @@ -302,19 +302,19 @@ var _ = Describe("TLS Config", func() { // check k8s service objects offlineSvc := feast.initFeastSvc(OfflineFeastType) Expect(offlineSvc.Annotations).To(BeEmpty()) - err = feast.setService(offlineSvc, OfflineFeastType) + err = feast.setService(offlineSvc, OfflineFeastType, false) Expect(err).ToNot(HaveOccurred()) Expect(offlineSvc.Annotations).NotTo(BeEmpty()) Expect(offlineSvc.Spec.Ports[0].Name).To(Equal(HttpsScheme)) onlineSvc := feast.initFeastSvc(OnlineFeastType) - err = feast.setService(onlineSvc, OnlineFeastType) + err = feast.setService(onlineSvc, OnlineFeastType, false) Expect(err).ToNot(HaveOccurred()) Expect(onlineSvc.Annotations).To(BeEmpty()) Expect(onlineSvc.Spec.Ports[0].Name).To(Equal(HttpScheme)) uiSvc := feast.initFeastSvc(UIFeastType) - err = feast.setService(uiSvc, UIFeastType) + err = feast.setService(uiSvc, UIFeastType, false) Expect(err).ToNot(HaveOccurred()) Expect(uiSvc.Annotations).To(BeEmpty()) Expect(uiSvc.Spec.Ports[0].Name).To(Equal(HttpScheme)) From 9785b9d33dc0f5a0a2aeccc1e12abdf37b902718 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Thu, 12 Jun 2025 11:06:06 +0530 Subject: [PATCH 13/18] feat: Added isRegistryRestEnabled and isRegistryGrpcEnabled methods Signed-off-by: ntkathole --- .../featurestore_controller_test.go | 6 -- .../internal/controller/services/services.go | 91 ++++++++++--------- 2 files changed, 48 insertions(+), 49 deletions(-) diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index 86ff6adc010..465b5696152 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -1255,12 +1255,6 @@ var _ = Describe("FeatureStore Controller", func() { restAPI: ptr(true), expectedArgs: []string{"--grpc", "--rest-api"}, }, - { - name: "explicitly disable both", - grpc: ptr(false), - restAPI: ptr(false), - expectedArgs: []string{"--no-grpc"}, - }, } for _, tc := range cases { diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index f4ecafd3c3b..81b6f918778 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -54,6 +54,11 @@ func (feast *FeastServices) Deploy() error { if feast.noLocalCoreServerConfigured() { return errors.New("at least one local server must be configured. e.g. registry / online / offline") } + if feast.isRegistryServer() { + if !feast.isRegistryGrpcEnabled() && !feast.isRegistryRestEnabled() { + return errors.New("at least one of gRPC or REST API must be enabled for registry service") + } + } openshiftTls, err := feast.checkOpenshiftTls() if err != nil { return err @@ -221,33 +226,30 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) _ = feast.Handler.DeleteOwnedFeastObj(feast.initPVC(feastType)) } if serviceConfig := feast.getServerConfigs(feastType); serviceConfig != nil { - // For registry service, only create service if gRPC is enabled + // For registry service, handle both gRPC and REST services if feastType == RegistryFeastType && feast.isRegistryServer() { - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC { + // Create gRPC service if enabled + if feast.isRegistryGrpcEnabled() { if err := feast.createService(feastType); err != nil { return feast.setFeastServiceCondition(err, feastType) } } else { - // Delete service if gRPC is disabled - _ = feast.Handler.DeleteOwnedFeastObj(feast.initFeastSvc(feastType)) - } - } else { - // For non-registry services, always create service - if err := feast.createService(feastType); err != nil { - return feast.setFeastServiceCondition(err, feastType) + // Delete gRPC service if disabled + _ = feast.Handler.DeleteOwnedFeastObj(&corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: feast.GetFeastServiceName(feastType), + Namespace: feast.Handler.FeatureStore.Namespace, + }, + }) } - } - // Create REST API service if needed - if feastType == RegistryFeastType && feast.isRegistryServer() { - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { + // Create REST service if enabled + if feast.isRegistryRestEnabled() { if err := feast.createRestService(feastType); err != nil { return feast.setFeastServiceCondition(err, feastType) } } else { - // Delete REST API service if REST API is disabled + // Delete REST service if disabled _ = feast.Handler.DeleteOwnedFeastObj(&corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: feast.GetFeastRestServiceName(feastType), @@ -255,6 +257,11 @@ func (feast *FeastServices) deployFeastServiceByType(feastType FeastServiceType) }, }) } + } else { + // For non-registry services, always create service + if err := feast.createService(feastType); err != nil { + return feast.setFeastServiceCondition(err, feastType) + } } } else { _ = feast.Handler.DeleteOwnedFeastObj(feast.initFeastSvc(feastType)) @@ -439,9 +446,8 @@ func (feast *FeastServices) setContainer(containers *[]corev1.Container, feastTy container.Ports = []corev1.ContainerPort{} isRegistry := feastType == RegistryFeastType && feast.isRegistryServer() - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - grpcEnabled := !isRegistry || registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC + grpcEnabled := !feast.isRegistryServer() || feast.isRegistryGrpcEnabled() if grpcEnabled { container.Ports = append(container.Ports, corev1.ContainerPort{ Name: name, @@ -450,7 +456,7 @@ func (feast *FeastServices) setContainer(containers *[]corev1.Container, feastTy }) } - if isRegistry && registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { + if isRegistry && feast.isRegistryRestEnabled() { container.Ports = append(container.Ports, corev1.ContainerPort{ Name: name + "-rest", ContainerPort: getTargetRestPort(feastType, tls), @@ -555,15 +561,12 @@ func (feast *FeastServices) getContainerCommand(feastType FeastServiceType) []st tls := feast.getTlsConfigs(feastType) if feastType == RegistryFeastType && feast.isRegistryServer() { - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.GRPC != nil { - if *registry.Local.Server.GRPC { - deploySettings.Args = append(deploySettings.Args, "--grpc") - } else { - deploySettings.Args = append(deploySettings.Args, "--no-grpc") - } + if feast.isRegistryGrpcEnabled() { + deploySettings.Args = append(deploySettings.Args, "--grpc") + } else { + deploySettings.Args = append(deploySettings.Args, "--no-grpc") } - if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { + if feast.isRegistryRestEnabled() { deploySettings.Args = append(deploySettings.Args, "--rest-api") deploySettings.Args = append(deploySettings.Args, "--rest-port", strconv.Itoa(int(getTargetRestPort(feastType, tls)))) } @@ -695,12 +698,9 @@ func (feast *FeastServices) setService(svc *corev1.Service, feastType FeastServi // createRestService creates a separate service for the Registry REST API func (feast *FeastServices) createRestService(feastType FeastServiceType) error { if feast.isRegistryServer() { - - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.RestAPI == nil || !*registry.Local.Server.RestAPI { + if !feast.isRegistryRestEnabled() { return nil } - logger := log.FromContext(feast.Handler.Context) svc := feast.initFeastRestSvc(feastType) if op, err := controllerutil.CreateOrUpdate(feast.Handler.Context, feast.Handler.Client, svc, controllerutil.MutateFn(func() error { @@ -711,7 +711,6 @@ func (feast *FeastServices) createRestService(feastType FeastServiceType) error logger.Info("Successfully reconciled", "Service", svc.Name, "operation", op) } } - return nil } @@ -818,10 +817,9 @@ func (feast *FeastServices) setServiceHostnames() error { } if feast.isRegistryServer() { objMeta := feast.initFeastSvc(RegistryFeastType) - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry feast.Handler.FeatureStore.Status.ServiceHostnames.Registry = objMeta.Name + "." + objMeta.Namespace + domain + getPortStr(feast.Handler.FeatureStore.Status.Applied.Services.Registry.Local.Server.TLS) - if registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI { + if feast.isRegistryRestEnabled() { // Use the REST API service name restSvcName := feast.GetFeastRestServiceName(RegistryFeastType) feast.Handler.FeatureStore.Status.ServiceHostnames.RegistryRest = restSvcName + "." + objMeta.Namespace + domain + @@ -867,8 +865,7 @@ func (feast *FeastServices) setRemoteRegistryURL() error { apimeta.IsStatusConditionTrue(remoteFeast.Handler.FeatureStore.Status.Conditions, feastdevv1alpha1.RegistryReadyType) && len(remoteFeast.Handler.FeatureStore.Status.ServiceHostnames.Registry) > 0 { // Check if gRPC server is enabled - registry := remoteFeast.Handler.FeatureStore.Status.Applied.Services.Registry - if registry.Local.Server.GRPC == nil || !*registry.Local.Server.GRPC { + if !remoteFeast.isRegistryGrpcEnabled() { return errors.New("Remote feast registry of referenced FeatureStore '" + remoteFeast.Handler.FeatureStore.Name + "' must have gRPC server enabled") } feast.Handler.FeatureStore.Status.ServiceHostnames.Registry = remoteFeast.Handler.FeatureStore.Status.ServiceHostnames.Registry @@ -1124,15 +1121,10 @@ func getTargetRestPort(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConf func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfigs) corev1.ProbeHandler { if feastType == RegistryFeastType { - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - grpcEnabled := registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC - restEnabled := registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI - - if restEnabled { + if feast.isRegistryRestEnabled() { targetPort := getTargetRestPort(feastType, tls) probeHandler := corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ - Path: "/health", Port: intstr.FromInt(int(targetPort)), }, } @@ -1140,7 +1132,8 @@ func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *fea probeHandler.HTTPGet.Scheme = corev1.URISchemeHTTPS } return probeHandler - } else if grpcEnabled { + } + if feast.isRegistryGrpcEnabled() { targetPort := getTargetPort(feastType, tls) return corev1.ProbeHandler{ TCPSocket: &corev1.TCPSocketAction{ @@ -1183,3 +1176,15 @@ func IsDeploymentAvailable(conditions []appsv1.DeploymentCondition) bool { func (feast *FeastServices) GetFeastRestServiceName(feastType FeastServiceType) string { return feast.GetFeastServiceName(feastType) + "-rest" } + +// isRegistryGrpcEnabled checks if gRPC is enabled for registry service +func (feast *FeastServices) isRegistryGrpcEnabled() bool { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + return registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC +} + +// isRegistryRestEnabled checks if REST API is enabled for registry service +func (feast *FeastServices) isRegistryRestEnabled() bool { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + return registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI +} From f231f0d30d2da2012034db5ca3a0068484a4d143 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Thu, 12 Jun 2025 19:06:50 +0530 Subject: [PATCH 14/18] fix: Fixed rest server port Signed-off-by: ntkathole --- sdk/python/feast/cli/serve.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/cli/serve.py b/sdk/python/feast/cli/serve.py index 049eb232165..b5ff950a042 100644 --- a/sdk/python/feast/cli/serve.py +++ b/sdk/python/feast/cli/serve.py @@ -232,7 +232,15 @@ def serve_registry_command( for p in servers: p.join() else: - store.serve_registry(port, tls_key_path, tls_cert_path, rest_api) + if grpc: + store.serve_registry(port, tls_key_path, tls_cert_path) + else: + store.serve_registry( + port=rest_port, + tls_key_path=tls_key_path, + tls_cert_path=tls_cert_path, + rest_api=rest_api, + ) def _serve_grpc_registry( From 68480ab68b021d1c6a42f90cbd7399c347bf15ea Mon Sep 17 00:00:00 2001 From: ntkathole Date: Fri, 13 Jun 2025 16:46:43 +0530 Subject: [PATCH 15/18] fix: Changes as per comments Signed-off-by: ntkathole --- .../api/v1alpha1/featurestore_types.go | 3 +- .../crd/bases/feast.dev_featurestores.yaml | 12 +++- infra/feast-operator/dist/install.yaml | 12 +++- infra/feast-operator/docs/api/markdown/ref.md | 2 +- .../featurestore_controller_test.go | 34 +++++++++- .../internal/controller/services/services.go | 66 +++++++++++-------- 6 files changed, 92 insertions(+), 37 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 3bef5b81e6b..8587fc98240 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -503,10 +503,11 @@ type ServerConfigs struct { } // RegistryServerConfigs creates a registry server for the feast service, with specified container configurations. +// +kubebuilder:validation:XValidation:rule="self.restAPI == true || self.grpc == true || !has(self.grpc)", message="At least one of restAPI or grpc must be true" type RegistryServerConfigs struct { ServerConfigs `json:",inline"` - // Enable REST API registry server. Defaults to false if unset. + // Enable REST API registry server. RestAPI *bool `json:"restAPI,omitempty"` // Enable gRPC registry server. Defaults to true if unset. 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 86a1b68bae3..b2fed6992d5 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -1992,8 +1992,7 @@ spec: type: object type: object restAPI: - description: Enable REST API registry server. Defaults - to false if unset. + description: Enable REST API registry server. type: boolean tls: description: TlsConfigs configures server TLS for @@ -2079,6 +2078,9 @@ spec: type: object type: array type: object + x-kubernetes-validations: + - message: At least one of restAPI or grpc must be true + rule: self.restAPI == true || self.grpc == true || !has(self.grpc) type: object remote: description: RemoteRegistryConfig points to a remote feast @@ -5979,7 +5981,6 @@ spec: type: object restAPI: description: Enable REST API registry server. - Defaults to false if unset. type: boolean tls: description: TlsConfigs configures server TLS @@ -6069,6 +6070,11 @@ spec: type: object type: array type: object + x-kubernetes-validations: + - message: At least one of restAPI or grpc must be + true + rule: self.restAPI == true || self.grpc == true + || !has(self.grpc) type: object remote: description: RemoteRegistryConfig points to a remote feast diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 39cb0be505d..8cfbf968b34 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -2000,8 +2000,7 @@ spec: type: object type: object restAPI: - description: Enable REST API registry server. Defaults - to false if unset. + description: Enable REST API registry server. type: boolean tls: description: TlsConfigs configures server TLS for @@ -2087,6 +2086,9 @@ spec: type: object type: array type: object + x-kubernetes-validations: + - message: At least one of restAPI or grpc must be true + rule: self.restAPI == true || self.grpc == true || !has(self.grpc) type: object remote: description: RemoteRegistryConfig points to a remote feast @@ -5987,7 +5989,6 @@ spec: type: object restAPI: description: Enable REST API registry server. - Defaults to false if unset. type: boolean tls: description: TlsConfigs configures server TLS @@ -6077,6 +6078,11 @@ spec: type: object type: array type: object + x-kubernetes-validations: + - message: At least one of restAPI or grpc must be + true + rule: self.restAPI == true || self.grpc == true + || !has(self.grpc) type: object remote: description: RemoteRegistryConfig points to a remote feast diff --git a/infra/feast-operator/docs/api/markdown/ref.md b/infra/feast-operator/docs/api/markdown/ref.md index acf833338f8..9452d9c838b 100644 --- a/infra/feast-operator/docs/api/markdown/ref.md +++ b/infra/feast-operator/docs/api/markdown/ref.md @@ -695,7 +695,7 @@ Allowed values: "debug", "info", "warning", "error", "critical". | This allows attaching persistent storage, config files, secrets, or other resources required by the Feast components. Ensure that each volume mount has a corresponding volume definition in the Volumes field. | -| `restAPI` _boolean_ | Enable REST API registry server. Defaults to false if unset. | +| `restAPI` _boolean_ | Enable REST API registry server. | | `grpc` _boolean_ | Enable gRPC registry server. Defaults to true if unset. | diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index 465b5696152..cb45d85e766 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -1265,7 +1265,6 @@ var _ = Describe("FeatureStore Controller", func() { Name: name, Namespace: "default", } - resource := &feastdevv1alpha1.FeatureStore{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -1320,7 +1319,40 @@ var _ = Describe("FeatureStore Controller", func() { To(ContainElement(expectedArg), "expected %s to be present in container command: %v", expectedArg, registryContainer.Command) } + Expect(resource.Status.Conditions).NotTo(BeEmpty()) + cond := apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.RegistryReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.RegistryReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.RegistryReadyMessage)) + } + + By("Verifying that creation fails when both REST API and gRPC are disabled") + disabledResource := &feastdevv1alpha1.FeatureStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: "disabled-both", + Namespace: "default", + }, + Spec: feastdevv1alpha1.FeatureStoreSpec{ + FeastProject: feastProject, + Services: &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + RestAPI: ptr(false), + GRPC: ptr(false), + }, + }, + }, + }, + }, } + disabledResource.SetGroupVersionKind(feastdevv1alpha1.GroupVersion.WithKind("FeatureStore")) + + err := k8sClient.Create(ctx, disabledResource) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("At least one of restAPI or grpc must be true")) }) It("should error on reconcile", func() { diff --git a/infra/feast-operator/internal/controller/services/services.go b/infra/feast-operator/internal/controller/services/services.go index 81b6f918778..959814b5a64 100644 --- a/infra/feast-operator/internal/controller/services/services.go +++ b/infra/feast-operator/internal/controller/services/services.go @@ -445,10 +445,22 @@ func (feast *FeastServices) setContainer(containers *[]corev1.Container, feastTy probeHandler := feast.getProbeHandler(feastType, tls) container.Ports = []corev1.ContainerPort{} - isRegistry := feastType == RegistryFeastType && feast.isRegistryServer() - - grpcEnabled := !feast.isRegistryServer() || feast.isRegistryGrpcEnabled() - if grpcEnabled { + if feastType == RegistryFeastType { + if feast.isRegistryGrpcEnabled() { + container.Ports = append(container.Ports, corev1.ContainerPort{ + Name: name, + ContainerPort: getTargetPort(feastType, tls), + Protocol: corev1.ProtocolTCP, + }) + } + if feast.isRegistryRestEnabled() { + container.Ports = append(container.Ports, corev1.ContainerPort{ + Name: name + "-rest", + ContainerPort: getTargetRestPort(feastType, tls), + Protocol: corev1.ProtocolTCP, + }) + } + } else { container.Ports = append(container.Ports, corev1.ContainerPort{ Name: name, ContainerPort: getTargetPort(feastType, tls), @@ -456,14 +468,6 @@ func (feast *FeastServices) setContainer(containers *[]corev1.Container, feastTy }) } - if isRegistry && feast.isRegistryRestEnabled() { - container.Ports = append(container.Ports, corev1.ContainerPort{ - Name: name + "-rest", - ContainerPort: getTargetRestPort(feastType, tls), - Protocol: corev1.ProtocolTCP, - }) - } - container.StartupProbe = &corev1.Probe{ ProbeHandler: probeHandler, PeriodSeconds: 3, @@ -1120,9 +1124,18 @@ func getTargetRestPort(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConf } func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *feastdevv1alpha1.TlsConfigs) corev1.ProbeHandler { + targetPort := getTargetPort(feastType, tls) + if feastType == RegistryFeastType { + if feast.isRegistryGrpcEnabled() { + return corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt(int(targetPort)), + }, + } + } if feast.isRegistryRestEnabled() { - targetPort := getTargetRestPort(feastType, tls) + targetPort = getTargetRestPort(feastType, tls) probeHandler := corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Port: intstr.FromInt(int(targetPort)), @@ -1133,16 +1146,8 @@ func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *fea } return probeHandler } - if feast.isRegistryGrpcEnabled() { - targetPort := getTargetPort(feastType, tls) - return corev1.ProbeHandler{ - TCPSocket: &corev1.TCPSocketAction{ - Port: intstr.FromInt(int(targetPort)), - }, - } - } - } else if feastType == OnlineFeastType { - targetPort := getTargetPort(feastType, tls) + } + if feastType == OnlineFeastType { probeHandler := corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Path: "/health", @@ -1154,7 +1159,6 @@ func (feast *FeastServices) getProbeHandler(feastType FeastServiceType, tls *fea } return probeHandler } - targetPort := getTargetPort(feastType, tls) return corev1.ProbeHandler{ TCPSocket: &corev1.TCPSocketAction{ Port: intstr.FromInt(int(targetPort)), @@ -1179,12 +1183,18 @@ func (feast *FeastServices) GetFeastRestServiceName(feastType FeastServiceType) // isRegistryGrpcEnabled checks if gRPC is enabled for registry service func (feast *FeastServices) isRegistryGrpcEnabled() bool { - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - return registry.Local.Server.GRPC == nil || *registry.Local.Server.GRPC + if feast.isRegistryServer() { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + return registry.Local.Server.GRPC != nil && *registry.Local.Server.GRPC + } + return false } // isRegistryRestEnabled checks if REST API is enabled for registry service func (feast *FeastServices) isRegistryRestEnabled() bool { - registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry - return registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI + if feast.isRegistryServer() { + registry := feast.Handler.FeatureStore.Status.Applied.Services.Registry + return registry.Local.Server.RestAPI != nil && *registry.Local.Server.RestAPI + } + return false } From d73e524463cba69cd35b81da7ec577f6a1b49e8c Mon Sep 17 00:00:00 2001 From: ntkathole Date: Fri, 13 Jun 2025 19:03:14 +0530 Subject: [PATCH 16/18] fix: Handle tls mount for rest server Signed-off-by: ntkathole --- .../internal/controller/services/tls.go | 16 ++++++-- .../internal/controller/services/tls_test.go | 40 +++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/infra/feast-operator/internal/controller/services/tls.go b/infra/feast-operator/internal/controller/services/tls.go index e3955f7d115..c447d9e99ec 100644 --- a/infra/feast-operator/internal/controller/services/tls.go +++ b/infra/feast-operator/internal/controller/services/tls.go @@ -71,10 +71,18 @@ func (feast *FeastServices) setOpenshiftTls() error { } } if feast.localRegistryOpenshiftTls() { - appliedServices.Registry.Local.Server.TLS = &feastdevv1alpha1.TlsConfigs{ - SecretRef: &corev1.LocalObjectReference{ - Name: feast.initFeastSvc(RegistryFeastType).Name + tlsNameSuffix, - }, + if feast.isRegistryRestEnabled() { + appliedServices.Registry.Local.Server.TLS = &feastdevv1alpha1.TlsConfigs{ + SecretRef: &corev1.LocalObjectReference{ + Name: feast.initFeastRestSvc(RegistryFeastType).Name + tlsNameSuffix, + }, + } + } else { + appliedServices.Registry.Local.Server.TLS = &feastdevv1alpha1.TlsConfigs{ + SecretRef: &corev1.LocalObjectReference{ + Name: feast.initFeastSvc(RegistryFeastType).Name + tlsNameSuffix, + }, + } } } else if remote, err := feast.remoteRegistryOpenshiftTls(); remote { // if the remote registry reference is using openshift's service serving certificates, we can use the injected service CA bundle configMap diff --git a/infra/feast-operator/internal/controller/services/tls_test.go b/infra/feast-operator/internal/controller/services/tls_test.go index cb1212811cd..1b63b9d9830 100644 --- a/infra/feast-operator/internal/controller/services/tls_test.go +++ b/infra/feast-operator/internal/controller/services/tls_test.go @@ -336,6 +336,46 @@ var _ = Describe("TLS Config", func() { Expect(GetUIContainer(*feastDeploy).Command).NotTo(ContainElements(ContainSubstring("--key"))) Expect(GetUIContainer(*feastDeploy).VolumeMounts).To(HaveLen(1)) + // Test REST registry server TLS configuration + feast.Handler.FeatureStore = minimalFeatureStore() + restEnabled := true + grpcEnabled := false + feast.Handler.FeatureStore.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{}, + RestAPI: &restEnabled, + GRPC: &grpcEnabled, + }, + }, + }, + } + testSetIsOpenShift() + err = feast.ApplyDefaults() + Expect(err).ToNot(HaveOccurred()) + + tls = feast.getTlsConfigs(RegistryFeastType) + Expect(tls).NotTo(BeNil()) + Expect(tls.IsTLS()).To(BeTrue()) + Expect(tls.SecretRef).NotTo(BeNil()) + Expect(tls.SecretRef.Name).To(Equal("feast-test-registry-rest-tls")) + Expect(tls.SecretKeyNames).To(Equal(secretKeyNames)) + Expect(getPortStr(tls)).To(Equal("443")) + Expect(GetTlsPath(RegistryFeastType)).To(Equal("/tls/registry/")) + + registryRestSvc := feast.initFeastRestSvc(RegistryFeastType) + err = feast.setService(registryRestSvc, RegistryFeastType, true) + Expect(err).ToNot(HaveOccurred()) + Expect(registryRestSvc.Annotations).NotTo(BeEmpty()) + Expect(registryRestSvc.Spec.Ports[0].Name).To(Equal(HttpsScheme)) + + feastDeploy = feast.initFeastDeploy() + err = feast.setDeployment(feastDeploy) + Expect(err).ToNot(HaveOccurred()) + registryContainer := GetRegistryContainer(*feastDeploy) + Expect(registryContainer).NotTo(BeNil()) + Expect(registryContainer.Command).To(ContainElements(ContainSubstring("--key"))) }) }) }) From 6ace5ecd0081b5e690557912081927985a4c2fd0 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Fri, 13 Jun 2025 20:18:32 +0530 Subject: [PATCH 17/18] test: Added tests for field validations Signed-off-by: ntkathole --- .../test/api/featurestore_types_test.go | 169 ++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/infra/feast-operator/test/api/featurestore_types_test.go b/infra/feast-operator/test/api/featurestore_types_test.go index 12a7406e80d..83ac2906ec0 100644 --- a/infra/feast-operator/test/api/featurestore_types_test.go +++ b/infra/feast-operator/test/api/featurestore_types_test.go @@ -18,6 +18,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +func boolPtr(b bool) *bool { + return &b +} + func createFeatureStore() *feastdevv1alpha1.FeatureStore { return &feastdevv1alpha1.FeatureStore{ ObjectMeta: metav1.ObjectMeta{ @@ -336,6 +340,104 @@ func registryStoreWithDBPersistenceType(dbPersistenceType string, featureStore * return fsCopy } +func registryWithRestAPIFalse(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + fsCopy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + RestAPI: boolPtr(false), + }, + }, + }, + } + return fsCopy +} + +func registryWithOnlyRestAPI(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + fsCopy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + RestAPI: boolPtr(true), + }, + }, + }, + } + return fsCopy +} + +func registryWithOnlyGRPC(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + fsCopy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + GRPC: boolPtr(true), + }, + }, + }, + } + return fsCopy +} + +func registryWithBothAPIs(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + fsCopy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + RestAPI: boolPtr(true), + GRPC: boolPtr(true), + }, + }, + }, + } + return fsCopy +} + +func registryWithNoAPIs(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + fsCopy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{}, + }, + }, + } + return fsCopy +} + +func registryWithBothFalse(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + fsCopy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + RestAPI: boolPtr(false), + GRPC: boolPtr(false), + }, + }, + }, + } + return fsCopy +} + +func registryWithGRPCFalse(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + fsCopy.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + GRPC: boolPtr(false), + }, + }, + }, + } + return fsCopy +} + func quotedSlice(stringSlice []string) string { quotedSlice := make([]string, len(stringSlice)) @@ -476,4 +578,71 @@ var _ = Describe("FeatureStore API", func() { attemptInvalidCreationAndAsserts(ctx, authzConfigWithOidc(authzConfigWithKubernetes(featurestore)), "One selection required between kubernetes or oidc") }) }) + + Context("When creating a Registry", func() { + ctx := context.Background() + + BeforeEach(func() { + By("verifying the custom resource FeatureStore is not there") + resource := &feastdevv1alpha1.FeatureStore{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err != nil && errors.IsNotFound(err)).To(BeTrue()) + }) + AfterEach(func() { + By("Cleaning up the test resource") + resource := &feastdevv1alpha1.FeatureStore{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + if err == nil { + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + } + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err != nil && errors.IsNotFound(err)).To(BeTrue()) + }) + + Context("with valid API configurations", func() { + It("should succeed when restAPI is false and grpc is not specified (defaults to true)", func() { + featurestore := createFeatureStore() + resource := registryWithRestAPIFalse(featurestore) + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }) + + It("should succeed when restAPI is true and grpc is not specified (defaults to true)", func() { + featurestore := createFeatureStore() + resource := registryWithOnlyRestAPI(featurestore) + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }) + + It("should succeed when only grpc is true", func() { + featurestore := createFeatureStore() + resource := registryWithOnlyGRPC(featurestore) + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }) + + It("should succeed when both APIs are true", func() { + featurestore := createFeatureStore() + resource := registryWithBothAPIs(featurestore) + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }) + + It("should succeed when no APIs are specified (grpc defaults to true)", func() { + featurestore := createFeatureStore() + resource := registryWithNoAPIs(featurestore) + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + }) + }) + + Context("with invalid API configurations", func() { + It("should fail when both APIs are explicitly false", func() { + featurestore := createFeatureStore() + resource := registryWithBothFalse(featurestore) + attemptInvalidCreationAndAsserts(ctx, resource, "At least one of restAPI or grpc must be true") + }) + + It("should fail when grpc is false and restAPI is not specified", func() { + featurestore := createFeatureStore() + resource := registryWithGRPCFalse(featurestore) + attemptInvalidCreationAndAsserts(ctx, resource, "At least one of restAPI or grpc must be true") + }) + }) + }) }) From 8c649c245e415430be5ef27a50ee00da1c4ccb44 Mon Sep 17 00:00:00 2001 From: ntkathole Date: Fri, 13 Jun 2025 21:36:47 +0530 Subject: [PATCH 18/18] test: Added tests for svc probe, container port and registry validations Signed-off-by: ntkathole --- .../controller/services/services_test.go | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 infra/feast-operator/internal/controller/services/services_test.go diff --git a/infra/feast-operator/internal/controller/services/services_test.go b/infra/feast-operator/internal/controller/services/services_test.go new file mode 100644 index 00000000000..0c43aff5954 --- /dev/null +++ b/infra/feast-operator/internal/controller/services/services_test.go @@ -0,0 +1,206 @@ +/* +Copyright 2024 Feast Community. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package services + +import ( + "context" + + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/handler" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" +) + +func ptr[T any](v T) *T { + return &v +} + +func (feast *FeastServices) refreshFeatureStore(ctx context.Context, key types.NamespacedName) { + fs := &feastdevv1alpha1.FeatureStore{} + Expect(k8sClient.Get(ctx, key, fs)).To(Succeed()) + feast.Handler.FeatureStore = fs +} + +func applySpecToStatus(fs *feastdevv1alpha1.FeatureStore) { + fs.Status.Applied.Services = fs.Spec.Services.DeepCopy() + fs.Status.Applied.FeastProject = fs.Spec.FeastProject + Expect(k8sClient.Status().Update(context.Background(), fs)).To(Succeed()) +} + +var _ = Describe("Registry Service", func() { + var ( + featureStore *feastdevv1alpha1.FeatureStore + feast *FeastServices + typeNamespacedName types.NamespacedName + ctx context.Context + ) + + var setFeatureStoreServerConfig = func(grpcEnabled, restEnabled bool) { + featureStore.Spec.Services.Registry.Local.Server.GRPC = ptr(grpcEnabled) + featureStore.Spec.Services.Registry.Local.Server.RestAPI = ptr(restEnabled) + Expect(k8sClient.Update(ctx, featureStore)).To(Succeed()) + Expect(feast.ApplyDefaults()).To(Succeed()) + applySpecToStatus(featureStore) + feast.refreshFeatureStore(ctx, typeNamespacedName) + } + + BeforeEach(func() { + ctx = context.Background() + typeNamespacedName = types.NamespacedName{ + Name: "testfeaturestore", + Namespace: "default", + } + + featureStore = &feastdevv1alpha1.FeatureStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: typeNamespacedName.Name, + Namespace: typeNamespacedName.Namespace, + }, + Spec: feastdevv1alpha1.FeatureStoreSpec{ + FeastProject: "testproject", + Services: &feastdevv1alpha1.FeatureStoreServices{ + Registry: &feastdevv1alpha1.Registry{ + Local: &feastdevv1alpha1.LocalRegistryConfig{ + Server: &feastdevv1alpha1.RegistryServerConfigs{ + ServerConfigs: feastdevv1alpha1.ServerConfigs{ + ContainerConfigs: feastdevv1alpha1.ContainerConfigs{ + DefaultCtrConfigs: feastdevv1alpha1.DefaultCtrConfigs{ + Image: ptr("test-image"), + }, + }, + }, + GRPC: ptr(true), + RestAPI: ptr(false), + }, + }, + }, + }, + }, + } + + Expect(k8sClient.Create(ctx, featureStore)).To(Succeed()) + applySpecToStatus(featureStore) + + feast = &FeastServices{ + Handler: handler.FeastHandler{ + Client: k8sClient, + Context: ctx, + Scheme: k8sClient.Scheme(), + FeatureStore: featureStore, + }, + } + }) + + AfterEach(func() { + Expect(k8sClient.Delete(ctx, featureStore)).To(Succeed()) + }) + + Describe("Probe Handler Configuration", func() { + It("should configure TCP socket probe when gRPC is enabled", func() { + setFeatureStoreServerConfig(true, false) + probeHandler := feast.getProbeHandler(RegistryFeastType, featureStore.Spec.Services.Registry.Local.Server.TLS) + Expect(probeHandler.TCPSocket).NotTo(BeNil()) + Expect(probeHandler.TCPSocket.Port).To(Equal(intstr.FromInt(int(FeastServiceConstants[RegistryFeastType].TargetHttpPort)))) + }) + + It("should configure HTTP GET probe when REST is enabled", func() { + setFeatureStoreServerConfig(false, true) + probeHandler := feast.getProbeHandler(RegistryFeastType, featureStore.Spec.Services.Registry.Local.Server.TLS) + Expect(probeHandler.HTTPGet).NotTo(BeNil()) + Expect(probeHandler.HTTPGet.Port).To(Equal(intstr.FromInt(int(FeastServiceConstants[RegistryFeastType].TargetRestHttpPort)))) + }) + }) + + Describe("Registry Server Configuration", func() { + It("should enable both gRPC and REST", func() { + setFeatureStoreServerConfig(true, true) + Expect(feast.isRegistryGrpcEnabled()).To(BeTrue()) + Expect(feast.isRegistryRestEnabled()).To(BeTrue()) + }) + + It("should create both gRPC and REST services", func() { + setFeatureStoreServerConfig(true, true) + Expect(feast.deployFeastServiceByType(RegistryFeastType)).To(Succeed()) + Expect(feast.initFeastSvc(RegistryFeastType)).NotTo(BeNil()) + Expect(feast.initFeastRestSvc(RegistryFeastType)).NotTo(BeNil()) + }) + + It("should enable only gRPC", func() { + setFeatureStoreServerConfig(true, false) + Expect(feast.isRegistryGrpcEnabled()).To(BeTrue()) + Expect(feast.isRegistryRestEnabled()).To(BeFalse()) + }) + + It("should create only gRPC service and not REST service", func() { + setFeatureStoreServerConfig(true, false) + Expect(feast.deployFeastServiceByType(RegistryFeastType)).To(Succeed()) + Expect(feast.initFeastSvc(RegistryFeastType)).NotTo(BeNil()) + }) + }) + + Describe("Container Ports Configuration", func() { + It("should configure correct gRPC container ports", func() { + setFeatureStoreServerConfig(true, false) + Expect(feast.deployFeastServiceByType(RegistryFeastType)).To(Succeed()) + deployment := feast.initFeastDeploy() + Expect(deployment).NotTo(BeNil()) + Expect(feast.setDeployment(deployment)).To(Succeed()) + + ports := deployment.Spec.Template.Spec.Containers[0].Ports + Expect(ports).To(HaveLen(1)) + Expect(ports[0].ContainerPort).To(Equal(FeastServiceConstants[RegistryFeastType].TargetHttpPort)) + Expect(ports[0].Name).To(Equal(string(RegistryFeastType))) + }) + + It("should configure correct REST container ports", func() { + setFeatureStoreServerConfig(false, true) + Expect(feast.deployFeastServiceByType(RegistryFeastType)).To(Succeed()) + deployment := feast.initFeastDeploy() + Expect(deployment).NotTo(BeNil()) + Expect(feast.setDeployment(deployment)).To(Succeed()) + + ports := deployment.Spec.Template.Spec.Containers[0].Ports + Expect(ports).To(HaveLen(1)) + Expect(ports[0].ContainerPort).To(Equal(FeastServiceConstants[RegistryFeastType].TargetRestHttpPort)) + Expect(ports[0].Name).To(Equal(string(RegistryFeastType) + "-rest")) + + Expect(deployment.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deployment.Spec.Template.Spec.Containers[0].Ports).To(HaveLen(1)) + Expect(deployment.Spec.Template.Spec.Containers[0].Ports[0].ContainerPort).To(Equal(FeastServiceConstants[RegistryFeastType].TargetRestHttpPort)) + Expect(deployment.Spec.Template.Spec.Containers[0].Ports[0].Name).To(Equal(string(RegistryFeastType) + "-rest")) + }) + + It("should configure correct ports for both services", func() { + setFeatureStoreServerConfig(true, true) + Expect(feast.deployFeastServiceByType(RegistryFeastType)).To(Succeed()) + + deployment := feast.initFeastDeploy() + Expect(deployment).NotTo(BeNil()) + Expect(feast.setDeployment(deployment)).To(Succeed()) + + ports := deployment.Spec.Template.Spec.Containers[0].Ports + Expect(ports).To(HaveLen(2)) + Expect(ports[0].ContainerPort).To(Equal(FeastServiceConstants[RegistryFeastType].TargetHttpPort)) + Expect(ports[0].Name).To(Equal(string(RegistryFeastType))) + Expect(ports[1].ContainerPort).To(Equal(FeastServiceConstants[RegistryFeastType].TargetRestHttpPort)) + Expect(ports[1].Name).To(Equal(string(RegistryFeastType) + "-rest")) + }) + }) +})