daemon: Add socket activation via /run/rpm-ostreed.socket

For historical reasons, the daemon ends up doing a lot of
initialization before even claiming the DBus name.  For example,
it calls `ostree_sysroot_load()`.  We also end up scanning
the RPM database, and actually parse all the GPG keys
in `/etc/pki/rpm-gpg` etc.

This causes two related problems:

- By doing all this work before claiming the bus name, we
  race against the (pretty low) DBus service timeout of 25s.
- If something hard fails at initialization, the client can't
  easily see the error because it just appears in the systemd
  journal.  The client will just see a service timeout.

The solution to this is to adopt systemd socket activation,
which drops out DBus as an intermediary.  On daemon startup,
we now do the process-global initialization (like ostree
sysroot) and if that fails, the daemon just sticks around
(but without claiming the bus name), ready to return the
error message to each client.

After this patch:

```
$ systemctl stop rpm-ostreed
$ umount /boot
$ rpm-ostree status
error: Couldn't start daemon: Error setting up sysroot: loading sysroot: Unexpected state: /run/ostree-booted found, but no /boot/loader directory
```

Author: cgwalters

Makefile-daemon.am

@@ -63,14 +63,15 @@ systemdunit_service_in_files = \
 	$(NULL)
 
 systemdunit_service_files = $(systemdunit_service_in_files:.service.in=.service)
-systemdunit_timer_files = \
+systemdunit_other_files = \
+	$(srcdir)/src/daemon/rpm-ostreed.socket \
 	$(srcdir)/src/daemon/rpm-ostreed-automatic.timer \
 	$(srcdir)/src/daemon/rpm-ostree-countme.timer \
 	$(NULL)
 
 systemdunit_DATA = \
 	$(systemdunit_service_files) \
-	$(systemdunit_timer_files) \
+	$(systemdunit_other_files) \
 	$(NULL)
 
 systemdunitdir       = $(prefix)/lib/systemd/system/
@@ -110,7 +111,7 @@ EXTRA_DIST += \
 	$(sysconf_DATA) \
 	$(service_in_files) \
 	$(systemdunit_service_in_files) \
-	$(systemdunit_timer_files) \
+	$(systemdunit_other_files) \
 	$(NULL)
 
 CLEANFILES += \

rust/src/daemon.rs

@@ -5,7 +5,11 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 
 use crate::{cxxrsutil::*, variant_utils};
+use anyhow::{anyhow, Context, Result};
 use openat_ext::OpenatDirExt;
+use std::io::prelude::*;
+use std::os::unix::net::{UnixListener, UnixStream};
+use std::os::unix::prelude::FromRawFd;
 use std::pin::Pin;
 
 /// Validate basic assumptions on daemon startup.
@@ -26,6 +30,108 @@ pub(crate) fn daemon_sanitycheck_environment(
     Ok(())
 }
 
