[2.4.x] Merge changes to mod_systemd

changes-entries/systemd-watchdog.txt

@@ -0,0 +1,2 @@
+  *) mod_systemd: Support the systemd watchdog, sending the keep-alive
+     notification if the service unit sets WatchdogSec=.  [Joe Orton]

docs/manual/mod/mod_systemd.xml

@@ -32,7 +32,7 @@
 <summary>
     <p>This module provides support for systemd integration. It allows
     httpd to be used in a service with the systemd
-    <code>Type=notify</code> (see <a
+    <code>Type=notify</code> or <code>Type=notify-reload</code> (see <a
     href="https://www.freedesktop.org/software/systemd/man/systemd.service.html">systemd.service(5)</a>
     for more information).  The module is activated if loaded.</p>
 
@@ -69,13 +69,90 @@ WantedBy=multi-user.target
    href="https://www.freedesktop.org/software/systemd/man/systemd.kill.html">systemd.kill(5)</a>
    for more information.</p>
 
-   <p>This module does not provide support for Systemd socket activation.</p>
+   <p>A service manager from systemd 253 onwards offers
+   <code>Type=notify-reload</code>, which is worth using in preference.
+   Under <code>Type=notify</code> a <code>systemctl reload</code>
+   returns as soon as the <code>ExecReload</code> command has sent its
+   signal, which is before the new configuration has been read, and it
+   reports success whatever becomes of the restart afterwards.
+   <code>Type=notify-reload</code> instead holds the reload open until
+   the server reports it finished, so the command waits for the new
+   configuration to be in use and fails if it never is.  mod_systemd
+   sends the <code>RELOADING=1</code> notification the protocol expects
+   while the configuration is being read, stamped with the
+   <code>MONOTONIC_USEC</code> the service manager requires, and
+   <code>READY=1</code> once it has been loaded.</p>
+
+    <example>
+    <title>Example of a service unit which reloads synchronously</title>
+    <pre>
+[Service]
+Type=notify-reload
+ReloadSignal=SIGCONT
+ExecStart=/usr/local/apache2/bin/httpd -D FOREGROUND -k start
+ExecReload=/usr/local/apache2/bin/httpd -k graceful
+KillMode=mixed
+    </pre>
+    </example>
+
+   <p>The service manager runs <code>ExecReload</code> first, and sends
+   the signal named by <code>ReloadSignal</code> only once that command
+   has exited successfully.  Keeping <code>ExecReload</code> is what
+   makes the reload safe: <code>httpd -k graceful</code> parses the new
+   configuration in a process of its own and exits without signalling
+   anything if it does not parse, so the reload fails and the running
+   server carries on with the configuration it already has.  Leaving
+   <code>ExecReload</code> out, and letting the service manager signal
+   the server directly, gives up that check: the running parent reads
+   the new configuration itself, and a configuration which does not
+   parse makes it exit, taking the server down.</p>
+
+   <p>The signal sent after <code>ExecReload</code> has run is then
+   redundant, so <code>ReloadSignal</code> should name one httpd does
+   not act on, such as <code>SIGCONT</code>.  It matters that it is set:
+   the default is <code>SIGHUP</code>, which httpd takes as an
+   <em>ungraceful</em> restart, dropping the connections a reload is
+   meant to preserve.  A unit which does leave out
+   <code>ExecReload</code> must set <code>ReloadSignal=SIGUSR1</code>,
+   the signal httpd restarts gracefully on.</p>
+
+   <p>Systemd socket activation is supported if httpd was built with
+   it.  Each <directive module="mpm_common">Listen</directive> port
+   must then be one passed in by systemd; a port which was not is a
+   fatal configuration error rather than one httpd opens for itself.
+   Socket activation is used only if this module is loaded, so it can
+   be built in and left unused.</p>
 
     <p><directive module="core">ExtendedStatus</directive> is
     enabled by default if the module is loaded.  If <directive
     module="core">ExtendedStatus</directive> is not disabled in
     the configuration, run-time load and request statistics are made
     available in the <code>systemctl status</code> output.</p>
+
+    <p>The systemd watchdog is supported.  If the service unit sets
+    <code>WatchdogSec=</code>, the parent process sends the keep-alive
+    notification which tells systemd the server is still alive; a server
+    which stops sending it is terminated and, with a suitable
+    <code>Restart=</code> setting, restarted.  The notification is sent
+    while the configuration is being read and again once it is loaded, so
+    that a reload is covered, and periodically from the parent process
+    while the server runs.</p>
+
+    <p>That periodic notification is sent about every ten seconds, which
+    is how often the parent process runs the hook it is sent from.  A
+    <code>WatchdogSec=</code> of less than twice that cannot be met, and
+    would have systemd terminating a server which is working normally;
+    such a setting is reported as a warning at startup.  Use a
+    <code>WatchdogSec=</code> of at least 20 seconds.</p>
+
+    <example>
+    <title>Adding watchdog supervision to either unit above</title>
+    <pre>
+[Service]
+WatchdogSec=30
+Restart=on-failure
+    </pre>
+    </example>
 </summary>
 
 </modulesynopsis>

modules/arch/unix/mod_systemd.c

@@ -16,6 +16,7 @@
  */
 
 #include <stdint.h>
+#include <time.h>
 #include <ap_config.h>
 #include "ap_mpm.h"
 #include "ap_listen.h"
@@ -39,12 +40,65 @@
 #include <unistd.h>
 #endif
 
+/* Microseconds on the clock systemd compares RELOADING=1 against, or
+ * zero if it cannot be read. */
+static apr_uint64_t monotonic_usec(void)
+{
+    struct timespec ts;
+
+    if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
+        return 0;
+    }
+    return (apr_uint64_t)ts.tv_sec * APR_USEC_PER_SEC + ts.tv_nsec / 1000;
+}
+
+/* ap_run_monitor() is called once every INTERVAL_OF_WRITABLE_PROBES turns
+ * of the parent's one second loop in ap_wait_or_timeout(), so that is how
+ * often a keep-alive notification can be sent, and the shortest watchdog
+ * timeout which can be met is twice that: sd_watchdog_enabled(3) asks for
+ * a notification every half of the configured timeout. */
+#define WATCHDOG_INTERVAL_SEC (10)
+
+/* The WatchdogSec= of the service in microseconds, or zero if the service
+ * manager is not watching.  Set in pre_config, before the first
+ * notification which could carry a keep-alive. */
+static apr_uint64_t watchdog_usec;
+
+/* A keep-alive assignment to paste into a notification, or nothing while
+ * the service manager is not asking for one.  Sending WATCHDOG=1 when it
+ * is not expected is harmless, but saying so only when asked keeps what
+ * httpd reports the same as what the service was configured for. */
+static const char *watchdog_ping(void)
+{
+    return watchdog_usec ? "WATCHDOG=1\n" : "";
+}
+
 static int systemd_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
                               apr_pool_t *ptemp)
 {
-    sd_notify(0,
-              "RELOADING=1\n"
-              "STATUS=Reading configuration...\n");
+    apr_uint64_t usec = monotonic_usec(), wd_usec;
+
+    /* Read afresh on each configuration load, since a restart unloads and
+     * loads the module again, and without unsetting it as server/listen.c
+     * does for $LISTEN_FDS, which would stop the keep-alive at the first
+     * reload. */
+    watchdog_usec = sd_watchdog_enabled(0, &wd_usec) > 0 ? wd_usec : 0;
+
+    /* A Type=notify-reload service ignores a reload notification which
+     * does not say when it was sent. */
+    if (usec) {
+        sd_notifyf(0,
+                   "RELOADING=1\n"
+                   "MONOTONIC_USEC=%" APR_UINT64_T_FMT "\n"
+                   "%s"
+                   "STATUS=Reading configuration...\n", usec, watchdog_ping());
+    }
+    else {
+        sd_notifyf(0,
+                   "RELOADING=1\n"
+                   "%s"
+                   "STATUS=Reading configuration...\n", watchdog_ping());
+    }
     ap_extended_status = 1;
     return OK;
 }
