fix(driver): get last partition of specific device (#32)

- get last partition of specific device
- add more tests

Author: peknur

.gitignore

@@ -1,4 +1,5 @@
 cmd/upcloud-csi-plugin/upcloud-csi-plugin
 /go.work
 /go.work.sum
-.vscode
+.vscode
+temp/ 

driver/healthcheck.go

@@ -33,7 +33,8 @@ func NewHealthChecker(checks ...HealthCheck) *HealthChecker {
 func (c *HealthChecker) Check(ctx context.Context) error {
 	var eg errgroup.Group
 
-	for _, check := range c.checks {
+	for _, c := range c.checks {
+		check := c
 		eg.Go(func() error {
 			return check.Check(ctx)
 		})

driver/mounter.go

@@ -1,17 +1,17 @@
 package driver
 
 import (
-	"bytes"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
-	"k8s.io/mount-utils"
 	"os"
 	"os/exec"
 	"strings"
 	"syscall"
 
+	"k8s.io/mount-utils"
+
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
@@ -91,16 +91,17 @@ func (m *mounter) Format(source, fsType string, mkfsArgs []string) error {
 		return errors.New("source is not specified for formatting the volume")
 	}
 
-	m.log.Infof("source: %s", source)
-
-	m.log.Info("create partition called")
-	err := createPartition(source)
+	m.log.WithFields(logrus.Fields{"source": source}).Info("create partition called")
+	err := m.createPartition(source)
 	if err != nil {
 		return err
 	}
 
-	m.log.Info("get last partition called")
-	lastPartition := getLastPartition()
+	m.log.WithFields(logrus.Fields{"source": source}).Info("get last partition called")
+	lastPartition, err := getLastPartition(source)
+	if err != nil {
+		return err
+	}
 
 	if fsType == "ext4" || fsType == "ext3" {
 		mkfsArgs = append(mkfsArgs, "-F", lastPartition)
@@ -117,8 +118,9 @@ func (m *mounter) Format(source, fsType string, mkfsArgs []string) error {
 	}
 
 	m.log.WithFields(logrus.Fields{
-		"cmd":  mkfsCmd,
-		"args": mkfsArgs,
+		"cmd":       mkfsCmd,
+		"args":      mkfsArgs,
+		"partition": lastPartition,
 	}).Info("executing format command")
 
 	out, err := exec.Command(mkfsCmd, mkfsArgs...).CombinedOutput()
@@ -176,11 +178,16 @@ func (m *mounter) Mount(source, target, fsType string, opts ...string) error {
 }
 
 func (m *mounter) Unmount(target string) error {
-	umountCmd := "umount"
 	if target == "" {
 		return errors.New("target is not specified for unmounting the volume")
 	}
 
+	if _, err := os.Stat(target); os.IsNotExist(err) {
+		m.log.WithFields(logrus.Fields{"target": target}).Info("unmount target does not exist")
+		return nil
+	}
+
+	umountCmd := "umount"
 	umountArgs := []string{target}
 
 	m.log.WithFields(logrus.Fields{
@@ -377,75 +384,49 @@ func (m *mounter) GetDeviceName(mounter mount.Interface, mountPath string) (stri
 	return devicePath, err
 }
 
-func getLastPartition() string {
-	sfdisk := exec.Command("sfdisk", "-q", "--list")
-	awk := exec.Command("awk", "NR>1{print $1}")
+func (m *mounter) createPartition(device string) error {
 
-	r, w := io.Pipe()
-	sfdisk.Stdout = w
-	awk.Stdin = r
-
-	var buf bytes.Buffer
-	awk.Stdout = &buf
-
-	sfdisk.Start()
-	awk.Start()
-	sfdisk.Wait()
-	w.Close()
-	awk.Wait()
-
-	out := buf.String()
-	partitions := strings.Split(out, "\n")
-
-	var lastPartition string
-	for i := len(partitions) - 1; i > 0; i-- {
-		if !strings.HasPrefix(partitions[i], "/dev") {
-			continue
-		} else {
-			lastPartition = partitions[i]
-			break
-		}
+	partedMklabel := exec.Command("parted", device, "mklabel", "gpt")
+	partedMklabelOut, err := partedMklabel.CombinedOutput()
+	if err != nil {
+		return err
 	}
+	m.log.WithFields(logrus.Fields{
+		"cmd":    partedMklabel.String(),
+		"output": string(partedMklabelOut)},
+	).Info("created new disklabel")
 
-	return lastPartition
-}
-
-func getLastDevice() string {
-	lsblk := exec.Command("lsblk", "-dp")
-	awk := exec.Command("awk", "NR>1{print $1}")
-
-	r, w := io.Pipe()
-	lsblk.Stdout = w
-	awk.Stdin = r
-
-	var buf bytes.Buffer
-	awk.Stdout = &buf
-
-	lsblk.Start()
-	awk.Start()
-	lsblk.Wait()
-	w.Close()
-	awk.Wait()
+	partedCreatePartition := exec.Command("parted", "-a", "opt", device, "mkpart", "primary", "2048s", "100%")
+	partedCreatePartitionOut, err := partedCreatePartition.CombinedOutput()
+	if err != nil {
+		return err
+	}
 
-	out := buf.String()
-	disks := strings.Split(out, "\n")
-	lastDevice := disks[len(disks)-1]
+	m.log.WithFields(logrus.Fields{
+		"cmd":    partedCreatePartition.String(),
+		"output": string(partedCreatePartitionOut)},
+	).Info("created new primary partition")
 
-	return lastDevice
+	return nil
 }
 
-func createPartition(device string) error {
-	partedMklabelOut, err := exec.Command("parted", device, "mklabel", "gpt").CombinedOutput()
+func getLastPartition(source string) (string, error) {
+	sfdisk, err := exec.Command("sfdisk", "-q", "--list", "-o", "device", source).CombinedOutput()
 	if err != nil {
-		return err
+		return "", err
 	}
-	fmt.Printf("parted mklabel output: %s\n", partedMklabelOut)
+	return sfdiskOutputGetLastPartition(source, string(sfdisk))
+}
 
-	partedCreatePartitionOut, err := exec.Command("parted", "-a", "opt", device, "mkpart", "primary", "2048s", "100%").CombinedOutput()
-	if err != nil {
-		return err
+func sfdiskOutputGetLastPartition(source, sfdiskOutput string) (string, error) {
+	outLines := strings.Split(sfdiskOutput, "\n")
+	var lastPartition string
+	for i := len(outLines) - 1; i >= 0; i-- {
+		partition := strings.TrimSpace(outLines[i])
+		if strings.HasPrefix(partition, source) {
+			lastPartition = partition
+			break
+		}
 	}
-	fmt.Printf("mkpart output: %s\n", partedCreatePartitionOut)
-
-	return nil
+	return lastPartition, nil
 }

driver/mounter_test.go

@@ -0,0 +1,36 @@
+package driver
+
+import "testing"
+
+func TestSfdiskOutputGetLastPartition(t *testing.T) {
+	outputMultiple := `
+		Device
+		/dev/vda1
+		/dev/vda2
+		/dev/vda3
+	`
+	outputSingle := `
+		Device
+		/dev/vda1
+	`
+	outputNone := `
+		Device
+	`
+	want := "/dev/vda3"
+	got, _ := sfdiskOutputGetLastPartition("/dev/vda", outputMultiple)
+	if want != got {
+		t.Errorf("sfdiskOutputGetLastPartition failed want %s got %s", want, got)
+	}
+
+	want = "/dev/vda1"
+	got, _ = sfdiskOutputGetLastPartition("/dev/vda", outputSingle)
+	if want != got {
+		t.Errorf("sfdiskOutputGetLastPartition failed want %s got %s", want, got)
+	}
+
+	want = ""
+	got, _ = sfdiskOutputGetLastPartition("/dev/vda", outputNone)
+	if want != got {
+		t.Errorf("sfdiskOutputGetLastPartition failed want %s got %s", want, got)
+	}
+}

driver/node.go

@@ -133,15 +133,19 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 	}
 
 	if !mounted {
-		source = getLastPartition()
+		partition, err := getLastPartition(source)
+		if err != nil {
+			return nil, status.Error(codes.Internal, err.Error())
+		}
 		stageMountedLog := d.log.WithFields(logrus.Fields{
-			"source":  source,
-			"target":  target,
-			"fsType":  fsType,
-			"options": options,
+			"source":    source,
+			"partition": partition,
+			"target":    target,
+			"fsType":    fsType,
+			"options":   options,
 		})
 		stageMountedLog.Info("mount options")
-		if err := d.mounter.Mount(source, target, fsType, options...); err != nil {
+		if err := d.mounter.Mount(partition, target, fsType, options...); err != nil {
 			return nil, status.Error(codes.Internal, err.Error())
 		}
 	} else {
@@ -289,21 +293,21 @@ func (d *Driver) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublish
 // NodeGetCapabilities returns the supported capabilities of the node server
 func (d *Driver) NodeGetCapabilities(ctx context.Context, req *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) {
 	nscaps := []*csi.NodeServiceCapability{
-		&csi.NodeServiceCapability{
+		{
 			Type: &csi.NodeServiceCapability_Rpc{
 				Rpc: &csi.NodeServiceCapability_RPC{
 					Type: csi.NodeServiceCapability_RPC_STAGE_UNSTAGE_VOLUME,
 				},
 			},
 		},
-		&csi.NodeServiceCapability{
+		{
 			Type: &csi.NodeServiceCapability_Rpc{
 				Rpc: &csi.NodeServiceCapability_RPC{
 					Type: csi.NodeServiceCapability_RPC_EXPAND_VOLUME,
 				},
 			},
 		},
-		&csi.NodeServiceCapability{
+		{
 			Type: &csi.NodeServiceCapability_Rpc{
 				Rpc: &csi.NodeServiceCapability_RPC{
 					Type: csi.NodeServiceCapability_RPC_GET_VOLUME_STATS,
@@ -377,13 +381,13 @@ func (d *Driver) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVolumeS
 
 	return &csi.NodeGetVolumeStatsResponse{
 		Usage: []*csi.VolumeUsage{
-			&csi.VolumeUsage{
+			{
 				Available: stats.availableBytes,
 				Total:     stats.totalBytes,
 				Used:      stats.usedBytes,
 				Unit:      csi.VolumeUsage_BYTES,
 			},
-			&csi.VolumeUsage{
+			{
 				Available: stats.availableInodes,
 				Total:     stats.totalInodes,
 				Used:      stats.usedInodes,
@@ -399,6 +403,7 @@ func (d *Driver) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolume
 		return nil, status.Error(codes.InvalidArgument, "NodeExpandVolume volume ID not provided")
 	}
 
+	source := d.getDiskSource(req.VolumeId)
 	volumePath := req.GetVolumePath()
 	if len(volumePath) == 0 {
 		return nil, status.Error(codes.InvalidArgument, "NodeExpandVolume volume path not provided")
@@ -413,6 +418,7 @@ func (d *Driver) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolume
 		"volume_id":   req.VolumeId,
 		"volume_path": req.VolumePath,
 		"method":      "node_expand_volume",
+		"source":      source,
 	})
 	log.Info("node expand volume called")
 
@@ -427,7 +433,12 @@ func (d *Driver) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolume
 		return nil, err
 	}
 
-	if err = d.mounter.Mount(getLastPartition(), stagingPath, "ext4"); err != nil {
+	lastPartition, err := getLastPartition(source)
+	if err != nil {
+		return nil, err
+	}
+
+	if err = d.mounter.Mount(lastPartition, stagingPath, "ext4"); err != nil {
 		return nil, err
 	}
 
@@ -442,17 +453,33 @@ func (d *Driver) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolume
 
 // getDiskSource returns the absolute path of the attached volume for the given volumeID
 func (d *Driver) getDiskSource(volumeID string) string {
-	fullId := strings.Join(strings.Split(volumeID, "-"), "")
-	if len(fullId) <= 20 {
+	diskID := volumeIDToDiskID(volumeID)
+	if diskID == "" {
 		return ""
 	}
+	return getDiskByID(diskID, "")
+}
 
-	link, err := os.Readlink(filepath.Join(diskIDPath, diskPrefix+fullId[:20]))
+func getDiskByID(diskID, basePath string) string {
+	if basePath == "" {
+		basePath = diskIDPath
+	}
+	link, err := os.Readlink(filepath.Join(basePath, diskPrefix+diskID))
 	if err != nil {
 		fmt.Println(fmt.Errorf("failed to get the link to source"))
 		return ""
 	}
-	source := "/dev" + strings.TrimPrefix(link, "../..")
+	if filepath.IsAbs(link) {
+		return link
+	}
+
+	return filepath.Join(basePath, link)
+}
 
-	return source
+func volumeIDToDiskID(volumeID string) string {
+	fullId := strings.Join(strings.Split(volumeID, "-"), "")
+	if len(fullId) <= 20 {
+		return ""
+	}
+	return fullId[:20]
 }