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

Add all available annotations to k8s Backend #2612

Merged
merged 12 commits into from Jan 31, 2018

Conversation

ldez
Copy link
Member

@ldez ldez commented Dec 21, 2017

What does this PR do?

  • add all available annotations
    • add passTLSCert annotations.
    • add error pages annotations.
    • add max conn annotations.
    • add rate limit annotations.
  • enhance template readability

Motivation

Homogenization of the providers [part2]: all providers must have the same options available.

More

  • Added/updated tests
  • Added/updated documentation

Additional Notes

Several PRs will come after that for each provider.

Related to #618, #1465
Closes #2783

Copy link
Contributor

@timoreimann timoreimann left a comment

Choose a reason for hiding this comment

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

Here's one point where our homogenization might not be good: IMHO, Traefik health checks make no sense with Kubernetes since readiness probes are supposed to take care of this. There's a high chance that if both are enabled, users will be confused because the Endpoints controller may remove a pod from rotation while Traefik still wants to health-check it, leading to weird error messages.

@dtomcej @errm can you think of a use case where it makes sense to support both?

Copy link
Member

@mmatur mmatur left a comment

Choose a reason for hiding this comment

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

LGTM 👏

@ldez
Copy link
Member Author

ldez commented Jan 4, 2018

ping @errm @dtomcej

@dtomcej
Copy link
Contributor

dtomcej commented Jan 4, 2018

@timoreimann @ldez If we are constructing backends by parsing service endpoints, then that endpoint list already takes into account pod readiness and liveliness probes. I agree that introducing an independent/redundant health check would be confusing and indeed cause lifecycle issues.

Copy link
Contributor

@dtomcej dtomcej left a comment

Choose a reason for hiding this comment

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

Outside of the h/c issue discussed,

LGTM
:shipit:

@ldez
Copy link
Member Author

ldez commented Jan 6, 2018

I removed the health check.
Now we have to define the annotation's names, before writing the documentation.

@traefiker traefiker merged commit c944d20 into traefik:master Jan 31, 2018
@ldez ldez deleted the refactor/label-k8s branch January 31, 2018 16:17
@ldez ldez changed the title Add all available annotations to k8s Backend Add all available annotations to k8s Backend Mar 24, 2018
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

6 participants