Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add systemd support #478

Closed
wants to merge 5 commits into from
Closed

Conversation

hrak
Copy link
Contributor

@hrak hrak commented Jun 21, 2016

This PR addresses the systemd part of #424

It installs a systemd unit file that overrides some settings of the default systemd service installed by the package. This is needed because the systemd service installed by the package (/usr/lib/systemd/system/elasticsearch.service) has a hardcoded setting for LimitNOFILE (65535) and does not configure LimitMEMLOCK.

With this PR the settings supplied to elasticsearch_configure are correctly applied on a systemd based system.

Tested on Ubuntu 16.04 and CentOS 7. I did not add tests yet because there is an issue with the Ubuntu 16.04 chef/bento box at the moment (See chef/bento#609)

@martinb3
Copy link
Contributor

This seems to break if you run elasticsearch_service from another cookbook. The sys-v style template has parameters and defaults to 'elasticsearch' so that even when used in other cookbooks, it uses the template from this cookbook by default.

@hrak
Copy link
Contributor Author

hrak commented Jun 22, 2016

This is what you get without tests :) Sorry about that. I will update the PR to allow the template to be overridden in a similar fashion. I think i will also switch from this drop-in snippet approach to just overriding the whole elasticsearch service unit file, which also saves creating a dir in /etc/systemd/system.

@martinb3
Copy link
Contributor

This is what you get without tests :)

For what it's worth, the local test-kitchen suite is how I found this one (I added a CentOS 7.x distro to the platforms we test against). We don't run them on CI, because they take forever, though.

@LXXero
Copy link

LXXero commented Jul 1, 2016

any progress on getting this merged? this is kind of annoying to be missing on rhel/centos7. Also, it breaks bootstrap.mlockall: true

i also noticed a minor issue which is that if you previously enabled the systemd service, it will keep trying to start/stop the script provided by the RPM in /usr/lib/systemd/system rather than the new one in /etc. after disabling/re-enabling it was fine though. this should probably only matter for people who didn't have this update. not sure there's a good fix for that.

@LXXero
Copy link

LXXero commented Jul 6, 2016

Found another bug, which is that ES 1.6.x doesn't have the file used by the ExecStartPre thing here:

#ExecStartPre=<%= @ES_HOME %>/bin/elasticsearch-systemd-pre-exec

Given that all this file does is check if the config file was changed from the default and spew an error, and chef manages the file anyway, i just commented it out.

@LXXero
Copy link

LXXero commented Jul 13, 2016

Also, heap size isn't getting set....

@feniix
Copy link

feniix commented Aug 17, 2016

Have you thought of using systemd drop-ins to change the parameter? i.e. instead of having to template the entire systemd startup file use create a directory /etc/systemd/system/elasticsearch.service.d and drop a .conf file with the correct parameter so it gets composed natively by systemd

see: https://www.freedesktop.org/software/systemd/man/systemd.unit.html

Along with a unit file foo.service, a "drop-in" directory foo.service.d/ may exist. All files with the suffix ".conf" from this directory will be parsed after the file itself is parsed. This is useful to alter or add configuration settings for a unit, without having to modify unit files. Each drop-in file must have appropriate section headers. Note that for instantiated units, this logic will first look for the instance ".d/" subdirectory and read its ".conf" files, followed by the template ".d/" subdirectory and the ".conf" files there. Also note that settings from the "[Install]" section are not honoured in drop-in unit files, and have no effect.

It looks similar to this:

[root@ip-10-1-1-144 system]# systemctl status elasticsearch
● elasticsearch.service - Elasticsearch
   Loaded: loaded (/usr/lib/systemd/system/elasticsearch.service; enabled; vendor preset: disabled)
  Drop-In: /etc/systemd/system/elasticsearch.service.d
           └─limit-memlock.conf
   Active: inactive (dead) since Wed 2016-08-17 04:05:25 UTC; 11s ago
     Docs: http://www.elastic.co
 Main PID: 27510 (code=exited, status=143)

The content of the file would look like /etc/systemd/system/elasticsearch.service.d/limit-memlock.conf

