Skip to content

Commit

Permalink
o/snapstate: reorder tasks such that snap and component downloads now…
Browse files Browse the repository at this point in the history 
… happen consecutively (#14697)

* o/snapstate: reorder tasks such that snap and component downloads now happen consecutively

* o/snapstate: add edge for local modifications to component installations

* o/snapstate: rename componentTaskSetsForSnapInstall to multiComponentInstallTaskSet and rename fields to be relative to component task chains

* o/snapstate: rename pointer that keeps track of LastBeforeLocalModificationsEdge edge
  • Loading branch information
@andrewphelpsj
andrewphelpsj authored Dec 4, 2024
1 parent ff58c9b commit d3134e6
Show file tree
Hide file tree
Showing 5 changed files with 328 additions and 177 deletions.
30 changes: 22 additions & 8 deletions overlord/snapstate/component.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand Down
24 changes: 17 additions & 7 deletions overlord/snapstate/component_install_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
95 changes: 56 additions & 39 deletions overlord/snapstate/snapstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -860,36 +861,52 @@ 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,
snapst *SnapState,
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 {
Expand Down
Loading

0 comments on commit d3134e6

Please sign in to comment.