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

[helm] add config options for proxy/registry, and add an optional k8s ingress alternative to traefik #8780

Merged
merged 9 commits into from
May 30, 2024

Conversation

sp3nx0r
Copy link
Contributor

@sp3nx0r sp3nx0r commented May 2, 2024

Description

PR adjusts the existing helm chart to allow flexibility in deploy around the optional components that might not be required in a production environment: registry (in case you are using another image registry already in your k8s cluster) and proxy (this allows for a regular k8s ingress to be used instead with whatever IngressClass you already have configured in the cluster).

Affected Dependencies

Helm chart

How has this been tested?

Tested this in an enterprise testing environment that comprises an nginx ingress (with two ingress classes, a public and a private) and with a registry. Validated that the proxy component still worked by exercising the following paths:

  • /blob - for seaweedfs including the path stripping
  • / - for frontend
  • /redoc - for backend

Checklist

@rasswanth-s
Copy link
Collaborator

I am currently merging the changes requested as they were minimal and to get the changes deployed in new version for testing.

rasswanth-s
rasswanth-s previously approved these changes May 28, 2024
Copy link
Collaborator

@rasswanth-s rasswanth-s left a comment

Choose a reason for hiding this comment

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

Great work @sp3nx0r 🎸

@rasswanth-s rasswanth-s dismissed their stale review May 28, 2024 08:18

Keeping my review pending, as I found some blockers during testing on a live cluster

Copy link
Collaborator

@rasswanth-s rasswanth-s left a comment

Choose a reason for hiding this comment

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

LGTM , tested it on live cluster with nginx with values
Screenshot 2024-05-30 at 12 20 32 AM

@rasswanth-s rasswanth-s merged commit 00818ac into OpenMined:dev May 30, 2024
23 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.

None yet

3 participants