Add API support for PidsLimit on services #39882
Conversation
| @@ -2870,6 +2870,14 @@ definitions: | |||
| description: "Run an init inside the container that forwards signals and reaps processes. This field is omitted if empty, and the default (as configured on the daemon) is used." | |||
| type: "boolean" | |||
| x-nullable: true | |||
| PidsLimit: | |||
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
| Tune a container's PIDs limit. Set `0` or `null` for unlimited. | ||
| type: "integer" | ||
| format: "int64" | ||
| x-nullable: true |
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 <github@gone.nl>
thaJeztah
force-pushed
the
swarm_pids_limit
branch
from
0efb966
to
157c53c
Compare
|
rebased and squashed |
continues the work started in moby/swarmkit#2415 (vendored through #35326)
addresses the API side of #28618