diff --git a/pkg/action/reset.go b/pkg/action/reset.go index b382a5f1..feb515e8 100644 --- a/pkg/action/reset.go +++ b/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,22 +168,26 @@ 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)) + + // 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) diff --git a/pkg/action/reset_test.go b/pkg/action/reset_test.go index fc1531c3..8473cbcb 100644 --- a/pkg/action/reset_test.go +++ b/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", }, }, diff --git a/pkg/config/spec.go b/pkg/config/spec.go index a018b65c..7e023ed7 100644 --- a/pkg/config/spec.go +++ b/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) diff --git a/pkg/elemental/elemental.go b/pkg/elemental/elemental.go index c3edeb52..ba76a1bc 100644 --- a/pkg/elemental/elemental.go +++ b/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 } diff --git a/pkg/utils/common.go b/pkg/utils/common.go index abc11b51..e7a26d94 100644 --- a/pkg/utils/common.go +++ b/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")