From bd60aa9629e1e0556d379df29037d628d9c05aa9 Mon Sep 17 00:00:00 2001 From: Tiago Castro Date: Tue, 18 Oct 2022 18:41:56 +0100 Subject: [PATCH] fix(csi-node): don't unstage if bind mount exists If an application creates a bind mount for our volume unstaging can be very troublesome as it leaves the mount in a really bad state. Even if we reconnect the device, it's very unlikely things won't recover. This commit adds a check on node_unstage for unknown mounts (as in, not a csi publish mount). Should there be any we output this as a trace and simply fail the unstage. It's up to the user to clean up non-csi mounts, otherwise we can never unstage. Signed-off-by: Tiago Castro --- csi/src/filesystem_vol.rs | 34 +++++++++++++++++++++++++++------- csi/src/mount.rs | 19 +++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/csi/src/filesystem_vol.rs b/csi/src/filesystem_vol.rs index b88437cd0..c5341f44f 100644 --- a/csi/src/filesystem_vol.rs +++ b/csi/src/filesystem_vol.rs @@ -132,15 +132,35 @@ pub async fn unstage_fs_volume( "Unstaging filesystem volume {}, unmounting device {:?} from {}", volume_id, mount.source, fs_staging_path ); + let device = mount.source.to_string_lossy().to_string(); + let mounts = mount::find_src_mounts(&device, Some(fs_staging_path)); + if let Some(unkown_mount) = mounts.first().cloned() { + for mount in mounts { + tracing::error!( + volume.uuid = %volume_id, + "Found unknown bind mount {} for device {:?}", + device, + mount.dest, + ); + } + + return Err(failure!( + Code::Internal, + "Failed to unstage volume {}: existing unknown bind mount {:?} for device {:?}", + volume_id, + unkown_mount.dest, + unkown_mount.source + )); + } if let Err(error) = mount::filesystem_unmount(fs_staging_path) { return Err(failure!( - Code::Internal, - "Failed to unstage volume {}: failed to unmount device {:?} from {}: {}", - volume_id, - mount.source, - fs_staging_path, - error - )); + Code::Internal, + "Failed to unstage volume {}: failed to unmount device {:?} from {}: {}", + volume_id, + mount.source, + fs_staging_path, + error + )); } } diff --git a/csi/src/mount.rs b/csi/src/mount.rs index 79d3666c8..ce316992b 100644 --- a/csi/src/mount.rs +++ b/csi/src/mount.rs @@ -54,6 +54,25 @@ pub fn find_mount( found.map(MountInfo::from) } +/// Return all mounts for a matching source. +/// Optionally ignore the given destination path. +pub(crate) fn find_src_mounts( + source: &str, + dest_ignore: Option<&str>, +) -> Vec { + MountIter::new() + .unwrap() + .flatten() + .filter(|mount| { + mount.source.to_string_lossy() == source + && match dest_ignore { + None => true, + Some(ignore) => ignore != mount.dest.to_string_lossy(), + } + }) + .collect() +} + /// Check if options in "first" are also present in "second", /// but exclude values "ro" and "rw" from the comparison. pub(super) fn subset(first: &[String], second: &[String]) -> bool {