added supported deployment modes using helm chart for online, offline, ui and registry by redhatHameed · Pull Request #8 · RHEcosystemAppEng/feast

redhatHameed · 2024-05-22T21:28:09Z

added supported deployment modes using helm chart for online, offline, ui and registry

What this PR does / why we need it:

added supported deployment modes using helm chart for online, offline, ui and registry

Test the PR for example

helm install feast-feature-server infra/charts/feast-feature-server --set feature_store_yaml_base64=$(base64 < feature_store.yaml)

helm install feast-ofline-server infra/charts/feast-feature-server --set feast_mode=offline --set feature_store_yaml_base64=$(base64 < feature_store.yaml)

helm install feast-ui-server infra/charts/feast-feature-server --set feast_mode=ui --set feature_store_yaml_base64=$(base64 < feature_store.yaml)

helm install feast-registry-server infra/charts/feast-feature-server --set feast_mode=ui --set ffeature_store_yaml_base64=$(base64 < feature_store.yaml)

helm list
NAME                 	NAMESPACE	REVISION	UPDATED                             	STATUS  	CHART                      	APP VERSION
feast-feature-server 	ahameed4 	1       	2024-05-31 10:30:10.036583 -0400 EDT	deployed	feast-feature-server-0.38.0	           
feast-ofline-server  	ahameed4 	1       	2024-05-31 10:32:18.615329 -0400 EDT	deployed	feast-feature-server-0.38.0	           
feast-registry-server	ahameed4 	1       	2024-05-31 10:33:30.229271 -0400 EDT	deployed	feast-feature-server-0.38.0	           
feast-ui-server      	ahameed4 	1       	2024-05-31 10:33:08.696063 -0400 EDT	deployed	feast-feature-server-0.38.0

Which issue(s) this PR fixes:

Fixes

tchughesiv · 2024-05-22T21:37:29Z

what kind of service template considerations should be made between modes? if any?

redhatHameed · 2024-05-23T00:07:08Z

what kind of service template considerations should be made between modes? if any?

agreed, we need to generate it, based on mode let me add the template for that.

dmartinol

lgtm

tchughesiv · 2024-05-23T17:36:51Z

i'm curious, and don't know the answer to this... should a service get created at all in offline mode?

tchughesiv · 2024-05-23T19:02:20Z

i'm curious, and don't know the answer to this... should a service get created at all in offline mode?

@redhatHameed thoughts on this?

dmartinol · 2024-05-23T19:50:37Z

i'm curious, and don't know the answer to this... should a service get created at all in offline mode?

@redhatHameed thoughts on this?

All these servers need a service to provide access within the cluster, so IMO yes

Anyway, I was thinking about the deployment model we want to define.
Is it an Helm release per server or can we install all the servers in one shot? (e.g. values enable-online, enable-offline and enable-registry) In this case, we can also have a single Service with 1 port for each enabled service and use a label to match the deployments.
Otherwise, if we deploy one helm release per type of server, we need different service instances.

I would vote for single helm for all servers, but not sure if this is the right solution for the operator.

redhatHameed · 2024-05-23T19:53:13Z

i'm curious, and don't know the answer to this... should a service get created at all in offline mode?

@redhatHameed thoughts on this?

i'm curious, and don't know the answer to this... should a service get created at all in offline mode?

@redhatHameed thoughts on this?

All these servers need a service to provide access within the cluster, so IMO yes
@dmartinol we need to test this part - let's suppose the offline server is deployed how the client will call using route ?

Anyway, I was thinking about the deployment model we want to define. Is it an Helm release per server or can we install all the servers in one shot? (e.g. values enable-online, enable-offline and enable-registry) In this case, we can also have a single Service with 1 port for each enabled service and use a label to match the deployments. Otherwise, if we deploy one helm release per type of server, we need different service instances.

tchughesiv · 2024-05-23T19:54:27Z

@dmartinol i'll leave that up to you guys... i could def. see the benefits of that type of implementation

dmartinol · 2024-05-23T19:59:36Z

@dmartinol i'll leave that up to you guys... i could def. see the benefits of that type of implementation

@redhatHameed what about raising the architectural question to the community? e.g. the RH Feast channel or the community channel

dmartinol · 2024-05-27T07:55:22Z

@dmartinol i'll leave that up to you guys... i could def. see the benefits of that type of implementation

@redhatHameed what about raising the architectural question to the community? e.g. the RH Feast channel or the community channel

@tokoko could you pls share your thoughts on this point? quick recap for your convenience (long thread)
we're extending the helm chart to include the capability to install any of the available (python) servers. Since they all run the same container image with different command (feast serve,feast ui, feast serve_offline and feast serve_registry), it doesn't make much sense to have separate charts when a single one can be configured to install what is needed.
That said, which approach do you think is more suitable:

