From 60f758639f85ec517a2a84177c7c5813b9ce5a3a Mon Sep 17 00:00:00 2001 From: Itxaka Date: Fri, 27 Sep 2024 15:27:06 +0200 Subject: [PATCH 1/4] Fix reset Seems like witht he older ghw, we were not receiving all the proper partitions and mountpoints and such, so on reset we could blindily try to unmount all partitions and call it a day. But now we get info about all of them, so doing so will even unmount the current recovery partition, where the reset source is at, thus failing the reset copy. This patch fixes it by instead of going crazy, making sure we have an order mount/unmount of the proper places at the proper times and we restore everything as is once we are done with it Signed-off-by: Itxaka --- pkg/action/reset.go | 55 ++++++++++++++++++++++++-------------- pkg/config/spec.go | 2 -- pkg/elemental/elemental.go | 4 +-- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/pkg/action/reset.go b/pkg/action/reset.go index b382a5f1..41beb2b9 100644 --- a/pkg/action/reset.go +++ b/pkg/action/reset.go @@ -113,24 +113,35 @@ 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 + /* + 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 +157,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 +165,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/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..9bd8a82f 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, out) _ = e.config.Fs.Remove(img.File) return nil, err } From 79097341ff97186f6fd12d7febbe48aef025396b Mon Sep 17 00:00:00 2001 From: Itxaka Date: Fri, 27 Sep 2024 15:56:41 +0200 Subject: [PATCH 2/4] Cast to string Signed-off-by: Itxaka --- pkg/elemental/elemental.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/elemental/elemental.go b/pkg/elemental/elemental.go index 9bd8a82f..ba76a1bc 100644 --- a/pkg/elemental/elemental.go +++ b/pkg/elemental/elemental.go @@ -352,7 +352,7 @@ func (e *Elemental) DeployImage(img *v1.Image, leaveMounted bool) (info interfac } else if img.Label != "" && img.FS != cnst.SquashFs { 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: %s", img.Label, img.File, out) + 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 } From 1e885cadb0d3886975701117a924455fcf80c981 Mon Sep 17 00:00:00 2001 From: Itxaka Date: Fri, 27 Sep 2024 16:05:01 +0200 Subject: [PATCH 3/4] Fix tests As we were not providing any mountpoints for our fake partitions, reset test failed to work Signed-off-by: Itxaka --- pkg/action/reset_test.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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", }, }, From efa3762acaf1a73d0b2d56696abc0ead4f23e329 Mon Sep 17 00:00:00 2001 From: Itxaka Date: Fri, 27 Sep 2024 16:07:24 +0200 Subject: [PATCH 4/4] TODO Signed-off-by: Itxaka --- pkg/action/reset.go | 3 +++ pkg/utils/common.go | 1 + 2 files changed, 4 insertions(+) diff --git a/pkg/action/reset.go b/pkg/action/reset.go index 41beb2b9..feb515e8 100644 --- a/pkg/action/reset.go +++ b/pkg/action/reset.go @@ -116,6 +116,9 @@ func (r ResetAction) Run() (err error) { // Reformat state partition // 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 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")