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

Cargo.toml

@@ -51,7 +51,7 @@ cap-std-ext = "2.0"
 cap-std = { version = "1.0.3", features = ["fs_utf8"] }
 containers-image-proxy = "0.5.1"
 # Explicitly force on libc
-rustix = { version = "0.37", features = ["use-libc"] }
+rustix = { version = "0.37", features = ["use-libc", "net"] }
 cap-primitives = "1.0.3"
 chrono = { version = "0.4.26", features = ["serde"] }
 clap = { version = "4.3", features = ["derive"] }

Makefile-daemon.am

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

rpmostree-cxxrs.cxx

@@ -8,6 +8,7 @@
 #include "rpmostree-package-variants.h"
 #include "rpmostree-rpm-util.h"
 #include "rpmostree-util.h"
+#include "rpmostreed-daemon.hpp"
 #include "rpmostreemain.h"
 #include "src/libpriv/rpmostree-cxxrs-prelude.h"
 #include <algorithm>
@@ -2195,6 +2196,10 @@ extern "C"
                                                ::rust::Str opt_deploy_id, ::rust::Str opt_os_name,
                                                ::rpmostreecxx::OstreeDeployment **return$) noexcept;
 
+  ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$daemon_main (bool debug) noexcept;
+
+  void rpmostreecxx$cxxbridge1$daemon_terminate () noexcept;
+
   ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$daemon_sanitycheck_environment (
       const ::rpmostreecxx::OstreeSysroot &sysroot) noexcept;
 
@@ -2970,6 +2975,34 @@ extern "C"
     return throw$;
   }
 
+  ::rust::repr::PtrLen
+  rpmostreecxx$cxxbridge1$daemon_init_inner (bool debug) noexcept
+  {
+    void (*daemon_init_inner$) (bool) = ::rpmostreecxx::daemon_init_inner;
+    ::rust::repr::PtrLen throw$;
+    ::rust::behavior::trycatch (
+        [&] {
+          daemon_init_inner$ (debug);
+          throw$.ptr = nullptr;
+        },
+        ::rust::detail::Fail (throw$));
+    return throw$;
+  }
+
+  ::rust::repr::PtrLen
+  rpmostreecxx$cxxbridge1$daemon_main_inner () noexcept
+  {
+    void (*daemon_main_inner$) () = ::rpmostreecxx::daemon_main_inner;
+    ::rust::repr::PtrLen throw$;
+    ::rust::behavior::trycatch (
+        [&] {
+          daemon_main_inner$ ();
+          throw$.ptr = nullptr;
+        },
+        ::rust::detail::Fail (throw$));
+    return throw$;
+  }
+
   ::rust::repr::PtrLen
   rpmostreecxx$cxxbridge1$client_require_root () noexcept
   {
@@ -4038,6 +4071,22 @@ deployment_get_base (::rpmostreecxx::OstreeSysroot &sysroot, ::rust::Str opt_dep
   return ::std::move (return$.value);
 }
 
+void
+daemon_main (bool debug)
+{
+  ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$daemon_main (debug);
+  if (error$.ptr)
+    {
+      throw ::rust::impl< ::rust::Error>::error (error$);
+    }
+}
+
+void
+daemon_terminate () noexcept
+{
+  rpmostreecxx$cxxbridge1$daemon_terminate ();
+}
+
 void
 daemon_sanitycheck_environment (const ::rpmostreecxx::OstreeSysroot &sysroot)
 {

rpmostree-cxxrs.h

@@ -8,6 +8,7 @@
 #include "rpmostree-package-variants.h"
 #include "rpmostree-rpm-util.h"
 #include "rpmostree-util.h"
+#include "rpmostreed-daemon.hpp"
 #include "rpmostreemain.h"
 #include "src/libpriv/rpmostree-cxxrs-prelude.h"
 #include <algorithm>
@@ -1843,6 +1844,10 @@ ::rpmostreecxx::OstreeDeployment *deployment_get_base (::rpmostreecxx::OstreeSys
                                                        ::rust::Str opt_deploy_id,
                                                        ::rust::Str opt_os_name);
 
+void daemon_main (bool debug);
+
+void daemon_terminate () noexcept;
+
 void daemon_sanitycheck_environment (const ::rpmostreecxx::OstreeSysroot &sysroot);
 
 ::rust::String deployment_generate_id (const ::rpmostreecxx::OstreeDeployment &deployment) noexcept;

rust/src/client.rs

@@ -6,12 +6,14 @@ use crate::core::OSTREE_BOOTED;
 use crate::cxxrsutil::*;
 use crate::ffi::SystemHostType;
 use crate::utils;
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, Context, Result};
+use camino::Utf8Path;
 use fn_error_context::context;
 use gio::prelude::*;
 use ostree_ext::{gio, glib};
 use std::io::{BufRead, Write};
 use std::os::unix::io::IntoRawFd;
+use std::os::unix::net::UnixStream;
 use std::process::Command;
 
 /// The well-known bus name.
@@ -20,6 +22,7 @@ const BUS_NAME: &str = "org.projectatomic.rpmostree1";
 const SYSROOT_PATH: &str = "/org/projectatomic/rpmostree1/Sysroot";
 const OS_INTERFACE: &str = "org.projectatomic.rpmostree1.OS";
 const OS_EX_INTERFACE: &str = "org.projectatomic.rpmostree1.OSExperimental";
+const CLIENT_SOCKET_PATH: &str = "/run/rpm-ostree/client.sock";
 
 /// A unique DBus connection to the rpm-ostree daemon.
 /// This currently wraps a C++ client connection.
@@ -49,7 +52,8 @@ impl ClientConnection {
             SYSROOT_PATH,
             "org.projectatomic.rpmostree1.Sysroot",
             gio::Cancellable::NONE,
-        )?;
+        )
+        .context("Initializing sysroot proxy")?;
         // Today the daemon mode requires running inside a booted deployment.
         let booted = sysroot_proxy
             .cached_property("Booted")
