diff --git a/overlord/snapstate/backend_test.go b/overlord/snapstate/backend_test.go index 89cc6c3ead5..b95b6a645c0 100644 --- a/overlord/snapstate/backend_test.go +++ b/overlord/snapstate/backend_test.go @@ -523,7 +523,7 @@ func (f *fakeStore) lookupRefresh(cand refreshCand) (*snap.Info, error) { confinement = snap.ClassicConfinement case "channel-for-devmode/stable": confinement = snap.DevModeConfinement - case "channel-for-components": + case "channel-for-components", "channel-for-components-only-component-refresh": components = map[string]*snap.Component{ "test-component": { Type: snap.TestComponent, @@ -628,6 +628,12 @@ func (f *fakeStore) lookupRefresh(cand refreshCand) (*snap.Info, error) { return info, nil } + // this is a special case for testing the case where we only refresh + // component revisions + if cand.channel == "channel-for-components-only-component-refresh" { + return info, nil + } + return nil, store.ErrNoUpdateAvailable } diff --git a/overlord/snapstate/component.go b/overlord/snapstate/component.go index 362cf8a50b5..fe10c06d226 100644 --- a/overlord/snapstate/component.go +++ b/overlord/snapstate/component.go @@ -256,18 +256,18 @@ type componentInstallFlags struct { } type componentInstallTaskSet struct { - compSetupTaskID string - beforeLink []*state.Task - linkTask *state.Task - postOpHookAndAfter []*state.Task - discardTask *state.Task + compSetupTaskID string + beforeLinkTasks []*state.Task + linkTask *state.Task + postHookToDiscardTasks []*state.Task + discardTask *state.Task } func (c *componentInstallTaskSet) taskSet() *state.TaskSet { - tasks := make([]*state.Task, 0, len(c.beforeLink)+1+len(c.postOpHookAndAfter)+1) - tasks = append(tasks, c.beforeLink...) + tasks := make([]*state.Task, 0, len(c.beforeLinkTasks)+1+len(c.postHookToDiscardTasks)+1) + tasks = append(tasks, c.beforeLinkTasks...) tasks = append(tasks, c.linkTask) - tasks = append(tasks, c.postOpHookAndAfter...) + tasks = append(tasks, c.postHookToDiscardTasks...) if c.discardTask != nil { tasks = append(tasks, c.discardTask) } @@ -345,13 +345,13 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup ComponentS compSetupTaskID: prepare.ID(), } - componentTS.beforeLink = append(componentTS.beforeLink, prepare) + componentTS.beforeLinkTasks = append(componentTS.beforeLinkTasks, prepare) if fromStore { validate := st.NewTask("validate-component", fmt.Sprintf( i18n.G("Fetch and check assertions for component %q%s"), compSetup.ComponentName(), revisionStr), ) - componentTS.beforeLink = append(componentTS.beforeLink, validate) + componentTS.beforeLinkTasks = append(componentTS.beforeLinkTasks, validate) addTask(validate) } @@ -360,7 +360,7 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup ComponentS mount := st.NewTask("mount-component", fmt.Sprintf(i18n.G("Mount component %q%s"), compSi.Component, revisionStr)) - componentTS.beforeLink = append(componentTS.beforeLink, mount) + componentTS.beforeLinkTasks = append(componentTS.beforeLinkTasks, mount) addTask(mount) } else { if compSetup.RemoveComponentPath { @@ -379,7 +379,7 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup ComponentS if !snapsup.Revert && compInstalled { preRefreshHook := SetupPreRefreshComponentHook(st, snapsup.InstanceName(), compSi.Component.ComponentName) - componentTS.beforeLink = append(componentTS.beforeLink, preRefreshHook) + componentTS.beforeLinkTasks = append(componentTS.beforeLinkTasks, preRefreshHook) addTask(preRefreshHook) } @@ -396,7 +396,7 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup ComponentS unlink := st.NewTask("unlink-current-component", fmt.Sprintf(i18n.G( "Make current revision for component %q unavailable"), compSi.Component)) - componentTS.beforeLink = append(componentTS.beforeLink, unlink) + componentTS.beforeLinkTasks = append(componentTS.beforeLinkTasks, unlink) addTask(unlink) } @@ -405,7 +405,7 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup ComponentS setupSecurity = st.NewTask("setup-profiles", fmt.Sprintf(i18n.G("Setup component %q%s security profiles"), compSi.Component, revisionStr)) setupSecurity.Set("component-setup-task", prepare.ID()) setupSecurity.Set("snap-setup-task", snapSetupTaskID) - componentTS.beforeLink = append(componentTS.beforeLink, setupSecurity) + componentTS.beforeLinkTasks = append(componentTS.beforeLinkTasks, setupSecurity) } if setupSecurity != nil { // note that we don't use addTask here because this task is shared and @@ -428,7 +428,7 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup ComponentS } else { postOpHook = SetupPostRefreshComponentHook(st, snapsup.InstanceName(), compSi.Component.ComponentName) } - componentTS.postOpHookAndAfter = append(componentTS.postOpHookAndAfter, postOpHook) + componentTS.postHookToDiscardTasks = append(componentTS.postHookToDiscardTasks, postOpHook) addTask(postOpHook) if !compSetup.MultiComponentInstall && kmodSetup == nil && compSetup.CompType == snap.KernelModulesComponent { @@ -437,7 +437,7 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup ComponentS compSi.Component, revisionStr)) kmodSetup.Set("component-setup-task", prepare.ID()) kmodSetup.Set("snap-setup-task", snapSetupTaskID) - componentTS.postOpHookAndAfter = append(componentTS.postOpHookAndAfter, kmodSetup) + componentTS.postHookToDiscardTasks = append(componentTS.postHookToDiscardTasks, kmodSetup) } if kmodSetup != nil { // note that we don't use addTask here because this task is shared and diff --git a/overlord/snapstate/handlers.go b/overlord/snapstate/handlers.go index 64cab875de0..64326d68f03 100644 --- a/overlord/snapstate/handlers.go +++ b/overlord/snapstate/handlers.go @@ -2714,7 +2714,8 @@ func (m *SnapManager) undoLinkSnap(t *state.Task, _ *tomb.Tomb) error { return err } - if len(snapst.Sequence.Revisions) == 1 { + firstInstall := oldCurrent.Unset() + if firstInstall { // XXX: shouldn't these two just log and carry on? this is an undo handler... timings.Run(perfTimings, "discard-snap-namespace", fmt.Sprintf("discard the namespace of snap %q", snapsup.InstanceName()), func(tm timings.Measurer) { err = m.backend.DiscardSnapNamespace(snapsup.InstanceName()) @@ -2801,7 +2802,6 @@ func (m *SnapManager) undoLinkSnap(t *state.Task, _ *tomb.Tomb) error { } pb := NewTaskProgressAdapterLocked(t) - firstInstall := oldCurrent.Unset() linkCtx := backend.LinkContext{ FirstInstall: firstInstall, IsUndo: true, diff --git a/overlord/snapstate/handlers_components.go b/overlord/snapstate/handlers_components.go index caecd4af38d..f956cff2749 100644 --- a/overlord/snapstate/handlers_components.go +++ b/overlord/snapstate/handlers_components.go @@ -523,12 +523,16 @@ func (m *SnapManager) doUnlinkCurrentComponent(t *state.Task, _ *tomb.Tomb) (err defer st.Unlock() // snapSt is a copy of the current state - compSetup, _, snapSt, err := compSetupAndState(t) + compSetup, snapsup, snapSt, err := compSetupAndState(t) if err != nil { return err } cref := compSetup.CompSideInfo.Component + if err := saveCurrentKernelModuleComponents(t, snapsup, snapSt); err != nil { + return err + } + // Expected to be installed snapInfo, err := snapSt.CurrentInfo() if err != nil { @@ -561,12 +565,6 @@ func (m *SnapManager) doUnlinkComponent(t *state.Task, _ *tomb.Tomb) (err error) return err } - // TODO:COMPS: test taking this branch when unlinking components during a - // refresh where we lose components - if err := saveCurrentKernelModuleComponents(t, snapSup, snapSt); err != nil { - return err - } - cref := compSetup.CompSideInfo.Component // Remove component for the specified revision if err := m.unlinkComponent( diff --git a/overlord/snapstate/handlers_components_link_test.go b/overlord/snapstate/handlers_components_link_test.go index cc188d3c037..6252d1cb4ec 100644 --- a/overlord/snapstate/handlers_components_link_test.go +++ b/overlord/snapstate/handlers_components_link_test.go @@ -289,7 +289,7 @@ func (s *linkCompSnapSuite) TestDoUnlinkCurrentComponent(c *C) { setStateWithOneComponent(s.state, snapName, snapRev, compName, compRev) s.state.Unlock() - s.testDoUnlinkComponent(c, snapName, snapRev, compName, compRev, "unlink-current-component", nil) + s.testDoUnlinkComponent(c, snapName, snapRev, compName, compRev, "unlink-current-component", []*snap.ComponentSideInfo{}) } func (s *linkCompSnapSuite) TestDoUnlinkCurrentComponentOtherCompPresent(c *C) { @@ -303,11 +303,11 @@ func (s *linkCompSnapSuite) TestDoUnlinkCurrentComponentOtherCompPresent(c *C) { csi1 := snap.NewComponentSideInfo(naming.NewComponentRef(snapName, compName), compRev) csi2 := snap.NewComponentSideInfo(naming.NewComponentRef(snapName, "other-comp"), snap.R(33)) cs1 := sequence.NewComponentState(csi1, snap.TestComponent) - cs2 := sequence.NewComponentState(csi2, snap.TestComponent) + cs2 := sequence.NewComponentState(csi2, snap.KernelModulesComponent) setStateWithComponents(s.state, snapName, snapRev, []*sequence.ComponentState{cs1, cs2}) s.state.Unlock() - s.testDoUnlinkComponent(c, snapName, snapRev, compName, compRev, "unlink-current-component", nil) + s.testDoUnlinkComponent(c, snapName, snapRev, compName, compRev, "unlink-current-component", []*snap.ComponentSideInfo{cs2.SideInfo}) } func (s *linkCompSnapSuite) TestDoUnlinkCurrentComponentTwoTasks(c *C) { @@ -471,11 +471,6 @@ func (s *linkCompSnapSuite) TestDoUnlinkComponent(c *C) { s.state.Lock() - kmodCsi := &snap.ComponentSideInfo{ - Component: naming.NewComponentRef(snapName, "kmod-comp"), - Revision: snap.R(10), - } - // State must contain the component. Note that in this case // the snap does not need to be active. ssi := &snap.SideInfo{RealName: snapName, Revision: snapRev, @@ -483,7 +478,6 @@ func (s *linkCompSnapSuite) TestDoUnlinkComponent(c *C) { csi := snap.NewComponentSideInfo(naming.NewComponentRef(snapName, compName), compRev) comps := []*sequence.ComponentState{ sequence.NewComponentState(csi, snap.TestComponent), - sequence.NewComponentState(kmodCsi, snap.KernelModulesComponent), } snapstate.Set(s.state, snapName, &snapstate.SnapState{ Active: false, @@ -495,7 +489,7 @@ func (s *linkCompSnapSuite) TestDoUnlinkComponent(c *C) { s.state.Unlock() - s.testDoUnlinkComponent(c, snapName, snapRev, compName, compRev, "unlink-component", []*snap.ComponentSideInfo{kmodCsi}) + s.testDoUnlinkComponent(c, snapName, snapRev, compName, compRev, "unlink-component", nil) } func (s *linkCompSnapSuite) TestDoUnlinkThenUndoUnlinkComponent(c *C) { diff --git a/overlord/snapstate/snapstate.go b/overlord/snapstate/snapstate.go index 71c729d1f07..b96e28c599f 100644 --- a/overlord/snapstate/snapstate.go +++ b/overlord/snapstate/snapstate.go @@ -453,8 +453,14 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ addTask(t) } - // run refresh hooks when updating existing snap, otherwise run install hook further down. - runRefreshHooks := snapst.IsInstalled() && !snapsup.Flags.Revert + // if the snap is already installed, and the revision we are refreshing to + // is the same as the current revision, and we're not forcing an update, + // then we know that we're really modifying the state of components. + componentOnlyUpdate := snapst.IsInstalled() && snapsup.Revision() == snapst.Current && !snapsup.AlwaysUpdate + + // run refresh hooks when updating existing snap, otherwise run install hook + // further down. + runRefreshHooks := snapst.IsInstalled() && !componentOnlyUpdate && !snapsup.Flags.Revert if runRefreshHooks { preRefreshHook := SetupPreRefreshHook(st, snapsup.InstanceName()) addTask(preRefreshHook) @@ -620,8 +626,6 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ startSnapServices := st.NewTask("start-snap-services", fmt.Sprintf(i18n.G("Start snap %q%s services"), snapsup.InstanceName(), revisionStr)) addTask(startSnapServices) - // TODO:COMPS: test discarding components during a snap refresh (coming - // soon!) for _, t := range tasksBeforeDiscard { addTask(t) } @@ -774,9 +778,9 @@ func splitComponentTasksForInstall( compSetupIDs = append(compSetupIDs, componentTS.compSetupTaskID) - tasksBeforePreRefreshHook = append(tasksBeforePreRefreshHook, componentTS.beforeLink...) + tasksBeforePreRefreshHook = append(tasksBeforePreRefreshHook, componentTS.beforeLinkTasks...) tasksAfterLinkSnap = append(tasksAfterLinkSnap, componentTS.linkTask) - tasksAfterPostOpHook = append(tasksAfterPostOpHook, componentTS.postOpHookAndAfter...) + tasksAfterPostOpHook = append(tasksAfterPostOpHook, componentTS.postHookToDiscardTasks...) if componentTS.discardTask != nil { tasksBeforeDiscard = append(tasksBeforeDiscard, componentTS.discardTask) } @@ -1876,17 +1880,35 @@ type update struct { Components []ComponentSetup } -// revisionSatisfied returns true if the state of the snap on the system matches the -// state specified in the update. This method is primarily concerned with the -// revision of the snap. -// -// TODO:COMPS: check if we need to change the state of components -func (u *update) revisionSatisfied() bool { +// revisionSatisfied returns true if the state of the snap on the system matches +// the state specified in the update. This method checks if the currently +// installed snap and components match what is specified in the update. +func (u *update) revisionSatisfied() (bool, error) { if u.Setup.AlwaysUpdate || !u.SnapState.IsInstalled() { - return false + return false, nil + } + + if u.SnapState.Current != u.Setup.Revision() { + return false, nil + } + + comps, err := u.SnapState.CurrentComponentInfos() + if err != nil { + return false, err + } + + currentCompRevs := make(map[string]snap.Revision, len(comps)) + for _, comp := range comps { + currentCompRevs[comp.Component.ComponentName] = comp.Revision } - return u.SnapState.Current == u.Setup.Revision() + for _, comp := range u.Components { + if currentCompRevs[comp.CompSideInfo.Component.ComponentName] != comp.Revision() { + return false, nil + } + } + + return true, nil } func doPotentiallySplitUpdate(st *state.State, requested []string, updates []update, opts Options) ([]string, *UpdateTaskSets, error) { @@ -1991,7 +2013,12 @@ func doUpdate(st *state.State, requested []string, updates []update, opts Option // and bases and then other snaps for _, up := range updates { // if the update is already satisfied, then we can skip it - if up.revisionSatisfied() { + ok, err := up.revisionSatisfied() + if err != nil { + return nil, false, nil, err + } + + if ok { alreadySatisfied = append(alreadySatisfied, up) continue } @@ -2267,7 +2294,12 @@ func autoAliasesUpdate(st *state.State, requested []string, updates []update) (c // snaps with updates updating := make(map[string]bool, len(updates)) for _, up := range updates { - updating[up.Setup.InstanceName()] = !up.revisionSatisfied() + ok, err := up.revisionSatisfied() + if err != nil { + return nil, nil, nil, err + } + + updating[up.Setup.InstanceName()] = !ok } // add explicitly auto-aliases only for snaps that are not updated diff --git a/overlord/snapstate/snapstate_install_test.go b/overlord/snapstate/snapstate_install_test.go index faf588fe2e6..2b5ff462567 100644 --- a/overlord/snapstate/snapstate_install_test.go +++ b/overlord/snapstate/snapstate_install_test.go @@ -6430,8 +6430,22 @@ func undoOps(instanceName string, newSequence, prevSequence *sequence.RevisionSi for i := len(newComponents) - 1; i >= 0; i-- { csi := newComponents[i].SideInfo - containerName := fmt.Sprintf("%s+%s", instanceName, csi.Component.ComponentName) - filename := fmt.Sprintf("%s_%v.comp", containerName, csi.Revision) + compName := csi.Component.ComponentName + compRev := csi.Revision + + containerName := fmt.Sprintf("%s+%s", instanceName, compName) + filename := fmt.Sprintf("%s_%v.comp", containerName, compRev) + + // if the snap revision isn't changing, then we need to re-link the old + // component + if snapRevision == prevRevision { + oldCS := prevSequence.FindComponent(csi.Component) + + ops = append(ops, []fakeOp{{ + op: "link-component", + path: snap.ComponentMountDir(compName, oldCS.SideInfo.Revision, instanceName), + }}...) + } ops = append(ops, []fakeOp{{ op: "undo-setup-component", @@ -6444,16 +6458,18 @@ func undoOps(instanceName string, newSequence, prevSequence *sequence.RevisionSi }}...) } - ops = append(ops, []fakeOp{{ - op: "undo-setup-snap", - name: instanceName, - stype: "app", - path: snapMount, - }, { - op: "remove-snap-dir", - name: instanceName, - path: filepath.Join(dirs.SnapMountDir, instanceName), - }}...) + if snapRevision != prevRevision { + ops = append(ops, []fakeOp{{ + op: "undo-setup-snap", + name: instanceName, + stype: "app", + path: snapMount, + }, { + op: "remove-snap-dir", + name: instanceName, + path: filepath.Join(dirs.SnapMountDir, instanceName), + }}...) + } return ops } diff --git a/overlord/snapstate/snapstate_update_test.go b/overlord/snapstate/snapstate_update_test.go index 8ec9a9b4bf8..3d66771965b 100644 --- a/overlord/snapstate/snapstate_update_test.go +++ b/overlord/snapstate/snapstate_update_test.go @@ -14057,17 +14057,32 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevision(c *C) { } func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThrough(c *C) { - s.testUpdateWithComponentsRunThrough(c, updateWIthComponentsOpts{ + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{ components: []string{"test-component", "kernel-modules-component"}, }) } func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughNoComponents(c *C) { - s.testUpdateWithComponentsRunThrough(c, updateWIthComponentsOpts{}) + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{}) +} + +func (s *snapmgrTestSuite) TestUpdateExplicitlyToSameRevisionRunThrough(c *C) { + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{ + useSameSnapRev: true, + refreshAppAwarenessUX: true, + }) +} + +func (s *snapmgrTestSuite) TestUpdateExplicitlyToSameRevisionRunThroughUndo(c *C) { + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{ + useSameSnapRev: true, + undo: true, + refreshAppAwarenessUX: true, + }) } func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughUndo(c *C) { - s.testUpdateWithComponentsRunThrough(c, updateWIthComponentsOpts{ + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{ components: []string{"test-component", "kernel-modules-component"}, refreshAppAwarenessUX: true, undo: true, @@ -14075,7 +14090,7 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughUndo(c *C) { } func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughInstanceKey(c *C) { - s.testUpdateWithComponentsRunThrough(c, updateWIthComponentsOpts{ + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{ instanceKey: "key", components: []string{"test-component", "kernel-modules-component"}, refreshAppAwarenessUX: true, @@ -14083,7 +14098,7 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughInstanceKey(c *C) { } func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughInstanceKeyUndo(c *C) { - s.testUpdateWithComponentsRunThrough(c, updateWIthComponentsOpts{ + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{ instanceKey: "key", components: []string{"test-component", "kernel-modules-component"}, refreshAppAwarenessUX: true, @@ -14092,7 +14107,7 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughInstanceKeyUndo(c * } func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughLoseComponents(c *C) { - s.testUpdateWithComponentsRunThrough(c, updateWIthComponentsOpts{ + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{ instanceKey: "key", components: []string{"test-component", "kernel-modules-component"}, postRefreshComponents: []string{"test-component"}, @@ -14101,7 +14116,7 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughLoseComponents(c *C } func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughLoseComponentsUndo(c *C) { - s.testUpdateWithComponentsRunThrough(c, updateWIthComponentsOpts{ + s.testUpdateWithComponentsRunThrough(c, updateWithComponentsOpts{ instanceKey: "key", components: []string{"test-component", "kernel-modules-component"}, postRefreshComponents: []string{"test-component"}, @@ -14110,15 +14125,16 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughLoseComponentsUndo( }) } -type updateWIthComponentsOpts struct { +type updateWithComponentsOpts struct { instanceKey string components []string postRefreshComponents []string refreshAppAwarenessUX bool undo bool + useSameSnapRev bool } -func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateWIthComponentsOpts) { +func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateWithComponentsOpts) { if opts.refreshAppAwarenessUX { s.enableRefreshAppAwarenessUX() } @@ -14126,11 +14142,16 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW const ( snapName = "some-snap" snapID = "some-snap-id" - channel = "channel-for-components" ) + channel := "channel-for-components" currentSnapRev := snap.R(7) + newSnapRev := snap.R(11) + if opts.useSameSnapRev { + newSnapRev = currentSnapRev + } + instanceName := snap.InstanceName(snapName, opts.instanceKey) if opts.postRefreshComponents == nil { @@ -14262,7 +14283,12 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW InstanceKey: opts.instanceKey, }) - ts, err := snapstate.Update(s.state, instanceName, nil, s.user.ID, snapstate.Flags{ + var revOpts *snapstate.RevisionOptions + if opts.useSameSnapRev { + revOpts = &snapstate.RevisionOptions{Revision: currentSnapRev} + } + + ts, err := snapstate.Update(s.state, instanceName, revOpts, s.user.ID, snapstate.Flags{ NoReRefresh: true, }) c.Assert(err, IsNil) @@ -14290,7 +14316,12 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW // local modifications, edge must be set te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - c.Assert(te.Kind(), Equals, "validate-snap") + + if opts.useSameSnapRev { + c.Assert(te.Kind(), Equals, "prepare-snap") + } else { + c.Assert(te.Kind(), Equals, "validate-snap") + } s.settle(c) @@ -14300,61 +14331,64 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW c.Assert(chg.Err(), IsNil, Commentf("change tasks:\n%s", printTasks(chg.Tasks()))) } - expected := fakeOps{ - { - op: "storesvc-snap-action", - curSnaps: []store.CurrentSnap{{ - InstanceName: instanceName, - SnapID: snapID, - Revision: currentSnapRev, - TrackingChannel: channel, - RefreshedDate: refreshedDate, - Epoch: snap.E("1*"), - Resources: currentResources, - }}, - userID: 1, - }, - { - op: "storesvc-snap-action:action", - action: store.SnapAction{ - Action: "refresh", - InstanceName: instanceName, - SnapID: snapID, - Channel: channel, - Flags: store.SnapActionEnforceValidation, + var expected fakeOps + if !opts.useSameSnapRev { + expected = fakeOps{ + { + op: "storesvc-snap-action", + curSnaps: []store.CurrentSnap{{ + InstanceName: instanceName, + SnapID: snapID, + Revision: currentSnapRev, + TrackingChannel: channel, + RefreshedDate: refreshedDate, + Epoch: snap.E("1*"), + Resources: currentResources, + }}, + userID: 1, }, - revno: newSnapRev, - userID: 1, - }, - { - op: "storesvc-download", - name: snapName, - }, - { - op: "validate-snap:Doing", - name: instanceName, - revno: newSnapRev, - }, - { - op: "current", - old: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), - }, - { - op: "open-snap-file", - path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), - sinfo: snap.SideInfo{ - RealName: snapName, - SnapID: snapID, - Channel: channel, - Revision: newSnapRev, + { + op: "storesvc-snap-action:action", + action: store.SnapAction{ + Action: "refresh", + InstanceName: instanceName, + SnapID: snapID, + Channel: channel, + Flags: store.SnapActionEnforceValidation, + }, + revno: newSnapRev, + userID: 1, }, - }, - { - op: "setup-snap", - name: instanceName, - path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), - revno: newSnapRev, - }, + { + op: "storesvc-download", + name: snapName, + }, + { + op: "validate-snap:Doing", + name: instanceName, + revno: newSnapRev, + }, + { + op: "current", + old: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), + }, + { + op: "open-snap-file", + path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), + sinfo: snap.SideInfo{ + RealName: snapName, + SnapID: snapID, + Channel: channel, + Revision: newSnapRev, + }, + }, + { + op: "setup-snap", + name: instanceName, + path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), + revno: newSnapRev, + }, + } } for _, cs := range expectedComponentStates { @@ -14414,7 +14448,7 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW { op: "setup-profiles:Doing", name: instanceName, - revno: snap.R(11), + revno: newSnapRev, }, { op: "candidate", @@ -14422,7 +14456,7 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW RealName: snapName, SnapID: snapID, Channel: channel, - Revision: snap.R(11), + Revision: newSnapRev, }, }, { @@ -14444,7 +14478,7 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW { op: "auto-connect:Doing", name: instanceName, - revno: snap.R(11), + revno: newSnapRev, }, { op: "update-aliases", @@ -14492,11 +14526,14 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW }) } - downloads := []fakeDownload{{ - macaroon: s.user.StoreMacaroon, - name: snapName, - target: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), - }} + var downloads []fakeDownload + if !opts.useSameSnapRev { + downloads = []fakeDownload{{ + macaroon: s.user.StoreMacaroon, + name: snapName, + target: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), + }} + } for _, compName := range opts.postRefreshComponents { downloads = append(downloads, fakeDownload{ macaroon: s.user.StoreMacaroon, @@ -14507,29 +14544,33 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW c.Check(s.fakeStore.downloads, DeepEquals, downloads) - c.Check(s.fakeStore.seenPrivacyKeys["privacy-key"], Equals, true, Commentf("salts seen: %v", s.fakeStore.seenPrivacyKeys)) + if !opts.useSameSnapRev { + c.Check(s.fakeStore.seenPrivacyKeys["privacy-key"], Equals, true, Commentf("salts seen: %v", s.fakeStore.seenPrivacyKeys)) + } + // start with an easier-to-read error if this fails: c.Assert(s.fakeBackend.ops.Ops(), DeepEquals, expected.Ops()) c.Assert(s.fakeBackend.ops, DeepEquals, expected) // check progress task := ts.Tasks()[1] - _, cur, total := task.Progress() - c.Assert(cur, Equals, s.fakeStore.fakeCurrentProgress) - c.Assert(total, Equals, s.fakeStore.fakeTotalProgress) + + if !opts.useSameSnapRev { + _, cur, total := task.Progress() + c.Assert(cur, Equals, s.fakeStore.fakeCurrentProgress) + c.Assert(total, Equals, s.fakeStore.fakeTotalProgress) + } // verify snapSetup info var snapsup snapstate.SnapSetup err = task.Get("snap-setup", &snapsup) c.Assert(err, IsNil) - c.Assert(snapsup, DeepEquals, snapstate.SnapSetup{ + + expectedSnapsup := snapstate.SnapSetup{ Channel: channel, UserID: s.user.ID, - SnapPath: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), - DownloadInfo: &snap.DownloadInfo{ - DownloadURL: "https://some-server.com/some/path.snap", - }, + SnapPath: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), SideInfo: snapsup.SideInfo, Type: snap.TypeApp, Version: "some-snapVer", @@ -14539,10 +14580,18 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW }, InstanceKey: opts.instanceKey, PreUpdateKernelModuleComponents: currentKmodComps, - }) + } + + if !opts.useSameSnapRev { + expectedSnapsup.DownloadInfo = &snap.DownloadInfo{ + DownloadURL: "https://some-server.com/some/path.snap", + } + } + + c.Assert(snapsup, DeepEquals, expectedSnapsup) c.Assert(snapsup.SideInfo, DeepEquals, &snap.SideInfo{ RealName: snapName, - Revision: snap.R(11), + Revision: newSnapRev, Channel: channel, SnapID: snapID, }) @@ -14556,7 +14605,11 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW c.Assert(snapst.LastRefreshTime, NotNil) c.Check(snapst.LastRefreshTime.Equal(now), Equals, true) c.Assert(snapst.Active, Equals, true) - c.Assert(snapst.Sequence.Revisions, HasLen, 2) + if opts.useSameSnapRev { + c.Assert(snapst.Sequence.Revisions, HasLen, 1) + } else { + c.Assert(snapst.Sequence.Revisions, HasLen, 2) + } c.Assert(snapst.Sequence.Revisions[0], DeepEquals, currentSeq.Revisions[0]) cand := sequence.NewRevisionSideState(&snap.SideInfo{ @@ -14576,8 +14629,11 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW }) } - // add our new revision to the sequence - currentSeq.Revisions = append(currentSeq.Revisions, cand) + if opts.useSameSnapRev { + currentSeq.Revisions = []*sequence.RevisionSideState{cand} + } else { + currentSeq.Revisions = append(currentSeq.Revisions, cand) + } c.Assert(snapst.Sequence, DeepEquals, currentSeq) @@ -14754,3 +14810,453 @@ func (s *snapmgrTestSuite) TestUpdateTasksWithComponentsRemoved(c *C) { c.Assert(err, IsNil) c.Check(compSup.CompSideInfo.Component, Equals, cref2) } + +func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughOnlyComponentUpdate(c *C) { + s.testUpdateWithComponentsRunThroughOnlyComponentUpdate(c, updateWithComponentsOpts{ + instanceKey: "key", + components: []string{"test-component", "kernel-modules-component"}, + refreshAppAwarenessUX: true, + }) +} + +func (s *snapmgrTestSuite) TestUpdateWithComponentsRunThroughOnlyComponentUpdateUndo(c *C) { + s.testUpdateWithComponentsRunThroughOnlyComponentUpdate(c, updateWithComponentsOpts{ + instanceKey: "key", + components: []string{"test-component", "kernel-modules-component"}, + refreshAppAwarenessUX: true, + undo: true, + }) +} + +func (s *snapmgrTestSuite) testUpdateWithComponentsRunThroughOnlyComponentUpdate(c *C, opts updateWithComponentsOpts) { + if opts.postRefreshComponents != nil { + c.Fatalf("when refreshing a snap that results in only component revision changes, you cannot lose or gain components") + } + + if opts.refreshAppAwarenessUX { + s.enableRefreshAppAwarenessUX() + } + + const ( + snapName = "some-snap" + snapID = "some-snap-id" + ) + + channel := "channel-for-components-only-component-refresh" + currentSnapRev := snap.R(7) + s.fakeStore.refreshRevnos = map[string]snap.Revision{ + snapID: currentSnapRev, + } + + instanceName := snap.InstanceName(snapName, opts.instanceKey) + + sort.Strings(opts.components) + + originalCompRevisions := make(map[string]snap.Revision) + for i, compName := range opts.components { + originalCompRevisions[compName] = snap.R(i + 1) + } + + updatedCompRevisions := make(map[string]snap.Revision) + for i, compName := range opts.components { + updatedCompRevisions[compName] = snap.R(i + 2) + } + + compNameToType := func(name string) snap.ComponentType { + typ := strings.TrimSuffix(name, "-component") + if typ == name { + c.Fatalf("unexpected component name %q", name) + } + return snap.ComponentType(typ) + } + + s.fakeStore.snapResourcesFn = func(info *snap.Info) []store.SnapResourceResult { + c.Assert(info.InstanceName(), DeepEquals, instanceName) + var results []store.SnapResourceResult + for _, compName := range opts.components { + results = append(results, store.SnapResourceResult{ + DownloadInfo: snap.DownloadInfo{ + DownloadURL: "http://example.com/" + compName, + }, + Name: compName, + Revision: updatedCompRevisions[compName].N, + Type: fmt.Sprintf("component/%s", compNameToType(compName)), + Version: "1.0", + CreatedAt: "2024-01-01T00:00:00Z", + }) + } + return results + } + + // we start without the auxiliary store info (or with an older one) + c.Check(snapstate.AuxStoreInfoFilename(snapID), testutil.FileAbsent) + + si := snap.SideInfo{ + RealName: snapName, + Revision: currentSnapRev, + SnapID: snapID, + Channel: channel, + } + snaptest.MockSnapInstance(c, instanceName, fmt.Sprintf("name: %s", snapName), &si) + fi, err := os.Stat(snap.MountFile(instanceName, si.Revision)) + c.Assert(err, IsNil) + + refreshedDate := fi.ModTime() + + restore := snapstate.MockRevisionDate(nil) + defer restore() + + now, err := time.Parse(time.RFC3339, "2021-06-10T10:00:00Z") + c.Assert(err, IsNil) + + restore = snapstate.MockTimeNow(func() time.Time { + return now + }) + defer restore() + + s.state.Lock() + defer s.state.Unlock() + + if opts.instanceKey != "" { + tr := config.NewTransaction(s.state) + tr.Set("core", "experimental.parallel-instances", true) + tr.Commit() + } + + currentSeq := snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{&si}) + + var currentResources map[string]snap.Revision + for _, comp := range opts.components { + err := currentSeq.AddComponentForRevision(currentSnapRev, &sequence.ComponentState{ + SideInfo: &snap.ComponentSideInfo{ + Component: naming.NewComponentRef(snapName, comp), + Revision: originalCompRevisions[comp], + }, + CompType: compNameToType(comp), + }) + c.Assert(err, IsNil) + + if currentResources == nil { + currentResources = make(map[string]snap.Revision, len(opts.components)) + } + currentResources[comp] = originalCompRevisions[comp] + } + currentComponentStates := currentSeq.Revisions[0].Components + + expectedComponentStates := make([]*sequence.ComponentState, 0, len(opts.components)) + for _, comp := range opts.components { + expectedComponentStates = append(expectedComponentStates, &sequence.ComponentState{ + SideInfo: &snap.ComponentSideInfo{ + Component: naming.NewComponentRef(snapName, comp), + Revision: updatedCompRevisions[comp], + }, + CompType: compNameToType(comp), + }) + } + + s.AddCleanup(snapstate.MockReadComponentInfo(func( + compMntDir string, info *snap.Info, csi *snap.ComponentSideInfo, + ) (*snap.ComponentInfo, error) { + return &snap.ComponentInfo{ + Component: csi.Component, + Type: compNameToType(csi.Component.ComponentName), + Version: "1.0", + ComponentSideInfo: *csi, + }, nil + })) + + snapstate.Set(s.state, instanceName, &snapstate.SnapState{ + Active: true, + Sequence: currentSeq, + Current: si.Revision, + SnapType: "app", + TrackingChannel: channel, + InstanceKey: opts.instanceKey, + }) + + ts, err := snapstate.Update(s.state, instanceName, nil, s.user.ID, snapstate.Flags{}) + c.Assert(err, IsNil) + + chg := s.state.NewChange("refresh", "refresh a snap") + chg.AddAll(ts) + + if opts.undo { + last := lastWithLane(ts.Tasks()) + c.Assert(last, NotNil) + + terr := s.state.NewTask("error-trigger", "provoking total undo") + terr.WaitFor(last) + terr.JoinLane(last.Lanes()[0]) + chg.AddTask(terr) + } + + // check unlink-reason + unlinkTask := findLastTask(chg, "unlink-current-snap") + c.Assert(unlinkTask, NotNil) + var unlinkReason string + unlinkTask.Get("unlink-reason", &unlinkReason) + c.Check(unlinkReason, Equals, "refresh") + + // local modifications, edge must be set + te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) + c.Assert(te, NotNil) + c.Assert(te.Kind(), Equals, "prepare-snap") + + s.settle(c) + + if opts.undo { + c.Assert(chg.Err(), NotNil, Commentf("change tasks:\n%s", printTasks(chg.Tasks()))) + } else { + c.Assert(chg.Err(), IsNil, Commentf("change tasks:\n%s", printTasks(chg.Tasks()))) + } + + expected := fakeOps{ + { + op: "storesvc-snap-action", + curSnaps: []store.CurrentSnap{{ + InstanceName: instanceName, + SnapID: snapID, + Revision: currentSnapRev, + TrackingChannel: channel, + RefreshedDate: refreshedDate, + Epoch: snap.E("1*"), + Resources: currentResources, + }}, + userID: 1, + }, + { + op: "storesvc-snap-action:action", + action: store.SnapAction{ + Action: "refresh", + InstanceName: instanceName, + SnapID: snapID, + Channel: channel, + Flags: store.SnapActionEnforceValidation, + }, + revno: currentSnapRev, + userID: 1, + }, + } + + if !opts.refreshAppAwarenessUX { + expected = append(expected, fakeOp{ + op: "remove-snap-aliases", + name: instanceName, + }) + } + + for _, cs := range expectedComponentStates { + compName := cs.SideInfo.Component.ComponentName + compRev := cs.SideInfo.Revision + containerName := fmt.Sprintf("%s+%s", instanceName, compName) + filename := fmt.Sprintf("%s_%v.comp", containerName, compRev) + + expected = append(expected, []fakeOp{{ + op: "storesvc-download", + name: cs.SideInfo.Component.String(), + }, { + op: "validate-component:Doing", + name: instanceName, + revno: currentSnapRev, + componentName: compName, + componentPath: filepath.Join(dirs.SnapBlobDir, filename), + componentRev: compRev, + componentSideInfo: *cs.SideInfo, + }, { + op: "setup-component", + containerName: containerName, + containerFileName: filename, + }, { + op: "unlink-component", + path: snap.ComponentMountDir(compName, originalCompRevisions[compName], instanceName), + }}...) + } + + expected = append(expected, fakeOps{ + { + op: "run-inhibit-snap-for-unlink", + name: instanceName, + inhibitHint: "refresh", + }, + { + op: "unlink-snap", + path: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), + unlinkSkipBinaries: opts.refreshAppAwarenessUX, + }, + { + op: "copy-data", + path: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), + old: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), + }, + { + op: "setup-snap-save-data", + path: filepath.Join(dirs.SnapDataSaveDir, instanceName), + }, + }...) + + expected = append(expected, fakeOps{ + { + op: "setup-profiles:Doing", + name: instanceName, + revno: currentSnapRev, + }, + { + op: "candidate", + sinfo: snap.SideInfo{ + RealName: snapName, + SnapID: snapID, + Channel: channel, + Revision: currentSnapRev, + }, + }, + { + op: "link-snap", + path: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), + }, + }...) + + for _, cs := range expectedComponentStates { + compName := cs.SideInfo.Component.ComponentName + compRev := cs.SideInfo.Revision + expected = append(expected, []fakeOp{ + { + op: "link-component", + path: snap.ComponentMountDir(compName, compRev, instanceName), + }, + }...) + } + + expected = append(expected, fakeOps{ + { + op: "auto-connect:Doing", + name: instanceName, + revno: currentSnapRev, + }, + { + op: "update-aliases", + }, + }...) + + var currentKmodComps []*snap.ComponentSideInfo + for _, cs := range currentComponentStates { + if cs.CompType == snap.KernelModulesComponent { + currentKmodComps = append(currentKmodComps, cs.SideInfo) + } + } + + var newKmodComps []*snap.ComponentSideInfo + for _, cs := range expectedComponentStates { + if cs.CompType == snap.KernelModulesComponent { + newKmodComps = append(newKmodComps, cs.SideInfo) + } + } + + if len(currentKmodComps) > 0 || len(newKmodComps) > 0 { + expected = append(expected, fakeOp{ + op: "prepare-kernel-modules-components", + currentComps: currentKmodComps, + finalComps: newKmodComps, + }) + } + + for _, cs := range currentComponentStates { + compName := cs.SideInfo.Component.ComponentName + compRev := cs.SideInfo.Revision + containerName := fmt.Sprintf("%s+%s", instanceName, compName) + removedFilename := fmt.Sprintf("%s_%v.comp", containerName, compRev) + expected = append(expected, []fakeOp{ + { + op: "undo-setup-component", + containerName: containerName, + containerFileName: removedFilename, + }, + { + op: "remove-component-dir", + containerName: containerName, + containerFileName: removedFilename, + }, + }...) + } + + expectedSideState := sequence.NewRevisionSideState(&si, expectedComponentStates) + originalSideState := currentSeq.Revisions[0] + + if opts.undo { + expected = append(expected, undoOps(instanceName, expectedSideState, originalSideState)...) + } else { + expected = append(expected, fakeOp{ + op: "cleanup-trash", + name: instanceName, + revno: currentSnapRev, + }) + } + + downloads := make([]fakeDownload, 0, len(opts.components)) + for _, compName := range opts.components { + downloads = append(downloads, fakeDownload{ + macaroon: s.user.StoreMacaroon, + name: fmt.Sprintf("%s+%s", snapName, compName), + target: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s+%s_%d.comp", instanceName, compName, updatedCompRevisions[compName].N)), + }) + } + c.Check(s.fakeStore.downloads, DeepEquals, downloads) + + c.Check(s.fakeStore.seenPrivacyKeys["privacy-key"], Equals, true, Commentf("salts seen: %v", s.fakeStore.seenPrivacyKeys)) + + // start with an easier-to-read error if this fails: + c.Assert(s.fakeBackend.ops.Ops(), DeepEquals, expected.Ops()) + c.Assert(s.fakeBackend.ops, DeepEquals, expected) + + task := ts.Tasks()[1] + + // verify snapSetup info + var snapsup snapstate.SnapSetup + err = task.Get("snap-setup", &snapsup) + c.Assert(err, IsNil) + c.Assert(snapsup, DeepEquals, snapstate.SnapSetup{ + Channel: channel, + UserID: s.user.ID, + DownloadInfo: &snap.DownloadInfo{ + DownloadURL: "https://some-server.com/some/path.snap", + }, + SideInfo: snapsup.SideInfo, + Type: snap.TypeApp, + Version: "some-snapVer", + PlugsOnly: true, + Flags: snapstate.Flags{ + Transaction: client.TransactionPerSnap, + }, + InstanceKey: opts.instanceKey, + PreUpdateKernelModuleComponents: currentKmodComps, + }) + c.Assert(snapsup.SideInfo, DeepEquals, &snap.SideInfo{ + RealName: snapName, + Revision: currentSnapRev, + Channel: channel, + SnapID: snapID, + }) + + // verify snaps in the system state + var snapst snapstate.SnapState + err = snapstate.Get(s.state, instanceName, &snapst) + c.Assert(err, IsNil) + + if !opts.undo { + c.Assert(snapst.LastRefreshTime, NotNil) + c.Check(snapst.LastRefreshTime.Equal(now), Equals, true) + c.Assert(snapst.Active, Equals, true) + + // no new revision added to the sequence, the components should have + // been replaced in-place + c.Assert(snapst.Sequence.Revisions, HasLen, 1) + c.Assert(snapst.Sequence.Revisions[0], DeepEquals, expectedSideState) + } else { + // make sure everything is back to how it started + c.Assert(snapst.Active, Equals, true) + c.Assert(snapst.Sequence.Revisions, HasLen, 1) + c.Assert(snapst.Sequence.Revisions[0].Snap, DeepEquals, currentSeq.Revisions[0].Snap) + + // TODO: figure out why this is out of order and if it is a problem + c.Assert(snapst.Sequence.Revisions[0].Components, testutil.DeepUnsortedMatches, currentSeq.Revisions[0].Components) + } +} diff --git a/overlord/snapstate/target.go b/overlord/snapstate/target.go index efcae64c971..28196e82832 100644 --- a/overlord/snapstate/target.go +++ b/overlord/snapstate/target.go @@ -860,7 +860,11 @@ func (p *updatePlan) revisionChanges(st *state.State, opts Options) ([]*snap.Inf changes := make([]*snap.Info, 0, len(updates)) for _, up := range updates { - if up.revisionSatisfied() { + ok, err := up.revisionSatisfied() + if err != nil { + return nil, err + } + if ok { continue }