Support Docker Pids Limit #4341
Conversation
| @@ -1216,6 +1220,8 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas | |||
| Binds: binds, | |||
|
|
|||
| VolumeDriver: driverConfig.VolumeDriver, | |||
|
|
|||
| PidsLimit: driverConfig.PidsLimit, | |||
There was a problem hiding this comment.
What happens when this is the default value for the type (0)? is it better not to set it?
Needs unit tests with the default and with setting it, and documentation.
See PR https://github.com/hashicorp/nomad/pull/3512/files for an example of all the little things to change to add a new option to the docker driver
There was a problem hiding this comment.
Agree on test + documentation. It looks like the setting of 0 is equivalent to not setting it so that shouldn't be an issue.
| @@ -368,6 +368,9 @@ The `docker` driver supports the following configuration in the job spec. Only | |||
| * `readonly_rootfs` - (Optional) `true` or `false` (default). Mount | |||
| the container's filesystem as read only. | |||
|
|
|||
| * `pids_limit` - (Optional) An integer value that specifies the pid limit for | |||
There was a problem hiding this comment.
should this be just pid_limit (singular vs plural)
There was a problem hiding this comment.
docker run calls it pids-limit https://docs.docker.com/engine/reference/commandline/run/ so would prefer to keep it the same for consistency
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Fixes #4318