Skip to content

Commit

Permalink
daemon: Add socket activation via /run/rpm-ostreed.socket
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
cgwalters committed Jul 25, 2022
1 parent f16ebf4 commit 1b41c5f
Show file tree
Hide file tree
Showing 7 changed files with 194 additions and 22 deletions.
7 changes: 4 additions & 3 deletions Makefile-daemon.am
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down Expand Up @@ -110,7 +111,7 @@ EXTRA_DIST += \
$(sysconf_DATA) \
$(service_in_files) \
$(systemdunit_service_in_files) \
$(systemdunit_timer_files) \
$(systemdunit_other_files) \
$(NULL)

CLEANFILES += \
Expand Down
109 changes: 106 additions & 3 deletions rust/src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,19 @@ use crate::cxxrsutil::*;
use crate::ffi::{
OverrideReplacementSource, OverrideReplacementType, ParsedRevision, ParsedRevisionKind,
};
use anyhow::{anyhow, format_err, Result};
use anyhow::{anyhow, format_err, Result, Context};
use cap_std::fs::Dir;
use cap_std_ext::dirext::CapStdExtDirExt;
use cap_std_ext::{cap_std, rustix};
use fn_error_context::context;
use glib::prelude::*;
use ostree_ext::{gio, glib, ostree};
use rustix::fd::BorrowedFd;
use rustix::fd::{BorrowedFd, FromRawFd};
use rustix::fs::MetadataExt;
use std::collections::{BTreeMap, BTreeSet};
use std::io::Read;
use std::io::{Read, Write};
use std::os::unix::fs::PermissionsExt;
use std::os::unix::net::{UnixStream, UnixListener};
use std::path::Path;

const RPM_OSTREED_COMMIT_VERIFICATION_CACHE: &str = "rpm-ostree/gpgcheck-cache";
Expand Down Expand Up @@ -131,6 +132,108 @@ fn deployment_populate_variant_origin(
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()?)
}

/// Serialize information about the given deployment into the `dict`;
/// this will be exposed via DBus and is hence public API.
pub(crate) fn deployment_populate_variant(
Expand Down
8 changes: 8 additions & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,8 @@ pub mod ffi {

// daemon.rs
extern "Rust" {
fn daemon_main(debug: bool) -> Result<()>;
fn start_daemon_via_socket() -> Result<()>;
fn daemon_sanitycheck_environment(sysroot: &OstreeSysroot) -> Result<()>;
fn deployment_generate_id(deployment: &OstreeDeployment) -> String;
fn deployment_populate_variant(
Expand Down Expand Up @@ -762,6 +764,12 @@ pub mod ffi {
fn c_unit_tests() -> Result<()>;
}

unsafe extern "C++" {
include!("rpmostreed-daemon.hpp");
fn daemon_init_inner(debug: bool) -> Result<()>;
fn daemon_main_inner() -> Result<()>;
}

unsafe extern "C++" {
include!("rpmostree-clientlib.h");
fn client_require_root() -> Result<()>;
Expand Down
2 changes: 2 additions & 0 deletions src/app/libmain.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@ rpmostree_option_context_parse (GOptionContext *context, const GOptionEntry *mai
return rpmostreecxx::client_throw_non_ostree_host_error (error);
}

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 */
Expand Down
53 changes: 37 additions & 16 deletions src/app/rpmostree-builtin-start-daemon.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "rpmostree-builtins.h"
#include "rpmostree-libbuiltin.h"
#include "rpmostree-util.h"
#include "rpmostreed-utils.h"
#include "rpmostreed-daemon.h"

typedef enum
Expand All @@ -53,6 +54,7 @@ static GOptionEntry opt_entries[] = { { "debug", 'd', 0, G_OPTION_ARG_NONE, &opt
"Use system root SYSROOT (default: /)", "SYSROOT" },
{ NULL } };

static GDBusConnection *bus = NULL;
static RpmostreedDaemon *rpm_ostree_daemon = NULL;

static void
Expand Down Expand Up @@ -211,17 +213,14 @@ on_log_handler (const gchar *log_domain, GLogLevelFlags log_level, const gchar *
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),
Expand All @@ -243,19 +242,24 @@ rpmostree_builtin_start_daemon (int argc, char **argv, RpmOstreeCommandInvocatio
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))
util::throw_gerror(local_error);
if (!start_daemon (bus, &local_error))
{
if (*error)
sd_notifyf (0, "STATUS=error: %s", (*error)->message);
return FALSE;
sd_notifyf (0, "STATUS=error: %s", local_error->message);
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)
Expand Down Expand Up @@ -285,6 +289,23 @@ rpmostree_builtin_start_daemon (int argc, char **argv, RpmOstreeCommandInvocatio
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;
}
9 changes: 9 additions & 0 deletions src/daemon/rpm-ostreed.socket
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[Unit]
ConditionKernelCommandLine=ostree

[Socket]
ListenStream=/run/rpm-ostree/client.sock
SocketMode=0600

[Install]
WantedBy=sockets.target
28 changes: 28 additions & 0 deletions src/daemon/rpmostreed-daemon.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright (C) 2022 Red Hat, Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/

#pragma once

#include "rust/cxx.h"

namespace rpmostreecxx
{
/* Note: Currently actually defined in rpmostree-builtin-start-daemon.cxx for historical reasons */
void daemon_init_inner (bool debug);
void daemon_main_inner ();
}

0 comments on commit 1b41c5f

Please sign in to comment.