Having a single chart that can deploy multiple servers in the same Helm release (e.g., adding options like enable-online, enable-ui, enable-offline, and enable-registry).
Or having each release deploy a configured server (e.g., adding an option feast-mode with values like online, ui, offline, and registry).

tokoko · 2024-05-27T09:48:20Z

I'd definitely go with the first approach, although I'm not sure I'm on board with simple enable-online, enable-offline params. Something like this makes more sense to me:

common:
    featurestoreyaml: ...

online:
    enabled: true/false
    featurestoreyaml: ... (only specified if want to override)

offline:
    enabled: true/false

ui:
    enabled: true/false

tchughesiv

otherwise, lgtm

tchughesiv

it looks like the branch as been merged with some other changes?

tchughesiv

@redhatHameed we need to run make build-helm-docs. otherwise, lgtm.

on a mac, first install helm-docs -
brew install helm-docs

redhatHameed · 2024-05-31T15:13:42Z

@redhatHameed we need to run make build-helm-docs. otherwise, lgtm.

on a mac, first install helm-docs - brew install helm-docs

Thanks updated

tchughesiv

lgtm

…), offline, ui and registry Signed-off-by: Abdul Hameed <ahameed@redhat.com>

dmartinol

~lgtm

redhatHameed mentioned this pull request May 22, 2024

added offline server remote deployment config and Dockerfile #7

Closed

tchughesiv requested changes May 23, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/templates/deployment.yaml Outdated

Comment thread infra/charts/feast-feature-server/values.yaml Outdated

tchughesiv requested changes May 23, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/templates/deployment.yaml Outdated

Comment thread infra/charts/feast-feature-server/templates/service.yaml Outdated

Comment thread infra/charts/feast-feature-server/values.yaml Outdated

redhatHameed requested a review from dmartinol May 23, 2024 15:48

dmartinol reviewed May 23, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/templates/service.yaml Outdated

dmartinol reviewed May 23, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/values.schema.json Outdated

dmartinol reviewed May 23, 2024

View reviewed changes

tchughesiv requested changes May 23, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/values.schema.json Outdated

Comment thread infra/charts/feast-feature-server/README.md Outdated

tchughesiv requested changes May 23, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/values.yaml

redhatHameed marked this pull request as ready for review May 23, 2024 18:50

tchughesiv requested changes May 23, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/README.md Outdated

Comment thread infra/charts/feast-feature-server/README.md Outdated

Comment thread infra/charts/feast-feature-server/templates/service.yaml Outdated

tchughesiv requested changes May 23, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/values.yaml Outdated

Comment thread infra/charts/feast-feature-server/values.yaml Outdated

tchughesiv requested changes May 23, 2024

View reviewed changes

redhatHameed marked this pull request as draft May 28, 2024 13:13

tchughesiv approved these changes May 28, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/README.md Outdated

redhatHameed force-pushed the remote_offline branch from d901ac2 to 90dbcde Compare May 28, 2024 20:58

tchughesiv requested changes May 30, 2024

View reviewed changes

redhatHameed marked this pull request as ready for review May 30, 2024 18:03

dmartinol reviewed May 31, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/README.md Outdated

Comment thread infra/charts/feast-feature-server/templates/deployment.yaml

dmartinol reviewed May 31, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/templates/service.yaml

tchughesiv requested changes May 31, 2024

View reviewed changes

tchughesiv approved these changes May 31, 2024

View reviewed changes

Comment thread infra/charts/feast-feature-server/templates/deployment.yaml

added supported deployment modes using helm chart for online (default…

62c9915

…), offline, ui and registry Signed-off-by: Abdul Hameed <ahameed@redhat.com>

dmartinol approved these changes May 31, 2024

View reviewed changes

redhatHameed merged commit 853f1e2 into RHEcosystemAppEng:remote_offline May 31, 2024

Uh oh!

Conversation

redhatHameed commented May 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes

Uh oh!

tchughesiv commented May 22, 2024

Uh oh!

redhatHameed commented May 23, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmartinol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tchughesiv commented May 23, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tchughesiv commented May 23, 2024

Uh oh!

dmartinol commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

redhatHameed commented May 23, 2024

Uh oh!

tchughesiv commented May 23, 2024

Uh oh!

dmartinol commented May 23, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmartinol commented May 27, 2024

Uh oh!

tokoko commented May 27, 2024

Uh oh!

tchughesiv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tchughesiv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tchughesiv left a comment

Choose a reason for hiding this comment

Uh oh!

redhatHameed commented May 31, 2024

Uh oh!

tchughesiv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dmartinol left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

redhatHameed commented May 22, 2024 •

edited

Loading

dmartinol commented May 23, 2024 •

edited

Loading