From 16c37b650751dd60cc615d2b39d909030ba45d7c Mon Sep 17 00:00:00 2001 From: Roy Lin Date: Thu, 11 Jun 2026 16:16:30 +0800 Subject: [PATCH 1/2] chore(ci): fix pre-existing fmt + clippy on main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main's CI was red from pre-existing issues unrelated to any open feature PR (even the docs-only PR failed them): cargo fmt drift in 6 files, plus two clippy -D warnings — manual char comparison in core/log.rs (use a char-array pattern) and an items-after-test-module in cri stats.rs (move the test module to the end). Greens `cargo fmt --all -- --check` and `cargo clippy --workspace --all-targets -- -D warnings` so the feature PRs can merge clean. No behavior change. --- src/cli/src/state/file.rs | 6 +- src/core/src/log.rs | 74 +++++++++++++++----- src/cri/src/runtime_service/service_ops.rs | 11 ++- src/cri/src/runtime_service/stats.rs | 66 ++++++++--------- src/runtime/src/oci/build/engine/handlers.rs | 3 +- src/runtime/src/oci/layers.rs | 10 ++- src/shim/src/main.rs | 10 ++- 7 files changed, 119 insertions(+), 61 deletions(-) diff --git a/src/cli/src/state/file.rs b/src/cli/src/state/file.rs index 038e826..7fad23b 100644 --- a/src/cli/src/state/file.rs +++ b/src/cli/src/state/file.rs @@ -189,9 +189,9 @@ impl StateFile { // start_enter takeover means we can't waitpid the VM, so liveness // polling alone would otherwise always yield exit 0. if record.exit_code.is_none() { - if let Ok(contents) = std::fs::read_to_string( - record.box_dir.join("upper").join(".a3s_exit_code"), - ) { + if let Ok(contents) = + std::fs::read_to_string(record.box_dir.join("upper").join(".a3s_exit_code")) + { if let Ok(code) = contents.trim().parse::() { record.exit_code = Some(code); } diff --git a/src/core/src/log.rs b/src/core/src/log.rs index db88b85..5ca1fdc 100644 --- a/src/core/src/log.rs +++ b/src/core/src/log.rs @@ -179,7 +179,11 @@ pub fn is_runtime_console_noise(line: &str) -> bool { /// reads. Returns `None` only when `stop` is set AND EOF is reached — i.e. the /// VM has exited and `console.log` is fully drained — flushing any final partial /// line as the last value before the subsequent `None`. -fn tail_next_line(reader: &mut impl BufRead, buf: &mut String, stop: &AtomicBool) -> Option { +fn tail_next_line( + reader: &mut impl BufRead, + buf: &mut String, + stop: &AtomicBool, +) -> Option { loop { match reader.read_line(buf) { Ok(0) | Err(_) => { @@ -190,7 +194,7 @@ fn tail_next_line(reader: &mut impl BufRead, buf: &mut String, stop: &AtomicBool return None; } let line = std::mem::take(buf); - return Some(line.trim_end_matches(|c| c == '\n' || c == '\r').to_string()); + return Some(line.trim_end_matches(['\n', '\r']).to_string()); } std::thread::sleep(std::time::Duration::from_millis(100)); continue; @@ -202,7 +206,7 @@ fn tail_next_line(reader: &mut impl BufRead, buf: &mut String, stop: &AtomicBool continue; } let line = std::mem::take(buf); - return Some(line.trim_end_matches(|c| c == '\n' || c == '\r').to_string()); + return Some(line.trim_end_matches(['\n', '\r']).to_string()); } } @@ -210,12 +214,21 @@ fn tail_next_line(reader: &mut impl BufRead, buf: &mut String, stop: &AtomicBool /// is drained. Intended to run on a dedicated thread for the VM's lifetime; set /// `stop` after the VM exits, then join, to guarantee the final lines are /// captured (no teardown race). -pub fn run_log_processor(console_log: &Path, log_dir: &Path, config: &LogConfig, stop: &AtomicBool) { +pub fn run_log_processor( + console_log: &Path, + log_dir: &Path, + config: &LogConfig, + stop: &AtomicBool, +) { match config.driver { LogDriver::None => {} - LogDriver::JsonFile => { - run_json_file_processor(console_log, log_dir, config.max_size(), config.max_file(), stop) - } + LogDriver::JsonFile => run_json_file_processor( + console_log, + log_dir, + config.max_size(), + config.max_file(), + stop, + ), LogDriver::Syslog => run_syslog_processor( console_log, config.syslog_address(), @@ -383,9 +396,18 @@ struct RotatingWriter { impl RotatingWriter { fn new(path: &Path, max_size: u64, max_file: u32) -> std::io::Result { - let file = std::fs::OpenOptions::new().create(true).append(true).open(path)?; + let file = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(path)?; let written = file.metadata()?.len(); - Ok(Self { path: path.to_path_buf(), file, written, max_size, max_file }) + Ok(Self { + path: path.to_path_buf(), + file, + written, + max_size, + max_file, + }) } fn write_line(&mut self, line: &str) -> std::io::Result<()> { @@ -414,7 +436,10 @@ impl RotatingWriter { let rotated = rotated_path(&self.path, 1); compress_file(&self.path, &rotated)?; std::fs::remove_file(&self.path)?; - self.file = std::fs::OpenOptions::new().create(true).append(true).open(&self.path)?; + self.file = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&self.path)?; self.written = 0; Ok(()) } @@ -558,8 +583,14 @@ mod tests { let mut reader = BufReader::new(Cursor::new(b"alpha\r\nbeta\n".to_vec())); let mut buf = String::new(); let stop = AtomicBool::new(true); - assert_eq!(tail_next_line(&mut reader, &mut buf, &stop), Some("alpha".to_string())); - assert_eq!(tail_next_line(&mut reader, &mut buf, &stop), Some("beta".to_string())); + assert_eq!( + tail_next_line(&mut reader, &mut buf, &stop), + Some("alpha".to_string()) + ); + assert_eq!( + tail_next_line(&mut reader, &mut buf, &stop), + Some("beta".to_string()) + ); assert_eq!(tail_next_line(&mut reader, &mut buf, &stop), None); assert!(buf.is_empty()); } @@ -583,7 +614,9 @@ mod tests { fn test_is_runtime_console_noise() { assert!(is_runtime_console_noise("init.krun: mount_filesystems ok")); assert!(!is_runtime_console_noise("L1")); - assert!(!is_runtime_console_noise("starting app (init.krun: ignored)")); + assert!(!is_runtime_console_noise( + "starting app (init.krun: ignored)" + )); assert!(!is_runtime_console_noise("")); } @@ -599,8 +632,14 @@ mod tests { run_json_file_processor(&console, dir.path(), 10 * 1024 * 1024, 3, &stop); let json = std::fs::read_to_string(json_log_path(dir.path())).unwrap(); assert!(json.contains("\"log\":\"AAA\\n\""), "AAA missing: {json}"); - assert!(json.contains("\"log\":\"BBB\\n\""), "BBB (after a quiet line) missing: {json}"); - assert!(!json.contains("init.krun"), "runtime noise must be filtered: {json}"); + assert!( + json.contains("\"log\":\"BBB\\n\""), + "BBB (after a quiet line) missing: {json}" + ); + assert!( + !json.contains("init.krun"), + "runtime noise must be filtered: {json}" + ); } #[test] @@ -611,6 +650,9 @@ mod tests { for i in 0..10 { w.write_line(&format!("line-{i}")).unwrap(); } - assert!(rotated_path(&path, 1).exists(), "expected a rotated .1.gz file"); + assert!( + rotated_path(&path, 1).exists(), + "expected a rotated .1.gz file" + ); } } diff --git a/src/cri/src/runtime_service/service_ops.rs b/src/cri/src/runtime_service/service_ops.rs index 0bdb5be..c45a8d7 100644 --- a/src/cri/src/runtime_service/service_ops.rs +++ b/src/cri/src/runtime_service/service_ops.rs @@ -487,8 +487,10 @@ mod cleanup_tests { 61 30 0:47 / /root/.a3s/images/cri-container-rootfs/sb/ctr/rootfs/data/deep rw - ext4 /dev/sda1 rw 62 30 0:48 / /root/.a3s/images/cri-container-rootfs/other/x rw - ext4 /dev/sda1 rw "; - let got = - submounts_under(mountinfo, "/root/.a3s/images/cri-container-rootfs/sb/ctr/rootfs"); + let got = submounts_under( + mountinfo, + "/root/.a3s/images/cri-container-rootfs/sb/ctr/rootfs", + ); assert_eq!( got, vec![ @@ -505,6 +507,9 @@ mod cleanup_tests { 1 2 0:1 / /root/xy rw - ext4 d rw "; // Exact root match included; a sibling sharing the string prefix is not. - assert_eq!(submounts_under(mountinfo, "/root/x"), vec!["/root/x".to_string()]); + assert_eq!( + submounts_under(mountinfo, "/root/x"), + vec!["/root/x".to_string()] + ); } } diff --git a/src/cri/src/runtime_service/stats.rs b/src/cri/src/runtime_service/stats.rs index cc3aa91..7be1bf9 100644 --- a/src/cri/src/runtime_service/stats.rs +++ b/src/cri/src/runtime_service/stats.rs @@ -321,39 +321,6 @@ pub(super) fn metric_descriptors() -> Vec { ] } -#[cfg(test)] -mod stats_tests { - use super::*; - - #[test] - fn test_per_container_split_sums_to_total() { - let total = VmUsage { - cpu_core_nanos: 900, - memory_bytes: 300, - }; - let per = total.per_container(3); - assert_eq!(per.cpu_core_nanos, 300); - assert_eq!(per.memory_bytes, 100); - // 0 or 1 container → the full usage (no divide-by-zero, single-container - // pods get the whole VM's usage). - assert_eq!(total.per_container(0).memory_bytes, 300); - assert_eq!(total.per_container(1).cpu_core_nanos, 900); - } - - #[cfg(target_os = "linux")] - #[test] - fn test_read_vm_usage_reports_real_memory_for_self() { - // Reading the test process's own procfs must yield a non-zero RSS (CPU - // may legitimately round to 0 immediately after start, so only memory - // is asserted). - let usage = read_vm_usage(std::process::id()); - assert!( - usage.memory_bytes > 0, - "expected a non-zero RSS reading the test process's own /proc" - ); - } -} - fn pod_sandbox_metric_labels(sandbox: &PodSandbox) -> HashMap { HashMap::from([ ("pod_sandbox_id".to_string(), sandbox.id.clone()), @@ -432,3 +399,36 @@ pub(super) fn pod_sandbox_metrics( ], } } + +#[cfg(test)] +mod stats_tests { + use super::*; + + #[test] + fn test_per_container_split_sums_to_total() { + let total = VmUsage { + cpu_core_nanos: 900, + memory_bytes: 300, + }; + let per = total.per_container(3); + assert_eq!(per.cpu_core_nanos, 300); + assert_eq!(per.memory_bytes, 100); + // 0 or 1 container → the full usage (no divide-by-zero, single-container + // pods get the whole VM's usage). + assert_eq!(total.per_container(0).memory_bytes, 300); + assert_eq!(total.per_container(1).cpu_core_nanos, 900); + } + + #[cfg(target_os = "linux")] + #[test] + fn test_read_vm_usage_reports_real_memory_for_self() { + // Reading the test process's own procfs must yield a non-zero RSS (CPU + // may legitimately round to 0 immediately after start, so only memory + // is asserted). + let usage = read_vm_usage(std::process::id()); + assert!( + usage.memory_bytes > 0, + "expected a non-zero RSS reading the test process's own /proc" + ); + } +} diff --git a/src/runtime/src/oci/build/engine/handlers.rs b/src/runtime/src/oci/build/engine/handlers.rs index 8e1d243..b6bd8f2 100644 --- a/src/runtime/src/oci/build/engine/handlers.rs +++ b/src/runtime/src/oci/build/engine/handlers.rs @@ -331,8 +331,7 @@ pub(super) fn handle_run( } let layer_path = layers_dir.join(format!("layer_{}.tar.gz", layer_index)); - let layer_info = - create_layer_with_deletions(rootfs_dir, &changed, &deleted, &layer_path)?; + let layer_info = create_layer_with_deletions(rootfs_dir, &changed, &deleted, &layer_path)?; Ok(Some(layer_info)) } diff --git a/src/runtime/src/oci/layers.rs b/src/runtime/src/oci/layers.rs index 189ffc4..1be8230 100644 --- a/src/runtime/src/oci/layers.rs +++ b/src/runtime/src/oci/layers.rs @@ -62,7 +62,10 @@ pub fn extract_layer(layer_path: &Path, target_dir: &Path) -> Result<()> { )) })?; file.seek(SeekFrom::Start(0)).map_err(|e| { - BoxError::OciImageError(format!("Failed to rewind layer {}: {e}", layer_path.display())) + BoxError::OciImageError(format!( + "Failed to rewind layer {}: {e}", + layer_path.display() + )) })?; let decoder: Box = if read >= 2 && magic[0] == 0x1f && magic[1] == 0x8b { @@ -410,6 +413,9 @@ mod tests { write_test_tar(File::create(&layer_path).unwrap(), &[("p.txt", b"plain")]); extract_layer(&layer_path, &target_dir).unwrap(); - assert_eq!(fs::read_to_string(target_dir.join("p.txt")).unwrap(), "plain"); + assert_eq!( + fs::read_to_string(target_dir.join("p.txt")).unwrap(), + "plain" + ); } } diff --git a/src/shim/src/main.rs b/src/shim/src/main.rs index 7c4ab37..3f2f087 100644 --- a/src/shim/src/main.rs +++ b/src/shim/src/main.rs @@ -769,7 +769,10 @@ unsafe fn configure_and_start_vm(spec: &InstanceSpec) -> Result<()> { use std::os::unix::io::AsRawFd; let err_path = console_path.with_file_name("console.err.log"); let open = |p: &std::path::Path| { - std::fs::OpenOptions::new().create(true).append(true).open(p) + std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(p) }; if let (Ok(out_f), Ok(err_f)) = (open(console_path), open(&err_path)) { ctx.add_split_console(-1, out_f.as_raw_fd(), err_f.as_raw_fd())?; @@ -781,7 +784,10 @@ unsafe fn configure_and_start_vm(spec: &InstanceSpec) -> Result<()> { } } if !split_done { - tracing::debug!(console_path = console_str, "Redirecting console output (merged)"); + tracing::debug!( + console_path = console_str, + "Redirecting console output (merged)" + ); ctx.set_console_output(console_str)?; } } From c65a6a62b24e8bc4a6c35443df232fd2d85c73e2 Mon Sep 17 00:00:00 2001 From: Roy Lin Date: Thu, 11 Jun 2026 16:22:19 +0800 Subject: [PATCH 2/2] chore(ci): fix Linux-only guest-init clippy (cross-platform gap) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit macOS clippy skipped the Linux-target guest-init code; CI (x86_64-linux) caught three more -D warnings: items-after-test-module in cgroup.rs (move test mod to end) and two manual_contains in main.rs (iter().any() → contains()). --- src/guest/init/src/cgroup.rs | 32 ++++++++++++++++---------------- src/guest/init/src/main.rs | 4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/guest/init/src/cgroup.rs b/src/guest/init/src/cgroup.rs index 2873404..37f5661 100644 --- a/src/guest/init/src/cgroup.rs +++ b/src/guest/init/src/cgroup.rs @@ -86,22 +86,6 @@ fn shares_to_weight(shares: u64) -> u64 { (1 + ((shares - 2) * 9999) / 262_142).clamp(1, 10_000) } -#[cfg(test)] -mod tests { - use super::shares_to_weight; - - #[test] - fn test_shares_to_weight_mapping() { - // Endpoints + the cgroup v1 default map to the runc-equivalent weights. - assert_eq!(shares_to_weight(2), 1); - assert_eq!(shares_to_weight(262_144), 10_000); - assert_eq!(shares_to_weight(1024), 39); // runc's mapping for the default - // Out-of-range inputs are clamped, never panic / overflow. - assert_eq!(shares_to_weight(0), 1); - assert_eq!(shares_to_weight(u64::MAX), 10_000); - } -} - /// A per-container cgroup v2 (memory + cpu limits). Dropping it removes the /// cgroup directory. pub struct ContainerCgroup { @@ -202,3 +186,19 @@ impl Drop for ContainerCgroup { } } } + +#[cfg(test)] +mod tests { + use super::shares_to_weight; + + #[test] + fn test_shares_to_weight_mapping() { + // Endpoints + the cgroup v1 default map to the runc-equivalent weights. + assert_eq!(shares_to_weight(2), 1); + assert_eq!(shares_to_weight(262_144), 10_000); + assert_eq!(shares_to_weight(1024), 39); // runc's mapping for the default + // Out-of-range inputs are clamped, never panic / overflow. + assert_eq!(shares_to_weight(0), 1); + assert_eq!(shares_to_weight(u64::MAX), 10_000); + } +} diff --git a/src/guest/init/src/main.rs b/src/guest/init/src/main.rs index 4a60564..b765163 100644 --- a/src/guest/init/src/main.rs +++ b/src/guest/init/src/main.rs @@ -764,8 +764,8 @@ fn mount_user_volumes() -> Result<(), Box> { let guest_path = parts[1]; // Flags after the guest path may appear in any order: "ro", "file". // The host decides "file" (it can stat the source); the guest obeys. - let read_only = parts[2..].iter().any(|&m| m == "ro"); - let is_file = parts[2..].iter().any(|&m| m == "file"); + let read_only = parts[2..].contains(&"ro"); + let is_file = parts[2..].contains(&"file"); let flags = if read_only { MsFlags::MS_RDONLY