diff --git a/overlord/snapstate/snapstate.go b/overlord/snapstate/snapstate.go index e96e5da595b..6aa1fdcc4ee 100644 --- a/overlord/snapstate/snapstate.go +++ b/overlord/snapstate/snapstate.go @@ -52,6 +52,7 @@ import ( "github.com/snapcore/snapd/release" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/channel" + "github.com/snapcore/snapd/snap/naming" "github.com/snapcore/snapd/snapdenv" "github.com/snapcore/snapd/store" "github.com/snapcore/snapd/strutil" @@ -273,6 +274,87 @@ func isCoreSnap(snapName string) bool { return snapName == defaultCoreSnapName } +// removeExtraComponentsTasks creates tasks that will remove unwanted components +// that are currently installed alongside the snap revision that is about to be +// installed. If the new snap revision is not in the sequence, then we don't +// have anything to do. If the revision is in the sequence, then we generate +// tasks that will unlink components that are not in compsups. +// +// This is mostly relevant when we're moving from one snap revision to another +// snap revision that has already been installed on the system. The target snap +// might have components that are installed that we don't want any more. +func removeExtraComponentsTasks(st *state.State, snapst *SnapState, newRevision snap.Revision, compsups []ComponentSetup) ( + unlinkTasks, discardTasks []*state.Task, err error, +) { + idx := snapst.LastIndex(newRevision) + if idx < 0 { + return nil, nil, nil + } + si := snapst.Sequence.Revisions[idx].Snap + + keep := make(map[naming.ComponentRef]bool, len(compsups)) + for _, compsup := range compsups { + keep[compsup.CompSideInfo.Component] = true + } + + snapst.CurrentComponentSideInfos() + + linkedForRevision, err := snapst.ComponentInfosForRevision(newRevision) + if err != nil { + return nil, nil, err + } + + // this is just a throwaway SnapSetup we create here so that + // unlink-component knows which snap revision we're unlinking the component + // from. note that we don't need to worry about kernel module components + // here, since the components that we are removing are not associated with + // the currently installed snap revision. + snapsup := SnapSetup{ + SideInfo: &snap.SideInfo{ + RealName: si.RealName, + SnapID: si.SnapID, + Revision: si.Revision, + }, + InstanceKey: snapst.InstanceKey, + Type: snap.Type(snapst.SnapType), + } + + for _, ci := range linkedForRevision { + if keep[ci.Component] { + continue + } + + // note that we shouldn't ever be able to lose components during a + // refresh without a snap revision change. this might be able to happen + // once we introduce components and validation sets? if that is the + // case, we'll need to take care here to use "unlink-current-component" + // and point it to the correct snap setup task. + if snapst.Current == newRevision { + return nil, nil, errors.New("internal error: cannot lose a component during a refresh without a snap revision change") + } + + unlink := st.NewTask("unlink-component", fmt.Sprintf( + i18n.G("Unlink component %q for snap revision %s"), ci.Component, snapsup.Revision(), + )) + + unlink.Set("snap-setup", snapsup) + unlink.Set("component-setup", ComponentSetup{ + CompSideInfo: &ci.ComponentSideInfo, + CompType: ci.Type, + }) + unlinkTasks = append(unlinkTasks, unlink) + + discard := st.NewTask("discard-component", fmt.Sprintf( + i18n.G("Discard previous revision for component %q"), ci.Component, + )) + discard.Set("snap-setup-task", unlink.ID()) + discard.Set("component-setup-task", unlink.ID()) + discardTasks = append(discardTasks, discard) + } + + return unlinkTasks, discardTasks, nil +} + func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups []ComponentSetup, flags int, fromChange string, inUseCheck func(snap.Type) (boot.InUseFunc, error)) (*state.TaskSet, error) { tr := config.NewTransaction(st) experimentalRefreshAppAwareness, err := features.Flag(tr, features.RefreshAppAwareness) @@ -442,6 +524,15 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ } } + removeExtraComps, discardExtraComps, err := removeExtraComponentsTasks(st, snapst, snapsup.Revision(), compsups) + if err != nil { + return nil, err + } + + for _, t := range removeExtraComps { + addTask(t) + } + tasksBeforePreRefreshHook, tasksAfterLinkSnap, tasksAfterPostOpHook, tasksBeforeDiscard, compSetupIDs, err := splitComponentTasksForInstall( compsups, st, snapst, snapsup, prepare.ID(), fromChange, ) @@ -449,6 +540,8 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ return nil, err } + tasksBeforeDiscard = append(tasksBeforeDiscard, discardExtraComps...) + for _, t := range tasksBeforePreRefreshHook { addTask(t) } diff --git a/overlord/snapstate/snapstate_update_test.go b/overlord/snapstate/snapstate_update_test.go index ede68e14e2a..c9a1b6bde29 100644 --- a/overlord/snapstate/snapstate_update_test.go +++ b/overlord/snapstate/snapstate_update_test.go @@ -13888,10 +13888,19 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevision(c *C) { }) currentKmodComps = append(currentKmodComps, ¤tCsi) } - currentResources[comp] = snap.R(i + 3) } + extraCsi := snap.ComponentSideInfo{ + Component: naming.NewComponentRef(snapName, "test-extra-component"), + Revision: snap.R(len(components) + 1), + } + err := seq.AddComponentForRevision(prevSnapRev, &sequence.ComponentState{ + SideInfo: &extraCsi, + CompType: compNameToType(extraCsi.Component.ComponentName), + }) + c.Assert(err, IsNil) + s.fakeStore.snapResourcesFn = func(info *snap.Info) []store.SnapResourceResult { c.Assert(info.InstanceName(), DeepEquals, instanceName) var results []store.SnapResourceResult @@ -13986,6 +13995,11 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevision(c *C) { }, } + expected = append(expected, fakeOp{ + op: "unlink-component", + path: snap.ComponentMountDir(extraCsi.Component.ComponentName, extraCsi.Revision, instanceName), + }) + for i, compName := range components { csi := snap.ComponentSideInfo{ Component: naming.NewComponentRef(snapName, compName), @@ -14083,6 +14097,21 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevision(c *C) { }) } + discardedContainerName := fmt.Sprintf("%s+%s", instanceName, extraCsi.Component.ComponentName) + discardedFilename := fmt.Sprintf("%s_%v.comp", discardedContainerName, extraCsi.Revision) + expected = append(expected, []fakeOp{ + { + op: "undo-setup-component", + containerName: discardedContainerName, + containerFileName: discardedFilename, + }, + { + op: "remove-component-dir", + containerName: discardedContainerName, + containerFileName: discardedFilename, + }, + }...) + expected = append(expected, fakeOp{ op: "cleanup-trash", name: instanceName,