Fix the feature test and turn on in CI (#2060)

* reworked the justfile to be explicit with path

Signed-off-by: yihuaf <yihuaf@unkies.org>

* fix the feature tests

Signed-off-by: yihuaf <yihuaf@unkies.org>

* add the musl test

Signed-off-by: yihuaf <yihuaf@unkies.org>

* moving all stub into a single dir

Signed-off-by: yihuaf <yihuaf@unkies.org>

---------

Signed-off-by: yihuaf <yihuaf@unkies.org>

.github/workflows/main.yml

@@ -27,6 +27,7 @@ jobs:
           for file in ${{ steps.filter.outputs.all_modified_files }}; do
             echo "$file was changed"
           done
+
   check:
     needs: [changes]
     if: needs.changes.outputs.any_modified == 'true'
@@ -66,6 +67,8 @@ jobs:
         run: |
           export LD_LIBRARY_PATH=$HOME/.wasmedge/lib
           cd ./crates && cargo test --all --all-features --no-fail-fast
+      - name: Run feature tests
+        run: just featuretest
 
   coverage:
     needs: [changes]

Cargo.toml

@@ -1,4 +1,5 @@
 [workspace]
+resolver = "2"
 members = ["tests/rust-integration-tests/*", "crates/*", "tools/*"]
 
 [profile.release]

crates/justfile

@@ -1,15 +0,0 @@
-
-# build debug version of youki
-debug: 
-    cargo build
-    cp ./target/debug/youki ./youki_bin
-
-# build release version of youki
-release:
-    cargo build --release
-    cp ./target/release/youki ./youki_bin
-
-# clean the generated artifacts
-clean:
-    rm -f ./youki_bin
-    cargo clean

crates/libcgroups/src/common.rs

@@ -16,11 +16,8 @@ use oci_spec::runtime::{
     LinuxDevice, LinuxDeviceBuilder, LinuxDeviceCgroup, LinuxDeviceCgroupBuilder, LinuxDeviceType,
 };
 
-#[cfg(feature = "systemd")]
 use super::systemd;
-#[cfg(feature = "v1")]
 use super::v1;
-#[cfg(feature = "v2")]
 use super::v2;
 
 use super::stats::Stats;
@@ -354,8 +351,10 @@ fn create_v1_cgroup_manager(
 }
 
 #[cfg(not(feature = "v1"))]
-fn create_v1_cgroup_manager(_cgroup_path: PathBuf) -> Result<Box<dyn CgroupManager>> {
-    bail!("cgroup v1 feature is required, but was not enabled during compile time");
+fn create_v1_cgroup_manager(
+    _cgroup_path: PathBuf,
+) -> Result<v1::manager::Manager, v1::manager::V1ManagerError> {
+    Err(v1::manager::V1ManagerError::NotEnabled)
 }
 
 #[cfg(feature = "v2")]
@@ -367,8 +366,10 @@ fn create_v2_cgroup_manager(
 }
 
 #[cfg(not(feature = "v2"))]
-fn create_v2_cgroup_manager(_cgroup_path: PathBuf) -> Result<Box<dyn CgroupManager>> {
-    bail!("cgroup v2 feature is required, but was not enabled during compile time");
+fn create_v2_cgroup_manager(
+    _cgroup_path: PathBuf,
+) -> Result<v2::manager::Manager, v2::manager::V2ManagerError> {
+    Err(v2::manager::V2ManagerError::NotEnabled)
 }
 
 #[cfg(feature = "systemd")]
@@ -400,8 +401,8 @@ fn create_systemd_cgroup_manager(
 fn create_systemd_cgroup_manager(
     _cgroup_path: PathBuf,
     _container_name: &str,
-) -> Result<Box<dyn CgroupManager>> {
-    bail!("systemd cgroup feature is required, but was not enabled during compile time");
+) -> Result<systemd::manager::Manager, systemd::manager::SystemdManagerError> {
+    Err(systemd::manager::SystemdManagerError::NotEnabled)
 }
 
 pub fn get_all_pids(path: &Path) -> Result<Vec<Pid>, WrappedIoError> {

crates/libcgroups/src/lib.rs

@@ -15,8 +15,17 @@ pub mod common;
 pub mod stats;
 #[cfg(feature = "systemd")]
 pub mod systemd;
+#[cfg(not(feature = "systemd"))]
+#[path = "stub/systemd/mod.rs"]
+pub mod systemd;
 pub mod test_manager;
 #[cfg(feature = "v1")]
 pub mod v1;
+#[cfg(not(feature = "v1"))]
+#[path = "stub/v1/mod.rs"]
+pub mod v1;
 #[cfg(feature = "v2")]
 pub mod v2;
+#[cfg(not(feature = "v2"))]
+#[path = "stub/v2/mod.rs"]
+pub mod v2;

crates/libcgroups/src/stub/systemd/manager.rs

@@ -0,0 +1,43 @@
+use crate::common::{AnyCgroupManager, CgroupManager};
+
+#[derive(thiserror::Error, Debug)]
+pub enum SystemdManagerError {
+    #[error("systemd cgroup feature is required, but was not enabled during compile time")]
+    NotEnabled,
+}
+
+pub struct Manager {}
+
+impl Manager {
+    pub fn any(self) -> AnyCgroupManager {
+        AnyCgroupManager::Systemd(self)
+    }
+}
+
+impl CgroupManager for Manager {
+    type Error = SystemdManagerError;
+
+    fn add_task(&self, _pid: nix::unistd::Pid) -> Result<(), Self::Error> {
+        Err(SystemdManagerError::NotEnabled)
+    }
+
+    fn apply(&self, _controller_opt: &crate::common::ControllerOpt) -> Result<(), Self::Error> {
+        Err(SystemdManagerError::NotEnabled)
+    }
+
+    fn remove(&self) -> Result<(), Self::Error> {
+        Err(SystemdManagerError::NotEnabled)
+    }
+
+    fn freeze(&self, _state: crate::common::FreezerState) -> Result<(), Self::Error> {
+        Err(SystemdManagerError::NotEnabled)
+    }
+
+    fn stats(&self) -> Result<crate::stats::Stats, Self::Error> {
+        Err(SystemdManagerError::NotEnabled)
+    }
+
+    fn get_all_pids(&self) -> Result<Vec<nix::unistd::Pid>, Self::Error> {
+        Err(SystemdManagerError::NotEnabled)
+    }
+}

crates/libcgroups/src/stub/systemd/mod.rs

@@ -0,0 +1 @@
+pub mod manager;

crates/libcgroups/src/stub/v1/manager.rs

@@ -0,0 +1,43 @@
+use crate::common::{AnyCgroupManager, CgroupManager};
+
+#[derive(thiserror::Error, Debug)]
+pub enum V1ManagerError {
+    #[error("v1 cgroup feature is required, but was not enabled during compile time")]
+    NotEnabled,
+}
+
+pub struct Manager {}
+
+impl Manager {
+    pub fn any(self) -> AnyCgroupManager {
+        crate::common::AnyCgroupManager::V1(self)
+    }
+}
+
+impl CgroupManager for Manager {
+    type Error = V1ManagerError;
+
+    fn add_task(&self, _pid: nix::unistd::Pid) -> Result<(), Self::Error> {
+        Err(V1ManagerError::NotEnabled)
+    }
+
+    fn apply(&self, _controller_opt: &crate::common::ControllerOpt) -> Result<(), Self::Error> {
+        Err(V1ManagerError::NotEnabled)
+    }
+
+    fn remove(&self) -> Result<(), Self::Error> {
+        Err(V1ManagerError::NotEnabled)
+    }
+
+    fn freeze(&self, _state: crate::common::FreezerState) -> Result<(), Self::Error> {
+        Err(V1ManagerError::NotEnabled)
+    }
+
+    fn stats(&self) -> Result<crate::stats::Stats, Self::Error> {
+        Err(V1ManagerError::NotEnabled)
+    }
+
+    fn get_all_pids(&self) -> Result<Vec<nix::unistd::Pid>, Self::Error> {
+        Err(V1ManagerError::NotEnabled)
+    }
+}

crates/libcgroups/src/stub/v1/mod.rs

@@ -0,0 +1 @@
+pub mod manager;

crates/libcgroups/src/stub/v2/manager.rs

@@ -0,0 +1,43 @@
+use crate::common::{AnyCgroupManager, CgroupManager};
+
+#[derive(thiserror::Error, Debug)]
+pub enum V2ManagerError {
+    #[error("v2 cgroup feature is required, but was not enabled during compile time")]
+    NotEnabled,
+}
+
+pub struct Manager {}
+
+impl Manager {
+    pub fn any(self) -> AnyCgroupManager {
+        crate::common::AnyCgroupManager::V2(self)
+    }
+}
+
+impl CgroupManager for Manager {
+    type Error = V2ManagerError;
+
+    fn add_task(&self, _pid: nix::unistd::Pid) -> Result<(), Self::Error> {
+        Err(V2ManagerError::NotEnabled)
+    }
+
+    fn apply(&self, _controller_opt: &crate::common::ControllerOpt) -> Result<(), Self::Error> {
+        Err(V2ManagerError::NotEnabled)
+    }
+
+    fn remove(&self) -> Result<(), Self::Error> {
+        Err(V2ManagerError::NotEnabled)
+    }
+
+    fn freeze(&self, _state: crate::common::FreezerState) -> Result<(), Self::Error> {
+        Err(V2ManagerError::NotEnabled)
+    }
+
+    fn stats(&self) -> Result<crate::stats::Stats, Self::Error> {
+        Err(V2ManagerError::NotEnabled)
+    }
+
+    fn get_all_pids(&self) -> Result<Vec<nix::unistd::Pid>, Self::Error> {
+        Err(V2ManagerError::NotEnabled)
+    }
+}

crates/libcgroups/src/stub/v2/mod.rs

@@ -0,0 +1 @@
+pub mod manager;

crates/libcgroups/src/v1/manager.rs

@@ -33,6 +33,7 @@ use crate::v1::ControllerType;
 pub struct Manager {
     subsystems: HashMap<CtrlType, PathBuf>,
 }
+
 #[derive(thiserror::Error, Debug)]
 pub enum V1ManagerError {
     #[error("io error: {0}")]

crates/libcontainer/Cargo.toml

@@ -18,7 +18,6 @@ systemd = ["libcgroups/systemd", "v2"]
 v2 = ["libcgroups/v2"]
 v1 = ["libcgroups/v1"]
 cgroupsv2_devices = ["libcgroups/cgroupsv2_devices"]
-test_utils = ["dep:rand"]
 
 [dependencies]
 bitflags = "2.3.2"
@@ -42,7 +41,6 @@ regex = "1.8.4"
 thiserror = "1.0.24"
 tracing = { version = "0.1.37", features = ["attributes"]}
 safe-path = "0.1.0"
-rand = { version = "0.8.5", optional = true }
 
 [dev-dependencies]
 oci-spec = { version = "^0.6.0", features = ["proptests", "runtime"] }
@@ -50,3 +48,4 @@ quickcheck = "1"
 serial_test = "2.0.0"
 tempfile = "3"
 anyhow = "1.0"
+rand = { version = "0.8.5" }

crates/libcontainer/src/lib.rs

@@ -14,7 +14,6 @@ pub mod rootless;
 pub mod seccomp;
 pub mod signal;
 pub mod syscall;
-#[cfg(feature = "test_utils")]
 pub mod test_utils;
 pub mod tty;
 pub mod utils;

crates/libcontainer/src/rootfs/symlink.rs

@@ -112,8 +112,8 @@ mod tests {
     #[cfg(feature = "v1")]
     use crate::syscall::linux::LinuxSyscall;
     use crate::syscall::test::TestHelperSyscall;
-    use anyhow::{Context, Result};
     #[cfg(feature = "v1")]
+    use anyhow::{Context, Result};
     use nix::{
         fcntl::{open, OFlag},
         sys::stat::Mode,

crates/libcontainer/src/rootless.rs

@@ -452,16 +452,18 @@ fn write_id_mapping(
 mod tests {
     use std::fs;
 
+    use super::*;
     use anyhow::Result;
     use nix::unistd::getpid;
     use oci_spec::runtime::{
         LinuxBuilder, LinuxIdMappingBuilder, LinuxNamespaceBuilder, SpecBuilder,
     };
+    use rand::Rng;
     use serial_test::serial;
 
-    use crate::test_utils::gen_u32;
-
-    use super::*;
+    fn gen_u32() -> u32 {
+        rand::thread_rng().gen()
+    }
 
     #[test]
     fn test_validate_ok() -> Result<()> {

crates/libcontainer/src/test_utils.rs

@@ -1,5 +1,4 @@
 use nix::sys::wait;
-use rand::Rng;
 use serde::{Deserialize, Serialize};
 
 // Normally, error types are not implemented as serialize/deserialize, but to
@@ -84,11 +83,15 @@ where
     let (mut sender, mut receiver) = crate::channel::channel::<ClosureResult>()?;
     match unsafe { nix::unistd::fork().map_err(TestError::Fork)? } {
         nix::unistd::ForkResult::Parent { child } => {
+            // Close unused senders
+            sender.close().map_err(TestError::Channel)?;
             let res = receiver.recv().map_err(TestError::Channel)?;
             wait::waitpid(child, None).map_err(TestError::Wait)?;
             res.map_err(|err| TestError::Execution(Box::new(err)))?;
         }
         nix::unistd::ForkResult::Child => {
+            // Close unused receiver in the child
+            receiver.close().map_err(TestError::Channel)?;
             let test_result = match std::panic::catch_unwind(cb) {
                 Ok(ret) => ret.map_err(|err| ErrorEnclosure::new(&err)),
                 Err(_) => Err(ErrorEnclosure::new(&TestError::Panic)),
@@ -104,10 +107,6 @@ where
     Ok(())
 }
 
-pub fn gen_u32() -> u32 {
-    rand::thread_rng().gen()
-}
-
 #[cfg(test)]
 mod tests {
     use core::panic;

crates/youki/Cargo.toml

@@ -54,7 +54,6 @@ tracing-journald = "0.3.0"
 serial_test = "2.0.0"
 tempfile = "3"
 scopeguard = "1.1.0"
-libcontainer = {version = "0.0.5", path = "../libcontainer", features = ["test_utils"]}
 
 [build-dependencies]
 anyhow = "1.0.71"

justfile

@@ -3,6 +3,8 @@ alias youki := youki-dev
 
 KIND_CLUSTER_NAME := 'youki'
 
+cwd := justfile_directory()
+
 # build
 
 # build all binaries
@@ -10,19 +12,19 @@ build-all: youki-release rust-oci-tests-bin runtimetest
 
 # build youki in dev mode
 youki-dev:
-    {{ justfile_directory() }}/scripts/build.sh -o {{ justfile_directory() }} -c youki
+    {{ cwd }}/scripts/build.sh -o {{ cwd }} -c youki
 
 # build youki in release mode
 youki-release:
-    {{ justfile_directory() }}/scripts/build.sh -o . -r -c youki
+    {{ cwd }}/scripts/build.sh -o {{ cwd }} -r -c youki
 
 # build runtimetest binary
 runtimetest:
-    {{ justfile_directory() }}/scripts/build.sh -o . -r -c runtimetest
+    {{ cwd }}/scripts/build.sh -o {{ cwd }} -r -c runtimetest
 
 # build rust oci tests binary
 rust-oci-tests-bin:
-    {{ justfile_directory() }}/scripts/build.sh -o {{ justfile_directory() }} -r -c integration-test
+    {{ cwd }}/scripts/build.sh -o {{ cwd }} -r -c integration-test
 
 # Tests
 
@@ -37,21 +39,21 @@ unittest:
     cd ./crates
     LD_LIBRARY_PATH=${HOME}/.wasmedge/lib cargo test --all --all-targets --all-features
 
-# run purmutated feature compilation tests
+# run permutated feature compilation tests
 featuretest:
-    ./scripts/features_test.sh
+    {{ cwd }}/scripts/features_test.sh
 
 # run oci integration tests
 oci-tests: 
-    ./scripts/oci_integration_tests.sh {{ justfile_directory() }}
+    {{ cwd }}/scripts/oci_integration_tests.sh {{ cwd }}
 
 # run rust oci integration tests
 rust-oci-tests: youki-release runtimetest rust-oci-tests-bin
-    ./scripts/rust_integration_tests.sh ./youki
+    {{ cwd }}/scripts/rust_integration_tests.sh {{ cwd }}/youki
 
 # validate rust oci integration tests on runc
 validate-rust-oci-runc: runtimetest rust-oci-tests-bin
-    ./scripts/rust_integration_tests.sh runc
+    {{ cwd }}/scripts/rust_integration_tests.sh runc
 
 # run containerd integration tests
 containerd-test: youki-dev
@@ -61,7 +63,7 @@ containerd-test: youki-dev
 [private]
 kind-cluster: bin-kind
     #!/usr/bin/env bash
-    set -euxo pipefail
+    set -euo pipefail
 
     mkdir -p tests/k8s/_out/
     docker buildx build -f tests/k8s/Dockerfile --iidfile=tests/k8s/_out/img --load .
@@ -108,7 +110,7 @@ format:
 
 # cleans up generated artifacts
 clean:
-    ./scripts/clean.sh .
+    {{ cwd }}/scripts/clean.sh {{ cwd }}
 
 # install tools used in dev
 dev-prepare:

scripts/build.sh

@@ -1,6 +1,5 @@
-#!/bin/bash
-
-set -e
+#!/usr/bin/env bash
+set -euo pipefail
 
 ROOT=$(git rev-parse --show-toplevel)
 
@@ -13,6 +12,7 @@ VERSION=debug
 TARGET="$(uname -m)-unknown-linux-gnu"
 CRATE="youki"
 RUNTIMETEST_TARGET="$ROOT/runtimetest-target"
+features=""
 while getopts f:ro:c:h OPT; do
     case $OPT in
         f) features=${OPTARG}

scripts/clean.sh

@@ -1,10 +1,9 @@
-#! /bin/bash
-# we don't set -eu here, as some of the binaries might be potentially be missing
-# and that is fine, that means they are already removed.
+#!/usr/bin/env bash
+set -euo pipefail
 
 for bin in youki integration_test runtimetest test.log; do
     if [ -f $bin ]; then
-        rm ${1}/$bin
+        rm -f ${1}/$bin
     fi
 done
 

scripts/features_test.sh

@@ -1,32 +1,33 @@
-#!/bin/bash
-set -eu
+#!/usr/bin/env bash
+set -euo pipefail
 
-# Build the different features individually
-cargo build --no-default-features -F v1
-cargo build --no-default-features -F v2
-cargo build --no-default-features -F systemd
-cargo build --no-default-features -F v2 -F cgroupsv2_devices
-cargo build --no-default-features -F systemd -F cgroupsv2_devices
+test_package_features() {
+    echo "[feature test] testing $1 with features $2"
+    cargo build --no-default-features --package "$1" --features "$2"
+}
 
-# Test the different features individually
-cargo test --no-default-features -F v1
-cargo test --no-default-features -F v2
-cargo test --no-default-features -F systemd
-cargo test --no-default-features -F v2 -F cgroupsv2_devices
-cargo test --no-default-features -F systemd -F cgroupsv2_devices
+test_package_features "libcontainer" "v1"
+test_package_features "libcontainer" "v2"
+test_package_features "libcontainer" "systemd"
+test_package_features "libcontainer" "v2 cgroupsv2_devices"
+test_package_features "libcontainer" "systemd cgroupsv2_devices"
 
-# Build with musl: libcontainer
-cargo +nightly build \
-    -Zbuild-std \
-    --target $(uname -m)-unknown-linux-musl \
-    --package libcontainer \
-    --no-default-features -F v2
+test_package_features "libcgroups" "v1"
+test_package_features "libcgroups" "v2"
+test_package_features "libcgroups" "systemd"
+test_package_features "libcgroups" "v2 cgroupsv2_devices"
+test_package_features "libcgroups" "systemd cgroupsv2_devices"
 
-# Test with musl: libcontainer
-cargo +nightly test \
-    -Zbuild-std \
-    --target $(uname -m)-unknown-linux-musl \
-    --package libcontainer \
-    --no-default-features -F v2
+test_features() {
+    echo "[feature test] testing features $1"
+    cargo build --no-default-features --features "$1"
+    cargo test run --no-default-features --features "$1"
+}
+
+test_features "v1"
+test_features "v2"
+test_features "systemd"
+test_features "v2 cgroupsv2_devices"
+test_features "systemd cgroupsv2_devices"
 
 exit 0

scripts/test_musl.sh

@@ -0,0 +1,21 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+test_musl() {
+    echo "[musl test] testing $1 with features $2"
+    cargo +nightly build \
+        -Zbuild-std \
+        --target $(uname -m)-unknown-linux-musl \
+        --package libcontainer \
+        --no-default-features -F v2
+    cargo +nightly test \
+        -Zbuild-std \
+        --target $(uname -m)-unknown-linux-musl \
+        --package libcontainer \
+        --no-default-features -F v2
+}
+
+test_musl "libcontainer" "v1"
+test_musl "libcontainer" "v2"
+test_musl "libcontainer" "v1 v2"
+

tests/rust-integration-tests/justfile

@@ -1,20 +0,0 @@
-TGT := "x86_64-unknown-linux-gnu"
-FLAG := ""
-DIR := if FLAG == "release" {"release"} else {"debug"}
-
-check:
-    echo {{DIR}}
-
-all: runtimetest integration-test
-
-runtimetest:
-    cd ./runtimetest && cargo build {{FLAG}}
-    cp ./target/{{TGT}}/{{DIR}}/runtimetest ../runtimetest_bin
-
-integration-test:
-    cd ./integration_test && cargo build {{FLAG}}
-    cp ./target/{{DIR}}/integration_test ../integration_test_bin
-
-clean:
-    rm ./integration_test_bin
-    rm ./runtimetest_bin