From afde847bc59ff121c83c53b4e188070ede24c449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alfonso=20S=C3=A1nchez-Beato?= Date: Thu, 17 Oct 2024 17:19:55 -0400 Subject: [PATCH] o/snapstate: move the reboot part of Backend.LinkSnap to a new method Backend.LinkSnap prepares the system for a reboot, move that bit to a different backend method so we can use it from tasks different from link-snap, as we now plan to do reboots later that said task in some cases. --- overlord/snapstate/backend.go | 3 +- overlord/snapstate/backend/link.go | 48 +++++++------ overlord/snapstate/backend/link_test.go | 72 +++++++++++++------- overlord/snapstate/backend_test.go | 29 +++++--- overlord/snapstate/handlers.go | 33 ++++++--- overlord/snapstate/handlers_link_test.go | 21 ++++++ overlord/snapstate/snapstate_install_test.go | 53 ++++++++++++-- overlord/snapstate/snapstate_remove_test.go | 3 + overlord/snapstate/snapstate_test.go | 35 +++++++++- overlord/snapstate/snapstate_update_test.go | 44 ++++++++++++ 10 files changed, 270 insertions(+), 71 deletions(-) diff --git a/overlord/snapstate/backend.go b/overlord/snapstate/backend.go index 5dc7dd5d647..33ccae9a872 100644 --- a/overlord/snapstate/backend.go +++ b/overlord/snapstate/backend.go @@ -79,11 +79,12 @@ type managerBackend interface { SetupKernelModulesComponents(currentComps, finalComps []*snap.ComponentSideInfo, ksnapName string, ksnapRev snap.Revision, meter progress.Meter) (err error) CopySnapData(newSnap, oldSnap *snap.Info, opts *dirs.SnapDirOptions, meter progress.Meter) error SetupSnapSaveData(info *snap.Info, dev snap.Device, meter progress.Meter) error - LinkSnap(info *snap.Info, dev snap.Device, linkCtx backend.LinkContext, tm timings.Measurer) (rebootInfo boot.RebootInfo, err error) + LinkSnap(info *snap.Info, dev snap.Device, linkCtx backend.LinkContext, tm timings.Measurer) error LinkComponent(cpi snap.ContainerPlaceInfo, snapRev snap.Revision) error StartServices(svcs []*snap.AppInfo, disabledSvcs *wrappers.DisabledServices, meter progress.Meter, tm timings.Measurer) error StopServices(svcs []*snap.AppInfo, reason snap.ServiceStopReason, meter progress.Meter, tm timings.Measurer) error QueryDisabledServices(info *snap.Info, pb progress.Meter) (*wrappers.DisabledServices, error) + MaybePrepareReboot(info *snap.Info, dev snap.Device, isUndo bool) (boot.RebootInfo, error) // the undoers for install UndoSetupSnap(s snap.PlaceInfo, typ snap.Type, installRecord *backend.InstallRecord, dev snap.Device, meter progress.Meter) error diff --git a/overlord/snapstate/backend/link.go b/overlord/snapstate/backend/link.go index 443f62b40ed..f4043625d1a 100644 --- a/overlord/snapstate/backend/link.go +++ b/overlord/snapstate/backend/link.go @@ -48,10 +48,6 @@ type LinkContext struct { // installed FirstInstall bool - // IsUndo is set when we are installing the previous snap while - // performing a revert of the latest one that was installed - IsUndo bool - // ServiceOptions is used to configure services. ServiceOptions *wrappers.SnapServiceOptions @@ -153,10 +149,28 @@ func updateCurrentSymlinks(info *snap.Info) (revert func(), e error) { return revertFunc, nil } +// MaybePrepareReboot configures the system for a reboot if necesssary because +// of a snap refresh. isUndo must be set when we are installing the previous +// snap while performing a revert of the latest one that was installed +func (b Backend) MaybePrepareReboot(info *snap.Info, dev snap.Device, isUndo bool) (boot.RebootInfo, error) { + if b.preseed { + return boot.RebootInfo{}, nil + } + + bootCtx := boot.NextBootContext{BootWithoutTry: isUndo} + rebootInfo, err := boot.Participant( + info, info.Type(), dev).SetNextBoot(bootCtx) + if err != nil { + return boot.RebootInfo{}, err + } + + return rebootInfo, nil +} + // LinkSnap makes the snap available by generating wrappers and setting the current symlinks. -func (b Backend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx LinkContext, tm timings.Measurer) (rebootRequired boot.RebootInfo, e error) { +func (b Backend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx LinkContext, tm timings.Measurer) (e error) { if info.Revision.Unset() { - return boot.RebootInfo{}, fmt.Errorf("cannot link snap %q with unset revision", info.InstanceName()) + return fmt.Errorf("cannot link snap %q with unset revision", info.InstanceName()) } osutil.MaybeInjectFault("link-snap") @@ -167,7 +181,7 @@ func (b Backend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx LinkContext, restart, err = b.generateWrappers(info, linkCtx) }) if err != nil { - return boot.RebootInfo{}, err + return err } defer func() { if e == nil { @@ -178,21 +192,11 @@ func (b Backend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx LinkContext, }) }() - var rebootInfo boot.RebootInfo - if !b.preseed { - bootCtx := boot.NextBootContext{BootWithoutTry: linkCtx.IsUndo} - rebootInfo, err = boot.Participant( - info, info.Type(), dev).SetNextBoot(bootCtx) - if err != nil { - return boot.RebootInfo{}, err - } - } - // only after link snap it will be possible to execute snap // applications, so ensure that the shared snap directory exists for // parallel installed snaps if err := createSharedSnapDirForParallelInstance(info); err != nil { - return boot.RebootInfo{}, err + return err } cleanupSharedParallelInstanceDir := func() { if !linkCtx.HasOtherInstances { @@ -203,7 +207,7 @@ func (b Backend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx LinkContext, revertSymlinks, err := updateCurrentSymlinks(info) if err != nil { cleanupSharedParallelInstanceDir() - return boot.RebootInfo{}, err + return err } // if anything below here could return error, you need to // somehow clean up whatever updateCurrentSymlinks did @@ -214,17 +218,17 @@ func (b Backend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx LinkContext, revertSymlinks() cleanupSharedParallelInstanceDir() - return boot.RebootInfo{}, err + return err } } // Stop inhibiting application startup by removing the inhibitor file. if err := runinhibit.Unlock(info.InstanceName()); err != nil { - return boot.RebootInfo{}, err + return err } - return rebootInfo, nil + return nil } func (b Backend) LinkComponent(cpi snap.ContainerPlaceInfo, snapRev snap.Revision) error { diff --git a/overlord/snapstate/backend/link_test.go b/overlord/snapstate/backend/link_test.go index bb705e64397..c1d8221f0e1 100644 --- a/overlord/snapstate/backend/link_test.go +++ b/overlord/snapstate/backend/link_test.go @@ -110,7 +110,7 @@ apps: ` info := snaptest.MockSnap(c, yaml, &snap.SideInfo{Revision: snap.R(11)}) - _, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, IsNil) l, err := filepath.Glob(filepath.Join(dirs.SnapBinariesDir, "*")) @@ -177,7 +177,7 @@ Exec=foo c.Assert(os.WriteFile(filepath.Join(iconsDir, "snap.hello.png"), []byte(""), 0644), IsNil) c.Assert(os.WriteFile(filepath.Join(iconsDir, "snap.hello.svg"), []byte(""), 0644), IsNil) - _, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, IsNil) l, err := filepath.Glob(filepath.Join(dirs.SnapBinariesDir, "*")) @@ -232,7 +232,7 @@ Exec=foo c.Assert(os.WriteFile(filepath.Join(iconsDir, "snap.hello.png"), []byte(""), 0644), IsNil) c.Assert(os.WriteFile(filepath.Join(iconsDir, "snap.hello.svg"), []byte(""), 0644), IsNil) - _, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, IsNil) l, err := filepath.Glob(filepath.Join(dirs.SnapBinariesDir, "*")) @@ -269,9 +269,12 @@ version: 1.0 ` info := snaptest.MockSnap(c, yaml, &snap.SideInfo{Revision: snap.R(11)}) - reboot, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, IsNil) + isUndo := false + reboot, err := s.be.MaybePrepareReboot(info, mockDev, isUndo) + c.Assert(err, IsNil) c.Check(reboot, Equals, boot.RebootInfo{RebootRequired: false}) mountDir := info.MountDir() @@ -301,11 +304,14 @@ version: 1.0 ` info := snaptest.MockSnapInstance(c, "hello_foo", yaml, &snap.SideInfo{Revision: snap.R(11)}) - reboot, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{ + err := s.be.LinkSnap(info, mockDev, backend.LinkContext{ HasOtherInstances: false, }, s.perfTimings) c.Assert(err, IsNil) + isUndo := false + reboot, err := s.be.MaybePrepareReboot(info, mockDev, isUndo) + c.Assert(err, IsNil) c.Check(reboot, Equals, boot.RebootInfo{RebootRequired: false}) mountDir := info.MountDir() @@ -360,7 +366,11 @@ type: base ` info := snaptest.MockSnap(c, yaml, &snap.SideInfo{Revision: snap.R(11)}) - reboot, err := s.be.LinkSnap(info, coreDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(info, coreDev, backend.LinkContext{}, s.perfTimings) + c.Assert(err, IsNil) + + isUndo := false + reboot, err := s.be.MaybePrepareReboot(info, coreDev, isUndo) c.Assert(err, IsNil) c.Check(reboot, Equals, boot.RebootInfo{RebootRequired: true}) } @@ -382,7 +392,11 @@ type: kernel ` info := snaptest.MockSnap(c, yaml, &snap.SideInfo{Revision: snap.R(11)}) - reboot, err := be.LinkSnap(info, coreDev, backend.LinkContext{}, s.perfTimings) + err := be.LinkSnap(info, coreDev, backend.LinkContext{}, s.perfTimings) + c.Assert(err, IsNil) + + isUndo := false + reboot, err := s.be.MaybePrepareReboot(info, mockDev, isUndo) c.Assert(err, IsNil) c.Check(reboot, DeepEquals, boot.RebootInfo{}) } @@ -412,7 +426,7 @@ type: snapd ` info := snaptest.MockSnap(c, yaml, &snap.SideInfo{Revision: snap.R(11)}) - _, err := be.LinkSnap(info, coreDev, backend.LinkContext{}, s.perfTimings) + err := be.LinkSnap(info, coreDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, IsNil) c.Assert(called, Equals, true) } @@ -433,10 +447,10 @@ apps: ` info := snaptest.MockSnap(c, yaml, &snap.SideInfo{Revision: snap.R(11)}) - _, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, IsNil) - _, err = s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err = s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, IsNil) l, err := filepath.Glob(filepath.Join(dirs.SnapBinariesDir, "*")) @@ -475,7 +489,7 @@ apps: ` info := snaptest.MockSnap(c, yaml, &snap.SideInfo{Revision: snap.R(11)}) - _, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, IsNil) err = s.be.UnlinkSnap(info, backend.LinkContext{}, progress.Null) @@ -506,7 +520,7 @@ func (s *linkSuite) TestLinkFailsForUnsetRevision(c *C) { info := &snap.Info{ SuggestedName: "foo", } - _, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, ErrorMatches, `cannot link snap "foo" with unset revision`) } @@ -545,7 +559,11 @@ func (s *linkSuite) TestLinkSnapdSnapOnCore(c *C) { info, _ := mockSnapdSnapForLink(c) - reboot, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err = s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + c.Assert(err, IsNil) + + isUndo := false + reboot, err := s.be.MaybePrepareReboot(info, mockDev, isUndo) c.Assert(err, IsNil) c.Assert(reboot, Equals, boot.RebootInfo{RebootRequired: false}) @@ -654,7 +672,7 @@ func (s *linkCleanupSuite) testLinkCleanupDirOnFail(c *C, dir string) { c.Assert(os.Chmod(dir, 0555), IsNil) defer os.Chmod(dir, 0755) - _, err := s.be.LinkSnap(s.info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(s.info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, NotNil) _, isPathError := err.(*os.PathError) _, isLinkError := err.(*os.LinkError) @@ -699,7 +717,7 @@ func (s *linkCleanupSuite) TestLinkCleanupOnSystemctlFail(c *C) { }) defer r() - _, err := s.be.LinkSnap(s.info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(s.info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, ErrorMatches, "ouchie") for _, d := range []string{dirs.SnapBinariesDir, dirs.SnapDesktopFilesDir, dirs.SnapServicesDir} { @@ -719,7 +737,7 @@ func (s *linkCleanupSuite) TestLinkCleansUpDataDirAndSymlinksOnSymlinkFail(c *C) c.Assert(os.Chmod(d, 0555), IsNil) defer os.Chmod(d, 0755) - _, err := s.be.LinkSnap(s.info, mockDev, backend.LinkContext{}, s.perfTimings) + err := s.be.LinkSnap(s.info, mockDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, ErrorMatches, `(?i).*symlink.*permission denied.*`) c.Check(s.info.DataDir(), testutil.FileAbsent) @@ -757,8 +775,12 @@ func (s *linkCleanupSuite) testLinkCleanupFailedSnapdSnapOnCorePastWrappers(c *C linkCtx := backend.LinkContext{ FirstInstall: firstInstall, } - reboot, err := s.be.LinkSnap(info, mockDev, linkCtx, s.perfTimings) + err = s.be.LinkSnap(info, mockDev, linkCtx, s.perfTimings) c.Assert(err, ErrorMatches, fmt.Sprintf("symlink %s /.*/snapd/current.*: permission denied", info.Revision)) + + isUndo := false + reboot, err := s.be.MaybePrepareReboot(info, mockDev, isUndo) + c.Assert(err, IsNil) c.Assert(reboot, Equals, boot.RebootInfo{RebootRequired: false}) checker := testutil.FilePresent @@ -814,7 +836,7 @@ version: 1.0 snapinstance := snaptest.MockSnapInstance(c, "instance-snap_foo", yaml, &snap.SideInfo{Revision: snap.R(11)}) - _, err := s.be.LinkSnap(snapinstance, mockDev, + err := s.be.LinkSnap(snapinstance, mockDev, backend.LinkContext{ HasOtherInstances: tc.otherInstances, }, @@ -871,7 +893,11 @@ func (s *snapdOnCoreUnlinkSuite) TestUndoGeneratedWrappers(c *C) { return filepath.Join(dirs.SnapServicesDir, filepath.Base(p)) } - reboot, err := s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + err = s.be.LinkSnap(info, mockDev, backend.LinkContext{}, s.perfTimings) + c.Assert(err, IsNil) + + isUndo := false + reboot, err := s.be.MaybePrepareReboot(info, mockDev, isUndo) c.Assert(err, IsNil) c.Assert(reboot, Equals, boot.RebootInfo{RebootRequired: false}) @@ -964,7 +990,7 @@ apps: linkCtxWithTooling := backend.LinkContext{ RequireMountedSnapdSnap: true, } - _, err := s.be.LinkSnap(info, mockDev, linkCtxWithTooling, s.perfTimings) + err := s.be.LinkSnap(info, mockDev, linkCtxWithTooling, s.perfTimings) c.Assert(err, IsNil) c.Assert(filepath.Join(dirs.SnapServicesDir, "snap.hello.svc.service"), testutil.FileContains, `Wants=usr-lib-snapd.mount @@ -978,7 +1004,7 @@ After=usr-lib-snapd.mount`) linkCtxNoTooling := backend.LinkContext{ RequireMountedSnapdSnap: false, } - _, err = s.be.LinkSnap(info, mockDev, linkCtxNoTooling, s.perfTimings) + err = s.be.LinkSnap(info, mockDev, linkCtxNoTooling, s.perfTimings) c.Assert(err, IsNil) c.Assert(filepath.Join(dirs.SnapServicesDir, "snap.hello.svc.service"), Not(testutil.FileContains), `usr-lib-snapd.mount`) } @@ -1002,7 +1028,7 @@ apps: QuotaGroup: grp, }, } - _, err = s.be.LinkSnap(info, mockDev, linkCtxWithGroup, s.perfTimings) + err = s.be.LinkSnap(info, mockDev, linkCtxWithGroup, s.perfTimings) c.Assert(err, IsNil) c.Assert(filepath.Join(dirs.SnapServicesDir, "snap.hello.svc.service"), testutil.FileContains, "\nSlice=snap.foogroup.slice\n") @@ -1055,7 +1081,7 @@ type: snapd be := backend.NewForPreseedMode() coreDev := boottest.MockUC20Device("run", nil) - _, err = be.LinkSnap(info, coreDev, backend.LinkContext{}, s.perfTimings) + err = be.LinkSnap(info, coreDev, backend.LinkContext{}, s.perfTimings) c.Assert(err, ErrorMatches, `BROKEN`) c.Assert(restartDone, Equals, true) diff --git a/overlord/snapstate/backend_test.go b/overlord/snapstate/backend_test.go index 58ab52ea88b..8cec35af971 100644 --- a/overlord/snapstate/backend_test.go +++ b/overlord/snapstate/backend_test.go @@ -1266,7 +1266,24 @@ func (f *fakeSnappyBackend) SetupSnapSaveData(info *snap.Info, _ snap.Device, me return f.maybeErrForLastOp() } -func (f *fakeSnappyBackend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx backend.LinkContext, tm timings.Measurer) (rebootInfo boot.RebootInfo, err error) { +func (f *fakeSnappyBackend) MaybePrepareReboot( + info *snap.Info, dev snap.Device, isUndo bool) (boot.RebootInfo, error) { + + op := &fakeOp{ + op: "maybe-prepare-reboot", + } + f.appendOp(op) + + reboot := false + if f.linkSnapMaybeReboot { + reboot = info.InstanceName() == dev.Base() || + (f.linkSnapRebootFor != nil && f.linkSnapRebootFor[info.InstanceName()]) + } + + return boot.RebootInfo{RebootRequired: reboot}, nil +} + +func (f *fakeSnappyBackend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx backend.LinkContext, tm timings.Measurer) (err error) { if info.MountDir() == f.linkSnapWaitTrigger { f.linkSnapWaitCh <- 1 <-f.linkSnapWaitCh @@ -1290,18 +1307,12 @@ func (f *fakeSnappyBackend) LinkSnap(info *snap.Info, dev snap.Device, linkCtx b if info.MountDir() == f.linkSnapFailTrigger { op.op = "link-snap.failed" f.appendOp(op) - return boot.RebootInfo{RebootRequired: false}, errors.New("fail") + return errors.New("fail") } f.appendOp(op) - reboot := false - if f.linkSnapMaybeReboot { - reboot = info.InstanceName() == dev.Base() || - (f.linkSnapRebootFor != nil && f.linkSnapRebootFor[info.InstanceName()]) - } - - return boot.RebootInfo{RebootRequired: reboot}, nil + return nil } func (f *fakeSnappyBackend) LinkComponent(cpi snap.ContainerPlaceInfo, snapRev snap.Revision) error { diff --git a/overlord/snapstate/handlers.go b/overlord/snapstate/handlers.go index e3b387c9870..28e404db21f 100644 --- a/overlord/snapstate/handlers.go +++ b/overlord/snapstate/handlers.go @@ -1341,11 +1341,10 @@ func (m *SnapManager) restoreUnlinkOnError(t *state.Task, info *snap.Info, other } linkCtx := backend.LinkContext{ FirstInstall: false, - IsUndo: true, ServiceOptions: opts, HasOtherInstances: otherInstances, } - _, err = m.backend.LinkSnap(info, deviceCtx, linkCtx, tm) + err = m.backend.LinkSnap(info, deviceCtx, linkCtx, tm) return err } @@ -1604,11 +1603,15 @@ func (m *SnapManager) undoUnlinkCurrentSnap(t *state.Task, _ *tomb.Tomb) error { } linkCtx := backend.LinkContext{ FirstInstall: false, - IsUndo: true, ServiceOptions: opts, HasOtherInstances: otherInstances, } - reboot, err := m.backend.LinkSnap(oldInfo, deviceCtx, linkCtx, perfTimings) + err = m.backend.LinkSnap(oldInfo, deviceCtx, linkCtx, perfTimings) + if err != nil { + return err + } + isUndo := true + reboot, err := m.backend.MaybePrepareReboot(oldInfo, deviceCtx, isUndo) if err != nil { return err } @@ -2247,7 +2250,7 @@ func (m *SnapManager) doLinkSnap(t *state.Task, _ *tomb.Tomb) (err error) { // links the new revision to current and ensures a shared base prefix // directory for parallel installed snaps - rebootInfo, err := m.backend.LinkSnap(newInfo, deviceCtx, linkCtx, perfTimings) + err = m.backend.LinkSnap(newInfo, deviceCtx, linkCtx, perfTimings) // defer a cleanup helper which will unlink the snap if anything fails after // this point defer func() { @@ -2261,7 +2264,7 @@ func (m *SnapManager) doLinkSnap(t *state.Task, _ *tomb.Tomb) (err error) { // snapd snap is special in the sense that we always // need the current symlink, so we restore the link to // the old revision - _, backendErr = m.backend.LinkSnap(oldInfo, deviceCtx, linkCtx, perfTimings) + backendErr = m.backend.LinkSnap(oldInfo, deviceCtx, linkCtx, perfTimings) } else { // snapd during first install and all other snaps backendErr = m.backend.UnlinkSnap(newInfo, linkCtx, pb) @@ -2274,6 +2277,12 @@ func (m *SnapManager) doLinkSnap(t *state.Task, _ *tomb.Tomb) (err error) { if err != nil { return err } + // Prepare for rebooting when needed + isUndo := false + rebootInfo, err := m.backend.MaybePrepareReboot(newInfo, deviceCtx, isUndo) + if err != nil { + return err + } // Restore configuration of the target revision (if available) on revert if isInstalled { @@ -2837,7 +2846,6 @@ func (m *SnapManager) undoLinkSnap(t *state.Task, _ *tomb.Tomb) error { pb := NewTaskProgressAdapterLocked(t) linkCtx := backend.LinkContext{ FirstInstall: firstInstall, - IsUndo: true, HasOtherInstances: otherInstances, } @@ -2853,7 +2861,7 @@ func (m *SnapManager) undoLinkSnap(t *state.Task, _ *tomb.Tomb) error { // the snapd snap is special in the sense that we need to make // sure that a sensible version is always linked as current, // also we never reboot when updating snapd snap - _, backendErr = m.backend.LinkSnap(oldInfo, deviceCtx, linkCtx, perfTimings) + backendErr = m.backend.LinkSnap(oldInfo, deviceCtx, linkCtx, perfTimings) } else { // snapd during first install and all other snaps backendErr = m.backend.UnlinkSnap(newInfo, linkCtx, pb) @@ -3565,11 +3573,16 @@ func (m *SnapManager) undoUnlinkSnap(t *state.Task, _ *tomb.Tomb) error { } linkCtx := backend.LinkContext{ FirstInstall: false, - IsUndo: true, ServiceOptions: opts, HasOtherInstances: otherInstances, } - reboot, err := m.backend.LinkSnap(info, deviceCtx, linkCtx, perfTimings) + err = m.backend.LinkSnap(info, deviceCtx, linkCtx, perfTimings) + if err != nil { + return err + } + + isUndo := true + reboot, err := m.backend.MaybePrepareReboot(info, deviceCtx, isUndo) if err != nil { return err } diff --git a/overlord/snapstate/handlers_link_test.go b/overlord/snapstate/handlers_link_test.go index 4a1361de39f..4b265e70410 100644 --- a/overlord/snapstate/handlers_link_test.go +++ b/overlord/snapstate/handlers_link_test.go @@ -714,6 +714,9 @@ func (s *linkSnapSuite) TestDoUndoUnlinkCurrentSnapWithVitalityScore(c *C) { path: filepath.Join(dirs.SnapMountDir, "foo/11"), vitalityRank: 2, }, + { + op: "maybe-prepare-reboot", + }, } c.Check(s.fakeBackend.ops, DeepEquals, expected) } @@ -924,6 +927,9 @@ func (s *linkSnapSuite) TestDoLinkSnapWithVitalityScore(c *C) { path: filepath.Join(dirs.SnapMountDir, "foo/33"), vitalityRank: 2, }, + { + op: "maybe-prepare-reboot", + }, } c.Check(s.fakeBackend.ops, DeepEquals, expected) } @@ -1536,6 +1542,9 @@ func (s *linkSnapSuite) TestDoLinkSnapdDiscardsNsOnDowngrade(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "snapd/41"), }, + { + op: "maybe-prepare-reboot", + }, } // start with an easier-to-read error if this fails: @@ -1621,6 +1630,9 @@ func (s *linkSnapSuite) TestDoLinkSnapdRemovesAppArmorProfilesOnSnapdDowngrade(c op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "snapd/41"), }, + { + op: "maybe-prepare-reboot", + }, } // start with an easier-to-read error if this fails: @@ -2272,6 +2284,9 @@ func (s *linkSnapSuite) TestUndoLinkSnapdFirstInstall(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "snapd/22"), }, + { + op: "maybe-prepare-reboot", + }, { op: "discard-namespace", name: "snapd", @@ -2350,6 +2365,9 @@ func (s *linkSnapSuite) TestUndoLinkSnapdNthInstall(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "snapd/22"), }, + { + op: "maybe-prepare-reboot", + }, { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "snapd/20"), @@ -2602,6 +2620,9 @@ func (s *linkSnapSuite) testDoLinkSnapWithToolingDependency(c *C, classicOrBase path: filepath.Join(dirs.SnapMountDir, "services-snap/11"), requireSnapdTooling: needsTooling, }, + { + op: "maybe-prepare-reboot", + }, } // start with an easier-to-read error if this fails: diff --git a/overlord/snapstate/snapstate_install_test.go b/overlord/snapstate/snapstate_install_test.go index c220b95cb71..aba84ba0ad4 100644 --- a/overlord/snapstate/snapstate_install_test.go +++ b/overlord/snapstate/snapstate_install_test.go @@ -1383,6 +1383,9 @@ func (s *snapmgrTestSuite) TestInstallRunThrough(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -1583,6 +1586,9 @@ func (s *snapmgrTestSuite) testParallelInstanceInstallRunThrough(c *C, inputFlag op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap_instance/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap_instance", @@ -1795,6 +1801,9 @@ func (s *snapmgrTestSuite) TestInstallUndoRunThroughJustOneSnap(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -1951,6 +1960,9 @@ func (s *snapmgrTestSuite) TestInstallWithCohortRunThrough(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/666"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -2150,6 +2162,9 @@ func (s *snapmgrTestSuite) testInstallWithRevisionRunThrough(c *C, snapName, req op: "link-snap", path: filepath.Join(dirs.SnapMountDir, filepath.Join(snapName, "42")), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: snapName, @@ -2350,6 +2365,9 @@ version: 1.0`) op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "mock/x1"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "mock", @@ -2482,6 +2500,9 @@ epoch: 1* op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "mock/x3"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "mock", @@ -2621,6 +2642,9 @@ epoch: 1* op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "mock/x1"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "mock", @@ -2677,7 +2701,7 @@ version: 1.0`) s.settle(c) // ensure only local install was run, i.e. first actions are pseudo-action current - c.Assert(s.fakeBackend.ops.Ops(), HasLen, 10) + c.Assert(s.fakeBackend.ops.Ops(), HasLen, 11) c.Check(s.fakeBackend.ops[0].op, Equals, "current") c.Check(s.fakeBackend.ops[0].old, Equals, "") // and setup-snap @@ -2690,6 +2714,7 @@ version: 1.0`) c.Check(s.fakeBackend.ops[5].sinfo, DeepEquals, *si) c.Check(s.fakeBackend.ops[6].op, Equals, "link-snap") c.Check(s.fakeBackend.ops[6].path, Equals, filepath.Join(dirs.SnapMountDir, "some-snap/42")) + c.Check(s.fakeBackend.ops[7].op, Equals, "maybe-prepare-reboot") // verify snapSetup info var snapsup snapstate.SnapSetup @@ -2864,6 +2889,9 @@ func (s *snapmgrTestSuite) TestInstallWithoutCoreRunThrough1(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "core/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "core", @@ -2929,6 +2957,9 @@ func (s *snapmgrTestSuite) TestInstallWithoutCoreRunThrough1(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/42"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -3323,6 +3354,8 @@ func (s *snapmgrTestSuite) TestInstallDefaultProviderRunThrough(c *C) { }, { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "snap-content-slot/11"), + }, { + op: "maybe-prepare-reboot", }, { op: "auto-connect:Doing", name: "snap-content-slot", @@ -3375,6 +3408,8 @@ func (s *snapmgrTestSuite) TestInstallDefaultProviderRunThrough(c *C) { }, { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "snap-content-plug/42"), + }, { + op: "maybe-prepare-reboot", }, { op: "auto-connect:Doing", name: "snap-content-plug", @@ -6436,10 +6471,14 @@ func undoOps(instanceName string, newSequence, prevSequence *sequence.RevisionSi path: filepath.Join(dirs.SnapDataDir, instanceName), }) } else { - ops = append(ops, fakeOp{ - op: "link-snap", - path: filepath.Join(dirs.SnapMountDir, instanceName, prevRevision.String()), - }) + ops = append(ops, + fakeOp{ + op: "link-snap", + path: filepath.Join(dirs.SnapMountDir, instanceName, prevRevision.String()), + }, + fakeOp{ + op: "maybe-prepare-reboot"}, + ) } for i := len(newComponents) - 1; i >= 0; i-- { @@ -6702,6 +6741,8 @@ func (s *snapmgrTestSuite) testInstallComponentsRunThrough(c *C, snapName, insta }, { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, filepath.Join(instanceName, snapRevision.String())), + }, { + op: "maybe-prepare-reboot", }}...) // ops for linking components @@ -7030,6 +7071,8 @@ components: }, { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, filepath.Join(instanceName, snapRevision.String())), + }, { + op: "maybe-prepare-reboot", }}...) for _, compName := range compNames { diff --git a/overlord/snapstate/snapstate_remove_test.go b/overlord/snapstate/snapstate_remove_test.go index 746805cfc59..3207a333ff0 100644 --- a/overlord/snapstate/snapstate_remove_test.go +++ b/overlord/snapstate/snapstate_remove_test.go @@ -1571,6 +1571,9 @@ func (s *snapmgrTestSuite) TestRemoveManyUndoRestoresCurrent(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/1"), }, + { + op: "maybe-prepare-reboot", + }, { op: "update-aliases", }, diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index 7a08c86d6b4..cce6bbccf2a 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/overlord/snapstate/snapstate_test.go @@ -1639,6 +1639,9 @@ func (s *snapmgrTestSuite) testRevertRunThrough(c *C, refreshAppAwarenessUX bool op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/2"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -1871,6 +1874,9 @@ func (s *snapmgrTestSuite) TestRevertWithBaseRunThrough(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap-with-base/2"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap-with-base", @@ -1961,6 +1967,9 @@ func (s *snapmgrTestSuite) TestParallelInstanceRevertRunThrough(c *C) { path: filepath.Join(dirs.SnapMountDir, "some-snap_instance/2"), otherInstances: true, }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap_instance", @@ -2030,7 +2039,7 @@ func (s *snapmgrTestSuite) TestRevertWithLocalRevisionRunThrough(c *C) { s.settle(c) - c.Assert(s.fakeBackend.ops.Ops(), HasLen, 8) + c.Assert(s.fakeBackend.ops.Ops(), HasLen, 9) // verify that LocalRevision is still -7 var snapst snapstate.SnapState @@ -2098,6 +2107,9 @@ func (s *snapmgrTestSuite) TestRevertToRevisionNewVersion(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/7"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -2189,6 +2201,9 @@ func (s *snapmgrTestSuite) TestRevertTotalUndoRunThrough(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/1"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -2220,6 +2235,9 @@ func (s *snapmgrTestSuite) TestRevertTotalUndoRunThrough(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/2"), }, + { + op: "maybe-prepare-reboot", + }, { op: "update-aliases", }, @@ -2311,6 +2329,9 @@ func (s *snapmgrTestSuite) TestRevertUndoRunThrough(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/2"), }, + { + op: "maybe-prepare-reboot", + }, { op: "update-aliases", }, @@ -2996,6 +3017,9 @@ func (s *snapmgrTestSuite) TestEnableRunThrough(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/7"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -3158,6 +3182,9 @@ func (s *snapmgrTestSuite) TestParallelInstanceEnableRunThrough(c *C) { path: filepath.Join(dirs.SnapMountDir, "some-snap_instance/7"), otherInstances: true, }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap_instance", @@ -5629,6 +5656,9 @@ func (s *snapmgrTestSuite) TestTransitionCoreRunThrough(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "core/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "core", @@ -8133,6 +8163,9 @@ func (s *snapmgrTestSuite) TestSnapdRefreshTasks(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "snapd/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "snapd", diff --git a/overlord/snapstate/snapstate_update_test.go b/overlord/snapstate/snapstate_update_test.go index 8cf4bbc33c0..0b045224470 100644 --- a/overlord/snapstate/snapstate_update_test.go +++ b/overlord/snapstate/snapstate_update_test.go @@ -129,6 +129,9 @@ func (s *snapmgrTestSuite) TestUpdateDoesGC(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -270,6 +273,7 @@ func (s *snapmgrTestSuite) testUpdateScenario(c *C, desc string, t switchScenari "setup-profiles:Doing", "candidate", "link-snap", + "maybe-prepare-reboot", "auto-connect:Doing", "update-aliases", "cleanup-trash", @@ -415,6 +419,9 @@ func (s *snapmgrTestSuite) testUpdateCanDoBackwards(c *C, refreshAppAwarenessUX op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/7"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -932,6 +939,7 @@ func (s *snapmgrTestSuite) testUpdateAmendRunThrough(c *C, tryMode bool, compone "setup-profiles:Doing", "candidate", "link-snap", + "maybe-prepare-reboot", }...) for range components { ops = append(ops, "link-component") @@ -1190,6 +1198,9 @@ func (s *snapmgrTestSuite) testUpdateRunThrough(c *C, refreshAppAwarenessUX bool op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "services-snap/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "services-snap", @@ -1565,6 +1576,9 @@ func (s *snapmgrTestSuite) testParallelInstanceUpdateRunThrough(c *C, refreshApp op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "services-snap_instance/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "services-snap_instance", @@ -2501,6 +2515,9 @@ func (s *snapmgrTestSuite) testUpdateUndoRunThrough(c *C, refreshAppAwarenessUX op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/7"), }, + { + op: "maybe-prepare-reboot", + }, }...) // aliases removal undo is skipped when refresh-app-awareness-ux is enabled if !refreshAppAwarenessUX { @@ -2816,6 +2833,9 @@ func (s *snapmgrTestSuite) testUpdateTotalUndoRunThrough(c *C, refreshAppAwarene op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/11"), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: "some-snap", @@ -2866,6 +2886,9 @@ func (s *snapmgrTestSuite) testUpdateTotalUndoRunThrough(c *C, refreshAppAwarene op: "link-snap", path: filepath.Join(dirs.SnapMountDir, "some-snap/7"), }, + { + op: "maybe-prepare-reboot", + }, }...) // aliases removal undo is skipped when refresh-app-awareness-ux is enabled if !refreshAppAwarenessUX { @@ -14344,6 +14367,9 @@ func (s *snapmgrTestSuite) TestUpdateBackToPrevRevision(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, instanceName, prevSnapRev.String()), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: instanceName, @@ -14565,6 +14591,9 @@ func (s *snapmgrTestSuite) testRevertWithComponents(c *C, undo bool) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, instanceName, prevSnapRev.String()), }, + { + op: "maybe-prepare-reboot", + }, { op: "auto-connect:Doing", name: instanceName, @@ -14609,6 +14638,9 @@ func (s *snapmgrTestSuite) testRevertWithComponents(c *C, undo bool) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), }, + { + op: "maybe-prepare-reboot", + }, { op: "update-aliases", }, @@ -14961,6 +14993,9 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevision(c *C) { op: "link-snap", path: filepath.Join(dirs.SnapMountDir, instanceName, prevSnapRev.String()), }, + { + op: "maybe-prepare-reboot", + }, }...) for i, compName := range components { @@ -15486,6 +15521,9 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW op: "link-snap", path: filepath.Join(dirs.SnapMountDir, instanceName, newSnapRev.String()), }, + { + op: "maybe-prepare-reboot", + }, }...) for _, cs := range expectedComponentStates { @@ -16131,6 +16169,9 @@ components: op: "link-snap", path: filepath.Join(dirs.SnapMountDir, instanceName, newSnapRev.String()), }, + { + op: "maybe-prepare-reboot", + }, }...) for _, cs := range expectedComponentStates { @@ -16577,6 +16618,9 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThroughOnlyComponentUpdate op: "link-snap", path: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), }, + { + op: "maybe-prepare-reboot", + }, }...) for _, cs := range expectedComponentStates {