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] The GCP deployment docs should fix/remove the instructions to use the GCP Ingress controller #3730

Closed
2 tasks done
fg91 opened this issue May 30, 2023 · 5 comments
Closed
2 tasks done
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@fg91
Copy link
Member

fg91 commented May 30, 2023

Description

The SSL Certificate section in the GCP deployment guide documents both the use of a GKE ManagedCertificate in combination with the GKE ingress controller as well as the use of cert manager with the nginx ingress controller.

However, when using the GCP ingress, the flyteadmin backend will always be shown as unhealthy (even though it isn't) as the GKE ingress controller cannot interpret probes of type exec.command which flyteadmin uses and which are recommended for gRPC services. Instead, the GKE ingress would need to either get a path + port where the app returns 200 if it is healthy or it needs to be given a TCP port where it will try to open a socket. (The way the GKE ingress checks for health can be customized with a so-called backend config. (In the Flyte Slack, users have reported that they used this mechanism to fix the broken health check.)

The documentation should either provide instructions for using the GKE ingress controller that work out of the box or remove references to it and instead only recommend the nginx ingress controller.

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@fg91 fg91 added documentation Improvements or additions to documentation untriaged This issues has not yet been looked at by the Maintainers labels May 30, 2023
@jeevb
Copy link
Contributor

jeevb commented May 31, 2023

Flyteadmin does have an HTTP health check that should work: https://github.com/flyteorg/flyteadmin/blob/3379031b05bec09df6b7bb1bd99d499cf900b268/pkg/server/service.go#L175

Unfortunately, it currently doesn't check for much: https://github.com/flyteorg/flyteadmin/blob/3379031b05bec09df6b7bb1bd99d499cf900b268/pkg/server/service.go#L158.

But this should work well with the existing GCP LB health checks, in theory, and we can continue to make it better.

@eapolinario eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label Jun 2, 2023
@davidmirror-ops
Copy link
Contributor

Also @fg91 while it's true, the latest version of docs doesn't include those sections, and currently the idea is to update only latest docs

Having said so, there's work in progress to publish a reference implementation for GCP, and we'll incorporate feedback there

@fg91
Copy link
Member Author

fg91 commented Jun 11, 2023

Having said so, there's work in progress to publish a reference implementation for GCP, and we'll incorporate feedback there

It would be really great if the reference implementation would use GCP Identity Aware Proxy instead of letting Flyte serve the login screen.

@fg91
Copy link
Member Author

fg91 commented Jun 11, 2023

the latest version of docs doesn't include those sections, and currently the idea is to update only latest docs

Ah interesting, I wasn't aware that this guide is not part of the latest docs anymore.

@davidmirror-ops
Copy link
Contributor

The manual instructions were removed from latest and there's currently an effort in progress to consolidate the build process for the documentation site, also exploring how to avoid older docs versions to be indexed.

In terms of the content of that section, the practical experience shows that probably flyteadmin is not prepared yet to meet the requirements from the GKE Ingress Controller for HTTP/2.

The ideal approach on GCP would be to use the work you did to enable IAP support and add it to the docs.

That effort will be tracked here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

4 participants