First version of the RateLimit reconciler #6
Conversation
|
|
||
| limitadorv1alpha1 "github.com/3scale/limitador-operator/api/v1alpha1" | ||
| ) | ||
|
|
||
| const rateLimitFinalizer = "finalizer.ratelimit.limitador.3scale.net" | ||
|
|
||
| // Assumes that there's only one Limitador per namespace. We might want to |
There was a problem hiding this comment.
@miguelsorianod @eguzki please keep this in mind while reviewing.
I think we'll need to be able to deploy multiple limitadors per namespace, but let's leave that scenario for when the use cases are more clear.
|
Generally it looks that it works as expected and does it's work. Just wanted to comment that it does not implement reconciliation pattern:
Anyway, this is not blocking for now. Just a side note. |
|
@eguzki Why do you say it does not implement the reconciliation pattern? Is it because the issue about updating the limits discussed in #6 (comment) or is it something else? |
|
Just a high level or generic comment of the approach. The last step of the reconciliation loop there is a "createLimit" operation which is unconditional. |
davidor
force-pushed
the
first-version-ratelimit-reconciler
branch
from
ef0a727 to
e8b8693
Compare
The idea is to have a test file for each controller.
davidor
force-pushed
the
first-version-ratelimit-reconciler
branch
from
e8b8693 to
2da7e7e
Compare
| Foo string `json:"foo,omitempty"` | ||
| Conditions []string `json:"conditions"` | ||
| MaxValue int `json:"max_value"` | ||
| Namespace string `json:"namespace"` |
There was a problem hiding this comment.
Why do you need Namespace? Is this is intended to "bind" this rate limit to some Limitador service in another namespace? In that case, I suggest having URL.
There was a problem hiding this comment.
All k8s objects have a meta property namespace you can use to bind one rate limit object to the limitador service of the same namespace. JFYI.
There was a problem hiding this comment.
The naming is confusing, sorry. A limitador namespace doesn't need to be a kubernetes namespace. Limitador also runs outside kubernetes. A limitador namespace is the "context" on which a limit applies, it could be a service, a product, a proxy, a kubernetes namespace or any other thing really. It's meant to be generic to support multiple use cases.
In a future version of Limitador we might want to rename this field so it's less confusing for users running it in Kubernetes.
There was a problem hiding this comment.
Ok, I see, it is an internal namespace.
This PR adds the RateLimit CR using
operator-sdk create-api .... The attributes of the CR are the ones accepted by the Limitador HTTP API.This PR also adds an initial version of a controller that allows us to manage the limits in limitador via the RateLimit CR.