Fix iSCSI storage plugin cleanup in block volumes

Author: bertinatto

pkg/volume/iscsi/iscsi.go

@@ -280,6 +280,7 @@ type iscsiDisk struct {
 	Portals       []string
 	Iqn           string
 	Lun           string
+	InitIface     string
 	Iface         string
 	chapDiscovery bool
 	chapSession   bool
@@ -551,12 +552,18 @@ func createISCSIDisk(spec *volume.Spec, podUID types.UID, plugin *iscsiPlugin, m
 		return nil, err
 	}
 
+	initIface := iface
+	if initiatorName != "" {
+		iface = bkportal[0] + ":" + spec.Name()
+	}
+
 	return &iscsiDisk{
 		podUID:        podUID,
 		VolName:       spec.Name(),
 		Portals:       bkportal,
 		Iqn:           iqn,
 		Lun:           lun,
+		InitIface:     initIface,
 		Iface:         iface,
 		chapDiscovery: chapDiscovery,
 		chapSession:   chapSession,

pkg/volume/iscsi/iscsi_util.go

@@ -296,27 +296,23 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) (string, error) {
 	var iscsiTransport string
 	var lastErr error
 
-	out, err := b.exec.Run("iscsiadm", "-m", "iface", "-I", b.Iface, "-o", "show")
+	out, err := b.exec.Run("iscsiadm", "-m", "iface", "-I", b.InitIface, "-o", "show")
 	if err != nil {
-		klog.Errorf("iscsi: could not read iface %s error: %s", b.Iface, string(out))
+		klog.Errorf("iscsi: could not read iface %s error: %s", b.InitIface, string(out))
 		return "", err
 	}
 
 	iscsiTransport = extractTransportname(string(out))
 
 	bkpPortal := b.Portals
 
-	// create new iface and copy parameters from pre-configured iface to the created iface
+	// If the initiator name was set, the iface isn't created yet,
+	// so create it and copy parameters from the pre-configured one
 	if b.InitiatorName != "" {
-		// new iface name is <target portal>:<volume name>
-		newIface := bkpPortal[0] + ":" + b.VolName
-		err = cloneIface(b, newIface)
-		if err != nil {
-			klog.Errorf("iscsi: failed to clone iface: %s error: %v", b.Iface, err)
+		if err = cloneIface(b); err != nil {
+			klog.Errorf("iscsi: failed to clone iface: %s error: %v", b.InitIface, err)
 			return "", err
 		}
-		// update iface name
-		b.Iface = newIface
 	}
 
 	// Lock the target while we login to avoid races between 2 volumes that share the same
@@ -860,10 +856,13 @@ func parseIscsiadmShow(output string) (map[string]string, error) {
 	return params, nil
 }
 
-func cloneIface(b iscsiDiskMounter, newIface string) error {
+func cloneIface(b iscsiDiskMounter) error {
 	var lastErr error
+	if b.InitIface == b.Iface {
+		return fmt.Errorf("iscsi: cannot clone iface with same name: %s", b.InitIface)
+	}
 	// get pre-configured iface records
-	out, err := b.exec.Run("iscsiadm", "-m", "iface", "-I", b.Iface, "-o", "show")
+	out, err := b.exec.Run("iscsiadm", "-m", "iface", "-I", b.InitIface, "-o", "show")
 	if err != nil {
 		lastErr = fmt.Errorf("iscsi: failed to show iface records: %s (%v)", string(out), err)
 		return lastErr
@@ -877,22 +876,22 @@ func cloneIface(b iscsiDiskMounter, newIface string) error {
 	// update initiatorname
 	params["iface.initiatorname"] = b.InitiatorName
 	// create new iface
-	out, err = b.exec.Run("iscsiadm", "-m", "iface", "-I", newIface, "-o", "new")
+	out, err = b.exec.Run("iscsiadm", "-m", "iface", "-I", b.Iface, "-o", "new")
 	if err != nil {
 		exit, ok := err.(utilexec.ExitError)
 		if ok && exit.ExitStatus() == iscsiadmErrorSessExists {
-			klog.Infof("iscsi: there is a session already logged in with iface %s", newIface)
+			klog.Infof("iscsi: there is a session already logged in with iface %s", b.Iface)
 		} else {
 			lastErr = fmt.Errorf("iscsi: failed to create new iface: %s (%v)", string(out), err)
 			return lastErr
 		}
 	}
 	// update new iface records
 	for key, val := range params {
-		_, err = b.exec.Run("iscsiadm", "-m", "iface", "-I", newIface, "-o", "update", "-n", key, "-v", val)
+		_, err = b.exec.Run("iscsiadm", "-m", "iface", "-I", b.Iface, "-o", "update", "-n", key, "-v", val)
 		if err != nil {
-			b.exec.Run("iscsiadm", "-m", "iface", "-I", newIface, "-o", "delete")
-			lastErr = fmt.Errorf("iscsi: failed to update iface records: %s (%v). iface(%s) will be used", string(out), err, b.Iface)
+			b.exec.Run("iscsiadm", "-m", "iface", "-I", b.Iface, "-o", "delete")
+			lastErr = fmt.Errorf("iscsi: failed to update iface records: %s (%v). iface(%s) will be used", string(out), err, b.InitIface)
 			break
 		}
 	}

pkg/volume/iscsi/iscsi_util_test.go

@@ -276,11 +276,11 @@ func TestClonedIface(t *testing.T) {
 	plugin := plugins[0]
 	fakeMounter := iscsiDiskMounter{
 		iscsiDisk: &iscsiDisk{
+			Iface:  "192.168.1.10:pv0001",
 			plugin: plugin.(*iscsiPlugin)},
 		exec: fakeExec,
 	}
-	newIface := "192.168.1.10:pv0001"
-	cloneIface(fakeMounter, newIface)
+	cloneIface(fakeMounter)
 	if cmdCount != 4 {
 		t.Errorf("expected 4 CombinedOutput() calls, got %d", cmdCount)
 	}
@@ -305,11 +305,11 @@ func TestClonedIfaceShowError(t *testing.T) {
 	plugin := plugins[0]
 	fakeMounter := iscsiDiskMounter{
 		iscsiDisk: &iscsiDisk{
+			Iface:  "192.168.1.10:pv0001",
 			plugin: plugin.(*iscsiPlugin)},
 		exec: fakeExec,
 	}
-	newIface := "192.168.1.10:pv0001"
-	cloneIface(fakeMounter, newIface)
+	cloneIface(fakeMounter)
 	if cmdCount != 1 {
 		t.Errorf("expected 1 CombinedOutput() calls, got %d", cmdCount)
 	}
@@ -350,11 +350,11 @@ func TestClonedIfaceUpdateError(t *testing.T) {
 	plugin := plugins[0]
 	fakeMounter := iscsiDiskMounter{
 		iscsiDisk: &iscsiDisk{
+			Iface:  "192.168.1.10:pv0001",
 			plugin: plugin.(*iscsiPlugin)},
 		exec: fakeExec,
 	}
-	newIface := "192.168.1.10:pv0001"
-	cloneIface(fakeMounter, newIface)
+	cloneIface(fakeMounter)
 	if cmdCount != 5 {
 		t.Errorf("expected 5 CombinedOutput() calls, got %d", cmdCount)
 	}