Merge df5ebb29ef into cd9bfd8d79

Signed-off-by: Yashodhan Joshi <yjdoc2@gmail.com>

.github/workflows/basic.yml

@@ -75,8 +75,6 @@ jobs:
         uses: taiki-e/install-action@just
       - name: Install cross-rs
         run: cargo install cross --git https://github.com/cross-rs/cross
-      - name: setup dbus
-        run: export $(dbus-launch)
       - name: Setup target
         run: |
           echo "CARGO=cross" >> ${GITHUB_ENV}
@@ -105,8 +103,6 @@ jobs:
       - uses: taiki-e/install-action@just
       - name: Install requirements
         run: sudo env PATH=$PATH just ci-prepare
-      - name: setup dbus
-        run: export $(dbus-launch)
       - name: Run Test Coverage for youki
         run: |
           cargo llvm-cov clean --workspace

Cross.toml

@@ -1,5 +1,6 @@
 [build]
 default-target = "x86_64-unknown-linux-gnu"
+env.passthrough = ["XDG_RUNTIME_DIR"]
 
 [target.aarch64-unknown-linux-gnu]
 dockerfile = "cross/Dockerfile.gnu"

crates/libcgroups/src/systemd/dbus_native/client.rs

@@ -26,4 +26,11 @@ pub trait SystemdClient {
     fn systemd_version(&self) -> Result<u32, SystemdClientError>;
 
     fn control_cgroup_root(&self) -> Result<PathBuf, SystemdClientError>;
+
+    fn add_process_to_unit(
+        &self,
+        unit_name: &str,
+        subcgroup: &str,
+        pid: u32,
+    ) -> Result<(), SystemdClientError>;
 }

crates/libcgroups/src/systemd/dbus_native/dbus.rs

@@ -453,6 +453,10 @@ impl SystemdClient for DbusConnection {
         let cgroup_root = proxy.control_group()?;
         Ok(PathBuf::from(&cgroup_root))
     }
+    fn add_process_to_unit(&self, unit_name: &str, subcgroup: &str, pid: u32) -> Result<()> {
+        let proxy = self.create_proxy();
+        proxy.attach_process(unit_name, subcgroup, pid)
+    }
 }
 
 #[cfg(test)]

crates/libcgroups/src/systemd/dbus_native/proxy.rs

@@ -239,4 +239,11 @@ impl<'conn> Proxy<'conn> {
             v => panic!("control group expected string variant, got {:?} instead", v),
         }
     }
+    pub fn attach_process(&self, name: &str, cgroup: &str, pid: u32) -> Result<()> {
+        self.method_call::<_, ()>(
+            "org.freedesktop.systemd1.Manager",
+            "AttachProcessesToUnit",
+            Some((name, cgroup, vec![pid])),
+        )
+    }
 }

crates/libcgroups/src/systemd/manager.rs

