[Bug 126] New: [systemd] Add installation of systemd unit file via build-sys
https://bugzilla.balabit.com/show_bug.cgi?id=126 Summary: [systemd] Add installation of systemd unit file via build-sys Product: syslog-ng Version: 3.2.x Platform: PC OS/Version: Linux Status: NEW Severity: normal Priority: unspecified Component: syslog-ng AssignedTo: bazsi@balabit.hu ReportedBy: d@falconindy.com Type of the Report: enhancement Estimated Hours: 0.0 I'd like to see syslog-ng offer installation of the systemd unit file via the build system, similar to how other services (e.g., udev or rsyslog) provide this, with the --with-systemdsystemunitdir configure option. autotools is unfortunately not my forte, so the attached patch may not be the most eloquent of solutions (borrowing bits from udev), but it does the job, and does not appear to bother configurations which avoid --enable-systemd or the new --with flag. I felt it was prudent to separate the --enable-systemd flag from this new flag as there's certainly a case for using the --enable flag without the --with flag. I've also provided a one line amendment to the service file itself, which removes the ExecReload. This is discouraged by upstream, as systemd is unable to properly track the behavior of the daemon due to the effects of an asynchronous trigger. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 --- Comment #1 from Dave Reisner <d@falconindy.com> 2011-06-25 04:45:44 --- Created an attachment (id=37) --> (https://bugzilla.balabit.com/attachment.cgi?id=37) service file amendment -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 --- Comment #2 from Balazs Scheidler <bazsi@balabit.hu> 2011-06-26 11:47:43 --- I miss the patch that adds the --with-systemdsystemunitdir option, I can only see the one that removes the ExecReload setting Is the suggested --with option a pattern/best-practice? Because if it isn't, I'd prefer to use a more readable option, like: --with-systemd-unit-dir Isn't it perhaps possible to query this from systemd? Also regarding the service file patch, I've read the linked discussion, and although I understand why async reloads are not the best, that thread is not talking about configuration files (which syslog-ng does without restarts), but rather an interesting reload mechanism implemented by lighttpd, which involves stopping and starting a new lighttpd instance. And removing that line from the config will mean that it's impossible to trigger a reload with systemd, which could break log rotation for example. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 --- Comment #3 from Dave Reisner <dreisner@archlinux.org> 2011-06-26 14:18:51 --- Created an attachment (id=38) --> (https://bugzilla.balabit.com/attachment.cgi?id=38) Add --with-systemdsystemunitdir build option Oops, I did forget to attach. I agree, the option name sucks, but it seems to have been standardized. The patch uses pkg-config to pull in a default path, but there's surely a use case for building syslog-ng without systemd present so the override is necessary. I guess in the case of the ExecReload it does seem slightly different from Lighttpd because the same process continues. That said, systemd upstream had the same response to the ExecReload in rsyslog (which has similar behavior to syslog-ng on HUP) and asked for it to be removed, which it subsequently was. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 --- Comment #4 from Balazs Scheidler <bazsi@balabit.hu> 2011-06-26 16:48:19 --- (In reply to comment #3)
Created an attachment (id=38) --> (https://bugzilla.balabit.com/attachment.cgi?id=38) [details] Add --with-systemdsystemunitdir build option
Oops, I did forget to attach. I agree, the option name sucks, but it seems to have been standardized. The patch uses pkg-config to pull in a default path, but there's surely a use case for building syslog-ng without systemd present so the override is necessary.
hmm... I was wondering what the proper default for this option would be. Certainly for packages unit files would need to go to /lib/systemd/system, but when the admin is compiling his own package, the proper place would rather be /etc/systemd/system, ain't that right? Do packages specify this option explicitly on the configure command line, or should configure prefer the package location over the user location?
I guess in the case of the ExecReload it does seem slightly different from Lighttpd because the same process continues. That said, systemd upstream had the same response to the ExecReload in rsyslog (which has similar behavior to syslog-ng on HUP) and asked for it to be removed, which it subsequently was.
syslog-ng 3.4 does have an option to initiate reload with a unix domain socket, but that still isn't synchronous (yet). How should a user initiate HUP based reloads using systemd then? I understand that this is not a perfect solution, but was good enough for quite some time. I wouldn't want to add synchronous reloads to syslog-ng 3.2 (which is not where development happens), though it might be viable for 3.4 So for the interim I'd leave ExecReload and change it to the alternative when we get there. What do you think? -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 --- Comment #5 from Dave Reisner <dreisner@archlinux.org> 2011-06-26 17:02:44 --- (In reply to comment #4)
hmm... I was wondering what the proper default for this option would be. Certainly for packages unit files would need to go to /lib/systemd/system, but when the admin is compiling his own package, the proper place would rather be /etc/systemd/system, ain't that right?
Do packages specify this option explicitly on the configure command line, or should configure prefer the package location over the user location?
Right. For distribution, /lib/systemd/system should be used, allowing /etc/systemd/system to be used as an override, or for configuration by a system administrator. For example, I have a bunch of my own units that I keep in /etc/systemd/system (that are specific to my own machine). In the arch ecosystem, systemd is not our default init, so any package that provides a service file will be ./configure'd with this option defined as /lib/systemd/system. If you're configuring and compiling outside the confines of a package manager, I'd say /etc/systemd is probably the right place to drop the service file. You wouldn't want this suddenly overwritten by a package providing the same service file.
I guess in the case of the ExecReload it does seem slightly different from Lighttpd because the same process continues. That said, systemd upstream had the same response to the ExecReload in rsyslog (which has similar behavior to syslog-ng on HUP) and asked for it to be removed, which it subsequently was.
syslog-ng 3.4 does have an option to initiate reload with a unix domain socket, but that still isn't synchronous (yet).
How should a user initiate HUP based reloads using systemd then?
The purist would tell you to avoid the reload entirely and instead do a full restart. A high volume logging server is probably not going to be happy about doing a restart instead of a simple reload.
I understand that this is not a perfect solution, but was good enough for quite some time. I wouldn't want to add synchronous reloads to syslog-ng 3.2 (which is not where development happens), though it might be viable for 3.4
So for the interim I'd leave ExecReload and change it to the alternative when we get there.
What do you think?
So I guess, personally, I agree. Leave it in, and let administrators decide whether to use it or not. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 Tom Gundersen <teg@jklm.no> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |teg@jklm.no --- Comment #6 from Tom Gundersen <teg@jklm.no> 2011-06-26 17:49:16 --- @Balazs,Dave: it's great to see work on systemd support in syslog-ng! A few (independent) comments: 1) the installation directories upstream should care about are: - /lib/systemd/system, where the unit files should be installed by the syslog-ng package, and - /usr/local/lib/systemd/system, where the unit files should be installed when you compile your own package. We should never install anything into /etc/systemd/system, this is for the admin only. 2) Just a quick note about something I noticed in #125: we should not need to alter syslog.socket. The correct behavior is that syslog.sockt, should start systemd-kmsg-syslogd.service if nothing is running, and when syslog-ng.service/rsyslog.service is started they take over the socket. At least that's the way I understood it. 3) The reason ExecReload was removed from rsyslog.service was that rsyslog does not support HUP at all <http://git.adiscon.com/?p=rsyslog.git;a=commit;h=fd26a42bdc04eaf497cafd9ef806a54f3de1a7e9>. While a synchronous reload would be ideal, I agree that keeping the HUP stuff is the correct thing to do for the time being. Looking forward to this work trickling down to the Arch packages soon :-) -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 --- Comment #7 from Balazs Scheidler <bazsi@balabit.hu> 2011-06-27 07:59:13 --- Hmm.. I've found this description: https://fedoraproject.org/wiki/User:Johannbg/QA/Systemd/Daemon It states that: "At the build installation time (e.g. make install during package build) packages are recommended to install their systemd unit files in the directory returned by pkg-config systemd --variable=systemdsystemunitdir (for system services), resp. pkg-config systemd --variable=systemdsessionunitdir (for session services). This will make the services available in the system on explicit request but not activate them automatically during boot. Optionally, during package installation (e.g. rpm -i by the administrator) symlinks should be created in the systemd configuration directories via the enable command of the systemctl(1) tool, to activate them automatically on boot. " I'm not sure I agree, however I wouldn't want to create a non-standard option, that looks like a standard one. I have another concern: if systemd is present on the system, and my development computer would support it, then the Makefiles would try to install it unconditionally to /lib/systemd/system, which would cause permission denied errors, since I'm doing development as a non-root user. Also, the hooks you used don't support "make uninstall", which automake otherwise does. Here's the currently experimental patch I've came up with, which: * does nothing with the unit file by default * if --with-systemdsystemunitdir is specified without arguments, it tries to autodetect it using pkg-config * if --without-systemdsystemunitdir is specified, it does nothing * if --with-systemdsystemunitdir is specified with an argument, it uses that directory Can you please check if this works for you? Also, can you please voice my concerns about the suggested implementation to the systemd developers? Thanks. Here's the patch currently (not yet on master): http://git.balabit.hu/?p=bazsi/syslog-ng-3.3.git;a=shortlog;h=systemd-unit-i... -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 --- Comment #8 from Dave Reisner <dreisner@archlinux.org> 2011-06-27 14:43:26 --- (In reply to comment #7)
Hmm.. I've found this description:
https://fedoraproject.org/wiki/User:Johannbg/QA/Systemd/Daemon
It states that:
"At the build installation time (e.g. make install during package build) packages are recommended to install their systemd unit files in the directory returned by pkg-config systemd --variable=systemdsystemunitdir (for system services), resp. pkg-config systemd --variable=systemdsessionunitdir (for session services). This will make the services available in the system on explicit request but not activate them automatically during boot. Optionally, during package installation (e.g. rpm -i by the administrator) symlinks should be created in the systemd configuration directories via the enable command of the systemctl(1) tool, to activate them automatically on boot. "
I'm not sure I agree, however I wouldn't want to create a non-standard option, that looks like a standard one.
Forgive me, I'm not sure what your concern is here, other than the awkward naming convention.
I have another concern: if systemd is present on the system, and my development computer would support it, then the Makefiles would try to install it unconditionally to /lib/systemd/system, which would cause permission denied errors, since I'm doing development as a non-root user.
Are you saying that a make install command such as the below ignores DESTDIR? make DESTDIR=/path/to/dev/root install I can't reproduce this, regardless of the value passed to --with-systemdsystemunitdir. DESTDIR is definitely honored both using your patch and mine.
Also, the hooks you used don't support "make uninstall", which automake otherwise does.
That's my lack of experience with autotools showing through. Completely unintended.
Here's the currently experimental patch I've came up with, which: * does nothing with the unit file by default * if --with-systemdsystemunitdir is specified without arguments, it tries to autodetect it using pkg-config * if --without-systemdsystemunitdir is specified, it does nothing * if --with-systemdsystemunitdir is specified with an argument, it uses that directory
Awesome. With the currently advertised autotools macro, specifying no argument to --with-ssud (getting tired of typing that) will result in no units being installed.
Can you please check if this works for you? Also, can you please voice my concerns about the suggested implementation to the systemd developers? Thanks.
Here's the patch currently (not yet on master):
http://git.balabit.hu/?p=bazsi/syslog-ng-3.3.git;a=shortlog;h=systemd-unit-i...
This is great, and it does work for me. Just curious though, could this a candidate for backport to 3.2, or is it too big of a change to be considered for a stable release? I will happily pass along the improvement to the autotools macro. It definitely makes sense. However, I believe that the Wiki you reference is more of a guideline, and not necessarily gospel. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 Balazs Scheidler <bazsi@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution| |FIXED Status|NEW |RESOLVED --- Comment #9 from Balazs Scheidler <bazsi@balabit.hu> 2011-07-01 16:29:47 --- (In reply to comment #8)
(In reply to comment #7)
Hmm.. I've found this description:
https://fedoraproject.org/wiki/User:Johannbg/QA/Systemd/Daemon
It states that:
"At the build installation time (e.g. make install during package build) packages are recommended to install their systemd unit files in the directory returned by pkg-config systemd --variable=systemdsystemunitdir (for system services), resp. pkg-config systemd --variable=systemdsessionunitdir (for session services). This will make the services available in the system on explicit request but not activate them automatically during boot. Optionally, during package installation (e.g. rpm -i by the administrator) symlinks should be created in the systemd configuration directories via the enable command of the systemctl(1) tool, to activate them automatically on boot. "
I'm not sure I agree, however I wouldn't want to create a non-standard option, that looks like a standard one.
Forgive me, I'm not sure what your concern is here, other than the awkward naming convention.
I was just writing down my thoughts. The thing was: I didn't quite agree to install service file from a .tar.gz installation into /lib/systemd/system, I've figured that tarball installation is more similar to installing a home-grewn script, therefore the defaults should go to /etc/systemd/system as if the user was doing it on her own. But going directly against documented "best practice" is not a good idea: creating a standard (--with-systemdsystemunitdir) option, which would have a different default than the documented best practice is sure to invite confusion. But anyway, with the change in option processing to only do installation if the user explicitly requests it, my concern is gone.
I have another concern: if systemd is present on the system, and my development computer would support it, then the Makefiles would try to install it unconditionally to /lib/systemd/system, which would cause permission denied errors, since I'm doing development as a non-root user.
Are you saying that a make install command such as the below ignores DESTDIR?
make DESTDIR=/path/to/dev/root install
I can't reproduce this, regardless of the value passed to --with-systemdsystemunitdir. DESTDIR is definitely honored both using your patch and mine.
No, but I'm not using DESTDIR when doing development. I simply "make install" to a $HOME relative prefix, but since systemd unit dir is ignoring $prefix (which it should be doing), the unit file would be copied to /lib/systemd/system, even if I wasn't the root user.
Also, the hooks you used don't support "make uninstall", which automake otherwise does.
That's my lack of experience with autotools showing through. Completely unintended.
No problem :)
Here's the currently experimental patch I've came up with, which: * does nothing with the unit file by default * if --with-systemdsystemunitdir is specified without arguments, it tries to autodetect it using pkg-config * if --without-systemdsystemunitdir is specified, it does nothing * if --with-systemdsystemunitdir is specified with an argument, it uses that directory
Awesome. With the currently advertised autotools macro, specifying no argument to --with-ssud (getting tired of typing that) will result in no units being installed.
ok.
Can you please check if this works for you? Also, can you please voice my concerns about the suggested implementation to the systemd developers? Thanks.
Here's the patch currently (not yet on master):
http://git.balabit.hu/?p=bazsi/syslog-ng-3.3.git;a=shortlog;h=systemd-unit-i...
This is great, and it does work for me.
Thanks. Committed for 3.3: commit db89491b07e2d6541a52b8ea75a5153d2c28f0a9 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Fri Jul 1 16:24:57 2011 +0200 add support for installing systemd unit files This patch adds a standard --with-systemdsystemunitdir configure option that: * without explicit options it does nothing with the unit file * if --with-systemdsystemunitdir is specified without arguments, it tries to autodetect the path using pkg-config * if --without-systemdsystemunitdir is specified, it does nothing * if --with-systemdsystemunitdir is specified with an argument, it uses that directory to install the unit file into Signed-off-by: Dave Reisner <dreisner@archlinux.org> Signed-off-by: Balazs Scheidler <bazsi@balabit.hu>
Just curious though, could this a candidate for backport to 3.2, or is it too big of a change to be considered for a stable release?
I've backported it to the 3.2 tree, can you please check if it works there too? There was a minor conflict, but I resolved it. Thanks. commit e2961e1c738c8b02ca6057ac34415536830dd0e9 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Fri Jul 1 16:28:02 2011 +0200 -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=126 --- Comment #10 from Dave Reisner <dreisner@archlinux.org> 2011-07-01 17:15:55 --- This is great, Bazsi. The backport to 3.2 checks out for me. Thanks for implementing this. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
participants (1)
-
bugzilla＠bugzilla.balabit.com