[docker] dual-publish setcap; default raise limits #1745

schallert · 2019-06-18T04:38:04Z

TODO:

docs

What this PR does / why we need it:

Special notes for your reviewer:

Default to attempting to raise FD limits rather than opt in: raising
the limits is benign if it fails.
Dual-publish Docker images with a setcap'd binary for users running as
non-root that want to raise limits.

Does this PR introduce a user-facing and/or backwards incompatible change?:

- Dual-publish Docker images with SYS_RESOURCE capabilities on the m3dbnode binary for users running as non-root with added capabilities that wish to raise process limits.

- Default to attempting to raise FD limits to max possible, with an environment variable to skip doing so.

Does this PR require updating code package or user-facing documentation?:

YES: Included

robskillington · 2019-06-18T11:23:30Z

docker/images.json

@@ -1,15 +1,24 @@
 {


Could we also add m3aggregator to this btw? Noticed we don't have an image (can do this in followup I suppose though).

robskillington · 2019-06-18T11:24:23Z

docker/images.json

+    "m3dbnode-setcap": {
+      "name": "m3dbnode",
+      "dockerfile": "docker/m3dbnode/Dockerfile",
+      "tag_suffix": "setcap"


Will it add a "-" automatically? i.e. 0.10.2-setcap or will it be 0.10.2setcap ?

Yeah, in the builder script we have

TAG="${TAG}-${TAG_SUFFIX}"

robskillington · 2019-06-18T11:29:24Z

docker/images.json

      "dockerfile": "docker/m3dbnode/Dockerfile"
    },
+    "m3dbnode-setcap": {
+      "name": "m3dbnode",
+      "dockerfile": "docker/m3dbnode/Dockerfile",


Hm don't we need a new dockerfile that actually calls setcap?

🤦‍♂ will add

robskillington · 2019-06-19T06:58:50Z

docker/m3dbnode/Dockerfile-setcap

+
+# Use setcap to set +e "effective" and +p "permitted" to adjust the SYS_RESOURCE
+# so the process can raise the hard file limit with setrlimit.
+RUN apk add --no-cache curl jq libcap && \


Do we need curl and jq? I'm fine to leave it out and add on login (i.e. just apk add when you start a shell session), or if you feel we should (which I'm fine with too), should we putting it in the other base image too?

We only need it for M3DB for the m3dbnode_bootstrapped.sh script. When we deprecate that though we can just remove this entirely

robskillington

LGTM other than minor comment about curl/jq

mway

looks good!

mway · 2019-07-30T12:41:10Z

docs/operational_guide/kernel_configuration.md

 ====================

 This document lists the Kernel tweaks M3DB needs to run well. If you are running on Kubernetes, you may use our
 `sysctl-setter` [DaemonSet](https://github.com/m3db/m3/blob/master/kube/sysctl-daemonset.yaml) that will set these
 values for you. Please read the comment in that manifest to understand the implications of applying it.

+## Running with Docker
+
+If running M3DB under Docker it is recommended to give M3DB the `SYS_RESOURCE` capability so that it may raise its file


slight tweak:

When running M3DB inside Docker, it is recommended to add the SYS_RESOURCE capability to the container (using the --cap-add argument to docker run) so that it can raise its file limits:

mway · 2019-07-30T12:42:41Z

docs/operational_guide/kernel_configuration.md

+docker run --cap-add SYS_RESOURCE quay.io/m3/m3dbnode:latest
+```
+
+If you wish to run M3DB as a non-root user, you will need to use our `setcap` images:


nit: no first or second person pronouns

If M3DB is being run as a non-root user, M3's setcap images are required:

mway · 2019-07-30T12:43:38Z

docs/operational_guide/kernel_configuration.md

+
+## Automatic Limit Raising
+
+When M3DB first starts up it will attempt to raise its open file limit to the current value of `fs.nr_open`. This is a


minor nits:

During startup, M3DB will attempt to raise [...]

This is a benign operation; if it fails, M3DB will [...]

- Default to attempting to raise FD limits rather than opt in: raising the limits is benign if it fails. - Dual-publish Docker images with a setcap'd binary for users running as non-root that want to raise limits.

codecov · 2019-08-01T17:12:12Z

Codecov Report

Merging #1745 into master will increase coverage by <.1%.
The diff coverage is 45.4%.

@@           Coverage Diff            @@
##           master   #1745     +/-   ##
========================================
+ Coverage    72.2%   72.2%   +<.1%     
========================================
  Files         987     987             
  Lines       83822   83820      -2     
========================================
+ Hits        60556   60560      +4     
+ Misses      19237   19229      -8     
- Partials     4029    4031      +2

Flag	Coverage Δ
#aggregator	`82.4% <ø> (ø)`	⬆️
#cluster	`85.5% <ø> (ø)`	⬆️
#collector	`63.7% <ø> (ø)`	⬆️
#dbnode	`79.8% <45.4%> (ø)`	⬆️
#m3em	`73.2% <ø> (ø)`	⬆️
#m3ninx	`74.1% <ø> (-0.1%)`	⬇️
#m3nsch	`51.1% <ø> (ø)`	⬆️
#metrics	`17.5% <ø> (ø)`	⬆️
#msg	`74.7% <ø> (ø)`	⬆️
#query	`67.8% <ø> (ø)`	⬆️
#x	`85.5% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ffd32d...a88580e. Read the comment docs.

Got updated in #1745 but reverted in #1846.

schallert requested a review from robskillington June 18, 2019 04:38

robskillington reviewed Jun 18, 2019

View reviewed changes

robskillington reviewed Jun 19, 2019

View reviewed changes

robskillington approved these changes Jun 19, 2019

View reviewed changes

schallert force-pushed the schallert/raise_caps branch from 4c507d0 to 09c732a Compare July 25, 2019 14:37

schallert marked this pull request as ready for review July 25, 2019 15:46

schallert force-pushed the schallert/raise_caps branch 2 times, most recently from 418624a to 7f83557 Compare July 29, 2019 22:04

mway approved these changes Jul 30, 2019

View reviewed changes

schallert added 5 commits August 1, 2019 13:10

[docker] dual-publish setcap; default raise limits

78de701

- Default to attempting to raise FD limits rather than opt in: raising the limits is benign if it fails. - Dual-publish Docker images with a setcap'd binary for users running as non-root that want to raise limits.

setcap dockerfile

dacce40

update CI

c8db94f

docs

f65ec4f

doc tweaks

a88580e

schallert force-pushed the schallert/raise_caps branch from 7f83557 to a88580e Compare August 1, 2019 17:10

schallert merged commit d860d9c into master Aug 1, 2019

schallert deleted the schallert/raise_caps branch August 1, 2019 17:29

schallert added a commit that referenced this pull request Aug 2, 2019

[ci] fix ci-scripts submodule

78df412

Got updated in #1745 but reverted in #1846.

schallert mentioned this pull request Aug 2, 2019

[ci] fix ci-scripts submodule #1851

Merged

schallert added a commit that referenced this pull request Aug 2, 2019

[ci] fix ci-scripts submodule (#1851)

0b5c7dd

Got updated in #1745 but reverted in #1846.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docker] dual-publish setcap; default raise limits #1745

[docker] dual-publish setcap; default raise limits #1745

schallert commented Jun 18, 2019 •

edited

Loading

robskillington Jun 18, 2019

schallert Jun 18, 2019

robskillington Jun 18, 2019

schallert Jun 18, 2019

robskillington Jun 19, 2019

robskillington Jun 18, 2019

schallert Jun 18, 2019

robskillington Jun 19, 2019

schallert Jul 25, 2019

robskillington left a comment

mway left a comment

mway Jul 30, 2019 •

edited

Loading

mway Jul 30, 2019

mway Jul 30, 2019

codecov bot commented Aug 1, 2019


		## Automatic Limit Raising

		When M3DB first starts up it will attempt to raise its open file limit to the current value of `fs.nr_open`. This is a

[docker] dual-publish setcap; default raise limits #1745

[docker] dual-publish setcap; default raise limits #1745

Conversation

schallert commented Jun 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robskillington left a comment

Choose a reason for hiding this comment

mway left a comment

Choose a reason for hiding this comment

mway Jul 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 1, 2019

Codecov Report

schallert commented Jun 18, 2019 •

edited

Loading

mway Jul 30, 2019 •

edited

Loading