Adding support for Traefik to respect the K8s ingress class annotation #1182
Conversation
…es.io/ingress.class” annotation.
|
Test failures seem unrelated. Something to do with timing out while pulling the golang container. Is there an easy way to retrigger that? |
|
Weird. Restarted. |
|
Thanks! Much appreciated. |
|
Well, apparently my tests are not properly formatted. Taking a look now. |
|
Moved the test for if an ingress should be processed into a separate function and made it a switch case. The code is a bit more verbose but i think a lot more clear as to its intentions. With that I think this is ready. :D |
…e purpose of the function is.
|
I believe this is ready for review. Sorry if I am spamming with the pings :) Pinging @emilevauge @SantoDE @errm @dtomcej @vdemeester |
docs/user-guide/kubernetes.md
Outdated
| You can control which ingress Træfɪk cares about by using the "kubernetes.io/ingress.class" | ||
| annotation. By default if the annotation is not set at all Træfɪk will include the | ||
| ingress. If the annotation is set to another other than traefik or a blank string |
There was a problem hiding this comment.
I think you want to say s/another/anything/ .
provider/kubernetes.go
Outdated
| PassHostHeader = false | ||
| default: | ||
| log.Warnf("Unknown value of %s for traefik.frontend.passHostHeader, falling back to %s", passHostHeaderAnnotation, PassHostHeader) | ||
| if shouldProcessIngress(ingressClass) { |
There was a problem hiding this comment.
Suggestion: Can we do
if !shouldProcessIngress(ingressClass) {
continue
}in order to avoid going even further nested to help readability?
provider/kubernetes.go
Outdated
| switch ingressClass { | ||
| case "": | ||
| return true | ||
| case "traefik": |
There was a problem hiding this comment.
You can merge this case with the previous like
case "traefik", "":
return true(The comma separator is like a logical OR.)
… code block more than needed
|
Thanks once again for the comments @timoreimann, I was unaware of the comma usage in the switch statements. Made the requested changes. :) |
|
@regner let's sync and merge. Looking forward to using this. :-) |
ldez
added
status/3-needs-merge
and removed
status/2-needs-review
status/1-needs-design-review
labels
ldez
added
area/provider/k8s/ingress
kind/enhancement
This is a first pass of making Traefik understand and respect the kubernetes.io/ingress.class annotation in Kubernetes.
There is a discussion in #1163 about how this should be handled. The objective of this PR is to have Traefik respond to the annotation in the same way other ingress controllers such as NGINX handle it. That being:
There is also discussion in the above mentioned issue about support for configuring what the annotation matches, for example being able to say "accept annotations that match traefik-internal" instead of matching on traefik. There doesn't seem to be clear consensus on how that should be handled. I think that should be spun out as a separate issue and done in isolation of this PR.
Fixes #1163
Pinging @emilevauge @SantoDE @errm @dtomcej @vdemeester