@@ -156,46 +160,63 @@ pub(crate) fn client_handle_fd_argument(
     }
 }
 
-/// Explicitly ensure the daemon is started via systemd, if possible.
-///
-/// This works around bugs from DBus activation, see
-/// https://github.com/coreos/rpm-ostree/pull/2932
-///
-/// Basically we load too much data before claiming the bus name,
-/// and dbus doesn't give us a useful error.  Instead, let's talk
-/// to systemd directly and use its client tools to scrape errors.
-///
-/// What we really should do probably is use native socket activation.
-pub(crate) fn client_start_daemon() -> CxxResult<()> {
-    let service = "rpm-ostreed.service";
-    // Assume non-root can't use systemd right now.
-    if rustix::process::getuid().as_raw() != 0 {
-        return Ok(());
+// If the client socket doesn't exist, try to ask systemd to start it.
+// This can happen even when the socket unit is installed because
+// presets may not enable it.
+fn check_and_start_daemon_socket() -> Result<bool> {
+    if Utf8Path::new(CLIENT_SOCKET_PATH).exists() {
+        return Ok(true);
     }
-    // Unfortunately, RHEL8 systemd will count "systemctl start"
-    // invocations against the restart limit, so query the status
-    // first.
-    let activeres = Command::new("systemctl")
-        .args(&["is-active", "rpm-ostreed"])
+    let socket_unit = "rpm-ostreed.socket";
+    tracing::debug!("{CLIENT_SOCKET_PATH} does not exist, explicitly starting socket unit");
+    let start_result = Command::new("systemctl")
+        .args(&["--no-ask-password", "start", socket_unit])
         .output()?;
-    // Explicitly don't check the error return value, we don't want to
-    // hard fail on it.
-    if String::from_utf8_lossy(&activeres.stdout).starts_with("active") {
-        // It's active, we're done.  Note that while this is a race
-        // condition, that's fine because it will be handled by DBus
-        // activation.
+    if !start_result.status.success() {
+        let err = String::from_utf8_lossy(&start_result.stderr);
+        tracing::warn!("Failed to start {socket_unit}: {err}");
+        return Ok(false);
+    }
+    Ok(true)
+}
+
+/// 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.
+#[context("Starting daemon via socket")]
+fn start_daemon_via_socket() -> Result<()> {
+    let capable = check_and_start_daemon_socket()?;
+    if !capable {
+        tracing::debug!("Falling back to DBus activation");
         return Ok(());
     }
-    let res = Command::new("systemctl")
-        .args(&["--no-ask-password", "start", service])
-        .status()?;
-    if !res.success() {
-        let _ = Command::new("systemctl")
-            .args(&["--no-pager", "status", service])
-            .status();
-        return Err(anyhow!("{}", res).into());
+
+    tracing::debug!("Starting daemon via {CLIENT_SOCKET_PATH}");
+    let mut socket = UnixStream::connect(CLIENT_SOCKET_PATH)?;
+    crate::daemon::write_message(
+        &mut socket,
+        crate::daemon::SocketMessage::ClientHello {
+            selfid: crate::core::self_id()?,
+        },
+    )
+    .context("Writing ClientHello")?;
+    let resp = crate::daemon::recv_message(&mut socket)?;
+    match resp {
+        crate::daemon::SocketMessage::ServerOk => Ok(()),
+        crate::daemon::SocketMessage::ServerError { msg } => {
+            Err(anyhow!("server error: {msg}").into())
+        }
+        o => Err(anyhow!("unexpected message: {o:?}").into()),
     }
-    Ok(())
+}
+
+pub(crate) fn client_start_daemon() -> CxxResult<()> {
+    // systemctl and socket paths only work for root right now; in the future
+    // the socket may be opened up.
+    if rustix::process::getuid().as_raw() != 0 {
+        return Ok(());
+    }
+    return start_daemon_via_socket().map_err(Into::into);
 }
 
 /// Convert the GVariant parameters from the DownloadProgress DBus API to a human-readable English string.

rust/src/core.rs

@@ -350,6 +350,14 @@ fn stage_container_rpm_files(rpms: Vec<File>) -> CxxResult<Vec<String>> {
     Ok(r)
 }
 
+/// Return an opaque identifier for the current executing binary.  This can
+/// be passed via IPC to verify that client and server are running the same code.
+pub(crate) fn self_id() -> Result<String> {
+    use std::os::unix::fs::MetadataExt;
+    let metadata = std::fs::metadata("/proc/self/exe").context("Failed to read /proc/self/exe")?;
+    Ok(format!("dev={};inode={}", metadata.dev(), metadata.ino()))
+}
+
 #[cfg(test)]
 mod test {
     use super::*;