feat: File persistence definition and implementation #4742

dmartinol · 2024-11-06T14:15:20Z

What this PR does / why we need it:

Add ephemeral persistence configuration to the Feast services deployed with the operator.
A sample manifest is also committed and reported here for your convenience:

apiVersion: feast.dev/v1alpha1
kind: FeatureStore
metadata:
  name: sample-ephemeral-persistence
spec:
  feastProject: my_project
  services:
    onlineStore:
      persistence:
        file:
          path: /data/online_store.db
    offlineStore:
      persistence:
        file:
          type: dask
    registry:
      local:
        persistence:
          file:
            path: /data/registry.db

Next PR will add persistent storage by using mounted volumed (Kubernetes PVC)

Which issue(s) this PR fixes:

Relates to #4561

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

dmartinol · 2024-11-06T14:16:19Z

@tchughesiv @tmihalac
Note: validations will be added to the CRD types as the other options will be integrated.

infra/feast-operator/api/v1alpha1/featurestore_types.go

infra/feast-operator/internal/controller/services/repo_config.go

infra/feast-operator/internal/controller/services/util.go

tchughesiv · 2024-11-06T14:52:31Z

infra/feast-operator/internal/controller/services/util.go

+				Path: DefaultOnlinePath,
+			}
+		}
+	}


would be good to have unit tests ensuring that proper defaults are being set in status when varying pointers around service persistence are set in the spec.

would be good to have unit tests ensuring that proper defaults are being set in status when varying pointers around service persistence are set in the spec.

This was the idea of repo_config_test.go, which verifies the repo config created after applying the defaults.

that's a great addition, and necessary, but its different from ensuring the status defaults are properly being reconciled in the CR.

I see what you mean, I'll add it in the next PR for PVC so there will be more cases to cover

infra/feast-operator/internal/controller/services/suite_test.go

infra/feast-operator/internal/controller/services/repo_config.go

infra/feast-operator/api/v1alpha1/featurestore_types.go

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

infra/feast-operator/internal/controller/suite_test.go

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

infra/feast-operator/internal/controller/services/util.go

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

tchughesiv

lgtm

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

tchughesiv

requested a couple changes. also, i'm noticing we're missing new FeatureStore CR checks in featurestore_controller_test.go for these new status fields and defaults after reconcile

infra/feast-operator/api/v1alpha1/featurestore_types.go

infra/feast-operator/internal/controller/services/util.go

infra/feast-operator/api/v1alpha1/featurestore_types.go

infra/feast-operator/internal/controller/featurestore_controller.go

…use it in deploy flow Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

dmartinol · 2024-11-07T13:53:48Z

requested a couple changes. also, i'm noticing we're missing new FeatureStore CR checks in featurestore_controller_test.go for these new status fields and defaults after reconcile

one more commit to add the missing checks

infra/feast-operator/internal/controller/services/util.go

…lePersistenceType Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

infra/feast-operator/internal/controller/services/util.go

tchughesiv

i've left a couple nits, but otherwise lgtm

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

dmartinol · 2024-11-07T16:37:53Z

requested a couple changes. also, i'm noticing we're missing new FeatureStore CR checks in featurestore_controller_test.go for these new status fields and defaults after reconcile

one more commit to add the missing checks

Latest commit includes new controller tests.
Please note that I had to change the resource name because some resources (svc?) are probably not deleted by previous tests, so it would fail due to an attempt to change ownership:

Object default/feast-services-offline is already owned by another Service controller feast-services-offline

I did not investigate further. I think we can handle this issue in a separate PR.

  2024-11-07T17:30:30+01:00     INFO    Successfully reconciled {"Service": "feast-services-offline", "operation": "updated"}
  2024-11-07T17:30:30+01:00     ERROR   Error deploying the FeatureStore client service {"error": "Object default/feast-services-offline is already owned by another Service controller feast-services-offline"}
  github.com/feast-dev/feast/infra/feast-operator/internal/controller/services.(*FeastServices).setFeastServiceCondition
        /Users/dmartino/projects/AI/feast/feast/infra/feast-operator/internal/controller/services/services.go:290
  github.com/feast-dev/feast/infra/feast-operator/internal/controller/services.(*FeastServices).deployFeastServiceByType
        /Users/dmartino/projects/AI/feast/feast/infra/feast-operator/internal/controller/services/services.go:95
  github.com/feast-dev/feast/infra/feast-operator/internal/controller/services.(*FeastServices).Deploy

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

