From efb7390357854043961ae26b501330e093daea56 Mon Sep 17 00:00:00 2001 From: Alex Lyn Date: Tue, 16 Jul 2024 15:54:19 +0800 Subject: [PATCH] kata-sys-utils: align OCI Spec with oci-spec-rs Do align oci spec and fix warnings to make clippy happy. Fixes #9766 Signed-off-by: Alex Lyn --- src/libs/kata-sys-util/Cargo.toml | 3 +- src/libs/kata-sys-util/src/cpu.rs | 6 +- src/libs/kata-sys-util/src/hooks.rs | 144 +++++++++++++++-------- src/libs/kata-sys-util/src/k8s.rs | 29 +++-- src/libs/kata-sys-util/src/mount.rs | 21 +++- src/libs/kata-sys-util/src/numa.rs | 2 +- src/libs/kata-sys-util/src/protection.rs | 2 +- src/libs/kata-sys-util/src/spec.rs | 41 ++++--- 8 files changed, 167 insertions(+), 81 deletions(-) diff --git a/src/libs/kata-sys-util/Cargo.toml b/src/libs/kata-sys-util/Cargo.toml index bf0a664310..13d860ee45 100644 --- a/src/libs/kata-sys-util/Cargo.toml +++ b/src/libs/kata-sys-util/Cargo.toml @@ -31,7 +31,8 @@ thiserror = "1.0.30" hex = "0.4.3" kata-types = { path = "../kata-types" } -oci = { path = "../oci" } +oci-spec = { version = "0.6.8", features = ["runtime"] } +runtime-spec = { path = "../runtime-spec" } safe-path = { path = "../safe-path" } [dev-dependencies] diff --git a/src/libs/kata-sys-util/src/cpu.rs b/src/libs/kata-sys-util/src/cpu.rs index 97bc2fd943..3ab2cc7f78 100644 --- a/src/libs/kata-sys-util/src/cpu.rs +++ b/src/libs/kata-sys-util/src/cpu.rs @@ -54,7 +54,7 @@ pub fn get_cpu_flags(cpu_info: &str, cpu_flags_tag: &str) -> Result { } if cpu_flags_tag.is_empty() { - return Err(anyhow!("cpu flags delimiter string is empty"))?; + return Err(anyhow!("cpu flags delimiter string is empty")); } get_cpu_flags_from_file(cpu_info, cpu_flags_tag) @@ -71,7 +71,7 @@ pub fn get_cpu_flags_vec(cpu_info: &str, cpu_flags_tag: &str) -> Result Result Result { if flag.is_empty() { - return Err(anyhow!("parameter specifying flag to look for is empty"))?; + return Err(anyhow!("parameter specifying flag to look for is empty")); } Ok(flags_vec.iter().any(|f| f == flag)) diff --git a/src/libs/kata-sys-util/src/hooks.rs b/src/libs/kata-sys-util/src/hooks.rs index 8a36e606e1..8dea2e499f 100644 --- a/src/libs/kata-sys-util/src/hooks.rs +++ b/src/libs/kata-sys-util/src/hooks.rs @@ -8,9 +8,9 @@ use std::collections::HashMap; use std::ffi::OsString; use std::hash::{Hash, Hasher}; use std::io::{self, Read, Result}; -use std::path::Path; use std::time::Duration; +use oci_spec::runtime as oci; use subprocess::{ExitStatus, Popen, PopenConfig, PopenError, Redirection}; use crate::validate::valid_env; @@ -40,10 +40,10 @@ impl Eq for HookKey {} impl Hash for HookKey { fn hash(&self, state: &mut H) { - self.0.path.hash(state); - self.0.args.hash(state); - self.0.env.hash(state); - self.0.timeout.hash(state); + self.0.path().hash(state); + self.0.args().hash(state); + self.0.env().hash(state); + self.0.timeout().hash(state); } } @@ -114,7 +114,11 @@ impl HookStates { /// /// The OCI spec also defines the context to invoke hooks, caller needs to take the responsibility /// to setup execution context, such as namespace etc. - pub fn execute_hook(&mut self, hook: &oci::Hook, state: Option) -> Result<()> { + pub fn execute_hook( + &mut self, + hook: &oci::Hook, + state: Option, + ) -> Result<()> { if self.get(hook) != HookState::Pending { return Ok(()); } @@ -150,7 +154,7 @@ impl HookStates { executor.execute_with_input(&mut popen, state)?; } executor.execute_and_wait(&mut popen)?; - info!(sl!(), "hook {} finished", hook.path); + info!(sl!(), "hook {} finished", hook.path().display()); self.states.insert(hook.into(), HookState::Done); Ok(()) @@ -165,11 +169,15 @@ impl HookStates { /// /// The execution result will be recorded for each hook. Once a hook returns success, it will not /// be invoked anymore. - pub fn execute_hooks(&mut self, hooks: &[oci::Hook], state: Option) -> Result<()> { + pub fn execute_hooks( + &mut self, + hooks: &[oci::Hook], + state: Option, + ) -> Result<()> { for hook in hooks.iter() { if let Err(e) = self.execute_hook(hook, state.clone()) { // Ignore error and try next hook, the caller should retry. - error!(sl!(), "hook {} failed: {}", hook.path, e); + error!(sl!(), "hook {} failed: {}", hook.path().display(), e); } } @@ -188,10 +196,10 @@ struct HookExecutor<'a> { impl<'a> HookExecutor<'a> { fn new(hook: &'a oci::Hook) -> Result { // Ensure Hook.path is present and is an absolute path. - let executable = if hook.path.is_empty() { + let executable = if !hook.path().exists() { return Err(eother!("path of hook {:?} is empty", hook)); } else { - let path = Path::new(&hook.path); + let path = hook.path(); if !path.is_absolute() { return Err(eother!("path of hook {:?} is not absolute", hook)); } @@ -199,22 +207,23 @@ impl<'a> HookExecutor<'a> { }; // Hook.args is optional, use Hook.path as arg0 if Hook.args is empty. - let args = if hook.args.is_empty() { - vec![hook.path.clone()] - } else { - hook.args.clone() + let args = match hook.args() { + Some(args) => args.clone(), + None => vec![hook.path().display().to_string()], }; let mut envs: Vec<(OsString, OsString)> = Vec::new(); - for e in hook.env.iter() { - if let Some((key, value)) = valid_env(e) { - envs.push((OsString::from(key), OsString::from(value))); + if let Some(env) = hook.env() { + for e in env.iter() { + if let Some((key, value)) = valid_env(e) { + envs.push((OsString::from(key), OsString::from(value))); + } } } // Use Hook.timeout if it's valid, otherwise default to 10s. let mut timeout = DEFAULT_HOOK_TIMEOUT_SEC as u64; - if let Some(t) = hook.timeout { + if let Some(t) = hook.timeout() { if t > 0 { timeout = t as u64; } @@ -229,7 +238,7 @@ impl<'a> HookExecutor<'a> { }) } - fn execute_with_input(&mut self, popen: &mut Popen, state: oci::State) -> Result<()> { + fn execute_with_input(&mut self, popen: &mut Popen, state: runtime_spec::State) -> Result<()> { let st = serde_json::to_string(&state)?; let (stdout, stderr) = popen .communicate_start(Some(st.as_bytes().to_vec())) @@ -238,12 +247,22 @@ impl<'a> HookExecutor<'a> { .map_err(|e| e.error)?; if let Some(err) = stderr { if !err.is_empty() { - error!(sl!(), "hook {} exec failed: {}", self.hook.path, err); + error!( + sl!(), + "hook {} exec failed: {}", + self.hook.path().display(), + err + ); } } if let Some(out) = stdout { if !out.is_empty() { - info!(sl!(), "hook {} exec stdout: {}", self.hook.path, out); + info!( + sl!(), + "hook {} exec stdout: {}", + self.hook.path().display(), + out + ); } } // Give a grace period for `execute_and_wait()`. @@ -333,6 +352,7 @@ mod tests { use std::fs::{self, set_permissions, File, Permissions}; use std::io::Write; use std::os::unix::fs::PermissionsExt; + use std::path::PathBuf; use std::time::Instant; fn test_hook_eq(hook1: &oci::Hook, hook2: &oci::Hook, expected: bool) { @@ -341,17 +361,41 @@ mod tests { assert_eq!(key1 == key2, expected); } + + // struct Hook is just for test cases + #[derive(Clone)] + pub struct Hook { + pub path: String, + pub args: Vec, + pub env: Vec, + pub timeout: Option, + } + + impl Hook { + fn build_oci_hook(self) -> oci::Hook { + let mut hook = oci::Hook::default(); + hook.set_path(PathBuf::from(self.path)); + hook.set_args(Some(self.args)); + hook.set_env(Some(self.env)); + hook.set_timeout(self.timeout); + + hook + } + } + #[test] fn test_hook_key() { - let hook = oci::Hook { + let hook = Hook { path: "1".to_string(), args: vec!["2".to_string(), "3".to_string()], env: vec![], timeout: Some(0), }; + let oci_hook = hook.build_oci_hook(); + let cases = [ ( - oci::Hook { + Hook { path: "1000".to_string(), args: vec!["2".to_string(), "3".to_string()], env: vec![], @@ -360,7 +404,7 @@ mod tests { false, ), ( - oci::Hook { + Hook { path: "1".to_string(), args: vec!["2".to_string(), "4".to_string()], env: vec![], @@ -369,7 +413,7 @@ mod tests { false, ), ( - oci::Hook { + Hook { path: "1".to_string(), args: vec!["2".to_string()], env: vec![], @@ -378,7 +422,7 @@ mod tests { false, ), ( - oci::Hook { + Hook { path: "1".to_string(), args: vec!["2".to_string(), "3".to_string()], env: vec!["5".to_string()], @@ -387,7 +431,7 @@ mod tests { false, ), ( - oci::Hook { + Hook { path: "1".to_string(), args: vec!["2".to_string(), "3".to_string()], env: vec![], @@ -396,7 +440,7 @@ mod tests { false, ), ( - oci::Hook { + Hook { path: "1".to_string(), args: vec!["2".to_string(), "3".to_string()], env: vec![], @@ -405,7 +449,7 @@ mod tests { false, ), ( - oci::Hook { + Hook { path: "1".to_string(), args: vec!["2".to_string(), "3".to_string()], env: vec![], @@ -416,7 +460,8 @@ mod tests { ]; for case in cases.iter() { - test_hook_eq(&hook, &case.0, case.1); + let case_hook = case.0.clone().build_oci_hook(); + test_hook_eq(&oci_hook, &case_hook, case.1); } } @@ -435,50 +480,53 @@ mod tests { // test case 1: normal // execute hook - let hook = oci::Hook { + let hook = Hook { path: "/bin/touch".to_string(), args: vec!["touch".to_string(), file_str.to_string()], env: vec![], timeout: Some(0), }; - let ret = states.execute_hook(&hook, None); + let oci_hook = hook.build_oci_hook(); + let ret = states.execute_hook(&oci_hook, None); assert!(ret.is_ok()); assert!(fs::metadata(&file).is_ok()); assert!(!states.should_retry()); // test case 2: timeout in 10s - let hook = oci::Hook { + let hook = Hook { path: "/bin/sleep".to_string(), args: vec!["sleep".to_string(), "3600".to_string()], env: vec![], timeout: Some(0), // default timeout is 10 seconds }; + let oci_hook = hook.build_oci_hook(); let start = Instant::now(); - let ret = states.execute_hook(&hook, None).unwrap_err(); + let ret = states.execute_hook(&oci_hook, None).unwrap_err(); let duration = start.elapsed(); let used = duration.as_secs(); assert!((10..12u64).contains(&used)); assert_eq!(ret.kind(), io::ErrorKind::TimedOut); - assert_eq!(states.get(&hook), HookState::Pending); + assert_eq!(states.get(&oci_hook), HookState::Pending); assert!(states.should_retry()); - states.remove(&hook); + states.remove(&oci_hook); // test case 3: timeout in 5s - let hook = oci::Hook { + let hook = Hook { path: "/bin/sleep".to_string(), args: vec!["sleep".to_string(), "3600".to_string()], env: vec![], timeout: Some(5), // timeout is set to 5 seconds }; + let oci_hook = hook.build_oci_hook(); let start = Instant::now(); - let ret = states.execute_hook(&hook, None).unwrap_err(); + let ret = states.execute_hook(&oci_hook, None).unwrap_err(); let duration = start.elapsed(); let used = duration.as_secs(); assert!((5..7u64).contains(&used)); assert_eq!(ret.kind(), io::ErrorKind::TimedOut); - assert_eq!(states.get(&hook), HookState::Pending); + assert_eq!(states.get(&oci_hook), HookState::Pending); assert!(states.should_retry()); - states.remove(&hook); + states.remove(&oci_hook); // test case 4: with envs let create_shell = |shell_path: &str, data_path: &str| -> Result<()> { @@ -500,13 +548,14 @@ echo -n "K1=${{K1}}" > {} let shell_path = format!("{}/test.sh", tmpdir.path().to_string_lossy()); let ret = create_shell(&shell_path, file_str.as_ref()); assert!(ret.is_ok()); - let hook = oci::Hook { + let hook = Hook { path: shell_path, args: vec![], env: vec!["K1=V1".to_string()], timeout: Some(5), }; - let ret = states.execute_hook(&hook, None); + let oci_hook = hook.build_oci_hook(); + let ret = states.execute_hook(&oci_hook, None); assert!(ret.is_ok()); assert!(!states.should_retry()); let contents = fs::read_to_string(file); @@ -516,22 +565,23 @@ echo -n "K1=${{K1}}" > {} } // test case 5: timeout in 5s with state - let hook = oci::Hook { + let hook = Hook { path: "/bin/sleep".to_string(), args: vec!["sleep".to_string(), "3600".to_string()], env: vec![], timeout: Some(6), // timeout is set to 5 seconds }; - let state = oci::State { + let oci_hook = hook.build_oci_hook(); + let state = runtime_spec::State { version: "".to_string(), id: "".to_string(), - status: oci::ContainerState::Creating, + status: runtime_spec::ContainerState::Creating, pid: 10, bundle: "nouse".to_string(), annotations: Default::default(), }; let start = Instant::now(); - let ret = states.execute_hook(&hook, Some(state)).unwrap_err(); + let ret = states.execute_hook(&oci_hook, Some(state)).unwrap_err(); let duration = start.elapsed(); let used = duration.as_secs(); assert!((6..8u64).contains(&used)); diff --git a/src/libs/kata-sys-util/src/k8s.rs b/src/libs/kata-sys-util/src/k8s.rs index 4ae31921e7..46450accef 100644 --- a/src/libs/kata-sys-util/src/k8s.rs +++ b/src/libs/kata-sys-util/src/k8s.rs @@ -10,7 +10,7 @@ //! to detect K8S EmptyDir medium type from `oci::spec::Mount` objects. use kata_types::mount; -use oci::Spec; +use oci_spec::runtime::Spec; use crate::mount::get_linux_mount_info; @@ -55,17 +55,24 @@ pub fn is_host_empty_dir(path: &str) -> bool { // backed by tmpfs inside the VM. For successive containers // of the same pod the already existing volume is reused. pub fn update_ephemeral_storage_type(oci_spec: &mut Spec) { - for m in oci_spec.mounts.iter_mut() { - if mount::is_kata_guest_mount_volume(&m.r#type) { - continue; - } + if let Some(mounts) = oci_spec.mounts_mut() { + for m in mounts.iter_mut() { + if let Some(typ) = &m.typ() { + if mount::is_kata_guest_mount_volume(typ) { + continue; + } + } - if is_ephemeral_volume(&m.source) { - m.r#type = String::from(mount::KATA_EPHEMERAL_VOLUME_TYPE); - } else if is_host_empty_dir(&m.source) { - // FIXME support disable_guest_empty_dir - // https://github.com/kata-containers/kata-containers/blob/02a51e75a7e0c6fce5e8abe3b991eeac87e09645/src/runtime/pkg/katautils/create.go#L105 - m.r#type = String::from(mount::KATA_HOST_DIR_VOLUME_TYPE); + if let Some(source) = &m.source() { + let mnt_src = &source.display().to_string(); + if is_ephemeral_volume(mnt_src) { + m.set_typ(Some(String::from(mount::KATA_EPHEMERAL_VOLUME_TYPE))); + } else if is_host_empty_dir(mnt_src) { + // FIXME support disable_guest_empty_dir + // https://github.com/kata-containers/kata-containers/blob/02a51e75a7e0c6fce5e8abe3b991eeac87e09645/src/runtime/pkg/katautils/create.go#L105 + m.set_typ(Some(String::from(mount::KATA_HOST_DIR_VOLUME_TYPE))); + } + } } } } diff --git a/src/libs/kata-sys-util/src/mount.rs b/src/libs/kata-sys-util/src/mount.rs index 873db5f5b9..ae9cee0a3a 100644 --- a/src/libs/kata-sys-util/src/mount.rs +++ b/src/libs/kata-sys-util/src/mount.rs @@ -134,9 +134,10 @@ pub struct LinuxMountInfo { /// Get the device and file system type of a mount point by parsing `/proc/mounts`. pub fn get_linux_mount_info(mount_point: &str) -> Result { let mount_file = fs::File::open(PROC_MOUNTS_FILE)?; - let lines = io::BufReader::new(mount_file).lines(); + let reader = io::BufReader::new(mount_file); - for mount in lines.flatten() { + for line in reader.lines() { + let mount = line?; let fields: Vec<&str> = mount.split(' ').collect(); if fields.len() != PROC_FIELDS_PER_LINE { @@ -615,7 +616,6 @@ fn compact_lowerdir_option(opts: &[String]) -> (Option, Vec) { } }; - let idx = idx; let common_dir = match get_longest_common_prefix(&lower_opts) { None => return (None, n_opts), Some(v) => { @@ -788,6 +788,21 @@ fn umount2>(path: P, lazy_umount: bool) -> std::io::Result<()> { nix::mount::umount2(path.as_ref(), flags).map_err(io::Error::from) } +pub fn get_mount_path(p: &Option) -> String { + p.clone().unwrap_or_default().display().to_string() +} + +pub fn get_mount_options(options: &Option>) -> Vec { + match options { + Some(o) => o.to_vec(), + None => vec![], + } +} + +pub fn get_mount_type(typ: &Option) -> String { + typ.clone().unwrap_or("bind".to_string()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/libs/kata-sys-util/src/numa.rs b/src/libs/kata-sys-util/src/numa.rs index 4a6b2e5767..d4ada3c7e0 100644 --- a/src/libs/kata-sys-util/src/numa.rs +++ b/src/libs/kata-sys-util/src/numa.rs @@ -84,7 +84,7 @@ pub fn get_node_map(cpus: &str) -> Result>> { for c in cpuset.iter() { let node_id = get_node_id(*c)?; - node_map.entry(node_id).or_insert_with(Vec::new).push(*c); + node_map.entry(node_id).or_default().push(*c); } Ok(node_map) diff --git a/src/libs/kata-sys-util/src/protection.rs b/src/libs/kata-sys-util/src/protection.rs index 63f6182806..74ccd5006e 100644 --- a/src/libs/kata-sys-util/src/protection.rs +++ b/src/libs/kata-sys-util/src/protection.rs @@ -7,12 +7,12 @@ use anyhow::anyhow; #[cfg(any(target_arch = "s390x", target_arch = "x86_64", target_arch = "aarch64"))] use anyhow::Result; +use serde::{Deserialize, Serialize}; use std::fmt; #[cfg(target_arch = "x86_64")] use std::path::Path; use std::path::PathBuf; use thiserror::Error; -use serde::{Deserialize, Serialize}; #[cfg(any(target_arch = "s390x", target_arch = "powerpc64le"))] use nix::unistd::Uid; diff --git a/src/libs/kata-sys-util/src/spec.rs b/src/libs/kata-sys-util/src/spec.rs index 24bcf16e91..c7a7ba405e 100644 --- a/src/libs/kata-sys-util/src/spec.rs +++ b/src/libs/kata-sys-util/src/spec.rs @@ -4,9 +4,9 @@ // SPDX-License-Identifier: Apache-2.0 // -use std::path::PathBuf; - use kata_types::container::ContainerType; +use oci_spec::{runtime as oci, OciSpecError}; +use std::path::PathBuf; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -18,7 +18,7 @@ pub enum Error { MissingSandboxID, /// oci error #[error("oci error")] - Oci(#[from] oci::Error), + Oci(#[from] OciSpecError), } const CRI_CONTAINER_TYPE_KEY_LIST: &[&str] = &[ @@ -50,13 +50,15 @@ pub enum ShimIdInfo { /// get container type pub fn get_container_type(spec: &oci::Spec) -> Result { - for k in CRI_CONTAINER_TYPE_KEY_LIST.iter() { - if let Some(type_value) = spec.annotations.get(*k) { - match type_value.as_str() { - "sandbox" => return Ok(ContainerType::PodSandbox), - "podsandbox" => return Ok(ContainerType::PodSandbox), - "container" => return Ok(ContainerType::PodContainer), - _ => return Err(Error::UnknownContainerType(type_value.clone())), + if let Some(annotations) = spec.annotations() { + for k in CRI_CONTAINER_TYPE_KEY_LIST.iter() { + if let Some(type_value) = annotations.get(*k) { + match type_value.as_str() { + "sandbox" => return Ok(ContainerType::PodSandbox), + "podsandbox" => return Ok(ContainerType::PodSandbox), + "container" => return Ok(ContainerType::PodContainer), + _ => return Err(Error::UnknownContainerType(type_value.clone())), + } } } } @@ -70,11 +72,14 @@ pub fn get_shim_id_info() -> Result { match get_container_type(&spec)? { ContainerType::PodSandbox | ContainerType::SingleContainer => Ok(ShimIdInfo::Sandbox), ContainerType::PodContainer => { - for k in CRI_SANDBOX_ID_KEY_LIST { - if let Some(sandbox_id) = spec.annotations.get(*k) { - return Ok(ShimIdInfo::Container(sandbox_id.into())); + if let Some(annotations) = spec.annotations() { + for k in CRI_SANDBOX_ID_KEY_LIST { + if let Some(sandbox_id) = annotations.get(*k) { + return Ok(ShimIdInfo::Container(sandbox_id.into())); + } } } + Err(Error::MissingSandboxID) } } @@ -86,9 +91,17 @@ pub fn get_bundle_path() -> std::io::Result { } /// load oci spec -pub fn load_oci_spec() -> oci::Result { +pub fn load_oci_spec() -> Result { let bundle_path = get_bundle_path()?; let spec_file = bundle_path.join("config.json"); oci::Spec::load(spec_file.to_str().unwrap_or_default()) } + +/// handle string parsing for input possibly be JSON string. +pub fn parse_json_string(input: &str) -> &str { + let json_str: &str = serde_json::from_str(input).unwrap_or(input); + let stripped_str = json_str.strip_prefix("CAP_").unwrap_or(json_str); + + stripped_str +}