Add sysctls support #2729
Add sysctls support #2729
Conversation
LGTM FWIW Obviously swarm doesn't care much about this ... just needs to carry the information. |
Adds support for sysctl options in docker services. * Adds API plumbing for creating services with sysctl options set. * Adds swagger.yaml documentation for new API field. * Changes executor package to make use of the Sysctls field on objects * Includes integration test to verify that new behavior works. Essentially, everything needed to support the equivalent of docker run's `--sysctl` option except the CLI. Depends on docker/swarmkit#2729, which is not merged yet, and so has my fork branch of swarmkit vendored in to demonstrate passing integration test. Signed-off-by: Drew Erny <drew.erny@docker.com>
// Sysctls sets namespaced kernel parameters (sysctls) in the container. This | ||
// option is equivalent to passing --sysctl to docker run. | ||
// | ||
// Note that while options are are subject to the same restrictions as |
anshulpundir
Aug 22, 2018
Contributor
remove one 'are'
remove one 'are'
Adds support for sysctl options in docker services. * Adds API plumbing for creating services with sysctl options set. * Adds swagger.yaml documentation for new API field. * Updates the API version history document. * Changes executor package to make use of the Sysctls field on objects * Includes integration test to verify that new behavior works. Essentially, everything needed to support the equivalent of docker run's `--sysctl` option except the CLI. Depends on docker/swarmkit#2729, which is not merged yet, and so has my fork branch of swarmkit vendored in to demonstrate passing integration test. Signed-off-by: Drew Erny <drew.erny@docker.com>
Adds support for sysctl options in docker services. * Adds API plumbing for creating services with sysctl options set. * Adds swagger.yaml documentation for new API field. * Updates the API version history document. * Changes executor package to make use of the Sysctls field on objects * Includes integration test to verify that new behavior works. Essentially, everything needed to support the equivalent of docker run's `--sysctl` option except the CLI. Depends on docker/swarmkit#2729, which is not merged yet, and so has my fork branch of swarmkit vendored in to demonstrate passing integration test. Signed-off-by: Drew Erny <drew.erny@docker.com>
LGTM (perhaps after @anshulpundir's nit was addressed, but not a blocker for me) |
@@ -2064,6 +2085,25 @@ func (m *ContainerSpec) MarshalTo(dAtA []byte) (int, error) { | |||
i++ | |||
i = encodeVarintSpecs(dAtA, i, uint64(m.PidsLimit)) | |||
} | |||
if len(m.Sysctls) > 0 { |
thaJeztah
Aug 23, 2018
Member
this check looks redundant; the code inside the for
loop won't be executed if it's empty; https://play.golang.org/p/SkHL_G_iJtv
edit: nevermind. Didn't notice I was looking at generated code ("Code generated by protoc-gen-gogo. DO NOT EDIT.") 😊
this check looks redundant; the code inside the for
loop won't be executed if it's empty; https://play.golang.org/p/SkHL_G_iJtv
edit: nevermind. Didn't notice I was looking at generated code ("Code generated by protoc-gen-gogo. DO NOT EDIT.")
@@ -2781,6 +2821,14 @@ func (m *ContainerSpec) Size() (n int) { | |||
if m.PidsLimit != 0 { | |||
n += 2 + sovSpecs(uint64(m.PidsLimit)) | |||
} | |||
if len(m.Sysctls) > 0 { |
thaJeztah
Aug 23, 2018
Member
Same here; this check looks redundant
edit: nevermind. Didn't notice I was looking at generated code ("Code generated by protoc-gen-gogo. DO NOT EDIT.") 😊
Same here; this check looks redundant
edit: nevermind. Didn't notice I was looking at generated code ("Code generated by protoc-gen-gogo. DO NOT EDIT.")
Adds support for sysctl options to the container spec. This is equivalent to the --sysctl flag on `docker run`. Only API changes are required for swarmkit. All of the other changes involving plumbing through these options happens downstream in the engine. Signed-off-by: Drew Erny <drew.erny@docker.com>
Removed one of two consecutive "are"s. |
Codecov Report
@@ Coverage Diff @@
## master #2729 +/- ##
==========================================
+ Coverage 61.71% 61.72% +<.01%
==========================================
Files 134 134
Lines 21888 21888
==========================================
+ Hits 13508 13510 +2
+ Misses 6916 6912 -4
- Partials 1464 1466 +2 |
Adds support for sysctl options in docker services. * Adds API plumbing for creating services with sysctl options set. * Adds swagger.yaml documentation for new API field. * Updates the API version history document. * Changes executor package to make use of the Sysctls field on objects * Includes integration test to verify that new behavior works. Essentially, everything needed to support the equivalent of docker run's `--sysctl` option except the CLI. Depends on docker/swarmkit#2729, which is not merged yet, and so has my fork branch of swarmkit vendored in to demonstrate passing integration test. Signed-off-by: Drew Erny <drew.erny@docker.com>
@@ -315,9 +315,24 @@ message ContainerSpec { | |||
// Runtimes that don't support it ignore that field | |||
Isolation isolation = 24; | |||
|
|||
// PidsLimit prevents from OS resource damage by applications inside the container | |||
// PidsLimit prevents from OS resource damage by applications inside the container |
anshulpundir
Aug 24, 2018
Contributor
supernit: remove 'from'
supernit: remove 'from'
I see this was merged into master. What version of I'm currently running docker version |
@thaJeztah The #2729 was pushed to SwarmKit on Aug 25. SwarmKit hasn't been updated in docker-ce since Aug 4th. You did the bump. This would be an important missing feature to add to docker-ce 18.9.0 while it's still in beta. Is this a possibility? |
This also brings in these PRs from swarmkit: - docker/swarmkit#2691 - docker/swarmkit#2744 - docker/swarmkit#2732 - docker/swarmkit#2729 - docker/swarmkit#2748 Signed-off-by: Tibor Vass <tibor@docker.com>
This also brings in these PRs from swarmkit: - docker/swarmkit#2691 - docker/swarmkit#2744 - docker/swarmkit#2732 - docker/swarmkit#2729 - docker/swarmkit#2748 Signed-off-by: Tibor Vass <tibor@docker.com>
This also brings in these PRs from swarmkit: - docker/swarmkit#2691 - docker/swarmkit#2744 - docker/swarmkit#2732 - docker/swarmkit#2729 - docker/swarmkit#2748 Signed-off-by: Tibor Vass <tibor@docker.com> Upstream-commit: cce1763d57b5c8fc446b0863517bb5313e7e53be Component: engine
Support for swarm and sysctls is available in 19.03 RC2 |
- What I did
Adds support for sysctl options to the container spec. This is equivalent to the --sysctl flag on
docker run
.- How I did it
Added a field to the protocol buffer for sysctl options.
Only API changes are required for swarmkit. All of the other changes involving plumbing through these options happens downstream in the engine.
- How to test it
N/A, we don't even use this field directly in swarmkit.
- Description for the changelog
Added support for sysctls in services. This is equivalent to the
--sysctl
flag ondocker run
.