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

fix: Refactor Operator to deploy all feast services to the same Deployment/Pod #4863

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

tchughesiv
Copy link
Contributor

@tchughesiv tchughesiv commented Dec 19, 2024

What this PR does / why we need it:

Current Operator approach to Feast deployment relies heavily on the remote provider type and feast server interaction over k8s services. This PR pulls feast services into a single Pod (but still separate containers) which allows for a more traditional Feast stack configuration. Each feast service container is able to share things like storage, feature_repo, and the feature_store.yaml.

Which issue(s) this PR fixes:

Fixes #4866 & #4848

@tchughesiv tchughesiv force-pushed the singleDeployNew branch 3 times, most recently from 45621f9 to ffe3ee1 Compare December 20, 2024 15:30
@tchughesiv tchughesiv marked this pull request as ready for review December 20, 2024 15:31
@tchughesiv tchughesiv requested a review from a team as a code owner December 20, 2024 15:31
Signed-off-by: Tommy Hughes <[email protected]>
}
}

func mountTlsRemoteRegistryConfig(feastType FeastServiceType, podSpec *corev1.PodSpec, tls *feastdevv1alpha1.TlsRemoteRegistryConfigs) {
func mountTlsRemoteRegistryConfig(podSpec *corev1.PodSpec, tls *feastdevv1alpha1.TlsRemoteRegistryConfigs) {
Copy link
Contributor

@lokeshrangineni lokeshrangineni Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious, TlsRemoteRegistryConfigs existing only for registry? not for other offline and online?

Copy link
Contributor Author

@tchughesiv tchughesiv Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious, TlsRemoteRegistryConfigs existing only for registry?

correct... in the event a remote registry is specified by the user with TLS configured via services.registry.remote.tls, the online/offline containers will need the public cert (or CA) in order to communicate with said remote registry.

not for other offline and online?

with this function the online/offline containers are configured with the remote registry's TLS volume.

Signed-off-by: Tommy Hughes <[email protected]>
@franciscojavierarceo franciscojavierarceo merged commit 88854dd into feast-dev:master Dec 20, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issues with Feast stack when deployed via Operator - due to using remote provider type
3 participants