drop systemd service

[cgwalters]

In Fedora and derivatives there's a policy I think of writing SELinux policies for things that are systemd .services.

We happen to have a .socket and .service units but we aren't really a service today. The socket actually denies non-root from connecting.

I think we should just delete those units and where we want to run things under systemd, we do it dynamically via systemd-run e.g.

The primary goal of this is to get this project off the list of things that need a SELinux policy. But I think it'd be a good cleanup anyways.

[EyeCantCU]

Hello!

I've opened a pull request here: https://github.com/coreos/bootupd/pull/586

Please let me know if there's anything that looks like it needs to be changed :)

[cgwalters]

I think this still makes sense, yes.

[HuijingHei]

Thanks @cgwalters for the confirmation, will look at this.

[HuijingHei]

Hi @cgwalters, to drop .socket and .service units, should remove ipc mode, make all subcommand working like generate-install-metadata, instead of creating connection with ClientToDaemonConnection::new(), connect(), send(), and shutdown(), is this right? Thanks!

[cgwalters]

I typed this out, only compile tested but hopefully illustrates the idea

diff --git a/src/bootupd.rs b/src/bootupd.rs
index d0eef93..2cd11e6 100644
--- a/src/bootupd.rs
+++ b/src/bootupd.rs
@@ -408,8 +408,8 @@ pub(crate) fn print_status(status: &Status) -> Result<()> {
     Ok(())
 }
 
-pub(crate) fn client_run_update(c: &mut ipc::ClientToDaemonConnection) -> Result<()> {
-    let status: Status = c.send(&ClientRequest::Status)?;
+pub(crate) fn client_run_update() -> Result<()> {
+    let status: Status = status()?;
     if status.components.is_empty() && status.adoptable.is_empty() {
         println!("No components installed.");
         return Ok(());
@@ -420,9 +420,7 @@ pub(crate) fn client_run_update(c: &mut ipc::ClientToDaemonConnection) -> Result
             ComponentUpdatable::Upgradable => {}
             _ => continue,
         };
-        match c.send(&ClientRequest::Update {
-            component: name.to_string(),
-        })? {
+        match update(name)? {
             ComponentUpdateResult::AtLatestVersion => {
                 // Shouldn't happen unless we raced with another client
                 eprintln!(
@@ -450,9 +448,7 @@ pub(crate) fn client_run_update(c: &mut ipc::ClientToDaemonConnection) -> Result
     }
     for (name, adoptable) in status.adoptable.iter() {
         if adoptable.confident {
-            let r: ContentMetadata = c.send(&ClientRequest::AdoptAndUpdate {
-                component: name.to_string(),
-            })?;
+            let r: ContentMetadata = adopt_and_update(name)?;
             println!("Adopted and updated: {}: {}", name, r.version);
             updated = true;
         } else {
diff --git a/src/cli/bootupctl.rs b/src/cli/bootupctl.rs
index 3883a7a..3e1ae3d 100644
--- a/src/cli/bootupctl.rs
+++ b/src/cli/bootupctl.rs
@@ -1,6 +1,10 @@
+use std::os::unix::process::CommandExt;
+use std::process::Command;
+
 use crate::bootupd;
 use crate::ipc::ClientToDaemonConnection;
 use crate::model::Status;
+use crate::util::CommandRunExt;
 use anyhow::Result;
 use clap::Parser;
 use log::LevelFilter;
@@ -107,13 +111,15 @@ impl CtlCommand {
 
     /// Runner for `update` verb.
     fn run_update() -> Result<()> {
-        let mut client = ClientToDaemonConnection::new();
-        client.connect()?;
-
-        bootupd::client_run_update(&mut client)?;
-
-        client.shutdown()?;
-        Ok(())
+        let running_in_systemd = std::env::var_os("INVOCATION_ID").is_some();
+        if !running_in_systemd {
+            let r = Command::new("systemd-run")
+                .args(["--unit", "bootupd", "bootupctl", "update"])
+                .exec();
+            // If we got here, it's always an error
+            return Err(r.into());
+        }
+        bootupd::client_run_update()
     }
 
     /// Runner for `update` verb.

[cgwalters]

Basically we detect if we're running in systemd; if we're not, we re-exec ourselves via systemd-run. Then we can just directly run code in what is now the daemon.

I think an important aspect of this is that we retain something like --unit bootupd which acts as a lock - only one unit with that name can run at a time to avoid two concurrent invocations breaking things.