fix: Made fixes to Go Operator DB persistence (#4830)

* Made fixes to Go Operator DB persistence Signed-off-by: Theodor Mihalache <[email protected]> * Fixes following review Signed-off-by: Theodor Mihalache <[email protected]> --------- Signed-off-by: Theodor Mihalache <[email protected]>
feast-dev · Dec 11, 2024 · cdc0753 · cdc0753
1 parent 732865f
commit cdc0753
Show file tree

Hide file tree

Showing 7 changed files with 128 additions and 94 deletions.
diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go
@@ -99,19 +99,20 @@ type OfflineStorePersistence struct {
 
 // OfflineStoreFilePersistence configures the file-based persistence for the offline store service
 type OfflineStoreFilePersistence struct {
-	// +kubebuilder:validation:Enum=dask;duckdb
+	// +kubebuilder:validation:Enum=file;dask;duckdb
 	Type      string     `json:"type,omitempty"`
 	PvcConfig *PvcConfig `json:"pvc,omitempty"`
 }
 
 var ValidOfflineStoreFilePersistenceTypes = []string{
 	"dask",
 	"duckdb",
+	"file",
 }
 
 // OfflineStoreDBStorePersistence configures the DB store persistence for the offline store service
 type OfflineStoreDBStorePersistence struct {
-	// +kubebuilder:validation:Enum=snowflake.offline;bigquery;redshift;spark;postgres;feast_trino.trino.TrinoOfflineStore;redis
+	// +kubebuilder:validation:Enum=snowflake.offline;bigquery;redshift;spark;postgres;trino;redis;athena;mssql
 	Type string `json:"type"`
 	// Data store parameters should be placed as-is from the "feature_store.yaml" under the secret key. "registry_type" & "type" fields should be removed.
 	SecretRef corev1.LocalObjectReference `json:"secretRef"`
@@ -125,8 +126,10 @@ var ValidOfflineStoreDBStorePersistenceTypes = []string{
 	"redshift",
 	"spark",
 	"postgres",
-	"feast_trino.trino.TrinoOfflineStore",
+	"trino",
 	"redis",
+	"athena",
+	"mssql",
 }
 
 // OnlineStore configures the deployed online store service
@@ -158,7 +161,7 @@ type OnlineStoreFilePersistence struct {
 
 // OnlineStoreDBStorePersistence configures the DB store persistence for the offline store service
 type OnlineStoreDBStorePersistence struct {
-	// +kubebuilder:validation:Enum=snowflake.online;redis;ikv;datastore;dynamodb;bigtable;postgres;cassandra;mysql;hazelcast;singlestore
+	// +kubebuilder:validation:Enum=snowflake.online;redis;ikv;datastore;dynamodb;bigtable;postgres;cassandra;mysql;hazelcast;singlestore;hbase;elasticsearch;qdrant;couchbase
 	Type string `json:"type"`
 	// Data store parameters should be placed as-is from the "feature_store.yaml" under the secret key. "registry_type" & "type" fields should be removed.
 	SecretRef corev1.LocalObjectReference `json:"secretRef"`
@@ -178,6 +181,10 @@ var ValidOnlineStoreDBStorePersistenceTypes = []string{
 	"mysql",
 	"hazelcast",
 	"singlestore",
+	"hbase",
+	"elasticsearch",
+	"qdrant",
+	"couchbase",
 }
 
 // LocalRegistryConfig configures the deployed registry service

diff --git a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml
@@ -323,6 +323,7 @@ spec:
                                   rule: self.mountPath.matches('^/[^:]*$')
                               type:
                                 enum:
+                                - file
                                 - dask
                                 - duckdb
                                 type: string
@@ -355,8 +356,10 @@ spec:
                                 - redshift
                                 - spark
                                 - postgres
-                                - feast_trino.trino.TrinoOfflineStore
+                                - trino
                                 - redis
+                                - athena
+                                - mssql
                                 type: string
                             required:
                             - secretRef
@@ -729,6 +732,10 @@ spec:
                                 - mysql
                                 - hazelcast
                                 - singlestore
+                                - hbase
+                                - elasticsearch
+                                - qdrant
+                                - couchbase
                                 type: string
                             required:
                             - secretRef
@@ -1559,6 +1566,7 @@ spec:
                                       rule: self.mountPath.matches('^/[^:]*$')
                                   type:
                                     enum:
+                                    - file
                                     - dask
                                     - duckdb
                                     type: string
@@ -1592,8 +1600,10 @@ spec:
                                     - redshift
                                     - spark
                                     - postgres
-                                    - feast_trino.trino.TrinoOfflineStore
+                                    - trino
                                     - redis
+                                    - athena
+                                    - mssql
                                     type: string
                                 required:
                                 - secretRef
@@ -1973,6 +1983,10 @@ spec:
                                     - mysql
                                     - hazelcast
                                     - singlestore
+                                    - hbase
+                                    - elasticsearch
+                                    - qdrant
+                                    - couchbase
                                     type: string
                                 required:
                                 - secretRef

diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml
@@ -331,6 +331,7 @@ spec:
                                   rule: self.mountPath.matches('^/[^:]*$')
                               type:
                                 enum:
+                                - file
                                 - dask
                                 - duckdb
                                 type: string
@@ -363,8 +364,10 @@ spec:
                                 - redshift
                                 - spark
                                 - postgres
-                                - feast_trino.trino.TrinoOfflineStore
+                                - trino
                                 - redis
+                                - athena
+                                - mssql
                                 type: string
                             required:
                             - secretRef
@@ -737,6 +740,10 @@ spec:
                                 - mysql
                                 - hazelcast
                                 - singlestore
+                                - hbase
+                                - elasticsearch
+                                - qdrant
+                                - couchbase
                                 type: string
                             required:
                             - secretRef
@@ -1567,6 +1574,7 @@ spec:
                                       rule: self.mountPath.matches('^/[^:]*$')
                                   type:
                                     enum:
+                                    - file
                                     - dask
                                     - duckdb
                                     type: string
@@ -1600,8 +1608,10 @@ spec:
                                     - redshift
                                     - spark
                                     - postgres
-                                    - feast_trino.trino.TrinoOfflineStore
+                                    - trino
                                     - redis
+                                    - athena
+                                    - mssql
                                     type: string
                                 required:
                                 - secretRef
@@ -1981,6 +1991,10 @@ spec:
                                     - mysql
                                     - hazelcast
                                     - singlestore
+                                    - hbase
+                                    - elasticsearch
+                                    - qdrant
+                                    - couchbase
                                     type: string
                                 required:
                                 - secretRef

diff --git a/infra/feast-operator/internal/controller/featurestore_controller_db_store_test.go b/infra/feast-operator/internal/controller/featurestore_controller_db_store_test.go
@@ -78,7 +78,7 @@ sqlalchemy_config_kwargs:
   pool_pre_ping: true
 `
 
-var invalidSecretContainingTypeYamlString = `
+var secretContainingValidTypeYamlString = `
 type: cassandra
 hosts:
   - 192.168.1.1
@@ -305,37 +305,12 @@ var _ = Describe("FeatureStore Controller - db storage services", func() {
 
 			Expect(err.Error()).To(Equal("secret key invalid.secret.key doesn't exist in secret online-store-secret"))
 
-			By("Referring to a secret that contains parameter named type")
-			resource = &feastdevv1alpha1.FeatureStore{}
-			err = k8sClient.Get(ctx, typeNamespacedName, resource)
-			Expect(err).NotTo(HaveOccurred())
-
-			secret := &corev1.Secret{}
-			err = k8sClient.Get(ctx, onlineSecretNamespacedName, secret)
-			Expect(err).NotTo(HaveOccurred())
-			secret.Data[string(services.OnlineDBPersistenceCassandraConfigType)] = []byte(invalidSecretContainingTypeYamlString)
-			Expect(k8sClient.Update(ctx, secret)).To(Succeed())
-
-			resource.Spec.Services.OnlineStore.Persistence.DBPersistence.SecretRef = corev1.LocalObjectReference{Name: "online-store-secret"}
-			resource.Spec.Services.OnlineStore.Persistence.DBPersistence.SecretKeyName = ""
-			Expect(k8sClient.Update(ctx, resource)).To(Succeed())
-			resource = &feastdevv1alpha1.FeatureStore{}
-			err = k8sClient.Get(ctx, typeNamespacedName, resource)
-			Expect(err).NotTo(HaveOccurred())
-
-			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
-				NamespacedName: typeNamespacedName,
-			})
-			Expect(err).To(HaveOccurred())
-
-			Expect(err.Error()).To(Equal("secret key cassandra in secret online-store-secret contains invalid tag named type"))
-
 			By("Referring to a secret that contains parameter named type with invalid value")
 			resource = &feastdevv1alpha1.FeatureStore{}
 			err = k8sClient.Get(ctx, typeNamespacedName, resource)
 			Expect(err).NotTo(HaveOccurred())
 
-			secret = &corev1.Secret{}
+			secret := &corev1.Secret{}
 			err = k8sClient.Get(ctx, onlineSecretNamespacedName, secret)
 			Expect(err).NotTo(HaveOccurred())
 			secret.Data[string(services.OnlineDBPersistenceCassandraConfigType)] = []byte(invalidSecretTypeYamlString)
@@ -353,39 +328,7 @@ var _ = Describe("FeatureStore Controller - db storage services", func() {
 			})
 			Expect(err).To(HaveOccurred())
 
-			Expect(err.Error()).To(Equal("secret key cassandra in secret online-store-secret contains invalid tag named type"))
-
-			By("Referring to a secret that contains parameter named registry_type")
-			resource = &feastdevv1alpha1.FeatureStore{}
-			err = k8sClient.Get(ctx, typeNamespacedName, resource)
-			Expect(err).NotTo(HaveOccurred())
-
-			secret = &corev1.Secret{}
-			err = k8sClient.Get(ctx, onlineSecretNamespacedName, secret)
-			Expect(err).NotTo(HaveOccurred())
-			secret.Data[string(services.OnlineDBPersistenceCassandraConfigType)] = []byte(cassandraYamlString)
-			Expect(k8sClient.Update(ctx, secret)).To(Succeed())
-
-			secret = &corev1.Secret{}
-			err = k8sClient.Get(ctx, registrySecretNamespacedName, secret)
-			Expect(err).NotTo(HaveOccurred())
-			secret.Data["sql_custom_registry_key"] = nil
-			secret.Data[string(services.RegistryDBPersistenceSQLConfigType)] = []byte(invalidSecretRegistryTypeYamlString)
-			Expect(k8sClient.Update(ctx, secret)).To(Succeed())
-
-			resource.Spec.Services.Registry.Local.Persistence.DBPersistence.SecretRef = corev1.LocalObjectReference{Name: "registry-store-secret"}
-			resource.Spec.Services.Registry.Local.Persistence.DBPersistence.SecretKeyName = ""
-			Expect(k8sClient.Update(ctx, resource)).To(Succeed())
-			resource = &feastdevv1alpha1.FeatureStore{}
-			err = k8sClient.Get(ctx, typeNamespacedName, resource)
-			Expect(err).NotTo(HaveOccurred())
-
-			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
-				NamespacedName: typeNamespacedName,
-			})
-			Expect(err).To(HaveOccurred())
-
-			Expect(err.Error()).To(Equal("secret key sql in secret registry-store-secret contains invalid tag named registry_type"))
+			Expect(err.Error()).To(Equal("secret key cassandra in secret online-store-secret contains tag named type with value wrong"))
 		})
 
 		It("should successfully reconcile the resource", func() {
@@ -506,6 +449,60 @@ var _ = Describe("FeatureStore Controller - db storage services", func() {
 			Expect(err).NotTo(HaveOccurred())
 			Expect(controllerutil.HasControllerReference(svc)).To(BeTrue())
 			Expect(svc.Spec.Ports[0].TargetPort).To(Equal(intstr.FromInt(int(services.FeastServiceConstants[services.RegistryFeastType].TargetHttpPort))))
+
+			By("Referring to a secret that contains parameter named type")
+			resource = &feastdevv1alpha1.FeatureStore{}
+			err = k8sClient.Get(ctx, typeNamespacedName, resource)
+			Expect(err).NotTo(HaveOccurred())
+
+			secret := &corev1.Secret{}
+			err = k8sClient.Get(ctx, onlineSecretNamespacedName, secret)
+			Expect(err).NotTo(HaveOccurred())
+			secret.Data[string(services.OnlineDBPersistenceCassandraConfigType)] = []byte(secretContainingValidTypeYamlString)
+			Expect(k8sClient.Update(ctx, secret)).To(Succeed())
+
+			resource.Spec.Services.OnlineStore.Persistence.DBPersistence.SecretRef = corev1.LocalObjectReference{Name: "online-store-secret"}
+			resource.Spec.Services.OnlineStore.Persistence.DBPersistence.SecretKeyName = ""
+			Expect(k8sClient.Update(ctx, resource)).To(Succeed())
+			resource = &feastdevv1alpha1.FeatureStore{}
+			err = k8sClient.Get(ctx, typeNamespacedName, resource)
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: typeNamespacedName,
+			})
+
+			Expect(err).To(Not(HaveOccurred()))
+
+			By("Referring to a secret that contains parameter named registry_type")
+			resource = &feastdevv1alpha1.FeatureStore{}
+			err = k8sClient.Get(ctx, typeNamespacedName, resource)
+			Expect(err).NotTo(HaveOccurred())
+
+			secret = &corev1.Secret{}
+			err = k8sClient.Get(ctx, onlineSecretNamespacedName, secret)
+			Expect(err).NotTo(HaveOccurred())
+			secret.Data[string(services.OnlineDBPersistenceCassandraConfigType)] = []byte(cassandraYamlString)
+			Expect(k8sClient.Update(ctx, secret)).To(Succeed())
+
+			secret = &corev1.Secret{}
+			err = k8sClient.Get(ctx, registrySecretNamespacedName, secret)
+			Expect(err).NotTo(HaveOccurred())
+			secret.Data["sql_custom_registry_key"] = nil
+			secret.Data[string(services.RegistryDBPersistenceSQLConfigType)] = []byte(invalidSecretRegistryTypeYamlString)
+			Expect(k8sClient.Update(ctx, secret)).To(Succeed())
+
+			resource.Spec.Services.Registry.Local.Persistence.DBPersistence.SecretRef = corev1.LocalObjectReference{Name: "registry-store-secret"}
+			resource.Spec.Services.Registry.Local.Persistence.DBPersistence.SecretKeyName = ""
+			Expect(k8sClient.Update(ctx, resource)).To(Succeed())
+			resource = &feastdevv1alpha1.FeatureStore{}
+			err = k8sClient.Get(ctx, typeNamespacedName, resource)
+			Expect(err).NotTo(HaveOccurred())
+
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
+				NamespacedName: typeNamespacedName,
+			})
+			Expect(err).To(Not(HaveOccurred()))
 		})
 
 		It("should properly encode a feature_store.yaml config", func() {