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

Define a templated systemd instance service unit in distribution packaging #26246

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions distribution/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@ configure(distributions.findAll { ['deb', 'rpm'].contains(it.name) }) {
fileType CONFIG | NOREPLACE
from "${packagingFiles}/systemd/elasticsearch.service"
}
configurationFile '/usr/lib/systemd/system/[email protected]'
into('/usr/lib/systemd/system') {
fileType CONFIG | NOREPLACE
from "${packagingFiles}/systemd/[email protected]"
}
into('/usr/lib/sysctl.d') {
fileType CONFIG | NOREPLACE
from "${packagingFiles}/systemd/sysctl/elasticsearch.conf"
Expand Down
60 changes: 60 additions & 0 deletions distribution/src/main/packaging/systemd/[email protected]
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
[Unit]
Description=Elasticsearch Instance %i
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use %I here

Documentation=http://www.elastic.co
Wants=network-online.target
After=network-online.target

[Service]
RuntimeDirectory=elasticsearch
Environment=ES_HOME=/usr/share/elasticsearch
Environment=ES_PATH_CONF=${path.conf}/%i
Environment=PID_DIR=/var/run/elasticsearch
EnvironmentFile=-${path.env}-%i
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should add AssertPathExists assertions on EnvironmentFile?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't technically need the EnvironmentFile to be present, as the most important thing it's doing is setting ES_PATH_CONF which the unit already does. It may be a good idea to add an AssertPathExists for the ES_PATH_CONF directory, though?


WorkingDirectory=/usr/share/elasticsearch

User=elasticsearch
Group=elasticsearch

ExecStart=/usr/share/elasticsearch/bin/elasticsearch -p ${PID_DIR}/elasticsearch-%i.pid --quiet

# StandardOutput is configured to redirect to journalctl since
# some error messages may be logged in standard output before
# elasticsearch logging system is initialized. Elasticsearch
# stores its logs in /var/log/elasticsearch and does not use
# journalctl by default. If you also want to enable journalctl
# logging, you can simply remove the "quiet" option from ExecStart.
StandardOutput=journal
StandardError=inherit

# Specifies the maximum file descriptor number that can be opened by this process
LimitNOFILE=65536
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like that we have to duplicate (and maintain) all the options like LimitNOFILE... Would it be possible to inherit/import from the main service file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed; I tried to come up with a better way with systemd natively but I couldn't come up with a good solution. For reference, here's some feedback from #systemd on freenode:

    leothrix | Am I mistaken, or am I remembering correctly that there's some sort of
             | `.import` option in unit files?
     zdzichu | leothrix: .include ?
     zdzichu | it's deprecated
    leothrix | zdzichu: Ah, that was the one I'd seen before. So it's being phased out?
     zdzichu | yep, .d/ snippets are better
     zdzichu | more clean and visible
    leothrix | Is there any way to define a new unit that includes the same options as
             | another unit instead of just overriding an existing unit?
boucman_work | leothrix: kinda, but that would be a bit ugly
     zdzichu | ha, that's one use case where .include would be helpful ;)
     zdzichu | right now, use 'cp'
boucman_work | your second unit would be an empty .service, then you create a .service.d/
             | directory and in there you put a symlink (renamed .conf) to your first unit
boucman_work | as I said : ugly
boucman_work | or put all the common parts in a .conf file, and symlink in both .d files (a
             | bit cleaner, but not exaclty what you asked)

So there's a deprecated way of doing so, and a hack to get the same effect, but nothing that looks ideal.


# Specifies the maximum number of processes
LimitNPROC=4096

# Specifies the maximum size of virtual memory
LimitAS=infinity

# Specifies the maximum file size
LimitFSIZE=infinity

# Disable timeout logic and wait until process is stopped
TimeoutStopSec=0

# SIGTERM signal is used to stop the Java process
KillSignal=SIGTERM

# Send the signal only to the JVM rather than its control group
KillMode=process

# Java process is never killed
SendSIGKILL=no

# When a JVM receives a SIGTERM signal it exits with code 143
SuccessExitStatus=143

[Install]
WantedBy=multi-user.target

# Built for ${project.name}-${project.version} (${project.name})