tchughesiv · 2024-11-07T16:40:32Z

I did not investigate further. I think we can handle this issue in a separate PR.

it's not an "issue" per say ... it's got to do w/ another unit test from a prior run where i've changed object ownership so we can test that ownership conflicts/failures are working properly. that said, we prob should manually cleanup that object after the fact.

so, yea ... i guess it is an issue :)

* File persistence definition and implementation Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * removed optional and default markers Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * removed global cfg variable Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * reviewed ApplyDefaultsToStatus Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * extended checks in ApplyDefaultsToStatus Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * removed var error Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * added IsValidOfflineStoreFilePersistenceType to the API definitions Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * moved IsValidOfflineStoreFilePersistenceType to services package and use it in deploy flow Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * renamed isValidOfflineStoreFilePersistenceType to checkOfflineStoreFilePersistenceType Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * adding controller tests for ephemeral stores Signed-off-by: Daniele Martinoli <dmartino@redhat.com> * using slices package Signed-off-by: Daniele Martinoli <dmartino@redhat.com> --------- Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

# [0.42.0](v0.41.0...v0.42.0) (2024-12-05) ### Bug Fixes * Add adapters for sqlite datetime conversion ([#4797](#4797)) ([e198b17](e198b17)) * Added grpcio extras to default feature-server image ([#4737](#4737)) ([e9cd373](e9cd373)) * Changing node version in release ([7089918](7089918)) * Feast create empty online table when FeatureView attribute online=False ([#4666](#4666)) ([237c453](237c453)) * Fix db store types in Operator CRD ([#4798](#4798)) ([f09339e](f09339e)) * Fix the config issue for postgres ([#4776](#4776)) ([a36f7e5](a36f7e5)) * Fixed example materialize-incremental and improved explanation ([#4734](#4734)) ([ca8a7ab](ca8a7ab)) * Fixed SparkSource docstrings so it wouldn't used inhereted class docstrings ([#4722](#4722)) ([32e6aa1](32e6aa1)) * Fixing PGVector integration tests ([#4778](#4778)) ([88a0320](88a0320)) * Incorrect type passed to assert_permissions in materialize endpoints ([#4727](#4727)) ([b72c2da](b72c2da)) * Issue of DataSource subclasses using parent abstract class docstrings ([#4730](#4730)) ([b24acd5](b24acd5)) * Operator envVar positioning & tls.SecretRef.Name ([#4806](#4806)) ([1115d96](1115d96)) * Populates project created_time correctly according to created ti… ([#4686](#4686)) ([a61b93c](a61b93c)) * Reduce feast-server container image size & fix dev image build ([#4781](#4781)) ([ccc9aea](ccc9aea)) * Removed version func from feature_store.py ([#4748](#4748)) ([f902bb9](f902bb9)) * Support registry instantiation for read-only users ([#4719](#4719)) ([ca3d3c8](ca3d3c8)) * Syntax Error in BigQuery While Retrieving Columns that Start wit… ([#4713](#4713)) ([60fbc62](60fbc62)) * Update release version in a pertinent Operator file ([#4708](#4708)) ([764a8a6](764a8a6)) ### Features * Add api contract to fastapi docs ([#4721](#4721)) ([1a165c7](1a165c7)) * Add Couchbase as an online store ([#4637](#4637)) ([824859b](824859b)) * Add Operator support for spec.feastProject & status.applied fields ([#4656](#4656)) ([430ac53](430ac53)) * Add services functionality to Operator ([#4723](#4723)) ([d1d80c0](d1d80c0)) * Add TLS support to the Operator ([#4796](#4796)) ([a617a6c](a617a6c)) * Added feast Go operator db stores support ([#4771](#4771)) ([3302363](3302363)) * Added support for setting env vars in feast services in feast controller ([#4739](#4739)) ([84b24b5](84b24b5)) * Adding docs outlining native Python transformations on singletons ([#4741](#4741)) ([0150278](0150278)) * Adding first feast operator e2e test. ([#4791](#4791)) ([8339f8d](8339f8d)) * Adding github action to run the operator end-to-end tests. ([#4762](#4762)) ([d8ccb00](d8ccb00)) * Adding ssl support for registry server. ([#4718](#4718)) ([ccf7a55](ccf7a55)) * Adding SSL support for the React UI server and feast UI command. ([#4736](#4736)) ([4a89252](4a89252)) * Adding support for native Python transformations on a single dictionary ([#4724](#4724)) ([9bbc1c6](9bbc1c6)) * Adding TLS support for offline server. ([#4744](#4744)) ([5d8d03f](5d8d03f)) * Building the feast image ([#4775](#4775)) ([6635dde](6635dde)) * File persistence definition and implementation ([#4742](#4742)) ([3bad4a1](3bad4a1)) * Object store persistence in operator ([#4758](#4758)) ([0ae86da](0ae86da)) * OIDC authorization in Feast Operator ([#4801](#4801)) ([eb111d6](eb111d6)) * Operator will create k8s serviceaccount for each feast service ([#4767](#4767)) ([cde5760](cde5760)) * Printing more verbose logs when we start the offline server ([#4660](#4660)) ([9d8d3d8](9d8d3d8)) * PVC configuration and impl ([#4750](#4750)) ([785a190](785a190)) * Qdrant vectorstore support ([#4689](#4689)) ([86573d2](86573d2)) * RBAC Authorization in Feast Operator ([#4786](#4786)) ([0ef5acc](0ef5acc)) * Support for nested timestamp fields in Spark Offline store ([#4740](#4740)) ([d4d94f8](d4d94f8)) * Update the go feature server from Expedia code repo. ([#4665](#4665)) ([6406625](6406625)) * Updated feast Go operator db stores ([#4809](#4809)) ([2c5a6b5](2c5a6b5)) * Updated sample secret following review ([#4811](#4811)) ([dc9f825](dc9f825))

