Fix reset

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,30 +160,34 @@ 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
  }
  err = e.FormatPartition(oem)
  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))
+
+ // Before reset hook happens once partitions are aready and before deploying the OS image
+ err = r.resetHook(cnst.BeforeResetHook, false)
  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)
+ // 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)

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_STATE",
+ FilesystemLabel: "COS_OEM",
  FS: "ext4",
+ MountPoint: "/oem",
  },
  {
  Name: "device3",
- FilesystemLabel: "COS_PERSISTENT",
+ FilesystemLabel: "COS_RECOVERY",
  FS: "ext4",
+ MountPoint: "/run/initramfs/cos-state",
  },
  {
  Name: "device4",
- FilesystemLabel: "COS_OEM",
+ FilesystemLabel: "COS_STATE",
  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")