partitioners: move and add abstraction for sgdisk and sfdisk

Add an interface to abstract implementation details for
partition management, allowing tooling to pivot between
implementations.

TBD: argument/tag reading for triggering the pivot

Fixes: https://issues.redhat.com/browse/COS-2930

Author: prestist

internal/exec/stages/disks/partitions.go

@@ -20,28 +20,32 @@ package disks
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"sort"
 	"strconv"
 	"strings"
 
-	sharedErrors "github.com/coreos/ignition/v2/config/shared/errors"
 	cutil "github.com/coreos/ignition/v2/config/util"
 	"github.com/coreos/ignition/v2/config/v3_5_experimental/types"
 	"github.com/coreos/ignition/v2/internal/distro"
 	"github.com/coreos/ignition/v2/internal/exec/util"
-	"github.com/coreos/ignition/v2/internal/sgdisk"
+	"github.com/coreos/ignition/v2/internal/log"
+	"github.com/coreos/ignition/v2/internal/partitioners"
+	"github.com/coreos/ignition/v2/internal/partitioners/sfdisk"
+	"github.com/coreos/ignition/v2/internal/partitioners/sgdisk"
 	iutil "github.com/coreos/ignition/v2/internal/util"
 )
 
-var (
-	ErrBadSgdiskOutput = errors.New("sgdisk had unexpected output")
-)
+func getDeviceManager(logger *log.Logger, dev string) partitioners.DeviceManager {
+	// To be replaced with build tag support or something similar.
+	if false {
+		return sgdisk.Begin(logger, dev)
+	}
+	return sfdisk.Begin(logger, dev)
+}
 
 // createPartitions creates the partitions described in config.Storage.Disks.
 func (s stage) createPartitions(config types.Config) error {
@@ -76,7 +80,7 @@ func (s stage) createPartitions(config types.Config) error {
 
 // partitionMatches determines if the existing partition matches the spec given. See doc/operator notes for what
 // what it means for an existing partition to match the spec. spec must have non-zero Start and Size.
-func partitionMatches(existing util.PartitionInfo, spec sgdisk.Partition) error {
+func partitionMatches(existing util.PartitionInfo, spec partitioners.Partition) error {
 	if err := partitionMatchesCommon(existing, spec); err != nil {
 		return err
 	}
@@ -88,13 +92,13 @@ func partitionMatches(existing util.PartitionInfo, spec sgdisk.Partition) error
 
 // partitionMatchesResize returns if the existing partition should be resized by evaluating if
 // `resize`field is true and partition matches in all respects except size.
-func partitionMatchesResize(existing util.PartitionInfo, spec sgdisk.Partition) bool {
+func partitionMatchesResize(existing util.PartitionInfo, spec partitioners.Partition) bool {
 	return cutil.IsTrue(spec.Resize) && partitionMatchesCommon(existing, spec) == nil
 }
 
 // partitionMatchesCommon handles the common tests (excluding the partition size) to determine
 // if the existing partition matches the spec given.
-func partitionMatchesCommon(existing util.PartitionInfo, spec sgdisk.Partition) error {
+func partitionMatchesCommon(existing util.PartitionInfo, spec partitioners.Partition) error {
 	if spec.Number != existing.Number {
 		return fmt.Errorf("partition numbers did not match (specified %d, got %d). This should not happen, please file a bug.", spec.Number, existing.Number)
 	}
@@ -114,7 +118,7 @@ func partitionMatchesCommon(existing util.PartitionInfo, spec sgdisk.Partition)
 }
 
 // partitionShouldBeInspected returns if the partition has zeroes that need to be resolved to sectors.
-func partitionShouldBeInspected(part sgdisk.Partition) bool {
+func partitionShouldBeInspected(part partitioners.Partition) bool {
 	if part.Number == 0 {
 		return false
 	}
@@ -134,17 +138,17 @@ func convertMiBToSectors(mib *int, sectorSize int) *int64 {
 // getRealStartAndSize returns a map of partition numbers to a struct that contains what their real start
 // and end sector should be. It runs sgdisk --pretend to determine what the partitions would look like if
 // everything specified were to be (re)created.
-func (s stage) getRealStartAndSize(dev types.Disk, devAlias string, diskInfo util.DiskInfo) ([]sgdisk.Partition, error) {
-	partitions := []sgdisk.Partition{}
+func (s stage) getRealStartAndSize(dev types.Disk, devAlias string, diskInfo util.DiskInfo) ([]partitioners.Partition, error) {
+	partitions := []partitioners.Partition{}
 	for _, cpart := range dev.Partitions {
-		partitions = append(partitions, sgdisk.Partition{
+		partitions = append(partitions, partitioners.Partition{
 			Partition:     cpart,
 			StartSector:   convertMiBToSectors(cpart.StartMiB, diskInfo.LogicalSectorSize),
 			SizeInSectors: convertMiBToSectors(cpart.SizeMiB, diskInfo.LogicalSectorSize),
 		})
 	}
 
-	op := sgdisk.Begin(s.Logger, devAlias)
+	op := getDeviceManager(s.Logger, devAlias)
 	for _, part := range partitions {
 		if info, exists := diskInfo.GetPartition(part.Number); exists {
 			// delete all existing partitions
@@ -177,175 +181,29 @@ func (s stage) getRealStartAndSize(dev types.Disk, devAlias string, diskInfo uti
 	if err != nil {
 		return nil, err
 	}
-
-	realDimensions, err := parseSfdiskPretend(output, partitionsToInspect)
+	realDimensions, err := op.ParseOutput(output, partitionsToInspect)
 	if err != nil {
 		return nil, err
 	}
 
-	result := []sgdisk.Partition{}
+	result := []partitioners.Partition{}
 	for _, part := range partitions {
 		if dims, ok := realDimensions[part.Number]; ok {
 			if part.StartSector != nil {
-				part.StartSector = &dims.start
+				part.StartSector = &dims.Start
 			}
 			if part.SizeInSectors != nil {
-				part.SizeInSectors = &dims.size
+				part.SizeInSectors = &dims.Size
 			}
 		}
 		result = append(result, part)
 	}
 	return result, nil
 }
 
-type sgdiskOutput struct {
-	start int64
-	size  int64
-}
-
-// parseLine takes a regexp that captures an int64 and a string to match on. On success it returns
-// the captured int64 and nil. If the regexp does not match it returns -1 and nil. If it encountered
-// an error it returns 0 and the error.
-func parseLine(r *regexp.Regexp, line string) (int64, error) {
-	matches := r.FindStringSubmatch(line)
-	switch len(matches) {
-	case 0:
-		return -1, nil
-	case 2:
-		return strconv.ParseInt(matches[1], 10, 64)
-	default:
-		return 0, ErrBadSgdiskOutput
-	}
-}
-
-// parseSgdiskPretend parses the output of running sgdisk pretend with --info specified for each partition
-// number specified in partitionNumbers. E.g. if paritionNumbers is [1,4,5], it is expected that the sgdisk
-// output was from running `sgdisk --pretend <commands> --info=1 --info=4 --info=5`. It assumes the the
-// partition labels are well behaved (i.e. contain no control characters). It returns a list of partitions
-// matching the partition numbers specified, but with the start and size information as determined by sgdisk.
-// The partition numbers need to passed in because sgdisk includes them in its output.
-func parseSgdiskPretend(sgdiskOut string, partitionNumbers []int) (map[int]sgdiskOutput, error) {
-	if len(partitionNumbers) == 0 {
-		return nil, nil
-	}
-	startRegex := regexp.MustCompile(`^First sector: (\d*) \(.*\)$`)
-	endRegex := regexp.MustCompile(`^Last sector: (\d*) \(.*\)$`)
-	const (
-		START             = iota
-		END               = iota
-		FAIL_ON_START_END = iota
-	)
-
-	output := map[int]sgdiskOutput{}
-	state := START
-	current := sgdiskOutput{}
-	i := 0
-
-	lines := strings.Split(sgdiskOut, "\n")
-	for _, line := range lines {
-		switch state {
-		case START:
-			start, err := parseLine(startRegex, line)
-			if err != nil {
-				return nil, err
-			}
-			if start != -1 {
-				current.start = start
-				state = END
-			}
-		case END:
-			end, err := parseLine(endRegex, line)
-			if err != nil {
-				return nil, err
-			}
-			if end != -1 {
-				current.size = 1 + end - current.start
-				output[partitionNumbers[i]] = current
-				i++
-				if i == len(partitionNumbers) {
-					state = FAIL_ON_START_END
-				} else {
-					current = sgdiskOutput{}
-					state = START
-				}
-			}
-		case FAIL_ON_START_END:
-			if len(startRegex.FindStringSubmatch(line)) != 0 ||
-				len(endRegex.FindStringSubmatch(line)) != 0 {
-				return nil, ErrBadSgdiskOutput
-			}
-		}
-	}
-
-	if state != FAIL_ON_START_END {
-		// We stopped parsing in the middle of a info block. Something is wrong
-		return nil, ErrBadSgdiskOutput
-	}
-
-	return output, nil
-}
-
-type sfdiskOutput struct {
-	start int64
-	size  int64
-}
-
-// ParsePretend takes the output from sfdisk running with the argument --no-act. Similar 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 parseSfdiskPretend(sfdiskOut string, partitionNumbers []int) (map[int]sfdiskOutput, 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]sfdiskOutput{}
-	current := sfdiskOutput{}
-	i := 0
-	lines := strings.Split(sfdiskOut, "\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) > 1 {
-			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
-}
-
 // partitionShouldExist returns whether a bool is indicating if a partition should exist or not.
 // nil (unspecified in json) is treated the same as true.
-func partitionShouldExist(part sgdisk.Partition) bool {
+func partitionShouldExist(part partitioners.Partition) bool {
 	return !cutil.IsFalse(part.ShouldExist)
 }
 
@@ -497,7 +355,7 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error {
 		return fmt.Errorf("refusing to operate on directly active disk %q", devAlias)
 	}
 	if cutil.IsTrue(dev.WipeTable) {
-		op := sgdisk.Begin(s.Logger, devAlias)
+		op := getDeviceManager(s.Logger, devAlias)
 		s.Logger.Info("wiping partition table requested on %q", devAlias)
 		if len(activeParts) > 0 {
 			return fmt.Errorf("refusing to wipe active disk %q", devAlias)
@@ -516,7 +374,7 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error {
 	// Ensure all partitions with number 0 are last
 	sort.Stable(PartitionList(dev.Partitions))
 
-	op := sgdisk.Begin(s.Logger, devAlias)
+	op := getDeviceManager(s.Logger, devAlias)
 
 	diskInfo, err := s.getPartitionMap(devAlias)
 	if err != nil {

internal/partitioners/partitioners.go

@@ -0,0 +1,28 @@
+package partitioners
+
+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
+}