Skip to content

Commit

Permalink
fix(csi-node): don't unstage if bind mount exists
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
tiagolobocastro committed Nov 7, 2022
1 parent 94c5a3e commit bd60aa9
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 7 deletions.
34 changes: 27 additions & 7 deletions csi/src/filesystem_vol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
));
}
}

Expand Down
19 changes: 19 additions & 0 deletions csi/src/mount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MountInfo> {
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 {
Expand Down

0 comments on commit bd60aa9

Please sign in to comment.