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

feat(argo-cd): gateway support #2965

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

tduverge
Copy link

@tduverge tduverge commented Oct 4, 2024

Resolves: #2634

Adding some changes to #2689

Especially :

  • Support gateway creation
  • Support GKE objects associated to gateway
  • Rearrange httproute key paths to be clear that httproute is gateway dependent
  • Remove API suffix from key gatewayAPI since it doesn't appear in any other key (like ingress or service)

I've also created a PR on pdrastil branch, you can keep its as you wish.

Checklist:

  • I have bumped the chart version according to versioning
  • I have updated the documentation according to documentation
  • I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • Any new values are backwards compatible and/or have sensible default.
  • I have signed off all my commits as required by DCO.
  • My build is green (troubleshooting builds).

@tduverge tduverge force-pushed the feature/gateway-api branch from 5c808e4 to 7429937 Compare October 4, 2024 08:57
@tduverge tduverge force-pushed the feature/gateway-api branch from c25f08c to 4ec0b60 Compare October 4, 2024 09:20
@tduverge tduverge force-pushed the feature/gateway-api branch from ef62a1f to cd7f480 Compare October 4, 2024 10:48
@tduverge tduverge force-pushed the feature/gateway-api branch from ee34452 to 281e031 Compare October 4, 2024 15:07
@JessieAMorris
Copy link

JessieAMorris commented Oct 11, 2024

Any news on what is preventing this from being merged? This would be a really nice addition.

@mkilchhofer
Copy link
Member

Any news on what is preventing this from being merged? This would be a really nice addition.

I think the community did not yet standardize on a pattern how Gateway API is integrated into arbitrary Helm charts. Just why I did not invest a lot if time into this PR:

  • Bitnami Helm charts do not offer Gateway API support for their charts
  • It is unclear whether an Application chart should deploy a kind: Gateway resource. From the ideology (of the Gateway API creators), the Gateway resource is maintained by the cluster operators, not the app operators:

    image

    Source: https://gateway-api.sigs.k8s.io/

  • I think the two points are the reason why, @pdrastil did not get a review either.

@pdrastil
Copy link
Member

@mkilchhofer I kept this as a draft for now

  1. Gateway API contains breaking changes between beta, 1.0 and 1.1 interfaces
  2. Other integrations use mixed versions (external-dns, cert-manager)
  3. Some implementations support dynamic Gateway resource provisioning (istio, cilium, contour), some act as static deployment (nginx, traefik)

@stemarks
Copy link

I am also in need of this feature, I may implement it myself for the time being until a decision is made.

  • It is unclear whether an Application chart should deploy a kind: Gateway resource. From the ideology (of the Gateway API creators), the Gateway resource is maintained by the cluster operators, not the app operators:

My personal view is that the app should only create the HTTPRoute, there will likely only be a single gateway shared between multiple services. However there is argument that if this is the only application that requires gateway then it should be included, or if this is the first application to be deployed, which in a lot of scenarios it is, then a gateway would be good here.

Personally I would plan to deploy ArgoCD and have a chart that deploys my gateway as part of an Argo app then the remote access would work once its up and running. I have multiple apps so need the gateway separated.

I think it would be good to at least implement HTTPRoute now and have the gateway discussion separately.

@tduverge
Copy link
Author

Hi.

From my pov, as a GKE user, it's not that clear that gateway should be shared between services. I had a case where a single HTTPRoute with a wrong configuration destroyed all the associated load balancer.

The implematation I made follow the same logic than Ingress for many cloud providers where deploying an Ingress creates a load balancer. This is how it works with the current Ingress already in that chart, and it could be easier to migrate from Ingress to Gateway for users if they can do the same with both.

Moreover, in that PR, the gateway deployment is facultative, as I said in the note there.. This let the ability to anyone to choice what he wants.

Of course, I can separate the gateway if this can accelerate things.

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

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.

Add gatewayAPI resources as an Ingress alternative.
5 participants