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

PR - Issue 49875 - Move SystemD service config to a drop-in file #3469

Closed
389-ds-bot opened this issue Sep 13, 2020 · 13 comments
Closed

PR - Issue 49875 - Move SystemD service config to a drop-in file #3469

389-ds-bot opened this issue Sep 13, 2020 · 13 comments
Labels
merged Migration flag - PR pr Migration flag - PR

Comments

@389-ds-bot
Copy link

389-ds-bot commented Sep 13, 2020

Cloned from Pagure Pull-Request: https://pagure.io/389-ds-base/pull-request/50411

  • Created at 2019-05-29 17:07:50 by mhonek (@kenoh)
  • Merged at 2019-06-03 14:35:05

Bug Description:
Runtime configuration options are mixed into the service specification
which should seldom be changed by users.

Fix Description:
Move the runtime configuration options into a drop-in file. These options
are then automatically pull in by SystemD.

Additional Info:
Erasing the default values of the mentioned options to implicitly pull in
system defaults which are more sane nowadays.

Related Resolves: #2934

Author: Matus Honek kenoh@redhat.com

Review by: ???

@389-ds-bot 389-ds-bot added merged Migration flag - PR pr Migration flag - PR labels Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2019-05-29 17:17:54

I was thinking of dropping systemd.template.xsan.service.in completely and use a drop-in file for Environment options for ASAN/MSAN/TSAN instead of maintaining two virtually identical files. WDYT?

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-05-30 04:17:47

It's not possible as you can't make certain sections "drop in" IIRC. I experimented with this and chose the split file method to avoid this. I could be mistaken here or it's changed since, but previous attempts were not valid unit files :(

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-05-30 04:18:46

Is it possible to show an "example" drop in, or link to the systemd.template.service.custom.conf.in somehow to guide admins to the "how" to achivee this?

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2019-05-30 06:46:57

It's not possible as you can't make certain sections "drop in" IIRC. I experimented with this and chose the split file method to avoid this. I could be mistaken here or it's changed since, but previous attempts were not valid unit files :(

I'm sorry to disappoint, but this is exactly what drop-in files are for: to override certain parameters in certain sections. This functionality is in systemd for quite a while.

Here's an example:

[vashirov@server ~]$ cat .config/systemd/user/hello.service
[Unit]
Description=Service that prints a greeting

[Service]
ExecStart=/usr/bin/echo "Hello ${WORLD}"
Environment=WORLD=World

[Install]
WantedBy=default.target

[vashirov@server ~]$ systemctl --user daemon-reload
[vashirov@server ~]$ systemctl --user start hello
[vashirov@server ~]$ journalctl --user -u hello
-- Logs begin at Thu 2019-05-30 00:35:33 EDT, end at Thu 2019-05-30 00:37:39 EDT. --
May 30 00:37:39 server.example.com systemd[1039]: Started Service that prints a greeting.
May 30 00:37:39 server.example.com echo[1085]: Hello World
May 30 00:37:39 server.example.com systemd[1039]: hello.service: Succeeded.

Now let's create an override:

[vashirov@server ~]$ systemctl --user edit hello

This will open an $EDITOR, but you can create drop-in directory by hand an place a .conf file there yourself.

[vashirov@server ~]$ cat .config/systemd/user/hello.service.d/override.conf
[Unit]
Description=Service with an overrided description

[Service]
Environment=WORLD="Down Under"

[vashirov@server ~]$ systemctl --user start hello
[vashirov@server ~]$ journalctl --user -u hello
-- Logs begin at Thu 2019-05-30 00:35:33 EDT, end at Thu 2019-05-30 00:39:00 EDT. --
May 30 00:37:39 server.example.com systemd[1039]: Started Service that prints a greeting.
May 30 00:37:39 server.example.com echo[1085]: Hello World
May 30 00:37:39 server.example.com systemd[1039]: hello.service: Succeeded.
May 30 00:39:00 server.example.com systemd[1039]: Started Service with an overrided description.
May 30 00:39:00 server.example.com echo[1096]: Hello Down Under
May 30 00:39:00 server.example.com systemd[1039]: hello.service: Succeeded.

Is it possible to show an "example" drop in, or link to the systemd.template.service.custom.conf.in somehow to guide admins to the "how" to achivee this?

We can point them to man systemd.unit where it is described.

@389-ds-bot
Copy link
Author

Comment from mhonek (@kenoh) at 2019-05-30 13:42:25

1 new commit added

  • Change xsan service into a drop-in file

@389-ds-bot
Copy link
Author

Comment from mhonek (@kenoh) at 2019-05-30 13:45:12

I've added a change as Viktor suggested. @Firstyear Please have a look. This drop-in file will be installed in case the server is built with a sanitizer and automatically picked up by systemd.

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-06-03 09:49:08

It's not possible as you can't make certain sections "drop in" IIRC. I experimented with this and chose the split file method to avoid this. I could be mistaken here or it's changed since, but previous attempts were not valid unit files :(

I'm sorry to disappoint, but this is exactly what drop-in files are for: to override certain parameters in certain sections. This functionality is in systemd for quite a while.

This is not my point. I know how drop in's work. My point was that drop in files may not function for some configuration sections in the unitfile. IIRC if you try to drop in over exec or certain other parameters you have to make a new unit file because you can't replace that statement. So if this works, whatever you are "replacing" is not falling under these rules. Additionally my comment was to ask you to check that it actually was working, because there are some conditions where the drop in is not used because of the requirement that you have to use or replace the section.

So if this is working for you, and tested, then great. But bear in mind there are some things that can not be drop-in overriden ...

We can point them to man systemd.unit where it is described.

I think I mean that our unitfile should say something like:

# To configure this, create a drop in file in:
# /etc/systemd/system/dirsrv.whatever.target.d/example.com
# It's contents should be from ...
# For more see <man page> 

@389-ds-bot
Copy link
Author

Comment from mhonek (@kenoh) at 2019-06-03 10:17:01

... IIRC if you try to drop in over exec or certain other parameters you have to make a new unit file because you can't replace that statement. ...

IIUC what your concern is, you can do e.g. ExecStart= (that is, without a value) in a drop-in file first which resets the value and subsequent assignmnets would build the value of it again.

... So if this is working for you, and tested, then great. But bear in mind there are some things that can not be drop-in overriden ...

At least me and Mark have successfully tested this to find it working as expected.

I think I mean that our unitfile should say something like:

The main .service file contains pointer to the drop-in file where supposed handling is described -- do you find this sufficient? Suggestions to particular changes are welcome. :)

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-06-03 10:46:38

Yep, that's fine, if it's all tested then my concerns are covered.

@389-ds-bot
Copy link
Author

Comment from mhonek (@kenoh) at 2019-06-03 10:48:19

rebased onto 4981ebf6a44cb0c3dd1a091786ee0a58eeeaccfe

@389-ds-bot
Copy link
Author

Comment from mhonek (@kenoh) at 2019-06-03 14:32:59

rebased onto 10bffac

@389-ds-bot
Copy link
Author

Comment from mhonek (@kenoh) at 2019-06-03 14:35:05

Pull-Request has been merged by kenoh

@389-ds-bot
Copy link
Author

Patch
50411.patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged Migration flag - PR pr Migration flag - PR
Projects
None yet
Development

No branches or pull requests

1 participant