API: split types for Resources Reservations and Limits#40937
API: split types for Resources Reservations and Limits#40937tiborvass merged 1 commit intomoby:masterfrom
Conversation
26b1812 to
a8b405c
Compare
|
I'm not strictly against it but I don't think this is critical for #40938 . On swarm side, the design uses the same type for limits, reservation and node description. I'd imagine when this is added on the swarm it will just be a new key in the same type with no exceptions. On swarm side it probably even makes sense to make the reservations work for it as it is just the comparison with the field in node description. On moby side, we could just ignore it by not setting the value in node description until someone has a use case. |
|
Actually, I think I was wrong guessing the intentions of the design in swarm. There are already similar fields like this in https://github.com/docker/swarmkit/blob/master/api/types.proto#L82-L91 that are not exposed in moby afaics. If this is the model then |
|
My main motivation for this is to express what we support as Limit, and what we support as Reservation, and allow to be flexible in that. Currently the types are shared but marked with For example, currently the documentation looks like this; GenericResources is listed in both, but only supported in
We had a similar discussion on #37872, where some options could be set as a "limit" but not as a "reservation" (can't "reserve" swap space or swappiness ). Not sure if they're in the right place directly in |
This introduces A new type (`Limit`), which allows Limits and "Reservations" to have different options, as it's not possible to make "Reservations" for some kind of limits. The `GenericResources` have been removed from the new type; the API did not handle specifying `GenericResources` as a _Limit_ (only as _Reservations_), and this field would therefore always be empty (omitted) in the `Limits` case. Signed-off-by: Sebastiaan van Stijn <[email protected]>
a8b405c to
84748c7
Compare
| } | ||
|
|
||
| // Limit describes limits on resources which can be requested by a task. | ||
| type Limit struct { |
There was a problem hiding this comment.
@tonistiigi @cpuguy83 I updated this from ResourceLimit to Limit as we discussed, but I kept Resources (we discussed renaming to Reservation, but the same type is used both for nodes as well as for "reservation" for Services.

This introduces A new type (
ResourceLimit), which allows "Limits" and "Reservations" to have different options, as it's not possible to make "Reservations" for some kind of limits.The
GenericResourceshave been removed from the new type (and thus fromLimits); the API did not handle specifyingGenericResourcesas a Limit (only as Reservations), and this field would therefore always be empty (omitted) in theLimitscase.- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)