diff --git a/boot/makebootable.go b/boot/makebootable.go index fd832d737ca..52483ebceec 100644 --- a/boot/makebootable.go +++ b/boot/makebootable.go @@ -356,13 +356,9 @@ func makeBootable20RunMode(model *asserts.Model, rootdir string, bootWith *Boota // installing boot config must be performed after the boot // partition has been populated with gadget data - ok, err := bl.InstallBootConfig(bootWith.UnpackedGadgetDir, opts) - if err != nil { + if err := bl.InstallBootConfig(bootWith.UnpackedGadgetDir, opts); err != nil { return fmt.Errorf("cannot install managed bootloader assets: %v", err) } - if !ok { - return fmt.Errorf("cannot install boot config with a mismatched gadget") - } } if sealer != nil { diff --git a/boot/makebootable_test.go b/boot/makebootable_test.go index 0bd696567b9..dce17d779cf 100644 --- a/boot/makebootable_test.go +++ b/boot/makebootable_test.go @@ -33,6 +33,7 @@ import ( "github.com/snapcore/snapd/bootloader" "github.com/snapcore/snapd/bootloader/assets" "github.com/snapcore/snapd/bootloader/bootloadertest" + "github.com/snapcore/snapd/bootloader/grubenv" "github.com/snapcore/snapd/bootloader/ubootenv" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/gadget" @@ -79,6 +80,7 @@ func makeSnapWithFiles(c *C, name, yaml string, revno snap.Revision, files [][]s } func (s *makeBootableSuite) TestMakeBootable(c *C) { + bootloader.Force(nil) model := boottest.MakeMockModel() grubCfg := []byte("#grub cfg") @@ -117,14 +119,11 @@ version: 4.0 c.Assert(err, IsNil) // check the bootloader config - m, err := s.bootloader.GetBootVars("snap_kernel", "snap_core", "snap_menuentry") - c.Assert(err, IsNil) - c.Check(m["snap_kernel"], Equals, "pc-kernel_5.snap") - c.Check(m["snap_core"], Equals, "core18_3.snap") - c.Check(m["snap_menuentry"], Equals, "My Model") - - // kernel was extracted as needed - c.Check(s.bootloader.ExtractKernelAssetsCalls, DeepEquals, []snap.PlaceInfo{kernelInfo}) + seedGenv := grubenv.NewEnv(filepath.Join(s.rootdir, "boot/grub/grubenv")) + c.Assert(seedGenv.Load(), IsNil) + c.Check(seedGenv.Get("snap_kernel"), Equals, "pc-kernel_5.snap") + c.Check(seedGenv.Get("snap_core"), Equals, "core18_3.snap") + c.Check(seedGenv.Get("snap_menuentry"), Equals, "My Model") // check symlinks from snap blob dir kernelBlob := filepath.Join(dirs.SnapBlobDirUnder(s.rootdir), kernelInfo.Filename()) @@ -173,6 +172,7 @@ func (s *makeBootable20UbootSuite) SetUpTest(c *C) { } func (s *makeBootable20Suite) TestMakeBootable20(c *C) { + bootloader.Force(nil) model := boottest.MakeMockUC20Model() unpackedGadgetDir := c.MkDir() @@ -228,7 +228,8 @@ version: 5.0 // ensure only a single file got copied (the grub.cfg) files, err := filepath.Glob(filepath.Join(s.rootdir, "EFI/ubuntu/*")) c.Assert(err, IsNil) - c.Check(files, HasLen, 1) + // grub.cfg and grubenv + c.Check(files, HasLen, 2) // check that the recovery bootloader configuration was installed with // the correct content c.Check(filepath.Join(s.rootdir, "EFI/ubuntu/grub.cfg"), testutil.FileEquals, grubRecoveryCfgAsset) @@ -237,13 +238,13 @@ version: 5.0 c.Check(filepath.Join(s.rootdir, "boot"), testutil.FileAbsent) // ensure the correct recovery system configuration was set - c.Check(s.bootloader.RecoverySystemDir, Equals, recoverySystemDir) - c.Check(s.bootloader.RecoverySystemBootVars, DeepEquals, map[string]string{ - "snapd_recovery_kernel": "/snaps/pc-kernel_5.snap", - }) - c.Check(s.bootloader.BootVars, DeepEquals, map[string]string{ - "snapd_recovery_system": label, - }) + seedGenv := grubenv.NewEnv(filepath.Join(s.rootdir, "EFI/ubuntu/grubenv")) + c.Assert(seedGenv.Load(), IsNil) + c.Check(seedGenv.Get("snapd_recovery_system"), Equals, label) + + systemGenv := grubenv.NewEnv(filepath.Join(s.rootdir, recoverySystemDir, "grubenv")) + c.Assert(systemGenv.Load(), IsNil) + c.Check(systemGenv.Get("snapd_recovery_kernel"), Equals, "/snaps/pc-kernel_5.snap") } func (s *makeBootable20Suite) TestMakeBootable20UnsetRecoverySystemLabelError(c *C) { @@ -802,6 +803,7 @@ version: 5.0 } func (s *makeBootable20UbootSuite) TestUbootMakeBootable20TraditionalUbootenvFails(c *C) { + bootloader.Force(nil) model := boottest.MakeMockUC20Model() unpackedGadgetDir := c.MkDir() @@ -849,7 +851,7 @@ version: 5.0 // TODO:UC20: enable this use case err = boot.MakeBootable(model, s.rootdir, bootWith, nil) - c.Assert(err, ErrorMatches, fmt.Sprintf("cannot find boot config in %q", unpackedGadgetDir)) + c.Assert(err, ErrorMatches, "non-empty uboot.env not supported on UC20 yet") } func (s *makeBootable20UbootSuite) TestUbootMakeBootable20BootScr(c *C) { diff --git a/bootloader/androidboot.go b/bootloader/androidboot.go index 782e5e8b6b8..48114ef6368 100644 --- a/bootloader/androidboot.go +++ b/bootloader/androidboot.go @@ -52,7 +52,7 @@ func (a *androidboot) dir() string { return filepath.Join(a.rootdir, "/boot/androidboot") } -func (a *androidboot) InstallBootConfig(gadgetDir string, opts *Options) (bool, error) { +func (a *androidboot) InstallBootConfig(gadgetDir string, opts *Options) error { gadgetFile := filepath.Join(gadgetDir, a.Name()+".conf") systemFile := a.ConfigFile() return genericInstallBootConfig(gadgetFile, systemFile) diff --git a/bootloader/bootloader.go b/bootloader/bootloader.go index 92aefe47984..bd40c4bb479 100644 --- a/bootloader/bootloader.go +++ b/bootloader/bootloader.go @@ -96,9 +96,8 @@ type Bootloader interface { ConfigFile() string // InstallBootConfig will try to install the boot config in the - // given gadgetDir to rootdir. If no boot config for this bootloader - // is found ok is false. - InstallBootConfig(gadgetDir string, opts *Options) (ok bool, err error) + // given gadgetDir to rootdir. + InstallBootConfig(gadgetDir string, opts *Options) error // ExtractKernelAssets extracts kernel assets from the given kernel snap. ExtractKernelAssets(s snap.PlaceInfo, snapf snap.Container) error @@ -198,25 +197,22 @@ type TrustedAssetsBootloader interface { BootChain(runBl Bootloader, kernelPath string) ([]BootFile, error) } -func genericInstallBootConfig(gadgetFile, systemFile string) (bool, error) { - if !osutil.FileExists(gadgetFile) { - return false, nil - } +func genericInstallBootConfig(gadgetFile, systemFile string) error { if err := os.MkdirAll(filepath.Dir(systemFile), 0755); err != nil { - return true, err + return err } - return true, osutil.CopyFile(gadgetFile, systemFile, osutil.CopyFlagOverwrite) + return osutil.CopyFile(gadgetFile, systemFile, osutil.CopyFlagOverwrite) } -func genericSetBootConfigFromAsset(systemFile, assetName string) (bool, error) { +func genericSetBootConfigFromAsset(systemFile, assetName string) error { bootConfig := assets.Internal(assetName) if bootConfig == nil { - return true, fmt.Errorf("internal error: no boot asset for %q", assetName) + return fmt.Errorf("internal error: no boot asset for %q", assetName) } if err := os.MkdirAll(filepath.Dir(systemFile), 0755); err != nil { - return true, err + return err } - return true, osutil.AtomicWriteFile(systemFile, bootConfig, 0644, 0) + return osutil.AtomicWriteFile(systemFile, bootConfig, 0644, 0) } func genericUpdateBootConfigFromAssets(systemFile string, assetName string) error { @@ -249,16 +245,11 @@ func InstallBootConfig(gadgetDir, rootDir string, opts *Options) error { if err := opts.validate(); err != nil { return err } - // TODO:UC20 use ForGadget() to obtain the right bootloader - for _, bl := range []installableBootloader{&grub{}, &uboot{}, &androidboot{}, &lk{}} { - bl.setRootDir(rootDir) - ok, err := bl.InstallBootConfig(gadgetDir, opts) - if ok { - return err - } + bl, err := ForGadget(gadgetDir, rootDir, opts) + if err != nil { + return fmt.Errorf("cannot find boot config in %q", gadgetDir) } - - return fmt.Errorf("cannot find boot config in %q", gadgetDir) + return bl.InstallBootConfig(gadgetDir, opts) } type bootloaderNewFunc func(rootdir string, opts *Options) Bootloader @@ -368,8 +359,9 @@ func ForGadget(gadgetDir, rootDir string, opts *Options) (Bootloader, error) { } for _, blNew := range bootloaders { bl := blNew(rootDir, opts) + markerConf := filepath.Join(gadgetDir, bl.Name()+".conf") // do we have a marker file? - if osutil.FileExists(filepath.Join(gadgetDir, bl.Name()+".conf")) { + if osutil.FileExists(markerConf) { return bl, nil } } diff --git a/bootloader/bootloadertest/bootloadertest.go b/bootloader/bootloadertest/bootloadertest.go index 5b4a0792f1f..27ff77cb03c 100644 --- a/bootloader/bootloadertest/bootloadertest.go +++ b/bootloader/bootloadertest/bootloadertest.go @@ -43,7 +43,6 @@ type MockBootloader struct { RemoveKernelAssetsCalls []snap.PlaceInfo InstallBootConfigCalled []string - InstallBootConfigResult bool InstallBootConfigErr error enabledKernel snap.PlaceInfo @@ -138,9 +137,9 @@ func (b *MockBootloader) SetEnabledTryKernel(s snap.PlaceInfo) (restore func()) // InstallBootConfig installs the boot config in the gadget directory to the // mock bootloader's root directory. -func (b *MockBootloader) InstallBootConfig(gadgetDir string, opts *bootloader.Options) (bool, error) { +func (b *MockBootloader) InstallBootConfig(gadgetDir string, opts *bootloader.Options) error { b.InstallBootConfigCalled = append(b.InstallBootConfigCalled, gadgetDir) - return b.InstallBootConfigResult, b.InstallBootConfigErr + return b.InstallBootConfigErr } // MockRecoveryAwareBootloader mocks a bootloader implementing the diff --git a/bootloader/grub.go b/bootloader/grub.go index 28c2fa4f30a..72366d1e268 100644 --- a/bootloader/grub.go +++ b/bootloader/grub.go @@ -87,29 +87,19 @@ func (g *grub) dir() string { return filepath.Join(g.rootdir, g.basedir) } -func (g *grub) installManagedRecoveryBootConfig(gadgetDir string) (bool, error) { - gadgetGrubCfg := filepath.Join(gadgetDir, g.Name()+".conf") - if !osutil.FileExists(gadgetGrubCfg) { - // gadget does not use grub bootloader - return false, nil - } +func (g *grub) installManagedRecoveryBootConfig(gadgetDir string) error { assetName := g.Name() + "-recovery.cfg" systemFile := filepath.Join(g.rootdir, "/EFI/ubuntu/grub.cfg") return genericSetBootConfigFromAsset(systemFile, assetName) } -func (g *grub) installManagedBootConfig(gadgetDir string) (bool, error) { - gadgetGrubCfg := filepath.Join(gadgetDir, g.Name()+".conf") - if !osutil.FileExists(gadgetGrubCfg) { - // gadget does not use grub bootloader - return false, nil - } +func (g *grub) installManagedBootConfig(gadgetDir string) error { assetName := g.Name() + ".cfg" systemFile := filepath.Join(g.rootdir, "/EFI/ubuntu/grub.cfg") return genericSetBootConfigFromAsset(systemFile, assetName) } -func (g *grub) InstallBootConfig(gadgetDir string, opts *Options) (bool, error) { +func (g *grub) InstallBootConfig(gadgetDir string, opts *Options) error { if opts != nil && opts.Role == RoleRecovery { // install managed config for the recovery partition return g.installManagedRecoveryBootConfig(gadgetDir) diff --git a/bootloader/lk.go b/bootloader/lk.go index 2405b30091c..3b95531298f 100644 --- a/bootloader/lk.go +++ b/bootloader/lk.go @@ -72,7 +72,7 @@ func (l *lk) dir() string { return filepath.Join(l.rootdir, "/boot/lk/") } -func (l *lk) InstallBootConfig(gadgetDir string, opts *Options) (bool, error) { +func (l *lk) InstallBootConfig(gadgetDir string, opts *Options) error { gadgetFile := filepath.Join(gadgetDir, l.Name()+".conf") systemFile := l.ConfigFile() return genericInstallBootConfig(gadgetFile, systemFile) diff --git a/bootloader/uboot.go b/bootloader/uboot.go index 8401fd7feab..419321eecb2 100644 --- a/bootloader/uboot.go +++ b/bootloader/uboot.go @@ -84,7 +84,7 @@ func (u *uboot) dir() string { return filepath.Join(u.rootdir, u.basedir) } -func (u *uboot) InstallBootConfig(gadgetDir string, blOpts *Options) (bool, error) { +func (u *uboot) InstallBootConfig(gadgetDir string, blOpts *Options) error { gadgetFile := filepath.Join(gadgetDir, u.Name()+".conf") // if the gadget file is empty, then we don't install anything // this is because there are some gadgets, namely the 20 pi gadget right @@ -95,7 +95,7 @@ func (u *uboot) InstallBootConfig(gadgetDir string, blOpts *Options) (bool, erro // actual format? st, err := os.Stat(gadgetFile) if err != nil { - return false, err + return err } if st.Size() == 0 { // we have an empty uboot.conf, and hence a uboot bootloader in the @@ -105,20 +105,20 @@ func (u *uboot) InstallBootConfig(gadgetDir string, blOpts *Options) (bool, erro err := os.MkdirAll(filepath.Dir(u.envFile()), 0755) if err != nil { - return false, err + return err } // TODO:UC20: what's a reasonable size for this file? env, err := ubootenv.Create(u.envFile(), 4096) if err != nil { - return false, err + return err } if err := env.Save(); err != nil { - return false, nil + return nil } - return true, nil + return nil } // InstallBootConfig gets called on a uboot that does not come from newUboot @@ -128,7 +128,7 @@ func (u *uboot) InstallBootConfig(gadgetDir string, blOpts *Options) (bool, erro if blOpts != nil && blOpts.Role == RoleRecovery { // not supported yet, this is traditional uboot.env from gadget // TODO:UC20: support this use-case - return false, fmt.Errorf("non-empty uboot.env not supported on UC20 yet") + return fmt.Errorf("non-empty uboot.env not supported on UC20 yet") } systemFile := u.ConfigFile() diff --git a/image/image_test.go b/image/image_test.go index cca4d3bbbca..02a449d1df9 100644 --- a/image/image_test.go +++ b/image/image_test.go @@ -38,6 +38,7 @@ import ( "github.com/snapcore/snapd/bootloader" "github.com/snapcore/snapd/bootloader/assets" "github.com/snapcore/snapd/bootloader/bootloadertest" + "github.com/snapcore/snapd/bootloader/grubenv" "github.com/snapcore/snapd/bootloader/ubootenv" "github.com/snapcore/snapd/image" "github.com/snapcore/snapd/osutil" @@ -2568,10 +2569,8 @@ func (s *imageSuite) makeUC20Model(extraHeaders map[string]interface{}) *asserts return s.Brands.Model("my-brand", "my-model", headers) } -func (s *imageSuite) TestSetupSeedCore20(c *C) { - bl := bootloadertest.Mock("grub", c.MkDir()).RecoveryAware() - bootloader.Force(bl) - +func (s *imageSuite) TestSetupSeedCore20Grub(c *C) { + bootloader.Force(nil) restore := image.MockTrusted(s.StoreSigning.Trusted) defer restore() @@ -2642,28 +2641,33 @@ func (s *imageSuite) TestSetupSeedCore20(c *C) { // check boot config grubCfg := filepath.Join(prepareDir, "system-seed", "EFI/ubuntu/grub.cfg") + seedGrubenv := filepath.Join(prepareDir, "system-seed", "EFI/ubuntu/grubenv") grubRecoveryCfgAsset := assets.Internal("grub-recovery.cfg") c.Assert(grubRecoveryCfgAsset, NotNil) c.Check(grubCfg, testutil.FileEquals, string(grubRecoveryCfgAsset)) - // make sure that grub.cfg is the only file present inside the directory + // make sure that grub.cfg and grubenv are the only files present inside + // the directory gl, err := filepath.Glob(filepath.Join(prepareDir, "system-seed/EFI/ubuntu/*")) c.Assert(err, IsNil) - c.Check(gl, DeepEquals, []string{grubCfg}) - - c.Check(s.stderr.String(), Equals, "") + c.Check(gl, DeepEquals, []string{ + grubCfg, + seedGrubenv, + }) // check recovery system specific config systems, err := filepath.Glob(filepath.Join(seeddir, "systems", "*")) c.Assert(err, IsNil) c.Assert(systems, HasLen, 1) - c.Check(bl.RecoverySystemDir, Equals, fmt.Sprintf("/systems/%s", filepath.Base(systems[0]))) - c.Check(bl.RecoverySystemBootVars, DeepEquals, map[string]string{ - "snapd_recovery_kernel": "/snaps/pc-kernel_1.snap", - }) - c.Check(bl.BootVars, DeepEquals, map[string]string{ - "snapd_recovery_system": filepath.Base(systems[0]), - }) + seedGenv := grubenv.NewEnv(seedGrubenv) + c.Assert(seedGenv.Load(), IsNil) + c.Check(seedGenv.Get("snapd_recovery_system"), Equals, filepath.Base(systems[0])) + + c.Check(s.stderr.String(), Equals, "") + + systemGenv := grubenv.NewEnv(filepath.Join(systems[0], "grubenv")) + c.Assert(systemGenv.Load(), IsNil) + c.Check(systemGenv.Get("snapd_recovery_kernel"), Equals, "/snaps/pc-kernel_1.snap") // check the downloads c.Check(s.storeActions, HasLen, 5)