Improve get partitions and reset spec (#530)

Co-authored-by: Dimitris Karakasilis <dimitris@karakasilis.me>

pkg/action/reset.go

@@ -114,6 +114,7 @@ func (r ResetAction) Run() (err error) {
 	defer func() { err = cleanup.Cleanup(err) }()
 
 	// Unmount partitions if any is already mounted before formatting
+	// TODO: Is this needed???
 	err = e.UnmountPartitions(r.spec.Partitions.PartitionsByMountPoint(true, r.spec.Partitions.Recovery))
 	if err != nil {
 		return err
@@ -129,6 +130,10 @@ func (r ResetAction) Run() (err error) {
 	if r.spec.FormatPersistent {
 		persistent := r.spec.Partitions.Persistent
 		if persistent != nil {
+			err = e.UnmountPartitions(r.spec.Partitions.PartitionsByMountPoint(true, persistent))
+			if err != nil {
+				return err
+			}
 			err = e.FormatPartition(persistent)
 			if err != nil {
 				return err
@@ -140,6 +145,11 @@ func (r ResetAction) Run() (err error) {
 	if r.spec.FormatOEM {
 		oem := r.spec.Partitions.OEM
 		if oem != nil {
+			// Try to umount
+			err = e.UnmountPartitions(r.spec.Partitions.PartitionsByMountPoint(true, oem))
+			if err != nil {
+				return err
+			}
 			err = e.FormatPartition(oem)
 			if err != nil {
 				return err

pkg/config/spec.go

@@ -438,34 +438,18 @@ func NewResetSpec(cfg *Config) (*v1.ResetSpec, error) {
 
 	target := ep.State.Disk
 
-	// OEM partition is not a hard requirement
-	if ep.OEM != nil {
-		if ep.OEM.MountPoint == "" {
-			ep.OEM.MountPoint = constants.OEMDir
-		}
-		ep.OEM.Name = constants.OEMPartName
-	} else {
+	// OEM partition is not a hard requirement for reset unless we have the reset oem flag
+	cfg.Logger.Info(litter.Sdump(ep.OEM))
+	if ep.OEM == nil {
 		// We could have oem in lvm which won't appear in ghw list
 		ep.OEM = partitions.GetPartitionViaDM(cfg.Fs, constants.OEMLabel)
 	}
 
-	if ep.OEM == nil {
-		cfg.Logger.Warnf("no OEM partition found")
-	}
-
 	// Persistent partition is not a hard requirement
-	if ep.Persistent != nil {
-		if ep.Persistent.MountPoint == "" {
-			ep.Persistent.MountPoint = constants.PersistentDir
-		}
-		ep.Persistent.Name = constants.PersistentPartName
-	} else {
+	if ep.Persistent == nil {
 		// We could have persistent encrypted or in lvm which won't appear in ghw list
 		ep.Persistent = partitions.GetPartitionViaDM(cfg.Fs, constants.PersistentLabel)
 	}
-	if ep.Persistent == nil {
-		cfg.Logger.Warnf("no Persistent partition found")
-	}
 
 	recoveryImg := filepath.Join(constants.RunningStateDir, "cOS", constants.RecoveryImgFile)
 	recoveryImg2 := filepath.Join(constants.RunningRecoveryStateDir, "cOS", constants.RecoveryImgFile)
@@ -522,6 +506,25 @@ func NewResetSpec(cfg *Config) (*v1.ResetSpec, error) {
 		return nil, fmt.Errorf("failed unmarshalling the full spec: %w", err)
 	}
 
+	if ep.OEM == nil && spec.FormatOEM {
+		cfg.Logger.Warnf("no OEM partition found, won't format it")
+	}
+
+	if ep.Persistent == nil && spec.FormatPersistent {
+		cfg.Logger.Warnf("no Persistent partition found, won't format it")
+	}
+
+	// If we mount partitions by the /dev/disk/by-label stanza, their mountpoints wont show up due to ghw not
+	// accounting for them. Normally this is not an issue but in this case, if we are formatting a given partition we
+	// want to unmount it first. Thus we need to make sure that if its mounted we have the mountpoint info
+	if ep.Persistent.MountPoint == "" {
+		ep.Persistent.MountPoint = partitions.GetMountPointByLabel(ep.Persistent.FilesystemLabel)
+	}
+
+	if ep.OEM.MountPoint == "" {
+		ep.OEM.MountPoint = partitions.GetMountPointByLabel(ep.OEM.FilesystemLabel)
+	}
+
 	return spec, nil
 }
 

pkg/elemental/elemental.go

@@ -47,14 +47,7 @@ func NewElemental(config *agentConfig.Config) *Elemental {
 
 // FormatPartition will format an already existing partition
 func (e *Elemental) FormatPartition(part *v1.Partition, opts ...string) error {
-	var name string
-	// Nice display name for logs
-	if part.Name == "" {
-		name = part.FilesystemLabel
-	} else {
-		name = part.Name
-	}
-	e.config.Logger.Infof("Formatting '%s' partition", name)
+	e.config.Logger.Infof("Formatting '%s' partition", part.FilesystemLabel)
 	return partitioner.FormatDevice(e.config.Runner, part.Path, part.FS, part.FilesystemLabel, opts...)
 }
 

pkg/utils/partitions/getpartitions.go

@@ -17,7 +17,10 @@ limitations under the License.
 package partitions
 
 import (
+	"bufio"
 	"fmt"
+	"io"
+	"os"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -61,6 +64,59 @@ func GetAllPartitions() (v1.PartitionList, error) {
 	return parts, nil
 }
 
+// GetMountPointByLabel will try to get the mountpoint by using the label only
+// so we can identify mounts the have been mounted with /dev/disk/by-label stanzas
+func GetMountPointByLabel(label string) string {
+	// mount entries for mounted partitions look like this:
+	// /dev/sda6 / ext4 rw,relatime,errors=remount-ro,data=ordered 0 0
+	var r io.ReadCloser
+	r, err := os.Open("/proc/mounts")
+	if err != nil {
+		return ""
+	}
+	defer r.Close()
+
+	scanner := bufio.NewScanner(r)
+	for scanner.Scan() {
+		line := scanner.Text()
+		partition, mountpoint := parseMountEntry(line)
+		if partition == fmt.Sprintf("/dev/disk/by-label/%s", label) {
+			return mountpoint
+		}
+	}
+	return ""
+}
+
+func parseMountEntry(line string) (string, string) {
+	// mount entries for mounted partitions look like this:
+	// /dev/sda6 / ext4 rw,relatime,errors=remount-ro,data=ordered 0 0
+	if line[0] != '/' {
+		return "", ""
+	}
+	fields := strings.Fields(line)
+
+	if len(fields) < 4 {
+		return "", ""
+	}
+
+	// We do some special parsing of the mountpoint, which may contain space,
+	// tab and newline characters, encoded into the mount entry line using their
+	// octal-to-string representations. From the GNU mtab man pages:
+	//
+	//   "Therefore these characters are encoded in the files and the getmntent
+	//   function takes care of the decoding while reading the entries back in.
+	//   '\040' is used to encode a space character, '\011' to encode a tab
+	//   character, '\012' to encode a newline character, and '\\' to encode a
+	//   backslash."
+	mp := fields[1]
+	r := strings.NewReplacer(
+		"\\011", "\t", "\\012", "\n", "\\040", " ", "\\\\", "\\",
+	)
+	mp = r.Replace(mp)
+
+	return fields[0], mp
+}
+
 // GetPartitionFS gets the FS of a partition given
 func GetPartitionFS(partition string) (string, error) {
 	// We want to have the device always prefixed with a /dev