@@ -63,6 +117,17 @@ static void log_selinux_context(void)
 }
 #endif
 
+/* pconf is also cleared on a restart, where the service is not stopping
+ * at all, so distinguish the two by the state of the process. */
+static apr_status_t systemd_stopping(void *unused)
+{
+    if (ap_state_query(AP_SQ_MAIN_STATE) == AP_SQ_MS_EXITING) {
+        sd_notify(0, "STOPPING=1\n"
+                  "STATUS=Shutting down.\n");
+    }
+    return APR_SUCCESS;
+}
+
 /* Report the service is ready in post_config, which could be during
  * startup or after a reload.  The server could still hit a fatal
  * startup error after this point during ap_run_mpm(), so this is
@@ -80,8 +145,33 @@ static int systemd_post_config(apr_pool_t *pconf, apr_pool_t *plog,
     log_selinux_context();
 #endif
 
-    sd_notify(0, "READY=1\n"
-              "STATUS=Configuration loaded.\n");
+    /* Not reached by "httpd -k stop" and friends, which signal the
+     * running server and exit before post_config. */
+    apr_pool_cleanup_register(pconf, NULL, systemd_stopping,
+                              apr_pool_cleanup_null);
+
+    /* A timeout the parent cannot meet would have the service manager
+     * killing a healthy server every WatchdogSec, so say so rather than
+     * leaving nothing in the log to explain it. */
+    if (watchdog_usec
+        && watchdog_usec / 2 < (apr_uint64_t)WATCHDOG_INTERVAL_SEC
+                               * APR_USEC_PER_SEC) {
+        ap_log_error(APLOG_MARK, APLOG_WARNING, 0, main_server, APLOGNO(10621)
+                     "WatchdogSec is %" APR_UINT64_T_FMT "us, but keep-alive "
+                     "notifications are sent from the parent process only "
+                     "every %ds; configure a WatchdogSec of at least %ds or "
+                     "the service will be killed while it is healthy",
+                     watchdog_usec, WATCHDOG_INTERVAL_SEC,
+                     2 * WATCHDOG_INTERVAL_SEC);
+    }
+
+    /* The keep-alive rides along with the notification which ends a
+     * reload: the configuration is read outside the parent's monitor loop,
+     * so nothing reports while it is being parsed, and the service manager
+     * keeps the timeout armed throughout. */
+    sd_notifyf(0, "READY=1\n"
+               "%s"
+               "STATUS=Configuration loaded.\n", watchdog_ping());
     return OK;
 }
 
