Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Operator / Feast should have matching Data Source types #5041

Merged
merged 1 commit into from
Feb 10, 2025
Merged
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
4 changes: 1 addition & 3 deletions .github/workflows/operator_pr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ jobs:
with:
go-version: 1.22.9
- name: Operator tests
run: |
cd infra/feast-operator/
make test
run: make -C infra/feast-operator test
- name: After code formatting, check for uncommitted differences
run: git diff --exit-code infra/feast-operator
6 changes: 6 additions & 0 deletions .github/workflows/pr_local_integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,9 @@ jobs:
- name: Test local integration tests
if: ${{ always() }} # this will guarantee that step won't be canceled and resources won't leak
run: make test-python-integration-local
- name: Install Go
uses: actions/setup-go@v5
with:
go-version: 1.22.9
- name: Operator Data Source types test
run: make -C infra/feast-operator test-datasources
3 changes: 3 additions & 0 deletions infra/feast-operator/.golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ issues:
linters:
- dupl
- lll
- path: "test/*"
linters:
- lll
linters:
disable-all: true
enable:
Expand Down
8 changes: 7 additions & 1 deletion infra/feast-operator/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,19 @@ vet: ## Run go vet against code.

.PHONY: test
test: build-installer vet lint envtest ## Run tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v test/e2e | grep -v test/data-source-types) -coverprofile cover.out

# Utilize Kind or modify the e2e tests to load the image locally, enabling compatibility with other vendors.
.PHONY: test-e2e # Run the e2e tests against a Kind k8s instance that is spun up.
test-e2e:
go test -timeout 30m ./test/e2e/ -v -ginkgo.v

# Requires python3
.PHONY: test-datasources
test-datasources:
python3 test/data-source-types/data-source-types.py
go test ./test/data-source-types/

