Bug#877512: slapd: enabled systemd integration (untested patch)
On Wed, Jun 28, 2023 at 01:03:33PM -0700, Ryan Tandy wrote: SLAPD_CONF is also used (at least) by anyone who still uses a slapd.conf file instead of cn=config. Using -f or -F depending on what SLAPD_CONF points to was the main reason I assumed we'd need a wrapper script. But that could also be replaced by a drop-in. I remembered later that the maintainer scripts also need to know the location of the config, for upgrades. Currently they get it by reading SLAPD_CONF from /etc/default/slapd. Not sure whether it would be feasible to parse that out from the slapd command line in a systemd unit. Probably not in the general case... (but of course a determined person can find ways to break the current setup as well.)
Bug#877512: slapd: enabled systemd integration (untested patch)
Am Wed, Jun 28, 2023 at 01:03:33PM -0700 schrieb Ryan Tandy: > Hmm. So on upgrade I suppose we would want to automatically migrate those > settings to a drop-in? That actually sounds doable; such a drop-in would > probably not have to be a conffile. Indeed, so my idea was that e.g. the systemd unit would default to User=openldap and Group=openldap and then the postinst could check if /etc/default/slapd has SLAPD_GROUP and/or SLAPD_USER set to something other than "openlap" and in that case a drop-in would be generated with those settings. Similar for Kerberos etc. > > The most commonly used option is probably SLAPD_OPTIONS, which could also > > be read via an EnvironmentFile from /etc/default. > > Right. Although if that's the only thing still being consumed, I'd be > tempted to just let it go too. :) Actually, that's a fair point, then there would be a clean cut so that it's obvious that /etc/default/slapd is only relevant for folks not using systemd. Again, SLAPD_OPTIONS could easily also be a drop-in after all. > Thanks for the input, it really does help. :) Glad to help! Cheers, Moritz
Bug#877512: slapd: enabled systemd integration (untested patch)
--On Thursday, June 29, 2023 12:27 PM +0200 Andreas Henriksson wrote: Feel free to file a bug upstream if you think the current configure.ac code needs adjustment. [...] It's my impression that configure.ac is missing a call to: PKG_PROG_PKG_CONFIG(0.29) Thus the PKG_CONFIG variable will be unset, and thus the PKG_CHECK_* macros will just skip over and do nothing. FWIW you do have this: m4_ifndef([PKG_PREREQ], [m4_fatal([must install pkg-config 0.29 or later before running autoconf/autogen])]) ... but that only seems to check that pkg.m4 is new enough, not that the actual pkg-config binary/utility exists. Adding `PKG_PROG_PKG_CONFIG(0.29)` directly after the m4_ifndef and rebuilding gave me the expected systemdsystemunitdir=/lib/systemd/system (as systemd.pc says on debian) rather than the hardcoded fallbacks. Please file a bug at https://bugs.openldap.org :) Regards, Quanah
Bug#877512: slapd: enabled systemd integration (untested patch)
Hello, On Wed, Jun 28, 2023 at 09:53:06AM -0700, Quanah Gibson-Mount wrote: > > > --On Wednesday, June 28, 2023 10:49 AM -0700 Ryan Tandy > wrote: > > > > > > TODO: For unknown reason configure seems to want to use > > > /usr/lib/systemd/system (rather than /lib/systemd/system) despite the > > > precense of systemd.pc ... the configure script has hard-coded fallback > > > paths... > > > > Thanks for noting this, definitely sounds like something we need to look > > into. > > Feel free to file a bug upstream if you think the current configure.ac code > needs adjustment. [...] It's my impression that configure.ac is missing a call to: PKG_PROG_PKG_CONFIG(0.29) Thus the PKG_CONFIG variable will be unset, and thus the PKG_CHECK_* macros will just skip over and do nothing. FWIW you do have this: m4_ifndef([PKG_PREREQ], [m4_fatal([must install pkg-config 0.29 or later before running autoconf/autogen])]) ... but that only seems to check that pkg.m4 is new enough, not that the actual pkg-config binary/utility exists. Adding `PKG_PROG_PKG_CONFIG(0.29)` directly after the m4_ifndef and rebuilding gave me the expected systemdsystemunitdir=/lib/systemd/system (as systemd.pc says on debian) rather than the hardcoded fallbacks. Regards, Andreas Henriksson
Bug#877512: slapd: enabled systemd integration (untested patch)
--On Wednesday, June 28, 2023 2:03 PM -0700 Ryan Tandy wrote: Another (lower priority) thing I meant to look into is the sd_notify(3) support. Enabling that means changing the service type and adding the -d flag to stop slapd from detaching. Yep, you want -d 0 specifically. --Quanah
Bug#877512: slapd: enabled systemd integration (untested patch)
On Wed, Jun 28, 2023 at 08:48:14PM +0200, Moritz Mühlenhoff wrote: OTOH, moving to the systemd unit might also be a good opportunity to reduce some complexity? Looking at slapd.default shipped with the current package SLAPD_SENTINEL_FILE, SLAPD_PIDFILE and SLAPD_NO_START are all settings which are no longer relevant with a systemd unit or can equally be achieved with commands built-in to systemd (e.g. systemctl mask). Yes, definitely meaning to review and drop at least those, as well as migrating /var/run/slapd to tmpfiles.d. Then there's a handful of settings which IMHO probably very people actually modify (SLAPD_USER, SLAPD_USER, SLAPD_CONF, SLAPD_SERVICES) and which folks wanting to modify can always tweak with a local unit override/dropins. Hmm. So on upgrade I suppose we would want to automatically migrate those settings to a drop-in? That actually sounds doable; such a drop-in would probably not have to be a conffile. SLAPD_CONF is also used (at least) by anyone who still uses a slapd.conf file instead of cn=config. Using -f or -F depending on what SLAPD_CONF points to was the main reason I assumed we'd need a wrapper script. But that could also be replaced by a drop-in. The most commonly used option is probably SLAPD_OPTIONS, which could also be read via an EnvironmentFile from /etc/default. Right. Although if that's the only thing still being consumed, I'd be tempted to just let it go too. :) Another (lower priority) thing I meant to look into is the sd_notify(3) support. Enabling that means changing the service type and adding the -d flag to stop slapd from detaching. Thanks for the input, it really does help. :) Ryan
Bug#877512: slapd: enabled systemd integration (untested patch)
Am Wed, Jun 28, 2023 at 09:49:06AM -0700 schrieb Ryan Tandy: > On Wed, Jun 28, 2023 at 06:29:31PM +0200, Andreas Henriksson wrote: > > I'm attaching a patch which has only been compile-tested as I don't > > use slapd myself. It would be great if someone who uses slapd could > > pick it up, test it and finish the remaining work. > > Thanks for the patch and for doing the compile-testing. Unfortunately > upstream's service file won't work for us as is. The remaining work includes > (and this is the part I've been procrastinating) extracting from the init > script the parts that determine the arguments to slapd (based on config from > /etc/default/slapd, and I think in some cases possibly from the slapd config > too), and turning that into a slapd launcher script that the service will > have to invoke. OTOH, moving to the systemd unit might also be a good opportunity to reduce some complexity? Looking at slapd.default shipped with the current package SLAPD_SENTINEL_FILE, SLAPD_PIDFILE and SLAPD_NO_START are all settings which are no longer relevant with a systemd unit or can equally be achieved with commands built-in to systemd (e.g. systemctl mask). Then there's a handful of settings which IMHO probably very people actually modify (SLAPD_USER, SLAPD_USER, SLAPD_CONF, SLAPD_SERVICES) and which folks wanting to modify can always tweak with a local unit override/dropins. The most commonly used option is probably SLAPD_OPTIONS, which could also be read via an EnvironmentFile from /etc/default. Cheers, Moritz
Bug#877512: slapd: enabled systemd integration (untested patch)
--On Wednesday, June 28, 2023 10:49 AM -0700 Ryan Tandy wrote: TODO: For unknown reason configure seems to want to use /usr/lib/systemd/system (rather than /lib/systemd/system) despite the precense of systemd.pc ... the configure script has hard-coded fallback paths... Thanks for noting this, definitely sounds like something we need to look into. Feel free to file a bug upstream if you think the current configure.ac code needs adjustment. PKG_CHECK_VAR(systemdsystemunitdir, systemd, systemdsystemunitdir) if test -z "$systemdsystemunitdir"; then if test -d /usr/lib/systemd/system; then systemdsystemunitdir=/usr/lib/systemd/system else systemdsystemunitdir=/lib/systemd/system fi fi --Quanah
Bug#877512: slapd: enabled systemd integration (untested patch)
On Wed, Jun 28, 2023 at 06:29:31PM +0200, Andreas Henriksson wrote: I'm attaching a patch which has only been compile-tested as I don't use slapd myself. It would be great if someone who uses slapd could pick it up, test it and finish the remaining work. Thanks for the patch and for doing the compile-testing. Unfortunately upstream's service file won't work for us as is. The remaining work includes (and this is the part I've been procrastinating) extracting from the init script the parts that determine the arguments to slapd (based on config from /etc/default/slapd, and I think in some cases possibly from the slapd config too), and turning that into a slapd launcher script that the service will have to invoke. TODO: For unknown reason configure seems to want to use /usr/lib/systemd/system (rather than /lib/systemd/system) despite the precense of systemd.pc ... the configure script has hard-coded fallback paths... Thanks for noting this, definitely sounds like something we need to look into. thanks, Ryan
Bug#877512: slapd: enabled systemd integration (untested patch)
Hello, I'm attaching a patch which has only been compile-tested as I don't use slapd myself. It would be great if someone who uses slapd could pick it up, test it and finish the remaining work. TODO: For unknown reason configure seems to want to use /usr/lib/systemd/system (rather than /lib/systemd/system) despite the precense of systemd.pc ... the configure script has hard-coded fallback paths... This can possibly cause problems with debhelper integration picking it up properly. Feel free to investigate! :) Regards, Andreas Henriksson >From 2ccd6781e2bb10dda99be0c4d0ad3c599fd96a20 Mon Sep 17 00:00:00 2001 From: Andreas Henriksson Date: Wed, 28 Jun 2023 18:13:45 +0200 Subject: [PATCH] Enable systemd integration Build-dep on: - libsystemd-dev for systemd/sd-daemon.h - systemd-dev for systemd.pc Use configure flag --with-systemd when building services (slapd). --- debian/control | 2 ++ debian/rules | 2 ++ debian/slapd.install | 2 ++ 3 files changed, 6 insertions(+) diff --git a/debian/control b/debian/control index 9b27cf69f..7c0baabb1 100644 --- a/debian/control +++ b/debian/control @@ -15,6 +15,8 @@ Build-Depends: debhelper-compat (= 12), libltdl-dev , libperl-dev (>= 5.8.0) , libsasl2-dev, + libsystemd-dev , + systemd-dev , libwrap0-dev , nettle-dev , openssl , diff --git a/debian/rules b/debian/rules index 7992a133d..94599e108 100755 --- a/debian/rules +++ b/debian/rules @@ -25,6 +25,8 @@ ifeq ($(DEB_HOST_ARCH_OS),hurd) endif ifneq ($(filter pkg.openldap.noslapd,$(DEB_BUILD_PROFILES)),) CONFIG += --disable-slapd +else + CONFIG += --with-systemd endif CONTRIB_MODULES = autogroup lastbind passwd passwd/pbkdf2 passwd/sha2 smbk5pwd diff --git a/debian/slapd.install b/debian/slapd.install index c5f7946cd..e4cde1359 100644 --- a/debian/slapd.install +++ b/debian/slapd.install @@ -1,3 +1,5 @@ +usr/lib/systemd/system/slapd.service + etc/ldap/schema usr/lib/slapd usr/sbin usr/lib/*/libslapi-*.so.* -- 2.39.2
