From 8e12d682195df8fd33e38ada3bd16abc35c6ad30 Mon Sep 17 00:00:00 2001 From: Samuele Pedroni Date: Tue, 4 Oct 2022 13:33:59 +0200 Subject: [PATCH] many: don't depend on RecoverySystemDir when RecoverySystemLabel is enough --- boot/makebootable.go | 12 +- boot/makebootable_test.go | 112 +++++++++--------- .../devicestate_install_mode_test.go | 6 +- overlord/devicestate/handlers_install.go | 31 ++--- 4 files changed, 79 insertions(+), 82 deletions(-) diff --git a/boot/makebootable.go b/boot/makebootable.go index 791b2ae8bd9..1d48b7d56c1 100644 --- a/boot/makebootable.go +++ b/boot/makebootable.go @@ -82,6 +82,9 @@ func MakeBootableImage(model *asserts.Model, rootdir string, bootWith *BootableS // using information from bootWith and bootFlags. Contrarily to // MakeBootableImage this happens in a live system. func MakeBootablePartition(partDir string, opts *bootloader.Options, bootWith *BootableSet, bootMode string, bootFlags []string) error { + if bootWith.RecoverySystemDir != "" { + return fmt.Errorf("internal error: RecoverySystemDir unexpectedly set for MakeBootablePartition") + } return configureBootloader(partDir, opts, bootWith, bootMode, bootFlags) } @@ -335,6 +338,9 @@ func makeRunnableSystem(model *asserts.Model, bootWith *BootableSet, sealer *Tru if model.Grade() == asserts.ModelGradeUnset { return fmt.Errorf("internal error: cannot make pre-UC20 system runnable") } + if bootWith.RecoverySystemDir != "" { + return fmt.Errorf("internal error: RecoverySystemDir unexpectedly set for MakeRunnableSystem") + } // TODO:UC20: // - figure out what to do for uboot gadgets, currently we require them to @@ -369,11 +375,7 @@ func makeRunnableSystem(model *asserts.Model, bootWith *BootableSet, sealer *Tru currentTrustedBootAssets = sealer.currentTrustedBootAssetsMap() currentTrustedRecoveryBootAssets = sealer.currentTrustedRecoveryBootAssetsMap() } - // filepath.Base("") returns ".", so we need to be a bit careful here - recoverySystemLabel := "" - if bootWith.RecoverySystemDir != "" { - recoverySystemLabel = filepath.Base(bootWith.RecoverySystemDir) - } + recoverySystemLabel := bootWith.RecoverySystemLabel // write modeenv on the ubuntu-data partition modeenv := &Modeenv{ Mode: "run", diff --git a/boot/makebootable_test.go b/boot/makebootable_test.go index e796f2b2d44..a7640f60f13 100644 --- a/boot/makebootable_test.go +++ b/boot/makebootable_test.go @@ -554,15 +554,15 @@ version: 5.0 c.Assert(err, IsNil) bootWith := &boot.BootableSet{ - RecoverySystemDir: "20191216", - BasePath: baseInSeed, - Base: baseInfo, - Gadget: gadgetInfo, - GadgetPath: gadgetInSeed, - KernelPath: kernelInSeed, - Kernel: kernelInfo, - Recovery: false, - UnpackedGadgetDir: unpackedGadgetDir, + RecoverySystemLabel: "20191216", + BasePath: baseInSeed, + Base: baseInfo, + Gadget: gadgetInfo, + GadgetPath: gadgetInSeed, + KernelPath: kernelInSeed, + Kernel: kernelInfo, + Recovery: false, + UnpackedGadgetDir: unpackedGadgetDir, } // set up observer state @@ -924,15 +924,15 @@ version: 5.0 c.Assert(err, IsNil) bootWith := &boot.BootableSet{ - RecoverySystemDir: "20191216", - BasePath: baseInSeed, - Base: baseInfo, - KernelPath: kernelInSeed, - Kernel: kernelInfo, - Gadget: gadgetInfo, - GadgetPath: gadgetInSeed, - Recovery: false, - UnpackedGadgetDir: unpackedGadgetDir, + RecoverySystemLabel: "20191216", + BasePath: baseInSeed, + Base: baseInfo, + KernelPath: kernelInSeed, + Kernel: kernelInfo, + Gadget: gadgetInfo, + GadgetPath: gadgetInSeed, + Recovery: false, + UnpackedGadgetDir: unpackedGadgetDir, } // no grub marker in gadget directory raises an error @@ -1033,15 +1033,15 @@ version: 5.0 c.Assert(err, IsNil) bootWith := &boot.BootableSet{ - RecoverySystemDir: "20191216", - BasePath: baseInSeed, - Base: baseInfo, - KernelPath: kernelInSeed, - Kernel: kernelInfo, - Gadget: gadgetInfo, - GadgetPath: gadgetInSeed, - Recovery: false, - UnpackedGadgetDir: unpackedGadgetDir, + RecoverySystemLabel: "20191216", + BasePath: baseInSeed, + Base: baseInfo, + KernelPath: kernelInSeed, + Kernel: kernelInfo, + Gadget: gadgetInfo, + GadgetPath: gadgetInSeed, + Recovery: false, + UnpackedGadgetDir: unpackedGadgetDir, } // set up observer state @@ -1228,15 +1228,15 @@ version: 5.0 c.Assert(err, IsNil) bootWith := &boot.BootableSet{ - RecoverySystemDir: "20191216", - BasePath: baseInSeed, - Base: baseInfo, - Gadget: gadgetInfo, - GadgetPath: gadgetInSeed, - KernelPath: kernelInSeed, - Kernel: kernelInfo, - Recovery: false, - UnpackedGadgetDir: unpackedGadgetDir, + RecoverySystemLabel: "20191216", + BasePath: baseInSeed, + Base: baseInfo, + Gadget: gadgetInfo, + GadgetPath: gadgetInSeed, + KernelPath: kernelInSeed, + Kernel: kernelInfo, + Recovery: false, + UnpackedGadgetDir: unpackedGadgetDir, } // set up observer state @@ -1469,15 +1469,15 @@ version: 5.0 c.Assert(err, IsNil) bootWith := &boot.BootableSet{ - RecoverySystemDir: "20191216", - BasePath: baseInSeed, - Base: baseInfo, - KernelPath: kernelInSeed, - Kernel: kernelInfo, - Gadget: gadgetInfo, - GadgetPath: gadgetInSeed, - Recovery: false, - UnpackedGadgetDir: unpackedGadgetDir, + RecoverySystemLabel: "20191216", + BasePath: baseInSeed, + Base: baseInfo, + KernelPath: kernelInSeed, + Kernel: kernelInfo, + Gadget: gadgetInfo, + GadgetPath: gadgetInSeed, + Recovery: false, + UnpackedGadgetDir: unpackedGadgetDir, } // set a mock recovery kernel @@ -1671,15 +1671,15 @@ version: 5.0 c.Assert(err, IsNil) bootWith := &boot.BootableSet{ - RecoverySystemDir: "20191216", - BasePath: baseInSeed, - Base: baseInfo, - Gadget: gadgetInfo, - GadgetPath: gadgetInSeed, - KernelPath: kernelInSeed, - Kernel: kernelInfo, - Recovery: false, - UnpackedGadgetDir: unpackedGadgetDir, + RecoverySystemLabel: "20191216", + BasePath: baseInSeed, + Base: baseInfo, + Gadget: gadgetInfo, + GadgetPath: gadgetInSeed, + KernelPath: kernelInSeed, + Kernel: kernelInfo, + Recovery: false, + UnpackedGadgetDir: unpackedGadgetDir, } err = boot.MakeRunnableSystem(model, bootWith, nil) c.Assert(err, IsNil) @@ -1857,7 +1857,6 @@ version: 5.0 KernelPath: kernelInSeed, Gadget: gadgetInfo, GadgetPath: gadgetInSeed, - RecoverySystemDir: "", RecoverySystemLabel: "", UnpackedGadgetDir: unpackedGadgetDir, Recovery: false, @@ -1962,7 +1961,6 @@ version: 5.0 c.Assert(err, IsNil) bootWith := &boot.BootableSet{ - RecoverySystemDir: "", BasePath: baseInSeed, Base: baseInfo, KernelPath: kernelInSeed, @@ -1976,7 +1974,7 @@ version: 5.0 err = boot.MakeRunnableSystem(model, bootWith, nil) c.Assert(err, IsNil) - // ensure that there are no good recovery systems as RecoverySystemDir was empty + // ensure that there are no good recovery systems as RecoverySystemLabel was empty mockSeedGrubenv := filepath.Join(mockSeedGrubDir, "grubenv") c.Check(mockSeedGrubenv, testutil.FilePresent) systemGenv := grubenv.NewEnv(mockSeedGrubenv) diff --git a/overlord/devicestate/devicestate_install_mode_test.go b/overlord/devicestate/devicestate_install_mode_test.go index b482efbc4a3..df8f1d6824b 100644 --- a/overlord/devicestate/devicestate_install_mode_test.go +++ b/overlord/devicestate/devicestate_install_mode_test.go @@ -296,7 +296,8 @@ func (s *deviceMgrInstallModeSuite) doRunChangeTestWithEncryption(c *C, grade st c.Check(model, DeepEquals, mockModel) c.Check(bootWith.KernelPath, Matches, ".*/var/lib/snapd/snaps/pc-kernel_1.snap") c.Check(bootWith.BasePath, Matches, ".*/var/lib/snapd/snaps/core20_2.snap") - c.Check(bootWith.RecoverySystemDir, Matches, "/systems/20191218") + c.Check(bootWith.RecoverySystemLabel, Equals, "20191218") + c.Check(bootWith.RecoverySystemDir, Equals, "") c.Check(bootWith.UnpackedGadgetDir, Equals, filepath.Join(dirs.SnapMountDir, "pc/1")) if tc.encrypt { c.Check(seal, NotNil) @@ -2445,7 +2446,8 @@ func (s *deviceMgrInstallModeSuite) doRunFactoryResetChange(c *C, model *asserts c.Check(makeRunnableModel, DeepEquals, model) c.Check(bootWith.KernelPath, Matches, ".*/var/lib/snapd/snaps/pc-kernel_1.snap") c.Check(bootWith.BasePath, Matches, ".*/var/lib/snapd/snaps/core20_2.snap") - c.Check(bootWith.RecoverySystemDir, Matches, "/systems/20191218") + c.Check(bootWith.RecoverySystemLabel, Equals, "20191218") + c.Check(bootWith.RecoverySystemDir, Equals, "") c.Check(bootWith.UnpackedGadgetDir, Equals, filepath.Join(dirs.SnapMountDir, "pc/1")) if tc.encrypt { c.Check(seal, NotNil) diff --git a/overlord/devicestate/handlers_install.go b/overlord/devicestate/handlers_install.go index 284118d0a76..96f5ac2ea50 100644 --- a/overlord/devicestate/handlers_install.go +++ b/overlord/devicestate/handlers_install.go @@ -1,7 +1,7 @@ // -*- Mode: Go; indent-tabs-mode: t -*- /* - * Copyright (C) 2021 Canonical Ltd + * Copyright (C) 2021-2022 Canonical Ltd * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License version 3 as @@ -379,7 +379,6 @@ func (m *DeviceManager) doSetupRunSystem(t *state.Task, _ *tomb.Tomb) error { if err != nil { return fmt.Errorf("cannot get boot base info: %v", err) } - recoverySystemDir := filepath.Join("/systems", modeEnv.RecoverySystem) bootWith := &boot.BootableSet{ Base: bootBaseInfo, BasePath: bootBaseInfo.MountFile(), @@ -387,8 +386,9 @@ func (m *DeviceManager) doSetupRunSystem(t *state.Task, _ *tomb.Tomb) error { GadgetPath: gadgetInfo.MountFile(), Kernel: kernelInfo, KernelPath: kernelInfo.MountFile(), - RecoverySystemDir: recoverySystemDir, UnpackedGadgetDir: gadgetDir, + + RecoverySystemLabel: modeEnv.RecoverySystem, } timings.Run(perfTimings, "boot-make-runnable", "Make target system runnable", func(timings.Measurer) { err = bootMakeRunnable(deviceCtx.Model(), bootWith, trustedInstallObserver) @@ -989,7 +989,6 @@ func (m *DeviceManager) doFactoryResetRunSystem(t *state.Task, _ *tomb.Tomb) err if err != nil { return fmt.Errorf("cannot get boot base info: %v", err) } - recoverySystemDir := filepath.Join("/systems", modeEnv.RecoverySystem) bootWith := &boot.BootableSet{ Base: bootBaseInfo, BasePath: bootBaseInfo.MountFile(), @@ -997,8 +996,9 @@ func (m *DeviceManager) doFactoryResetRunSystem(t *state.Task, _ *tomb.Tomb) err GadgetPath: gadgetInfo.MountFile(), Kernel: kernelInfo, KernelPath: kernelInfo.MountFile(), - RecoverySystemDir: recoverySystemDir, UnpackedGadgetDir: gadgetDir, + + RecoverySystemLabel: modeEnv.RecoverySystem, } timings.Run(perfTimings, "boot-make-runnable", "Make target system runnable", func(timings.Measurer) { err = bootMakeRunnableAfterDataReset(deviceCtx.Model(), bootWith, trustedInstallObserver) @@ -1314,20 +1314,15 @@ func (m *DeviceManager) doInstallFinish(t *state.Task, _ *tomb.Tomb) error { defer unmount() bootWith := &boot.BootableSet{ - Base: snapInfos[snap.TypeBase], - BasePath: snapSeeds[snap.TypeBase].Path, - Kernel: snapInfos[snap.TypeKernel], - KernelPath: snapSeeds[snap.TypeKernel].Path, - Gadget: snapInfos[snap.TypeGadget], - GadgetPath: snapSeeds[snap.TypeGadget].Path, - - // modeenv:recovery_system will be set based on - // RecoverySystemDir and seeding reads this var - // from modeenv to know what it's seeding from - RecoverySystemDir: systemLabel, - RecoverySystemLabel: systemLabel, - + Base: snapInfos[snap.TypeBase], + BasePath: snapSeeds[snap.TypeBase].Path, + Kernel: snapInfos[snap.TypeKernel], + KernelPath: snapSeeds[snap.TypeKernel].Path, + Gadget: snapInfos[snap.TypeGadget], + GadgetPath: snapSeeds[snap.TypeGadget].Path, UnpackedGadgetDir: mntPtForType[snap.TypeGadget], + + RecoverySystemLabel: systemLabel, } // installs in ESP: grub.cfg, grubenv