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

Toggle support for experimental channel #10435

Merged
merged 1 commit into from
Apr 2, 2024

Conversation

SantoDE
Copy link
Collaborator

@SantoDE SantoDE commented Feb 14, 2024

What does this PR do?

Motivation

The standard channel of Gateway API does not include Alpha API Versions anymore. If a user does the standard channel installation and then starts Traefik, Traefik won't be able to start properly.

With adding that knob, users can opt-in into the usage of the alpha APIs in case they need / want to.

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

Fixes #10440

@xEBIx
Copy link

xEBIx commented Feb 21, 2024

Thank you for filing this pull request.
I have some questions, are you just making v1alpha2 API Versions switchable, so I have the choice to use this or no one? To make it work we must use the API Version "V1" as cert-manager and of course Gateway API are already using it.
Also I see that you are making TCP and TLS routes switchable, Dont we need all of them, like HTTProute, Gateway and Gatewayclass is also using these versions?
Or dont I understand something right?

@rtribotte
Copy link
Member

Hello @SantoDE,

Nice to see you around, and thanks for this contribution!

I started to review the PR, and first I wanted to know if toggling the informers' setup would be enough. After some testing, there are no errors raised, even if Traefik RBAC does not give rights for resources of the experimental channel.

Nonetheless, as the provider will go through resource discovery, toggling only the informers' setup, will not prevent the experimental channel resources (for now TCPRoutes and TLSRoutes) code path from being executed, and by doing so, the user won't get much information on why this part of the discovery fails. Moreover, debug logs would only mention that no resources were found, which is misleading.
I think the option could be used to raise an error if the gateway is using route kinds not in the standard channel if the option is not set and prevent further operation on resources that are not "allowed".

I noticed that Kubernetes CRDs are distributed distinctively for each channel, and an annotation (gateway.networking.k8s.io/channel) is present to indicate the channel in the definition. Maybe, we could also leverage this information somehow to check consistency with Traefik configuration. In that regard, it could answer @xEBIx question, Traefik would expect to be in a cluster with definitions matching its configuration, being to standard channel or experimental channel.

Regarding the option naming, I would find it more accurate to reuse the channel vocabulary, standard channel versus experimental channel, more than alpha versus beta.

@xEBIx
Copy link

xEBIx commented Feb 21, 2024

I dont get it, I am sorry.
The Gateway API has moved forward to release the API V1, v1alpha1 is basicly deprecated. Making v1alpha1 switchable is only making sense to me, if the user has the old software in his cluster and doesnt want to update because of some reason.
The Experimental or Standard question is not about API Versions, just the question if you need TLS and TCP Routes, that is correct, as far as I understood. For now the most urgend change would be to use the V1 API Version, both in the helm Chart and in Traefik itself.

@SantoDE
Copy link
Collaborator Author

SantoDE commented Feb 21, 2024

Thank you all for the comments. Let me try to elaborate :)

@xEBIx:
That is not my understanding, as you can see in the public APIs. v1 only has a subset of resources included, such as HTTPRoute, Gateway and so on.

v1alpha2 is as of today, still perfectly in use for resources that have not graduated yet, such as e.g. TLSRoute. You can see that here.

Does that make more sense?

Last but not least, yes, bumping the CRDs on the other side is done already. Helm Chart PR
@rtribotte:
Re-using or better, also checking on gateway.networking.k8s.io/channel could indeed make sense. I didn't thought about that before, but it could definitely make sense. Let me noodle on that for a bit :)

About the other code paths being executed: I was under the impression, since we don't add the eventHandler, it wouldn't see changes. I was wrong :) Hence, I agree with you - I think we should check at the actual route kind when building config, whether or not we should process it. WDYT?

@rtribotte
Copy link
Member

Hello @SantoDE,

Sure, indeed I think checking if the route kind is supported seems to way to go.
Operations for one gateway should stop as soon as it is identified that it references "unauthorized" route kinds, and update its status accordingly.

@xEBIx
Copy link

xEBIx commented Feb 22, 2024

I am afraid, no it does not make sense to me.
Please have a look into their v1.0.0 release CRD files (https://github.com/kubernetes-sigs/gateway-api/releases/tag/v1.0.0).
There in the standard Version, you only have the referencegrants as v1alpha2, the rest of these CRDs is already V1. And furthermore the referencegrants is marked as deprecated in v1alpha2. The Experimental release has more CRDs still in v1alpha2, like tlsroute, that is true, but not the problem here.

It wouldnt be that kind of a deal, if it would work in the end. But it does NOT! I cant install traefik with helm, because of the CRDs Traefik wants to have in v1alpha2 and Gateway has it in v1, and also cert-manager is using it in v1. So, in the process of creating certificates, certmanager is creating resources in v1 and Traefik searches them in v1alpha2, and of course cant find them, because they have been created in v1 by cert-manager. Please see my other issue here. Or the one at Gateway API: kubernetes-sigs/gateway-api#2771

Furthermore there is another issue on there end, I created, because their admissions server did not support your created resources in their recent release, they reanabled those in the admission server again, more or less for traefik to work.

@SantoDE
Copy link
Collaborator Author

SantoDE commented Feb 22, 2024

Hello @xEBIx,

I think there is a bit of confusion here. The V1 release (i.e. 1.0.0) is distributed via two different channels. The standard installation and the experimental installation. The standard installation features all APIs which have graduated by now, which are Gateway, HTTPRoute and so on. As you have seen that by yourself :)

However, not graduated resources yet do (as of now) still have the v1alpha2 API Version. There is for example no TCPRoute with the v1 API Version as of now.

However, your claim is correct. Prior to the Traefik version 3.0.0-rc1, Traefik is looking for v1alpha1 versions despite the fact that new versions are available. If you want to use Gateway API with its V1, I suggest you to test it with Traefik version 3.0.0-rc1. This version is looking for v1 resources where v1 versions exists.

Im also on the CNCF slack or the Kubernetes slack if you wanna discuss more.

@SantoDE SantoDE force-pushed the alpha_api_toggle branch 3 times, most recently from d06d83f to a66236d Compare February 26, 2024 20:04
@rtribotte rtribotte changed the title Add toggle to enable alpha APIs Toggle support for Gateway API experimental channel Mar 11, 2024
@rtribotte rtribotte removed their assignment Mar 14, 2024
Copy link
Member

@rtribotte rtribotte left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@youkoulayley youkoulayley left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kevinpollet kevinpollet self-requested a review April 2, 2024 13:47
docs/content/providers/kubernetes-gateway.md Outdated Show resolved Hide resolved
pkg/provider/kubernetes/gateway/kubernetes.go Outdated Show resolved Hide resolved
Copy link
Member

@kevinpollet kevinpollet left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@traefiker traefiker merged commit c84b510 into traefik:v3.0 Apr 2, 2024
23 checks passed
@kevinpollet kevinpollet changed the title Toggle support for Gateway API experimental channel Toggle support for experimental channel Apr 10, 2024
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.

Using outdated Gateway API CRD Versions
7 participants