Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ var _ = Describe("FeatureStore Controller - Feast service LogLevel", func() {
},
},
},
OnlineStore: &feastdevv1.OnlineStore{},
OfflineStore: &feastdevv1.OfflineStore{},
UI: &feastdevv1.ServerConfigs{},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
39 changes: 20 additions & 19 deletions infra/feast-operator/internal/controller/services/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
64 changes: 64 additions & 0 deletions infra/feast-operator/internal/controller/services/util_test.go
Original file line number Diff line number Diff line change
@@ -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())
})
})
Loading