From aebaee39d596b00bb066d7d6ad4bc4a0f9dbff63 Mon Sep 17 00:00:00 2001 From: mtanino Date: Wed, 31 May 2017 15:52:00 -0400 Subject: [PATCH] Log out from multiple portals with iscsi storage plugin When using iscsi storage with multiple target portal (TP) addresses and multipathing the volume manager logs on to the IQN for all portal addresses, but when a pod gets destroyed the volume manager only logs out for the primary TP and sessions for another TPs are always remained. This patch adds methods to store and load iscsi disk configrations, then uses the stored config at DetachDisk path. Fix #45394 --- pkg/volume/iscsi/iscsi.go | 12 +-- pkg/volume/iscsi/iscsi_util.go | 132 +++++++++++++++++++--------- pkg/volume/iscsi/iscsi_util_test.go | 10 +++ 3 files changed, 107 insertions(+), 47 deletions(-) diff --git a/pkg/volume/iscsi/iscsi.go b/pkg/volume/iscsi/iscsi.go index 9293523c256b5..408f707cdc9eb 100644 --- a/pkg/volume/iscsi/iscsi.go +++ b/pkg/volume/iscsi/iscsi.go @@ -136,10 +136,10 @@ func (plugin *iscsiPlugin) newMounterInternal(spec *volume.Spec, podUID types.UI iscsiDisk: &iscsiDisk{ podUID: podUID, volName: spec.Name(), - portals: bkportal, - iqn: iscsi.IQN, + Portals: bkportal, + Iqn: iscsi.IQN, lun: lun, - iface: iface, + Iface: iface, chap_discovery: iscsi.DiscoveryCHAPAuth, chap_session: iscsi.SessionCHAPAuth, secret: secret, @@ -191,10 +191,10 @@ func (plugin *iscsiPlugin) ConstructVolumeSpec(volumeName, mountPath string) (*v type iscsiDisk struct { volName string podUID types.UID - portals []string - iqn string + Portals []string + Iqn string lun string - iface string + Iface string chap_discovery bool chap_session bool secret map[string]string diff --git a/pkg/volume/iscsi/iscsi_util.go b/pkg/volume/iscsi/iscsi_util.go index 36c2a12d72585..aced5285829ae 100755 --- a/pkg/volume/iscsi/iscsi_util.go +++ b/pkg/volume/iscsi/iscsi_util.go @@ -17,6 +17,7 @@ limitations under the License. package iscsi import ( + "encoding/json" "fmt" "os" "path" @@ -45,7 +46,7 @@ var ( func updateISCSIDiscoverydb(b iscsiDiskMounter, tp string) error { if b.chap_discovery { - out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.iface, "-o", "update", "-n", "discovery.sendtargets.auth.authmethod", "-v", "CHAP"}) + out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "update", "-n", "discovery.sendtargets.auth.authmethod", "-v", "CHAP"}) if err != nil { return fmt.Errorf("iscsi: failed to update discoverydb with CHAP, output: %v", string(out)) } @@ -53,7 +54,7 @@ func updateISCSIDiscoverydb(b iscsiDiskMounter, tp string) error { for _, k := range chap_st { v := b.secret[k] if len(v) > 0 { - out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.iface, "-o", "update", "-n", k, "-v", v}) + out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "update", "-n", k, "-v", v}) if err != nil { return fmt.Errorf("iscsi: failed to update discoverydb key %q with value %q error: %v", k, v, string(out)) } @@ -65,7 +66,7 @@ func updateISCSIDiscoverydb(b iscsiDiskMounter, tp string) error { func updateISCSINode(b iscsiDiskMounter, tp string) error { if b.chap_session { - out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.iqn, "-I", b.iface, "-o", "update", "-n", "node.session.auth.authmethod", "-v", "CHAP"}) + out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "-o", "update", "-n", "node.session.auth.authmethod", "-v", "CHAP"}) if err != nil { return fmt.Errorf("iscsi: failed to update node with CHAP, output: %v", string(out)) } @@ -73,7 +74,7 @@ func updateISCSINode(b iscsiDiskMounter, tp string) error { for _, k := range chap_sess { v := b.secret[k] if len(v) > 0 { - out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.iqn, "-I", b.iface, "-o", "update", "-n", k, "-v", v}) + out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "-o", "update", "-n", k, "-v", v}) if err != nil { return fmt.Errorf("iscsi: failed to update node session key %q with value %q error: %v", k, v, string(out)) } @@ -150,7 +151,36 @@ func makePDNameInternal(host volume.VolumeHost, portal string, iqn string, lun s type ISCSIUtil struct{} func (util *ISCSIUtil) MakeGlobalPDName(iscsi iscsiDisk) string { - return makePDNameInternal(iscsi.plugin.host, iscsi.portals[0], iscsi.iqn, iscsi.lun, iscsi.iface) + return makePDNameInternal(iscsi.plugin.host, iscsi.Portals[0], iscsi.Iqn, iscsi.lun, iscsi.Iface) +} + +func (util *ISCSIUtil) persistISCSI(conf iscsiDisk, mnt string) error { + file := path.Join(mnt, "iscsi.json") + fp, err := os.Create(file) + if err != nil { + return fmt.Errorf("iscsi: create %s err %s", file, err) + } + defer fp.Close() + encoder := json.NewEncoder(fp) + if err = encoder.Encode(conf); err != nil { + return fmt.Errorf("iscsi: encode err: %v.", err) + } + return nil +} + +func (util *ISCSIUtil) loadISCSI(conf *iscsiDisk, mnt string) error { + // NOTE: The iscsi config json is not deleted after logging out from target portals. + file := path.Join(mnt, "iscsi.json") + fp, err := os.Open(file) + if err != nil { + return fmt.Errorf("iscsi: open %s err %s", file, err) + } + defer fp.Close() + decoder := json.NewDecoder(fp) + if err = decoder.Decode(conf); err != nil { + return fmt.Errorf("iscsi: decode err: %v.", err) + } + return nil } func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error { @@ -159,45 +189,45 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error { var iscsiTransport string var lastErr error - out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "iface", "-I", b.iface, "-o", "show"}) + out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "iface", "-I", b.Iface, "-o", "show"}) if err != nil { - glog.Errorf("iscsi: could not read iface %s error: %s", b.iface, string(out)) + glog.Errorf("iscsi: could not read iface %s error: %s", b.Iface, string(out)) return err } iscsiTransport = extractTransportname(string(out)) - bkpPortal := b.portals + bkpPortal := b.Portals for _, tp := range bkpPortal { // Rescan sessions to discover newly mapped LUNs. Do not specify the interface when rescanning // to avoid establishing additional sessions to the same target. - out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.iqn, "-R"}) + out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.Iqn, "-R"}) if err != nil { glog.Errorf("iscsi: failed to rescan session with error: %s (%v)", string(out), err) } if iscsiTransport == "" { - glog.Errorf("iscsi: could not find transport name in iface %s", b.iface) - return fmt.Errorf("Could not parse iface file for %s", b.iface) + glog.Errorf("iscsi: could not find transport name in iface %s", b.Iface) + return fmt.Errorf("Could not parse iface file for %s", b.Iface) } else if iscsiTransport == "tcp" { - devicePath = strings.Join([]string{"/dev/disk/by-path/ip", tp, "iscsi", b.iqn, "lun", b.lun}, "-") + devicePath = strings.Join([]string{"/dev/disk/by-path/ip", tp, "iscsi", b.Iqn, "lun", b.lun}, "-") } else { - devicePath = strings.Join([]string{"/dev/disk/by-path/pci", "*", "ip", tp, "iscsi", b.iqn, "lun", b.lun}, "-") + devicePath = strings.Join([]string{"/dev/disk/by-path/pci", "*", "ip", tp, "iscsi", b.Iqn, "lun", b.lun}, "-") } exist := waitForPathToExist(devicePath, 1, iscsiTransport) if exist == false { // build discoverydb and discover iscsi target - b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.iface, "-o", "new"}) + b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "new"}) // update discoverydb with CHAP secret err = updateISCSIDiscoverydb(b, tp) if err != nil { lastErr = fmt.Errorf("iscsi: failed to update discoverydb to portal %s error: %v", tp, err) continue } - out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.iface, "--discover"}) + out, err := b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "--discover"}) if err != nil { // delete discoverydb record - b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.iface, "-o", "delete"}) + b.plugin.execCommand("iscsiadm", []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", b.Iface, "-o", "delete"}) lastErr = fmt.Errorf("iscsi: failed to sendtargets to portal %s output: %s, err %v", tp, string(out), err) continue } @@ -208,10 +238,10 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error { continue } // login to iscsi target - out, err = b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.iqn, "-I", b.iface, "--login"}) + out, err = b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-T", b.Iqn, "-I", b.Iface, "--login"}) if err != nil { // delete the node record from database - b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-I", b.iface, "-T", b.iqn, "-o", "delete"}) + b.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", tp, "-I", b.Iface, "-T", b.Iqn, "-o", "delete"}) lastErr = fmt.Errorf("iscsi: failed to attach disk: Error: %s (%v)", string(out), err) continue } @@ -251,6 +281,9 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) error { return err } + // Persist iscsi disk config to json file for DetachDisk path + util.persistISCSI(*(b.iscsiDisk), globalPDPath) + for _, path := range devicePaths { // There shouldnt be any empty device paths. However adding this check // for safer side to avoid the possibility of an empty entry. @@ -290,37 +323,41 @@ func (util *ISCSIUtil) DetachDisk(c iscsiDiskUnmounter, mntPath string) error { } refCount, err := getDevicePrefixRefCount(c.mounter, prefix) if err == nil && refCount == 0 { - // This portal/iqn/iface is no longer referenced, log out. - // Extract the portal and iqn from device path. - portal, iqn, err := extractPortalAndIqn(device) - if err != nil { - return err - } - // Extract the iface from the mountPath and use it to log out. If the iface - // is not found, maintain the previous behavior to facilitate kubelet upgrade. - // Logout may fail as no session may exist for the portal/IQN on the specified interface. - iface, found := extractIface(mntPath) - if found { - glog.Infof("iscsi: log out target %s iqn %s iface %s", portal, iqn, iface) - out, err := c.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", portal, "-T", iqn, "-I", iface, "--logout"}) + var bkpPortal []string + var iqn, iface string + found := true + + // load iscsi disk config from json file + if err := util.loadISCSI(c.iscsiDisk, mntPath); err == nil { + bkpPortal, iqn, iface = c.iscsiDisk.Portals, c.iscsiDisk.Iqn, c.iscsiDisk.Iface + } else { + // If the iscsi disk config is not found, fall back to the original behavior. + // This portal/iqn/iface is no longer referenced, log out. + // Extract the portal and iqn from device path. + bkpPortal[0], iqn, err = extractPortalAndIqn(device) if err != nil { - glog.Errorf("iscsi: failed to detach disk Error: %s", string(out)) + return err } - // Delete the node record - glog.Infof("iscsi: delete node record target %s iqn %s", portal, iqn) - out, err = c.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", portal, "-T", iqn, "-I", iface, "-o", "delete"}) - if err != nil { - glog.Errorf("iscsi: failed to delete node record Error: %s", string(out)) + // Extract the iface from the mountPath and use it to log out. If the iface + // is not found, maintain the previous behavior to facilitate kubelet upgrade. + // Logout may fail as no session may exist for the portal/IQN on the specified interface. + iface, found = extractIface(mntPath) + } + for _, portal := range removeDuplicate(bkpPortal) { + logout := []string{"-m", "node", "-p", portal, "-T", iqn, "--logout"} + delete := []string{"-m", "node", "-p", portal, "-T", iqn, "-o", "delete"} + if found { + logout = append(logout, []string{"-I", iface}...) + delete = append(delete, []string{"-I", iface}...) } - } else { - glog.Infof("iscsi: log out target %s iqn %s", portal, iqn) - out, err := c.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", portal, "-T", iqn, "--logout"}) + glog.Infof("iscsi: log out target %s iqn %s iface %s", portal, iqn, iface) + out, err := c.plugin.execCommand("iscsiadm", logout) if err != nil { glog.Errorf("iscsi: failed to detach disk Error: %s", string(out)) } // Delete the node record glog.Infof("iscsi: delete node record target %s iqn %s", portal, iqn) - out, err = c.plugin.execCommand("iscsiadm", []string{"-m", "node", "-p", portal, "-T", iqn, "-o", "delete"}) + out, err = c.plugin.execCommand("iscsiadm", delete) if err != nil { glog.Errorf("iscsi: failed to delete node record Error: %s", string(out)) } @@ -390,3 +427,16 @@ func extractPortalAndIqn(device string) (string, string, error) { iqn := device[ind2:ind] return portal, iqn, nil } + +// Remove duplicates or string +func removeDuplicate(s []string) []string { + m := map[string]bool{} + for _, v := range s { + if v != "" && !m[v] { + s[len(m)] = v + m[v] = true + } + } + s = s[:len(m)] + return s +} diff --git a/pkg/volume/iscsi/iscsi_util_test.go b/pkg/volume/iscsi/iscsi_util_test.go index a5f573dcbf5ba..50dc4e339119a 100755 --- a/pkg/volume/iscsi/iscsi_util_test.go +++ b/pkg/volume/iscsi/iscsi_util_test.go @@ -19,6 +19,7 @@ package iscsi import ( "os" "path/filepath" + "reflect" "testing" "k8s.io/kubernetes/pkg/util/mount" @@ -91,6 +92,15 @@ func TestExtractPortalAndIqn(t *testing.T) { } } +func TestRemoveDuplicate(t *testing.T) { + dupPortals := []string{"127.0.0.1:3260", "127.0.0.1:3260", "127.0.0.100:3260"} + portals := removeDuplicate(dupPortals) + want := []string{"127.0.0.1:3260", "127.0.0.100:3260"} + if reflect.DeepEqual(portals, want) == false { + t.Errorf("removeDuplicate: want: %s, got: %s", want, portals) + } +} + func fakeOsStat(devicePath string) (fi os.FileInfo, err error) { var cmd os.FileInfo return cmd, nil