.PHONY: lint
lint: golangci-lint ## Run golangci-lint linter & yamllint
$(GOLANGCI_LINT) run
Expand Down
11 changes: 5 additions & 6 deletions infra/feast-operator/api/v1alpha1/featurestore_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ var ValidOfflineStoreFilePersistenceTypes = []string{

// OfflineStoreDBStorePersistence configures the DB store persistence for the offline store service
type OfflineStoreDBStorePersistence struct {
// Type of the persistence type you want to use. Allowed values are: snowflake.offline, bigquery, redshift, spark, postgres, trino, redis, athena, mssql
// +kubebuilder:validation:Enum=snowflake.offline;bigquery;redshift;spark;postgres;trino;redis;athena;mssql
// Type of the persistence type you want to use.
// +kubebuilder:validation:Enum=snowflake.offline;bigquery;redshift;spark;postgres;trino;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"`
Expand All @@ -130,7 +130,6 @@ var ValidOfflineStoreDBStorePersistenceTypes = []string{
"spark",
"postgres",
"trino",
"redis",
"athena",
"mssql",
}
Expand Down Expand Up @@ -160,7 +159,7 @@ type OnlineStoreFilePersistence struct {

// OnlineStoreDBStorePersistence configures the DB store persistence for the online store service
type OnlineStoreDBStorePersistence struct {
// Type of the persistence type you want to use. Allowed values are: snowflake.online, redis, ikv, datastore, dynamodb, bigtable, postgres, cassandra, mysql, hazelcast, singlestore, hbase, elasticsearch, qdrant, couchbase, milvus
// Type of the persistence type you want to use.
redhatHameed marked this conversation as resolved.
Show resolved Hide resolved
// +kubebuilder:validation:Enum=snowflake.online;redis;ikv;datastore;dynamodb;bigtable;postgres;cassandra;mysql;hazelcast;singlestore;hbase;elasticsearch;qdrant;couchbase;milvus
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.
Expand Down Expand Up @@ -215,7 +214,7 @@ type RegistryFilePersistence struct {

// RegistryDBStorePersistence configures the DB store persistence for the registry service
type RegistryDBStorePersistence struct {
// Type of the persistence type you want to use. Allowed values are: sql, snowflake.registry
// Type of the persistence type you want to use.
// +kubebuilder:validation:Enum=sql;snowflake.registry
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.
Expand All @@ -225,8 +224,8 @@ type RegistryDBStorePersistence struct {
}

var ValidRegistryDBStorePersistenceTypes = []string{
"snowflake.registry",
"sql",
"snowflake.registry",
}

// PvcConfig defines the settings for a persistent file store based on PVCs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ spec:
- spark
- postgres
- trino
- redis
- athena
- mssql
type: string
Expand Down Expand Up @@ -2083,7 +2082,6 @@ spec:
- spark
- postgres
- trino
- redis
- athena
- mssql
type: string
Expand Down
2 changes: 0 additions & 2 deletions infra/feast-operator/dist/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ spec:
- spark
- postgres
- trino
- redis
- athena
- mssql
type: string
Expand Down Expand Up @@ -2091,7 +2090,6 @@ spec:
- spark
- postgres
- trino
- redis
- athena
- mssql
type: string
Expand Down
4 changes: 2 additions & 2 deletions infra/feast-operator/docs/api/markdown/ref.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ _Appears in:_

| Field | Description |
| --- | --- |
| `type` _string_ | Type of the persistence type you want to use. Allowed values are: snowflake.offline, bigquery, redshift, spark, postgres, trino, redis, athena, mssql |
| `type` _string_ | Type of the persistence type you want to use. |
| `secretRef` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#localobjectreference-v1-core)_ | Data store parameters should be placed as-is from the "feature_store.yaml" under the secret key. "registry_type" & "type" fields should be removed. |
| `secretKeyName` _string_ | By default, the selected store "type" is used as the SecretKeyName |

Expand Down Expand Up @@ -286,7 +286,7 @@ _Appears in:_

| Field | Description |
| --- | --- |
| `type` _string_ | Type of the persistence type you want to use. Allowed values are: snowflake.online, redis, ikv, datastore, dynamodb, bigtable, postgres, cassandra, mysql, hazelcast, singlestore, hbase, elasticsearch, qdrant, couchbase, milvus |
| `type` _string_ | Type of the persistence type you want to use. |
| `secretRef` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#localobjectreference-v1-core)_ | Data store parameters should be placed as-is from the "feature_store.yaml" under the secret key. "registry_type" & "type" fields should be removed. |
| `secretKeyName` _string_ | By default, the selected store "type" is used as the SecretKeyName |

Expand Down
18 changes: 15 additions & 3 deletions infra/feast-operator/test/api/featurestore_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package api

import (
"context"
"fmt"
"strings"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -334,6 +336,16 @@ func registryStoreWithDBPersistenceType(dbPersistenceType string, featureStore *
return fsCopy
}

func quotedSlice(stringSlice []string) string {
quotedSlice := make([]string, len(stringSlice))

for i, str := range stringSlice {
quotedSlice[i] = fmt.Sprintf("\"%s\"", str)
}

return strings.Join(quotedSlice, ", ")
}

const resourceName = "test-resource"
const namespaceName = "default"

Expand Down Expand Up @@ -377,7 +389,7 @@ var _ = Describe("FeatureStore API", func() {
})

It("should fail when db persistence type is invalid", func() {
attemptInvalidCreationAndAsserts(ctx, onlineStoreWithDBPersistenceType("invalid", featurestore), "Unsupported value: \"invalid\": supported values: \"snowflake.online\", \"redis\", \"ikv\", \"datastore\", \"dynamodb\", \"bigtable\", \"postgres\", \"cassandra\", \"mysql\", \"hazelcast\", \"singlestore\", \"hbase\", \"elasticsearch\", \"qdrant\", \"couchbase\"")
attemptInvalidCreationAndAsserts(ctx, onlineStoreWithDBPersistenceType("invalid", featurestore), "Unsupported value: \"invalid\": supported values: "+quotedSlice(feastdevv1alpha1.ValidOnlineStoreDBStorePersistenceTypes))
})
})

Expand All @@ -388,7 +400,7 @@ var _ = Describe("FeatureStore API", func() {
attemptInvalidCreationAndAsserts(ctx, offlineStoreWithUnmanagedFileType(featurestore), "Unsupported value")
})
It("should fail when db persistence type is invalid", func() {
attemptInvalidCreationAndAsserts(ctx, offlineStoreWithDBPersistenceType("invalid", featurestore), "Unsupported value: \"invalid\": supported values: \"snowflake.offline\", \"bigquery\", \"redshift\", \"spark\", \"postgres\", \"trino\", \"redis\", \"athena\", \"mssql\"")
attemptInvalidCreationAndAsserts(ctx, offlineStoreWithDBPersistenceType("invalid", featurestore), "Unsupported value: \"invalid\": supported values: "+quotedSlice(feastdevv1alpha1.ValidOfflineStoreDBStorePersistenceTypes))
})
})

Expand All @@ -410,7 +422,7 @@ var _ = Describe("FeatureStore API", func() {
attemptInvalidCreationAndAsserts(ctx, registryWithS3AdditionalKeywordsForGsBucket(featurestore), "Additional S3 settings are available only for S3 object store URIs")
})
It("should fail when db persistence type is invalid", func() {
attemptInvalidCreationAndAsserts(ctx, registryStoreWithDBPersistenceType("invalid", featurestore), "Unsupported value: \"invalid\": supported values: \"sql\", \"snowflake.registry\"")
attemptInvalidCreationAndAsserts(ctx, registryStoreWithDBPersistenceType("invalid", featurestore), "Unsupported value: \"invalid\": supported values: "+quotedSlice(feastdevv1alpha1.ValidRegistryDBStorePersistenceTypes))
})
})

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import os
from feast.repo_config import REGISTRY_CLASS_FOR_TYPE, OFFLINE_STORE_CLASS_FOR_TYPE, ONLINE_STORE_CLASS_FOR_TYPE, LEGACY_ONLINE_STORE_CLASS_FOR_TYPE

def save_in_script_directory(filename: str, typedict: dict[str, str]):
script_dir = os.path.dirname(os.path.abspath(__file__))
file_path = os.path.join(script_dir, filename)

with open(file_path, 'w') as file:
for k in typedict.keys():
file.write(k+"\n")

for legacyType in LEGACY_ONLINE_STORE_CLASS_FOR_TYPE.keys():
if legacyType in ONLINE_STORE_CLASS_FOR_TYPE:
del ONLINE_STORE_CLASS_FOR_TYPE[legacyType]

save_in_script_directory("registry.out", REGISTRY_CLASS_FOR_TYPE)
save_in_script_directory("online-store.out", ONLINE_STORE_CLASS_FOR_TYPE)
save_in_script_directory("offline-store.out", OFFLINE_STORE_CLASS_FOR_TYPE)
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package datasources

import (
"bufio"
"os"
"slices"
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1"
"github.com/feast-dev/feast/infra/feast-operator/internal/controller/services"
)

func TestDataSourceTypes(t *testing.T) {
RegisterFailHandler(Fail)

RunSpecs(t, "Data Source Suite")
}

var _ = Describe("FeatureStore Data Source Types", func() {
Context("When checking against the python code in feast.repo_config", func() {
It("should match defined registry persistence types in the operator", func() {
registryFilePersistenceTypes := []string{string(services.RegistryFileConfigType)}
registryPersistenceTypes := append(feastdevv1alpha1.ValidRegistryDBStorePersistenceTypes, registryFilePersistenceTypes...)
checkPythonPersistenceTypes("registry.out", registryPersistenceTypes)
})
It("should match defined onlineStore persistence types in the operator", func() {
onlineFilePersistenceTypes := []string{string(services.OnlineSqliteConfigType)}
onlinePersistenceTypes := append(feastdevv1alpha1.ValidOnlineStoreDBStorePersistenceTypes, onlineFilePersistenceTypes...)
checkPythonPersistenceTypes("online-store.out", onlinePersistenceTypes)
})
It("should match defined offlineStore persistence types in the operator", func() {
offlinePersistenceTypes := append(feastdevv1alpha1.ValidOfflineStoreDBStorePersistenceTypes, feastdevv1alpha1.ValidOfflineStoreFilePersistenceTypes...)
checkPythonPersistenceTypes("offline-store.out", offlinePersistenceTypes)
})
})
})

func checkPythonPersistenceTypes(fileName string, operatorDsTypes []string) {
feastDsTypes, err := readFileLines(fileName)
Expect(err).NotTo(HaveOccurred())

// Add remote type to slice, as its not a file or db type and we want to limit its use to registry service when deploying with the operator
operatorDsTypes = append(operatorDsTypes, "remote")
missingFeastTypes := []string{}
for _, ods := range operatorDsTypes {
if len(ods) > 0 {
if !slices.Contains(feastDsTypes, ods) {
missingFeastTypes = append(missingFeastTypes, ods)
}
}
}
Expect(missingFeastTypes).To(BeEmpty())

missingOperatorTypes := []string{}
for _, fds := range feastDsTypes {
if len(fds) > 0 {
if !slices.Contains(operatorDsTypes, fds) {
missingOperatorTypes = append(missingOperatorTypes, fds)
}
}
}
Expect(missingOperatorTypes).To(BeEmpty())
}

func readFileLines(filePath string) ([]string, error) {
file, err := os.Open(filePath)
Expect(err).NotTo(HaveOccurred())
defer closeFile(file)

var lines []string
scanner := bufio.NewScanner(file)
for scanner.Scan() {
lines = append(lines, scanner.Text())
}

err = scanner.Err()
Expect(err).NotTo(HaveOccurred())

return lines, nil
}

func closeFile(file *os.File) {
err := file.Close()
Expect(err).NotTo(HaveOccurred())
}
Loading