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/OWNERS b/OWNERS index c7f62cb..6effc7e 100644 --- a/OWNERS +++ b/OWNERS @@ -1,8 +1,16 @@ # See the OWNERS docs: https://git.k8s.io/community/contributors/guide/owners.md approvers: -- saad-ali +- humblec - j-griffith -reviews: +- jsafrane +- msau42 - saad-ali +- xing-yang +reviewers: +- humblec - j-griffith +- jsafrane +- msau42 +- saad-ali +- xing-yang diff --git a/README.md b/README.md index bc88192..4c83e64 100644 --- a/README.md +++ b/README.md @@ -17,20 +17,23 @@ golang libs. This may prove to not be ideal, and may be changed over time, but ## Logging and Debug -By default the library does not provide any logging, but provides an error message that includes any messages from -iscsiadm as well as exit-codes. In the event that you need to debug the library, we provide a function: +This library uses klog/v2 with structured and contextual logging to produce Info and Error log entries. A caller +can change the verbosity level by using the command line option of "-v=#" and using "-v=0" will not produce any +log entries. To increase the verbosity of the log entries, use "-v=2". External functions require context.Context +and the logger is extracted from the context using klog.FromContext(ctx), and then a logger pointer is passed around +to internal functions that rely on InfoS and ErrorS calls. -``` -func EnableDebugLogging(writer io.Writer) -``` +## External Binary Dependencies -This will turn on verbose logging directed to the provided io.Writer and include the response of every iscsiadm command -issued. +This library relies on the following operating system executables: +* iscsiadm - Open-iscsi administration utility. +* multipath - Device mapper target autoconfig. +* multipathd - Multipath daemon. ## Intended Usage -Curently the intended usage of this library is simply to provide a golang package to standardize how plugins are implementing -iscsi connect and disconnect. It's not intended to be a "service", although that's a possible next step. It's currenty been +Currently the intended usage of this library is simply to provide a golang package to standardize how plugins are implementing +iscsi connect and disconnect. It's not intended to be a "service", although that's a possible next step. It's currently been used for plugins where iscsid is installed in containers only, as well as designs where it uses the nodes iscsid. Each of these approaches has their own pros and cons. Currently, it's up to the plugin author to determine which model suits them best and to deploy their node plugin appropriately. diff --git a/example/main.go b/example/main.go index dc0f8eb..4814935 100644 --- a/example/main.go +++ b/example/main.go @@ -1,6 +1,7 @@ package main import ( + "context" "flag" "log" "os" @@ -8,34 +9,31 @@ import ( "time" "github.com/kubernetes-csi/csi-lib-iscsi/iscsi" + "k8s.io/klog/v2" ) 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, "") ) func main() { + klog.InitFlags(nil) + klog.EnableContextualLogging(true) flag.Parse() - tgtp := strings.Split(*portals, ",") - if *debug { - iscsi.EnableDebugLogging(os.Stdout) - } + tgtps := strings.Split(*portals, ",") // 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,31 +43,32 @@ 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) + // Number of times we check for device path, waiting for CheckInterval seconds in-between 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 + // CheckInterval is the time in seconds to wait in-between device path checks when logging in to a target CheckInterval: 1, } + // Create a context object to be shared with the iscsi routines + ctx := context.Background() + // 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) + // A successful connection will include the device path to access our iscsi volume + path, err := c.Connect(ctx) 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(ctx); 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(ctx) } diff --git a/go.mod b/go.mod new file mode 100644 index 0000000..2603945 --- /dev/null +++ b/go.mod @@ -0,0 +1,16 @@ +module github.com/kubernetes-csi/csi-lib-iscsi + +go 1.18 + +require ( + github.com/go-logr/logr v1.2.0 + github.com/prashantv/gostub v1.0.0 + github.com/stretchr/testify v1.7.0 + k8s.io/klog/v2 v2.60.1 +) + +require ( + github.com/davecgh/go-spew v1.1.0 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..294ccfc --- /dev/null +++ b/go.sum @@ -0,0 +1,17 @@ +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/go-logr/logr v1.2.0 h1:QK40JKJyMdUDz+h+xvCsru/bJhvG0UxvePV0ufL/AcE= +github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +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= +k8s.io/klog/v2 v2.60.1 h1:VW25q3bZx9uE3vvdL6M8ezOX79vA2Aq1nEWLqNQclHc= +k8s.io/klog/v2 v2.60.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= diff --git a/iscsi/iscsi.go b/iscsi/iscsi.go index d2ba7ae..fcf96e8 100644 --- a/iscsi/iscsi.go +++ b/iscsi/iscsi.go @@ -1,11 +1,11 @@ package iscsi import ( + "context" "encoding/json" + "errors" "fmt" - "io" "io/ioutil" - "log" "os" "os/exec" "path/filepath" @@ -14,19 +14,23 @@ import ( "strings" "syscall" "time" + + "k8s.io/klog/v2" ) const defaultPort = "3260" var ( - debug *log.Logger - execCommand = exec.Command - execWithTimeout = ExecWithTimeout + 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,47 +39,51 @@ type iscsiSession struct { Name string } -type TargetInfo struct { - Iqn string `json:"iqn"` - Portal string `json:"portal"` - Port string `json:"port"` -} - -//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"` +type deviceInfo []Device - RetryCount int32 `json:"retry_count"` - CheckInterval int32 `json:"check_interval"` - DoDiscovery bool `json:"do_discovery"` - DoCHAPDiscovery bool `json:"do_chap_discovery"` +// Device contains information 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"` } -func init() { - // by default we don't log anything, EnableDebugLogging() can turn on some tracing - debug = log.New(ioutil.Discard, "", 0) - +type HCTL struct { + HBA int + Channel int + Target int + LUN int } -// EnableDebugLogging provides a mechanism to turn on debug logging for this package -// output is written to the provided io.Writer -func EnableDebugLogging(writer io.Writer) { - debug = log.New(writer, "DEBUG: ", log.Ldate|log.Ltime|log.Lshortfile) +// Connector provides a struct to hold all of the needed parameters to make our iSCSI connection +type Connector struct { + 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"` + HandleUnalignedChildren bool `json:"handle_unaligned_children"` } -// parseSession takes the raw stdout from the iscsiadm -m session command and encodes it into an iscsi session type +var version string = "1.0.0" + +// 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(string(lines)), "\n") + entries := strings.Split(strings.TrimSpace(lines), "\n") r := strings.NewReplacer("[", "", "]", "") @@ -102,8 +110,9 @@ func parseSessions(lines string) []iscsiSession { return sessions } -func sessionExists(tgtPortal, tgtIQN string) (bool, error) { - sessions, err := getCurrentSessions() +// sessionExists checks if an iSCSI session exists +func sessionExists(ctx context.Context, tgtPortal, tgtIQN string) (bool, error) { + sessions, err := getCurrentSessions(ctx) if err != nil { return false, err } @@ -115,6 +124,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 { @@ -126,9 +136,9 @@ func extractTransportName(output string) string { return res[1] } -func getCurrentSessions() ([]iscsiSession, error) { - - out, err := GetSessions() +// getCurrentSessions list current iSCSI sessions +func getCurrentSessions(ctx context.Context) ([]iscsiSession, error) { + out, err := GetSessions(ctx) if err != nil { exitErr, ok := err.(*exec.ExitError) if ok && exitErr.ProcessState.Sys().(syscall.WaitStatus).ExitStatus() == 21 { @@ -140,96 +150,111 @@ 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(logger klog.Logger, 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 { + logger.V(1).Info("Device path doesn't exists yet, retrying", "device", *devicePath, "seconds", intervalSeconds, "retries", 1, "max", maxRetries) + sleep(time.Second * time.Duration(intervalSeconds)) + } + + if err := pathExists(logger, 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(logger klog.Logger, devicePath *string, deviceTransport string) error { + if deviceTransport == "tcp" { + _, err := osStat(*devicePath) + if err != nil { + if !os.IsNotExist(err) { + logger.Error(err, "Error attempting to stat device") + return err } + logger.V(1).Info("Device not found", "device", *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(logger klog.Logger, devices []Device, handleUnalignedChildren bool) (*Device, error) { + var multipathDevice *Device + + // handleUnalignedChildren when set to false (default) requires that a parent be listed for every child. + // This is the current behavior. This change allows this function to work with older versions of lsblk + // that did not print a parent for each child. + + for _, device := range devices { + logger.V(1).Info("find multipath device", "device", device, "children", len(device.Children)) + if len(device.Children) != 1 { + if !handleUnalignedChildren { + 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) + } else { + logger.V(1).Info("WARNING: children != 1", "name", device.Name) + } + } + if !handleUnalignedChildren { + if multipathDevice != nil && device.Children[0].Name != multipathDevice.Name { + return nil, fmt.Errorf("devices don't share a common multipath device: %v", devices) + } + } else { + if len(device.Children) == 1 { + multipathDevice = &device.Children[0] + logger.V(1).Info("SET: multipath device", "name", multipathDevice.Name, "requireChildren", handleUnalignedChildren) + break } } + multipathDevice = &device.Children[0] + logger.V(1).Info("SET: multipath device", "name", multipathDevice.Name, "requireChildren", handleUnalignedChildren) + } + + if multipathDevice == nil { + return nil, fmt.Errorf("multipath device not found") + } + + if multipathDevice.Type != "mpath" { + return nil, fmt.Errorf("device is not of mpath type: %v", multipathDevice) } - 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) + + return multipathDevice, nil } // Connect attempts to connect a volume to this node using the provided Connector info -func Connect(c Connector) (string, error) { - var lastErr error +func (c *Connector) Connect(ctx context.Context) (string, error) { + + logger := klog.FromContext(ctx) + logger.Info("[] csi-lib-iscsi connect", "version", version) + if c.RetryCount == 0 { c.RetryCount = 10 } @@ -237,207 +262,413 @@ 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 } // make sure our iface exists and extract the transport type - out, err := ShowInterface(iFace) + out, err := ShowInterface(ctx, iFace) if err != nil { return "", err } 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(ctx, c.TargetIqn, target, iFace, iscsiTransport) + if err != nil { + lastErr = err + } else { + logger.V(1).Info("Appending device path", "device", 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(logger, devicePaths, true); err != nil { + return "", err + } + + if len(c.Devices) < 1 { + logger.Error(lastErr, "failed to find device path", "length", len(c.Devices)) + iscsiCmd(ctx, []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(logger) + c.MountTargetDevice = mountTargetDevice + if err != nil { + logger.Error(err, "Connect failed") + err := RemoveSCSIDevices(logger, c.Devices...) + if err != nil { + return "", err } + c.MountTargetDevice = nil + c.Devices = []Device{} + return "", err + } + + if c.IsMultipathEnabled() { + if err := c.IsMultipathConsistent(ctx); 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(ctx context.Context, targetIqn string, target string, iFace string, iscsiTransport string) (string, error) { + logger := klog.FromContext(ctx) + logger.V(1).Info("Connect target", "iqn", targetIqn, "portal", 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(ctx, append(baseArgs, []string{"-R"}...)...); err != nil { + logger.Error(err, "Failed to rescan session") + if os.IsTimeout(err) { + logger.V(1).Info("iscsiadm timed out, 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(ctx, portal, targetIqn) + if exists { + logger.V(1).Info("Session already exists, checking if device path exists", "device", devicePath) + if err := waitForPathToExist(logger, &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(ctx, 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(ctx, targetIqn, portal) + if err != nil { + logger.Error(err, "Failed to login") + return "", err } - for i, path := range devicePaths { - if path != "" { - if mappedDevicePath, err := getMultipathDisk(path); mappedDevicePath != "" { - devicePaths[i] = mappedDevicePath - if err != nil { - return "", err - } - } + logger.V(1).Info("Waiting for device path to exist", "device", devicePath) + if err := waitForPathToExist(logger, &devicePath, c.RetryCount, c.CheckInterval, iscsiTransport); err != nil { + return "", err + } + + return devicePath, nil +} + +func (c *Connector) discoverTarget(ctx context.Context, targetIqn string, iFace string, portal string) error { + logger := klog.FromContext(ctx) + if c.DoDiscovery { + // build discoverydb and discover iscsi target + if err := Discoverydb(ctx, portal, iFace, c.DiscoverySecrets, c.DoCHAPDiscovery); err != nil { + logger.Error(err, "Error in discovery of the target") + 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(ctx, targetIqn, portal, iFace, c.DiscoverySecrets, c.SessionSecrets) + if err != nil { + logger.Error(err, "Error creating db entry") + 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) +// Disconnect performs a disconnect operation from an appliance. +// Be sure to disconnect all devices properly before doing this as it can result in data loss. +func (c *Connector) Disconnect(ctx context.Context) { + for _, target := range c.TargetPortals { + targetPortal := strings.Split(target, ":")[0] + err := Logout(ctx, c.TargetIqn, targetPortal) + if err != nil { + return + } + } + + deleted := map[string]bool{} + if _, ok := deleted[c.TargetIqn]; ok { + return + } + deleted[c.TargetIqn] = true + err := DeleteDBEntry(ctx, c.TargetIqn) if err != nil { - return err + return } - err = DeleteDBEntry(tgtIqn) - return err } // DisconnectVolume removes a volume from a Linux host. -func DisconnectVolume(c Connector) error { +func (c *Connector) DisconnectVolume(ctx context.Context) 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 + logger := klog.FromContext(ctx) + + if c.IsMultipathEnabled() { + if err := c.IsMultipathConsistent(ctx); err != nil { + return fmt.Errorf("multipath is inconsistent: %v", err) } - err := FlushMultipathDevice(c.DevicePath) + + logger.V(1).Info("Removing multipath device in path", "device", c.MountTargetDevice.GetPath()) + err := FlushMultipathDevice(ctx, 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(logger, c.Devices...); err != nil { return err } } else { - debug.Printf("Removing normal device.\n") - if err := RemovePhysicalDevice(c.DevicePath); err != nil { + devicePath := c.MountTargetDevice.GetPath() + logger.V(1).Info("Removing normal device in path", "device", devicePath) + if err := RemoveSCSIDevices(logger, *c.MountTargetDevice); err != nil { return err } } - debug.Printf("Finished disconnecting volume.\n") + logger.V(1).Info("Finished disconnecting volume.") 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(logger klog.Logger) (*Device, error) { + if len(c.Devices) > 1 { + multipathDevice, err := getMultipathDevice(logger, c.Devices, c.HandleUnalignedChildren) + if err != nil { + logger.Error(err, "Mount target is not a multipath device") + return nil, err + } + logger.V(1).Info("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(logger klog.Logger, devicePaths []string, strict bool) ([]Device, error) { + logger.V(1).Info("Getting info about SCSI devices", "devices", devicePaths) + + deviceInfo, err := lsblk(logger, devicePaths, strict) + if err != nil { + logger.Error(err, "An error occurred while looking info about SCSI devices") + 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(logger klog.Logger, devicePaths []string, strict bool) (devices []Device, err error) { + scsiDevices, err := GetSCSIDevices(logger, devicePaths, strict) + if err != nil { + return + } + + for i := range scsiDevices { + device := &scsiDevices[i] + if device.Transport == "iscsi" { + logger.V(1).Info("append iscsi device", "device", *device) + devices = append(devices, *device) + } + } + + return +} + +// lsblk execute the lsblk commands +func lsblk(logger klog.Logger, devicePaths []string, strict bool) (deviceInfo, error) { + flags := []string{"-rn", "-o", "NAME,KNAME,PKNAME,HCTL,TYPE,TRAN,SIZE"} + command := execCommand("lsblk", append(flags, devicePaths...)...) + logger.V(1).Info("lsblk", "command", command.String()) + out, err := command.Output() + logger.V(1).Info("lsblk", "output", out, "error", "err") + 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 + } + logger.Error(err, "Could find only some devices") + } 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, " ") + logger.V(1).Info("parse devices", "columns", columns) + if len(columns) < 5 { + logger.V(1).Info("invalid output from lsblk", "line", line) + 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], + } + logger.V(1).Info("append device", "column1", columns[1], "column2", columns[2], "device", device) + 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{} + } + logger.V(1).Info("append child", "pkName", pkName, "device", *device) + parent.Children = append(devicesMap[pkName].Children, *device) + } + + // Filter devices to keep only the roots of the tree + var deviceInfo deviceInfo + for i, device := range devices { + if pkNames[i] == "" { + logger.V(1).Info("append device info", "pkName", pkNames[i], "device", *device) + deviceInfo = append(deviceInfo, *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) + return deviceInfo, nil +} + +// writeInSCSIDeviceFile write into special devices files to change devices state +func writeInSCSIDeviceFile(logger klog.Logger, hctl string, file string, content string) error { + filename := filepath.Join("/sys/class/scsi_device", hctl, "device", file) + logger.V(1).Info("Write to SCSI device", "content", content, "filename", filename) + + f, err := osOpenFile(filename, os.O_TRUNC|os.O_WRONLY, 0200) + if err != nil { + logger.Error(err, "Error attempting to open file", "filename", filename) + return err + } + + defer f.Close() + if _, err := f.WriteString(content); err != nil { + logger.Error(err, "Error attempting to write to file", "filename", filename) + return err + } + + return nil +} + +// RemoveSCSIDevices removes SCSI device(s) from a Linux host. +func RemoveSCSIDevices(logger klog.Logger, devices ...Device) error { + logger.V(1).Info("Removing SCSI devices", "devices", devices) + + var errs []error + for _, device := range devices { + logger.V(1).Info("Flush SCSI device", "device", device.Name) + if err := device.Exists(); err == nil { + out, err := execCommand("blockdev", "--flushbufs", device.GetPath()).CombinedOutput() + if err != nil { + logger.Error(err, "Command 'blockdev --flushbufs ' did not succeed to flush the device", "device", device.GetPath()) + return errors.New(string(out)) + } + } else if !os.IsNotExist(err) { + return err + } + + logger.V(1).Info("Put SCSI device offline", "device", device.Name) + err := device.Shutdown(logger) + 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 + } + + logger.V(1).Info("Delete SCSI device", "device", device.Name) + err = device.Delete(logger) + 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.") + logger.V(1).Info("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) @@ -445,22 +676,153 @@ 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) { +func GetConnectorFromFile(logger klog.Logger, 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()) + } + if c.MountTargetDevice == nil { + return nil, fmt.Errorf("mountTargetDevice in the connector is nil") + } + if devices, err := GetSCSIDevices(logger, []string{c.MountTargetDevice.GetPath()}, false); err != nil { + return nil, err + } else { + c.MountTargetDevice = &devices[0] + } + + if c.Devices, err = GetSCSIDevices(logger, 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(ctx context.Context) 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(ctx) + 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) } - data := Connector{} - err = json.Unmarshal([]byte(f), &data) + + return filepath.Join("/dev", d.Name) +} + +// WWID returns the WWID of a device +func (d *Device) WWID(ctx context.Context) (string, error) { + timeout := 1 * time.Second + out, err := execWithTimeout(ctx, "scsi_id", []string{"-g", "-u", d.GetPath()}, timeout) if err != nil { - return &Connector{}, err + return "", err } - return &data, nil + 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 &HCTL{ + HBA: hctl[0], + Channel: hctl[1], + Target: hctl[2], + LUN: hctl[3], + }, nil +} + +// WriteDeviceFile write in a device file +func (d *Device) WriteDeviceFile(logger klog.Logger, name string, content string) error { + return writeInSCSIDeviceFile(logger, 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(logger klog.Logger) error { + return d.WriteDeviceFile(logger, "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(logger klog.Logger) error { + return d.WriteDeviceFile(logger, "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(logger klog.Logger) error { + return d.WriteDeviceFile(logger, "rescan", "1") } diff --git a/iscsi/iscsi_test.go b/iscsi/iscsi_test.go index dceec27..2707332 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" @@ -11,9 +13,22 @@ import ( "strconv" "testing" "time" + + "github.com/prashantv/gostub" + "github.com/stretchr/testify/assert" + "k8s.io/klog/v2/ktesting" ) -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 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...) + } +} + +func makeFakeExecWithTimeout(ctx context.Context, withTimeout bool, output []byte, err error) func(context.Context, string, []string, time.Duration) ([]byte, error) { + return func(ctx context.Context, command string, args []string, timeout time.Duration) ([]byte, error) { + if withTimeout { + return nil, context.DeadlineExceeded + } + return output, err + } } -func fakeExecWithTimeout(command string, args []string, timeout time.Duration) ([]byte, error) { - if testCmdTimeout { - return nil, context.DeadlineExceeded +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 + } } - return []byte(testCmdOutput), testExecWithTimeoutError + 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,10 @@ 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 + _, ctx := ktesting.NewTestContext(t) + 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(ctx, false, []byte(fakeOutput), nil)).Reset() + type args struct { tgtPortal string tgtIQN string @@ -259,7 +289,7 @@ func Test_sessionExists(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := sessionExists(tt.args.tgtPortal, tt.args.tgtIQN) + got, err := sessionExists(ctx, tt.args.tgtPortal, tt.args.tgtIQN) if (err != nil) != tt.wantErr { t.Errorf("sessionExists() error = %v, wantErr %v", err, tt.wantErr) return @@ -272,49 +302,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) + _, ctx := ktesting.NewTestContext(t) + device := Device{Name: "test"} + c := Connector{Devices: []Device{device}, MountTargetDevice: &device} + err := c.DisconnectVolume(ctx) 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 +348,500 @@ 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(ctx context.Context, cmd string, args []string, timeout time.Duration) ([]byte, error) { + mockedOutput := []byte("") + if cmd == "scsi_id" { + mockedOutput = []byte(wwid + "\n") + } + return makeFakeExecWithTimeout(ctx, tt.timeout, mockedOutput, nil)(ctx, 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) + + _, ctx := ktesting.NewTestContext(t) + err := c.DisconnectVolume(ctx) 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") } } + }) + } +} + +func Test_waitForPathToExist(t *testing.T) { + logger, _ := ktesting.NewTestContext(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, + }, + } - 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 - } + 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(logger, &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(logger, &path, 0, 0, "tcp") + assert.NotNil(err) + + err = waitForPathToExist(logger, &path, 0, 0, "") + assert.NotNil(err) + + err = waitForPathToExist(logger, nil, 0, 0, "tcp") + assert.NotNil(err) + + err = waitForPathToExist(logger, 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(logger, &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}} + + logger, _ := ktesting.NewTestContext(t) + 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(logger, tt.mockedDevices, false) + + 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}) + + logger, _ := ktesting.NewTestContext(t) + 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(logger, 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() + + logger, _ := ktesting.NewTestContext(t) + c.Persist("/tmp/connector.json") + c2, err := GetConnectorFromFile(logger, "/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(logger, "/tmp/shouldNotExists.json") + assert.NotNil(err) + assert.IsType(&os.PathError{}, err) + + ioutil.WriteFile("/tmp/connector.json", []byte("not a connector"), 0600) + _, err = GetConnectorFromFile(logger, "/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" + + _, ctx := ktesting.NewTestContext(t) + 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(_ context.Context, _ 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(ctx) + 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 68bbf30..97bb6ea 100644 --- a/iscsi/iscsiadm.go +++ b/iscsi/iscsiadm.go @@ -1,9 +1,12 @@ package iscsi import ( - "bytes" + "context" "fmt" "strings" + "time" + + "k8s.io/klog/v2" ) // Secrets provides optional iscsi security credentials (CHAP settings) @@ -20,90 +23,67 @@ 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() +func iscsiCmd(ctx context.Context, args ...string) (string, error) { + logger := klog.FromContext(ctx) + stdout, err := execWithTimeout(ctx, "iscsiadm", args, time.Second*3) - // 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()) - - } - } + logger.V(1).Info("Run iscsiadm", "command", strings.Join(append([]string{"iscsiadm"}, args...), " ")) + iscsiadmDebug(logger, string(stdout), err) - iscsiadmDebug(string(stdout.Bytes()), iscsiadmError) - return string(stdout.Bytes()), iscsiadmError + return string(stdout), err } -func iscsiadmDebug(output string, cmdError error) { +func iscsiadmDebug(logger klog.Logger, output string, cmdError error) { debugOutput := strings.Replace(output, "\n", "\\n", -1) - debug.Printf("Output of iscsiadm command: {output: %s}", debugOutput) + logger.V(1).Info("Output of iscsiadm command", "output", debugOutput) if cmdError != nil { - debug.Printf("Error message returned from iscsiadm command: %s", cmdError.Error()) + logger.Error(cmdError, "Error message returned from iscsiadm command") } } // ListInterfaces returns a list of all iscsi interfaces configured on the node /// along with the raw output in Response.StdOut we add the convenience of // returning a list of entries found -func ListInterfaces() ([]string, error) { - debug.Println("Begin ListInterface...") - out, err := iscsiCmd("-m", "iface", "-o", "show") +func ListInterfaces(ctx context.Context) ([]string, error) { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin ListInterface...") + out, err := iscsiCmd(ctx, "-m", "iface", "-o", "show") return strings.Split(out, "\n"), err } // ShowInterface retrieves the details for the specified iscsi interface // caller should inspect r.Err and use r.StdOut for interface details -func ShowInterface(iface string) (string, error) { - debug.Println("Begin ShowInterface...") - out, err := iscsiCmd("-m", "iface", "-o", "show", "-I", iface) +func ShowInterface(ctx context.Context, iface string) (string, error) { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin ShowInterface...") + out, err := iscsiCmd(ctx, "-m", "iface", "-o", "show", "-I", iface) return out, err } // CreateDBEntry sets up a node entry for the specified tgt in the nodes iscsi nodes db -func CreateDBEntry(tgtIQN, portal, iFace string, discoverySecrets, sessionSecrets Secrets) error { - debug.Println("Begin CreateDBEntry...") +func CreateDBEntry(ctx context.Context, tgtIQN, portal, iFace string, discoverySecrets, sessionSecrets Secrets) error { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin CreateDBEntry...") baseArgs := []string{"-m", "node", "-T", tgtIQN, "-p", portal} - _, err := iscsiCmd(append(baseArgs, []string{"-I", iFace, "-o", "new"}...)...) + _, err := iscsiCmd(ctx, append(baseArgs, "-I", iFace, "-o", "new")...) if err != nil { return err } if discoverySecrets.SecretsType == "chap" { - debug.Printf("Setting CHAP Discovery...") - createCHAPEntries(baseArgs, discoverySecrets, true) + logger.V(1).Info("Setting CHAP Discovery...") + err := createCHAPEntries(ctx, baseArgs, discoverySecrets, true) + if err != nil { + return err + } } if sessionSecrets.SecretsType == "chap" { - debug.Printf("Setting CHAP Session...") - createCHAPEntries(baseArgs, sessionSecrets, false) + logger.V(1).Info("Setting CHAP Session...") + err := createCHAPEntries(ctx, baseArgs, sessionSecrets, false) + if err != nil { + return err + } } return err @@ -111,32 +91,34 @@ func CreateDBEntry(tgtIQN, portal, iFace string, discoverySecrets, sessionSecret } // Discoverydb discovers the iscsi target -func Discoverydb(tp, iface string, discoverySecrets Secrets, chapDiscovery bool) error { - debug.Println("Begin Discoverydb...") +func Discoverydb(ctx context.Context, tp, iface string, discoverySecrets Secrets, chapDiscovery bool) error { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin Discoverydb...") baseArgs := []string{"-m", "discoverydb", "-t", "sendtargets", "-p", tp, "-I", iface} - out, err := iscsiCmd(append(baseArgs, []string{"-o", "new"}...)...) + out, err := iscsiCmd(ctx, append(baseArgs, []string{"-o", "new"}...)...) if err != nil { - return fmt.Errorf("failed to create new entry of target in discoverydb, output: %v, err: %v", string(out), err) + return fmt.Errorf("failed to create new entry of target in discoverydb, output: %v, err: %v", out, err) } if chapDiscovery { - if err := createCHAPEntries(baseArgs, discoverySecrets, true); err != nil { + if err := createCHAPEntries(ctx, baseArgs, discoverySecrets, true); err != nil { return err } } - _, err = iscsiCmd(append(baseArgs, []string{"--discover"}...)...) + _, err = iscsiCmd(ctx, append(baseArgs, []string{"--discover"}...)...) if err != nil { //delete the discoverydb record - iscsiCmd(append(baseArgs, []string{"-o", "delete"}...)...) + iscsiCmd(ctx, append(baseArgs, []string{"-o", "delete"}...)...) return fmt.Errorf("failed to sendtargets to portal %s, err: %v", tp, err) } return nil } -func createCHAPEntries(baseArgs []string, secrets Secrets, discovery bool) error { +func createCHAPEntries(ctx context.Context, baseArgs []string, secrets Secrets, discovery bool) error { args := []string{} - debug.Printf("Begin createCHAPEntries (discovery=%t)...", discovery) + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin createCHAPEntries...", "discovery", discovery) if discovery { args = append(baseArgs, []string{"-o", "update", "-n", "discovery.sendtargets.auth.authmethod", "-v", "CHAP", @@ -163,7 +145,7 @@ func createCHAPEntries(baseArgs []string, secrets Secrets, discovery bool) error } } - _, err := iscsiCmd(args...) + _, err := iscsiCmd(ctx, args...) if err != nil { return fmt.Errorf("failed to update discoverydb with CHAP, err: %v", err) } @@ -172,47 +154,48 @@ func createCHAPEntries(baseArgs []string, secrets Secrets, discovery bool) error } // GetSessions retrieves a list of current iscsi sessions on the node -func GetSessions() (string, error) { - debug.Println("Begin GetSessions...") - out, err := iscsiCmd("-m", "session") +func GetSessions(ctx context.Context) (string, error) { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin GetSessions...") + out, err := iscsiCmd(ctx, "-m", "session") return out, err } // Login performs an iscsi login for the specified target -func Login(tgtIQN, portal string) error { +func Login(ctx context.Context, tgtIQN, portal string) error { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin Login...") baseArgs := []string{"-m", "node", "-T", tgtIQN, "-p", portal} - _, err := iscsiCmd(append(baseArgs, []string{"-l"}...)...) - if err != nil { + if _, err := iscsiCmd(ctx, append(baseArgs, []string{"-l"}...)...); err != nil { //delete the node record from database - iscsiCmd(append(baseArgs, []string{"-o", "delete"}...)...) + iscsiCmd(ctx, 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 { - 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...) - } +// Logout logs out the specified target +func Logout(ctx context.Context, tgtIQN, portal string) error { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin Logout...") + args := []string{"-m", "node", "-T", tgtIQN, "-p", portal, "-u"} + iscsiCmd(ctx, args...) return nil } -// DeleteDBEntry deletes the iscsi db entry fo rthe specified target -func DeleteDBEntry(tgtIQN string) error { - debug.Println("Begin DeleteDBEntry...") +// DeleteDBEntry deletes the iscsi db entry for the specified target +func DeleteDBEntry(ctx context.Context, tgtIQN string) error { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin DeleteDBEntry...") args := []string{"-m", "node", "-T", tgtIQN, "-o", "delete"} - iscsiCmd(args...) + iscsiCmd(ctx, args...) return nil } // DeleteIFace delete the iface -func DeleteIFace(iface string) error { - debug.Println("Begin DeleteIFace...") - iscsiCmd([]string{"-m", "iface", "-I", iface, "-o", "delete"}...) +func DeleteIFace(ctx context.Context, iface string) error { + logger := klog.FromContext(ctx) + logger.V(1).Info("Begin DeleteIFace...") + iscsiCmd(ctx, []string{"-m", "iface", "-I", iface, "-o", "delete"}...) return nil } diff --git a/iscsi/iscsiadm_test.go b/iscsi/iscsiadm_test.go index 3d91976..0b93a77 100644 --- a/iscsi/iscsiadm_test.go +++ b/iscsi/iscsiadm_test.go @@ -1,24 +1,90 @@ package iscsi -import "testing" +import ( + "os/exec" + "testing" + + "github.com/prashantv/gostub" + "github.com/stretchr/testify/assert" + "k8s.io/klog/v2/ktesting" +) + +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 + _, ctx := ktesting.NewTestContext(t) 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 +95,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 +107,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,16 +122,15 @@ 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 - err := Discoverydb(tt.tgtPortal, tt.iface, tt.discoverySecret, tt.chapDiscovery) + defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(ctx, false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset() + err := Discoverydb(ctx, tt.tgtPortal, tt.iface, tt.discoverySecret, tt.chapDiscovery) if (err != nil) != tt.wantErr { t.Errorf("Discoverydb() error = %v, wantErr %v", err, tt.wantErr) return @@ -75,16 +140,16 @@ iscsiadm: Could not perform SendTargets discovery.\n`, } func TestCreateDBEntry(t *testing.T) { - execCommand = fakeExecCommand + _, ctx := ktesting.NewTestContext(t) 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,24 +165,23 @@ 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 - err := CreateDBEntry(tt.tgtIQN, tt.tgtPortal, tt.iface, tt.discoverySecret, tt.sessionSecret) + defer gostub.Stub(&execWithTimeout, makeFakeExecWithTimeout(ctx, false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset() + err := CreateDBEntry(ctx, 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) return @@ -126,3 +190,91 @@ func TestCreateDBEntry(t *testing.T) { } } + +func TestListInterfaces(t *testing.T) { + _, ctx := ktesting.NewTestContext(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(ctx, false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset() + interfaces, err := ListInterfaces(ctx) + + if tt.wantErr { + assert.NotNil(err) + } else { + assert.Nil(err) + assert.Equal(interfaces, tt.interfaces) + } + }) + } +} + +func TestShowInterface(t *testing.T) { + _, ctx := ktesting.NewTestContext(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(ctx, false, []byte(tt.mockedStdout), tt.mockedCmdError)).Reset() + interfaces, err := ShowInterface(ctx, "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..035ef6e 100644 --- a/iscsi/multipath.go +++ b/iscsi/multipath.go @@ -2,86 +2,86 @@ package iscsi import ( "context" - "io/ioutil" + "errors" + "fmt" "os" "os/exec" - "path/filepath" + "strings" "time" -) -var sysBlockPath = "/sys/block" -var devPath = "/dev" + "k8s.io/klog/v2" +) -func ExecWithTimeout(command string, args []string, timeout time.Duration) ([]byte, error) { - debug.Printf("Executing command '%v' with args: '%v'.\n", command, args) +// ExecWithTimeout execute a command with a timeout and returns an error if timeout is exceeded +func ExecWithTimeout(ctx context.Context, command string, args []string, timeout time.Duration) ([]byte, error) { + logger := klog.FromContext(ctx) + logger.V(1).Info("Executing command with timeout", "command", command, "args", args, "timeout", timeout) // Create a new context and add a timeout to it - ctx, cancel := context.WithTimeout(context.Background(), timeout) + ctx, cancel := context.WithTimeout(ctx, timeout) 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() + if err != nil { + logger.Error(err, "Command error", "command", command, "timeout", timeout, "output", out) + } // 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 // happens when a process is killed. - if ctx.Err() == context.DeadlineExceeded { - debug.Printf("Command '%s' timeout reached.\n", command) + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + logger.V(1).Info("Command timeout reached", "command", command, "timeout", timeout) 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) - } - - 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 + var ee *exec.ExitError + if ok := errors.Is(err, ee); ok { + logger.Error(err, "Non-zero exit code", "command", command) + err = fmt.Errorf("%s", ee.Stderr) } - 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 + return out, err } // 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(ctx context.Context, device *Device) error { + devicePath := device.GetPath() + logger := klog.FromContext(ctx) + logger.V(1).Info("Flushing multipath device", "device", devicePath) - fullDevice := filepath.Join(devPath, device) timeout := 5 * time.Second - _, err := execWithTimeout("multipath", []string{"-f", fullDevice}, timeout) + _, err := execWithTimeout(ctx, "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) { + logger.V(1).Info("Multipath device has been removed", "device", 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) + } + logger.Error(err, "Command 'multipath -f ' did not succeed to delete the device", "device", devicePath) return err } } - debug.Printf("Finshed flushing multipath device %v.\n", device) + logger.V(1).Info("Finished flushing multipath device", "device", devicePath) + return nil +} + +// ResizeMultipathDevice resize a multipath device based on its underlying devices +func ResizeMultipathDevice(ctx context.Context, device *Device) error { + logger := klog.FromContext(ctx) + logger.V(1).Info("Resizing multipath device", "device", 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..9167dbc --- /dev/null +++ b/iscsi/multipath_test.go @@ -0,0 +1,81 @@ +package iscsi + +import ( + "context" + "os/exec" + "testing" + "time" + + "github.com/prashantv/gostub" + "github.com/stretchr/testify/assert" + "k8s.io/klog/v2/ktesting" +) + +func TestExecWithTimeout(t *testing.T) { + _, ctx := ktesting.NewTestContext(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(ctx, "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)) + } + }) + } +}