vulcand
Reusing middlewares in multiple frontends
I don't know is there a better way to do this. Copying the same middleware definition to multiple frontends and keeping them up to date may be hard to manage.
Storing middlewares under a key like /vulcand/middlewares/<middlewareId> and referring them from multiple frontends would be a nice feature.
I'm upvoting this. Imagine having a hundred thousand routes all sharing middleware with identical initialization parameters. This would be a total performance win in that situation.
I think the design suggestion with middlewares with Id and the use case makes sense to me, @archisgore feel free to send PR my way!
Sorry for the delay. I'm close to having this done. Sometimes, doing things on the side as a hobby means that I don't always get to contribute on the project's schedule. :-( My apologies.
Expect a PR by Friday 6/26
Before I go too far, here's how I'm treating the middleware spec, so as not to break backwards-compatibility.
- PUT /middlewares/m1
- PUT /frontends/f1/middlewares/m1 (empty value)
This adds m1, to frontend f1
HOWEVER
- PUT /frontends/f1/middlewares/m2 {json blob}
will treat JSON Blob like any current middleware is treated today.
This way, if you only create a key with the id, you'll get an existing middleware. But current behavior will not break.
I'll state a few problems that'll need fixing based on experience. There is no "obvious spec" when we try to build a relational data based on a key/value store like etcd or memng.
Consider the following cases:
- PUT /middlewares/m1
- PUT /frontends/middlewares/m1
- DELETE /middlewares/m1
It is unclear if this should remove m1 from all frontends that reference it, or it should simply let them be.
Similarly consider
- PUT /frontends/middlewares/m1
- PUT /middlewares/m1
In this case, would frontend f1 retro-actively get middleware m1 if it was created after the frontend key creation?
We'll run into a lot of race conditions. I don't think that should prevent the feature because a low-level system cannot be made completely idiot-proof like packaged software, and I also believe the memory savings outweigh the potential confusion caused for a very fringe set of users.
@archisgore I think the proper fix (although not backwards-compatible) would be to change middlewares to be a JSON array on the frontend object:
PUT /frontends/f1/middlewares [ "m1", "m2" ]
This way it can be updated atomically and we aren't confusingly treating empty value and json blob with same middleware name differently.
The frontend middlewares endpoint would validate that the referenced middlewares exist. The middleware deletion endpoint should fail any delete request if the middleware is in use by any frontend. I think silently deleting middlewares is the worse option.
Right. So the more I see how vulcan works on the inside, it doesn't seem reasonable to delete middlewares at all. Any frontends that have middleware m1, will continue to retain it. This is because inside vulcan, middlewares are not shared through pointers, and even if they were, modifying a struct in use is just bad design.
So here's what I ended up with:
- PUT /middlewares/m1
- PUT /frontends/f1/middlwares/m1 (reference somehow)
- DELETE /middlewares/m1
- PUT /frontends/f1/middlewares/m1 <-- This call fails!
The reason I can't do anything about a delete request, is because the deletion happens in etcd and there's nothing vulcan can do to prevent that change. The best vulcan can do is ignore it.
Another work-around to backwards-compatibility is to create a /frontends/
There's a ton of tooling that uses vulcan's current middleware format and it would be quite a feat to make a breaking change.
I think it is possible to implement @bkeroackdsc 's suggestion without breaking backwards-compatibility. We can add additional path:
PUT /frontends/f1/middleware-refs [ "m1", "m2" ]
that will be treated exactly as a reference.
All global middlewares pointers should be stored in one slice and the frontend should have pointers to them, so they would be truly shared.
In case if middleware was deleted, the initialization of the fronted can have a choice of simply ignoring it or backing off.
Yes I agree. That's precisely what I'm doing. To summarize:
- Make a new middleware-refs endpoint to maintain backward compatibility.
- Middleware-frontends are loosely coupled, and inconsistent update behavior is undefined (By undefined I mean the engine has no contract to behave in a particular way. In actual implementation, I'll do my best to do the right thing.)
The specific contract is: If m1 exists under /middlewares/m1, and AFTER that, a reference is added to /frontend/f1/middleware-refs, then it MUST work.
Other operations are undefined, and users must read the latest documentation what the current implementation is doing.
Does that sound agreeable?
I dislike the JSON array variant. With vctl it's quite easy to manipulate that, but if you manage vulcand with etcdctl array manipulation sucks pretty hard. I would prefer an entry for each middleware reference, maybe with a boolean value to enable or disable the reference or something like that.
@tboerger Shouldn't we be optimizing for using the vulcand configuration API? I don't see any reason to compromise the semantics to make it easier to manually hack items into etcd.
Manually hack? Why do you think the API documentation shows all variants (including etcdctl calls)?
@tboerger @bkeroackdsc
but if you manage vulcand with etcdctl array manipulation sucks pretty hard.
That's a fair point.
The disadvantage is that you have to read the items before any modification, the advantage is that the operation will be atomic, because it will be the just one set vs multiple operations.
@archisgore let's actually follow @bkeroackdsc's suggestion and implement it not as JSON blob, but as a list of sub-keys. I would ignore the value at all, because I don't see a reason to introduce another state, if someone wishes to disable middleware, they can just delete it and then add it back.
Sounds good, and this results in a single etcd client operation as well :)
@tboerger @klizhentas A list of sub-keys are unordered. So, we need a way to determine the middleware execution order, don't we?
middleware order is defined by priority:
https://github.com/mailgun/vulcand/blob/master/engine/model.go#L362
so if we keep the same semantics with global middlewares, this will do the trick.
@klizhentas Current design presumes that a particular middleware "instance" is related with only a single frontend. In this case, a single priority attribute is enough.
New design differs in this aspect.
IMHO, execution order should be defined in the context(frontend) not in the middleware itself since there may be multiple frontends that require different execution order.
