Fix multiple connections problems to one same nqn in a single node (#23)

* fix: make multiple connections to one same nqn possible in a single node

Multiple connections to one same nqn in a single node return -EALREADY error.
So, we should use a hostnqn to make every connections seperate. It is appropriate
to use a target path from a volume request as a hostnqn.

* fix: make ro and rw mounts for one same nqn possible in a single node

ReadOnly and ReadWrite mounts for one same nqn in a single node return -EBUSY error.
So, we should remove readonly and readwrite options when mounting a block device in a
csi driver. A cri runtime makes a mount as readonly inside containers if needed.

* fix: add fallback supports for no hostnqn sysfs file supports

Directories per each nqn will be created in /run/nvmf and files per each hostnqn
will be created in /run/nvmf/{nqn}. If linux kernel has no hostnqn sysfs file
supports, we will disconnect all connections for a nqn at once when a directory for
the nqn is empty.

* refactor: rename sysfs_nqn_path to sysfs_subsysnqn_path

* refactor: use filepath.Join instead of strings.Join

* fix: add explicitly UnsupportedHostnqnError for fallback supports

If linux kernel has no hostnqn sysfs file supports, return UnsupportedHostnqnError
and switch to fallback mode which will disconnect all connections at once when
a last controller in a nqn is disconnected.

* fix: return directly after fallback mode

* fix: mount the host's /run/nvmf directory to csi-node-driver

deploy/kubernetes/csi-nvmf-node.yaml

@@ -73,6 +73,8 @@ spec:
             - name: pods-mount-dir
               mountPath: /var/lib/kubelet/pods
               mountPropagation: "Bidirectional"
+            - name: run-nvmf-dir
+              mountPath: /run/nvmf
             - name: host-dev
               mountPath: /dev
               mountPropagation: "HostToContainer"
@@ -94,6 +96,10 @@ spec:
           hostPath:
             path: /var/lib/kubelet/pods
             type: Directory
+        - name: run-nvmf-dir
+          hostPath:
+            path: /run/nvmf
+            type: DirectoryOrCreate
         - name: host-dev
           hostPath:
             path: /dev
@@ -102,4 +108,4 @@ spec:
             path: /sys
         - name: lib-modules
           hostPath:
-            path: /lib/modules
+            path: /lib/modules

pkg/nvmf/const.go

@@ -18,6 +18,7 @@ package nvmf
 const (
 	NVMF_NQN_SIZE = 223
 	SYS_NVMF      = "/sys/class/nvme"
+	RUN_NVMF      = "/run/nvmf"
 )
 
 // Here erron

pkg/nvmf/errors.go

@@ -0,0 +1,36 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package nvmf
+
+import "fmt"
+
+type NoControllerError struct {
+	Nqn     string
+	Hostnqn string
+}
+
+func (e *NoControllerError) Error() string {
+	return fmt.Sprintf("not found controller: nqn=%s, hostnqn=%s", e.Nqn, e.Hostnqn)
+}
+
+type UnsupportedHostnqnError struct {
+	Target string
+}
+
+func (e *UnsupportedHostnqnError) Error() string {
+	return fmt.Sprintf("unsupported hostnqn sysfs file: target=%s", e.Target)
+}

pkg/nvmf/fabrics.go

@@ -17,10 +17,12 @@ limitations under the License.
 package nvmf
 
 import (
+	b64 "encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/kubernetes-csi/csi-driver-nvmf/pkg/utils"
@@ -34,11 +36,12 @@ type Connector struct {
 	TargetAddr    string
 	TargetPort    string
 	Transport     string
+	HostNqn       string
 	RetryCount    int32
 	CheckInterval int32
 }
 
-func getNvmfConnector(nvmfInfo *nvmfDiskInfo) *Connector {
+func getNvmfConnector(nvmfInfo *nvmfDiskInfo, hostnqn string) *Connector {
 	return &Connector{
 		VolumeID:   nvmfInfo.VolName,
 		DeviceUUID: nvmfInfo.DeviceUUID,
@@ -46,6 +49,7 @@ func getNvmfConnector(nvmfInfo *nvmfDiskInfo) *Connector {
 		TargetAddr: nvmfInfo.Addr,
 		TargetPort: nvmfInfo.Port,
 		Transport:  nvmfInfo.Transport,
+		HostNqn:    hostnqn,
 	}
 }
 
@@ -84,44 +88,109 @@ func _disconnect(sysfs_path string) error {
 	return nil
 }
 
-func disconnectSubsys(nqn, ctrl string) (res bool) {
+func disconnectSubsysWithHostNqn(nqn, hostnqn, ctrl string) error {
-	sysfs_nqn_path := fmt.Sprintf("%s/%s/subsysnqn", SYS_NVMF, ctrl)
+	sysfs_subsysnqn_path := fmt.Sprintf("%s/%s/subsysnqn", SYS_NVMF, ctrl)
+	sysfs_hostnqn_path := fmt.Sprintf("%s/%s/hostnqn", SYS_NVMF, ctrl)
 	sysfs_del_path := fmt.Sprintf("%s/%s/delete_controller", SYS_NVMF, ctrl)
 
-	file, err := os.Open(sysfs_nqn_path)
+	file, err := os.Open(sysfs_subsysnqn_path)
 	if err != nil {
-		klog.Errorf("Disconnect: open file %s err: %v", file.Name(), err)
+		klog.Errorf("Disconnect: open file %s err: %v", sysfs_subsysnqn_path, err)
-		return false
+		return &NoControllerError{Nqn: nqn, Hostnqn: hostnqn}
 	}
 	defer file.Close()
 
 	lines, err := utils.ReadLinesFromFile(file)
 	if err != nil {
 		klog.Errorf("Disconnect: read file %s err: %v", file.Name(), err)
-		return false
+		return &NoControllerError{Nqn: nqn, Hostnqn: hostnqn}
 	}
 
 	if lines[0] != nqn {
 		klog.Warningf("Disconnect: not this subsystem, skip")
-		return false
+		return &NoControllerError{Nqn: nqn, Hostnqn: hostnqn}
+	}
+
+	file, err = os.Open(sysfs_hostnqn_path)
+	if err != nil {
+		klog.Errorf("Disconnect: open file %s err: %v", sysfs_hostnqn_path, err)
+		return &UnsupportedHostnqnError{Target: sysfs_hostnqn_path}
+	}
+	defer file.Close()
+
+	lines, err = utils.ReadLinesFromFile(file)
+	if err != nil {
+		klog.Errorf("Disconnect: read file %s err: %v", file.Name(), err)
+		return &NoControllerError{Nqn: nqn, Hostnqn: hostnqn}
+	}
+
+	if lines[0] != hostnqn {
+		klog.Warningf("Disconnect: not this subsystem, skip")
+		return &NoControllerError{Nqn: nqn, Hostnqn: hostnqn}
 	}
 
 	err = _disconnect(sysfs_del_path)
 	if err != nil {
 		klog.Errorf("Disconnect: disconnect error: %s", err)
-		return false
+		return &NoControllerError{Nqn: nqn, Hostnqn: hostnqn}
 	}
 
-	return true
+	return nil
 }
 
-func disconnectByNqn(nqn string) int {
+func disconnectSubsys(nqn, ctrl string) error {
+	sysfs_subsysnqn_path := fmt.Sprintf("%s/%s/subsysnqn", SYS_NVMF, ctrl)
+	sysfs_del_path := fmt.Sprintf("%s/%s/delete_controller", SYS_NVMF, ctrl)
+
+	file, err := os.Open(sysfs_subsysnqn_path)
+	if err != nil {
+		klog.Errorf("Disconnect: open file %s err: %v", sysfs_subsysnqn_path, err)
+		return &NoControllerError{Nqn: nqn, Hostnqn: ""}
+	}
+	defer file.Close()
+
+	lines, err := utils.ReadLinesFromFile(file)
+	if err != nil {
+		klog.Errorf("Disconnect: read file %s err: %v", file.Name(), err)
+		return &NoControllerError{Nqn: nqn, Hostnqn: ""}
+	}
+
+	if lines[0] != nqn {
+		klog.Warningf("Disconnect: not this subsystem, skip")
+		return &NoControllerError{Nqn: nqn, Hostnqn: ""}
+	}
+
+	err = _disconnect(sysfs_del_path)
+	if err != nil {
+		klog.Errorf("Disconnect: disconnect error: %s", err)
+		return &NoControllerError{Nqn: nqn, Hostnqn: ""}
+	}
+
+	return nil
+}
+
+func disconnectByNqn(nqn, hostnqn string) int {
 	ret := 0
 	if len(nqn) > NVMF_NQN_SIZE {
 		klog.Errorf("Disconnect: nqn %s is too long ", nqn)
 		return -EINVAL
 	}
 
+	// delete hostnqn file
+	hostnqnPath := filepath.Join(RUN_NVMF, nqn, b64.StdEncoding.EncodeToString([]byte(hostnqn)))
+	os.Remove(hostnqnPath)
+
+	// delete nqn directory if has no hostnqn files
+	nqnPath := filepath.Join(RUN_NVMF, nqn)
+	hostnqns, err := ioutil.ReadDir(nqnPath)
+	if err != nil {
+		klog.Errorf("Disconnect: readdir %s err: %v", nqnPath, err)
+		return -ENOENT
+	}
+	if len(hostnqns) <= 0 {
+		os.RemoveAll(nqnPath)
+	}
+
 	devices, err := ioutil.ReadDir(SYS_NVMF)
 	if err != nil {
 		klog.Errorf("Disconnect: readdir %s err: %s", SYS_NVMF, err)
@@ -129,10 +198,32 @@ func disconnectByNqn(nqn string) int {
 	}
 
 	for _, device := range devices {
-		if disconnectSubsys(nqn, device.Name()) {
+		if err := disconnectSubsysWithHostNqn(nqn, hostnqn, device.Name()); err != nil {
+			if _, ok := err.(*UnsupportedHostnqnError); ok {
+				klog.Infof("Fallback because you have no hostnqn supports!")
+
+				// disconnect all controllers if has no hostnqn files
+				if len(hostnqns) <= 0 {
+					devices, err := ioutil.ReadDir(SYS_NVMF)
+					if err != nil {
+						klog.Errorf("Disconnect: readdir %s err: %s", SYS_NVMF, err)
+						return -ENOENT
+					}
+
+					for _, device := range devices {
+						if err := disconnectSubsys(nqn, device.Name()); err == nil {
+							ret++
+						}
+					}
+				}
+
+				return ret
+			}
+		} else {
 			ret++
 		}
 	}
+
 	return ret
 }
 
@@ -154,7 +245,7 @@ func (c *Connector) Connect() (string, error) {
 		return "", fmt.Errorf("csi transport only support tcp/rdma ")
 	}
 
-	baseString := fmt.Sprintf("nqn=%s,transport=%s,traddr=%s,trsvcid=%s", c.TargetNqn, c.Transport, c.TargetAddr, c.TargetPort)
+	baseString := fmt.Sprintf("nqn=%s,transport=%s,traddr=%s,trsvcid=%s,hostnqn=%s", c.TargetNqn, c.Transport, c.TargetAddr, c.TargetPort, c.HostNqn)
 	devicePath := strings.Join([]string{"/dev/disk/by-id/nvme-uuid", c.DeviceUUID}, ".")
 
 	// connect to nvmf disk
@@ -162,25 +253,49 @@ func (c *Connector) Connect() (string, error) {
 	if err != nil {
 		return "", err
 	}
-	klog.Infof("Connect Volume %s success nqn: %s", c.VolumeID, c.TargetNqn)
+	klog.Infof("Connect Volume %s success nqn: %s, hostnqn: %s", c.VolumeID, c.TargetNqn, c.HostNqn)
 	retries := int(c.RetryCount / c.CheckInterval)
 	if exists, err := waitForPathToExist(devicePath, retries, int(c.CheckInterval), c.Transport); !exists {
 		klog.Errorf("connect nqn %s error %v, rollback!!!", c.TargetNqn, err)
-		ret := disconnectByNqn(c.TargetNqn)
+		ret := disconnectByNqn(c.TargetNqn, c.HostNqn)
 		if ret < 0 {
 			klog.Errorf("rollback error !!!")
 		}
 		return "", err
 	}
 
+	// create nqn directory
+	nqnPath := filepath.Join(RUN_NVMF, c.TargetNqn)
+	if err := os.MkdirAll(nqnPath, 0750); err != nil {
+		klog.Errorf("create nqn directory %s error %v, rollback!!!", c.TargetNqn, err)
+		ret := disconnectByNqn(c.TargetNqn, c.HostNqn)
+		if ret < 0 {
+			klog.Errorf("rollback error !!!")
+		}
+		return "", err
+	}
+
+	// create hostnqn file
+	hostnqnPath := filepath.Join(RUN_NVMF, c.TargetNqn, b64.StdEncoding.EncodeToString([]byte(c.HostNqn)))
+	file, err := os.Create(hostnqnPath)
+	if err != nil {
+		klog.Errorf("create hostnqn file %s:%s error %v, rollback!!!", c.TargetNqn, c.HostNqn, err)
+		ret := disconnectByNqn(c.TargetNqn, c.HostNqn)
+		if ret < 0 {
+			klog.Errorf("rollback error !!!")
+		}
+		return "", err
+	}
+	defer file.Close()
+
 	klog.Infof("After connect we're returning devicePath: %s", devicePath)
 	return devicePath, nil
 }
 
 // we disconnect only by nqn
 func (c *Connector) Disconnect() error {
-	ret := disconnectByNqn(c.TargetNqn)
+	ret := disconnectByNqn(c.TargetNqn, c.HostNqn)
-	if ret == 0 {
+	if ret < 0 {
 		return fmt.Errorf("Disconnect: failed to disconnect by nqn: %s ", c.TargetNqn)
 	}
 

pkg/nvmf/nodeserver.go

@@ -53,7 +53,6 @@ func (n *NodeServer) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetCa
 }
 
 func (n *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolumeRequest) (*csi.NodePublishVolumeResponse, error) {
-
 	// 1. check parameters
 	if req.GetVolumeCapability() == nil {
 		return nil, status.Errorf(codes.InvalidArgument, "NodePublishVolume missing Volume Capability in req.")

pkg/nvmf/nvmf.go

@@ -88,7 +88,7 @@ func getNVMfDiskMounter(nvmfInfo *nvmfDiskInfo, req *csi.NodePublishVolumeReques
 		mounter:      &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: exec.New()},
 		exec:         exec.New(),
 		targetPath:   req.GetTargetPath(),
-		connector:    getNvmfConnector(nvmfInfo),
+		connector:    getNvmfConnector(nvmfInfo, req.GetTargetPath()),
 	}
 }
 
@@ -142,11 +142,6 @@ func AttachDisk(req *csi.NodePublishVolumeRequest, nm nvmfDiskMounter) (string,
 
 	// Tips: use k8s mounter to mount fs and only support "ext4"
 	var options []string
-	if nm.readOnly {
-		options = append(options, "ro")
-	} else {
-		options = append(options, "rw")
-	}
 	options = append(options, nm.mountOptions...)
 	err = nm.mounter.FormatAndMount(devicePath, mntPath, nm.fsType, options)
 	if err != nil {
@@ -161,25 +156,16 @@ func AttachDisk(req *csi.NodePublishVolumeRequest, nm nvmfDiskMounter) (string,
 }
 
 func DetachDisk(volumeID string, num *nvmfDiskUnMounter, targetPath string) error {
-	_, cnt, err := mount.GetDeviceNameFromMount(num.mounter, targetPath)
-	if err != nil {
-		klog.Errorf("nvmf detach disk: failed to get device from mnt: %s\nError: %v", targetPath, err)
-		return err
-	}
 	if pathExists, pathErr := mount.PathExists(targetPath); pathErr != nil {
 		return fmt.Errorf("Error checking if path exists: %v", pathErr)
 	} else if !pathExists {
 		klog.Warningf("Warning: Unmount skipped because path does not exist: %v", targetPath)
 		return nil
 	}
-	if err = num.mounter.Unmount(targetPath); err != nil {
+	if err := num.mounter.Unmount(targetPath); err != nil {
 		klog.Errorf("iscsi detach disk: failed to unmount: %s\nError: %v", targetPath, err)
 		return err
 	}
-	cnt--
-	if cnt != 0 {
-		return nil
-	}
 
 	connector, err := GetConnectorFromFile(targetPath + ".json")
 	if err != nil {