Skip to content

Commit

Permalink
fix: Improve status.applied updates & add offline pvc unit test (#4871)
Browse files Browse the repository at this point in the history 
improve status.applied update & add offline pvc unit test

Signed-off-by: Tommy Hughes <[email protected]>
  • Loading branch information
@tchughesiv
tchughesiv authored Dec 23, 2024
1 parent 4de187d commit 3f49517
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 38 deletions.
4 changes: 2 additions & 2 deletions infra/feast-operator/internal/controller/services/repo_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,17 @@ func getBaseServiceRepoConfig(
featureStore *feastdevv1alpha1.FeatureStore,
secretExtractionFunc func(storeType string, secretRef string, secretKeyName string) (map[string]interface{}, error)) (RepoConfig, error) {

appliedSpec := featureStore.Status.Applied
repoConfig := defaultRepoConfig(featureStore)
clientRepoConfig, err := getClientRepoConfig(featureStore, secretExtractionFunc)
if err != nil {
return repoConfig, err
}
repoConfig.AuthzConfig = clientRepoConfig.AuthzConfig
if isRemoteRegistry(featureStore) {
repoConfig.Registry = clientRepoConfig.Registry
}
repoConfig.AuthzConfig = clientRepoConfig.AuthzConfig

appliedSpec := featureStore.Status.Applied
if appliedSpec.AuthzConfig != nil && appliedSpec.AuthzConfig.OidcAuthz != nil {
propertiesMap, authSecretErr := secretExtractionFunc("", appliedSpec.AuthzConfig.OidcAuthz.SecretRef.Name, "")
if authSecretErr != nil {
Expand Down
30 changes: 29 additions & 1 deletion infra/feast-operator/internal/controller/services/repo_config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ var _ = Describe("Repo Config", func() {
Path: EphemeralPath + "/" + DefaultOnlineStorePath,
}

var repoConfig RepoConfig
repoConfig, err := getServiceRepoConfig(featureStore, emptyMockExtractConfigFromSecret)
Expect(err).NotTo(HaveOccurred())
Expect(repoConfig.AuthzConfig.Type).To(Equal(NoAuthAuthType))
Expand Down Expand Up @@ -100,6 +99,35 @@ var _ = Describe("Repo Config", func() {
Expect(repoConfig.OnlineStore).To(Equal(expectedOnlineConfig))
Expect(repoConfig.Registry).To(Equal(emptyRegistryConfig))

By("Having an offlineStore with PVC")
mountPath := "/testing"
expectedOnlineConfig.Path = mountPath + "/" + DefaultOnlineStorePath
expectedRegistryConfig.Path = mountPath + "/" + DefaultRegistryPath

featureStore = minimalFeatureStore()
featureStore.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{
OfflineStore: &feastdevv1alpha1.OfflineStore{
Persistence: &feastdevv1alpha1.OfflineStorePersistence{
FilePersistence: &feastdevv1alpha1.OfflineStoreFilePersistence{
PvcConfig: &feastdevv1alpha1.PvcConfig{
MountPath: mountPath,
},
},
},
},
}
ApplyDefaultsToStatus(featureStore)
appliedServices := featureStore.Status.Applied.Services
Expect(appliedServices.OnlineStore).To(BeNil())
Expect(appliedServices.Registry.Local.Persistence.FilePersistence.Path).To(Equal(expectedRegistryConfig.Path))

repoConfig, err = getServiceRepoConfig(featureStore, emptyMockExtractConfigFromSecret)
Expect(err).NotTo(HaveOccurred())
Expect(repoConfig.OfflineStore).To(Equal(defaultOfflineStoreConfig))
Expect(repoConfig.AuthzConfig.Type).To(Equal(NoAuthAuthType))
Expect(repoConfig.Registry).To(Equal(expectedRegistryConfig))
Expect(repoConfig.OnlineStore).To(Equal(expectedOnlineConfig))

By("Having the all the file services")
featureStore = minimalFeatureStore()
featureStore.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{
Expand Down
64 changes: 29 additions & 35 deletions infra/feast-operator/internal/controller/services/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,18 @@ func hasPvcConfig(featureStore *feastdevv1alpha1.FeatureStore, feastType FeastSe
if services != nil {
switch feastType {
case OnlineFeastType:
if services.OnlineStore != nil && services.OnlineStore.Persistence.FilePersistence != nil {
if services.OnlineStore != nil && services.OnlineStore.Persistence != nil &&
services.OnlineStore.Persistence.FilePersistence != nil {
pvcConfig = services.OnlineStore.Persistence.FilePersistence.PvcConfig
}
case OfflineFeastType:
if services.OfflineStore != nil && services.OfflineStore.Persistence.FilePersistence != nil {
if services.OfflineStore != nil && services.OfflineStore.Persistence != nil &&
services.OfflineStore.Persistence.FilePersistence != nil {
pvcConfig = services.OfflineStore.Persistence.FilePersistence.PvcConfig
}
case RegistryFeastType:
if IsLocalRegistry(featureStore) && services.Registry.Local.Persistence.FilePersistence != nil {
if IsLocalRegistry(featureStore) && services.Registry.Local.Persistence != nil &&
services.Registry.Local.Persistence.FilePersistence != nil {
pvcConfig = services.Registry.Local.Persistence.FilePersistence.PvcConfig
}
}
Expand All @@ -66,18 +69,18 @@ func shouldMountEmptyDir(featureStore *feastdevv1alpha1.FeatureStore) bool {
}

func getOfflineMountPath(featureStore *feastdevv1alpha1.FeatureStore) string {
if featureStore.Status.Applied.Services != nil {
if pvcConfig, ok := hasPvcConfig(featureStore, OfflineFeastType); ok {
return pvcConfig.MountPath
}
if pvcConfig, ok := hasPvcConfig(featureStore, OfflineFeastType); ok {
return pvcConfig.MountPath
}
return EphemeralPath
}

func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) {
// overwrite status.applied with every reconcile
cr.Spec.DeepCopyInto(&cr.Status.Applied)
cr.Status.FeastVersion = feastversion.FeastVersion
applied := cr.Spec.DeepCopy()

applied := &cr.Status.Applied
if applied.Services == nil {
applied.Services = &feastdevv1alpha1.FeatureStoreServices{}
}
Expand Down Expand Up @@ -106,10 +109,7 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) {
services.Registry.Local.Persistence.FilePersistence.Path = defaultRegistryPath(cr)
}

if services.Registry.Local.Persistence.FilePersistence.PvcConfig != nil {
pvc := services.Registry.Local.Persistence.FilePersistence.PvcConfig
ensurePVCDefaults(pvc, RegistryFeastType)
}
ensurePVCDefaults(services.Registry.Local.Persistence.FilePersistence.PvcConfig, RegistryFeastType)
}

setServiceDefaultConfigs(&services.Registry.Local.ServiceConfigs.DefaultConfigs)
Expand All @@ -131,10 +131,7 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) {
services.OfflineStore.Persistence.FilePersistence.Type = string(OfflineFilePersistenceDaskConfigType)
}

if services.OfflineStore.Persistence.FilePersistence.PvcConfig != nil {
pvc := services.OfflineStore.Persistence.FilePersistence.PvcConfig
ensurePVCDefaults(pvc, OfflineFeastType)
}
ensurePVCDefaults(services.OfflineStore.Persistence.FilePersistence.PvcConfig, OfflineFeastType)
}

setServiceDefaultConfigs(&services.OfflineStore.ServiceConfigs.DefaultConfigs)
Expand All @@ -154,16 +151,11 @@ func ApplyDefaultsToStatus(cr *feastdevv1alpha1.FeatureStore) {
services.OnlineStore.Persistence.FilePersistence.Path = defaultOnlineStorePath(cr)
}

if services.OnlineStore.Persistence.FilePersistence.PvcConfig != nil {
pvc := services.OnlineStore.Persistence.FilePersistence.PvcConfig
ensurePVCDefaults(pvc, OnlineFeastType)
}
ensurePVCDefaults(services.OnlineStore.Persistence.FilePersistence.PvcConfig, OnlineFeastType)
}

setServiceDefaultConfigs(&services.OnlineStore.ServiceConfigs.DefaultConfigs)
}
// overwrite status.applied with every reconcile
applied.DeepCopyInto(&cr.Status.Applied)
}

func setServiceDefaultConfigs(defaultConfigs *feastdevv1alpha1.DefaultConfigs) {
Expand All @@ -189,19 +181,21 @@ func ensureRequestedStorage(resources *v1.VolumeResourceRequirements, requestedS
}

func ensurePVCDefaults(pvc *feastdevv1alpha1.PvcConfig, feastType FeastServiceType) {
var storageRequest string
switch feastType {
case OnlineFeastType:
storageRequest = DefaultOnlineStorageRequest
case OfflineFeastType:
storageRequest = DefaultOfflineStorageRequest
case RegistryFeastType:
storageRequest = DefaultRegistryStorageRequest
}
if pvc.Create != nil {
ensureRequestedStorage(&pvc.Create.Resources, storageRequest)
if pvc.Create.AccessModes == nil {
pvc.Create.AccessModes = DefaultPVCAccessModes
if pvc != nil {
var storageRequest string
switch feastType {
case OnlineFeastType:
storageRequest = DefaultOnlineStorageRequest
case OfflineFeastType:
storageRequest = DefaultOfflineStorageRequest
case RegistryFeastType:
storageRequest = DefaultRegistryStorageRequest
}
if pvc.Create != nil {
ensureRequestedStorage(&pvc.Create.Resources, storageRequest)
if pvc.Create.AccessModes == nil {
pvc.Create.AccessModes = DefaultPVCAccessModes
}
}
}
}
Expand Down

0 comments on commit 3f49517

Please sign in to comment.