podman rootless with dbus native (#2370)

* Move dbus_native code to youki, add tests Signed-off-by: Yashodhan Joshi <[email protected]> * (incomplete) move individual systemd components to dbus_native Signed-off-by: Yashodhan Joshi <[email protected]> * (incomplete) Change tests to use dbus_native, move systemdClientInterface into dbus_native Signed-off-by: Yashodhan Joshi <[email protected]> * (incomplete) Change variant to actual enum, Fix types for systemd Signed-off-by: Yashodhan Joshi <[email protected]> * Fix bug in vector ser/de Signed-off-by: Yashodhan Joshi <[email protected]> * Fix mut requirements, minor lints etc., add client id in dbus Signed-off-by: Yashodhan Joshi <[email protected]> * Add implementation for proxy methods Signed-off-by: Yashodhan Joshi <[email protected]> * Split dbus error from SystemdClientError Signed-off-by: Yashodhan Joshi <[email protected]> * implement system and session connection creation Signed-off-by: Yashodhan Joshi <[email protected]> * Fix typos and failing feature test build Signed-off-by: Yashodhan Joshi <[email protected]> * Completely remove dbus dependency and add docs regarding dbus-native Signed-off-by: Yashodhan Joshi <[email protected]> * Update migration guide, move documentation od dbus_native to cgroups Signed-off-by: Yashodhan Joshi <[email protected]> --------- Signed-off-by: Yashodhan Joshi <[email protected]>
youki-dev · Oct 5, 2023 · cc39179 · cc39179
1 parent fc16798
commit cc39179
Show file tree

Hide file tree

Showing 33 changed files with 2,509 additions and 2,839 deletions.
diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml
@@ -42,7 +42,7 @@ jobs:
           go-version: '1.20'
           cache: true
       - run: sudo apt-get -y update
-      - run: sudo apt-get install -y pkg-config libsystemd-dev libdbus-glib-1-dev libelf-dev libseccomp-dev btrfs-progs libbtrfs-dev
+      - run: sudo apt-get install -y pkg-config libsystemd-dev libelf-dev libseccomp-dev btrfs-progs libbtrfs-dev
       - name: Build containerd
         run: |
           make build

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -121,7 +121,7 @@ jobs:
         run: |
           cargo llvm-cov clean --workspace
           cargo llvm-cov --no-report
-          cargo llvm-cov --no-run --lcov --ignore-filename-regex "libcgroups/src/systemd/dbus/systemd_api.rs" --output-path ./coverage.lcov
+          cargo llvm-cov --no-run --lcov --output-path ./coverage.lcov
       - name: Upload Youki Code Coverage Results
         uses: codecov/codecov-action@v3
         with:

diff --git a/.github/workflows/podman_tests.yaml b/.github/workflows/podman_tests.yaml
@@ -15,7 +15,7 @@ jobs:
       - name: Install requirements
         run: sudo env PATH=$PATH just ci-prepare
       - name: Install skopeo and podman requirements
-        run: sudo apt-get install -y pkg-config libsystemd-dev libdbus-glib-1-dev libelf-dev libseccomp-dev libgpgme-dev libassuan-dev libbtrfs-dev libdevmapper-dev bats socat
+        run: sudo apt-get install -y pkg-config libsystemd-dev libelf-dev libseccomp-dev libgpgme-dev libassuan-dev libbtrfs-dev libdevmapper-dev bats socat
 
       # setup go
       - uses: actions/setup-go@v4

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/MirgationGuide.md b/MirgationGuide.md
@@ -2,7 +2,12 @@
 
 This contains information for migrating library versions.
 
-## V0.1.0 -> v0.2.0
+## v0.2.0 -> v0.3.0
+
+### libcgroups
+- Switched from dbus-rs to a native dbus implementation see [#2208](https://github.com/containers/youki/issues/2208) for motivation behind this. This replaces the `dbus` module with `dbus_native` module. However, As this is not in public interface for the crate, the users of this crate should not need any code changes. As this removes the dependency on the `libdbus` system library, you can uninstall it if desired.
+
+## v0.1.0 -> v0.2.0
 
 ### libcontainer
 

diff --git a/README.md b/README.md
@@ -158,7 +158,6 @@ To install `just`, follow the instruction [here](https://github.com/casey/just#i
 $ sudo apt-get install    \
       pkg-config          \
       libsystemd-dev      \
-      libdbus-glib-1-dev  \
       build-essential     \
       libelf-dev          \
       libseccomp-dev      \
@@ -173,7 +172,6 @@ $ sudo apt-get install    \
 $ sudo dnf install          \
       pkg-config            \
       systemd-devel         \
-      dbus-devel            \
       elfutils-libelf-devel \
       libseccomp-devel      \
       clang-devel           \

diff --git a/crates/libcgroups/Cargo.toml b/crates/libcgroups/Cargo.toml
@@ -16,14 +16,13 @@ keywords = ["youki", "container", "cgroups"]
 default = ["v1", "v2", "systemd"]
 v1 = []
 v2 = []
-systemd = ["v2", "dep:dbus"]
+systemd = ["v2"]
 cgroupsv2_devices = ["rbpf", "libbpf-sys", "errno", "libc"]
 
 [dependencies]
 nix = "0.26.2"
 procfs = "0.15.1"
 oci-spec = { version = "~0.6.2", features = ["runtime"] }
-dbus = { version = "0.9.7", optional = true }
 fixedbitset = "0.4.2"
 serde = { version = "1.0", features = ["derive"] }
 rbpf = {version = "0.2.0", optional = true }

diff --git a/crates/libcgroups/src/common.rs b/crates/libcgroups/src/common.rs
@@ -68,8 +68,9 @@ pub enum AnyManagerError {
     V2(#[from] v2::manager::V2ManagerError),
 }
 
+// systemd is boxed due to size lint https://rust-lang.github.io/rust-clippy/master/index.html#/large_enum_variant
 pub enum AnyCgroupManager {
-    Systemd(systemd::manager::Manager),
+    Systemd(Box<systemd::manager::Manager>),
     V1(v1::manager::Manager),
     V2(v2::manager::Manager),
 }

diff --git a/crates/libcgroups/src/stub/systemd/manager.rs b/crates/libcgroups/src/stub/systemd/manager.rs
@@ -10,7 +10,7 @@ pub struct Manager {}
 
 impl Manager {
     pub fn any(self) -> AnyCgroupManager {
-        AnyCgroupManager::Systemd(self)
+        AnyCgroupManager::Systemd(Box::new(self))
     }
 }
 

diff --git a/crates/libcgroups/src/systemd/controller.rs b/crates/libcgroups/src/systemd/controller.rs
@@ -1,6 +1,6 @@
 use std::collections::HashMap;
 
-use dbus::arg::RefArg;
+use super::dbus_native::serialize::Variant;
 
 use crate::common::ControllerOpt;
 
@@ -10,6 +10,6 @@ pub(super) trait Controller {
     fn apply(
         options: &ControllerOpt,
         systemd_version: u32,
-        properties: &mut HashMap<&str, Box<dyn RefArg>>,
+        properties: &mut HashMap<&str, Variant>,
     ) -> Result<(), Self::Error>;
 }
diff --git a/crates/libcgroups/src/systemd/cpu.rs b/crates/libcgroups/src/systemd/cpu.rs
@@ -1,6 +1,6 @@
 use std::collections::HashMap;
 
-use dbus::arg::RefArg;
+use super::dbus_native::serialize::Variant;
 use oci_spec::runtime::LinuxCpu;
 
 use super::controller::Controller;
@@ -25,7 +25,7 @@ impl Controller for Cpu {
     fn apply(
         options: &ControllerOpt,
         _: u32,
-        properties: &mut HashMap<&str, Box<dyn RefArg>>,
+        properties: &mut HashMap<&str, Variant>,
     ) -> Result<(), Self::Error> {
         if let Some(cpu) = options.resources.cpu() {
             tracing::debug!("Applying cpu resource restrictions");
@@ -39,7 +39,7 @@ impl Controller for Cpu {
 impl Cpu {
     fn apply(
         cpu: &LinuxCpu,
-        properties: &mut HashMap<&str, Box<dyn RefArg>>,
+        properties: &mut HashMap<&str, Variant>,
     ) -> Result<(), SystemdCpuError> {
         if Self::is_realtime_requested(cpu) {
             return Err(SystemdCpuError::RealtimeSystemd);
@@ -48,7 +48,7 @@ impl Cpu {
         if let Some(mut shares) = cpu.shares() {
             shares = convert_shares_to_cgroup2(shares);
             if shares != 0 {
-                properties.insert(CPU_WEIGHT, Box::new(shares));
+                properties.insert(CPU_WEIGHT, Variant::U64(shares));
             }
         }
 
@@ -63,15 +63,15 @@ impl Cpu {
                 quota = specified_quota as u64 * MICROSECS_PER_SEC / period;
             }
         }
-        properties.insert(CPU_QUOTA, Box::new(quota));
+        properties.insert(CPU_QUOTA, Variant::U64(quota));
 
         let mut period: u64 = 100_000;
         if let Some(specified_period) = cpu.period() {
             if specified_period > 0 {
                 period = specified_period;
             }
         }
-        properties.insert(CPU_PERIOD, Box::new(period));
+        properties.insert(CPU_PERIOD, Variant::U64(period));
 
         Ok(())
     }
@@ -92,9 +92,11 @@ pub fn convert_shares_to_cgroup2(shares: u64) -> u64 {
 #[cfg(test)]
 mod tests {
     use anyhow::{Context, Result};
-    use dbus::arg::ArgType;
     use oci_spec::runtime::LinuxCpuBuilder;
 
+    use super::super::dbus_native::serialize::DbusSerialize;
+    use crate::recast;
+
     use super::*;
 
     #[test]
@@ -104,7 +106,7 @@ mod tests {
             .shares(22000u64)
             .build()
             .context("build cpu spec")?;
-        let mut properties: HashMap<&str, Box<dyn RefArg>> = HashMap::new();
+        let mut properties: HashMap<&str, Variant> = HashMap::new();
 
         // act
         Cpu::apply(&cpu, &mut properties)?;
@@ -113,8 +115,8 @@ mod tests {
         assert!(properties.contains_key(CPU_WEIGHT));
 
         let cpu_weight = &properties[CPU_WEIGHT];
-        assert_eq!(cpu_weight.arg_type(), ArgType::UInt64);
-        assert_eq!(cpu_weight.as_u64().unwrap(), 840u64);
+        let val = recast!(cpu_weight, Variant)?;
+        assert_eq!(val, Variant::U64(840));
 
         Ok(())
     }
@@ -126,16 +128,16 @@ mod tests {
         for quota in quotas {
             // arrange
             let cpu = LinuxCpuBuilder::default().quota(quota.0).build().unwrap();
-            let mut properties: HashMap<&str, Box<dyn RefArg>> = HashMap::new();
+            let mut properties: HashMap<&str, Variant> = HashMap::new();
 
             // act
             Cpu::apply(&cpu, &mut properties)?;
 
             // assert
             assert!(properties.contains_key(CPU_QUOTA));
             let cpu_quota = &properties[CPU_QUOTA];
-            assert_eq!(cpu_quota.arg_type(), ArgType::UInt64);
-            assert_eq!(cpu_quota.as_u64().unwrap(), quota.1);
+            let val = recast!(cpu_quota, Variant)?;
+            assert_eq!(val, Variant::U64(quota.1));
         }
 
         Ok(())
@@ -150,16 +152,16 @@ mod tests {
                 .period(period.0)
                 .build()
                 .context("build cpu spec")?;
-            let mut properties: HashMap<&str, Box<dyn RefArg>> = HashMap::new();
+            let mut properties: HashMap<&str, Variant> = HashMap::new();
 
             // act
             Cpu::apply(&cpu, &mut properties)?;
 
             // assert
             assert!(properties.contains_key(CPU_PERIOD));
             let cpu_quota = &properties[CPU_PERIOD];
-            assert_eq!(cpu_quota.arg_type(), ArgType::UInt64);
-            assert_eq!(cpu_quota.as_u64().unwrap(), period.1);
+            let val = recast!(cpu_quota, Variant)?;
+            assert_eq!(val, Variant::U64(period.1));
         }
 
         Ok(())

diff --git a/crates/libcgroups/src/systemd/cpuset.rs b/crates/libcgroups/src/systemd/cpuset.rs
@@ -1,6 +1,6 @@
 use std::collections::HashMap;
 
-use dbus::arg::RefArg;
+use super::dbus_native::serialize::Variant;
 use fixedbitset::FixedBitSet;
 use oci_spec::runtime::LinuxCpu;
 
@@ -29,7 +29,7 @@ impl Controller for CpuSet {
     fn apply(
         options: &ControllerOpt,
         systemd_version: u32,
-        properties: &mut HashMap<&str, Box<dyn RefArg>>,
+        properties: &mut HashMap<&str, Variant>,
     ) -> Result<(), Self::Error> {
         if let Some(cpu) = options.resources.cpu() {
             tracing::debug!("Applying cpuset resource restrictions");
@@ -44,20 +44,28 @@ impl CpuSet {
     fn apply(
         cpu: &LinuxCpu,
         systemd_version: u32,
-        properties: &mut HashMap<&str, Box<dyn RefArg>>,
+        properties: &mut HashMap<&str, Variant>,
     ) -> Result<(), SystemdCpuSetError> {
         if systemd_version <= 243 {
             return Err(SystemdCpuSetError::OldSystemd);
         }
 
         if let Some(cpus) = cpu.cpus() {
-            let cpu_mask = to_bitmask(cpus).map_err(SystemdCpuSetError::CpusBitmask)?;
-            properties.insert(ALLOWED_CPUS, Box::new(cpu_mask));
+            let cpu_mask: Vec<_> = to_bitmask(cpus)
+                .map_err(SystemdCpuSetError::CpusBitmask)?
+                .into_iter()
+                .map(|v| v as u64)
+                .collect();
+            properties.insert(ALLOWED_CPUS, Variant::ArrayU64(cpu_mask));
         }
 
         if let Some(mems) = cpu.mems() {
-            let mems_mask = to_bitmask(mems).map_err(SystemdCpuSetError::MemoryNodesBitmask)?;
-            properties.insert(ALLOWED_NODES, Box::new(mems_mask));
+            let mems_mask: Vec<_> = to_bitmask(mems)
+                .map_err(SystemdCpuSetError::MemoryNodesBitmask)?
+                .into_iter()
+                .map(|v| v as u64)
+                .collect();
+            properties.insert(ALLOWED_NODES, Variant::ArrayU64(mems_mask));
         }
 
         Ok(())
@@ -128,9 +136,11 @@ pub fn to_bitmask(range: &str) -> Result<Vec<u8>, BitmaskError> {
 #[cfg(test)]
 mod tests {
     use anyhow::{Context, Result};
-    use dbus::arg::{ArgType, RefArg};
     use oci_spec::runtime::LinuxCpuBuilder;
 
+    use super::super::dbus_native::serialize::DbusSerialize;
+    use crate::recast;
+
     use super::*;
 
     #[test]
@@ -217,7 +227,7 @@ mod tests {
         let cpu = LinuxCpuBuilder::default()
             .build()
             .context("build cpu spec")?;
-        let mut properties: HashMap<&str, Box<dyn RefArg>> = HashMap::new();
+        let mut properties: HashMap<&str, Variant> = HashMap::new();
 
         let result = CpuSet::apply(&cpu, systemd_version, &mut properties);
 
@@ -233,18 +243,20 @@ mod tests {
             .mems("0-3")
             .build()
             .context("build cpu spec")?;
-        let mut properties: HashMap<&str, Box<dyn RefArg>> = HashMap::new();
+        let mut properties: HashMap<&str, Variant> = HashMap::new();
 
         CpuSet::apply(&cpu, systemd_version, &mut properties).context("apply cpuset")?;
 
         assert_eq!(properties.len(), 2);
         assert!(properties.contains_key(ALLOWED_CPUS));
         let cpus = properties.get(ALLOWED_CPUS).unwrap();
-        assert_eq!(cpus.arg_type(), ArgType::Array);
+        let v = recast!(cpus, Variant)?;
+        assert!(matches!(v, Variant::ArrayU64(_)));
 
         assert!(properties.contains_key(ALLOWED_NODES));
         let mems = properties.get(ALLOWED_NODES).unwrap();
-        assert_eq!(mems.arg_type(), ArgType::Array);
+        let v = recast!(mems, Variant)?;
+        assert!(matches!(v, Variant::ArrayU64(_)));
 
         Ok(())
     }