From 796d66e83d2782055ca9673b09b221285aa09872 Mon Sep 17 00:00:00 2001 From: Nikolaus Schuetz Date: Wed, 8 Jul 2026 21:18:04 -0700 Subject: [PATCH 1/2] fix(operator): only deploy an online store when it is declared ApplyDefaultsToStatus created an online store service (including a serving pod) whenever spec.services.onlineStore was omitted, so the operator always deployed an online store even for a standalone registry or offline store. This contradicts the documented federated / shared-registry pattern and differs from how registry and offlineStore behave (both are only defaulted when declared). Gate the online-store defaulting behind `services.OnlineStore != nil`, mirroring registry and offlineStore. A declared online store still receives all of its defaults. Adds unit coverage for both cases. Closes #6586 Signed-off-by: Nikolaus Schuetz --- .../internal/controller/services/util.go | 39 +++++------ .../internal/controller/services/util_test.go | 64 +++++++++++++++++++ 2 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 infra/feast-operator/internal/controller/services/util_test.go diff --git a/infra/feast-operator/internal/controller/services/util.go b/infra/feast-operator/internal/controller/services/util.go index 387ccdb631b..3ca486e9deb 100644 --- a/infra/feast-operator/internal/controller/services/util.go +++ b/infra/feast-operator/internal/controller/services/util.go @@ -163,30 +163,31 @@ func ApplyDefaultsToStatus(cr *feastdevv1.FeatureStore) { } } - // default to onlineStore service deployment - if services.OnlineStore == nil { - services.OnlineStore = &feastdevv1.OnlineStore{} - } - if services.OnlineStore.Persistence == nil { - services.OnlineStore.Persistence = &feastdevv1.OnlineStorePersistence{} - } - - if services.OnlineStore.Persistence.DBPersistence == nil { - if services.OnlineStore.Persistence.FilePersistence == nil { - services.OnlineStore.Persistence.FilePersistence = &feastdevv1.OnlineStoreFilePersistence{} + // Only apply online store defaults when the online store is declared, so a + // FeatureStore can omit spec.services.onlineStore (mirroring registry and + // offlineStore) and not have an online store deployed. (#6586) + if services.OnlineStore != nil { + if services.OnlineStore.Persistence == nil { + services.OnlineStore.Persistence = &feastdevv1.OnlineStorePersistence{} } - if len(services.OnlineStore.Persistence.FilePersistence.Path) == 0 { - services.OnlineStore.Persistence.FilePersistence.Path = defaultOnlineStorePath(cr) - } + if services.OnlineStore.Persistence.DBPersistence == nil { + if services.OnlineStore.Persistence.FilePersistence == nil { + services.OnlineStore.Persistence.FilePersistence = &feastdevv1.OnlineStoreFilePersistence{} + } - ensurePVCDefaults(services.OnlineStore.Persistence.FilePersistence.PvcConfig, OnlineFeastType) - } + if len(services.OnlineStore.Persistence.FilePersistence.Path) == 0 { + services.OnlineStore.Persistence.FilePersistence.Path = defaultOnlineStorePath(cr) + } - if services.OnlineStore.Server == nil { - services.OnlineStore.Server = &feastdevv1.ServerConfigs{} + ensurePVCDefaults(services.OnlineStore.Persistence.FilePersistence.PvcConfig, OnlineFeastType) + } + + if services.OnlineStore.Server == nil { + services.OnlineStore.Server = &feastdevv1.ServerConfigs{} + } + setDefaultCtrConfigs(&services.OnlineStore.Server.ContainerConfigs.DefaultCtrConfigs) } - setDefaultCtrConfigs(&services.OnlineStore.Server.ContainerConfigs.DefaultCtrConfigs) if services.UI != nil { setDefaultCtrConfigs(&services.UI.ContainerConfigs.DefaultCtrConfigs) diff --git a/infra/feast-operator/internal/controller/services/util_test.go b/infra/feast-operator/internal/controller/services/util_test.go new file mode 100644 index 00000000000..c4641ee172e --- /dev/null +++ b/infra/feast-operator/internal/controller/services/util_test.go @@ -0,0 +1,64 @@ +/* +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 ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + feastdevv1 "github.com/feast-dev/feast/infra/feast-operator/api/v1" +) + +var _ = Describe("ApplyDefaultsToStatus", func() { + // Regression test for #6586: the operator must only deploy the services a + // FeatureStore declares. An online store should not be created when it is + // not specified, mirroring registry and offlineStore. + It("does not create an online store when it is not declared", func() { + cr := &feastdevv1.FeatureStore{ + Spec: feastdevv1.FeatureStoreSpec{ + FeastProject: "test_project", + Services: &feastdevv1.FeatureStoreServices{}, + }, + } + + ApplyDefaultsToStatus(cr) + + applied := cr.Status.Applied.Services + Expect(applied).ToNot(BeNil()) + Expect(applied.OnlineStore).To(BeNil(), "online store should not be created when not declared") + Expect(applied.OfflineStore).To(BeNil()) + Expect(applied.Registry).To(BeNil()) + }) + + It("applies online store defaults when it is declared", func() { + cr := &feastdevv1.FeatureStore{ + Spec: feastdevv1.FeatureStoreSpec{ + FeastProject: "test_project", + Services: &feastdevv1.FeatureStoreServices{ + OnlineStore: &feastdevv1.OnlineStore{}, + }, + }, + } + + ApplyDefaultsToStatus(cr) + + online := cr.Status.Applied.Services.OnlineStore + Expect(online).ToNot(BeNil()) + Expect(online.Persistence).ToNot(BeNil(), "a declared online store should still receive its defaults") + Expect(online.Server).ToNot(BeNil()) + }) +}) From 4a735a8138789abc3facfad7f8bedff6d5222329 Mon Sep 17 00:00:00 2001 From: Nikolaus Schuetz Date: Wed, 8 Jul 2026 22:07:40 -0700 Subject: [PATCH 2/2] test(operator): declare online store explicitly in tests that relied on the implicit default The operator no longer creates an online store when a FeatureStore omits spec.services.onlineStore (#6586). These existing tests built CRs that declared other services (or nothing) and relied on the online store being created implicitly, so they now either fail the "at least one local server" guard or find no online container/service. Declare the online store explicitly where the test intent is to exercise an online store, and assert its absence in the registry+offlineStore case so it also serves as a regression check for #6586. Signed-off-by: Nikolaus Schuetz --- .../featurestore_controller_cronjob_test.go | 9 ++++++++- .../featurestore_controller_loglevel_test.go | 1 + .../controller/featurestore_controller_test.go | 12 +++++++++++- .../internal/controller/services/repo_config_test.go | 9 +++++++-- .../internal/controller/services/tls_test.go | 3 +++ 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/infra/feast-operator/internal/controller/featurestore_controller_cronjob_test.go b/infra/feast-operator/internal/controller/featurestore_controller_cronjob_test.go index 11ae2af7777..246da77ff47 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_cronjob_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_cronjob_test.go @@ -56,7 +56,14 @@ var _ = Describe("FeatureStore Controller - Feast CronJob", func() { Name: resourceName, Namespace: "default", }, - Spec: feastdevv1.FeatureStoreSpec{FeastProject: feastProject}, + Spec: feastdevv1.FeatureStoreSpec{ + FeastProject: feastProject, + // #6586: an online store is no longer created implicitly, + // so declare one explicitly to keep this a valid CR. + Services: &feastdevv1.FeatureStoreServices{ + OnlineStore: &feastdevv1.OnlineStore{}, + }, + }, } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } 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 948ddec210d..aec5f0073cd 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go @@ -203,6 +203,7 @@ var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() { }, }, }, + OnlineStore: &feastdevv1.OnlineStore{}, OfflineStore: &feastdevv1.OfflineStore{}, UI: &feastdevv1.ServerConfigs{}, } diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index a9ac5235eda..c6d616ed0c6 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -77,7 +77,14 @@ var _ = Describe("FeatureStore Controller", func() { Name: resourceName, Namespace: "default", }, - Spec: feastdevv1.FeatureStoreSpec{FeastProject: feastProject}, + Spec: feastdevv1.FeatureStoreSpec{ + FeastProject: feastProject, + // #6586: an online store is no longer created implicitly, + // so declare one explicitly to keep this a valid CR. + Services: &feastdevv1.FeatureStoreServices{ + OnlineStore: &feastdevv1.OnlineStore{}, + }, + }, } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) } @@ -375,6 +382,9 @@ var _ = Describe("FeatureStore Controller", func() { resourceNew := resource.DeepCopy() resourceNew.Spec.FeastProject = "changed" resourceNew.Spec.Services = &feastdevv1.FeatureStoreServices{ + // #6586: declare the online store explicitly; it is no longer + // created implicitly for a service block that omits it. + OnlineStore: &feastdevv1.OnlineStore{}, DeploymentStrategy: &appsv1.DeploymentStrategy{ Type: appsv1.RollingUpdateDeploymentStrategyType, }, 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 5ae22a795f6..3f8f5687323 100644 --- a/infra/feast-operator/internal/controller/services/repo_config_test.go +++ b/infra/feast-operator/internal/controller/services/repo_config_test.go @@ -95,7 +95,10 @@ var _ = Describe("Repo Config", func() { } ApplyDefaultsToStatus(featureStore) appliedServices := featureStore.Status.Applied.Services - Expect(appliedServices.OnlineStore).NotTo(BeNil()) + // #6586: declaring registry + offlineStore (but not onlineStore) must + // not deploy an online store service. The client repo config still + // defaults to a sqlite online store (asserted below). + Expect(appliedServices.OnlineStore).To(BeNil()) Expect(appliedServices.Registry.Local).NotTo(BeNil()) repoConfig, err = getServiceRepoConfig(featureStore, emptyMockExtractConfigFromSecret, emptyMockExtractConfigFromConfigMap, false) @@ -741,8 +744,10 @@ func minimalFeatureStore() *feastdevv1.FeatureStore { func minimalFeatureStoreWithAllServers() *feastdevv1.FeatureStore { feast := minimalFeatureStore() - // onlineStore configured by default + // #6586: the online store is no longer configured implicitly, so declare + // it explicitly here so this helper truly deploys all servers. feast.Spec.Services = &feastdevv1.FeatureStoreServices{ + OnlineStore: &feastdevv1.OnlineStore{}, OfflineStore: &feastdevv1.OfflineStore{ Server: &feastdevv1.ServerConfigs{}, }, diff --git a/infra/feast-operator/internal/controller/services/tls_test.go b/infra/feast-operator/internal/controller/services/tls_test.go index 7f1c94789bc..fd85e29193c 100644 --- a/infra/feast-operator/internal/controller/services/tls_test.go +++ b/infra/feast-operator/internal/controller/services/tls_test.go @@ -85,6 +85,9 @@ var _ = Describe("TLS Config", func() { testSetIsOpenShift() feast.Handler.FeatureStore = minimalFeatureStore() feast.Handler.FeatureStore.Spec.Services = &feastdevv1.FeatureStoreServices{ + // #6586: declare the online store explicitly so this case can + // still exercise online TLS defaulting; it is no longer implicit. + OnlineStore: &feastdevv1.OnlineStore{}, Registry: &feastdevv1.Registry{ Local: &feastdevv1.LocalRegistryConfig{ Server: &feastdevv1.RegistryServerConfigs{