* Add adapters for sqlite datetime conversion ([feast-dev#4797](feast-dev#4797)) ([e198b17](feast-dev@e198b17)) * Added grpcio extras to default feature-server image ([feast-dev#4737](feast-dev#4737)) ([e9cd373](feast-dev@e9cd373)) * Changing node version in release ([7089918](feast-dev@7089918)) * Feast create empty online table when FeatureView attribute online=False ([feast-dev#4666](feast-dev#4666)) ([237c453](feast-dev@237c453)) * Fix db store types in Operator CRD ([feast-dev#4798](feast-dev#4798)) ([f09339e](feast-dev@f09339e)) * Fix the config issue for postgres ([feast-dev#4776](feast-dev#4776)) ([a36f7e5](feast-dev@a36f7e5)) * Fixed example materialize-incremental and improved explanation ([feast-dev#4734](feast-dev#4734)) ([ca8a7ab](feast-dev@ca8a7ab)) * Fixed SparkSource docstrings so it wouldn't used inhereted class docstrings ([feast-dev#4722](feast-dev#4722)) ([32e6aa1](feast-dev@32e6aa1)) * Fixing PGVector integration tests ([feast-dev#4778](feast-dev#4778)) ([88a0320](feast-dev@88a0320)) * Incorrect type passed to assert_permissions in materialize endpoints ([feast-dev#4727](feast-dev#4727)) ([b72c2da](feast-dev@b72c2da)) * Issue of DataSource subclasses using parent abstract class docstrings ([feast-dev#4730](feast-dev#4730)) ([b24acd5](feast-dev@b24acd5)) * Operator envVar positioning & tls.SecretRef.Name ([feast-dev#4806](feast-dev#4806)) ([1115d96](feast-dev@1115d96)) * Populates project created_time correctly according to created ti… ([feast-dev#4686](feast-dev#4686)) ([a61b93c](feast-dev@a61b93c)) * Reduce feast-server container image size & fix dev image build ([feast-dev#4781](feast-dev#4781)) ([ccc9aea](feast-dev@ccc9aea)) * Removed version func from feature_store.py ([feast-dev#4748](feast-dev#4748)) ([f902bb9](feast-dev@f902bb9)) * Support registry instantiation for read-only users ([feast-dev#4719](feast-dev#4719)) ([ca3d3c8](feast-dev@ca3d3c8)) * Syntax Error in BigQuery While Retrieving Columns that Start wit… ([feast-dev#4713](feast-dev#4713)) ([60fbc62](feast-dev@60fbc62)) * Update release version in a pertinent Operator file ([feast-dev#4708](feast-dev#4708)) ([764a8a6](feast-dev@764a8a6)) * Add api contract to fastapi docs ([feast-dev#4721](feast-dev#4721)) ([1a165c7](feast-dev@1a165c7)) * Add Couchbase as an online store ([feast-dev#4637](feast-dev#4637)) ([824859b](feast-dev@824859b)) * Add Operator support for spec.feastProject & status.applied fields ([feast-dev#4656](feast-dev#4656)) ([430ac53](feast-dev@430ac53)) * Add services functionality to Operator ([feast-dev#4723](feast-dev#4723)) ([d1d80c0](feast-dev@d1d80c0)) * Add TLS support to the Operator ([feast-dev#4796](feast-dev#4796)) ([a617a6c](feast-dev@a617a6c)) * Added feast Go operator db stores support ([feast-dev#4771](feast-dev#4771)) ([3302363](feast-dev@3302363)) * Added support for setting env vars in feast services in feast controller ([feast-dev#4739](feast-dev#4739)) ([84b24b5](feast-dev@84b24b5)) * Adding docs outlining native Python transformations on singletons ([feast-dev#4741](feast-dev#4741)) ([0150278](feast-dev@0150278)) * Adding first feast operator e2e test. ([feast-dev#4791](feast-dev#4791)) ([8339f8d](feast-dev@8339f8d)) * Adding github action to run the operator end-to-end tests. ([feast-dev#4762](feast-dev#4762)) ([d8ccb00](feast-dev@d8ccb00)) * Adding ssl support for registry server. ([feast-dev#4718](feast-dev#4718)) ([ccf7a55](feast-dev@ccf7a55)) * Adding SSL support for the React UI server and feast UI command. ([feast-dev#4736](feast-dev#4736)) ([4a89252](feast-dev@4a89252)) * Adding support for native Python transformations on a single dictionary ([feast-dev#4724](feast-dev#4724)) ([9bbc1c6](feast-dev@9bbc1c6)) * Adding TLS support for offline server. ([feast-dev#4744](feast-dev#4744)) ([5d8d03f](feast-dev@5d8d03f)) * Building the feast image ([feast-dev#4775](feast-dev#4775)) ([6635dde](feast-dev@6635dde)) * File persistence definition and implementation ([feast-dev#4742](feast-dev#4742)) ([3bad4a1](feast-dev@3bad4a1)) * Object store persistence in operator ([feast-dev#4758](feast-dev#4758)) ([0ae86da](feast-dev@0ae86da)) * OIDC authorization in Feast Operator ([feast-dev#4801](feast-dev#4801)) ([eb111d6](feast-dev@eb111d6)) * Operator will create k8s serviceaccount for each feast service ([feast-dev#4767](feast-dev#4767)) ([cde5760](feast-dev@cde5760)) * Printing more verbose logs when we start the offline server ([feast-dev#4660](feast-dev#4660)) ([9d8d3d8](feast-dev@9d8d3d8)) * PVC configuration and impl ([feast-dev#4750](feast-dev#4750)) ([785a190](feast-dev@785a190)) * Qdrant vectorstore support ([feast-dev#4689](feast-dev#4689)) ([86573d2](feast-dev@86573d2)) * RBAC Authorization in Feast Operator ([feast-dev#4786](feast-dev#4786)) ([0ef5acc](feast-dev@0ef5acc)) * Support for nested timestamp fields in Spark Offline store ([feast-dev#4740](feast-dev#4740)) ([d4d94f8](feast-dev@d4d94f8)) * Update the go feature server from Expedia code repo. ([feast-dev#4665](feast-dev#4665)) ([6406625](feast-dev@6406625)) * Updated feast Go operator db stores ([feast-dev#4809](feast-dev#4809)) ([2c5a6b5](feast-dev@2c5a6b5)) * Updated sample secret following review ([feast-dev#4811](feast-dev#4811)) ([dc9f825](feast-dev@dc9f825))

File persistence definition and implementation

d64da1a

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

dmartinol requested a review from a team as a code owner November 6, 2024 14:15

dmartinol changed the title ~~File persistence definition and implementation~~ fea: File persistence definition and implementation Nov 6, 2024

dmartinol changed the title ~~fea: File persistence definition and implementation~~ feat: File persistence definition and implementation Nov 6, 2024

tchughesiv requested changes Nov 6, 2024

View reviewed changes

infra/feast-operator/internal/controller/services/suite_test.go Outdated Show resolved Hide resolved

infra/feast-operator/internal/controller/services/repo_config.go Outdated Show resolved Hide resolved

tchughesiv reviewed Nov 6, 2024

View reviewed changes

infra/feast-operator/internal/controller/services/repo_config.go Show resolved Hide resolved

tchughesiv requested changes Nov 6, 2024

View reviewed changes

infra/feast-operator/internal/controller/services/repo_config.go Outdated Show resolved Hide resolved

infra/feast-operator/api/v1alpha1/featurestore_types.go Show resolved Hide resolved

dmartinol added 2 commits November 6, 2024 16:35

removed optional and default markers

08b1011

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

removed global cfg variable

9b07506

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

tchughesiv requested changes Nov 6, 2024

View reviewed changes

infra/feast-operator/internal/controller/suite_test.go Outdated Show resolved Hide resolved

reviewed ApplyDefaultsToStatus

db3c5c5

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

tchughesiv reviewed Nov 6, 2024

View reviewed changes

infra/feast-operator/internal/controller/services/util.go Outdated Show resolved Hide resolved

dmartinol added 2 commits November 6, 2024 22:08

extended checks in ApplyDefaultsToStatus

74c518c

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

removed var error

da6a84f

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

tchughesiv approved these changes Nov 6, 2024

View reviewed changes

dmartinol and others added 2 commits November 7, 2024 10:25

added IsValidOfflineStoreFilePersistenceType to the API definitions

bce16ca

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

Merge branch 'feast-dev:master' into persistence_file

15c595b

tchughesiv requested changes Nov 7, 2024

View reviewed changes

infra/feast-operator/api/v1alpha1/featurestore_types.go Outdated Show resolved Hide resolved

infra/feast-operator/internal/controller/services/util.go Show resolved Hide resolved

tchughesiv requested changes Nov 7, 2024

View reviewed changes

infra/feast-operator/api/v1alpha1/featurestore_types.go Outdated Show resolved Hide resolved

infra/feast-operator/internal/controller/featurestore_controller.go Outdated Show resolved Hide resolved

moved IsValidOfflineStoreFilePersistenceType to services package and …

251538d

…use it in deploy flow Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

tchughesiv reviewed Nov 7, 2024

View reviewed changes

infra/feast-operator/internal/controller/services/util.go Outdated Show resolved Hide resolved

renamed isValidOfflineStoreFilePersistenceType to checkOfflineStoreFi…

ca09666

…lePersistenceType Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

tchughesiv requested changes Nov 7, 2024

View reviewed changes

infra/feast-operator/internal/controller/services/util.go Outdated Show resolved Hide resolved

tchughesiv reviewed Nov 7, 2024

View reviewed changes

infra/feast-operator/internal/controller/services/util.go Outdated Show resolved Hide resolved

tchughesiv approved these changes Nov 7, 2024

View reviewed changes

adding controller tests for ephemeral stores

f1134ac

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

using slices package

8677162

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

Merge branch 'feast-dev:master' into persistence_file

3b76db9

lokeshrangineni enabled auto-merge (squash) November 8, 2024 07:06

lokeshrangineni added the ok-to-test label Nov 8, 2024

lokeshrangineni approved these changes Nov 8, 2024

View reviewed changes

lokeshrangineni merged commit 3bad4a1 into feast-dev:master Nov 8, 2024
25 checks passed

tchughesiv mentioned this pull request Nov 11, 2024

Feast Go Operator #4561

Closed

feat: File persistence definition and implementation #4742

feat: File persistence definition and implementation #4742

Uh oh!

Conversation

dmartinol commented Nov 6, 2024

What this PR does / why we need it:

Which issue(s) this PR fixes:

Uh oh!

dmartinol commented Nov 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tchughesiv Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

tchughesiv Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

dmartinol Nov 6, 2024

Choose a reason for hiding this comment

Uh oh!

tchughesiv Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmartinol Nov 7, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tchughesiv left a comment

Choose a reason for hiding this comment

Uh oh!

tchughesiv left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmartinol commented Nov 7, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tchughesiv left a comment

Choose a reason for hiding this comment

Uh oh!

dmartinol commented Nov 7, 2024

Uh oh!

tchughesiv commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tchughesiv Nov 7, 2024 •

edited

Loading

tchughesiv left a comment •

edited

Loading

tchughesiv commented Nov 7, 2024 •

edited

Loading