-
Notifications
You must be signed in to change notification settings - Fork 574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bootloader: use ForGadget when installing boot config #9643
Changes from 3 commits
00b0744
50db1a0
697ee65
628149e
a13a6f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,7 +98,7 @@ type Bootloader interface { | |
// 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) | ||
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 +198,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 +246,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) | ||
Comment on lines
+248
to
+250
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah nice, this does look much better, thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, this is nice! |
||
} | ||
|
||
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 +360,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 | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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,32 @@ 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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
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) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.