dnsdist: Add support for systemd-notify #3677

Merged
merged 3 commits into from Apr 13, 2016

Projects

None yet

3 participants

@pieterlexis
Member

We now also install our dnsdist.service file from make install.

Needs review

@rgacogne
Member
rgacogne commented Apr 8, 2016

Nice! Perhaps we could also fallback to 'simple' if systemd support is not available, like weakforced does since 1?

@pieterlexis
Member

After pondering this, I feel there is no reason to generate a service file at all if there are no systemd(-dev) libraries installed, as the program is not intended to be run inside of systemd. We can stick a Type=simple service file in the contrib dir and have some documentation about this.

@Habbie Habbie and 1 other commented on an outdated diff Apr 11, 2016
pdns/dnsdist.cc
@@ -1475,6 +1479,14 @@ try
break;
}
}
+
+#ifdef HAVE_SYSTEMD
+ char *sock;
+ sock = getenv("NOTIFY_SOCKET");
+ if(sock != NULL) // Are we indeed running inside of systemd?
+ g_cmdLine.beSupervised=true;
+#endif
@Habbie
Habbie Apr 11, 2016 Member

This can be overridden from the commandline still, I presume?

@pieterlexis
pieterlexis Apr 11, 2016 Member

not if your actually running inside systemd. There is no other way (as daemonizing will break systemd's detection of a startup and not being supervised will spawn a console with a closed stdin, leading to program termination) to do this. We could just exit(1) here and print a message asking the user to start with --supervised

@rgacogne rgacogne and 1 other commented on an outdated diff Apr 11, 2016
pdns/dnsdistdist/m4/systemd.m4
+ AC_SUBST(SYSTEMD_MODULES_LOAD)
+])
+
+AC_DEFUN([AX_ENABLE_SYSTEMD_OPTS], [
+ AX_ARG_DEFAULT_ENABLE([systemd], [Disable systemd support])
+ AX_SYSTEMD_OPTIONS()
+])
+
+AC_DEFUN([AX_ALLOW_SYSTEMD_OPTS], [
+ AX_ARG_DEFAULT_DISABLE([systemd], [Enable systemd support])
+ AX_SYSTEMD_OPTIONS()
+])
+
+AC_DEFUN([AX_CHECK_SYSTEMD_LIBS], [
+ AC_CHECK_HEADER([systemd/sd-daemon.h], [
+ AC_CHECK_LIB([systemd-daemon], [sd_listen_fds], [libsystemddaemon="y"])
@rgacogne
rgacogne Apr 11, 2016 Member

This seems to fail on Arch because libsystemd-daemon.so has been merged into libsystemd.so a while ago (looks like it was in version 209).

@pieterlexis
pieterlexis Apr 12, 2016 Member

Even centos 7's pkg-config mentions it as 'deprecated'. Debian jessie has systemd 215, so I'll just change the systemd-daemon to systemd

@Habbie Habbie commented on an outdated diff Apr 11, 2016
pdns/dnsdistdist/m4/systemd.m4
@@ -0,0 +1,148 @@
+# systemd.m4 - Macros to check for and enable systemd -*- Autoconf -*-
+#
+# Copyright (C) 2014 Luis R. Rodriguez <mcgrof@suse.com>
+#
@Habbie
Habbie Apr 11, 2016 Member

Can we note the origin (including revision) of this file (or whatever we replace it with to fix @rgacogne's report)?

pieterlexis added some commits Apr 12, 2016
@pieterlexis pieterlexis dnsdist: Import systemd autoconf macros ac0d6ee
@pieterlexis pieterlexis dnsdist: Add systemd notify support 6ab6522
@pieterlexis pieterlexis dnsdist: patch the systemd.m4
This patch is twofold:
1 - It detects the correct library for sd-daemon (either
libsystemd-daemon (systemd pre-209) or libsystemd (systemd post-209)
2 - Detection of the library and files is only run once, even when
systemd support detection is automatic (when using AX_AVAILABLE_SYSTEMD)
a1c0a38
@pieterlexis
Member

I fixed up this PR so our packages use this and tested the resulting packages

@rgacogne rgacogne merged commit 369bec1 into PowerDNS:master Apr 13, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pieterlexis pieterlexis deleted the pieterlexis:dnsdist-systemd-notify-support branch Apr 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment