Merge pull request #3 from serverscom/fix-cleanup-target

Fix cleanup iscsi target

Author: olegy89

README.md

@@ -68,9 +68,11 @@ Labels merge with priority: **System > PVC > StorageClass**
 Example:
 ```yaml
 # StorageClass labels
-metadata:
-  labels:
-    environment: production
+parameters:
+  rbs.csi.servers.com/labels: |
+    {
+      "managed-by": "kubernetes"
+    }
 
 ---
 # PVC labels

go.mod

@@ -11,6 +11,7 @@ require (
 	go.uber.org/mock v0.6.0
 	google.golang.org/grpc v1.75.0
 	google.golang.org/protobuf v1.36.10
+	k8s.io/api v0.34.2
 	k8s.io/apimachinery v0.34.2
 	k8s.io/client-go v0.34.2
 	k8s.io/klog/v2 v2.130.1
@@ -52,7 +53,6 @@ require (
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
-	k8s.io/api v0.34.2 // indirect
 	k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect
 	sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect
 	sigs.k8s.io/randfill v1.0.0 // indirect

pkg/csi/helpers.go

@@ -4,14 +4,19 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"os"
+	"path/filepath"
 	"strconv"
 	"time"
 
+	"github.com/serverscom/rbs-csi-driver/pkg/iscsi"
 	serverscom "github.com/serverscom/serverscom-go-client/pkg"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
 )
 
+const targetInfoFile = ".target-info"
+
 // getLocationID gets location ID from parameters, supporting both ID and name
 func (s *ControllerService) getLocationID(ctx context.Context, parameters map[string]string) (int, error) {
 	locationStr, ok := parameters["rbs.csi.servers.com/location"]
@@ -113,3 +118,38 @@ func (s *ControllerService) waitForVolumeActive(ctx context.Context, volumeID st
 		}
 	}
 }
+
+func targetInfoPath(stagingPath string) string {
+	return filepath.Join(stagingPath, targetInfoFile)
+}
+
+func SaveTargetInfo(stagingPath string, target *iscsi.TargetInfo) error {
+	data, err := json.Marshal(target)
+	if err != nil {
+		return err
+	}
+
+	return os.WriteFile(targetInfoPath(stagingPath), data, 0600)
+}
+
+func LoadTargetInfo(stagingPath string) (*iscsi.TargetInfo, error) {
+	data, err := os.ReadFile(targetInfoPath(stagingPath))
+	if err != nil {
+		return nil, err
+	}
+
+	var target iscsi.TargetInfo
+	if err := json.Unmarshal(data, &target); err != nil {
+		return nil, err
+	}
+
+	return &target, nil
+}
+
+func DeleteTargetInfo(stagingPath string) error {
+	err := os.Remove(targetInfoPath(stagingPath))
+	if err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	return nil
+}

pkg/csi/node.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"strings"
 	"time"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
@@ -86,6 +85,11 @@ func (s *NodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStageVol
 	}
 	klog.V(2).InfoS("Prepared iSCSI target", "portal", portal, "iqn", targetIQN)
 
+	if err := SaveTargetInfo(stagingPath, target); err != nil {
+		klog.ErrorS(err, "Failed to save target info")
+		return nil, status.Errorf(codes.Internal, "failed to save target info: %v", err)
+	}
+
 	// Check if already logged in
 	klog.V(2).InfoS("Checking iscsi login status")
 	loggedIn, err := s.iscsiManager.IsLoggedIn(ctx, target)
@@ -158,37 +162,33 @@ func (s *NodeService) NodeUnstageVolume(ctx context.Context, req *csi.NodeUnstag
 
 	stagingPath := req.GetStagingTargetPath()
 
-	// Check if staging path is mounted
 	mounted, err := s.mountManager.IsMounted(stagingPath)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "failed to check if staging path is mounted: %v", err)
+		return nil, status.Errorf(codes.Internal, "failed to check mount: %v", err)
 	}
 
 	if mounted {
-		// Get mount info to determine device
-		mountInfo, err := s.mountManager.GetMountInfo(stagingPath)
-		if err != nil {
-			return nil, status.Errorf(codes.Internal, "failed to get mount info: %v", err)
-		}
-
-		// Unmount the staging path
-		klog.V(1).InfoS("Unmounting staging path", "staging_path", stagingPath)
+		klog.V(2).InfoS("Unmounting staging path", "staging_path", stagingPath)
 		if err := s.mountManager.Unmount(ctx, stagingPath); err != nil {
 			return nil, status.Errorf(codes.Internal, "failed to unmount staging path: %v", err)
 		}
+	}
 
-		// Try to logout from iSCSI target
-		if strings.Contains(mountInfo.Device, "/dev/") {
-			klog.V(1).InfoS("Device unmounted", "device", mountInfo.Device)
+	// Load target info and cleanup iSCSI
+	target, err := LoadTargetInfo(stagingPath)
+	if err == nil {
+		if err := s.iscsiManager.CleanupTarget(ctx, target); err != nil {
+			return nil, status.Errorf(codes.Internal, "iscsi cleanup failed: %v", err)
 		}
 	}
 
-	// Remove staging directory
+	_ = DeleteTargetInfo(stagingPath)
+
 	if err := os.RemoveAll(stagingPath); err != nil {
-		klog.ErrorS(err, "Failed to remove staging directory")
+		klog.ErrorS(err, "Failed to remove staging directory", "path", stagingPath)
 	}
 
-	klog.InfoS("Volume unstaged successfully", "volume_id", req.GetVolumeId())
+	klog.V(1).InfoS("Volume unstaged successfully", "volume_id", req.GetVolumeId())
 	return &csi.NodeUnstageVolumeResponse{}, nil
 }
 

pkg/csi/node_test.go

@@ -2,7 +2,10 @@ package csi
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
+	"os"
+	"path/filepath"
 	"testing"
 	"time"
 
@@ -221,7 +224,7 @@ func TestNodeUnstageVolume_Success(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	svc, _, mmount := newTestNode(ctrl)
+	svc, iscsiMgr, mmount := newTestNode(ctrl)
 	ctx := context.Background()
 
 	req := &csi.NodeUnstageVolumeRequest{
@@ -230,24 +233,32 @@ func TestNodeUnstageVolume_Success(t *testing.T) {
 	}
 
 	mmount.EXPECT().IsMounted("/tmp/staging").Return(true, nil)
-	mmount.EXPECT().GetMountInfo("/tmp/staging").Return(&mount.MountInfo{
-		Device: "/dev/sdb",
-		FSType: "ext4",
-	}, nil)
 	mmount.EXPECT().Unmount(ctx, "/tmp/staging").Return(nil)
 
+	target := &iscsi.TargetInfo{
+		IQN:    "iqn.test",
+		Portal: "127.0.0.1:3260",
+	}
+	data, _ := json.Marshal(target)
+	_ = os.MkdirAll(req.StagingTargetPath, 0755)
+	_ = os.WriteFile(filepath.Join(req.StagingTargetPath, ".target-info"), data, 0600)
+
+	iscsiMgr.EXPECT().CleanupTarget(ctx, target).Return(nil)
+
 	resp, err := svc.NodeUnstageVolume(ctx, req)
 
 	g.Expect(err).To(BeNil())
 	g.Expect(resp).NotTo(BeNil())
+
+	_ = os.RemoveAll(req.StagingTargetPath)
 }
 
 func TestNodeUnstageVolume_NotMounted(t *testing.T) {
 	g := NewGomegaWithT(t)
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	svc, _, mmount := newTestNode(ctrl)
+	svc, iscsiMgr, mmount := newTestNode(ctrl)
 	ctx := context.Background()
 
 	req := &csi.NodeUnstageVolumeRequest{
@@ -257,10 +268,21 @@ func TestNodeUnstageVolume_NotMounted(t *testing.T) {
 
 	mmount.EXPECT().IsMounted("/tmp/staging").Return(false, nil)
 
+	target := &iscsi.TargetInfo{
+		IQN:    "iqn.test",
+		Portal: "127.0.0.1:3260",
+	}
+	_ = os.MkdirAll(req.StagingTargetPath, 0755)
+	_ = os.WriteFile(filepath.Join(req.StagingTargetPath, ".target-info"), []byte(`{"IQN":"iqn.test","Portal":"127.0.0.1:3260"}`), 0600)
+
+	iscsiMgr.EXPECT().CleanupTarget(ctx, target).Return(nil)
+
 	resp, err := svc.NodeUnstageVolume(ctx, req)
 
 	g.Expect(err).To(BeNil())
 	g.Expect(resp).NotTo(BeNil())
+
+	_ = os.RemoveAll(req.StagingTargetPath)
 }
 
 func TestNodePublishVolume_Success(t *testing.T) {