removing client-socket feature

Author: RishabhSaini

Cargo.lock

@@ -94,7 +94,6 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] }
 tokio = { version = "1.28.2", features = ["time", "process", "rt", "net"] }
 xmlrpc = "0.15.1"
 termcolor = "1.1.3"
-tokio-unix-ipc = { version = "0.2.0", features = ["serde"] }
 
 [build-dependencies]
 anyhow = "1.0"
@@ -126,8 +125,6 @@ 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 = []

Cargo.toml

@@ -66,12 +66,9 @@ systemdunit_service_files = $(addprefix $(srcdir)/src/daemon/,$(systemdunit_serv
 systemdunit_other_files = \
 	$(srcdir)/src/daemon/rpm-ostreed-automatic.timer \
 	$(srcdir)/src/daemon/rpm-ostree-countme.timer \
+       $(srcdir)/src/daemon/rpm-ostreed.socket \
 	$(NULL)
 
-if CLIENT_SOCKET
-systemdunit_other_files += $(srcdir)/src/daemon/rpm-ostreed.socket
-endif
-
 systemdunit_DATA = \
 	$(systemdunit_service_files) \
 	$(systemdunit_other_files) \

Makefile-daemon.am

@@ -70,13 +70,6 @@ AC_ARG_ENABLE(featuresrs,
                              [Rust features, see Cargo.toml for more information]),,
               [enable_featuresrs=])
 
-AC_ARG_ENABLE(client-socket,
-              AS_HELP_STRING([--enable-client-socket],
-                             [(default: no)]),,
-              [enable_client_socket=no])
-AS_IF([test x$enable_client_socket = xyes], [enable_featuresrs="$enable_featuresrs client-socket"])
-AM_CONDITIONAL(CLIENT_SOCKET, [echo $enable_featuresrs | grep -q 'client-socket'])
-
 AC_SUBST([RUST_FEATURES], $enable_featuresrs)
 
 # Initialize libtool

configure.ac

@@ -34,13 +34,6 @@ BuildRequires: rust
 %bcond_with rhsm
 %endif
 
-# https://bugzilla.redhat.com/show_bug.cgi?id=2110012
-%if 0%{?fedora} >= 37
-%bcond_without client_socket
-%else
-%bcond_with client_socket
-%endif
-
 # RHEL (8,9) doesn't ship zchunk today.  Keep this in sync
 # with libdnf: https://gitlab.com/redhat/centos-stream/rpms/libdnf/-/blob/762f631e36d1e42c63a794882269d26c156b68c1/libdnf.spec#L45
 %if 0%{?rhel}
@@ -182,7 +175,7 @@ env NOCONFIGURE=1 ./autogen.sh
 export RUSTFLAGS="%{build_rustflags}"
 %endif
 %configure --disable-silent-rules --enable-gtk-doc %{?rpmdb_default} %{?with_sanitizers:--enable-sanitizers}  %{?with_bin_unit_tests:--enable-bin-unit-tests} \
-  %{?with_rhsm:--enable-featuresrs=rhsm} %{?with_client_socket:--enable-client-socket} 
+  %{?with_rhsm:--enable-featuresrs=rhsm} 
 
 %make_build
 

packaging/rpm-ostree.spec.in

@@ -159,11 +159,13 @@ 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<()> {
     use cap_std::io_lifetimes::IntoSocketlike;
 
-    let conn = tokio::net::UnixStream::connect("/run/rpm-ostree/client.sock")?;
+    futures::executor::block_on(async {
+        let c = tokio::net::UnixStream::connect("/run/rpm-ostree/client.sock").await?;
+        Ok::<_, anyhow::Error>(c)
+    })?;
 
     let address = sockaddr()?;
     let socket = rustix::net::socket(
@@ -197,62 +199,13 @@ pub(crate) fn sockaddr() -> Result<rustix::net::SocketAddrUnix> {
     rustix::net::SocketAddrUnix::new("/run/rpm-ostree/client.sock").map_err(anyhow::Error::msg)
 }
 
-/// 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.
-#[cfg(not(feature = "client-socket"))]
-fn start_daemon_via_systemctl() -> Result<()> {
-    use std::process::Command;
-
-    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.
-    let activeres = Command::new("systemctl")
-        .args(&["is-active", "rpm-ostreed"])
-        .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.
-        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());
-    }
-    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.

rust/src/client.rs

@@ -239,7 +239,6 @@ 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());
     tracing::debug!("Initialization result: {init_res:?}");
 
@@ -251,20 +250,18 @@ pub(crate) fn daemon_main(debug: bool) -> Result<()> {
             // with a socket.
             init_res.context("Initializing via dbus")?;
             tracing::debug!("Initializing directly (not socket activated)");
-            let listener = cfg!(feature = "client-socket")
-                .then(|| {
-                    let socket = rustix::net::socket(
-                        rustix::net::AddressFamily::UNIX,
-                        rustix::net::SocketType::SEQPACKET,
-                        rustix::net::Protocol::from_raw(0),
-                    )?;
-                    let addr = crate::client::sockaddr()?;
-                    rustix::net::bind_unix(&socket, &addr)?;
-                    Ok::<_, anyhow::Error>(socket.into_socketlike())
-                })
-                .transpose()
-                .context("Binding to socket")?;
-            (listener, Ok(()))
+            let listener = {
+                let socket = rustix::net::socket(
+                    rustix::net::AddressFamily::UNIX,
+                    rustix::net::SocketType::SEQPACKET,
+                    rustix::net::Protocol::from_raw(0),
+                )?;
+                let addr = crate::client::sockaddr()?;
+                rustix::net::bind_unix(&socket, &addr)?;
+                Ok::<_, anyhow::Error>(socket.into_socketlike())
+            }
+            .context("Binding to socket")?;
+            (Some(listener), Ok(()))
         }
         Some(fd) => {
             if fds.next().is_some() {
@@ -276,17 +273,19 @@ pub(crate) fn daemon_main(debug: bool) -> Result<()> {
             (Some(listener), init_res)
         }
     };
-
+    
     if let Some(listener) = 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");
         match init_res {
             Ok(()) => {
+                dbg!(&listener);
                 std::thread::spawn(move || run_acknowledgement_worker(listener));
             }
             Err(e) => {
+                tracing::debug!("Sending error to client: {}", e);
                 let err_copy = anyhow::format_err!("{e}");
                 let r = std::thread::spawn(move || {
                     if let Err(suberr) = process_one_client(listener, err_copy) {

rust/src/daemon.rs

@@ -2,7 +2,7 @@
 ConditionKernelCommandLine=ostree
 
 [Socket]
-ListenStream=/run/rpm-ostree/client.sock
+ListenSequentialPacket=/run/rpm-ostree/client.sock
 SocketMode=0600
 
 [Install]

src/daemon/rpm-ostreed.socket

@@ -109,6 +109,10 @@ rpmostree_busctl_call_os Search as 1 should-not-exist-p-equals-np > out.txt
 assert_file_has_content_literal out.txt 'aa{sv} 0'
 echo "ok dbus Search"
 
+systemctl stop rpm-ostreed
+rpmostree_busctl_call_os ListRepos
+echo "restarting the daemon works"
+
 rpm-ostree search testdaemon > out.txt
 assert_file_has_content_literal out.txt '===== Name Matched ====='
 assert_file_has_content_literal out.txt 'testdaemon : awesome-daemon-for-testing'