WIP:device_managers: add abstraction for sgdisk and sfdisk

Add an interface to abstract implementation details for
partition management.

TBD: need proper way to pivot implementation currently
hard coded, will eventually be some type of argument

Author: prestist

internal/device_managers/deviceManagers.go

@@ -0,0 +1,28 @@
+package device_managers
+
+import (
+	"github.com/coreos/ignition/v2/config/v3_5_experimental/types"
+)
+
+type DeviceManager interface {
+	CreatePartition(p Partition)
+	DeletePartition(num int)
+	Info(num int)
+	WipeTable(wipe bool)
+	Pretend() (string, error)
+	Commit() error
+	ParseOutput(string, []int) (map[int]Output, error)
+}
+
+type Partition struct {
+	types.Partition
+	StartSector   *int64
+	SizeInSectors *int64
+	StartMiB      string
+	SizeMiB       string
+}
+
+type Output struct {
+	Start int64
+	Size  int64
+}

internal/device_managers/sfdisk/sfdisk.go

@@ -19,9 +19,13 @@ import (
 	"fmt"
 	"io"
 	"os/exec"
+	"regexp"
+	"strconv"
+	"strings"
 
+	sharedErrors "github.com/coreos/ignition/v2/config/shared/errors"
 	"github.com/coreos/ignition/v2/config/util"
-	"github.com/coreos/ignition/v2/config/v3_5_experimental/types"
+	"github.com/coreos/ignition/v2/internal/device_managers"
 	"github.com/coreos/ignition/v2/internal/distro"
 	"github.com/coreos/ignition/v2/internal/log"
 )
@@ -30,30 +34,18 @@ type Operation struct {
 	logger    *log.Logger
 	dev       string
 	wipe      bool
-	parts     []Partition
+	parts     []device_managers.Partition
 	deletions []int
 	infos     []int
 }
 
-// We ignore types.Partition.StartMiB/SizeMiB in favor of
-// StartSector/SizeInSectors.  The caller is expected to do the conversion.
-type Partition struct {
-	types.Partition
-	StartSector   *int64
-	SizeInSectors *int64
-
-	// shadow StartMiB/SizeMiB so they're not accidentally used
-	StartMiB string
-	SizeMiB  string
-}
-
 // Begin begins an sfdisk operation
 func Begin(logger *log.Logger, dev string) *Operation {
 	return &Operation{logger: logger, dev: dev}
 }
 
