refactor lsblk

- include error code
- remove unused properties
- retrieve specific columns including device size
- always output as JSON

Author: plaffitt

iscsi/iscsi.go

@@ -56,10 +56,8 @@ type Device struct {
 	Hctl      string   `json:"hctl"`
 	Children  []Device `json:"children"`
 	Type      string   `json:"type"`
-	Vendor    string   `json:"vendor"`
-	Model     string   `json:"model"`
-	Revision  string   `json:"rev"`
 	Transport string   `json:"tran"`
+	Size      string   `json:"size,omitempty"`
 }
 
 // Connector provides a struct to hold all of the needed parameters to make our iSCSI connection
@@ -216,17 +214,8 @@ func pathExists(devicePath *string, deviceTransport string) error {
 // getMultipathDevice returns a multipath device for the configured targets if it exists
 func getMultipathDevice(devices []Device) (*Device, error) {
 	var multipathDevice *Device
-	var devicePaths []string
 
 	for _, device := range devices {
-		devicePaths = append(devicePaths, device.GetPath())
-	}
-	deviceInfo, err := lsblk("", devicePaths)
-	if err != nil {
-		return nil, err
-	}
-
-	for _, device := range deviceInfo.BlockDevices {
 		if len(device.Children) != 1 {
 			return nil, fmt.Errorf("device is not mapped to exactly one multipath device: %v", device.Children)
 		}
@@ -454,7 +443,7 @@ func (c *Connector) IsMultipathEnabled() bool {
 func GetSCSIDevices(devicePaths []string) ([]Device, error) {
 	debug.Printf("Getting info about SCSI devices %s.\n", devicePaths)
 
-	deviceInfo, err := lsblk("-S", devicePaths)
+	deviceInfo, err := lsblk(devicePaths)
 	if err != nil {
 		debug.Printf("An error occured while looking info about SCSI devices: %v", err)
 		return nil, err
@@ -482,24 +471,19 @@ func GetISCSIDevices(devicePaths []string) (devices []Device, err error) {
 }
 
 // lsblk execute the lsblk commands
-func lsblkRaw(flags string, devicePaths []string) ([]byte, error) {
-	out, err := execCommand("lsblk", append([]string{flags}, devicePaths...)...).CombinedOutput()
-	debug.Printf("lsblk %s %s", flags, strings.Join(devicePaths, " "))
-	if err != nil {
-		return nil, fmt.Errorf("lsblk: %s", string(out))
-	}
-
-	return out, nil
-}
-
-func lsblk(flags string, devicePaths []string) (*deviceInfo, error) {
-	var deviceInfo deviceInfo
-
-	out, err := lsblkRaw("-J "+flags, devicePaths)
+func lsblk(devicePaths []string) (*deviceInfo, error) {
+	flags := []string{"-J", "-o", "NAME,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 {
+			return nil, fmt.Errorf("%s, (%v)", strings.Trim(string(ee.Stderr), "\n"), ee)
+		}
 		return nil, err
 	}
 
+	var deviceInfo deviceInfo
 	if err = json.Unmarshal(out, &deviceInfo); err != nil {
 		return nil, err
 	}
@@ -589,16 +573,30 @@ func (c *Connector) Persist(filePath string) error {
 func GetConnectorFromFile(filePath string) (*Connector, error) {
 	f, err := ioutil.ReadFile(filePath)
 	if err != nil {
-		return &Connector{}, err
-
+		return nil, err
 	}
-	data := Connector{}
-	err = json.Unmarshal(f, &data)
+	c := Connector{}
+	err = json.Unmarshal([]byte(f), &c)
 	if err != nil {
-		return &Connector{}, err
+		return nil, err
+	}
+
+	devicePaths := []string{}
+	for _, device := range c.Devices {
+		devicePaths = append(devicePaths, device.GetPath())
+	}
+
+	if devices, err := GetSCSIDevices([]string{c.MountTargetDevice.GetPath()}); err != nil {
+		return nil, err
+	} else {
+		c.MountTargetDevice = &devices[0]
+	}
+
+	if c.Devices, err = GetSCSIDevices(devicePaths); err != nil {
+		return nil, err
 	}
 
-	return &data, nil
+	return &c, nil
 }
 
 // Exists check if the device exists at its path and returns an error otherwise

iscsi/iscsi_test.go

@@ -586,6 +586,58 @@ func Test_getMultipathDevice(t *testing.T) {
 	}
 }
 
+func Test_lsblk(t *testing.T) {
+	sda := Device{Name: "sda", Children: []Device{{}}}
+
+	tests := map[string]struct {
+		devicePaths      []string
+		mockedStdout     string
+		mockedDevices    []Device
+		mockedExitStatus int
+		wantErr          bool
+	}{
+		"Basic": {
+			devicePaths:   []string{"/dev/sda"},
+			mockedDevices: []Device{sda},
+		},
+		"NotABlockDevice": {
+			devicePaths:      []string{"/dev/sdzz"},
+			mockedStdout:     "lsblk: sdzz: not a block device",
+			mockedExitStatus: 32,
+		},
+		"InvalidJson": {
+			mockedStdout:     "{",
+			mockedExitStatus: 0,
+			wantErr:          true,
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			mockedStdout := tt.mockedStdout
+			if mockedStdout == "" {
+				out, err := json.Marshal(deviceInfo{BlockDevices: tt.mockedDevices})
+				assert.Nil(err, "could not setup test")
+				mockedStdout = string(out)
+			}
+
+			defer gostub.Stub(&execCommand, makeFakeExecCommand(tt.mockedExitStatus, mockedStdout)).Reset()
+			deviceInfo, err := lsblk(tt.devicePaths)
+
+			if tt.mockedExitStatus != 0 || tt.wantErr {
+				assert.Nil(deviceInfo)
+				assert.NotNil(err)
+			} else {
+				assert.NotNil(deviceInfo)
+				assert.Equal(tt.mockedDevices, deviceInfo.BlockDevices)
+				assert.Nil(err)
+			}
+		})
+	}
+}
+
 func TestConnectorPersistance(t *testing.T) {
 	assert := assert.New(t)
 
@@ -600,19 +652,13 @@ func TestConnectorPersistance(t *testing.T) {
 		Name:      "child name",
 		Hctl:      "child hctl",
 		Type:      "child type",
-		Vendor:    "child vendor",
-		Model:     "child model",
-		Revision:  "child revision",
 		Transport: "child transport",
 	}
 	device := Device{
 		Name:      "device name",
 		Hctl:      "device hctl",
 		Children:  []Device{childDevice},
 		Type:      "device type",
-		Vendor:    "device vendor",
-		Model:     "device model",
-		Revision:  "device revision",
 		Transport: "device transport",
 	}
 	c := Connector{
@@ -624,13 +670,25 @@ func TestConnectorPersistance(t *testing.T) {
 		SessionSecrets:    secret,
 		Interface:         "fake interface",
 		MountTargetDevice: &device,
-		Devices:           []Device{device, childDevice},
+		Devices:           []Device{childDevice},
 		RetryCount:        24,
 		CheckInterval:     13,
 		DoDiscovery:       true,
 		DoCHAPDiscovery:   true,
 	}
 
+	defer gostub.Stub(&execCommand, func(cmd string, args ...string) *exec.Cmd {
+		mockedDevice := device
+		if args[3] == "/dev/child name" {
+			mockedDevice = childDevice
+		}
+
+		mockedOutput, err := json.Marshal(deviceInfo{BlockDevices: []Device{mockedDevice}})
+		assert.Nil(err, "could not setup test")
+
+		return makeFakeExecCommand(0, string(mockedOutput))(cmd, args...)
+	}).Reset()
+
 	c.Persist("/tmp/connector.json")
 	c2, err := GetConnectorFromFile("/tmp/connector.json")
 	assert.Nil(err)