From 62bd056a3427738f0bebd9ca405d84e29275755b Mon Sep 17 00:00:00 2001
From: chrislusf <chris.lu@gmail.com>
Date: Sat, 6 Dec 2025 12:13:31 -0800
Subject: fix: address gemini-code-assist review comments

- Change mountService updateStrategy from RollingUpdate to OnDelete
  (mount service not yet resilient to its own restarts)
- Change mountService image from :latest to :dev for consistency
- Fix defer os.RemoveAll: explicitly remove cache dir after process stops
  to avoid removing while process might still be running
---
 deploy/helm/seaweedfs-csi-driver/values.yaml | 8 ++++----
 pkg/mountmanager/manager.go                  | 7 +++++--
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/deploy/helm/seaweedfs-csi-driver/values.yaml b/deploy/helm/seaweedfs-csi-driver/values.yaml
index e2de520..eed5b5e 100644
--- a/deploy/helm/seaweedfs-csi-driver/values.yaml
+++ b/deploy/helm/seaweedfs-csi-driver/values.yaml
@@ -80,7 +80,7 @@ mountService:
   # Set to true (default) for mount resilience - mounts survive CSI driver restarts
   # Set to false to disable the separate mount service (mounts managed by CSI driver directly)
   enabled: true
-  image:  chrislusf/seaweedfs-mount:latest
+  image:  chrislusf/seaweedfs-mount:dev
   # Endpoint for communication between CSI driver and mount service
   endpoint: unix:///var/lib/seaweedfs-mount/seaweedfs-mount.sock
   # Host directory for mount service socket files
@@ -92,10 +92,10 @@ mountService:
     capabilities:
       add: ["SYS_ADMIN"]
     allowPrivilegeEscalation: true
+  # Use OnDelete strategy since mount service is not yet resilient to its own restarts.
+  # This allows manual, controlled updates to prevent automated disruption of active mounts.
   updateStrategy:
-    type: RollingUpdate
-    rollingUpdate:
-      maxUnavailable: 25%
+    type: OnDelete
   affinity: {}
   tolerations:
   resources: {}
diff --git a/pkg/mountmanager/manager.go b/pkg/mountmanager/manager.go
index e4e4329..1bdeb3e 100644
--- a/pkg/mountmanager/manager.go
+++ b/pkg/mountmanager/manager.go
@@ -102,12 +102,15 @@ func (m *Manager) Unmount(req *UnmountRequest) (*UnmountResponse, error) {
 		}
 	}
 
-	defer os.RemoveAll(entry.cacheDir)
-
 	if err := entry.process.stop(); err != nil {
 		return nil, err
 	}
 
+	// Remove cache dir only after process has been successfully stopped
+	if err := os.RemoveAll(entry.cacheDir); err != nil {
+		glog.Warningf("failed to remove cache dir %s for volume %s: %v", entry.cacheDir, req.VolumeID, err)
+	}
+
 	glog.Infof("stopped weed mount process for volume %s at %s", req.VolumeID, entry.targetPath)
 	return &UnmountResponse{}, nil
 }
-- 
cgit v1.2.3