From 71f304ce175b9dc426d3d0dbce224b46ad70f934 Mon Sep 17 00:00:00 2001 From: Eric Ernst Date: Mon, 9 Aug 2021 15:58:52 -0700 Subject: [PATCH] agent: watcher: cleanup mount if needed when container is removed If a bind mount was created for watchable storage, make sure we remove when removing a container. Signed-off-by: Eric Ernst --- src/agent/src/mount.rs | 2 +- src/agent/src/watcher.rs | 68 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/agent/src/mount.rs b/src/agent/src/mount.rs index 3be698cf4a..5b023b3393 100644 --- a/src/agent/src/mount.rs +++ b/src/agent/src/mount.rs @@ -609,7 +609,7 @@ pub async fn add_storages( DRIVER_NVDIMM_TYPE => nvdimm_storage_handler(&logger, &storage, sandbox.clone()).await, DRIVER_WATCHABLE_BIND_TYPE => { bind_watcher_storage_handler(&logger, &storage, sandbox.clone()).await?; - // Don't register watch mounts, they're hanlded separately by the watcher. + // Don't register watch mounts, they're handled separately by the watcher. Ok(String::new()) } _ => { diff --git a/src/agent/src/watcher.rs b/src/agent/src/watcher.rs index a04e8bae97..ddbfd8a60b 100644 --- a/src/agent/src/watcher.rs +++ b/src/agent/src/watcher.rs @@ -70,6 +70,11 @@ pub enum WatcherError { impl Drop for Storage { fn drop(&mut self) { + if !&self.watch { + // If we weren't watching this storage entry, it means that a bind mount + // was created. + let _ = umount(&self.target_mount_point); + } let _ = std::fs::remove_dir_all(&self.target_mount_point); } } @@ -927,4 +932,67 @@ mod tests { let out = fs::read_to_string(dest_dir.path().join("1.txt")).unwrap(); assert_eq!(out, "one"); } + + #[tokio::test] + async fn verify_container_cleanup_watching() { + skip_if_not_root!(); + + let source_dir = tempfile::tempdir().unwrap(); + fs::write(source_dir.path().join("1.txt"), "one").unwrap(); + + let dest_dir = tempfile::tempdir().unwrap(); + + let storage = protos::Storage { + source: source_dir.path().display().to_string(), + mount_point: dest_dir.path().display().to_string(), + ..Default::default() + }; + + let logger = slog::Logger::root(slog::Discard, o!()); + let mut watcher = BindWatcher::default(); + + watcher + .add_container("test".into(), std::iter::once(storage), &logger) + .await + .unwrap(); + + thread::sleep(Duration::from_secs(WATCH_INTERVAL_SECS)); + + let out = fs::read_to_string(dest_dir.path().join("1.txt")).unwrap(); + assert!(dest_dir.path().exists()); + assert_eq!(out, "one"); + + watcher.remove_container("test").await; + + thread::sleep(Duration::from_secs(WATCH_INTERVAL_SECS)); + assert!(!dest_dir.path().exists()); + + for i in 1..21 { + fs::write(source_dir.path().join(format!("{}.txt", i)), "fluff").unwrap(); + } + + // verify non-watched storage is cleaned up correctly + let storage1 = protos::Storage { + source: source_dir.path().display().to_string(), + mount_point: dest_dir.path().display().to_string(), + ..Default::default() + }; + + watcher + .add_container("test".into(), std::iter::once(storage1), &logger) + .await + .unwrap(); + + thread::sleep(Duration::from_secs(WATCH_INTERVAL_SECS)); + + assert!(dest_dir.path().exists()); + assert!(is_mounted(dest_dir.path().to_str().unwrap()).unwrap()); + + watcher.remove_container("test").await; + + thread::sleep(Duration::from_secs(WATCH_INTERVAL_SECS)); + + assert!(!dest_dir.path().exists()); + assert!(!is_mounted(dest_dir.path().to_str().unwrap()).unwrap()); + } }