From 8ad0353b67fa4d68b474e6db267974e765c69aba Mon Sep 17 00:00:00 2001 From: jiaxiao zhou Date: Tue, 5 Nov 2024 23:38:30 +0000 Subject: [PATCH 1/5] runc: split Pipe, Io, and PipedIo to async and sync modules Signed-off-by: jiaxiao zhou --- crates/runc/src/asynchronous/mod.rs | 118 ++++++++++++++++++ crates/runc/src/asynchronous/pipe.rs | 38 ++++++ crates/runc/src/io.rs | 178 +-------------------------- crates/runc/src/lib.rs | 7 ++ crates/runc/src/options.rs | 2 +- crates/runc/src/synchronous/mod.rs | 117 ++++++++++++++++++ crates/runc/src/synchronous/pipe.rs | 30 +++++ 7 files changed, 314 insertions(+), 176 deletions(-) create mode 100644 crates/runc/src/asynchronous/mod.rs create mode 100644 crates/runc/src/asynchronous/pipe.rs create mode 100644 crates/runc/src/synchronous/mod.rs create mode 100644 crates/runc/src/synchronous/pipe.rs diff --git a/crates/runc/src/asynchronous/mod.rs b/crates/runc/src/asynchronous/mod.rs new file mode 100644 index 000000000..ae7fa6efa --- /dev/null +++ b/crates/runc/src/asynchronous/mod.rs @@ -0,0 +1,118 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +mod pipe; +use std::{fmt::Debug, io::Result, os::fd::AsRawFd}; + +use log::debug; +pub use pipe::Pipe; +use tokio::io::{AsyncRead, AsyncWrite}; + +use crate::Command; + +pub trait Io: Debug + Send + Sync { + /// Return write side of stdin + #[cfg(feature = "async")] + fn stdin(&self) -> Option> { + None + } + + /// Return read side of stdout + #[cfg(feature = "async")] + fn stdout(&self) -> Option> { + None + } + + /// Return read side of stderr + #[cfg(feature = "async")] + fn stderr(&self) -> Option> { + None + } + + /// Set IO for passed command. + /// Read side of stdin, write side of stdout and write side of stderr should be provided to command. + fn set(&self, cmd: &mut Command) -> Result<()>; + + /// Only close write side (should be stdout/err "from" runc process) + fn close_after_start(&self); +} + +#[derive(Debug)] +pub struct PipedIo { + pub stdin: Option, + pub stdout: Option, + pub stderr: Option, +} + +impl Io for PipedIo { + fn stdin(&self) -> Option> { + self.stdin.as_ref().and_then(|pipe| { + let fd = pipe.wr.as_raw_fd(); + tokio_pipe::PipeWrite::from_raw_fd_checked(fd) + .map(|x| Box::new(x) as Box) + .ok() + }) + } + + fn stdout(&self) -> Option> { + self.stdout.as_ref().and_then(|pipe| { + let fd = pipe.rd.as_raw_fd(); + tokio_pipe::PipeRead::from_raw_fd_checked(fd) + .map(|x| Box::new(x) as Box) + .ok() + }) + } + + fn stderr(&self) -> Option> { + self.stderr.as_ref().and_then(|pipe| { + let fd = pipe.rd.as_raw_fd(); + tokio_pipe::PipeRead::from_raw_fd_checked(fd) + .map(|x| Box::new(x) as Box) + .ok() + }) + } + + // Note that this internally use [`std::fs::File`]'s `try_clone()`. + // Thus, the files passed to commands will be not closed after command exit. + fn set(&self, cmd: &mut Command) -> std::io::Result<()> { + if let Some(p) = self.stdin.as_ref() { + let pr = p.rd.try_clone()?; + cmd.stdin(pr); + } + + if let Some(p) = self.stdout.as_ref() { + let pw = p.wr.try_clone()?; + cmd.stdout(pw); + } + + if let Some(p) = self.stderr.as_ref() { + let pw = p.wr.try_clone()?; + cmd.stdout(pw); + } + + Ok(()) + } + + fn close_after_start(&self) { + if let Some(p) = self.stdout.as_ref() { + nix::unistd::close(p.wr.as_raw_fd()).unwrap_or_else(|e| debug!("close stdout: {}", e)); + } + + if let Some(p) = self.stderr.as_ref() { + nix::unistd::close(p.wr.as_raw_fd()).unwrap_or_else(|e| debug!("close stderr: {}", e)); + } + } +} diff --git a/crates/runc/src/asynchronous/pipe.rs b/crates/runc/src/asynchronous/pipe.rs new file mode 100644 index 000000000..83a8cb797 --- /dev/null +++ b/crates/runc/src/asynchronous/pipe.rs @@ -0,0 +1,38 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +use std::os::unix::io::OwnedFd; + +use tokio::net::unix::pipe; + +/// Struct to represent a pipe that can be used to transfer stdio inputs and outputs. +/// +/// With this Io driver, methods of [crate::Runc] may capture the output/error messages. +/// When one side of the pipe is closed, the state will be represented with [`None`]. +#[derive(Debug)] +pub struct Pipe { + pub rd: OwnedFd, + pub wr: OwnedFd, +} + +impl Pipe { + pub fn new() -> std::io::Result { + let (tx, rx) = pipe::pipe()?; + let rd = tx.into_blocking_fd()?; + let wr = rx.into_blocking_fd()?; + Ok(Self { rd, wr }) + } +} diff --git a/crates/runc/src/io.rs b/crates/runc/src/io.rs index ee2babb32..98ad6ddf2 100644 --- a/crates/runc/src/io.rs +++ b/crates/runc/src/io.rs @@ -13,72 +13,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -#[cfg(not(feature = "async"))] -use std::io::{Read, Write}; + use std::{ fmt::Debug, fs::{File, OpenOptions}, io::Result, - os::unix::{ - fs::OpenOptionsExt, - io::{AsRawFd, OwnedFd}, - }, + os::unix::{fs::OpenOptionsExt, io::AsRawFd}, process::Stdio, sync::Mutex, }; -use log::debug; use nix::unistd::{Gid, Uid}; -#[cfg(feature = "async")] -use tokio::io::{AsyncRead, AsyncWrite}; -use tokio::net::unix::pipe; - -use crate::Command; - -pub trait Io: Debug + Send + Sync { - /// Return write side of stdin - #[cfg(not(feature = "async"))] - fn stdin(&self) -> Option> { - None - } - - /// Return read side of stdout - #[cfg(not(feature = "async"))] - fn stdout(&self) -> Option> { - None - } - - /// Return read side of stderr - #[cfg(not(feature = "async"))] - fn stderr(&self) -> Option> { - None - } - - /// Return write side of stdin - #[cfg(feature = "async")] - fn stdin(&self) -> Option> { - None - } - - /// Return read side of stdout - #[cfg(feature = "async")] - fn stdout(&self) -> Option> { - None - } - - /// Return read side of stderr - #[cfg(feature = "async")] - fn stderr(&self) -> Option> { - None - } - /// Set IO for passed command. - /// Read side of stdin, write side of stdout and write side of stderr should be provided to command. - fn set(&self, cmd: &mut Command) -> Result<()>; - - /// Only close write side (should be stdout/err "from" runc process) - fn close_after_start(&self); -} +use crate::{Command, Io, Pipe, PipedIo}; #[derive(Debug, Clone)] pub struct IOOption { @@ -97,32 +44,6 @@ impl Default for IOOption { } } -/// Struct to represent a pipe that can be used to transfer stdio inputs and outputs. -/// -/// With this Io driver, methods of [crate::Runc] may capture the output/error messages. -/// When one side of the pipe is closed, the state will be represented with [`None`]. -#[derive(Debug)] -pub struct Pipe { - rd: OwnedFd, - wr: OwnedFd, -} - -#[derive(Debug)] -pub struct PipedIo { - stdin: Option, - stdout: Option, - stderr: Option, -} - -impl Pipe { - fn new() -> std::io::Result { - let (tx, rx) = pipe::pipe()?; - let rd = tx.into_blocking_fd()?; - let wr = rx.into_blocking_fd()?; - Ok(Self { rd, wr }) - } -} - impl PipedIo { pub fn new(uid: u32, gid: u32, opts: &IOOption) -> std::io::Result { Ok(Self { @@ -156,99 +77,6 @@ impl PipedIo { } } -impl Io for PipedIo { - #[cfg(not(feature = "async"))] - fn stdin(&self) -> Option> { - self.stdin.as_ref().and_then(|pipe| { - pipe.wr - .try_clone() - .map(|x| Box::new(x) as Box) - .ok() - }) - } - - #[cfg(feature = "async")] - fn stdin(&self) -> Option> { - self.stdin.as_ref().and_then(|pipe| { - let fd = pipe.wr.as_raw_fd(); - tokio_pipe::PipeWrite::from_raw_fd_checked(fd) - .map(|x| Box::new(x) as Box) - .ok() - }) - } - - #[cfg(not(feature = "async"))] - fn stdout(&self) -> Option> { - self.stdout.as_ref().and_then(|pipe| { - pipe.rd - .try_clone() - .map(|x| Box::new(x) as Box) - .ok() - }) - } - - #[cfg(feature = "async")] - fn stdout(&self) -> Option> { - self.stdout.as_ref().and_then(|pipe| { - let fd = pipe.rd.as_raw_fd(); - tokio_pipe::PipeRead::from_raw_fd_checked(fd) - .map(|x| Box::new(x) as Box) - .ok() - }) - } - - #[cfg(not(feature = "async"))] - fn stderr(&self) -> Option> { - self.stderr.as_ref().and_then(|pipe| { - pipe.rd - .try_clone() - .map(|x| Box::new(x) as Box) - .ok() - }) - } - - #[cfg(feature = "async")] - fn stderr(&self) -> Option> { - self.stderr.as_ref().and_then(|pipe| { - let fd = pipe.rd.as_raw_fd(); - tokio_pipe::PipeRead::from_raw_fd_checked(fd) - .map(|x| Box::new(x) as Box) - .ok() - }) - } - - // Note that this internally use [`std::fs::File`]'s `try_clone()`. - // Thus, the files passed to commands will be not closed after command exit. - fn set(&self, cmd: &mut Command) -> std::io::Result<()> { - if let Some(p) = self.stdin.as_ref() { - let pr = p.rd.try_clone()?; - cmd.stdin(pr); - } - - if let Some(p) = self.stdout.as_ref() { - let pw = p.wr.try_clone()?; - cmd.stdout(pw); - } - - if let Some(p) = self.stderr.as_ref() { - let pw = p.wr.try_clone()?; - cmd.stdout(pw); - } - - Ok(()) - } - - fn close_after_start(&self) { - if let Some(p) = self.stdout.as_ref() { - nix::unistd::close(p.wr.as_raw_fd()).unwrap_or_else(|e| debug!("close stdout: {}", e)); - } - - if let Some(p) = self.stderr.as_ref() { - nix::unistd::close(p.wr.as_raw_fd()).unwrap_or_else(|e| debug!("close stderr: {}", e)); - } - } -} - /// IO driver to direct output/error messages to /dev/null. /// /// With this Io driver, all methods of [crate::Runc] can't capture the output/error messages. diff --git a/crates/runc/src/lib.rs b/crates/runc/src/lib.rs index 8b9fd67ae..53c18f045 100644 --- a/crates/runc/src/lib.rs +++ b/crates/runc/src/lib.rs @@ -50,8 +50,14 @@ use async_trait::async_trait; use log::debug; use oci_spec::runtime::{LinuxResources, Process}; +#[cfg(feature = "async")] +pub use crate::asynchronous::*; +#[cfg(not(feature = "async"))] +pub use crate::synchronous::*; use crate::{container::Container, error::Error, options::*, utils::write_value_to_temp_file}; +#[cfg(feature = "async")] +pub mod asynchronous; pub mod container; pub mod error; pub mod events; @@ -59,6 +65,7 @@ pub mod io; #[cfg(feature = "async")] pub mod monitor; pub mod options; +pub mod synchronous; pub mod utils; pub type Result = std::result::Result; diff --git a/crates/runc/src/options.rs b/crates/runc/src/options.rs index 2df488bd0..627853887 100644 --- a/crates/runc/src/options.rs +++ b/crates/runc/src/options.rs @@ -39,7 +39,7 @@ use std::{ time::Duration, }; -use crate::{error::Error, io::Io, utils, DefaultExecutor, LogFormat, Runc, Spawner}; +use crate::{error::Error, utils, DefaultExecutor, Io, LogFormat, Runc, Spawner}; // constants for log format pub const JSON: &str = "json"; diff --git a/crates/runc/src/synchronous/mod.rs b/crates/runc/src/synchronous/mod.rs new file mode 100644 index 000000000..7ee8d35e0 --- /dev/null +++ b/crates/runc/src/synchronous/mod.rs @@ -0,0 +1,117 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +mod pipe; +use std::{ + fmt::Debug, + io::{Read, Result, Write}, + os::fd::AsRawFd, +}; + +use log::debug; +pub use pipe::Pipe; + +use crate::Command; + +pub trait Io: Debug + Send + Sync { + /// Return write side of stdin + fn stdin(&self) -> Option> { + None + } + + /// Return read side of stdout + fn stdout(&self) -> Option> { + None + } + + /// Return read side of stderr + fn stderr(&self) -> Option> { + None + } + + /// Set IO for passed command. + /// Read side of stdin, write side of stdout and write side of stderr should be provided to command. + fn set(&self, cmd: &mut Command) -> Result<()>; + + /// Only close write side (should be stdout/err "from" runc process) + fn close_after_start(&self); +} + +#[derive(Debug)] +pub struct PipedIo { + pub stdin: Option, + pub stdout: Option, + pub stderr: Option, +} + +impl Io for PipedIo { + fn stdin(&self) -> Option> { + self.stdin.as_ref().and_then(|pipe| { + pipe.wr + .try_clone() + .map(|x| Box::new(x) as Box) + .ok() + }) + } + + fn stdout(&self) -> Option> { + self.stdout.as_ref().and_then(|pipe| { + pipe.rd + .try_clone() + .map(|x| Box::new(x) as Box) + .ok() + }) + } + + fn stderr(&self) -> Option> { + self.stderr.as_ref().and_then(|pipe| { + pipe.rd + .try_clone() + .map(|x| Box::new(x) as Box) + .ok() + }) + } + // Note that this internally use [`std::fs::File`]'s `try_clone()`. + // Thus, the files passed to commands will be not closed after command exit. + fn set(&self, cmd: &mut Command) -> std::io::Result<()> { + if let Some(p) = self.stdin.as_ref() { + let pr = p.rd.try_clone()?; + cmd.stdin(pr); + } + + if let Some(p) = self.stdout.as_ref() { + let pw = p.wr.try_clone()?; + cmd.stdout(pw); + } + + if let Some(p) = self.stderr.as_ref() { + let pw = p.wr.try_clone()?; + cmd.stdout(pw); + } + + Ok(()) + } + + fn close_after_start(&self) { + if let Some(p) = self.stdout.as_ref() { + nix::unistd::close(p.wr.as_raw_fd()).unwrap_or_else(|e| debug!("close stdout: {}", e)); + } + + if let Some(p) = self.stderr.as_ref() { + nix::unistd::close(p.wr.as_raw_fd()).unwrap_or_else(|e| debug!("close stderr: {}", e)); + } + } +} diff --git a/crates/runc/src/synchronous/pipe.rs b/crates/runc/src/synchronous/pipe.rs new file mode 100644 index 000000000..7c882160a --- /dev/null +++ b/crates/runc/src/synchronous/pipe.rs @@ -0,0 +1,30 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +use os_pipe::{pipe, PipeReader, PipeWriter}; + +#[derive(Debug)] +pub struct Pipe { + pub rd: PipeReader, + pub wr: PipeWriter, +} + +impl Pipe { + pub fn new() -> std::io::Result { + let (rd, wr) = pipe()?; + Ok(Self { rd, wr }) + } +} From b6ed34384f1ecd62f44d5d809e2a503c6f7ade31 Mon Sep 17 00:00:00 2001 From: "Jiaxiao Zhou (Mossaka)" Date: Sat, 16 Nov 2024 13:19:27 -0700 Subject: [PATCH 2/5] runc/Cargo: add the missing os_pipe dependency Signed-off-by: Jiaxiao Zhou (Mossaka) --- crates/runc/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/runc/Cargo.toml b/crates/runc/Cargo.toml index 1e969eb6c..40167b430 100644 --- a/crates/runc/Cargo.toml +++ b/crates/runc/Cargo.toml @@ -28,6 +28,7 @@ tempfile.workspace = true thiserror.workspace = true time.workspace = true uuid.workspace = true +os_pipe.workspace = true # Async dependencies async-trait = { workspace = true, optional = true } From 347ab57ea52a08d23467a581abccbbf675b36a87 Mon Sep 17 00:00:00 2001 From: "Jiaxiao Zhou (Mossaka)" Date: Sat, 16 Nov 2024 13:20:09 -0700 Subject: [PATCH 3/5] runc/src/io: expose Io as a public API Signed-off-by: Jiaxiao Zhou (Mossaka) --- crates/runc/src/io.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/runc/src/io.rs b/crates/runc/src/io.rs index 98ad6ddf2..87747ff9d 100644 --- a/crates/runc/src/io.rs +++ b/crates/runc/src/io.rs @@ -25,7 +25,8 @@ use std::{ use nix::unistd::{Gid, Uid}; -use crate::{Command, Io, Pipe, PipedIo}; +pub use crate::Io; +use crate::{Command, Pipe, PipedIo}; #[derive(Debug, Clone)] pub struct IOOption { From 9559a5232ce92e6a64f3a4468f1e3db94714f2a8 Mon Sep 17 00:00:00 2001 From: "Jiaxiao Zhou (Mossaka)" Date: Sat, 16 Nov 2024 13:33:33 -0700 Subject: [PATCH 4/5] .github/workflows/ci: add check to runc shim The runc shim, like the containerd-shim, has a 'sync' feature that is not checked by the workspace. Thus we need to add that individually to the CI to get full coverage Signed-off-by: Jiaxiao Zhou (Mossaka) --- .github/workflows/ci.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c3af16d25..2e2e0615f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,18 +22,21 @@ jobs: - run: ./scripts/install-protobuf.sh shell: bash - - run: cargo check --examples --tests --all-targets - - run: cargo check --examples --tests --all-targets --all-features - - run: rustup toolchain install nightly --component rustfmt - run: cargo +nightly fmt --all -- --check --files-with-diff + # the "runc" and "containerd-shim" crates have `sync` code that is not covered by the workspace + - run: cargo check -p runc --all-targets + - run: cargo clippy -p runc --all-targets -- -D warnings + - run: cargo check -p containerd-shim --all-targets + - run: cargo clippy -p containerd-shim --all-targets -- -D warnings + + # check the workspace + - run: cargo check --examples --tests --all-targets + - run: cargo check --examples --tests --all-targets --all-features - run: cargo clippy --all-targets -- -D warnings - run: cargo clippy --all-targets --all-features -- -D warnings - # the shim has sync code that is not covered when running with --all-features - - run: cargo clippy -p containerd-shim --all-targets -- -D warnings - - run: cargo doc --no-deps --features docs env: RUSTDOCFLAGS: -Dwarnings From 7110c93fa6138171b81ae9aa9876b5a815e55988 Mon Sep 17 00:00:00 2001 From: "Jiaxiao Zhou (Mossaka)" Date: Sat, 16 Nov 2024 19:25:56 -0800 Subject: [PATCH 5/5] runc/src/io: added the missing imports to unit tests Signed-off-by: Jiaxiao Zhou (Mossaka) --- crates/runc/src/io.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/runc/src/io.rs b/crates/runc/src/io.rs index 87747ff9d..cb787c7e8 100644 --- a/crates/runc/src/io.rs +++ b/crates/runc/src/io.rs @@ -212,6 +212,8 @@ mod tests { #[cfg(not(feature = "async"))] #[test] fn test_create_piped_io() { + use std::io::{Read, Write}; + let opts = IOOption::default(); let uid = nix::unistd::getuid(); let gid = nix::unistd::getgid();