Skip to content

Commit

Permalink
Make client-socket into a feature
Browse files Browse the repository at this point in the history
So we can use it on F37+ builds.
  • Loading branch information
cgwalters committed Jul 27, 2022
1 parent d8997f3 commit b1f7105
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 33 deletions.
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ lto = "thin"
# Note: If you add a feature here, you also probably want to update utils.rs:get_features()
fedora-integration = []
rhsm = ["libdnf-sys/rhsm"]
# Enable hard requirement on `rpm-ostreed.socket`; requires https://bugzilla.redhat.com/show_bug.cgi?id=2110012
client-socket = []
bin-unit-tests = []
# ASAN+UBSAN
sanitizers = []
Expand Down
36 changes: 35 additions & 1 deletion rust/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,26 @@ pub(crate) fn client_handle_fd_argument(
}
}

/// 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.
#[cfg(feature = "client-socket")]
fn start_daemon_via_socket() -> Result<()> {
let address = "/run/rpm-ostree/client.sock";
tracing::debug!("Starting daemon via {address}");
let s = std::os::unix::net::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())
}
}

/// Explicitly ensure the daemon is started via systemd, if possible.
///
/// This works around bugs from DBus activation, see
Expand All @@ -164,12 +184,14 @@ pub(crate) fn client_handle_fd_argument(
/// 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<()> {
#[cfg(not(feature = "client-socket"))]
fn start_daemon_via_systemctl() -> Result<()> {
let service = "rpm-ostreed.service";
// Assume non-root can't use systemd right now.
if rustix::process::getuid().as_raw() != 0 {
return Ok(());
}

// Unfortunately, RHEL8 systemd will count "systemctl start"
// invocations against the restart limit, so query the status
// first.
Expand All @@ -196,6 +218,18 @@ pub(crate) fn client_start_daemon() -> CxxResult<()> {
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(());
}
#[cfg(feature = "client-socket")]
return start_daemon_via_socket().map_err(Into::into);
#[cfg(not(feature = "client-socket"))]
return start_daemon_via_systemctl().map_err(Into::into);
}

/// Convert the GVariant parameters from the DownloadProgress DBus API to a human-readable English string.
pub(crate) fn client_render_download_progress(progress: &crate::ffi::GVariant) -> String {
let progress = progress
Expand Down
44 changes: 15 additions & 29 deletions rust/src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,25 +135,6 @@ 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";
tracing::debug!("Starting daemon via {address}");
let s = std::os::unix::net::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())
}
}

async fn send_ok_result_to_client(_client: UnixStream) {
// On success we close the stream without writing anything,
// which acknowledges successful startup to the client.
Expand Down Expand Up @@ -229,7 +210,10 @@ pub(crate) fn daemon_main(debug: bool) -> Result<()> {
// directly propagate the error back to our exit code.
init_res?;
tracing::debug!("Initializing directly (not socket activated)");
StdUnixListener::bind("/run/rpm-ostree/client.sock")?
cfg!(feature = "client-socket")
.then(|| StdUnixListener::bind("/run/rpm-ostree/client.sock"))
.transpose()
.context("Binding to socket")?
}
Some(fd) => {
if fds.next().is_some() {
Expand All @@ -239,7 +223,7 @@ pub(crate) fn daemon_main(debug: bool) -> Result<()> {
let listener = unsafe { StdUnixListener::from_raw_fd(fd) };

match init_res {
Ok(_) => listener,
Ok(_) => Some(listener),
Err(e) => {
let err_copy = anyhow!("{e}");
tracing::debug!("Reporting initialization error: {e}");
Expand All @@ -257,16 +241,18 @@ pub(crate) fn daemon_main(debug: bool) -> Result<()> {
}
};

let (shutdown_send, shutdown_recv) = tokio::sync::oneshot::channel();
(*SHUTDOWN_SIGNAL).lock().unwrap().replace(shutdown_send);
if let Some(listener) = listener {
let (shutdown_send, shutdown_recv) = tokio::sync::oneshot::channel();
(*SHUTDOWN_SIGNAL).lock().unwrap().replace(shutdown_send);

let listener = UnixListener::from_std(listener)?;
let listener = UnixListener::from_std(listener)?;

// On success, we spawn a helper task that just responds with
// sucess to clients that connect via the socket. In the future,
// perhaps we'll expose an API here.
tracing::debug!("Spawning acknowledgement task");
tokio::task::spawn(async { process_clients_with_ok(listener, shutdown_recv).await });
// On success, we spawn a helper task that just responds with
// sucess to clients that connect via the socket. In the future,
// perhaps we'll expose an API here.
tracing::debug!("Spawning acknowledgement task");
tokio::task::spawn(async { process_clients_with_ok(listener, shutdown_recv).await });
}

tracing::debug!("Entering daemon mainloop");
// And now, enter the main loop.
Expand Down
1 change: 0 additions & 1 deletion rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@ pub mod ffi {
// daemon.rs
extern "Rust" {
fn daemon_main(debug: bool) -> Result<()>;
fn start_daemon_via_socket() -> Result<()>;
fn daemon_terminate();
fn daemon_sanitycheck_environment(sysroot: &OstreeSysroot) -> Result<()>;
fn deployment_generate_id(deployment: &OstreeDeployment) -> String;
Expand Down
2 changes: 0 additions & 2 deletions src/app/libmain.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,6 @@ rpmostree_option_context_parse (GOptionContext *context, const GOptionEntry *mai
return rpmostreecxx::client_throw_non_ostree_host_error (error);
}

CXX_TRY (rpmostreecxx::start_daemon_via_socket (), error);

/* root never needs to auth */
if (getuid () != 0)
/* ignore errors; we print out a warning if we fail to spawn pkttyagent */
Expand Down

0 comments on commit b1f7105

Please sign in to comment.