[Service]
LimitMEMLOCK=infinity

@hrak
Copy link
Contributor Author

hrak commented Aug 17, 2016

@feniix yes, that was my initial approach in the first version of this PR (see f5c4500), but that broke the way this cookbook uses templates for the service, and how running elasticsearch_service from another cookbook allows you to override the template.

@martinb3
Copy link
Contributor

Hi there -- it looks like this doesn't pass for at least 2 of the test-kitchen suites. Have you also considered using the new systemd_unit resource provided by Chef? I'd love to release a new major version of this cookbook to correspond with v5 of ES, and include support for systemd.

@hrak
Copy link
Contributor Author

hrak commented Aug 23, 2016

@martinb3 the systemd_unit resource is only available starting from Chef client 12.11, which would put a pretty big version restriction on the whole thing.

The test failures are due to elasticsearch-systemd-pre-exec missing from the tarball distribution.

I can solve this in two ways:

<% if ::File.exists?("#{@ES_HOME}/bin/elasticsearch-systemd-pre-exec") %>
ExecStartPre=<%= @ES_HOME %>/bin/elasticsearch-systemd-pre-exec
<% end -%>

Please let me know your preference. I also have a pending fix for ES_HEAP_SIZE etc. not being set which i will commit together with what we decide on the former issue.

@feniix
Copy link

feniix commented Aug 23, 2016

why not just do, just curious

ExecStartPre=-<%= @ES_HOME %>/bin/elasticsearch-systemd-pre-exec

@LXXero
Copy link

LXXero commented Aug 23, 2016

because that script is not included with every version of elasticsearch and it will fail on some versions if you leave it like that. please see my comments from earlier

@martinb3
Copy link
Contributor

I'm concerned we may be trying to support systemd a bit too early; what if we include a flag for which init system to use, and try to give it smart defaults? And then if systemd is selected, we use systemd_unit unless chef-client is less than 12.11, where we can emit a warning/failure?

Also, the systemd functionality as-is seems to be broken if you try to deploy multiple instances on a single system (the test-kitchen tests confirm this).

I apologize for the slow responses -- I'm trying to wrap my head around this :)

@hrak
Copy link
Contributor Author

hrak commented Aug 24, 2016

@martinb3 I have fixes pending for the test kitchen issues, just waiting on your input on which of the proposed fixes have your preference. But I'll prep an update based on option 2 in the meantime.

@martinb3
Copy link
Contributor

@hrak If elasticsearch-systemd-pre-exec is shipped with the package from Elastic.org, I'm okay including it in the cookbook (option 1). If it doesn't come with the packaged version, I'd rather do your option 2.

@LXXero
Copy link

LXXero commented Aug 24, 2016

the problem with the systemd-pre-exec script is that its included in 2.x but not 1.6.x or etc...

@hrak
Copy link
Contributor Author

hrak commented Aug 24, 2016

Indeed, elasticsearch-systemd-pre-exec is not part of the Elastic package < 2.x, and is also not included in the tarball releases, so i guess the current approach is the way to go.

@adagios
Copy link

adagios commented Aug 31, 2016

I've tried this with a fedora system, using package install, and it fails because there's no applicable template for a tradicional init.d script.

If systemd is detected, shouldn't the init.d script be skipped?

@martinb3
Copy link
Contributor

martinb3 commented Aug 31, 2016

If systemd is detected, shouldn't the init.d script be skipped?

I believe the current Elasticsearch packages still ship an init.d script, regardless of what init system is used. I'm guessing it's there for compatibility or fallback (you could use systemd or the wrapper to start the service).

@martinb3 martinb3 self-assigned this Aug 31, 2016
@adagios
Copy link

adagios commented Aug 31, 2016

I meant tarball install, sorry!

The problem is that elasticseach_service is trying to create an init.d script, and it can't find an appropriate template.

@martinb3
Copy link
Contributor

@adagios What version of Fedora? We can probably support that as well, but we might want to open it as a separate issue. We don't currently test against Fedora either.

@adagios
Copy link

adagios commented Aug 31, 2016

