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{ 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()) + }) +})