diff --git a/overlord/snapstate/component.go b/overlord/snapstate/component.go index d3cb07cb26a..caa4d76f9ed 100644 --- a/overlord/snapstate/component.go +++ b/overlord/snapstate/component.go @@ -294,15 +294,17 @@ type ComponentInstallFlags struct { } type componentInstallTaskSet struct { - compSetupTaskID string - beforeLinkTasks []*state.Task - maybeLinkTask *state.Task - postHookToDiscardTasks []*state.Task - maybeDiscardTask *state.Task + compSetupTaskID string + beforeLocalSystemModificationsTasks []*state.Task + beforeLinkTasks []*state.Task + maybeLinkTask *state.Task + postHookToDiscardTasks []*state.Task + maybeDiscardTask *state.Task } func (c *componentInstallTaskSet) taskSet() *state.TaskSet { - tasks := make([]*state.Task, 0, len(c.beforeLinkTasks)+1+len(c.postHookToDiscardTasks)+1) + tasks := make([]*state.Task, 0, len(c.beforeLocalSystemModificationsTasks)+len(c.beforeLinkTasks)+1+len(c.postHookToDiscardTasks)+1) + tasks = append(tasks, c.beforeLocalSystemModificationsTasks...) tasks = append(tasks, c.beforeLinkTasks...) if c.maybeLinkTask != nil { tasks = append(tasks, c.maybeLinkTask) @@ -312,8 +314,20 @@ func (c *componentInstallTaskSet) taskSet() *state.TaskSet { tasks = append(tasks, c.maybeDiscardTask) } + if len(c.beforeLocalSystemModificationsTasks) == 0 { + panic("component install task set should have at least one task before local modifications are done") + } + + // get the id of the last task right before we do any local modifications + beforeLocalModsID := c.beforeLocalSystemModificationsTasks[len(c.beforeLocalSystemModificationsTasks)-1].ID() + ts := state.NewTaskSet(tasks...) for _, t := range ts.Tasks() { + // note, this can't be a switch since one task might be multiple edges + if t.ID() == beforeLocalModsID { + ts.MarkEdge(t, LastBeforeLocalModificationsEdge) + } + if t.ID() == c.compSetupTaskID { ts.MarkEdge(t, BeginEdge) } @@ -403,7 +417,7 @@ func doInstallComponent( compSetupTaskID: prepare.ID(), } - componentTS.beforeLinkTasks = append(componentTS.beforeLinkTasks, prepare) + componentTS.beforeLocalSystemModificationsTasks = append(componentTS.beforeLocalSystemModificationsTasks, prepare) // if the component we're installing has a revision from the store, then we // need to validate it. note that we will still run this task even if we're @@ -414,7 +428,7 @@ func doInstallComponent( validate := st.NewTask("validate-component", fmt.Sprintf( i18n.G("Fetch and check assertions for component %q%s"), compSetup.ComponentName(), revisionStr), ) - componentTS.beforeLinkTasks = append(componentTS.beforeLinkTasks, validate) + componentTS.beforeLocalSystemModificationsTasks = append(componentTS.beforeLocalSystemModificationsTasks, validate) addTask(validate) } diff --git a/overlord/snapstate/component_install_test.go b/overlord/snapstate/component_install_test.go index 9dcf711c244..4262003fca8 100644 --- a/overlord/snapstate/component_install_test.go +++ b/overlord/snapstate/component_install_test.go @@ -64,19 +64,19 @@ const ( // opts is a bitset with compOpt* as possible values. func expectedComponentInstallTasks(opts int) []string { - beforeLink, link, postOpHooksAndAfter, discard := expectedComponentInstallTasksSplit(opts) - return append(append(append(beforeLink, link...), postOpHooksAndAfter...), discard...) + beforeMount, beforeLink, link, postOpHooksAndAfter, discard := expectedComponentInstallTasksSplit(opts) + return append(append(append(append(beforeMount, beforeLink...), link...), postOpHooksAndAfter...), discard...) } -func expectedComponentInstallTasksSplit(opts int) (beforeLink, link, postOpHooksAndAfter, discard []string) { +func expectedComponentInstallTasksSplit(opts int) (beforeMount, beforeLink, link, postOpHooksAndAfter, discard []string) { if opts&compOptIsLocal != 0 || opts&compOptRevisionPresent != 0 { - beforeLink = []string{"prepare-component"} + beforeMount = []string{"prepare-component"} } else { - beforeLink = []string{"download-component"} + beforeMount = []string{"download-component"} } if opts&compOptIsUnasserted == 0 { - beforeLink = append(beforeLink, "validate-component") + beforeMount = append(beforeMount, "validate-component") } // Revision is not the same as the current one installed @@ -116,7 +116,7 @@ func expectedComponentInstallTasksSplit(opts int) (beforeLink, link, postOpHooks discard = append(discard, "discard-component") } - return beforeLink, link, postOpHooksAndAfter, discard + return beforeMount, beforeLink, link, postOpHooksAndAfter, discard } func checkSetupTasks(c *C, compOpts int, ts *state.TaskSet) { @@ -149,6 +149,7 @@ func checkSetupTasks(c *C, compOpts int, ts *state.TaskSet) { c.Assert(t.Get("snap-setup-task", &storedTaskID), IsNil) c.Assert(storedTaskID, Equals, snapSetupTaskID) } + // ComponentSetup/SnapSetup found must match the ones from the first task csup, ssup, err := snapstate.TaskComponentSetup(t) c.Assert(err, IsNil) @@ -172,6 +173,15 @@ func verifyComponentInstallTasks(c *C, opts int, ts *state.TaskSet) { c.Assert(kinds, DeepEquals, expected) checkSetupTasks(c, opts, ts) + + t, err := ts.Edge(snapstate.LastBeforeLocalModificationsEdge) + c.Assert(err, IsNil) + + if opts&compOptIsUnasserted == 0 { + c.Assert(t.Kind(), Equals, "validate-component") + } else { + c.Assert(t.Kind(), Equals, "prepare-component") + } } func createTestComponent(c *C, snapName, compName string, snapInfo *snap.Info) (*snap.ComponentInfo, string) { diff --git a/overlord/snapstate/snapstate.go b/overlord/snapstate/snapstate.go index 68928791f57..c5f33ca9cbc 100644 --- a/overlord/snapstate/snapstate.go +++ b/overlord/snapstate/snapstate.go @@ -480,11 +480,26 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ prev = tasks[len(tasks)-1] } + componentsTSS, err := splitComponentTasksForInstall( + compsups, st, snapst, snapsup, prepare.ID(), fromChange, + ) + if err != nil { + return nil, err + } + + finalBeforeLocalSystemModifications := prepare + var checkAsserts *state.Task if fromStore { // fetch and check assertions checkAsserts = st.NewTask("validate-snap", fmt.Sprintf(i18n.G("Fetch and check assertions for snap %q%s"), snapsup.InstanceName(), revisionStr)) addTask(checkAsserts) + finalBeforeLocalSystemModifications = checkAsserts + } + + for _, t := range componentsTSS.beforeLocalSystemModificationsTasks { + finalBeforeLocalSystemModifications = t + addTask(t) } // mount @@ -513,16 +528,9 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ addTask(t) } - tasksBeforePreRefreshHook, tasksAfterLinkSnap, tasksAfterPostOpHook, tasksBeforeDiscard, compSetupIDs, err := splitComponentTasksForInstall( - compsups, st, snapst, snapsup, prepare.ID(), fromChange, - ) - if err != nil { - return nil, err - } - - tasksBeforeDiscard = append(tasksBeforeDiscard, discardExtraComps...) + componentsTSS.discardTasks = append(componentsTSS.discardTasks, discardExtraComps...) - for _, t := range tasksBeforePreRefreshHook { + for _, t := range componentsTSS.beforeLinkTasks { addTask(t) } @@ -538,7 +546,7 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ preRefreshHook := SetupPreRefreshHook(st, snapsup.InstanceName()) addTask(preRefreshHook) } - prepare.Set("component-setup-tasks", compSetupIDs) + prepare.Set("component-setup-tasks", componentsTSS.compSetupTaskIDs) if snapst.IsInstalled() { // unlink-current-snap (will stop services for copy-data) @@ -610,7 +618,7 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ linkSnap := st.NewTask("link-snap", fmt.Sprintf(i18n.G("Make snap %q%s available to the system"), snapsup.InstanceName(), revisionStr)) addTask(linkSnap) - for _, t := range tasksAfterLinkSnap { + for _, t := range componentsTSS.linkTasks { addTask(t) } @@ -662,7 +670,7 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ addTask(installHook) } - for _, t := range tasksAfterPostOpHook { + for _, t := range componentsTSS.postHookToDiscardTasks { addTask(t) } @@ -707,7 +715,7 @@ 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) - for _, t := range tasksBeforeDiscard { + for _, t := range componentsTSS.discardTasks { addTask(t) } @@ -809,14 +817,7 @@ func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups [ if installHook != nil { installSet.MarkEdge(installHook, HooksEdge) } - // if snap is being installed from the store, then the last task before - // any system modifications are done is check validate-snap, otherwise - // it's the prepare-snap - if checkAsserts != nil { - installSet.MarkEdge(checkAsserts, LastBeforeLocalModificationsEdge) - } else { - installSet.MarkEdge(prepare, LastBeforeLocalModificationsEdge) - } + installSet.MarkEdge(finalBeforeLocalSystemModifications, LastBeforeLocalModificationsEdge) if flags&noRestartBoundaries == 0 { if err := SetEssentialSnapsRestartBoundaries(st, nil, []*state.TaskSet{installSet}); err != nil { return nil, err @@ -860,6 +861,35 @@ func requiresKmodSetup(snapst *SnapState, compsups []ComponentSetup) bool { return false } +// multiComponentInstallTaskSet contains the tasks that are needed to install +// multiple components. The tasks are partitioned into groups so that they can +// be easily spliced into the chain of tasks created to install a snap. +type multiComponentInstallTaskSet struct { + compSetupTaskIDs []string + beforeLocalSystemModificationsTasks []*state.Task + beforeLinkTasks []*state.Task + linkTasks []*state.Task + postHookToDiscardTasks []*state.Task + discardTasks []*state.Task +} + +func newMultiComponentInstallTaskSet(ctss ...componentInstallTaskSet) multiComponentInstallTaskSet { + var mcts multiComponentInstallTaskSet + for _, cts := range ctss { + mcts.compSetupTaskIDs = append(mcts.compSetupTaskIDs, cts.compSetupTaskID) + mcts.beforeLocalSystemModificationsTasks = append(mcts.beforeLocalSystemModificationsTasks, cts.beforeLocalSystemModificationsTasks...) + mcts.beforeLinkTasks = append(mcts.beforeLinkTasks, cts.beforeLinkTasks...) + if cts.maybeLinkTask != nil { + mcts.linkTasks = append(mcts.linkTasks, cts.maybeLinkTask) + } + mcts.postHookToDiscardTasks = append(mcts.postHookToDiscardTasks, cts.postHookToDiscardTasks...) + if cts.maybeDiscardTask != nil { + mcts.discardTasks = append(mcts.discardTasks, cts.maybeDiscardTask) + } + } + return mcts +} + func splitComponentTasksForInstall( compsups []ComponentSetup, st *state.State, @@ -867,29 +897,16 @@ func splitComponentTasksForInstall( snapsup SnapSetup, snapSetupTaskID string, fromChange string, -) ( - tasksBeforePreRefreshHook, tasksAfterLinkSnap, tasksAfterPostOpHook, tasksBeforeDiscard []*state.Task, - compSetupIDs []string, - err error, -) { +) (multiComponentInstallTaskSet, error) { + componentTSS := make([]componentInstallTaskSet, 0, len(compsups)) for _, compsup := range compsups { componentTS, err := doInstallComponent(st, snapst, compsup, snapsup, snapSetupTaskID, nil, nil, fromChange) if err != nil { - return nil, nil, nil, nil, nil, fmt.Errorf("cannot install component %q: %v", compsup.CompSideInfo.Component, err) - } - - compSetupIDs = append(compSetupIDs, componentTS.compSetupTaskID) - - tasksBeforePreRefreshHook = append(tasksBeforePreRefreshHook, componentTS.beforeLinkTasks...) - if componentTS.maybeLinkTask != nil { - tasksAfterLinkSnap = append(tasksAfterLinkSnap, componentTS.maybeLinkTask) - } - tasksAfterPostOpHook = append(tasksAfterPostOpHook, componentTS.postHookToDiscardTasks...) - if componentTS.maybeDiscardTask != nil { - tasksBeforeDiscard = append(tasksBeforeDiscard, componentTS.maybeDiscardTask) + return multiComponentInstallTaskSet{}, fmt.Errorf("cannot install component %q: %v", compsup.CompSideInfo.Component, err) } + componentTSS = append(componentTSS, componentTS) } - return tasksBeforePreRefreshHook, tasksAfterLinkSnap, tasksAfterPostOpHook, tasksBeforeDiscard, compSetupIDs, nil + return newMultiComponentInstallTaskSet(componentTSS...), nil } func NeedsKernelSetup(model *asserts.Model) bool { diff --git a/overlord/snapstate/snapstate_install_test.go b/overlord/snapstate/snapstate_install_test.go index 024617fcded..121f453a3d7 100644 --- a/overlord/snapstate/snapstate_install_test.go +++ b/overlord/snapstate/snapstate_install_test.go @@ -78,48 +78,53 @@ func expectedDoInstallTasks(typ snap.Type, opts, compOpts, discards int, startTa opts |= updatesBootConfig } } + + var tasksAfterDownload, tasksBeforePreRefreshHook, tasksAfterLinkSnap, tasksAfterPostOpHook, tasksBeforeDiscard []string + for range components { + compOpts |= compOptMultiCompInstall + if opts&localSnap != 0 { + compOpts |= compOptIsLocal + } + if opts&unlinkBefore != 0 { + compOpts |= compOptIsActive | compOptDuringSnapRefresh + } + + beforeMount, beforeLink, link, hooksAndAfter, discard := expectedComponentInstallTasksSplit(compOpts) + + tasksAfterDownload = append(tasksAfterDownload, beforeMount...) + tasksBeforePreRefreshHook = append(tasksBeforePreRefreshHook, beforeLink...) + tasksAfterLinkSnap = append(tasksAfterLinkSnap, link...) + tasksAfterPostOpHook = append(tasksAfterPostOpHook, hooksAndAfter...) + tasksBeforeDiscard = append(tasksBeforeDiscard, discard...) + } + if startTasks == nil { switch { case opts&localSnap != 0: startTasks = []string{ "prerequisites", "prepare-snap", - "mount-snap", } + startTasks = append(startTasks, tasksAfterDownload...) + startTasks = append(startTasks, "mount-snap") case opts&localRevision != 0: startTasks = []string{ "prerequisites", "prepare-snap", } + startTasks = append(startTasks, tasksAfterDownload...) default: startTasks = []string{ "prerequisites", "download-snap", "validate-snap", - "mount-snap", } + startTasks = append(startTasks, tasksAfterDownload...) + startTasks = append(startTasks, "mount-snap") } } expected := startTasks - var tasksBeforePreRefreshHook, tasksAfterLinkSnap, tasksAfterPostOpHook, tasksBeforeDiscard []string - for range components { - compOpts |= compOptMultiCompInstall - if opts&localSnap != 0 { - compOpts |= compOptIsLocal - } - if opts&unlinkBefore != 0 { - compOpts |= compOptIsActive | compOptDuringSnapRefresh - } - - beforeLink, link, hooksAndAfter, discard := expectedComponentInstallTasksSplit(compOpts) - - tasksBeforePreRefreshHook = append(tasksBeforePreRefreshHook, beforeLink...) - tasksAfterLinkSnap = append(tasksAfterLinkSnap, link...) - tasksAfterPostOpHook = append(tasksAfterPostOpHook, hooksAndAfter...) - tasksBeforeDiscard = append(tasksBeforeDiscard, discard...) - } - expected = append(expected, tasksBeforePreRefreshHook...) if opts&unlinkBefore != 0 { @@ -211,16 +216,24 @@ func verifyInstallTasksWithComponents(c *C, typ snap.Type, opts, discards int, c kinds := taskKinds(ts.Tasks()) expected := expectedDoInstallTasks(typ, opts, 0, discards, nil, components, nil) - c.Assert(kinds, DeepEquals, expected) if opts&noLastBeforeModificationsEdge == 0 { te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - if opts&localSnap != 0 { - c.Assert(te.Kind(), Equals, "prepare-snap") + + if len(components) == 0 { + if opts&localSnap != 0 { + c.Assert(te.Kind(), Equals, "prepare-snap") + } else { + c.Assert(te.Kind(), Equals, "validate-snap") + } } else { - c.Assert(te.Kind(), Equals, "validate-snap") + if opts&compOptIsUnasserted == 0 { + c.Assert(te.Kind(), Equals, "validate-component") + } else { + c.Assert(te.Kind(), Equals, "prepare-component") + } } } @@ -6736,7 +6749,30 @@ func (s *snapmgrTestSuite) testInstallComponentsRunThrough(c *C, opts testInstal op: "validate-snap:Doing", name: instanceName, revno: snapRevision, - }, { + }} + + // ops for downloading a component (but not yet mounting it) + for _, cs := range componentStates { + compName := cs.SideInfo.Component.ComponentName + compRev := cs.SideInfo.Revision + containerName := fmt.Sprintf("%s+%s", instanceName, compName) + filename := fmt.Sprintf("%s_%d.comp", containerName, compRev.N) + + expected = append(expected, []fakeOp{{ + op: "storesvc-download", + name: cs.SideInfo.Component.String(), + }, { + op: "validate-component:Doing", + name: instanceName, + revno: snapRevision, + componentName: compName, + componentPath: filepath.Join(dirs.SnapBlobDir, filename), + componentRev: compRev, + componentSideInfo: *cs.SideInfo, + }}...) + } + + expected = append(expected, []fakeOp{{ op: "current", old: "", }, { @@ -6753,31 +6789,19 @@ func (s *snapmgrTestSuite) testInstallComponentsRunThrough(c *C, opts testInstal name: instanceName, path: filepath.Join(dirs.SnapBlobDir, snapFileName), revno: snapRevision, - }} + }}...) // ops for mounting a component (but not yet linking it) for _, cs := range componentStates { compName := cs.SideInfo.Component.ComponentName compRev := cs.SideInfo.Revision containerName := fmt.Sprintf("%s+%s", instanceName, compName) - filename := fmt.Sprintf("%s_%d.comp", containerName, compRev.N) - expected = append(expected, []fakeOp{{ - op: "storesvc-download", - name: cs.SideInfo.Component.String(), - }, { - op: "validate-component:Doing", - name: instanceName, - revno: snapRevision, - componentName: compName, - componentPath: filepath.Join(dirs.SnapBlobDir, filename), - componentRev: compRev, - componentSideInfo: *cs.SideInfo, - }, { + expected = append(expected, fakeOp{ op: "setup-component", containerName: containerName, - containerFileName: filename, - }}...) + containerFileName: fmt.Sprintf("%s_%d.comp", containerName, compRev.N), + }) } if opts.snapName == "some-kernel" { @@ -7155,16 +7179,7 @@ components: c.Assert(chg.Err(), IsNil, Commentf("change tasks:\n%s", printTasks(chg.Tasks()))) } - expected := fakeOps{{ - op: "current", - old: "", - }, { - op: "setup-snap", - name: instanceName, - path: snapPath, - revno: snapRevision, - }} - + var expected fakeOps for _, cs := range componentStates { compName := cs.SideInfo.Component.ComponentName if !opts.unasserted { @@ -7179,7 +7194,20 @@ components: componentSkipAssertionsDownload: true, }) } + } + + expected = append(expected, fakeOps{{ + op: "current", + old: "", + }, { + op: "setup-snap", + name: instanceName, + path: snapPath, + revno: snapRevision, + }}...) + for _, cs := range componentStates { + compName := cs.SideInfo.Component.ComponentName containerName := fmt.Sprintf("%s+%s", instanceName, compName) filename := fmt.Sprintf("%s_%s.comp", containerName, cs.SideInfo.Revision) expected = append(expected, fakeOp{ diff --git a/overlord/snapstate/snapstate_update_test.go b/overlord/snapstate/snapstate_update_test.go index 8f687e5c2a6..53e624d66a3 100644 --- a/overlord/snapstate/snapstate_update_test.go +++ b/overlord/snapstate/snapstate_update_test.go @@ -87,10 +87,19 @@ func verifyUpdateTasksWithComponents(c *C, typ snap.Type, opts, compOpts, discar te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - if opts&localSnap != 0 || opts&localRevision != 0 { - c.Assert(te.Kind(), Equals, "prepare-snap") + + if len(components) == 0 { + if opts&localSnap != 0 || opts&localRevision != 0 { + c.Assert(te.Kind(), Equals, "prepare-snap") + } else { + c.Assert(te.Kind(), Equals, "validate-snap") + } } else { - c.Assert(te.Kind(), Equals, "validate-snap") + if opts&compOptIsUnasserted == 0 { + c.Assert(te.Kind(), Equals, "validate-component") + } else { + c.Assert(te.Kind(), Equals, "prepare-component") + } } } @@ -926,17 +935,21 @@ func (s *snapmgrTestSuite) testUpdateAmendRunThrough(c *C, tryMode bool, compone "storesvc-snap-action:action", "storesvc-download", "validate-snap:Doing", - "current", - "open-snap-file", - "setup-snap", } for range components { ops = append(ops, []string{ "storesvc-download", "validate-component:Doing", - "setup-component", }...) } + ops = append(ops, []string{ + "current", + "open-snap-file", + "setup-snap", + }...) + for range components { + ops = append(ops, "setup-component") + } ops = append(ops, []string{ "remove-snap-aliases", "run-inhibit-snap-for-unlink", @@ -14697,7 +14710,7 @@ func (s *snapmgrTestSuite) testRevertWithComponents(c *C, undo bool) { // local modifications, edge must be set te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - c.Assert(te.Kind(), Equals, "prepare-snap") + c.Assert(te.Kind(), Equals, "validate-component") s.settle(c) @@ -15035,7 +15048,7 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevision(c *C) { // local modifications, edge must be set te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - c.Assert(te.Kind(), Equals, "prepare-snap") + c.Assert(te.Kind(), Equals, "validate-component") s.settle(c) @@ -15076,13 +15089,6 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevision(c *C) { }, } - for _, unlinked := range []snap.ComponentSideInfo{extraCsi, presentInSeqCsi} { - expected = append(expected, fakeOp{ - op: "unlink-component", - path: snap.ComponentMountDir(unlinked.Component.ComponentName, unlinked.Revision, instanceName), - }) - } - for i, compName := range components { csi := snap.ComponentSideInfo{ Component: naming.NewComponentRef(snapName, compName), @@ -15103,11 +15109,30 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevision(c *C) { componentPath: filepath.Join(dirs.SnapBlobDir, filename), componentRev: csi.Revision, componentSideInfo: csi, - }, { + }}...) + } + + for _, unlinked := range []snap.ComponentSideInfo{extraCsi, presentInSeqCsi} { + expected = append(expected, fakeOp{ + op: "unlink-component", + path: snap.ComponentMountDir(unlinked.Component.ComponentName, unlinked.Revision, instanceName), + }) + } + + for i, compName := range components { + csi := snap.ComponentSideInfo{ + Component: naming.NewComponentRef(snapName, compName), + Revision: snap.R(i + 2), + } + + containerName := fmt.Sprintf("%s+%s", instanceName, compName) + filename := fmt.Sprintf("%s_%v.comp", containerName, csi.Revision) + + expected = append(expected, fakeOp{ op: "setup-component", containerName: containerName, containerFileName: filename, - }}...) + }) } expected = append(expected, fakeOp{ @@ -15400,7 +15425,7 @@ func (s *snapmgrTestSuite) TestUpdateWithComponentsBackToPrevRevisionAddComponen // local modifications, edge must be set te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - c.Assert(te.Kind(), Equals, "prepare-snap") + c.Assert(te.Kind(), Equals, "validate-component") s.settle(c) @@ -16002,10 +16027,19 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - if opts.useSameSnapRev { - c.Assert(te.Kind(), Equals, "prepare-snap") + if len(opts.components) == 0 { + if opts.useSameSnapRev { + c.Assert(te.Kind(), Equals, "prepare-snap") + } else { + c.Assert(te.Kind(), Equals, "validate-snap") + } } else { - c.Assert(te.Kind(), Equals, "validate-snap") + if opts.useSameSnapRev { + c.Assert(te.Kind(), Equals, "validate-component") + } else { + c.Assert(te.Kind(), Equals, "validate-component") + } + } // we manually settle here since this test can be slow when the host is @@ -16052,6 +16086,29 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW name: instanceName, revno: newSnapRev, }, + } + + 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: newSnapRev, + componentName: compName, + componentPath: filepath.Join(dirs.SnapBlobDir, filename), + componentRev: compRev, + componentSideInfo: *cs.SideInfo, + }}...) + } + + expected = append(expected, []fakeOp{ { op: "current", old: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), @@ -16072,6 +16129,26 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, newSnapRev)), revno: newSnapRev, }, + }...) + } else { + 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: newSnapRev, + componentName: compName, + componentPath: filepath.Join(dirs.SnapBlobDir, filename), + componentRev: compRev, + componentSideInfo: *cs.SideInfo, + }}...) } } @@ -16081,22 +16158,11 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThrough(c *C, opts updateW 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: newSnapRev, - componentName: compName, - componentPath: filepath.Join(dirs.SnapBlobDir, filename), - componentRev: compRev, - componentSideInfo: *cs.SideInfo, - }, { + expected = append(expected, fakeOp{ op: "setup-component", containerName: containerName, containerFileName: filename, - }}...) + }) } if !opts.refreshAppAwarenessUX { @@ -16534,7 +16600,7 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThroughShareComponents(c * // local modifications, edge must be set te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - c.Assert(te.Kind(), Equals, "validate-snap") + c.Assert(te.Kind(), Equals, "validate-component") // we manually settle here since this test can be slow when the host is // under load @@ -16584,26 +16650,6 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThroughShareComponents(c * name: snapName, revno: newSnapRev, }, - { - op: "current", - old: filepath.Join(dirs.SnapMountDir, snapName, currentSnapRev.String()), - }, - { - op: "open-snap-file", - path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", snapName, newSnapRev)), - sinfo: snap.SideInfo{ - RealName: snapName, - SnapID: snapID, - Channel: channel, - Revision: newSnapRev, - }, - }, - { - op: "setup-snap", - name: snapName, - path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", snapName, newSnapRev)), - revno: newSnapRev, - }, } for _, cs := range expectedComponentStates { @@ -16626,6 +16672,26 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThroughShareComponents(c * } expected = append(expected, fakeOps{ + { + op: "current", + old: filepath.Join(dirs.SnapMountDir, snapName, currentSnapRev.String()), + }, + { + op: "open-snap-file", + path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", snapName, newSnapRev)), + sinfo: snap.SideInfo{ + RealName: snapName, + SnapID: snapID, + Channel: channel, + Revision: newSnapRev, + }, + }, + { + op: "setup-snap", + name: snapName, + path: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", snapName, newSnapRev)), + revno: newSnapRev, + }, { op: "run-inhibit-snap-for-unlink", name: snapName, @@ -17274,7 +17340,7 @@ components: // local modifications, edge must be set te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - c.Assert(te.Kind(), Equals, "prepare-snap") + c.Assert(te.Kind(), Equals, "validate-component") // we manually settle here since this test can be slow when the host is // under load @@ -17289,7 +17355,23 @@ components: c.Assert(chg.Err(), IsNil, Commentf("change tasks:\n%s", printTasks(chg.Tasks()))) } - expected := fakeOps{ + var expected fakeOps + for _, cs := range expectedComponentStates { + compName := cs.SideInfo.Component.ComponentName + compRev := cs.SideInfo.Revision + expected = append(expected, fakeOp{ + op: "validate-component:Doing", + name: instanceName, + revno: newSnapRev, + componentName: compName, + componentPath: components[cs.SideInfo], + componentRev: compRev, + componentSideInfo: *cs.SideInfo, + componentSkipAssertionsDownload: true, + }) + } + + expected = append(expected, fakeOps{ { op: "current", old: filepath.Join(dirs.SnapMountDir, instanceName, currentSnapRev.String()), @@ -17300,7 +17382,7 @@ components: path: snapPath, revno: newSnapRev, }, - } + }...) if !refreshAppAwarenessUX { expected = append(expected, fakeOp{ @@ -17315,20 +17397,11 @@ components: containerName := fmt.Sprintf("%s+%s", instanceName, compName) filename := fmt.Sprintf("%s_%v.comp", containerName, compRev) - expected = append(expected, fakeOps{{ - op: "validate-component:Doing", - name: instanceName, - revno: newSnapRev, - componentName: compName, - componentPath: components[cs.SideInfo], - componentRev: compRev, - componentSideInfo: *cs.SideInfo, - componentSkipAssertionsDownload: true, - }, { + expected = append(expected, fakeOp{ op: "setup-component", containerName: containerName, containerFileName: filename, - }}...) + }) } expected = append(expected, fakeOps{ @@ -17758,7 +17831,7 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThroughOnlyComponentUpdate // local modifications, edge must be set te := ts.MaybeEdge(snapstate.LastBeforeLocalModificationsEdge) c.Assert(te, NotNil) - c.Assert(te.Kind(), Equals, "prepare-snap") + c.Assert(te.Kind(), Equals, "validate-component") // we manually settle here since this test can be slow when the host is // under load @@ -17825,7 +17898,16 @@ func (s *snapmgrTestSuite) testUpdateWithComponentsRunThroughOnlyComponentUpdate componentPath: filepath.Join(dirs.SnapBlobDir, filename), componentRev: compRev, componentSideInfo: *cs.SideInfo, - }, { + }}...) + } + + 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: "setup-component", containerName: containerName, containerFileName: filename,