Fix reset (#565)

pkg/action/reset.go

@@ -113,24 +113,38 @@ func (r ResetAction) Run() (err error) {
 	cleanup := utils.NewCleanStack()
 	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
-	}
-
 	// Reformat state partition
-	err = e.FormatPartition(r.spec.Partitions.State)
-	if err != nil {
-		return err
-	}
+	// We should expose this under a flag, to reformat state before starting
+	// In case state fs is broken somehow
+	// We used to format it blindly but if something happens between this format and the new image copy down below
+	// You end up with a broken system with no active or passive and reset may not even work at that point
+	// TODO: Either create a flag to do this or ignore it and never format the partition
+	/*
+		if r.spec.FormatState {
+			state := r.spec.Partitions.State
+			if state != nil {
+				err = e.UnmountPartition(state)
+				if err != nil {
+					return err
+				}
+				err = e.FormatPartition(state)
+				if err != nil {
+					return err
+				}
+				// Mount it back
+				err = e.MountPartition(state)
+				if err != nil {
+					return err
+				}
+			}
+		}
+	*/
 
 	// Reformat persistent partition
 	if r.spec.FormatPersistent {
 		persistent := r.spec.Partitions.Persistent
 		if persistent != nil {
-			err = e.UnmountPartitions(r.spec.Partitions.PartitionsByMountPoint(true, persistent))
+			err = e.UnmountPartition(persistent)
 			if err != nil {
 				return err
 			}
@@ -146,7 +160,7 @@ func (r ResetAction) Run() (err error) {
 		oem := r.spec.Partitions.OEM
 		if oem != nil {
 			// Try to umount
-			err = e.UnmountPartitions(r.spec.Partitions.PartitionsByMountPoint(true, oem))
+			err = e.UnmountPartition(oem)
 			if err != nil {
 				return err
 			}
@@ -154,16 +168,13 @@ func (r ResetAction) Run() (err error) {
 			if err != nil {
 				return err
 			}
+			// Mount it back, as oem is mounted during recovery, keep everything as is
+			err = e.MountPartition(oem)
+			if err != nil {
+				return err
+			}
 		}
 	}
-	// Mount configured partitions
-	err = e.MountPartitions(r.spec.Partitions.PartitionsByMountPoint(false, r.spec.Partitions.Recovery))
-	if err != nil {
-		return err
-	}
-	cleanup.Push(func() error {
-		return e.UnmountPartitions(r.spec.Partitions.PartitionsByMountPoint(true, r.spec.Partitions.Recovery))
-	})
 
 	// Before reset hook happens once partitions are aready and before deploying the OS image
 	err = r.resetHook(cnst.BeforeResetHook, false)
@@ -171,6 +182,13 @@ func (r ResetAction) Run() (err error) {
 		return err
 	}
 
+	// Mount COS_STATE so we can write the new images
+	err = e.MountPartition(r.spec.Partitions.State)
+	if err != nil {
+		return err
+	}
+	cleanup.Push(func() error { return e.UnmountPartition(r.spec.Partitions.State) })
+
 	// Deploy active image
 	meta, err := e.DeployImage(&r.spec.Active, true)
 	if err != nil {

pkg/action/reset_test.go

@@ -89,7 +89,6 @@ var _ = Describe("Reset action tests", func() {
 		var cmdFail, bootedFrom string
 		var err error
 		BeforeEach(func() {
-
 			Expect(err).ShouldNot(HaveOccurred())
 			cmdFail = ""
 			recoveryImg := filepath.Join(constants.RunningStateDir, "cOS", constants.RecoveryImgFile)
@@ -108,22 +107,24 @@ var _ = Describe("Reset action tests", func() {
 					},
 					{
 						Name:            "device2",
+						FilesystemLabel: "COS_OEM",
+						FS:              "ext4",
+						MountPoint:      "/oem",
+					},
+					{
+						Name:            "device3",
+						FilesystemLabel: "COS_RECOVERY",
+						FS:              "ext4",
+						MountPoint:      "/run/initramfs/cos-state",
+					},
+					{
+						Name:            "device4",
 						FilesystemLabel: "COS_STATE",
 						FS:              "ext4",
 					},
-					{
-						Name:            "device3",
-						FilesystemLabel: "COS_PERSISTENT",
-						FS:              "ext4",
-					},
-					{
-						Name:            "device4",
-						FilesystemLabel: "COS_OEM",
-						FS:              "ext4",
-					},
 					{
 						Name:            "device5",
-						FilesystemLabel: "COS_RECOVERY",
+						FilesystemLabel: "COS_PERSISTENT",
 						FS:              "ext4",
 					},
 				},

pkg/config/spec.go

@@ -447,7 +447,6 @@ func NewResetSpec(cfg *Config) (*v1.ResetSpec, error) {
 		return nil, fmt.Errorf("could not read host partitions")
 	}
 	ep := v1.NewElementalPartitionsFromList(parts)
-
 	if efiExists {
 		if ep.EFI == nil {
 			return nil, fmt.Errorf("EFI partition not found")
@@ -480,7 +479,6 @@ func NewResetSpec(cfg *Config) (*v1.ResetSpec, error) {
 	target := ep.State.Disk
 
 	// 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)

pkg/elemental/elemental.go

@@ -350,9 +350,9 @@ func (e *Elemental) DeployImage(img *v1.Image, leaveMounted bool) (info interfac
 			}
 		}
 	} else if img.Label != "" && img.FS != cnst.SquashFs {
-		_, err = e.config.Runner.Run("tune2fs", "-L", img.Label, img.File)
+		out, err := e.config.Runner.Run("tune2fs", "-L", img.Label, img.File)
 		if err != nil {
-			e.config.Logger.Errorf("Failed to apply label %s to %s", img.Label, img.File)
+			e.config.Logger.Errorf("Failed to apply label %s to %s: %s", img.Label, img.File, string(out))
 			_ = e.config.Fs.Remove(img.File)
 			return nil, err
 		}

pkg/utils/common.go

@@ -83,6 +83,7 @@ func CopyFile(fs v1.FS, source string, target string) (err error) {
 // Source files are concatenated into target file in the given order.
 // If target is a directory source is copied into that directory using
 // 1st source name file.
+// TODO: Log errors, return errors, whatever but dont ignore them
 func ConcatFiles(fs v1.FS, sources []string, target string) (err error) {
 	if len(sources) == 0 {
 		return fmt.Errorf("Empty sources list")