fix: Don't stack tmpfs mounts on repeated NodePublishVolume

CHANGELOG.md

@@ -23,6 +23,8 @@ All notable changes to this project will be documented in this file.
   `serviceAccount.create=false` now requires `serviceAccount.name`; it used to fall back to the
   namespace default ServiceAccount, which lacks the operator ClusterRole ([#736]).
 - `CreateVolume` no longer returns gRPC codes that make external-provisioner retry indefinitely ([#743]).
+- In privileged mode, `NodePublishVolume` reuses an existing tmpfs mount instead of mounting another one on top, so a repeated publish (for example after a kubelet restart) no longer stacks mounts, which kept Pods in `Terminating` for minutes ([#770]).
+- `NodePublishVolume` truncates the secret files it rewrites, so a repeated publish no longer leaves trailing bytes of a longer previous file behind ([#770]).
 - Report the certificate expiry for the `certManager` backend, so that Pods are restarted before
   their certificate expires and pick up the renewed one. Previously no expiry was reported at all,
   so a Pod kept the certificate it was given at startup and eventually ran on an expired one
@@ -39,6 +41,7 @@ All notable changes to this project will be documented in this file.
 [#762]: https://github.com/stackabletech/secret-operator/pull/762
 [#763]: https://github.com/stackabletech/secret-operator/pull/763
 [#766]: https://github.com/stackabletech/secret-operator/pull/766
+[#770]: https://github.com/stackabletech/secret-operator/pull/770
 
 ## [26.7.0] - 2026-07-21
 

rust/operator-binary/src/csi_server/node.rs

@@ -1,6 +1,6 @@
 use std::{
     fs::Permissions,
-    os::unix::prelude::PermissionsExt,
+    os::unix::prelude::{MetadataExt, PermissionsExt},
     path::{Component, Path, PathBuf},
 };
 
@@ -66,6 +66,12 @@ enum PublishError {
         path: PathBuf,
     },
 
+    #[snafu(display("failed to read metadata of {path:?}"))]
+    ReadMetadata {
+        source: std::io::Error,
+        path: PathBuf,
+    },
+
     #[snafu(display("failed to mount volume mount directory {path:?}"))]
     Mount {
         source: std::io::Error,
@@ -122,6 +128,7 @@ impl From<PublishError> for Status {
                 Status::new(source.grpc_code(), full_msg)
             }
             PublishError::CreateDir { .. } => Status::unavailable(full_msg),
+            PublishError::ReadMetadata { .. } => Status::unavailable(full_msg),
             PublishError::Mount { .. } => Status::unavailable(full_msg),
             PublishError::FormatData { .. } => Status::unavailable(full_msg),
             PublishError::SetDirPermissions { .. } => Status::unavailable(full_msg),
@@ -194,11 +201,21 @@ impl SecretProvisionerNode {
             },
         }
         if self.privileged {
-            Mount::builder()
-                .fstype("tmpfs")
-                .flags(MountFlags::NODEV | MountFlags::NOEXEC | MountFlags::NOSUID)
-                .mount("", target_path)
-                .context(publish_error::MountSnafu { path: target_path })?;
+            // NodePublishVolume must be idempotent: the kubelet calls it again for volumes it has
+            // already published, for example after a kubelet restart. Mounting on every call would
+            // stack a new empty tmpfs over the previous one. The container keeps the lowest layer,
+            // and `clean_secret_dir` unmounts only one layer per NodeUnpublishVolume, so removing
+            // the directory fails with EBUSY until the kubelet has retried once per layer.
+            // And that is the reason we check here whether a mount already exists.
+            if is_mount_point(target_path).await? {
+                tracing::info!(volume.path = %target_path.display(), "Volume path is already mounted, reusing the mount");
+            } else {
+                Mount::builder()
+                    .fstype("tmpfs")
+                    .flags(MountFlags::NODEV | MountFlags::NOEXEC | MountFlags::NOSUID)
+                    .mount("", target_path)
+                    .context(publish_error::MountSnafu { path: target_path })?;
+            }
         } else {
             tracing::info!("Running in unprivileged mode, not creating mount for secret volume");
         }
@@ -231,6 +248,10 @@ impl SecretProvisionerNode {
             let mut opts = OpenOptions::new();
             opts.create(true)
                 .write(true)
+                // A repeated publish rewrites existing files, and regenerated keys and
+                // certificates vary in length. Without truncating, a shorter file keeps the
+                // trailing bytes of the previous one. Not good. So we truncate.
+                .truncate(true)
                 // User: root/secret-operator
                 // Group: Controlled by Pod.securityContext.fsGroup, the actual application
                 // (when running as unprivileged user)
@@ -373,6 +394,28 @@ impl SecretProvisionerNode {
     }
 }
 
+/// Returns whether `path` is the root of a mount, by comparing its device with its parent's.
+///
+/// Bind mounts within the same filesystem are not detected.
+/// That is fine for the tmpfs mounts created by [`SecretProvisionerNode::prepare_secret_dir`].
+async fn is_mount_point(path: &Path) -> Result<bool, PublishError> {
+    match path.parent() {
+        Some(parent) => {
+            let device = tokio::fs::metadata(path)
+                .await
+                .context(publish_error::ReadMetadataSnafu { path })?
+                .dev();
+            let parent_device = tokio::fs::metadata(parent)
+                .await
+                .context(publish_error::ReadMetadataSnafu { path: parent })?
+                .dev();
+            Ok(device != parent_device)
+        }
+        // If the path and its parent is the same, we can always return false.
+        None => Ok(false),
+    }
+}
+
 // Most of the services are not yet implemented, most of them will never be, because they are
 // not needed for this use case.
 // The main two services are publish_volume und unpublish_volume, which get called whenever a
@@ -496,3 +539,29 @@ impl Node for SecretProvisionerNode {
         }))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::path::Path;
+
+    use super::is_mount_point;
+
+    #[tokio::test]
+    async fn plain_directory_is_not_mount_point() {
+        let dir = tempfile::tempdir().expect("temporary directory can be created");
+        assert!(
+            !is_mount_point(dir.path())
+                .await
+                .expect("metadata of temporary directory is readable")
+        );
+    }
+
+    #[tokio::test]
+    async fn proc_is_mount_point() {
+        assert!(
+            is_mount_point(Path::new("/proc"))
+                .await
+                .expect("metadata of /proc is readable")
+        );
+    }
+}