Adjust controller to process one ingress at a time. Use per-item rate limiting workqueue.

munnerz commented 6 years ago

This is in relation to https://github.com/jetstack/cert-manager/issues/407

From that issue:

Based on the code in WatchEvents (https://github.com/jetstack/kube-lego/blob/master/pkg/kubelego/watch.go#L73) - it appears that whenever any Kubernetes Ingress resource is created, updated, or deleted, all Ingress resources are immediately scheduled for re-processing.

Ingresses that already have a valid certificate will be skipped, but any user with a number of failing/invalid ingresses will make requests to LE APIs in an attempt to validate those ingresses.

As part of those syncs, more updates will likely be made to ingresses, thus re-queuing these ingresses to be immediately reprocessed after the 'round' of processing ingresses fails.

The good news is we do only process one Ingress resource at a time, which should reduce the hits to the API somewhat (this could be a lot worse otherwise).

This PR changes the usage of the workqueue to only process one ingress at a time. It also switches the workqueue to use the rate limiting interface instead of a plain workqueue. This allows us to exponentially backoff validation attempts on a per ingress basis.

~NOTE: I have not run e2e tests against this patch yet - I will update this PR with results once I have~

/cc @jsha @simonswine

ref #328

jetstack-bot commented 6 years ago

@munnerz: GitHub didn't allow me to request PR reviews from the following users: jsha.

Note that only jetstack members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to [this](https://github.com/jetstack/kube-lego/pull/329): >This is in relation to https://github.com/jetstack/cert-manager/issues/407 > >From that issue: > >``` >Based on the code in WatchEvents (https://github.com/jetstack/kube-lego/blob/master/pkg/kubelego/watch.go#L73) - it appears that whenever any Kubernetes Ingress resource is created, updated, or deleted, all Ingress resources are immediately scheduled for re-processing. > >Ingresses that already have a valid certificate will be skipped, but any user with a number of failing/invalid ingresses will make requests to LE APIs in an attempt to validate those ingresses. > >As part of those syncs, more updates will likely be made to ingresses, thus re-queuing these ingresses to be immediately reprocessed after the 'round' of processing ingresses fails. > >The good news is we do only process one Ingress resource at a time, which should reduce the hits to the API somewhat (this could be a lot worse otherwise). >``` > >This PR changes the usage of the workqueue to only process one ingress at a time. It also switches the workqueue to use the rate limiting interface instead of a plain workqueue. This allows us to exponentially backoff validation attempts on a per ingress basis. > >**NOTE**: I have not run e2e tests against this patch yet - I will update this PR with results once I have > >/cc @jsha @simonswine > >ref #328 > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/devel/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

munnerz commented 6 years ago

e2e tests passed after a few more commits 😃