+/// Connect to the client socket and ensure the daemon is initialized;
+/// this avoids DBus and ensures that we get any early startup errors
+/// returned cleanly.
+pub(crate) fn start_daemon_via_socket() -> CxxResult<()> {
+    let address = "/run/rpm-ostree/client.sock";
+    let s = UnixStream::connect(address)
+        .with_context(|| anyhow!("Failed to connect to {}", address))?;
+    let mut s = std::io::BufReader::new(s);
+    let mut r = String::new();
+    s.read_to_string(&mut r)
+        .context("Reading from client socket")?;
+    if r.is_empty() {
+        Ok(())
+    } else {
+        Err(anyhow!("{}", r).into())
+    }
+}
+
+fn send_init_result_to_client(client: &UnixStream, err: &Result<()>) {
+    let mut client = std::io::BufWriter::new(client);
+    match err {
+        Ok(_) => {
+            // On successwe close the stream without writing anything,
+            // which acknowledges successful startup to the client.
+        }
+        Err(e) => {
+            let msg = e.to_string();
+            match client
+                .write_all(msg.as_bytes())
+                .and_then(|_| client.flush())
+            {
+                Ok(_) => {}
+                Err(inner_err) => {
+                    eprintln!(
+                        "Failed to write error message to client socket (original error: {}): {}",
+                        e, inner_err
+                    );
+                }
+            }
+        }
+    }
+}
+
+fn process_clients(listener: UnixListener, res: &Result<()>) {
+    for stream in listener.incoming() {
+        match stream {
+            Ok(stream) => send_init_result_to_client(&stream, res),
+            Err(e) => {
+                // This shouldn't be fatal, we continue to start up.
+                eprintln!("Failed to listen for client stream: {}", e);
+            }
+        }
+        if res.is_err() {
+            break;
+        }
+    }
+}
+
+/// Perform initialization steps required by systemd service activation.
+///
+/// This ensures that the system is running under systemd, then receives the
+/// socket-FD for main IPC logic, and notifies systemd about ready-state.
+pub(crate) fn daemon_main(debug: bool) -> Result<()> {
+    if !systemd::daemon::booted()? {
+        return Err(anyhow!("not running as a systemd service"));
+    }
+
+    let init_res: Result<()> = crate::ffi::daemon_init_inner(debug).map_err(|e| e.into());
+
+    let mut fds = systemd::daemon::listen_fds(false)?.iter();
+    let listener = match fds.next() {
+        None => {
+            // If started directly via `systemctl start` or DBus activation, we
+            // directly propagate the error back to our exit code.
+            init_res?;
+            UnixListener::bind("/run/rpmostreed.socket")?
+        }
+        Some(fd) => {
+            if fds.next().is_some() {
+                return Err(anyhow!("Expected exactly 1 fd from systemd activation"));
+            }
+            let listener = unsafe { UnixListener::from_raw_fd(fd) };
+            match init_res {
+                Ok(_) => listener,
+                Err(e) => {
+                    let err_copy = Err(anyhow!("{}", e));
+                    process_clients(listener, &err_copy);
+                    return Err(e);
+                }
+            }
+        }
+    };
+
+    // On success, we spawn a helper thread that just responds with
+    // sucess to clients that connect via the socket.  In the future,
+    // perhaps we'll expose an API here.
+    std::thread::spawn(move || process_clients(listener, &Ok(())));
+
+    // And now, enter the main loop.
+    Ok(crate::ffi::daemon_main_inner()?)
+}
+
 /// Get a currently unique (for this host) identifier for the
 /// deployment; TODO - adding the deployment timestamp would make it
 /// persistently unique, needs API in libostree.

rust/src/lib.rs

