Honor masking of systemd-sysctl.service #24234
Conversation
During package install on systemd-based systems, some sysctl settings should be set (e.g. vm.max_map_count). In some environments, changing sysctl settings plainly does not work; previously a global environment variable named ES_SKIP_SET_KERNEL_PARAMETERS was introduced to skip calling sysctl, but this causes trouble for: * configuration management systems, which usually cannot apply an env var when running a package manager * package upgrades, which will not have the env var set any more, and thus leaving the package management system in a bad state (possibly half-way upgraded, can be very hard to recover) This removes the env var again and instead of calling systemd-sysctl manually, tells systemd to restart the wrapper unit - which itself can be masked by system administrators or management tools if it is known that sysctl does not work in a given environment. The restart is not silent on systems in their default configuration, but is ignored if the unit is masked.
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
clintongormley
added
:Delivery/Packaging
* master: (619 commits) Add support for clear scroll to high level REST client (elastic#25038) Tiny correction in inner-hits.asciidoc (elastic#25066) Added release notes for 6.0.0-alpha2 Expand index expressions against indices only when managing aliases (elastic#23997) Collapse inner hits rest test should not skip 5.x Settings: Fix secure settings by prefix (elastic#25064) add `exclude_keys` option to KeyValueProcessor (elastic#24876) Test: update missing body tests to run against versions >= 5.5.0 Track EWMA[1] of task execution time in search threadpool executor Removes an invalid assert in resizing big arrays which does not always hold (resizing can result in a smaller size than the current size, while the assert attempted to verify the new size is always greater than the current). Fixed NPEs caused by requests without content. (elastic#23497) Plugins can register pre-configured char filters (elastic#25000) Build: Allow preserving shared dir (elastic#24962) Tests: Make secure settings available from settings builder for tests (elastic#25037) [TEST] Skip wildcard expansion test due to breaking change Test that gradle and Java version types match (elastic#24943) Include duplicate jar when jarhell check fails Change ScriptContexts to use needs instead of uses$. (elastic#25036) Change `has_child`, `has_parent` queries and `childen` aggregation to work with the new join field type and at the same time maintaining support for the `_parent` meta field type. Remove comma-separated feature parsing for GetIndicesAction ...
|
In the course of doing my due diligence on this one, I've learned that we are violating packaging guidelines by allowing the installation scripts to exit with non-zero status code. In the bluntest possible terms:
Technically this is a breaking change so I will integrate this and add a note to the migration docs. Thank you for this PR @zeha! |
During package install on systemd-based systems, some sysctl settings should be set (e.g. vm.max_map_count). In some environments, changing sysctl settings plainly does not work; previously a global environment variable named ES_SKIP_SET_KERNEL_PARAMETERS was introduced to skip calling sysctl, but this causes trouble for: - configuration management systems, which usually cannot apply an env var when running a package manager - package upgrades, which will not have the env var set any more, and thus leaving the package management system in a bad state (possibly half-way upgraded, can be very hard to recover) This removes the env var again and instead of calling systemd-sysctl manually, tells systemd to restart the wrapper unit - which itself can be masked by system administrators or management tools if it is known that sysctl does not work in a given environment. The restart is not silent on systems in their default configuration, but is ignored if the unit is masked. Relates #24234
|
I pushed 2f8ba3a to add a note to the migration docs. |
|
This breaks deb systems where the |
| echo "unrecognized value [$ES_SKIP_SET_KERNEL_PARAMETERS] for ES_SKIP_SET_KERNEL_PARAMETERS; must [false] (default) or [true]" | ||
| exit 1 | ||
| if command -v systemctl > /dev/null; then | ||
| systemctl restart systemd-sysctl.service |
There was a problem hiding this comment.
If this call is meant to fail silently, it should be:
systemctl restart systemd-sysctl.service || true
|
When trying to upgrade ES on ubuntu 16.04: change in postinst script, line 57 (add trailing "|| true"): Now it works! :) |
|
ping @jasontedor your currently live packages are borken. |
|
Sorry for this. I tested the PR on my workstation which runs Fedora 25 and it does not behave this way; it's only Debian based systems that see this problem. I opened #25657. |
|
Taking a look at #25657, i think adding /usr/lib/sysctl.d/elasticsearch.conf is breaking systemd-sysctl in the first place on systems, where this setting is not allowed/possible (e.g. container). |
|
@juergenknaack not necessarily saying that this is a non-problem, but your unprivileged containers really should be masking |
|
@pdf I see where you are coming from... If there is no reasonable usecase for systemd-sysctl in containers at all, masking would be an appropriate way. Agree to that. |
|
@pdf I rechecked our container environment (Virtuozzo). We use systemd-sysctl to enforce some settings (e.g. net.ipv4.ip_forward=1) and it's working for us. The additonal /usr/lib/sysctl.d/elasticsearch file breaks this. |
|
@juergenknaack looks like the way lxc handles this is to add start condition overrides to NB: I'm not in any way affiliated with ES, they may decide to try and do something about it, but the above is my read. |
|
Thank you @pdf, you have it exactly right. |
During package install on systemd-based systems, some sysctl settings
should be set (e.g. vm.max_map_count).
In some environments, changing sysctl settings plainly does not work;
previously a global environment variable named ES_SKIP_SET_KERNEL_PARAMETERS
was introduced to skip calling sysctl, but this causes trouble for:
var when running a package manager
thus leaving the package management system in a bad state (possibly
half-way upgraded, can be very hard to recover)
This removes the env var again and instead of calling systemd-sysctl manually,
tells systemd to restart the wrapper unit - which itself can be masked by
system administrators or management tools if it is known that sysctl does
not work in a given environment.
The restart is not silent on systems in their default configuration, but
is ignored if the unit is masked.
Related: #21899, voxpupuli/puppet-elasticsearch#806