Conflicts because of sysctl

[soylent]

• Module version: 0.15
• Puppet version: 4.2.2
• OS and version: CentOS 7

Bug description

I think that this module should not change kernel parameters. It seems out of scope of the module and too error-prone.

If I want to use sysctl using a third-party module (other than thias-sysctl), I get a duplicate declaration error for File['/etc/sysctl.d/'].

If I use thias-sysctl module and try to change vm.max_map_count, I get the following error:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Evaluation Error: Error while evaluating a Resource Statement, Duplicate declaration: Sysctl[vm.max_map_count] is already declared; cannot redeclare at /etc/puppetlabs/code/vendor/modules/elasticsearch/manifests/config.pp:131 at /etc/puppetlabs/code/vendor/modules/elasticsearch/manifests/config.pp:131:7

[tylerjl]

@soylent the logic for handling the kernel parameter within the module is due to the fact that Elasticsearch 5.x will refuse to start without max_map_count set correctly, and the module tries hard to be functional out-of-the-box for users.

There could be a few options here:

1. Expose the setting via a top-level parameter (like elasticsearch::max_map_count so that it can be overridden if a user wants to explicitly set it.
2. Create a parameter like elasticsearch::manage_kernel_flags that dictates whether the module should attempt to set kernel values
3. Remove it entirely and explicitly mention in the documentation that users have to set vm.max_map_count somehow.

I'd be inclined to go with 1 or 2 if people feel strongly about it so the module can leave those settings alone if users want, but purging it entirely shifts a lot of the work onto users when the module is here in the first place to handle these sort of configuration changes for users.

[soylent]

Thank you for your reply. The documentation says that MAX_MAP_COUNT is 262144 by default, which is the same value as in /manifests/config.pp. I don't quite understand why

sysctl { 'vm.max_map_count':
  value => '262144',
}

is necessary.

Elasticsearch 5.x will refuse to start without max_map_count set correctly

This seems like an issue with init scripts or systemd configs

[tylerjl]

Ah, I see what you're saying. While you're right that in some of the init scripts that Elasticsearch ships with, this is set given the value passed in through MAX_MAP_COUNT, in other cases, such as the systemd service unit config, this isn't handled by the startup scripts and is left to the user. In the other startup scripts sysctl is invoked similarly to how the sysctl puppet module does it.

We could conceivably remove the sysctl resource in the puppet configs and move them into init scripts. However, this also means:

• systemd-based systems need manual action outside the module to set the value
• If an Elasticsearch user wants to change the kernel value on the machine, there's lots of room for unexpected behavior as all of the init scripts make this kernel parameter change when starting up, so changing it in the CLI with sysctl -w <value> will get overwritten with every service elasticsearch start
• The value can't be changed in puppet (either through a potential top-level parameter or currently with something like Sysctl<| title == 'vm.max_map_count |> { value => "100000" })

[cdenneen]

@tylerjl
0.15.0
Centos 7
Elasticsearch 2.x, 2.4.1

Error: Evaluation Error: Error while evaluating a Resource Statement, Invalid resource type sysctl at /tmp/puppet/thirdparty/elasticsearch/manifests/config.pp:131:7

[tylerjl]

@cdenneen that would just mean the elasticsearch module can't find the sysctl module, double check it's installed and available in puppet.

[soylent]

@tylerjl

systemd-based systems need manual action outside the module to set the value

This should not be necessary: elastic/elasticsearch@1fde263

If an Elasticsearch user wants to change the kernel value on the machine, there's lots of room for unexpected behavior as all of the init scripts make this kernel parameter change when starting up, so changing it in the CLI with sysctl -w will get overwritten with every service elasticsearch start

True, but we don't want to add even more complexity by configuring the kernel in this module. So I think, it is better to keep the kernel params in the init/systemd files.

[tylerjl]

Ah, okay. In that case I think it's a good idea to just follow upstream (Elasticsearch) and copy the init script behavior. That should cover the use cases I enumerated pretty well.

I'll submit a PR for this when I get time, and the CI acceptance tests should cover the relevant distros to ensure the change doesn't break them. Thanks!

[baurmatt]

We're running a OpenVZ/Virtuozzo where it isn't possible to set most of the sysctl parameters from within the VE. Therefor we strongly support an option to disable the configuration of sysctl parameters with in this module/init scripts. If I can help with the implementation please let me know.

[nshobe]

Consider this another vote for modification of sysctl to be far outside of scope of what a module should do. That's something that should be included as part of a profile one might build around this module. Documentation of "suggested considerations" regarding the application would be more in line with the idea of module containment.

Also, this module breaks trying to use thias/sysctl on the latest version of PE. There's been issues brought up regarding namespacing and scoping problems with different puppet versions on that modules issues.

This has left me with two options:

1. Fork thias/sysctl or use tpdownes/sysctl and then also fork this module to make corresponding changes (changes which would not be backwards compatible and thus likely not acceptable for merge requests)
2. Fork this module and omit sysctl entirely.

Either way, I'm forking something and bound to back myself into a corner.

[tylerjl]

I've PRd a fix in #762 that's baking in CI, it looks good to me, please do provide feedback if anyone has concerns about the fix. 🐟

[tylerjl]

I've merged #762, so this change should go out in the next release. The acceptance tests have confirmed that setting sysctl happens either in the init scripts or package postinst scripts (as upstream manages it). Thanks for the feedback!