Skip to content

Commit

Permalink
Log out from multiple portals with iscsi storage plugin
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
mtanino committed May 31, 2017
1 parent 7043372 commit aebaee3
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 47 deletions.
12 changes: 6 additions & 6 deletions pkg/volume/iscsi/iscsi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
132 changes: 91 additions & 41 deletions pkg/volume/iscsi/iscsi_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package iscsi

import (
"encoding/json"
"fmt"
"os"
"path"
Expand Down Expand Up @@ -45,15 +46,15 @@ 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))
}

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))
}
Expand All @@ -65,15 +66,15 @@ 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))
}

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))
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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
}
10 changes: 10 additions & 0 deletions pkg/volume/iscsi/iscsi_util_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package iscsi
import (
"os"
"path/filepath"
"reflect"
"testing"

"k8s.io/kubernetes/pkg/util/mount"
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit aebaee3

Please sign in to comment.