fix: mounting the same volume more than once doesn't work

warm-metal · Nov 28, 2023 · 35b085e · 35b085e
1 parent f1a9c63
commit 35b085e
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 20 deletions.
diff --git a/cmd/plugin/node_server.go b/cmd/plugin/node_server.go
@@ -145,7 +145,7 @@ func (n NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishV
 		return
 	}
 
-	if mountstatus.Get(namedRef) == mountstatus.Mounted {
+	if mountstatus.Get(req.VolumeId) == mountstatus.Mounted {
 		return &csi.NodePublishVolumeResponse{}, nil
 	}
 
@@ -193,6 +193,11 @@ func (n NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpubl
 		return
 	}
 
+	// Clear the mountstatus since the volume has been unmounted
+	// Not doing this will make mount not work properly if the same volume is
+	// attempted to mount twice
+	mountstatus.Delete(req.VolumeId)
+
 	return &csi.NodeUnpublishVolumeResponse{}, nil
 }
 

diff --git a/pkg/mountexecutor/mountexecutor.go b/pkg/mountexecutor/mountexecutor.go
@@ -58,7 +58,7 @@ func NewMountExecutor(o *MountExecutorOptions) *MountExecutor {
 // StartMounting starts the mounting
 func (m *MountExecutor) StartMounting(o *MountOptions) error {
 
-	if pullstatus.Get(o.NamedRef) != pullstatus.Pulled || mountstatus.Get(o.NamedRef) == mountstatus.StillMounting {
+	if pullstatus.Get(o.NamedRef) != pullstatus.Pulled || mountstatus.Get(o.VolumeId) == mountstatus.StillMounting {
 		return nil
 	}
 
@@ -67,12 +67,12 @@ func (m *MountExecutor) StartMounting(o *MountOptions) error {
 		o.VolumeCapability.AccessMode.Mode == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY
 
 	if !m.asyncMount {
-		mountstatus.Update(o.NamedRef, mountstatus.StillMounting)
+		mountstatus.Update(o.VolumeId, mountstatus.StillMounting)
 		if err := m.mounter.Mount(o.Context, o.VolumeId, backend.MountTarget(o.TargetPath), o.NamedRef, ro); err != nil {
-			mountstatus.Update(o.NamedRef, mountstatus.Errored)
+			mountstatus.Update(o.VolumeId, mountstatus.Errored)
 			return err
 		}
-		mountstatus.Update(o.NamedRef, mountstatus.Mounted)
+		mountstatus.Update(o.VolumeId, mountstatus.Mounted)
 		return nil
 	}
 
@@ -81,15 +81,15 @@ func (m *MountExecutor) StartMounting(o *MountOptions) error {
 		ctx, cancel := context.WithTimeout(context.Background(), mountCtxTimeout)
 		defer cancel()
 
-		mountstatus.Update(o.NamedRef, mountstatus.StillMounting)
+		mountstatus.Update(o.VolumeId, mountstatus.StillMounting)
 		if err := m.mounter.Mount(ctx, o.VolumeId, backend.MountTarget(o.TargetPath), o.NamedRef, ro); err != nil {
 			klog.Errorf("mount err: %v", err.Error())
-			mountstatus.Update(o.NamedRef, mountstatus.Errored)
+			mountstatus.Update(o.VolumeId, mountstatus.Errored)
 			m.asyncErrs[o.NamedRef] = fmt.Errorf("err: %v: %v", err, m.asyncErrs[o.NamedRef])
 			m.mutex.Unlock()
 			return
 		}
-		mountstatus.Update(o.NamedRef, mountstatus.Mounted)
+		mountstatus.Update(o.VolumeId, mountstatus.Mounted)
 		m.mutex.Unlock()
 	}()
 
@@ -107,7 +107,7 @@ func (m *MountExecutor) WaitForMount(o *MountOptions) error {
 	}
 
 	mountCondFn := func() (done bool, err error) {
-		if mountstatus.Get(o.NamedRef) == mountstatus.Mounted {
+		if mountstatus.Get(o.VolumeId) == mountstatus.Mounted {
 			return true, nil
 		}
 		if m.asyncErrs[o.NamedRef] != nil {

diff --git a/pkg/mountstatus/mountstatus.go b/pkg/mountstatus/mountstatus.go
@@ -2,8 +2,6 @@ package mountstatus
 
 import (
 	"sync"
-
-	"github.com/containerd/containerd/reference/docker"
 )
 
 // ImagePullStatus represents mount status of an image
@@ -23,43 +21,43 @@ const (
 
 // ImageMountStatusRecorder records the status of image mounts
 type ImageMountStatusRecorder struct {
-	status map[docker.Named]ImageMountStatus
+	status map[string]ImageMountStatus
 	mutex  sync.Mutex
 }
 
 var i ImageMountStatusRecorder
 
 func init() {
 	i = ImageMountStatusRecorder{
-		status: make(map[docker.Named]ImageMountStatus),
+		status: make(map[string]ImageMountStatus),
 		mutex:  sync.Mutex{},
 	}
 }
 
 // Update updates the mount status of an image
-func Update(imageRef docker.Named, status ImageMountStatus) {
+func Update(volumeId string, status ImageMountStatus) {
 	i.mutex.Lock()
 	defer i.mutex.Unlock()
 
-	i.status[imageRef] = status
+	i.status[volumeId] = status
 }
 
 // Delete deletes the mount status of an image
-func Delete(imageRef docker.Named) {
+func Delete(volumeId string) {
 	i.mutex.Lock()
 	defer i.mutex.Unlock()
 
-	delete(i.status, imageRef)
+	delete(i.status, volumeId)
 }
 
 // Get gets the mount status of an image
-func Get(imageRef docker.Named) ImageMountStatus {
+func Get(volumeId string) ImageMountStatus {
 	i.mutex.Lock()
 	defer i.mutex.Unlock()
 
-	if _, ok := i.status[imageRef]; !ok {
+	if _, ok := i.status[volumeId]; !ok {
 		return StatusNotFound
 	}
 
-	return i.status[imageRef]
+	return i.status[volumeId]
 }