@@ -21,7 +21,7 @@ use super::{
 use crate::{
     common::{
         self, AnyCgroupManager, CgroupManager, ControllerOpt, FreezerState, JoinSafelyError,
-        PathBufExt, WrapIoResult, WrappedIoError, CGROUP_PROCS,
+        PathBufExt, WrapIoResult, WrappedIoError,
     },
     systemd::{dbus_native::serialize::Variant, unified::Unified},
     v2::manager::V2ManagerError,
@@ -355,7 +355,8 @@ impl CgroupManager for Manager {
         }
         if self.client.transient_unit_exists(&self.unit_name) {
             tracing::debug!("Transient unit {:?} already exists", self.unit_name);
-            common::write_cgroup_file(self.full_path.join(CGROUP_PROCS), pid)?;
+            self.client
+                .add_process_to_unit(&self.unit_name, "", pid.as_raw() as u32)?;
             return Ok(());
         }
 
@@ -482,6 +483,15 @@ mod tests {
         fn control_cgroup_root(&self) -> Result<PathBuf, SystemdClientError> {
             Ok(PathBuf::from("/"))
         }
+
+        fn add_process_to_unit(
+            &self,
+            _unit_name: &str,
+            _subcgroup: &str,
+            _pid: u32,
+        ) -> Result<(), SystemdClientError> {
+            Ok(())
+        }
     }
 
     #[test]
@@ -545,7 +555,6 @@ mod tests {
             false,
         )
         .unwrap();
-        fs::create_dir_all(&manager.full_path).unwrap();
         let mut p1 = std::process::Command::new("sleep")
             .arg("1s")
             .spawn()
@@ -565,6 +574,8 @@ mod tests {
         let _ = p1.wait();
         let _ = p2.wait();
         manager.remove().unwrap();
-        fs::remove_dir(&manager.full_path).unwrap();
+        // the remove call above should remove the dir, we just do this again
+        // for contingency, and thus ignore the result
+        let _ = fs::remove_dir(&manager.full_path);
     }
 }

cross/Dockerfile.gnu

@@ -13,3 +13,6 @@ RUN dpkg --add-architecture ${CROSS_DEB_ARCH} && \
     zlib1g-dev:${CROSS_DEB_ARCH} \
     # dependencies to build wasmedge-sys
     libzstd-dev:${CROSS_DEB_ARCH}
+
+COPY hack/busctl.sh /bin/busctl
+RUN chmod +x /bin/busctl

cross/Dockerfile.musl

@@ -22,6 +22,9 @@ ENV LIBSECCOMP_LIB_PATH="${CROSS_SYSROOT}/lib"
 ENV WASMEDGE_DEP_STDCXX_LINK_TYPE="static"
 ENV WASMEDGE_DEP_STDCXX_LIB_PATH="${CROSS_SYSROOT}/lib"
 
+COPY hack/busctl.sh /bin/busctl
+RUN chmod +x /bin/busctl
+
 # wasmedge-sys (through llvm) needs some symbols defined in libgcc
 RUN mkdir /.cargo && cat <<'EOF' > /.cargo/config.toml
 [target.'cfg(target_env = "musl")']

hack/busctl.sh

@@ -0,0 +1,14 @@
+#!/bin/sh
+
+# This hack script is the dummy busctl command used when running tests with cross containers.
+
+# The issue is that we cannot run systemd or dbus inside the test container without a lot 
+# of hacks. For one specific test - test_task_addition, we need to check that the task
+# addition via systemd manager works. We mount the host dbus socket in the test container, so 
+# dbus calls work, but for the initial authentication, we use busctl which needs dbus and systemd
+# to be present and running. So instead of doing all that, we simply run the container with the 
+# actual test running user's uid/gid and here we echo the only relevant line from busctl's 
+# output, using id to get the uid. This is a hack, but less complex than actually setting up
+# and running the systemd+dbus inside the container.
+
+echo "OwnerUID=$(id -u)"

scripts/cargo.sh

@@ -56,7 +56,11 @@ if [ "$CARGO" == "cross" ]; then
 
     # mount run to have access to dbus socket.
     # mount /tmp so as shared for test_make_parent_mount_private
-    export CROSS_CONTAINER_OPTS="--privileged -v/run:/run --mount=type=bind,source=/tmp,destination=/tmp,bind-propagation=shared"
+    # Then there are few "hacks" specificallt for test_task_addition
+    # run with user same as the invoking user, so that the dbus is connected with correct user
+    # we want pid ns of host, because we will be connecting to the host dbus, and it needs task pid from host
+    # finally we need to mount the cgroup as read-only, as we need that to check if the tasks are correctly added
+    export CROSS_CONTAINER_OPTS="--privileged --user `id -u`:`id -g` --pid=host -v /sys/fs/cgroup:/sys/fs/cgroup:ro -v/run:/run --mount=type=bind,source=/tmp,destination=/tmp,bind-propagation=shared"
 fi
 
 if [ "$1" == "--print-target-dir" ]; then