-// CreatePartition adds the supplied partition to the list of partitions to be created as part of an operation.
-func (op *Operation) CreatePartition(p Partition) {
+// CreatePartition adds the supplied partition to the list of partitions to be created as part of an operation
+func (op *Operation) CreatePartition(p device_managers.Partition) {
 	op.parts = append(op.parts, p)
 }
 
@@ -81,7 +73,7 @@ func (op *Operation) Pretend() (string, error) {
 		return "", err
 	}
 
-	script := op.sfdiskBuildOptions()
+	script := op.buildOptions()
 	cmd := exec.Command("sh", "-c", fmt.Sprintf("echo -e \"%s\" | sudo %s --no-act %s", script, distro.SfdiskCmd(), op.dev))
 	stdout, err := cmd.StdoutPipe()
 
@@ -114,8 +106,8 @@ func (op *Operation) Pretend() (string, error) {
 }
 
 // Commit commits an partitioning operation.
-func (op *Operation) SfdiskCommit() error {
-	script := op.sfdiskBuildOptions()
+func (op *Operation) Commit() error {
+	script := op.buildOptions()
 	if len(script) == 0 {
 		return nil
 	}
@@ -139,7 +131,60 @@ func (op *Operation) SfdiskCommit() error {
 	return nil
 }
 
-func (op Operation) sfdiskBuildOptions() string {
+// ParseOutput takes the output from sfdisk. Similarly to sgdisk
+// it then uses regex to parse the output into understood values like 'start' 'size' and attempts
+// to catch any failures and wrap them to return to the caller.
+func (op *Operation) ParseOutput(sfdiskOutput string, partitionNumbers []int) (map[int]device_managers.Output, error) {
+	if len(partitionNumbers) == 0 {
+		return nil, nil
+	}
+
+	// Prepare the data, and a regex for matching on partitions
+	partitionRegex := regexp.MustCompile(`^/dev/\S+\s+\S*\s+(\d+)\s+(\d+)\s+\d+\s+\S+\s+\S+\s+\S+.*$`)
+	output := map[int]device_managers.Output{}
+	current := device_managers.Output{}
+	i := 0
+	lines := strings.Split(sfdiskOutput, "\n")
+	for _, line := range lines {
+		matches := partitionRegex.FindStringSubmatch(line)
+
+		// Sanity check number of partition entries
+		if i > len(partitionNumbers) {
+			return nil, sharedErrors.ErrBadSfdiskPretend
+		}
+
+		// Verify that we are not reading a 'failed' or 'error'
+		errorRegex := regexp.MustCompile(`(?i)(failed|error)`)
+		if errorRegex.MatchString(line) {
+			return nil, fmt.Errorf("%w: sfdisk returned :%v", sharedErrors.ErrBadSfdiskPretend, line)
+		}
+
+		// When we get a match it should be
+		// Whole line at [0]
+		// Start at [1]
+		// Size at [2]
+		if len(matches) > 2 {
+			start, err := strconv.Atoi(matches[1])
+			if err != nil {
+				return nil, err
+			}
+			end, err := strconv.Atoi(matches[2])
+			if err != nil {
+				return nil, err
+			}
+
+			current.Start = int64(start)
+			// Add one due to overlap
+			current.Size = int64(end - start + 1)
+			output[partitionNumbers[i]] = current
+			i++
+		}
+	}
+
+	return output, nil
+}
+
+func (op Operation) buildOptions() string {
 	var script bytes.Buffer
 
 	for _, p := range op.parts {
@@ -202,75 +247,3 @@ func (op *Operation) handleInfo() error {
 	}
 	return nil
 }
-
-// Copy old functionality from sgdisk to switch between the two during testing.
-// Will be removed.
-func (op *Operation) SgdiskCommit() error {
-	opts := op.sgdiskBuildOptions()
-	if len(opts) == 0 {
-		return nil
-	}
-	op.logger.Info("running sgdisk with options: %v", opts)
-	cmd := exec.Command(distro.SgdiskCmd(), opts...)
-
-	if _, err := op.logger.LogCmd(cmd, "deleting %d partitions and creating %d partitions on %q", len(op.deletions), len(op.parts), op.dev); err != nil {
-		return fmt.Errorf("create partitions failed: %v", err)
-	}
-
-	return nil
-}
-
-// Copy old functionality from sgdisk to switch between the two during testing.
-// Will be removed.
-func (op Operation) sgdiskBuildOptions() []string {
-	opts := []string{}
-
-	if op.wipe {
-		opts = append(opts, "--zap-all")
-	}
-
-	// Do all deletions before creations
-	for _, partition := range op.deletions {
-		opts = append(opts, fmt.Sprintf("--delete=%d", partition))
-	}
-
-	for _, p := range op.parts {
-		opts = append(opts, fmt.Sprintf("--new=%d:%s:+%s", p.Number, partitionGetStart(p), partitionGetSize(p)))
-		if p.Label != nil {
-			opts = append(opts, fmt.Sprintf("--change-name=%d:%s", p.Number, *p.Label))
-		}
-		if util.NotEmpty(p.TypeGUID) {
-			opts = append(opts, fmt.Sprintf("--typecode=%d:%s", p.Number, *p.TypeGUID))
-		}
-		if util.NotEmpty(p.GUID) {
-			opts = append(opts, fmt.Sprintf("--partition-guid=%d:%s", p.Number, *p.GUID))
-		}
-	}
-
-	for _, partition := range op.infos {
-		opts = append(opts, fmt.Sprintf("--info=%d", partition))
-	}
-
-	if len(opts) == 0 {
-		return nil
-	}
-
-	opts = append(opts, op.dev)
-	return opts
-}
-
-// Copy old functionality from sgdisk to switch between the two during testing.
-// Will be removed.
-func partitionGetStart(p Partition) string {
-	if p.StartSector != nil {
-		return fmt.Sprintf("%d", *p.StartSector)
-	}
-	return "0"
-}
-
-func partitionGetSize(p Partition) string {
-	if p.SizeInSectors != nil {
-		return fmt.Sprintf("%d", *p.SizeInSectors)
-	}
-	return "0"
-}

…nal/exec/stages/disks/partitions_test.go …al/device_managers/sfdisk/sfdisk_test.gointernal/exec/stages/disks/partitions_test.go renamed to internal/device_managers/sfdisk/sfdisk_test.go internal/exec/stages/disks/partitions_test.go renamed to internal/device_managers/sfdisk/sfdisk_test.go

@@ -1,11 +1,13 @@
-package disks
+package sfdisk_test
 
 import (
 	"errors"
 	"reflect"
 	"testing"
 
 	internalErrors "github.com/coreos/ignition/v2/config/shared/errors"
+	"github.com/coreos/ignition/v2/internal/device_managers"
+	"github.com/coreos/ignition/v2/internal/device_managers/sfdisk"
 )
 
 func TestPartitionParse(t *testing.T) {
@@ -14,7 +16,7 @@ func TestPartitionParse(t *testing.T) {
 		name             string
 		sfdiskOut        string
 		partitionNumbers []int
-		expectedOutput   map[int]sfdiskOutput
+		expectedOutput   map[int]device_managers.Output
 		expectedError    error
 	}{
 		{
@@ -37,8 +39,8 @@ Device     Boot Start   End Sectors Size Id Type
 /dev/vda1        2048  2057      10   5K 83 Linux
 The partition table is unchanged (--no-act).`,
 			partitionNumbers: []int{1},
-			expectedOutput: map[int]sfdiskOutput{
-				1: {start: 2048, size: 10},
+			expectedOutput: map[int]device_managers.Output{
+				1: {Start: 2048, Size: 10},
 			},
 			expectedError: nil,
 		},
@@ -64,9 +66,9 @@ Device     Boot Start   End Sectors Size Id Type
 /dev/vda2        4096  4105      10   5K 83 Linux
 The partition table is unchanged (--no-act).`,
 			partitionNumbers: []int{1, 2},
-			expectedOutput: map[int]sfdiskOutput{
-				1: {start: 2048, size: 10},
-				2: {start: 4096, size: 10},
+			expectedOutput: map[int]device_managers.Output{
+				1: {Start: 2048, Size: 10},
+				2: {Start: 4096, Size: 10},
 			},
 			expectedError: nil,
 		},
@@ -84,16 +86,16 @@ Failed to add #1 partition: Numerical result out of range
 Leaving.
 `,
 			partitionNumbers: []int{1},
-			expectedOutput: map[int]sfdiskOutput{
-				1: {start: 0, size: 0},
+			expectedOutput: map[int]device_managers.Output{
+				1: {Start: 0, Size: 0},
 			},
 			expectedError: internalErrors.ErrBadSfdiskPretend,
 		},
 	}
-
+	op := sfdisk.Begin(nil, "")
 	for i, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			output, err := parseSfdiskPretend(tt.sfdiskOut, tt.partitionNumbers)
+			output, err := op.ParseOutput(tt.sfdiskOut, tt.partitionNumbers)
 			if tt.expectedError != nil {
 				if !errors.Is(err, tt.expectedError) {
 					t.Errorf("#%d: bad error: result = %v, expected = %v", i, err, tt.expectedError)