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

Docs: Clarify how networking between data plane propeller and control plane data catalog can be configured in multi-cluster deployment #5345

Merged
merged 1 commit into from
May 16, 2024

Conversation

fg91
Copy link
Member

@fg91 fg91 commented May 9, 2024

Why are the changes needed?

The multi-cluster deployment documentation is slightly ambiguous on how the networking between the data plane cluster flytepropeller service and control plane cluster datacatalog service works, suggesting that datacatalog was exposed via the ingress.

I clarify that the user would need to expose datacatalog themselves and that datacatalog does not have its own auth mechanism. I also suggest to use a VPC-internal load balancer service for this purpose instead.

  • I updated the documentation accordingly.
  • All commits are signed-off.

… data catalog can be configured

Signed-off-by: Fabio Grätz <[email protected]>
@fg91 fg91 requested review from neverett and ppiegaze as code owners May 9, 2024 20:58
Copy link

codecov bot commented May 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.10%. Comparing base (c0f5b10) to head (62c0645).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5345      +/-   ##
==========================================
- Coverage   61.10%   61.10%   -0.01%     
==========================================
  Files         794      794              
  Lines       51213    51213              
==========================================
- Hits        31295    31294       -1     
- Misses      17037    17038       +1     
  Partials     2881     2881              
Flag Coverage Δ
unittests-datacatalog 69.31% <ø> (ø)
unittests-flyteadmin 58.90% <ø> (ø)
unittests-flytecopilot 17.79% <ø> (ø)
unittests-flytectl 68.30% <ø> (ø)
unittests-flyteidl 79.30% <ø> (ø)
unittests-flyteplugins 61.94% <ø> (ø)
unittests-flytepropeller 57.32% <ø> (ø)
unittests-flytestdlib 65.73% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

in the control plane cluster can for instance be made available to the ``flytepropeller`` services in the data plane
clusters with an internal load balancer service (see e.g. `GKE documentation <https://cloud.google.com/kubernetes-engine/docs/how-to/internal-load-balancing#create>`_
or `AWS Load Balancer Controller <https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/guide/service/nlb/>`_).
if the clusters use the same VPC network.

Copy link
Member Author

Choose a reason for hiding this comment

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

Happy to modify the formulation.

Copy link
Contributor

Choose a reason for hiding this comment

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

this looks good to me.

@fg91 fg91 requested review from wild-endeavor and eapolinario May 9, 2024 21:07
@fg91 fg91 changed the title Docs: Clarify how networking between data plane propeller and control plane data catalog can be configured Docs: Clarify how networking between data plane propeller and control plane data catalog can be configured in multi-cluster deployment May 9, 2024
Copy link
Contributor

@neverett neverett left a comment

Choose a reason for hiding this comment

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

Looks OK to me docs-wise -- @davidmirror-ops would also appreciate your input here

not exposed via the ingress by default and does not have its own authentication mechanism. The ``catalog`` service
in the control plane cluster can for instance be made available to the ``flytepropeller`` services in the data plane
clusters with an internal load balancer service (see e.g. `GKE documentation <https://cloud.google.com/kubernetes-engine/docs/how-to/internal-load-balancing#create>`_
or `AWS Load Balancer Controller <https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/guide/service/nlb/>`_).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
or `AWS Load Balancer Controller <https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/guide/service/nlb/>`_).
or `AWS Load Balancer Controller <https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/guide/service/nlb/>`_)

Copy link
Contributor

@eapolinario eapolinario left a comment

Choose a reason for hiding this comment

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

The last phrase is rendering slightly wrong in github but it'll be fine once picked up by sphinx.

in the control plane cluster can for instance be made available to the ``flytepropeller`` services in the data plane
clusters with an internal load balancer service (see e.g. `GKE documentation <https://cloud.google.com/kubernetes-engine/docs/how-to/internal-load-balancing#create>`_
or `AWS Load Balancer Controller <https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/guide/service/nlb/>`_).
if the clusters use the same VPC network.

Copy link
Contributor

Choose a reason for hiding this comment

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

this looks good to me.

@fg91 fg91 merged commit 16d54de into master May 16, 2024
47 of 48 checks passed
@fg91 fg91 deleted the fg91/doc/multi-cluster-datacatalog-networking branch May 16, 2024 08:53
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this pull request Jul 2, 2024
… data catalog can be configured (flyteorg#5345)

Signed-off-by: Fabio Grätz <[email protected]>
Co-authored-by: Fabio Grätz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants