mirror of
https://github.com/vhaudiquet/ecrou.git
synced 2026-11-12 22:22:44 +00:00
fix(rootfs): make cache writes safe against concurrent runs
Downloads, OCI bundle packing and rootfs packing all wrote through one fixed partial name, so two processes preparing the same entry interleaved bytes and renamed the wreckage into place for good. Each writer now gets a unique partial file (utils::partial_path) and the last atomic rename wins with whole bytes either way; the layer pointer is written the same way.
This commit is contained in:
1 parent
6835056be9
commit
56af198d54
3 files changed
+44
-9
No files matched your search
@@ -302,12 +302,15 @@ async fn download_file_async(
|
|||||||
(total_size > 0).then_some(total_size),
|
(total_size > 0).then_some(total_size),
|
||||||
);
|
);
|
||||||
|
|
||||||
let temp_dest = dest.with_extension("partial");
|
// Unique per call: concurrent runs writing the same entry each
|
||||||
|
// complete their own file, and the last rename wins with whole
|
||||||
|
// bytes either way.
|
||||||
|
let temp_dest = crate::utils::partial_path(dest);
|
||||||
|
|
||||||
// Download into the temp file, then atomically rename to the final path.
|
// Download into the temp file, then atomically rename to the final path.
|
||||||
// Verification runs before the rename so a corrupted download never
|
// Verification runs before the rename so a corrupted download never
|
||||||
// reaches the cache; on any failure after the temp file is created we
|
// reaches the cache; on any failure after the temp file is created we
|
||||||
// remove it so stale .partial files don't accumulate.
|
// remove it so stale partial files don't accumulate.
|
||||||
let mut hasher = DownloadHasher::new();
|
let mut hasher = DownloadHasher::new();
|
||||||
let result: anyhow::Result<()> = async {
|
let result: anyhow::Result<()> = async {
|
||||||
let mut file = tokio::fs::File::create(&temp_dest)
|
let mut file = tokio::fs::File::create(&temp_dest)
|
||||||
@@ -579,7 +582,7 @@ async fn try_download_oci_image(
|
|||||||
|
|
||||||
// Bundle layers.manifest + all layer blobs into a single .tar.gz cache file
|
// Bundle layers.manifest + all layer blobs into a single .tar.gz cache file
|
||||||
// using the tar + flate2 crates — no external `tar` binary required.
|
// using the tar + flate2 crates — no external `tar` binary required.
|
||||||
let temp_dest = dest.with_extension("tar.partial");
|
let temp_dest = crate::utils::partial_path(dest);
|
||||||
|
|
||||||
let bundle_result: anyhow::Result<()> = (|| {
|
let bundle_result: anyhow::Result<()> = (|| {
|
||||||
use flate2::{write::GzEncoder, Compression};
|
use flate2::{write::GzEncoder, Compression};
|
||||||
|
|||||||
@@ -136,8 +136,14 @@ impl RootfsCache {
|
|||||||
let dir = self.dir.join("layers");
|
let dir = self.dir.join("layers");
|
||||||
std::fs::create_dir_all(&dir)
|
std::fs::create_dir_all(&dir)
|
||||||
.with_context(|| format!("Failed to create the layer directory {}", dir.display()))?;
|
.with_context(|| format!("Failed to create the layer directory {}", dir.display()))?;
|
||||||
std::fs::write(dir.join(name), cache_filename)
|
// Atomic write: a concurrent run reading the pointer must
|
||||||
|
// never see a truncated filename mid-write.
|
||||||
|
let pointer = dir.join(name);
|
||||||
|
let partial = crate::utils::partial_path(&pointer);
|
||||||
|
std::fs::write(&partial, cache_filename)
|
||||||
.with_context(|| format!("Failed to write the layer pointer for {name}"))?;
|
.with_context(|| format!("Failed to write the layer pointer for {name}"))?;
|
||||||
|
std::fs::rename(&partial, &pointer)
|
||||||
|
.with_context(|| format!("Failed to move the layer pointer into place for {name}"))?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -522,7 +528,7 @@ fn drop_loopback_nameservers(config: &str) -> String {
|
|||||||
|
|
||||||
/// Write a tarball of `rootfs` at `dest`, compressed according to dest's
|
/// Write a tarball of `rootfs` at `dest`, compressed according to dest's
|
||||||
/// extension (.tar.gz, .tar.xz, .tar.zst, plain .tar), atomically via a
|
/// extension (.tar.gz, .tar.xz, .tar.zst, plain .tar), atomically via a
|
||||||
/// `.partial` file.
|
/// unique partial file ([`crate::utils::partial_path`]).
|
||||||
fn pack_rootfs(rootfs: &Path, dest: &Path) -> anyhow::Result<()> {
|
fn pack_rootfs(rootfs: &Path, dest: &Path) -> anyhow::Result<()> {
|
||||||
// Packing the tree into a tarball placed inside it would recurse
|
// Packing the tree into a tarball placed inside it would recurse
|
||||||
// forever: the walker keeps seeing the archive grow.
|
// forever: the walker keeps seeing the archive grow.
|
||||||
@@ -541,7 +547,7 @@ fn pack_rootfs(rootfs: &Path, dest: &Path) -> anyhow::Result<()> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let filename = dest.file_name().and_then(|n| n.to_str()).unwrap_or("");
|
let filename = dest.file_name().and_then(|n| n.to_str()).unwrap_or("");
|
||||||
let partial = dest.with_extension("partial");
|
let partial = crate::utils::partial_path(dest);
|
||||||
|
|
||||||
let file = std::fs::File::create(&partial)
|
let file = std::fs::File::create(&partial)
|
||||||
.with_context(|| format!("Failed to create {}", partial.display()))?;
|
.with_context(|| format!("Failed to create {}", partial.display()))?;
|
||||||
@@ -848,12 +854,21 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn pack_failure_leaves_no_partial_file() {
|
fn pack_failure_leaves_no_partial_file() {
|
||||||
// A nonexistent rootfs must fail cleanly and clean up the .partial
|
// A nonexistent rootfs must fail cleanly and clean up its
|
||||||
|
// partial file, whatever unique name it carried.
|
||||||
let cache = tempfile::tempdir().unwrap();
|
let cache = tempfile::tempdir().unwrap();
|
||||||
let dest = cache.path().join("x.tar.gz");
|
let dest = cache.path().join("x.tar.gz");
|
||||||
assert!(pack_rootfs(Path::new("/nonexistent-rootfs"), &dest).is_err());
|
assert!(pack_rootfs(Path::new("/nonexistent-rootfs"), &dest).is_err());
|
||||||
assert!(!dest.exists());
|
assert!(!dest.exists());
|
||||||
assert!(!dest.with_extension("partial").exists());
|
let leftovers: Vec<String> = std::fs::read_dir(cache.path())
|
||||||
|
.unwrap()
|
||||||
|
.map(|e| e.unwrap().file_name().to_string_lossy().into_owned())
|
||||||
|
.filter(|name| name.contains("partial"))
|
||||||
|
.collect();
|
||||||
|
assert!(
|
||||||
|
leftovers.is_empty(),
|
||||||
|
"partial files left behind: {leftovers:?}"
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
use anyhow::anyhow;
|
use anyhow::anyhow;
|
||||||
|
|
||||||
use crate::Result;
|
use crate::Result;
|
||||||
use std::path::Path;
|
use std::path::{Path, PathBuf};
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
// Constants
|
// Constants
|
||||||
@@ -35,6 +35,23 @@ pub fn random_suffix(len: usize) -> String {
|
|||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ============================================================================
|
||||||
|
// Unique scratch paths for atomic writes
|
||||||
|
// ============================================================================
|
||||||
|
|
||||||
|
/// Sequence counter for [`partial_path`], unique within the process.
|
||||||
|
static PARTIAL_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
|
||||||
|
|
||||||
|
/// A sibling scratch path for writing `dest` before an atomic rename:
|
||||||
|
/// the pid separates concurrent processes, the sequence separates
|
||||||
|
/// threads. Two writers racing on the same cache entry each complete
|
||||||
|
/// their own file, so whichever rename lands last, the bytes at `dest`
|
||||||
|
/// are whole.
|
||||||
|
pub fn partial_path(dest: &Path) -> PathBuf {
|
||||||
|
let seq = PARTIAL_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
|
||||||
|
dest.with_extension(format!("partial.{}-{seq}", std::process::id()))
|
||||||
|
}
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
// Architecture handling
|
// Architecture handling
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
|
|||||||
Reference in new issue
Block a user