@@ -181,6 +181,8 @@ pub mod ffi {
     // daemon.rs
     extern "Rust" {
         fn daemon_sanitycheck_environment(sysroot: Pin<&mut OstreeSysroot>) -> Result<()>;
+        fn daemon_main(debug: bool) -> Result<()>;
+        fn start_daemon_via_socket() -> Result<()>;
         fn deployment_generate_id(deployment: Pin<&mut OstreeDeployment>) -> String;
         fn deployment_populate_variant(
             mut sysroot: Pin<&mut OstreeSysroot>,
@@ -468,6 +470,12 @@ pub mod ffi {
         fn main_print_error(msg: &str);
     }
 
+    unsafe extern "C++" {
+        include!("rpmostreed-daemon.h");
+        fn daemon_init_inner(debug: bool) -> Result<()>;
+        fn daemon_main_inner() -> Result<()>;
+    }
+
     unsafe extern "C++" {
         include!("rpmostree-clientlib.h");
         fn client_require_root() -> Result<()>;

src/app/libmain.cxx

@@ -296,6 +296,8 @@ rpmostree_option_context_parse (GOptionContext *context,
             }
         }
 
+      rpmostreecxx::start_daemon_via_socket();
+
       /* root never needs to auth */
       if (getuid () != 0)
         /* ignore errors; we print out a warning if we fail to spawn pkttyagent */

src/app/rpmostree-builtin-start-daemon.cxx

@@ -33,6 +33,7 @@
 
 #include "rpmostree-builtins.h"
 #include "rpmostree-util.h"
+#include "rpmostreed-utils.h"
 #include "rpmostreed-daemon.h"
 #include "rpmostree-libbuiltin.h"
 
@@ -53,6 +54,7 @@ static GOptionEntry opt_entries[] =
   { NULL }
 };
 
+static GDBusConnection *bus = NULL;
 static RpmostreedDaemon *rpm_ostree_daemon = NULL;
 
 static void
@@ -225,20 +227,14 @@ on_log_handler (const gchar *log_domain,
   sd_journal_print (priority, "%s", message);
 }
 
-gboolean
-rpmostree_builtin_start_daemon (int             argc,
-                                char          **argv,
-                                RpmOstreeCommandInvocation *invocation,
-                                GCancellable   *cancellable,
-                                GError        **error)
+namespace rpmostreecxx {
+// This function is always called from the Rust side.  Hopefully
+// soon we'll move more of this code into daemon.rs.
+void
+daemon_init_inner (bool debug) 
 {
-  g_autoptr(GOptionContext) opt_context = g_option_context_new (" - start the daemon process");
-  g_option_context_add_main_entries (opt_context, opt_entries, NULL);
-
-  if (!g_option_context_parse (opt_context, &argc, &argv, error))
-    return FALSE;
-
-  if (opt_debug)
+  g_autoptr(GError) local_error = NULL;
+  if (debug)
     {
       g_autoptr(GIOChannel) channel = NULL;
       g_log_set_handler (G_LOG_DOMAIN, (GLogLevelFlags)(G_LOG_LEVEL_DEBUG | G_LOG_LEVEL_INFO), on_log_debug, NULL);
@@ -258,15 +254,21 @@ rpmostree_builtin_start_daemon (int             argc,
   g_unix_signal_add (SIGTERM, on_sigint, NULL);
 
   /* Get an explicit ref to the bus so we can use it later */
-  g_autoptr(GDBusConnection) bus = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, error);
+  g_autoptr(GDBusConnection) bus = g_bus_get_sync (G_BUS_TYPE_SYSTEM, NULL, &local_error);
   if (!bus)
-    return FALSE;
-  if (!start_daemon (bus, error))
-    return FALSE;
+    util::throw_gerror(local_error);
+  if (!start_daemon (bus, &local_error))
+    util::throw_gerror(local_error);
+}
 
+// Called from rust side to enter mainloop.
+void
+daemon_main_inner () 
+{
   state_transition (APPSTATE_RUNNING);
 
   g_debug ("Entering main event loop");
+  g_assert (rpm_ostree_daemon);
   rpmostreed_daemon_run_until_idle_exit (rpm_ostree_daemon);
 
   if (bus)
@@ -300,6 +302,23 @@ rpmostree_builtin_start_daemon (int             argc,
   g_autoptr(GMainContext) mainctx = g_main_context_default ();
   while (appstate == APPSTATE_FLUSHING)
     g_main_context_iteration (mainctx, TRUE);
+}
+} /* namespace */
+
+gboolean
+rpmostree_builtin_start_daemon (int             argc,
+                                char          **argv,
+                                RpmOstreeCommandInvocation *invocation,
+                                GCancellable   *cancellable,
+                                GError        **error)
+{
+  g_autoptr(GOptionContext) opt_context = g_option_context_new (" - start the daemon process");
+  g_option_context_add_main_entries (opt_context, opt_entries, NULL);
+
+  if (!g_option_context_parse (opt_context, &argc, &argv, error))
+    return FALSE;
+
+  rpmostreecxx::daemon_main (opt_debug);
 
   return TRUE;
 }

src/daemon/rpm-ostreed.socket

@@ -0,0 +1,7 @@
+[Socket]
+ListenStream=/run/rpm-ostree/client.sock
+ConditionKernelCommandLine=ostree
+SocketMode=0600
+
+[Install]
+WantedBy=sockets.target

src/daemon/rpmostreed-daemon.h

@@ -36,6 +36,12 @@ G_BEGIN_DECLS
 #define RPMOSTREE_DRIVER_SD_UNIT "driver-sd-unit"
 #define RPMOSTREE_DRIVER_NAME "driver-name"
 
+/* Note: Currently actually defined in rpmostree-builtin-start-daemon.cxx for historical reasons */
+namespace rpmostreecxx {
+void daemon_init_inner (bool debug);
+void daemon_main_inner ();
+}
+
 GType              rpmostreed_daemon_get_type       (void) G_GNUC_CONST;
 RpmostreedDaemon * rpmostreed_daemon_get            (void);
 GDBusConnection  * rpmostreed_daemon_connection     (void);