Skip to content

Commit 6d28576

Browse files
RishabhSainiRishabhSaini
committed
removing client-socket feature
1 parent 913e32a commit 6d28576

File tree

9 files changed

+26
-127
lines changed

9 files changed

+26
-127
lines changed

‎Cargo.lock‎

Lines changed: 0 additions & 37 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ tracing-subscriber = { version = "0.3", features = ["env-filter"] }
9494
tokio = { version = "1.28.2", features = ["time", "process", "rt", "net"] }
9595
xmlrpc = "0.15.1"
9696
termcolor = "1.1.3"
97-
tokio-unix-ipc = { version = "0.2.0", features = ["serde"] }
9897

9998
[build-dependencies]
10099
anyhow = "1.0"
@@ -126,8 +125,6 @@ lto = "thin"
126125
# Note: If you add a feature here, you also probably want to update utils.rs:get_features()
127126
fedora-integration = []
128127
rhsm = ["libdnf-sys/rhsm"]
129-
# Enable hard requirement on `rpm-ostreed.socket`; requires https://bugzilla.redhat.com/show_bug.cgi?id=2110012
130-
client-socket = []
131128
bin-unit-tests = []
132129
# ASAN+UBSAN
133130
sanitizers = []

‎Makefile-daemon.am‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,9 @@ systemdunit_service_files = $(addprefix $(srcdir)/src/daemon/,$(systemdunit_serv
6666
systemdunit_other_files = \
6767
$(srcdir)/src/daemon/rpm-ostreed-automatic.timer \
6868
$(srcdir)/src/daemon/rpm-ostree-countme.timer \
69+
$(srcdir)/src/daemon/rpm-ostreed.socket \
6970
$(NULL)
7071

71-
if CLIENT_SOCKET
72-
systemdunit_other_files += $(srcdir)/src/daemon/rpm-ostreed.socket
73-
endif
74-
7572
systemdunit_DATA = \
7673
$(systemdunit_service_files) \
7774
$(systemdunit_other_files) \

‎configure.ac‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,6 @@ AC_ARG_ENABLE(featuresrs,
7070
[Rust features, see Cargo.toml for more information]),,
7171
[enable_featuresrs=])
7272

73-
AC_ARG_ENABLE(client-socket,
74-
AS_HELP_STRING([--enable-client-socket],
75-
[(default: no)]),,
76-
[enable_client_socket=no])
77-
AS_IF([test x$enable_client_socket = xyes], [enable_featuresrs="$enable_featuresrs client-socket"])
78-
AM_CONDITIONAL(CLIENT_SOCKET, [echo $enable_featuresrs | grep -q 'client-socket'])
79-
8073
AC_SUBST([RUST_FEATURES], $enable_featuresrs)
8174

8275
# Initialize libtool

‎packaging/rpm-ostree.spec.in‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,6 @@ BuildRequires: rust
3434
%bcond_with rhsm
3535
%endif
3636

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

187180
%make_build
188181

‎rust/src/client.rs‎

Lines changed: 4 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -159,11 +159,13 @@ pub(crate) fn client_handle_fd_argument(
159159
/// Connect to the client socket and ensure the daemon is initialized;
160160
/// this avoids DBus and ensures that we get any early startup errors
161161
/// returned cleanly.
162-
#[cfg(feature = "client-socket")]
163162
fn start_daemon_via_socket() -> Result<()> {
164163
use cap_std::io_lifetimes::IntoSocketlike;
165164

166-
let conn = tokio::net::UnixStream::connect("/run/rpm-ostree/client.sock")?;
165+
futures::executor::block_on(async {
166+
let c = tokio::net::UnixStream::connect("/run/rpm-ostree/client.sock").await?;
167+
Ok::<_, anyhow::Error>(c)
168+
})?;
167169

168170
let address = sockaddr()?;
169171
let socket = rustix::net::socket(
@@ -197,62 +199,13 @@ pub(crate) fn sockaddr() -> Result<rustix::net::SocketAddrUnix> {
197199
rustix::net::SocketAddrUnix::new("/run/rpm-ostree/client.sock").map_err(anyhow::Error::msg)
198200
}
199201

200-
/// Explicitly ensure the daemon is started via systemd, if possible.
201-
///
202-
/// This works around bugs from DBus activation, see
203-
/// https://github.com/coreos/rpm-ostree/pull/2932
204-
///
205-
/// Basically we load too much data before claiming the bus name,
206-
/// and dbus doesn't give us a useful error. Instead, let's talk
207-
/// to systemd directly and use its client tools to scrape errors.
208-
///
209-
/// What we really should do probably is use native socket activation.
210-
#[cfg(not(feature = "client-socket"))]
211-
fn start_daemon_via_systemctl() -> Result<()> {
212-
use std::process::Command;
213-
214-
let service = "rpm-ostreed.service";
215-
// Assume non-root can't use systemd right now.
216-
if rustix::process::getuid().as_raw() != 0 {
217-
return Ok(());
218-
}
219-
220-
// Unfortunately, RHEL8 systemd will count "systemctl start"
221-
// invocations against the restart limit, so query the status
222-
// first.
223-
let activeres = Command::new("systemctl")
224-
.args(&["is-active", "rpm-ostreed"])
225-
.output()?;
226-
// Explicitly don't check the error return value, we don't want to
227-
// hard fail on it.
228-
if String::from_utf8_lossy(&activeres.stdout).starts_with("active") {
229-
// It's active, we're done. Note that while this is a race
230-
// condition, that's fine because it will be handled by DBus
231-
// activation.
232-
return Ok(());
233-
}
234-
let res = Command::new("systemctl")
235-
.args(&["--no-ask-password", "start", service])
236-
.status()?;
237-
if !res.success() {
238-
let _ = Command::new("systemctl")
239-
.args(&["--no-pager", "status", service])
240-
.status();
241-
return Err(anyhow!("{}", res).into());
242-
}
243-
Ok(())
244-
}
245-
246202
pub(crate) fn client_start_daemon() -> CxxResult<()> {
247203
// systemctl and socket paths only work for root right now; in the future
248204
// the socket may be opened up.
249205
if rustix::process::getuid().as_raw() != 0 {
250206
return Ok(());
251207
}
252-
#[cfg(feature = "client-socket")]
253208
return start_daemon_via_socket().map_err(Into::into);
254-
#[cfg(not(feature = "client-socket"))]
255-
return start_daemon_via_systemctl().map_err(Into::into);
256209
}
257210

258211
/// Convert the GVariant parameters from the DownloadProgress DBus API to a human-readable English string.

‎rust/src/daemon.rs‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,6 @@ pub(crate) fn daemon_main(debug: bool) -> Result<()> {
239239
if !systemd::daemon::booted()? {
240240
return Err(anyhow!("not running as a systemd service"));
241241
}
242-
243242
let init_res: Result<()> = crate::ffi::daemon_init_inner(debug).map_err(|e| e.into());
244243
tracing::debug!("Initialization result: {init_res:?}");
245244

@@ -251,20 +250,18 @@ pub(crate) fn daemon_main(debug: bool) -> Result<()> {
251250
// with a socket.
252251
init_res.context("Initializing via dbus")?;
253252
tracing::debug!("Initializing directly (not socket activated)");
254-
let listener = cfg!(feature = "client-socket")
255-
.then(|| {
256-
let socket = rustix::net::socket(
257-
rustix::net::AddressFamily::UNIX,
258-
rustix::net::SocketType::SEQPACKET,
259-
rustix::net::Protocol::from_raw(0),
260-
)?;
261-
let addr = crate::client::sockaddr()?;
262-
rustix::net::bind_unix(&socket, &addr)?;
263-
Ok::<_, anyhow::Error>(socket.into_socketlike())
264-
})
265-
.transpose()
266-
.context("Binding to socket")?;
267-
(listener, Ok(()))
253+
let listener = {
254+
let socket = rustix::net::socket(
255+
rustix::net::AddressFamily::UNIX,
256+
rustix::net::SocketType::SEQPACKET,
257+
rustix::net::Protocol::from_raw(0),
258+
)?;
259+
let addr = crate::client::sockaddr()?;
260+
rustix::net::bind_unix(&socket, &addr)?;
261+
Ok::<_, anyhow::Error>(socket.into_socketlike())
262+
}
263+
.context("Binding to socket")?;
264+
(Some(listener), Ok(()))
268265
}
269266
Some(fd) => {
270267
if fds.next().is_some() {
@@ -276,17 +273,19 @@ pub(crate) fn daemon_main(debug: bool) -> Result<()> {
276273
(Some(listener), init_res)
277274
}
278275
};
279-
276+
280277
if let Some(listener) = listener {
281278
// On success, we spawn a helper task that just responds with
282279
// sucess to clients that connect via the socket. In the future,
283280
// perhaps we'll expose an API here.
284281
tracing::debug!("Spawning acknowledgement task");
285282
match init_res {
286283
Ok(()) => {
284+
dbg!(&listener);
287285
std::thread::spawn(move || run_acknowledgement_worker(listener));
288286
}
289287
Err(e) => {
288+
tracing::debug!("Sending error to client: {}", e);
290289
let err_copy = anyhow::format_err!("{e}");
291290
let r = std::thread::spawn(move || {
292291
if let Err(suberr) = process_one_client(listener, err_copy) {

‎src/daemon/rpm-ostreed.socket‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
ConditionKernelCommandLine=ostree
33

44
[Socket]
5-
ListenStream=/run/rpm-ostree/client.sock
5+
ListenSequentialPacket=/run/rpm-ostree/client.sock
66
SocketMode=0600
77

88
[Install]

‎tests/kolainst/nondestructive/misc.sh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,10 @@ rpmostree_busctl_call_os Search as 1 should-not-exist-p-equals-np > out.txt
109109
assert_file_has_content_literal out.txt 'aa{sv} 0'
110110
echo "ok dbus Search"
111111

112+
systemctl stop rpm-ostreed
113+
rpmostree_busctl_call_os ListRepos
114+
echo "restarting the daemon works"
115+
112116
rpm-ostree search testdaemon > out.txt
113117
assert_file_has_content_literal out.txt '===== Name Matched ====='
114118
assert_file_has_content_literal out.txt 'testdaemon : awesome-daemon-for-testing'

0 commit comments

Comments
 (0)