@@ -100,31 +190,64 @@ static int systemd_monitor(apr_pool_t *p, server_rec *s)
     apr_interval_time_t up_time;
     char bps[5];
 
+    /* Before anything which might decline: reporting the server is alive
+     * does not depend on there being a status line to report with it. */
+    if (watchdog_usec) {
+        sd_notify(0, "WATCHDOG=1\n");
+    }
+
     if (!ap_extended_status) {
         /* Nothing useful to report with ExtendedStatus disabled. */
         return DECLINED;
     }
 
     ap_get_sload(&sload);
-    /* up_time in seconds */
-    up_time = (apr_uint32_t) apr_time_sec(apr_time_now() -
-                               ap_scoreboard_image->global->restart_time);
+    /* up_time in seconds, and never zero: a restart resets restart_time,
+     * so this hook can run in the same second it was set. */
+    up_time = apr_time_sec(apr_time_now() -
+                           ap_scoreboard_image->global->restart_time);
+    if (up_time < 1) {
+        up_time = 1;
+    }
 
-    apr_strfsize((unsigned long)((float) (sload.bytes_served)
-                                 / (float) up_time), bps);
+    apr_strfsize(sload.bytes_served / up_time, bps);
 
+    /* ap_get_sload() gives idle and busy as percentages of the workers
+     * available, not as counts. */
     sd_notifyf(0, "READY=1\n"
-               "STATUS=Total requests: %lu; Idle/Busy workers %d/%d;"
+               "STATUS=Total requests: %lu; Idle/Busy workers %d%%/%d%%; "
                "Requests/sec: %.3g; Bytes served/sec: %sB/sec\n",
                sload.access_count, sload.idle, sload.busy,
                ((float) sload.access_count) / (float) up_time, bps);
 
     return DECLINED;
 }
 
+/* The number of sockets passed by the service manager has to be
+ * remembered: the configuration is read again on restart, by which time
+ * the environment sd_listen_fds() reads has been cleared, and the module
+ * itself has been unloaded and loaded again.  Hence retained data rather
+ * than a static. */
+static const char *const retained_key = "mod_systemd_listen_fds";
+
+static int ap_systemd_listen_fds(int unset_environment)
+{
+    int *fds = ap_retained_data_get(retained_key);
+
+    if (fds == NULL) {
+        fds = ap_retained_data_create(retained_key, sizeof(*fds));
+        *fds = sd_listen_fds(0);
+    }
+    if (unset_environment) {
+        /* Take the variables out of the environment, keeping the count. */
+        sd_listen_fds(1);
+    }
+    return *fds;
+}
+
 static int ap_find_systemd_socket(process_rec * process, apr_port_t port) {
-    int fdcount, fd;
-    int sdc = sd_listen_fds(0);
+    int fd;
+    int sdc = ap_systemd_listen_fds(0);
 
     if (sdc < 0) {
         ap_log_perror(APLOG_MARK, APLOG_CRIT, sdc, process->pool, APLOGNO(02486)
@@ -139,8 +262,7 @@ static int ap_find_systemd_socket(process_rec * process, apr_port_t port) {
         return -1;
     }
 
-    fdcount = atoi(getenv("LISTEN_FDS"));
-    for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + fdcount; fd++) {
+    for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + sdc; fd++) {
         if (sd_is_socket_inet(fd, 0, 0, -1, port) > 0) {
             return fd;
         }
@@ -149,10 +271,6 @@ static int ap_find_systemd_socket(process_rec * process, apr_port_t port) {
     return -1;
 }
 
-static int ap_systemd_listen_fds(int unset_environment){
-    return sd_listen_fds(unset_environment);
-}
-
 static void systemd_register_hooks(apr_pool_t *p)
 {
     APR_REGISTER_OPTIONAL_FN(ap_systemd_listen_fds);