Add API support for PidsLimit on services#39882
Conversation
api/swagger.yaml
Outdated
There was a problem hiding this comment.
So, this is the only thing I'm not sure about; PidsLimit is directly in TaskTemplate.ContainerSpec (which is similar to how it's for container create), but other limits are in TaskTemplate.Resources.Limits / TaskTemplate.Resources.Reservation.
moby/swarmkit#2415 was merged a long time ago, and implemented it like this on the SwarmKit side, so I wasn't sure if we should change it?
thaJeztah
force-pushed
the
swarm_pids_limit
branch
from
fd6da91 to
af469ff
Compare
api/swagger.yaml
Outdated
There was a problem hiding this comment.
Does this need to be nullable?
For services we do not support partial updates (e.g. the client must send the entire service spec), this is different than the containers API.
There was a problem hiding this comment.
Hm, good point; but actually thinking of a scenario now that I'm not sure we're taking into account right now (also for other options)
- create service with API v1.x, using features of API v1.x
- use an older client (v1.x - 1) to update the service
- the older client doesn't know about features of v1.x, and will strip those fields
- the daemon detects the older API version, thus resets those fields
I'll have to check our logic, and make sure that on update, we restore the previous value of any field that was in the service spec, but that's not supported by the API version used to update.
Otherwise (for this particular feature), the PID limit will be removed when updating the service with an older client, which ain't nice
thaJeztah
force-pushed
the
swarm_pids_limit
branch
from
af469ff to
46c8bab
Compare
thaJeztah
force-pushed
the
swarm_pids_limit
branch
4 times, most recently
from
753d412 to
0efb966
Compare
|
@cpuguy83 updated; added a second commit to change it from a pointer; let me know if that looks ok then I'll squash the commits |
|
(this currently includes #40816, I'll rebase once that's merged) |
Support for PidsLimit was added to SwarmKit in moby/swarmkit/pull/2415, but never exposed through the Docker remove API. This patch exposes the feature in the repote API. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
swarm_pids_limit
branch
from
0efb966 to
157c53c
Compare
|
rebased and squashed |
5 participants
continues the work started in moby/swarmkit#2415 (vendored through #35326)
addresses the API side of #28618