@martinb3 the issue is not really specific to Fedora. As part of supporting systemd, I think it shouldn't create the init.d script. This would make it work on all systemd installations, regardless of the existence or not of an, unneeded, init.d script template.

@majormoses
Copy link

@adagios not sure I agree with this:

As part of supporting systemd, I think it shouldn't create the init.d script. This would make it work on all systemd installations, regardless of the existence or not of an, unneeded, init.d script template.

For example Ubuntu 15.10 supported sysvinit, upstart, and systemd, I don't necessarily agree that just because systemd is available we should ignore the packages behavior. The package maintainers spent time on deciding the "best" defaults and this cookbook tries to mirror this. If you want something different then you can always alter the behavior in your wrapper.

@adagios
Copy link

adagios commented Oct 12, 2016

@majormoses I agree with you. It should not ignore the packages behaviour. What I'm defending is that it should not error out if it only knows how to create one type of init script.

If it knows how to create a systemd script, it should not give up on a systemd server because it doesn't know how to create an init.d script.

@martinb3 martinb3 closed this in a9c60af Oct 28, 2016
martinb3 added a commit that referenced this pull request Oct 28, 2016
- Drop support for ES < 5.0
  * Remove the shield/watcher plugin installs, since they have become x-pack in ES 5.0
  * Remove hashes and different download URLs, other logic that distinguishes between 1.x and 2.x.
  * Update README.md to reflect that this change drops support for < 5.0 (too many changes)

- elasticsearch_install:
  * Drop warnings for installing as root under ES < 2.0

- elasticsearch_configure:
  * Replace logging.yml with new log4j2.properties file, add template for this
  * Update variables that are allowed in /etc/sysconfig/elasticsearch or /etc/default/elasticsearch
  * Drop ES_JAVA_OPTS and introduce option 'jvm_options' with new template for /etc/elasticsearch/jvm.options
  * Add new restart_on_upgrade option, drop thread_stack_size, drop env_options, drop gc_settings
  * Drop some of the defaults in elasticsearch.yml that are no longer present in upstream packaging

- elasticsearch_plugin:
  * change bin/plugin to bin/elasticsearch-plugin when managing plugins
  * Remove the shield/watcher plugin installs, since they have become x-pack in ES 5.0
  * Add new `options` parameter/attribute so that you can pass things like `--batch` to this resource

- elasticsearch_service:
  * Introduce systemd unit files for distros that support them
  * Guard on which init systems based on presence of /etc/init.d or /usr/lib/systemd/system directories

Fixes #424.
Fixes #478.
Fixes #503.
martinb3 added a commit that referenced this pull request Oct 28, 2016
- Drop support for ES < 5.0
  * Remove the shield/watcher plugin installs, since they have become x-pack in ES 5.0
  * Remove hashes and different download URLs, other logic that distinguishes between 1.x and 2.x.
  * Update README.md to reflect that this change drops support for < 5.0 (too many changes)

- elasticsearch_install:
  * Drop warnings for installing as root under ES < 2.0

- elasticsearch_configure:
  * Replace logging.yml with new log4j2.properties file, add template for this
  * Update variables that are allowed in /etc/sysconfig/elasticsearch or /etc/default/elasticsearch
  * Drop ES_JAVA_OPTS and introduce option 'jvm_options' with new template for /etc/elasticsearch/jvm.options
  * Add new restart_on_upgrade option, drop thread_stack_size, drop env_options, drop gc_settings
  * Drop some of the defaults in elasticsearch.yml that are no longer present in upstream packaging

- elasticsearch_plugin:
  * change bin/plugin to bin/elasticsearch-plugin when managing plugins
  * Remove the shield/watcher plugin installs, since they have become x-pack in ES 5.0
  * Add new `options` parameter/attribute so that you can pass things like `--batch` to this resource

- elasticsearch_service:
  * Introduce systemd unit files for distros that support them, copy latest init scripts for all platforms
  * Guard on which init systems based on presence of /etc/init.d or /usr/lib/systemd/system directories

Fixes #424.
Fixes #478.
Fixes #503.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants