boot,overlord: address some review comments

soumyaDghosh · Oct 3, 2022 · 88b796b · 88b796b
1 parent 66a345b
commit 88b796b
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 6 deletions.
diff --git a/boot/makebootable.go b/boot/makebootable.go
@@ -82,7 +82,7 @@ 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 {
-	return configureBootloader(partDir, bootWith, bootMode, opts, bootFlags)
+	return configureBootloader(partDir, opts, bootWith, bootMode, bootFlags)
 }
 
 // makeBootable16 setups the image filesystem for boot with UC16
@@ -162,7 +162,7 @@ func makeBootable16(model *asserts.Model, rootdir string, bootWith *BootableSet)
 	return nil
 }
 
-func configureBootloader(rootdir string, bootWith *BootableSet, bootMode string, opts *bootloader.Options, bootFlags []string) error {
+func configureBootloader(rootdir string, opts *bootloader.Options, bootWith *BootableSet, bootMode string, bootFlags []string) error {
 	blVars := make(map[string]string, 3)
 	if len(bootFlags) != 0 {
 		if err := setImageBootFlags(bootFlags, blVars); err != nil {
@@ -218,7 +218,7 @@ func makeBootable20(rootdir string, bootWith *BootableSet, bootFlags []string) e
 		// setup the recovery bootloader
 		Role: bootloader.RoleRecovery,
 	}
-	if err := configureBootloader(rootdir, bootWith, ModeInstall, opts, bootFlags); err != nil {
+	if err := configureBootloader(rootdir, opts, bootWith, ModeInstall, bootFlags); err != nil {
 		return fmt.Errorf("cannot install bootloader: %v", err)
 	}
 

diff --git a/boot/makebootable_test.go b/boot/makebootable_test.go
@@ -1978,6 +1978,8 @@ version: 5.0
 
 	// ensure that there are no good recovery systems as RecoverySystemDir was empty
 	mockSeedGrubenv := filepath.Join(mockSeedGrubDir, "grubenv")
-	c.Assert(mockSeedGrubenv, testutil.FilePresent)
-	c.Check(mockSeedGrubenv, testutil.FileContains, "snapd_good_recovery_systems=")
+	c.Check(mockSeedGrubenv, testutil.FilePresent)
+	systemGenv := grubenv.NewEnv(mockSeedGrubenv)
+	c.Assert(systemGenv.Load(), IsNil)
+	c.Check(systemGenv.Get("snapd_good_recovery_systems"), Equals, "")
 }
diff --git a/overlord/devicestate/devicestate_install_mode_test.go b/overlord/devicestate/devicestate_install_mode_test.go
@@ -3897,6 +3897,7 @@ func (s *installStepSuite) TestDeviceManagerInstallFinishRunthrough(c *C) {
 	st.Lock()
 
 	c.Check(chg.IsReady(), Equals, true)
+	// TODO: update once the change actually does something
 	c.Check(chg.Err().Error(), Equals, `cannot perform the following tasks:
 - Finish setup of run system for "1234" (cannot load assertions for label "1234": no seed assertions)`)
 }

diff --git a/overlord/devicestate/handlers_install.go b/overlord/devicestate/handlers_install.go
@@ -1297,7 +1297,7 @@ func (m *DeviceManager) doInstallFinish(t *state.Task, _ *tomb.Tomb) error {
 	}
 
 	logger.Debugf("writing content to partitions")
-	timings.Run(perfTimings, "install-finish", "Writing content to partitions", func(tm timings.Measurer) {
+	timings.Run(perfTimings, "install-content", "Writing content to partitions", func(tm timings.Measurer) {
 		st.Unlock()
 		defer st.Lock()
 		_, err = installWriteContent(onVolumes, installObserver, mntPtForType[snap.TypeGadget], mntPtForType[snap.TypeKernel], sys.Model, perfTimings)