diff --git a/Makefile b/Makefile index 26cf14c..9e3fde2 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: all build clean install +.PHONY: all build clean install test coverage all: clean build install @@ -16,3 +16,6 @@ install: test: go test ./iscsi/ +coverage: + go test ./iscsi -coverprofile=coverage.out + go tool cover -html=coverage.out diff --git a/example/main.go b/example/main.go index dc0f8eb..bdc309a 100644 --- a/example/main.go +++ b/example/main.go @@ -11,31 +11,29 @@ import ( ) var ( - portals = flag.String("portals", "192.168.1.112:3260", "Comma delimited. Eg: 1.1.1.1,2.2.2.2") - iqn = flag.String("iqn", "iqn.2010-10.org.openstack:volume-95739000-1557-44f8-9f40-e9d29fe6ec47", "") - multipath = flag.Bool("multipath", false, "") - username = flag.String("username", "3aX7EEf3CEgvESQG75qh", "") - password = flag.String("password", "eJBDC7Bt7WE3XFDq", "") - lun = flag.Int("lun", 1, "") - debug = flag.Bool("debug", false, "enable logging") + portals = flag.String("portals", "192.168.1.112:3260", "Comma delimited. Eg: 1.1.1.1,2.2.2.2") + iqn = flag.String("iqn", "iqn.2010-10.org.openstack:volume-95739000-1557-44f8-9f40-e9d29fe6ec47", "") + username = flag.String("username", "3aX7EEf3CEgvESQG75qh", "") + password = flag.String("password", "eJBDC7Bt7WE3XFDq", "") + lun = flag.Int("lun", 1, "") + debug = flag.Bool("debug", false, "enable logging") ) func main() { flag.Parse() - tgtp := strings.Split(*portals, ",") + tgtps := strings.Split(*portals, ",") if *debug { iscsi.EnableDebugLogging(os.Stdout) } // You can utilize the iscsiadm calls directly if you wish, but by creating a Connector // you can simplify interactions to simple calls like "Connect" and "Disconnect" - c := iscsi.Connector{ + c := &iscsi.Connector{ // Our example uses chap AuthType: "chap", - // Specify the target iqn we're dealing with - TargetIqn: *iqn, - // List of portals must be >= 1 (>1 signals multipath/mpio) - TargetPortals: tgtp, + // List of targets must be >= 1 (>1 signals multipath/mpio) + TargetIqn: *iqn, + TargetPortals: tgtps, // CHAP can be setup up for discovery as well as sessions, our example // device only uses CHAP security for sessions, for those that use Discovery // as well, we'd add a DiscoverySecrets entry the same way @@ -45,8 +43,6 @@ func main() { SecretsType: "chap"}, // Lun is the lun number the devices uses for exports Lun: int32(*lun), - // Multipath indicates that we want to configure this connection as a multipath device - Multipath: *multipath, // Number of times we check for device path, waiting for CheckInterval seconds inbetween each check (defaults to 10 if omitted) RetryCount: 11, // CheckInterval is the time in seconds to wait inbetween device path checks when logging in to a target @@ -55,21 +51,21 @@ func main() { // Now we can just issue a connection request using our Connector // A succesful connection will include the device path to access our iscsi volume - path, err := iscsi.Connect(c) + path, err := c.Connect() if err != nil { - log.Printf("Error returned from iscsi.Connect: %s", err.Error()) - os.Exit(1) - } - - if path == "" { - log.Printf("Failed to connect, didn't receive a path, but also no error!") + log.Printf("Error returned from c.Connect: %s", err.Error()) os.Exit(1) } log.Printf("Connected device at path: %s\n", path) time.Sleep(3 * time.Second) - // Disconnect is easy as well, we don't need the full Connector any more, just the Target IQN and the Portals - /// this should disconnect the volume as well as clear out the iscsi DB entries associated with it - iscsi.Disconnect(c.TargetIqn, c.TargetPortals) + // This will disconnect the volume + if err := c.DisconnectVolume(); err != nil { + log.Printf("Error returned from c.DisconnectVolume: %s", err.Error()) + os.Exit(1) + } + + // This will disconnect the session as well as clear out the iscsi DB entries associated with it + c.Disconnect() } diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..c358400 --- /dev/null +++ b/go.mod @@ -0,0 +1,8 @@ +module github.com/kubernetes-csi/csi-lib-iscsi + +go 1.15 + +require ( + github.com/prashantv/gostub v1.0.0 + github.com/stretchr/testify v1.7.0 +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..f0297e0 --- /dev/null +++ b/go.sum @@ -0,0 +1,13 @@ +github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/prashantv/gostub v1.0.0 h1:wTzvgO04xSS3gHuz6Vhuo0/kvWelyJxwNS0IRBPAwGY= +github.com/prashantv/gostub v1.0.0/go.mod h1:dP1v6T1QzyGJJKFocwAU0lSZKpfjstjH8TlhkEU0on0= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/iscsi/iscsi.go b/iscsi/iscsi.go index b70c9ce..e49ef2d 100644 --- a/iscsi/iscsi.go +++ b/iscsi/iscsi.go @@ -2,6 +2,7 @@ package iscsi import ( "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -19,14 +20,17 @@ import ( const defaultPort = "3260" var ( - debug *log.Logger - execCommand = exec.Command - execWithTimeout = ExecWithTimeout + debug *log.Logger + execCommand = exec.Command + execCommandContext = exec.CommandContext + execWithTimeout = ExecWithTimeout + osStat = os.Stat + filepathGlob = filepath.Glob + osOpenFile = os.OpenFile + sleep = time.Sleep ) -type statFunc func(string) (os.FileInfo, error) -type globFunc func(string) ([]string, error) - +// iscsiSession contains information avout an iSCSI session type iscsiSession struct { Protocol string ID int32 @@ -35,38 +39,48 @@ type iscsiSession struct { Name string } -type TargetInfo struct { - Iqn string `json:"iqn"` - Portal string `json:"portal"` - Port string `json:"port"` +type deviceInfo []Device + +// Device contains informations about a device +type Device struct { + Name string `json:"name"` + Hctl string `json:"hctl"` + Children []Device `json:"children"` + Type string `json:"type"` + Transport string `json:"tran"` + Size string `json:"size,omitempty"` +} + +type HCTL struct { + HBA int + Channel int + Target int + LUN int } -//Connector provides a struct to hold all of the needed parameters to make our iscsi connection +// Connector provides a struct to hold all of the needed parameters to make our iSCSI connection type Connector struct { - VolumeName string `json:"volume_name"` - Targets []TargetInfo `json:"targets"` - Lun int32 `json:"lun"` - AuthType string `json:"auth_type"` - DiscoverySecrets Secrets `json:"discovery_secrets"` - SessionSecrets Secrets `json:"session_secrets"` - Interface string `json:"interface"` - Multipath bool `json:"multipath"` - - // DevicePath is dm-x for a multipath device, and sdx for a normal device. - DevicePath string `json:"device_path"` - - RetryCount int32 `json:"retry_count"` - CheckInterval int32 `json:"check_interval"` - DoDiscovery bool `json:"do_discovery"` - DoCHAPDiscovery bool `json:"do_chap_discovery"` - TargetIqn string `json:"target_iqn"` - TargetPortals []string `json:"target_portals"` + VolumeName string `json:"volume_name"` + TargetIqn string `json:"target_iqn"` + TargetPortals []string `json:"target_portal"` + Lun int32 `json:"lun"` + AuthType string `json:"auth_type"` + DiscoverySecrets Secrets `json:"discovery_secrets"` + SessionSecrets Secrets `json:"session_secrets"` + Interface string `json:"interface"` + + MountTargetDevice *Device `json:"mount_target_device"` + Devices []Device `json:"devices"` + + RetryCount uint `json:"retry_count"` + CheckInterval uint `json:"check_interval"` + DoDiscovery bool `json:"do_discovery"` + DoCHAPDiscovery bool `json:"do_chap_discovery"` } func init() { // by default we don't log anything, EnableDebugLogging() can turn on some tracing debug = log.New(ioutil.Discard, "", 0) - } // EnableDebugLogging provides a mechanism to turn on debug logging for this package @@ -75,7 +89,7 @@ func EnableDebugLogging(writer io.Writer) { debug = log.New(writer, "DEBUG: ", log.Ldate|log.Ltime|log.Lshortfile) } -// parseSession takes the raw stdout from the iscsiadm -m session command and encodes it into an iscsi session type +// parseSession takes the raw stdout from the iscsiadm -m session command and encodes it into an iSCSI session type func parseSessions(lines string) []iscsiSession { entries := strings.Split(strings.TrimSpace(lines), "\n") r := strings.NewReplacer("[", "", @@ -104,6 +118,7 @@ func parseSessions(lines string) []iscsiSession { return sessions } +// sessionExists checks if an iSCSI session exists func sessionExists(tgtPortal, tgtIQN string) (bool, error) { sessions, err := getCurrentSessions() if err != nil { @@ -117,6 +132,7 @@ func sessionExists(tgtPortal, tgtIQN string) (bool, error) { return false, nil } +// extractTransportName returns a transport_name from getCurrentSessions output func extractTransportName(output string) string { res := regexp.MustCompile(`iface.transport_name = (.*)\n`).FindStringSubmatch(output) if res == nil { @@ -128,8 +144,8 @@ func extractTransportName(output string) string { return res[1] } +// getCurrentSessions list current iSCSI sessions func getCurrentSessions() ([]iscsiSession, error) { - out, err := GetSessions() if err != nil { exitErr, ok := err.(*exec.ExitError) @@ -142,96 +158,94 @@ func getCurrentSessions() ([]iscsiSession, error) { return sessions, err } -func waitForPathToExist(devicePath *string, maxRetries, intervalSeconds int, deviceTransport string) (bool, error) { - return waitForPathToExistImpl(devicePath, maxRetries, intervalSeconds, deviceTransport, os.Stat, filepath.Glob) -} +// waitForPathToExist wait for a file at a path to exists on disk +func waitForPathToExist(devicePath *string, maxRetries, intervalSeconds uint, deviceTransport string) error { + if devicePath == nil || *devicePath == "" { + return fmt.Errorf("unable to check unspecified devicePath") + } -func waitForPathToExistImpl(devicePath *string, maxRetries, intervalSeconds int, deviceTransport string, osStat statFunc, filepathGlob globFunc) (bool, error) { - if devicePath == nil { - return false, fmt.Errorf("unable to check unspecified devicePath") + for i := uint(0); i <= maxRetries; i++ { + if i != 0 { + debug.Printf("Device path %q doesn't exists yet, retrying in %d seconds (%d/%d)", *devicePath, intervalSeconds, i, maxRetries) + sleep(time.Second * time.Duration(intervalSeconds)) + } + + if err := pathExists(devicePath, deviceTransport); err == nil { + return nil + } else if !os.IsNotExist(err) { + return err + } } - var err error - for i := 0; i < maxRetries; i++ { - err = nil - if deviceTransport == "tcp" { - _, err = osStat(*devicePath) - if err != nil && !os.IsNotExist(err) { - debug.Printf("Error attempting to stat device: %s", err.Error()) - return false, err - } else if err != nil { - debug.Printf("Device not found for: %s", *devicePath) - } + return os.ErrNotExist +} - } else { - fpath, _ := filepathGlob(*devicePath) - if fpath == nil { - err = os.ErrNotExist - } else { - // There might be a case that fpath contains multiple device paths if - // multiple PCI devices connect to same iscsi target. We handle this - // case at subsequent logic. Pick up only first path here. - *devicePath = fpath[0] +// pathExists checks if a file at a path exists on disk +func pathExists(devicePath *string, deviceTransport string) error { + if deviceTransport == "tcp" { + _, err := osStat(*devicePath) + if err != nil { + if !os.IsNotExist(err) { + debug.Printf("Error attempting to stat device: %s", err.Error()) + return err } + debug.Printf("Device not found for: %s", *devicePath) + return err } - if err == nil { - return true, nil + } else { + fpath, err := filepathGlob(*devicePath) + + if err != nil { + return err } - if i == maxRetries-1 { - break + if fpath == nil { + return os.ErrNotExist } - time.Sleep(time.Second * time.Duration(intervalSeconds)) + // There might be a case that fpath contains multiple device paths if + // multiple PCI devices connect to same iscsi target. We handle this + // case at subsequent logic. Pick up only first path here. + *devicePath = fpath[0] } - return false, err + + return nil } -func getMultipathDisk(path string) (string, error) { - // Follow link to destination directory - debug.Printf("Checking for multipath device for path: %s", path) - devicePath, err := os.Readlink(path) - if err != nil { - debug.Printf("Failed reading link for multipath disk: %s -- error: %s\n", path, err.Error()) - return "", err - } - sdevice := filepath.Base(devicePath) - // If destination directory is already identified as a multipath device, - // just return its path - if strings.HasPrefix(sdevice, "dm-") { - debug.Printf("Already found multipath device: %s", sdevice) - return path, nil - } - // Fallback to iterating through all the entries under /sys/block/dm-* and - // check to see if any have an entry under /sys/block/dm-*/slaves matching - // the device the symlink was pointing at - dmPaths, err := filepath.Glob("/sys/block/dm-*") - if err != nil { - debug.Printf("Glob error: %s", err) - return "", err - } - for _, dmPath := range dmPaths { - sdevices, err := filepath.Glob(filepath.Join(dmPath, "slaves", "*")) - if err != nil { - debug.Printf("Glob error: %s", err) - } - for _, spath := range sdevices { - s := filepath.Base(spath) - debug.Printf("Basepath: %s", s) - if sdevice == s { - // We've found a matching entry, return the path for the - // dm-* device it was found under - p := filepath.Join("/dev", filepath.Base(dmPath)) - debug.Printf("Found matching multipath device: %s under dm-* device path %s", sdevice, dmPath) - return p, nil +// getMultipathDevice returns a multipath device for the configured targets if it exists +func getMultipathDevice(devices []Device) (*Device, error) { + var multipathDevice *Device + + for _, device := range devices { + if len(device.Children) != 1 { + multipathdNotRunning := "" + if len(device.Children) == 0 { + multipathdNotRunning = " (is multipathd running?)" } + return nil, fmt.Errorf("device is not mapped to exactly one multipath device%s: %v", multipathdNotRunning, device.Children) + } + if multipathDevice != nil && device.Children[0].Name != multipathDevice.Name { + return nil, fmt.Errorf("devices don't share a common multipath device: %v", devices) } + multipathDevice = &device.Children[0] + } + + if multipathDevice == nil { + return nil, fmt.Errorf("multipath device not found") } - debug.Printf("Couldn't find dm-* path for path: %s, found non dm-* path: %s", path, devicePath) - return "", fmt.Errorf("couldn't find dm-* path for path: %s, found non dm-* path: %s", path, devicePath) + + if multipathDevice.Type != "mpath" { + return nil, fmt.Errorf("device is not of mpath type: %v", multipathDevice) + } + + return multipathDevice, nil } -// Connect attempts to connect a volume to this node using the provided Connector info +// Connect is for backward-compatiblity with c.Connect() func Connect(c Connector) (string, error) { - var lastErr error + return c.Connect() +} + +// Connect attempts to connect a volume to this node using the provided Connector info +func (c *Connector) Connect() (string, error) { if c.RetryCount == 0 { c.RetryCount = 10 } @@ -239,11 +253,6 @@ func Connect(c Connector) (string, error) { c.CheckInterval = 1 } - if c.RetryCount < 0 || c.CheckInterval < 0 { - return "", fmt.Errorf("invalid RetryCount and CheckInterval combination, both must be positive integers. "+ - "RetryCount: %d, CheckInterval: %d", c.RetryCount, c.CheckInterval) - } - var devicePaths []string iFace := "default" if c.Interface != "" { iFace = c.Interface @@ -256,139 +265,179 @@ func Connect(c Connector) (string, error) { } iscsiTransport := extractTransportName(out) - for _, target := range c.Targets { - debug.Printf("process targetIqn: %s, portal: %s\n", target.Iqn, target.Portal) - baseArgs := []string{"-m", "node", "-T", target.Iqn, "-p", target.Portal} - // Rescan sessions to discover newly mapped LUNs. Do not specify the interface when rescanning - // to avoid establishing additional sessions to the same target. - if _, err := iscsiCmd(append(baseArgs, []string{"-R"}...)...); err != nil { - debug.Printf("failed to rescan session, err: %v", err) - } - - // create our devicePath that we'll be looking for based on the transport being used - port := defaultPort - if target.Port != "" { - port = target.Port - } - // portal with port - p := strings.Join([]string{target.Portal, port}, ":") - devicePath := strings.Join([]string{"/dev/disk/by-path/ip", p, "iscsi", target.Iqn, "lun", fmt.Sprint(c.Lun)}, "-") - if iscsiTransport != "tcp" { - devicePath = strings.Join([]string{"/dev/disk/by-path/pci", "*", "ip", p, "iscsi", target.Iqn, "lun", fmt.Sprint(c.Lun)}, "-") - } - - exists, _ := sessionExists(p, target.Iqn) - if exists { - if exists, err := waitForPathToExist(&devicePath, 1, 1, iscsiTransport); exists { - debug.Printf("Appending device path: %s", devicePath) - devicePaths = append(devicePaths, devicePath) - continue - } else if err != nil { - return "", err - } + var lastErr error + var devicePaths []string + for _, target := range c.TargetPortals { + devicePath, err := c.connectTarget(c.TargetIqn, target, iFace, iscsiTransport) + if err != nil { + lastErr = err + } else { + debug.Printf("Appending device path: %s", devicePath) + devicePaths = append(devicePaths, devicePath) } + } - if c.DoDiscovery { - // build discoverydb and discover iscsi target - if err := Discoverydb(p, iFace, c.DiscoverySecrets, c.DoCHAPDiscovery); err != nil { - debug.Printf("Error in discovery of the target: %s\n", err.Error()) - lastErr = err - continue - } + // GetISCSIDevices returns all devices if no paths are given + if len(devicePaths) < 1 { + c.Devices = []Device{} + } else if c.Devices, err = GetISCSIDevices(devicePaths, true); err != nil { + return "", err + } + + if len(c.Devices) < 1 { + iscsiCmd([]string{"-m", "iface", "-I", iFace, "-o", "delete"}...) + return "", fmt.Errorf("failed to find device path: %s, last error seen: %v", devicePaths, lastErr) + } + + mountTargetDevice, err := c.getMountTargetDevice() + c.MountTargetDevice = mountTargetDevice + if err != nil { + debug.Printf("Connect failed: %v", err) + RemoveSCSIDevices(c.Devices...) + c.MountTargetDevice = nil + c.Devices = []Device{} + return "", err + } + + if c.IsMultipathEnabled() { + if err := c.IsMultipathConsistent(); err != nil { + return "", fmt.Errorf("multipath is inconsistent: %v", err) } + } + + return c.MountTargetDevice.GetPath(), nil +} - if c.DoCHAPDiscovery { - // Make sure we don't log the secrets - err := CreateDBEntry(target.Iqn, p, iFace, c.DiscoverySecrets, c.SessionSecrets) +func (c *Connector) connectTarget(targetIqn string, target string, iFace string, iscsiTransport string) (string, error) { + debug.Printf("Process targetIqn: %s, portal: %s\n", targetIqn, target) + targetParts := strings.Split(target, ":") + targetPortal := targetParts[0] + targetPort := defaultPort + if len(targetParts) > 1 { + targetPort = targetParts[1] + } + baseArgs := []string{"-m", "node", "-T", targetIqn, "-p", targetPortal} + // Rescan sessions to discover newly mapped LUNs. Do not specify the interface when rescanning + // to avoid establishing additional sessions to the same target. + if _, err := iscsiCmd(append(baseArgs, []string{"-R"}...)...); err != nil { + debug.Printf("Failed to rescan session, err: %v", err) + if os.IsTimeout(err) { + debug.Printf("iscsiadm timeouted, logging out") + cmd := execCommand("iscsiadm", append(baseArgs, []string{"-u"}...)...) + out, err := cmd.CombinedOutput() if err != nil { - debug.Printf("Error creating db entry: %s\n", err.Error()) - continue + return "", fmt.Errorf("could not logout from target: %s", out) } } + } - // perform the login - err = Login(target.Iqn, p) - if err != nil { - debug.Printf("failed to login, err: %v", err) - lastErr = err - continue - } - retries := int(c.RetryCount / c.CheckInterval) - if exists, err := waitForPathToExist(&devicePath, retries, int(c.CheckInterval), iscsiTransport); exists { - devicePaths = append(devicePaths, devicePath) - continue - } else if err != nil { - lastErr = fmt.Errorf("couldn't attach disk, err: %v", err) + // create our devicePath that we'll be looking for based on the transport being used + // portal with port + portal := strings.Join([]string{targetPortal, targetPort}, ":") + devicePath := strings.Join([]string{"/dev/disk/by-path/ip", portal, "iscsi", targetIqn, "lun", fmt.Sprint(c.Lun)}, "-") + if iscsiTransport != "tcp" { + devicePath = strings.Join([]string{"/dev/disk/by-path/pci", "*", "ip", portal, "iscsi", targetIqn, "lun", fmt.Sprint(c.Lun)}, "-") + } + + exists, _ := sessionExists(portal, targetIqn) + if exists { + debug.Printf("Session already exists, checking if device path %q exists", devicePath) + if err := waitForPathToExist(&devicePath, c.RetryCount, c.CheckInterval, iscsiTransport); err != nil { + return "", err } + return devicePath, nil } - if len(devicePaths) < 1 { - iscsiCmd([]string{"-m", "iface", "-I", iFace, "-o", "delete"}...) - return "", fmt.Errorf("failed to find device path: %s, last error seen: %v", devicePaths, lastErr) + if err := c.discoverTarget(targetIqn, iFace, portal); err != nil { + return "", err } - if lastErr != nil { - debug.Printf("Last error occured during iscsi init: \n%v", lastErr) + // perform the login + err := Login(targetIqn, portal) + if err != nil { + debug.Printf("Failed to login: %v", err) + return "", err } - for i, path := range devicePaths { - if path != "" { - if mappedDevicePath, err := getMultipathDisk(path); mappedDevicePath != "" { - devicePaths[i] = mappedDevicePath - if err != nil { - return "", err - } - } + debug.Printf("Waiting for device path %q to exist", devicePath) + if err := waitForPathToExist(&devicePath, c.RetryCount, c.CheckInterval, iscsiTransport); err != nil { + return "", err + } + + return devicePath, nil +} + +func (c *Connector) discoverTarget(targetIqn string, iFace string, portal string) error { + if c.DoDiscovery { + // build discoverydb and discover iscsi target + if err := Discoverydb(portal, iFace, c.DiscoverySecrets, c.DoCHAPDiscovery); err != nil { + debug.Printf("Error in discovery of the target: %s\n", err.Error()) + return err } } - debug.Printf("After connect we're returning devicePaths: %s", devicePaths) - if len(devicePaths) > 0 { - return devicePaths[0], err + if c.DoCHAPDiscovery { + // Make sure we don't log the secrets + err := CreateDBEntry(targetIqn, portal, iFace, c.DiscoverySecrets, c.SessionSecrets) + if err != nil { + debug.Printf("Error creating db entry: %s\n", err.Error()) + return err + } } - return "", err + + return nil } -//Disconnect performs a disconnect operation on a volume -func Disconnect(tgtIqn string, portals []string) error { - err := Logout(tgtIqn, portals) - if err != nil { - return err +// Disconnect is for backward-compatibility with c.Disconnect() +func Disconnect(targetIqn string, targets []string) { + for _, target := range targets { + targetPortal := strings.Split(target, ":")[0] + Logout(targetIqn, targetPortal) } - err = DeleteDBEntry(tgtIqn) - return err + + deleted := map[string]bool{} + if _, ok := deleted[targetIqn]; ok { + return + } + deleted[targetIqn] = true + DeleteDBEntry(targetIqn) +} + +// Disconnect performs a disconnect operation from an appliance. +// Be sure to disconnect all deivces properly before doing this as it can result in data loss. +func (c *Connector) Disconnect() { + Disconnect(c.TargetIqn, c.TargetPortals) } // DisconnectVolume removes a volume from a Linux host. -func DisconnectVolume(c Connector) error { +func (c *Connector) DisconnectVolume() error { // Steps to safely remove an iSCSI storage volume from a Linux host are as following: // 1. Unmount the disk from a filesystem on the system. // 2. Flush the multipath map for the disk we’re removing (if multipath is enabled). // 3. Remove the physical disk entities that Linux maintains. // 4. Take the storage volume (disk) offline on the storage subsystem. - // 5. Rescan the iSCSI sessions. + // 5. Rescan the iSCSI sessions (after unmapping only). // // DisconnectVolume focuses on step 2 and 3. // Note: make sure the volume is already unmounted before calling this method. - debug.Printf("Disconnecting volume in path %s.\n", c.DevicePath) - if c.Multipath { - debug.Printf("Removing multipath device.\n") - devices, err := GetSysDevicesFromMultipathDevice(c.DevicePath) - if err != nil { - return err + if c.IsMultipathEnabled() { + if err := c.IsMultipathConsistent(); err != nil { + return fmt.Errorf("multipath is inconsistent: %v", err) } - err = FlushMultipathDevice(c.DevicePath) + + debug.Printf("Removing multipath device in path %s.\n", c.MountTargetDevice.GetPath()) + err := FlushMultipathDevice(c.MountTargetDevice) if err != nil { return err } - debug.Printf("Found multipath slaves %v, removing all of them.\n", devices) - if err := RemovePhysicalDevice(devices...); err != nil { + if err := RemoveSCSIDevices(c.Devices...); err != nil { return err } } else { - debug.Printf("Removing normal device.\n") - if err := RemovePhysicalDevice(c.DevicePath); err != nil { + devicePath := c.MountTargetDevice.GetPath() + debug.Printf("Removing normal device in path %s.\n", devicePath) + if err := RemoveSCSIDevices(*c.MountTargetDevice); err != nil { return err } } @@ -397,49 +446,204 @@ func DisconnectVolume(c Connector) error { return nil } -// RemovePhysicalDevice removes device(s) sdx from a Linux host. -func RemovePhysicalDevice(devices ...string) error { - debug.Printf("Removing scsi device %v.\n", devices) - var errs []error - for _, deviceName := range devices { - if deviceName == "" { +// getMountTargetDevice returns the device to be mounted among the configured devices +func (c *Connector) getMountTargetDevice() (*Device, error) { + if len(c.Devices) > 1 { + multipathDevice, err := getMultipathDevice(c.Devices) + if err != nil { + debug.Printf("mount target is not a multipath device: %v", err) + return nil, err + } + debug.Printf("mount target is a multipath device") + return multipathDevice, nil + } + + if len(c.Devices) == 0 { + return nil, fmt.Errorf("could not find mount target device: connector does not contain any device") + } + + return &c.Devices[0], nil +} + +// IsMultipathEnabled check if multipath is enabled on devices handled by this connector +func (c *Connector) IsMultipathEnabled() bool { + return c.MountTargetDevice.Type == "mpath" +} + +// GetSCSIDevices get SCSI devices from device paths +// It will returns all SCSI devices if no paths are given +func GetSCSIDevices(devicePaths []string, strict bool) ([]Device, error) { + debug.Printf("Getting info about SCSI devices %s.\n", devicePaths) + + deviceInfo, err := lsblk(devicePaths, strict) + if err != nil { + debug.Printf("An error occured while looking info about SCSI devices: %v", err) + return nil, err + } + + return deviceInfo, nil +} + +// GetISCSIDevices get iSCSI devices from device paths +// It will returns all iSCSI devices if no paths are given +func GetISCSIDevices(devicePaths []string, strict bool) (devices []Device, err error) { + scsiDevices, err := GetSCSIDevices(devicePaths, strict) + if err != nil { + return + } + + for i := range scsiDevices { + device := &scsiDevices[i] + if device.Transport == "iscsi" { + devices = append(devices, *device) + } + } + + return +} + +// lsblk execute the lsblk commands +func lsblk(devicePaths []string, strict bool) (deviceInfo, error) { + flags := []string{"-rn", "-o", "NAME,KNAME,PKNAME,HCTL,TYPE,TRAN,SIZE"} + command := execCommand("lsblk", append(flags, devicePaths...)...) + debug.Println(command.String()) + out, err := command.Output() + if err != nil { + if ee, ok := err.(*exec.ExitError); ok { + err = fmt.Errorf("%s, (%w)", strings.Trim(string(ee.Stderr), "\n"), ee) + if strict || ee.ExitCode() != 64 { // ignore the error if some devices have been found when not strict + return nil, err + } + debug.Printf("Could find only some devices: %v", err) + } else { + return nil, err + } + } + + var devices []*Device + devicesMap := make(map[string]*Device) + pkNames := []string{} + + // Parse devices + lines := strings.Split(strings.Trim(string(out), "\n"), "\n") + for _, line := range lines { + columns := strings.Split(line, " ") + if len(columns) < 5 { + return nil, fmt.Errorf("invalid output from lsblk: %s", line) + } + device := &Device{ + Name: columns[0], + Hctl: columns[3], + Type: columns[4], + Transport: columns[5], + Size: columns[6], + } + devices = append(devices, device) + pkNames = append(pkNames, columns[2]) + devicesMap[columns[1]] = device + } + + // Reconstruct devices tree + for i, pkName := range pkNames { + if pkName == "" { continue } + device := devices[i] + parent, ok := devicesMap[pkName] + if !ok { + return nil, fmt.Errorf("invalid output from lsblk: parent device %q not found", pkName) + } + if parent.Children == nil { + parent.Children = []Device{} + } + parent.Children = append(devicesMap[pkName].Children, *device) + } - debug.Printf("Delete scsi device %v.\n", deviceName) - // Remove a scsi device by executing 'echo "1" > /sys/block/sdx/device/delete - filename := filepath.Join(sysBlockPath, deviceName, "device", "delete") - if f, err := os.OpenFile(filename, os.O_TRUNC|os.O_WRONLY, 0200); err != nil { - if os.IsNotExist(err) { - continue - } else { - debug.Printf("Error while opening file %v: %v\n", filename, err) + // Filter devices to keep only the roots of the tree + var deviceInfo deviceInfo + for i, device := range devices { + if pkNames[i] == "" { + deviceInfo = append(deviceInfo, *device) + } + } + + return deviceInfo, nil +} + +// writeInSCSIDeviceFile write into special devices files to change devices state +func writeInSCSIDeviceFile(hctl string, file string, content string) error { + filename := filepath.Join("/sys/class/scsi_device", hctl, "device", file) + debug.Printf("Write %q in %q.\n", content, filename) + + f, err := osOpenFile(filename, os.O_TRUNC|os.O_WRONLY, 0200) + if err != nil { + debug.Printf("Error while opening file %v: %v\n", filename, err) + return err + } + + defer f.Close() + if _, err := f.WriteString(content); err != nil { + debug.Printf("Error while writing to file %v: %v", filename, err) + return err + } + + return nil +} + +// RemoveSCSIDevices removes SCSI device(s) from a Linux host. +func RemoveSCSIDevices(devices ...Device) error { + debug.Printf("Removing SCSI devices %v.\n", devices) + + var errs []error + for _, device := range devices { + debug.Printf("Flush SCSI device %v.\n", device.Name) + if err := device.Exists(); err == nil { + out, err := execCommand("blockdev", "--flushbufs", device.GetPath()).CombinedOutput() + if err != nil { + debug.Printf("Command 'blockdev --flushbufs %s' did not succeed to flush the device: %v\n", device.GetPath(), err) + return errors.New(string(out)) + } + } else if !os.IsNotExist(err) { + return err + } + + debug.Printf("Put SCSI device %q offline.\n", device.Name) + err := device.Shutdown() + if err != nil { + if !os.IsNotExist(err) { // Ignore device already removed errs = append(errs, err) - continue } - } else { - defer f.Close() - if _, err := f.WriteString("1"); err != nil { - debug.Printf("Error while writing to file %v: %v", filename, err) + continue + } + + debug.Printf("Delete SCSI device %q.\n", device.Name) + err = device.Delete() + if err != nil { + if !os.IsNotExist(err) { // Ignore device already removed errs = append(errs, err) - continue } + continue } } if len(errs) > 0 { return errs[0] } - debug.Println("Finshed removing SCSI devices.") + debug.Println("Finished removing SCSI devices.") return nil } -// PersistConnector persists the provided Connector to the specified file (ie /var/lib/pfile/myConnector.json) +// PersistConnector is for backward-compatibility with c.Persist() func PersistConnector(c *Connector, filePath string) error { + return c.Persist(filePath) +} + +// Persist persists the Connector to the specified file (ie /var/lib/pfile/myConnector.json) +func (c *Connector) Persist(filePath string) error { //file := path.Join("mnt", c.VolumeName+".json") f, err := os.Create(filePath) if err != nil { - return fmt.Errorf("error creating iscsi persistence file %s: %s", filePath, err) + return fmt.Errorf("error creating iSCSI persistence file %s: %s", filePath, err) } defer f.Close() encoder := json.NewEncoder(f) @@ -447,22 +651,151 @@ func PersistConnector(c *Connector, filePath string) error { return fmt.Errorf("error encoding connector: %v", err) } return nil - } // GetConnectorFromFile attempts to create a Connector using the specified json file (ie /var/lib/pfile/myConnector.json) func GetConnectorFromFile(filePath string) (*Connector, error) { f, err := ioutil.ReadFile(filePath) if err != nil { - return &Connector{}, err + return nil, err + } + c := Connector{} + err = json.Unmarshal([]byte(f), &c) + if err != nil { + return nil, err + } + devicePaths := []string{} + for _, device := range c.Devices { + devicePaths = append(devicePaths, device.GetPath()) } - data := Connector{} - err = json.Unmarshal(f, &data) + + if devices, err := GetSCSIDevices([]string{c.MountTargetDevice.GetPath()}, false); err != nil { + return nil, err + } else { + c.MountTargetDevice = &devices[0] + } + + if c.Devices, err = GetSCSIDevices(devicePaths, false); err != nil { + return nil, err + } + + return &c, nil +} + +// IsMultipathConsistent check if the currently used device is using a consistent multipath mapping +func (c *Connector) IsMultipathConsistent() error { + devices := append([]Device{*c.MountTargetDevice}, c.Devices...) + + referenceLUN := struct { + LUN int + Name string + }{LUN: -1, Name: ""} + HBA := map[int]string{} + referenceDevice := devices[0] + for _, device := range devices { + if device.Size != referenceDevice.Size { + return fmt.Errorf("devices size differ: %s (%s) != %s (%s)", device.Name, device.Size, referenceDevice.Name, referenceDevice.Size) + } + + if device.Type != "mpath" { + hctl, err := device.HCTL() + if err != nil { + return err + } + if referenceLUN.LUN == -1 { + referenceLUN.LUN = hctl.LUN + referenceLUN.Name = device.Name + } else if hctl.LUN != referenceLUN.LUN { + return fmt.Errorf("devices LUNs differ: %s (%d) != %s (%d)", device.Name, hctl.LUN, referenceLUN.Name, referenceLUN.LUN) + } + + if name, ok := HBA[hctl.HBA]; !ok { + HBA[hctl.HBA] = device.Name + } else { + return fmt.Errorf("two devices are using the same controller (%d): %s and %s", hctl.HBA, device.Name, name) + } + } + + wwid, err := device.WWID() + if err != nil { + return fmt.Errorf("could not find WWID for device %s: %v", device.Name, err) + } + if wwid != referenceDevice.Name { + return fmt.Errorf("devices WWIDs differ: %s (wwid:%s) != %s (wwid:%s)", device.Name, wwid, referenceDevice.Name, referenceDevice.Name) + } + } + + return nil +} + +// Exists check if the device exists at its path and returns an error otherwise +func (d *Device) Exists() error { + _, err := osStat(d.GetPath()) + return err +} + +// GetPath returns the path of a device +func (d *Device) GetPath() string { + if d.Type == "mpath" { + return filepath.Join("/dev/mapper", d.Name) + } + + return filepath.Join("/dev", d.Name) +} + +// WWID returns the WWID of a device +func (d *Device) WWID() (string, error) { + timeout := 1 * time.Second + out, err := execWithTimeout("scsi_id", []string{"-g", "-u", d.GetPath()}, timeout) if err != nil { - return &Connector{}, err + return "", err + } + + return string(out[:len(out)-1]), nil +} + +// HCTL returns the HCTL of a device +func (d *Device) HCTL() (*HCTL, error) { + var hctl []int + + for _, idstr := range strings.Split(d.Hctl, ":") { + id, err := strconv.Atoi(idstr) + if err != nil { + hctl = []int{} + break + } + hctl = append(hctl, id) + } + + if len(hctl) != 4 { + return nil, fmt.Errorf("invalid HCTL (%s) for device %q", d.Hctl, d.Name) } - return &data, nil + return &HCTL{ + HBA: hctl[0], + Channel: hctl[1], + Target: hctl[2], + LUN: hctl[3], + }, nil +} + +// WriteDeviceFile write in a device file +func (d *Device) WriteDeviceFile(name string, content string) error { + return writeInSCSIDeviceFile(d.Hctl, name, content) +} + +// Shutdown turn off an SCSI device by writing offline\n in /sys/class/scsi_device/h:c:t:l/device/state +func (d *Device) Shutdown() error { + return d.WriteDeviceFile("state", "offline\n") +} + +// Delete detach an SCSI device by writing 1 in /sys/class/scsi_device/h:c:t:l/device/delete +func (d *Device) Delete() error { + return d.WriteDeviceFile("delete", "1") +} +// Rescan rescan an SCSI device by writing 1 in /sys/class/scsi_device/h:c:t:l/device/rescan +func (d *Device) Rescan() error { + return d.WriteDeviceFile("rescan", "1") } diff --git a/iscsi/iscsi_test.go b/iscsi/iscsi_test.go index dceec27..3ef70d1 100644 --- a/iscsi/iscsi_test.go +++ b/iscsi/iscsi_test.go @@ -2,6 +2,8 @@ package iscsi import ( "context" + "encoding/json" + "errors" "fmt" "io/ioutil" "os" @@ -9,11 +11,24 @@ import ( "path/filepath" "reflect" "strconv" + "strings" "testing" "time" + + "github.com/prashantv/gostub" + "github.com/stretchr/testify/assert" ) -var nodeDB = ` +type testWriter struct { + data *[]byte +} + +func (w testWriter) Write(data []byte) (n int, err error) { + *w.data = append(*w.data, data...) + return len(data), nil +} + +const nodeDB = ` # BEGIN RECORD 6.2.0.874 node.name = iqn.2010-10.org.openstack:volume-eb393993-73d0-4e39-9ef4-b5841e244ced node.tpgt = -1 @@ -80,37 +95,50 @@ node.conn[0].iscsi.OFMarker = No # END RECORD ` -var emptyTransportName = "iface.transport_name = \n" -var emptyDbRecord = "\n\n\n" -var testCmdOutput = "" -var testCmdTimeout = false -var testCmdError error -var testExecWithTimeoutError error -var mockedExitStatus = 0 -var mockedStdout string - -var normalDevice = "sda" -var multipathDevice = "dm-1" -var slaves = []string{"sdb", "sdc"} - -type testCmdRunner struct{} - -func fakeExecCommand(command string, args ...string) *exec.Cmd { - cs := []string{"-test.run=TestExecCommandHelper", "--", command} - cs = append(cs, args...) - cmd := exec.Command(os.Args[0], cs...) - es := strconv.Itoa(mockedExitStatus) - cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1", - "STDOUT=" + mockedStdout, - "EXIT_STATUS=" + es} - return cmd +const emptyTransportName = "iface.transport_name = \n" +const emptyDbRecord = "\n\n\n" +const testRootFS = "/tmp/iscsi-tests" + +func makeFakeExecCommand(exitStatus int, stdout string) func(string, ...string) *exec.Cmd { + return func(command string, args ...string) *exec.Cmd { + cs := []string{"-test.run=TestExecCommandHelper", "--", command} + cs = append(cs, args...) + cmd := exec.Command(os.Args[0], cs...) + es := strconv.Itoa(exitStatus) + cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1", + "STDOUT=" + stdout, + "EXIT_STATUS=" + es} + return cmd + } } -func fakeExecWithTimeout(command string, args []string, timeout time.Duration) ([]byte, error) { - if testCmdTimeout { - return nil, context.DeadlineExceeded +func makeFakeExecCommandContext(exitStatus int, stdout string) func(context.Context, string, ...string) *exec.Cmd { + return func(ctx context.Context, command string, args ...string) *exec.Cmd { + return makeFakeExecCommand(exitStatus, stdout)(command, args...) } - return []byte(testCmdOutput), testExecWithTimeoutError +} + +func makeFakeExecWithTimeout(withTimeout bool, output []byte, err error) func(string, []string, time.Duration) ([]byte, error) { + return func(command string, args []string, timeout time.Duration) ([]byte, error) { + if withTimeout { + return nil, context.DeadlineExceeded + } + return output, err + } +} + +func marshalDeviceInfo(d *deviceInfo) string { + var output string + pkNames := map[string]string{} + for _, device := range *d { + for _, child := range device.Children { + pkNames[child.Name] = device.Name + } + } + for _, device := range *d { + output += fmt.Sprintf("%s %s %s %s %s %s %s\n", device.Name, device.Name, pkNames[device.Name], device.Hctl, device.Type, device.Transport, device.Size) + } + return output } func TestExecCommandHelper(t *testing.T) { @@ -123,36 +151,37 @@ func TestExecCommandHelper(t *testing.T) { os.Exit(i) } -func (tr testCmdRunner) execCmd(cmd string, args ...string) (string, error) { - return testCmdOutput, testCmdError - +func getDevicePath(device *Device) string { + sysDevicePath := "/tmp/iscsi-tests/sys/class/scsi_device/" + return filepath.Join(sysDevicePath, device.Hctl, "device") } -func preparePaths(sysBlockPath string) error { - for _, d := range append(slaves, normalDevice) { - devicePath := filepath.Join(sysBlockPath, d, "device") - err := os.MkdirAll(devicePath, os.ModePerm) - if err != nil { - return err - } - err = ioutil.WriteFile(filepath.Join(devicePath, "delete"), []byte(""), 0600) - if err != nil { +func preparePaths(devices []Device) error { + for _, d := range devices { + devicePath := getDevicePath(&d) + + if err := os.MkdirAll(devicePath, os.ModePerm); err != nil { return err } - } - for _, s := range slaves { - err := os.MkdirAll(filepath.Join(sysBlockPath, multipathDevice, "slaves", s), os.ModePerm) - if err != nil { - return err + + for _, filename := range []string{"delete", "state"} { + if err := ioutil.WriteFile(filepath.Join(devicePath, filename), []byte(""), 0600); err != nil { + return err + } } } - err := os.MkdirAll(filepath.Join(sysBlockPath, "dev", multipathDevice), os.ModePerm) - if err != nil { - return err - } return nil +} +func checkFileContents(t *testing.T, path string, contents string) { + if out, err := ioutil.ReadFile(path); err != nil { + t.Errorf("could not read file: %v", err) + return + } else if string(out) != contents { + t.Errorf("file content mismatch, got = %q, want = %q", string(out), contents) + return + } } func Test_parseSessions(t *testing.T) { @@ -232,9 +261,9 @@ func Test_extractTransportName(t *testing.T) { } func Test_sessionExists(t *testing.T) { - mockedExitStatus = 0 - mockedStdout = "tcp: [4] 192.168.1.107:3260,1 iqn.2010-10.org.openstack:volume-eb393993-73d0-4e39-9ef4-b5841e244ced (non-flash)\n" - execCommand = fakeExecCommand + fakeOutput := "tcp: [4] 192.168.1.107:3260,1 iqn.2010-10.org.openstack:volume-eb393993-73d0-4e39-9ef4-b5841e244ced (non-flash)\n" + defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(fakeOutput), nil)).Reset() + type args struct { tgtPortal string tgtIQN string @@ -272,49 +301,40 @@ func Test_sessionExists(t *testing.T) { } func Test_DisconnectNormalVolume(t *testing.T) { - - tmpDir, err := ioutil.TempDir("", "") - if err != nil { - t.Errorf("can not create temp directory: %v", err) - return - } - sysBlockPath = tmpDir - defer os.RemoveAll(tmpDir) - - err = preparePaths(tmpDir) - if err != nil { - t.Errorf("can not create temp directories and files: %v", err) - return - } - - execWithTimeout = fakeExecWithTimeout - devicePath := normalDevice + deleteDeviceFile := "/tmp/deleteDevice" + defer gostub.Stub(&osOpenFile, func(name string, flag int, perm os.FileMode) (*os.File, error) { + return os.OpenFile(deleteDeviceFile, flag, perm) + }).Reset() tests := []struct { - name string - removeDevice bool - wantErr bool + name string + withDeviceFile bool + wantErr bool }{ - {"DisconnectNormalVolume", false, false}, - {"DisconnectNonexistentNormalVolume", true, false}, + {"DisconnectNormalVolume", true, false}, + {"DisconnectNonexistentNormalVolume", false, false}, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.removeDevice { - os.RemoveAll(filepath.Join(sysBlockPath, devicePath)) + if tt.withDeviceFile { + os.Create(deleteDeviceFile) + } else { + os.RemoveAll(testRootFS) } - c := Connector{Multipath: false, DevicePath: devicePath} - err := DisconnectVolume(c) + + device := Device{Name: "test"} + c := Connector{Devices: []Device{device}, MountTargetDevice: &device} + err := c.DisconnectVolume() if (err != nil) != tt.wantErr { t.Errorf("DisconnectVolume() error = %v, wantErr %v", err, tt.wantErr) return } - if !tt.removeDevice { - deleteFile := filepath.Join(sysBlockPath, devicePath, "device", "delete") - out, err := ioutil.ReadFile(deleteFile) + if tt.withDeviceFile { + out, err := ioutil.ReadFile(deleteDeviceFile) if err != nil { - t.Errorf("can not read file %v: %v", deleteFile, err) + t.Errorf("can not read file %v: %v", deleteDeviceFile, err) return } if string(out) != "1" { @@ -327,72 +347,508 @@ func Test_DisconnectNormalVolume(t *testing.T) { } func Test_DisconnectMultipathVolume(t *testing.T) { - - execWithTimeout = fakeExecWithTimeout - devicePath := multipathDevice + defer gostub.Stub(&osStat, func(name string) (os.FileInfo, error) { + return nil, nil + }).Reset() tests := []struct { - name string - timeout bool - removeDevice bool - wantErr bool - cmdError error + name string + timeout bool + withDeviceFile bool + wantErr bool }{ - {"DisconnectMultipathVolume", false, false, false, nil}, - {"DisconnectMultipathVolumeFlushFailed", false, false, true, fmt.Errorf("error")}, - {"DisconnectMultipathVolumeFlushTimeout", true, false, true, nil}, - {"DisconnectNonexistentMultipathVolume", false, true, false, fmt.Errorf("error")}, + {"DisconnectMultipathVolume", false, true, false}, + {"DisconnectMultipathVolumeFlushTimeout", true, true, true}, + {"DisconnectNonexistentMultipathVolume", false, false, false}, } + + wwid := "3600c0ff0000000000000000000000000" + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - - tmpDir, err := ioutil.TempDir("", "") - if err != nil { - t.Errorf("can not create temp directory: %v", err) - return + defer gostub.Stub(&execWithTimeout, func(cmd string, args []string, timeout time.Duration) ([]byte, error) { + mockedOutput := []byte("") + if cmd == "scsi_id" { + mockedOutput = []byte(wwid + "\n") + } + return makeFakeExecWithTimeout(tt.timeout, mockedOutput, nil)(cmd, args, timeout) + }).Reset() + c := Connector{ + Devices: []Device{{Hctl: "0:0:0:0"}, {Hctl: "1:0:0:0"}}, + MountTargetDevice: &Device{Name: wwid, Type: "mpath"}, } - sysBlockPath = tmpDir - devPath = filepath.Join(tmpDir, "dev") - defer os.RemoveAll(tmpDir) - err = preparePaths(tmpDir) - if err != nil { - t.Errorf("can not create temp directories and files: %v", err) - return - } - testExecWithTimeoutError = tt.cmdError - testCmdTimeout = tt.timeout - if tt.removeDevice { - os.RemoveAll(filepath.Join(sysBlockPath, devicePath)) - os.RemoveAll(devPath) + defer gostub.Stub(&osOpenFile, func(name string, flag int, perm os.FileMode) (*os.File, error) { + return os.OpenFile(testRootFS+name, flag, perm) + }).Reset() + + defer gostub.Stub(&execCommand, makeFakeExecCommand(0, wwid)).Reset() + + if tt.withDeviceFile { + if err := preparePaths(c.Devices); err != nil { + t.Errorf("could not prepare paths: %v", err) + return + } + } else { + os.Remove(testRootFS) } - c := Connector{Multipath: true, DevicePath: devicePath} - err = DisconnectVolume(c) + + err := c.DisconnectVolume() if (err != nil) != tt.wantErr { t.Errorf("DisconnectVolume() error = %v, wantErr %v", err, tt.wantErr) return } + if tt.timeout { - if err != context.DeadlineExceeded { - t.Errorf("DisconnectVolume() error = %v, wantErr %v", err, context.DeadlineExceeded) - return + assert.New(t).Contains(err.Error(), "context deadline exceeded") + } + + if tt.withDeviceFile && !tt.wantErr { + for _, device := range c.Devices { + checkFileContents(t, getDevicePath(&device)+"/delete", "1") + checkFileContents(t, getDevicePath(&device)+"/state", "offline\n") } } + }) + } +} - if !tt.removeDevice && !tt.wantErr { - for _, s := range slaves { - deleteFile := filepath.Join(sysBlockPath, s, "device", "delete") - out, err := ioutil.ReadFile(deleteFile) - if err != nil { - t.Errorf("can not read file %v: %v", deleteFile, err) - return - } - if string(out) != "1" { - t.Errorf("file content mismatch, got = %s, want = 1", string(out)) - return - } +func Test_EnableDebugLogging(t *testing.T) { + assert := assert.New(t) + data := []byte{} + writer := testWriter{data: &data} + EnableDebugLogging(writer) + + assert.Equal("", string(data)) + assert.Len(strings.Split(string(data), "\n"), 1) + + debug.Print("testing debug logs") + assert.Contains(string(data), "testing debug logs") + assert.Len(strings.Split(string(data), "\n"), 2) +} + +func Test_waitForPathToExist(t *testing.T) { + tests := map[string]struct { + attempts int + fileNotFound bool + withErr bool + transport string + }{ + "Basic": { + attempts: 1, + }, + "WithRetry": { + attempts: 2, + }, + "WithRetryFail": { + attempts: 3, + fileNotFound: true, + }, + "WithError": { + withErr: true, + }, + } + + for name, tt := range tests { + tt.transport = "tcp" + tests[name+"OverTCP"] = tt + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + attempts := 0 + maxRetries := tt.attempts - 1 + if tt.fileNotFound { + maxRetries-- + } + if maxRetries < 0 { + maxRetries = 0 + } + doAttempt := func(err error) error { + attempts++ + if tt.withErr { + return err } + if attempts < tt.attempts { + return os.ErrNotExist + } + return nil + } + defer gostub.Stub(&osStat, func(name string) (os.FileInfo, error) { + if err := doAttempt(os.ErrPermission); err != nil { + return nil, err + } + return nil, nil + }).Reset() + defer gostub.Stub(&filepathGlob, func(name string) ([]string, error) { + if err := doAttempt(filepath.ErrBadPattern); err != nil { + return nil, err + } + return []string{"/somefilewithalongname"}, nil + }).Reset() + defer gostub.Stub(&sleep, func(_ time.Duration) {}).Reset() + path := "/somefile" + err := waitForPathToExist(&path, uint(maxRetries), 1, tt.transport) + + if tt.withErr { + if tt.transport == "tcp" { + assert.Equal(os.ErrPermission, err) + } else { + assert.Equal(filepath.ErrBadPattern, err) + } + return + } + if tt.fileNotFound { + assert.Equal(os.ErrNotExist, err) + assert.Equal(maxRetries, attempts-1) + } else { + assert.Nil(err) + assert.Equal(tt.attempts, attempts) + if tt.transport == "tcp" { + assert.Equal("/somefile", path) + } else { + assert.Equal("/somefilewithalongname", path) + } + } + }) + } + + t.Run("PathEmptyOrNil", func(t *testing.T) { + assert := assert.New(t) + path := "" + + err := waitForPathToExist(&path, 0, 0, "tcp") + assert.NotNil(err) + + err = waitForPathToExist(&path, 0, 0, "") + assert.NotNil(err) + + err = waitForPathToExist(nil, 0, 0, "tcp") + assert.NotNil(err) + + err = waitForPathToExist(nil, 0, 0, "") + assert.NotNil(err) + }) + + t.Run("PathNotFound", func(t *testing.T) { + assert := assert.New(t) + defer gostub.Stub(&filepathGlob, func(name string) ([]string, error) { + return nil, nil + }).Reset() + + path := "/test" + err := waitForPathToExist(&path, 0, 0, "") + assert.NotNil(err) + assert.Equal(os.ErrNotExist, err) + }) +} + +func Test_getMultipathDevice(t *testing.T) { + mpath1 := Device{Name: "3600c0ff0000000000000000000000000", Type: "mpath"} + mpath2 := Device{Name: "3600c0ff1111111111111111111111111", Type: "mpath"} + sda := Device{Name: "sda", Children: []Device{{Name: "sda1"}}} + sdb := Device{Name: "sdb", Children: []Device{mpath1}} + sdc := Device{Name: "sdc", Children: []Device{mpath1}} + sdd := Device{Name: "sdc", Children: []Device{mpath2}} + sde := Device{Name: "sdc", Children: []Device{mpath1, mpath2}} + + tests := map[string]struct { + mockedDevices []Device + multipathDevice *Device + wantErr bool + }{ + "Basic": { + mockedDevices: []Device{sdb, sdc}, + multipathDevice: &mpath1, + }, + "NotSharingTheSameMultipathDevice": { + mockedDevices: []Device{sdb, sdd}, + wantErr: true, + }, + "MoreThanOneMultipathDevice": { + mockedDevices: []Device{sde}, + wantErr: true, + }, + "NotAMultipathDevice": { + mockedDevices: []Device{sda}, + wantErr: true, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + multipathDevice, err := getMultipathDevice(tt.mockedDevices) + + if tt.wantErr { + assert.Nil(multipathDevice) + assert.NotNil(err) + } else { + assert.Equal(tt.multipathDevice, multipathDevice) + assert.Nil(err) + } + }) + } +} + +func Test_lsblk(t *testing.T) { + sda1 := Device{Name: "sda1"} + sda := Device{Name: "sda", Children: []Device{sda1}} + sdaOutput := marshalDeviceInfo(&deviceInfo{sda, sda1}) + + tests := map[string]struct { + devicePaths []string + strict bool + mockedStdout string + mockedDevices deviceInfo + mockedExitStatus int + wantErr bool + }{ + "Basic": { + devicePaths: []string{"/dev/sda"}, + mockedDevices: []Device{sda}, + mockedStdout: string(sdaOutput), + }, + "NotABlockDevice": { + devicePaths: []string{"/dev/sdzz"}, + mockedStdout: "lsblk: sdzz: not a block device", + mockedExitStatus: 32, + wantErr: true, + }, + "InvalidOutput": { + mockedStdout: "{", + mockedExitStatus: 0, + wantErr: true, + }, + "StrictWithMissingDevices": { + devicePaths: []string{"/dev/sda", "/dev/sdb"}, + strict: true, + mockedDevices: []Device{sda}, + mockedStdout: string(sdaOutput), + mockedExitStatus: 64, + wantErr: true, + }, + "NotStrictWithMissingDevices": { + devicePaths: []string{"/dev/sda", "/dev/sdb"}, + mockedDevices: []Device{sda}, + mockedStdout: string(sdaOutput), + mockedExitStatus: 64, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + + defer gostub.Stub(&execCommand, makeFakeExecCommand(tt.mockedExitStatus, tt.mockedStdout)).Reset() + deviceInfo, err := lsblk(tt.devicePaths, tt.strict) + + if tt.wantErr { + assert.Nil(deviceInfo) + assert.NotNil(err) + } else { + assert.NotNil(deviceInfo) + assert.Equal(tt.mockedDevices, deviceInfo) + assert.Nil(err) + } + }) + } +} + +func TestConnectorPersistance(t *testing.T) { + assert := assert.New(t) + + secret := Secrets{ + SecretsType: "fake secret type", + UserName: "fake username", + Password: "fake password", + UserNameIn: "fake username in", + PasswordIn: "fake password in", + } + childDevice := Device{ + Name: "child-name", + Hctl: "child-hctl", + Type: "child-type", + Transport: "child-transport", + } + device := Device{ + Name: "device-name", + Hctl: "device-hctl", + Children: []Device{childDevice}, + Type: "device-type", + Transport: "device-transport", + } + c := Connector{ + VolumeName: "fake volume name", + TargetIqn: "fake target iqn", + TargetPortals: []string{}, + Lun: 42, + AuthType: "fake auth type", + DiscoverySecrets: secret, + SessionSecrets: secret, + Interface: "fake interface", + MountTargetDevice: &device, + Devices: []Device{childDevice}, + RetryCount: 24, + CheckInterval: 13, + DoDiscovery: true, + DoCHAPDiscovery: true, + } + devicesByPath := map[string]*Device{} + devicesByPath[childDevice.GetPath()] = &childDevice + devicesByPath[device.GetPath()] = &device + + defer gostub.Stub(&execCommand, func(name string, arg ...string) *exec.Cmd { + blockDevices := deviceInfo{} + for _, path := range arg[3:] { + blockDevices = append(blockDevices, *devicesByPath[path]) + } + + out := marshalDeviceInfo(&blockDevices) + return makeFakeExecCommand(0, string(out))(name, arg...) + }).Reset() + + defer gostub.Stub(&execCommand, func(cmd string, args ...string) *exec.Cmd { + devInfo := &deviceInfo{device, childDevice} + if args[3] == "/dev/child-name" { + devInfo = &deviceInfo{childDevice} + } + + mockedOutput := marshalDeviceInfo(devInfo) + return makeFakeExecCommand(0, string(mockedOutput))(cmd, args...) + }).Reset() + + c.Persist("/tmp/connector.json") + c2, err := GetConnectorFromFile("/tmp/connector.json") + assert.Nil(err) + assert.NotNil(c2) + if c2 != nil { + assert.Equal(c, *c2) + } + + err = c.Persist("/tmp") + assert.NotNil(err) + + os.Remove("/tmp/shouldNotExists.json") + _, err = GetConnectorFromFile("/tmp/shouldNotExists.json") + assert.NotNil(err) + assert.IsType(&os.PathError{}, err) + + ioutil.WriteFile("/tmp/connector.json", []byte("not a connector"), 0600) + _, err = GetConnectorFromFile("/tmp/connector.json") + assert.NotNil(err) + assert.IsType(&json.SyntaxError{}, err) +} + +func Test_IsMultipathConsistent(t *testing.T) { + mpath1 := Device{Name: "3600c0ff0000000000000000000000000", Type: "mpath", Size: "10G", Hctl: "0:0:0:1"} + mpath2 := Device{Name: "3600c0ff0000000000000000000000042", Type: "mpath", Size: "5G", Hctl: "0:0:0:2"} + sda := Device{Name: "sda", Size: "10G", Hctl: "1:0:0:1"} + sdb := Device{Name: "sdb", Size: "10G", Hctl: "2:0:0:1"} + sdc := Device{Name: "sdc", Size: "5G", Hctl: "1:0:0:2"} + sdd := Device{Name: "sdd", Size: "5G", Hctl: "2:0:0:2"} + invalidHCTL := Device{Name: "sde", Size: "5G", Hctl: "2:b"} + sdf := Device{Name: "sdf", Size: "10G", Hctl: "2:0:0:3"} + sdg := Device{Name: "sdg", Size: "10G", Hctl: "1:0:0:1"} + devicesWWIDs := map[string]string{} + devicesWWIDs[mpath1.GetPath()] = "3600c0ff0000000000000000000000000" + devicesWWIDs[sda.GetPath()] = "3600c0ff0000000000000000000000000" + devicesWWIDs[sdb.GetPath()] = "3600c0ff0000000000000000000000000" + devicesWWIDs[sdg.GetPath()] = "3600c0ff0000000000000000000000024" + + tests := map[string]struct { + connector *Connector + wantErr bool + errContains string + }{ + "Basic": { + connector: &Connector{ + MountTargetDevice: &mpath1, + Devices: []Device{sda, sdb}, + }, + }, + "Different sizes 1": { + connector: &Connector{ + MountTargetDevice: &mpath1, + Devices: []Device{sda, sdc}, + }, + wantErr: true, + errContains: "size differ", + }, + "Different sizes 2": { + connector: &Connector{ + MountTargetDevice: &mpath1, + Devices: []Device{sdc, sdd}, + }, + wantErr: true, + errContains: "size differ", + }, + "Invalid HCTL": { + connector: &Connector{ + MountTargetDevice: &invalidHCTL, + Devices: []Device{}, + }, + wantErr: true, + errContains: "invalid HCTL", + }, + "LUNs differs": { + connector: &Connector{ + MountTargetDevice: &mpath1, + Devices: []Device{sda, sdf}, + }, + wantErr: true, + errContains: "LUNs differ", + }, + "Same controller": { + connector: &Connector{ + MountTargetDevice: &mpath1, + Devices: []Device{sda, sdg}, + }, + wantErr: true, + errContains: "same controller", + }, + "Missing WWID": { + connector: &Connector{ + MountTargetDevice: &mpath2, + Devices: []Device{sdc, sdd}, + }, + wantErr: true, + errContains: "could not find WWID", + }, + "WWIDs differ": { + connector: &Connector{ + MountTargetDevice: &mpath1, + Devices: []Device{sdb, sdg}, + }, + wantErr: true, + errContains: "WWIDs differ", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + c := tt.connector + + defer gostub.Stub(&execWithTimeout, func(_ string, args []string, _ time.Duration) ([]byte, error) { + devicePath := args[len(args)-1] + wwid, ok := devicesWWIDs[devicePath] + if !ok { + return []byte(""), errors.New("") + } + return []byte(wwid + "\n"), nil + }).Reset() + + err := c.IsMultipathConsistent() + + if tt.wantErr { + assert.Error(err) + if tt.errContains != "" { + assert.Contains(err.Error(), tt.errContains) + } + } else { + assert.Nil(err) } }) } diff --git a/iscsi/iscsiadm.go b/iscsi/iscsiadm.go index a75f218..b87b5c6 100644 --- a/iscsi/iscsiadm.go +++ b/iscsi/iscsiadm.go @@ -1,9 +1,9 @@ package iscsi import ( - "bytes" "fmt" "strings" + "time" ) // Secrets provides optional iscsi security credentials (CHAP settings) @@ -20,46 +20,13 @@ type Secrets struct { PasswordIn string `json:"passwordIn,omitempty"` } -// CmdError is a custom error to provide details including the command, stderr output and exit code. -// iscsiadm in some cases requires all of this info to determine success or failure -type CmdError struct { - CMD string - StdErr string - ExitCode int -} - -func (e *CmdError) Error() string { - // we don't output the command in the error string to avoid leaking any security info - // the command is still available in the error structure if the caller wants it though - return fmt.Sprintf("iscsiadm returned an error: %s, exit-code: %d", e.StdErr, e.ExitCode) -} - func iscsiCmd(args ...string) (string, error) { - cmd := execCommand("iscsiadm", args...) - var stdout bytes.Buffer - var iscsiadmError error - cmd.Stdout = &stdout - cmd.Stderr = &stdout - defer stdout.Reset() - - // we're using Start and Wait because we want to grab exit codes - err := cmd.Start() - if err != nil { - // This is usually a cmd not found so we'll set our own error here - formattedOutput := strings.Replace(string(stdout.Bytes()), "\n", "", -1) - iscsiadmError = fmt.Errorf("iscsiadm error: %s (%s)", formattedOutput, err.Error()) - - } else { - err = cmd.Wait() - if err != nil { - formattedOutput := strings.Replace(string(stdout.Bytes()), "\n", "", -1) - iscsiadmError = fmt.Errorf("iscsiadm error: %s (%s)", formattedOutput, err.Error()) + stdout, err := execWithTimeout("iscsiadm", args, time.Second*3) - } - } + debug.Printf("Run iscsiadm command: %s", strings.Join(append([]string{"iscsiadm"}, args...), " ")) + iscsiadmDebug(string(stdout), err) - iscsiadmDebug(string(stdout.Bytes()), iscsiadmError) - return string(stdout.Bytes()), iscsiadmError + return string(stdout), err } func iscsiadmDebug(output string, cmdError error) { @@ -91,7 +58,7 @@ func ShowInterface(iface string) (string, error) { func CreateDBEntry(tgtIQN, portal, iFace string, discoverySecrets, sessionSecrets Secrets) error { debug.Println("Begin CreateDBEntry...") baseArgs := []string{"-m", "node", "-T", tgtIQN, "-p", portal} - _, err := iscsiCmd(append(baseArgs, []string{"-I", iFace, "-o", "new"}...)...) + _, err := iscsiCmd(append(baseArgs, "-I", iFace, "-o", "new")...) if err != nil { return err } @@ -180,29 +147,25 @@ func GetSessions() (string, error) { // Login performs an iscsi login for the specified target func Login(tgtIQN, portal string) error { + debug.Println("Begin Login...") baseArgs := []string{"-m", "node", "-T", tgtIQN, "-p", portal} - _, err := iscsiCmd(append(baseArgs, []string{"-l"}...)...) - if err != nil { + if _, err := iscsiCmd(append(baseArgs, []string{"-l"}...)...); err != nil { //delete the node record from database iscsiCmd(append(baseArgs, []string{"-o", "delete"}...)...) return fmt.Errorf("failed to sendtargets to portal %s, err: %v", portal, err) } - return err + return nil } -// Logout logs out the specified target, if the target is not logged in it's not considered an error -func Logout(tgtIQN string, portals []string) error { +// Logout logs out the specified target +func Logout(tgtIQN, portal string) error { debug.Println("Begin Logout...") - baseArgs := []string{"-m", "node", "-T", tgtIQN} - for _, p := range portals { - debug.Printf("attempting logout for portal: %s", p) - args := append(baseArgs, []string{"-p", p, "-u"}...) - iscsiCmd(args...) - } + args := []string{"-m", "node", "-T", tgtIQN, "-p", portal, "-u"} + iscsiCmd(args...) return nil } -// DeleteDBEntry deletes the iscsi db entry fo rthe specified target +// DeleteDBEntry deletes the iscsi db entry for the specified target func DeleteDBEntry(tgtIQN string) error { debug.Println("Begin DeleteDBEntry...") args := []string{"-m", "node", "-T", tgtIQN, "-o", "delete"} diff --git a/iscsi/iscsiadm_test.go b/iscsi/iscsiadm_test.go index 3d91976..1d84d80 100644 --- a/iscsi/iscsiadm_test.go +++ b/iscsi/iscsiadm_test.go @@ -1,24 +1,88 @@ package iscsi -import "testing" +import ( + "os/exec" + "testing" + + "github.com/prashantv/gostub" + "github.com/stretchr/testify/assert" +) + +const defaultInterface = ` +# BEGIN RECORD 2.0-874 +iface.iscsi_ifacename = default +iface.net_ifacename = +iface.ipaddress = +iface.hwaddress = +iface.transport_name = tcp +iface.initiatorname = +iface.state = +iface.vlan_id = 0 +iface.vlan_priority = 0 +iface.vlan_state = +iface.iface_num = 0 +iface.mtu = 0 +iface.port = 0 +iface.bootproto = +iface.subnet_mask = +iface.gateway = +iface.dhcp_alt_client_id_state = +iface.dhcp_alt_client_id = +iface.dhcp_dns = +iface.dhcp_learn_iqn = +iface.dhcp_req_vendor_id_state = +iface.dhcp_vendor_id_state = +iface.dhcp_vendor_id = +iface.dhcp_slp_da = +iface.fragmentation = +iface.gratuitous_arp = +iface.incoming_forwarding = +iface.tos_state = +iface.tos = 0 +iface.ttl = 0 +iface.delayed_ack = +iface.tcp_nagle = +iface.tcp_wsf_state = +iface.tcp_wsf = 0 +iface.tcp_timer_scale = 0 +iface.tcp_timestamp = +iface.redirect = +iface.def_task_mgmt_timeout = 0 +iface.header_digest = +iface.data_digest = +iface.immediate_data = +iface.initial_r2t = +iface.data_seq_inorder = +iface.data_pdu_inorder = +iface.erl = 0 +iface.max_receive_data_len = 0 +iface.first_burst_len = 0 +iface.max_outstanding_r2t = 0 +iface.max_burst_len = 0 +iface.chap_auth = +iface.bidi_chap = +iface.strict_login_compliance = +iface.discovery_auth = +iface.discovery_logout = +# END RECORD +` func TestDiscovery(t *testing.T) { - execCommand = fakeExecCommand tests := map[string]struct { - tgtPortal string - iface string - discoverySecret Secrets - chapDiscovery bool - wantErr bool - mockedStdout string - mockedExitStatus int + tgtPortal string + iface string + discoverySecret Secrets + chapDiscovery bool + wantErr bool + mockedStdout string + mockedCmdError error }{ "DiscoverySuccess": { - tgtPortal: "172.18.0.2:3260", - iface: "default", - chapDiscovery: false, - mockedStdout: "172.18.0.2:3260,1 iqn.2016-09.com.openebs.jiva:store1\n", - mockedExitStatus: 0, + tgtPortal: "172.18.0.2:3260", + iface: "default", + chapDiscovery: false, + mockedStdout: "172.18.0.2:3260,1 iqn.2016-09.com.openebs.jiva:store1\n", + mockedCmdError: nil, }, "ConnectionFailure": { @@ -29,8 +93,8 @@ func TestDiscovery(t *testing.T) { iscsiadm: cannot make connection to 172.18.0.2: Connection refused iscsiadm: connection login retries (reopen_max) 5 exceeded iscsiadm: Could not perform SendTargets discovery: encountered connection failure\n`, - mockedExitStatus: 4, - wantErr: true, + mockedCmdError: exec.Command("exit", "4").Run(), + wantErr: true, }, "ChapEntrySuccess": { @@ -41,8 +105,8 @@ iscsiadm: Could not perform SendTargets discovery: encountered connection failur UserNameIn: "dummyuser", PasswordIn: "dummypass", }, - mockedStdout: "172.18.0.2:3260,1 iqn.2016-09.com.openebs.jiva:store1\n", - mockedExitStatus: 0, + mockedStdout: "172.18.0.2:3260,1 iqn.2016-09.com.openebs.jiva:store1\n", + mockedCmdError: nil, }, "ChapEntryFailure": { @@ -56,15 +120,14 @@ iscsiadm: Could not perform SendTargets discovery: encountered connection failur mockedStdout: `iscsiadm: Login failed to authenticate with target iscsiadm: discovery login to 172.18.0.2 rejected: initiator error (02/01), non-retryable, giving up iscsiadm: Could not perform SendTargets discovery.\n`, - mockedExitStatus: 4, - wantErr: true, + mockedCmdError: exec.Command("exit", "4").Run(), + wantErr: true, }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { - mockedExitStatus = tt.mockedExitStatus - mockedStdout = tt.mockedStdout + defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset() err := Discoverydb(tt.tgtPortal, tt.iface, tt.discoverySecret, tt.chapDiscovery) if (err != nil) != tt.wantErr { t.Errorf("Discoverydb() error = %v, wantErr %v", err, tt.wantErr) @@ -75,16 +138,15 @@ iscsiadm: Could not perform SendTargets discovery.\n`, } func TestCreateDBEntry(t *testing.T) { - execCommand = fakeExecCommand tests := map[string]struct { - tgtPortal string - tgtIQN string - iface string - discoverySecret Secrets - sessionSecret Secrets - wantErr bool - mockedStdout string - mockedExitStatus int + tgtPortal string + tgtIQN string + iface string + discoverySecret Secrets + sessionSecret Secrets + wantErr bool + mockedStdout string + mockedCmdError error }{ "CreateDBEntryWithChapDiscoverySuccess": { tgtPortal: "192.168.1.107:3260", @@ -100,23 +162,22 @@ func TestCreateDBEntry(t *testing.T) { PasswordIn: "dummypass", SecretsType: "chap", }, - mockedStdout: nodeDB, - mockedExitStatus: 0, + mockedStdout: nodeDB, + mockedCmdError: nil, }, "CreateDBEntryWithChapDiscoveryFailure": { - tgtPortal: "172.18.0.2:3260", - tgtIQN: "iqn.2016-09.com.openebs.jiva:store1", - iface: "default", - mockedStdout: "iscsiadm: No records found\n", - mockedExitStatus: 21, - wantErr: true, + tgtPortal: "172.18.0.2:3260", + tgtIQN: "iqn.2016-09.com.openebs.jiva:store1", + iface: "default", + mockedStdout: "iscsiadm: No records found\n", + mockedCmdError: exec.Command("exit", "21").Run(), + wantErr: true, }, } for name, tt := range tests { t.Run(name, func(t *testing.T) { - mockedExitStatus = tt.mockedExitStatus - mockedStdout = tt.mockedStdout + defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset() err := CreateDBEntry(tt.tgtIQN, tt.tgtPortal, tt.iface, tt.discoverySecret, tt.sessionSecret) if (err != nil) != tt.wantErr { t.Errorf("CreateDBEntry() error = %v, wantErr %v", err, tt.wantErr) @@ -126,3 +187,89 @@ func TestCreateDBEntry(t *testing.T) { } } + +func TestListInterfaces(t *testing.T) { + tests := map[string]struct { + mockedStdout string + mockedCmdError error + interfaces []string + wantErr bool + }{ + "EmptyOutput": { + mockedStdout: "", + mockedCmdError: nil, + interfaces: []string{""}, + wantErr: false, + }, + "DefaultInterface": { + mockedStdout: "default", + mockedCmdError: nil, + interfaces: []string{"default"}, + wantErr: false, + }, + "TwoInterface": { + mockedStdout: "default\ntest", + mockedCmdError: nil, + interfaces: []string{"default", "test"}, + wantErr: false, + }, + "HasError": { + mockedStdout: "", + mockedCmdError: exec.Command("exit", "1").Run(), + interfaces: []string{}, + wantErr: true, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset() + interfaces, err := ListInterfaces() + + if tt.wantErr { + assert.NotNil(err) + } else { + assert.Nil(err) + assert.Equal(interfaces, tt.interfaces) + } + }) + } +} + +func TestShowInterface(t *testing.T) { + tests := map[string]struct { + mockedStdout string + mockedCmdError error + iFace string + wantErr bool + }{ + "DefaultInterface": { + mockedStdout: defaultInterface, + mockedCmdError: nil, + iFace: defaultInterface, + wantErr: false, + }, + "HasError": { + mockedStdout: "", + mockedCmdError: exec.Command("exit", "1").Run(), + iFace: "", + wantErr: true, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset() + interfaces, err := ShowInterface("default") + + if tt.wantErr { + assert.NotNil(err) + } else { + assert.Nil(err) + assert.Equal(interfaces, tt.iFace) + } + }) + } +} diff --git a/iscsi/multipath.go b/iscsi/multipath.go index 3a93b12..347ae33 100644 --- a/iscsi/multipath.go +++ b/iscsi/multipath.go @@ -2,16 +2,22 @@ package iscsi import ( "context" - "io/ioutil" + "fmt" "os" "os/exec" - "path/filepath" + "strings" "time" ) -var sysBlockPath = "/sys/block" -var devPath = "/dev" +type pathGroup struct { + Paths []path `json:"paths"` +} + +type path struct { + Device string `json:"dev"` +} +// ExecWithTimeout execute a command with a timeout and returns an error if timeout is excedeed func ExecWithTimeout(command string, args []string, timeout time.Duration) ([]byte, error) { debug.Printf("Executing command '%v' with args: '%v'.\n", command, args) @@ -20,10 +26,11 @@ func ExecWithTimeout(command string, args []string, timeout time.Duration) ([]by defer cancel() // Create command with context - cmd := exec.CommandContext(ctx, command, args...) + cmd := execCommandContext(ctx, command, args...) // This time we can simply use Output() to get the result. out, err := cmd.Output() + debug.Println(err) // We want to check the context error to see if the timeout was executed. // The error returned by cmd.Output() will be OS specific based on what @@ -33,55 +40,48 @@ func ExecWithTimeout(command string, args []string, timeout time.Duration) ([]by return nil, ctx.Err() } - // If there's no context error, we know the command completed (or errored). - debug.Printf("Output from command: %s", string(out)) if err != nil { - debug.Printf("Non-zero exit code: %s\n", err) + if ee, ok := err.(*exec.ExitError); ok { + debug.Printf("Non-zero exit code: %s\n", err) + err = fmt.Errorf("%s", ee.Stderr) + } } debug.Println("Finished executing command.") return out, err } -// GetSysDevicesFromMultipathDevice gets all slaves for multipath device dm-x -// in /sys/block/dm-x/slaves/ -func GetSysDevicesFromMultipathDevice(device string) ([]string, error) { - debug.Printf("Getting all slaves for multipath device %s.\n", device) - deviceSlavePath := filepath.Join(sysBlockPath, device, "slaves") - slaves, err := ioutil.ReadDir(deviceSlavePath) - if err != nil { - if os.IsNotExist(err) { - return nil, nil - } - debug.Printf("An error occured while looking for slaves: %v\n", err) - return nil, err - } - - var s []string - for _, slave := range slaves { - s = append(s, slave.Name()) - } - debug.Printf("Found slaves: %v.\n", s) - return s, nil -} - // FlushMultipathDevice flushes a multipath device dm-x with command multipath -f /dev/dm-x -func FlushMultipathDevice(device string) error { - debug.Printf("Flushing multipath device '%v'.\n", device) +func FlushMultipathDevice(device *Device) error { + devicePath := device.GetPath() + debug.Printf("Flushing multipath device '%v'.\n", devicePath) - fullDevice := filepath.Join(devPath, device) timeout := 5 * time.Second - _, err := execWithTimeout("multipath", []string{"-f", fullDevice}, timeout) + _, err := execWithTimeout("multipath", []string{"-f", devicePath}, timeout) if err != nil { - if _, e := os.Stat(fullDevice); os.IsNotExist(e) { - debug.Printf("Multipath device %v was deleted.\n", device) + if _, e := osStat(devicePath); os.IsNotExist(e) { + debug.Printf("Multipath device %v has been removed.\n", devicePath) } else { - debug.Printf("Command 'multipath -f %v' did not succeed to delete the device: %v\n", fullDevice, err) + if strings.Contains(err.Error(), "map in use") { + err = fmt.Errorf("device is probably still in use somewhere else: %v", err) + } + debug.Printf("Command 'multipath -f %v' did not succeed to delete the device: %v\n", devicePath, err) return err } } - debug.Printf("Finshed flushing multipath device %v.\n", device) + debug.Printf("Finshed flushing multipath device %v.\n", devicePath) + return nil +} + +// ResizeMultipathDevice resize a multipath device based on its underlying devices +func ResizeMultipathDevice(device *Device) error { + debug.Printf("Resizing multipath device %s\n", device.GetPath()) + + if output, err := execCommand("multipathd", "resize", "map", device.Name).CombinedOutput(); err != nil { + return fmt.Errorf("could not resize multipath device: %s (%v)", output, err) + } + return nil } diff --git a/iscsi/multipath_test.go b/iscsi/multipath_test.go new file mode 100644 index 0000000..abf5249 --- /dev/null +++ b/iscsi/multipath_test.go @@ -0,0 +1,79 @@ +package iscsi + +import ( + "context" + "os/exec" + "testing" + "time" + + "github.com/prashantv/gostub" + "github.com/stretchr/testify/assert" +) + +func TestExecWithTimeout(t *testing.T) { + tests := map[string]struct { + mockedStdout string + mockedExitStatus int + wantTimeout bool + }{ + "Success": { + mockedStdout: "some output", + mockedExitStatus: 0, + wantTimeout: false, + }, + "WithError": { + mockedStdout: "some\noutput", + mockedExitStatus: 1, + wantTimeout: false, + }, + "WithTimeout": { + mockedStdout: "", + mockedExitStatus: 0, + wantTimeout: true, + }, + "WithTimeoutAndOutput": { + mockedStdout: "should not be returned", + mockedExitStatus: 0, + wantTimeout: true, + }, + "WithTimeoutAndError": { + mockedStdout: "", + mockedExitStatus: 1, + wantTimeout: true, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + + timeout := time.Second + if tt.wantTimeout { + timeout = time.Millisecond * 50 + } + + defer gostub.Stub(&execCommandContext, func(ctx context.Context, command string, args ...string) *exec.Cmd { + if tt.wantTimeout { + time.Sleep(timeout + time.Millisecond*10) + } + return makeFakeExecCommandContext(tt.mockedExitStatus, tt.mockedStdout)(ctx, command, args...) + }).Reset() + + out, err := ExecWithTimeout("dummy", []string{}, timeout) + + if tt.wantTimeout || tt.mockedExitStatus != 0 { + assert.NotNil(err) + if tt.wantTimeout { + assert.Equal(context.DeadlineExceeded, err) + } + } else { + assert.Nil(err) + } + + if tt.wantTimeout { + assert.Equal("", string(out)) + } else { + assert.Equal(tt.mockedStdout, string(out)) + } + }) + } +}