Test sysctl vm.max_map_count before failing init script

[biggreenogre]

Elasticsearch version (bin/elasticsearch --version): <= current master

Plugins installed: n/a

JVM version (java -version): any

OS version (uname -a if on a Unix-like system): Linux xxxx 4.10.0-37-generic #41~16.04.1-Ubuntu SMP Fri Oct 6 22:42:59 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux

Description of the problem including expected versus actual behavior:
elasticsearch daemon always fails to start when run inside a container.
Assuming value of vm.max_map_count >= MAX_MAP_COUNT, I expect the daemon to start when the init script is run.

The elasticsearch daemon requires that the value of /proc/sys/vm/max_map_count be equal to or greater than MAX_MAP_COUNT. The daemon init script attempts to set this value with sysctl.
Unfortunately, since this value cannot be set from inside a container, the init script always fails, even when the current value is perfectly good.

Issue was encountered in LXC containers, it also applies to LXD, Docker and other container environments.

Given that the smarter DevOps practitioner will set this value on the host PRIOR to starting the container, would it be reasonable to TEST the value before refusing to start the daemon?

I proposes a minor change to distribution/deb/src/main/packaging/init.d/elasticsearch (and other OS scripts as applicable) as follows:

126c126,128
< 		sysctl -q -w vm.max_map_count=$MAX_MAP_COUNT
---
> 		if [ `sysctl -n vm.max_map_count` -lt $MAX_MAP_COUNT ];then
> 			sysctl -q -w vm.max_map_count=$MAX_MAP_COUNT
> 		fi

As a sysadmin, it "feels wrong" to change a kernel parameter, potentially affecting other applications on the host, without first checking if the change is required. This patch adds a test of the current value of vm.max_map_count against MAX_MAP_COUNT. If the current value equals or exceeds the required value, no action is taken and the daemon will start. Only if the current value is insufficient, will the script attempt to make the change and fail.

My only concern is for the portability of the test construct, in case it is too "BASH-centric".

I will submit a pull request including patches for all the supported OS init scripts if this would be acceptable.

More background info at voxpupuli/puppet-elasticsearch#887

regards,

Drew

Steps to reproduce:

1. Install elasticsearch in a container
2. Start with the init script

[jasontedor]

I would accept a PR for this.

My only concern is for the portability of the test construct, in case it is too "BASH-centric".

This is fine, we require bash (although I would prefer $() instead of backticks, I plan to go through our scripts someday and replace the latter with the former).

Please ensure that the packaging tests pass too, perhaps expanding a test case if needed to cover the case when vm.max_map_count is already set appropriately. I can offer guidance running these if needed.

[biggreenogre]

Apologies for delay, vacation, work, etc.
Basic patches to deb and rpm init scripts is straight-forward but regarding tests, guidance would be most welcome.
Should I be looking in dev-tools/smoke_test_rc.py? Looks like it fires up a container but how can I set the sysctl var?

[jasontedor]

Should I be looking in dev-tools/smoke_test_rc.py? Looks like it fires up a container but how can I set the sysctl var?

No, look at the packaging tests in qa/vagrant; you can create test that sets vm.max_map_count at the start of the test and then exercise the condition here.

[rjernst]

@biggreenogre Are you still interested in contributing a PR for this?

[biggreenogre]

Yes, changed jobs just after last communication and it got lost in the shuffle. Let me figure out where I was and I'll create one soon regards, Drew

…

On 12/03/18, Ryan Ernst ***@***.***> wrote: @biggreenogre(https://github.com/biggreenogre) Are you still interested in contributing a PR for this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub(#27236 (comment)), or mute the thread(https://github.com/notifications/unsubscribe-auth/ADzaY1bFqu7EAOM1VCamc_l-4rRSj5g1ks5tdvXugaJpZM4QP6tp).

[danielmitterdorfer]

This is already addressed by #31285 and #31512, hence closing.