Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

o/snapstate, o/devicestate: remodel with components from the store #14933

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
216 changes: 169 additions & 47 deletions overlord/devicestate/devicestate.go

Large diffs are not rendered by default.

1,255 changes: 1,248 additions & 7 deletions overlord/devicestate/devicestate_remodel_test.go

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions overlord/devicestate/devicestate_systems_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3599,6 +3599,7 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid

tValidate.WaitFor(tDownload)
ts := state.NewTaskSet(tDownload, tValidate)
ts.MarkEdge(tDownload, snapstate.SnapSetupEdge)
ts.MarkEdge(tValidate, snapstate.LastBeforeLocalModificationsEdge)
return ts, info, nil
})
Expand Down Expand Up @@ -4155,6 +4156,7 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
}

snapsupTask.Set("component-setup-tasks", compsupTaskIDs)
ts.MarkEdge(snapsupTask, snapstate.SnapSetupEdge)
ts.MarkEdge(prev, snapstate.LastBeforeLocalModificationsEdge)

return ts, nil
Expand Down Expand Up @@ -4214,6 +4216,7 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
}

download.Set("component-setup-tasks", compsupTaskIDs)
ts.MarkEdge(download, snapstate.SnapSetupEdge)
ts.MarkEdge(prev, snapstate.LastBeforeLocalModificationsEdge)

_, info := snaptest.MakeTestSnapInfoWithFiles(c, withComponents(snapYamls[name], compsToTypes(name)), snapFiles[name], si)
Expand Down Expand Up @@ -5295,6 +5298,7 @@ plugs:

tValidate.WaitFor(tDownload)
ts := state.NewTaskSet(tDownload, tValidate)
ts.MarkEdge(tDownload, snapstate.SnapSetupEdge)
ts.MarkEdge(tValidate, snapstate.LastBeforeLocalModificationsEdge)
return ts, info, nil
})
Expand Down
9 changes: 9 additions & 0 deletions overlord/devicestate/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"time"

"github.com/snapcore/snapd/asserts"
"github.com/snapcore/snapd/asserts/snapasserts"
"github.com/snapcore/snapd/boot"
"github.com/snapcore/snapd/gadget"
"github.com/snapcore/snapd/gadget/install"
Expand Down Expand Up @@ -185,6 +186,14 @@ func MockSnapstatePathInstallGoal(mock func(snapstate.PathSnap) snapstate.Instal
return testutil.Mock(&snapstatePathInstallGoal, mock)
}

func MockSnapstateInstallComponents(mock func(ctx context.Context, st *state.State, names []string, info *snap.Info, vsets *snapasserts.ValidationSets, opts snapstate.Options) ([]*state.TaskSet, error)) (restore func()) {
return testutil.Mock(&snapstateInstallComponents, mock)
}

func MockSnapstateInstallComponentPath(mock func(st *state.State, csi *snap.ComponentSideInfo, info *snap.Info, path string, opts snapstate.Options) (*state.TaskSet, error)) (restore func()) {
return testutil.Mock(&snapstateInstallComponentPath, mock)
}

func MockSnapstateDownload(f func(ctx context.Context, st *state.State, name string, components []string, blobDirectory string, revOpts snapstate.RevisionOptions, opts snapstate.Options) (*state.TaskSet, *snap.Info, error)) (restore func()) {
r := testutil.Backup(&snapstateDownload)
snapstateDownload = f
Expand Down
49 changes: 33 additions & 16 deletions overlord/snapstate/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,15 @@
// TODO:COMPS: verify validation sets here

snapsup := SnapSetup{
Base: info.Base,
SideInfo: &info.SideInfo,
Channel: info.Channel,
Flags: opts.Flags.ForSnapSetup(),
Type: info.Type(),
Version: info.Version,
PlugsOnly: len(info.Slots) == 0,
InstanceKey: info.InstanceKey,
Base: info.Base,
SideInfo: &info.SideInfo,
Channel: info.Channel,
Flags: opts.Flags.ForSnapSetup(),
Type: info.Type(),
Version: info.Version,
PlugsOnly: len(info.Slots) == 0,
InstanceKey: info.InstanceKey,
ComponentExclusiveSetup: true,
}

setupSecurity := st.NewTask("setup-profiles",
Expand Down Expand Up @@ -143,6 +144,8 @@
setupSecurity.Set("component-setup-tasks", compSetupIDs)

ts := state.NewTaskSet(setupSecurity)
ts.MarkEdge(setupSecurity, SnapSetupEdge)

if kmodSetup != nil {
ts.AddTask(kmodSetup)
}
Expand Down Expand Up @@ -264,14 +267,15 @@
}

snapsup := SnapSetup{
Base: info.Base,
SideInfo: &info.SideInfo,
Channel: info.Channel,
Flags: opts.Flags.ForSnapSetup(),
Type: info.Type(),
Version: info.Version,
PlugsOnly: len(info.Slots) == 0,
InstanceKey: info.InstanceKey,
Base: info.Base,
SideInfo: &info.SideInfo,
Channel: info.Channel,
Flags: opts.Flags.ForSnapSetup(),
Type: info.Type(),
Version: info.Version,
PlugsOnly: len(info.Slots) == 0,
InstanceKey: info.InstanceKey,
ComponentExclusiveSetup: true,
}
compSetup := ComponentSetup{
CompSideInfo: csi,
Expand All @@ -289,6 +293,19 @@
}

ts := componentTS.taskSet()

// TODO:COMPS: instead of doing this, we should convert this function to
// operate on multiple components so that it works like InstallComponents.
// this would improve performance, especially in the case of kernel module
// components.
begin, err := ts.Edge(BeginEdge)
if err != nil {
return nil, fmt.Errorf("internal error: cannot find begin edge on component install task set: %v", err)
}

Check warning on line 304 in overlord/snapstate/component.go

View check run for this annotation

Codecov / codecov/patch

overlord/snapstate/component.go#L303-L304

Added lines #L303 - L304 were not covered by tests

begin.Set("component-setup-tasks", []string{componentTS.compSetupTaskID})
ts.MarkEdge(begin, SnapSetupEdge)

ts.JoinLane(generateLane(st, opts))

return ts, nil
Expand Down
21 changes: 19 additions & 2 deletions overlord/snapstate/component_install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,15 @@ func verifyComponentInstallTasks(c *C, opts int, ts *state.TaskSet) {
} else {
c.Assert(t.Kind(), Equals, "prepare-component")
}

if opts&compOptMultiCompInstall == 0 {
snapsupTask, err := ts.Edge(snapstate.SnapSetupEdge)
c.Assert(err, IsNil)

// for now, all non-multi-component installs are by path, so this will
// point to prepare-component
c.Assert(snapsupTask.Kind(), Equals, "prepare-component")
}
}

func createTestComponent(c *C, snapName, compName string, snapInfo *snap.Info) (*snap.ComponentInfo, string) {
Expand Down Expand Up @@ -502,6 +511,7 @@ func (s *snapmgrTestSuite) TestInstallComponentPathForParallelInstall(c *C) {
var snapsup snapstate.SnapSetup
c.Assert(ts.Tasks()[0].Get("snap-setup", &snapsup), IsNil)
c.Assert(snapsup.InstanceKey, Equals, snapKey)
c.Assert(snapsup.ComponentExclusiveSetup, Equals, true)
}

func (s *snapmgrTestSuite) TestInstallComponentPathWrongSnap(c *C) {
Expand Down Expand Up @@ -1028,12 +1038,18 @@ func (s *snapmgrTestSuite) testInstallComponents(c *C, opts testInstallComponent
tss, err := snapstate.InstallComponents(context.Background(), s.state, components, info, nil, installOpts)
c.Assert(err, IsNil)

setupProfiles := tss[len(tss)-1].Tasks()[0]
setupTs := tss[len(tss)-1]

setupProfiles := setupTs.Tasks()[0]
c.Assert(setupProfiles.Kind(), Equals, "setup-profiles")

prepareKmodComps := tss[len(tss)-1].Tasks()[1]
prepareKmodComps := setupTs.Tasks()[1]
c.Assert(prepareKmodComps.Kind(), Equals, "prepare-kernel-modules-components")

snapsupTask, err := setupTs.Edge(snapstate.SnapSetupEdge)
c.Assert(err, IsNil)
c.Assert(snapsupTask.Kind(), Equals, "setup-profiles")

expectedLane := opts.lane
if opts.transaction != "" && opts.lane == 0 {
expectedLane = 1
Expand All @@ -1052,6 +1068,7 @@ func (s *snapmgrTestSuite) testInstallComponents(c *C, opts testInstallComponent
snapsup, err := snapstate.TaskSnapSetup(prepareKmodComps)
c.Assert(err, IsNil)
c.Assert(snapsup, NotNil)
c.Assert(snapsup.ComponentExclusiveSetup, Equals, true)

for _, ts := range tss[0 : len(tss)-1] {
task := ts.Tasks()[0]
Expand Down
4 changes: 4 additions & 0 deletions overlord/snapstate/snapmgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ type SnapSetup struct {
// case of an undo. Note that this cannot be tagged as omitempty, since we
// need to distinguish between empty and nil.
PreUpdateKernelModuleComponents []*snap.ComponentSideInfo `json:"pre-update-kernel-module-components"`

// ComponentExclusiveSetup is set if this SnapSetup exists only to deal with
// components, and not the snap itself.
ComponentExclusiveSetup bool `json:"component-exclusive-setup,omitempty"`
}

// ConfdbID identifies a confdb.
Expand Down
53 changes: 37 additions & 16 deletions overlord/snapstate/snapstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@

const (
BeginEdge = state.TaskSetEdge("begin")
SnapSetupEdge = state.TaskSetEdge("snap-setup")
BeforeHooksEdge = state.TaskSetEdge("before-hooks")
HooksEdge = state.TaskSetEdge("hooks")
BeforeMaybeRebootEdge = state.TaskSetEdge("before-maybe-reboot")
Expand Down Expand Up @@ -788,6 +789,7 @@

installSet := state.NewTaskSet(tasks...)
installSet.MarkEdge(prereq, BeginEdge)
installSet.MarkEdge(prepare, SnapSetupEdge)
installSet.MarkEdge(setupAliases, BeforeHooksEdge)

// Let tasks know if they have to do something about restarts
Expand Down Expand Up @@ -1641,18 +1643,19 @@
}

snapsup := &SnapSetup{
Channel: revOpts.Channel,
Base: info.Base,
UserID: opts.UserID,
Flags: opts.Flags.ForSnapSetup(),
DownloadInfo: &info.DownloadInfo,
SideInfo: &info.SideInfo,
Type: info.Type(),
Version: info.Version,
InstanceKey: info.InstanceKey,
CohortKey: revOpts.CohortKey,
ExpectedProvenance: info.SnapProvenance,
DownloadBlobDir: downloadDir,
Channel: revOpts.Channel,
Base: info.Base,
UserID: opts.UserID,
Flags: opts.Flags.ForSnapSetup(),
DownloadInfo: &info.DownloadInfo,
SideInfo: &info.SideInfo,
Type: info.Type(),
Version: info.Version,
InstanceKey: info.InstanceKey,
CohortKey: revOpts.CohortKey,
ExpectedProvenance: info.SnapProvenance,
DownloadBlobDir: downloadDir,
ComponentExclusiveSetup: skipSnapDownload,
}

if sar.RedirectChannel != "" {
Expand Down Expand Up @@ -2440,22 +2443,30 @@
return nil, nil
}

var tasks []*state.Task
if err := checkChangeConflictIgnoringOneChange(st, snapst.InstanceName(), nil, opts.FromChange); err != nil {
return nil, err
}

var snapsupTask *state.Task

var tasks []*state.Task
if switchChannel || switchCohortKey {
summary := switchSummary(snapsup.InstanceName(), snapst.TrackingChannel, snapsup.Channel, snapst.CohortKey, snapsup.CohortKey)
switchSnap := st.NewTask("switch-snap-channel", summary)
switchSnap.Set("snap-setup", &snapsup)
snapsupTask = switchSnap

tasks = append(tasks, switchSnap)
}

if toggleIgnoreValidation {
toggle := st.NewTask("toggle-snap-flags", fmt.Sprintf(i18n.G("Toggle snap %q flags"), snapsup.InstanceName()))
toggle.Set("snap-setup", &snapsup)
if snapsupTask == nil {
toggle.Set("snap-setup", &snapsup)
snapsupTask = toggle
} else {
toggle.Set("snap-setup-task", snapsupTask.ID())
}

Check warning on line 2469 in overlord/snapstate/snapstate.go

View check run for this annotation

Codecov / codecov/patch

overlord/snapstate/snapstate.go#L2468-L2469

Added lines #L2468 - L2469 were not covered by tests

for _, tasks := range tasks {
toggle.WaitFor(tasks)
Expand All @@ -2464,7 +2475,12 @@
tasks = append(tasks, toggle)
}

return state.NewTaskSet(tasks...), nil
ts := state.NewTaskSet(tasks...)
if snapsupTask != nil {
ts.MarkEdge(snapsupTask, SnapSetupEdge)
}

return ts, nil
}

func splitEssentialUpdates(deviceCtx DeviceContext, updates []update) (essential, nonEssential []update) {
Expand Down Expand Up @@ -2811,7 +2827,10 @@
switchSnap := st.NewTask("switch-snap", summary)
switchSnap.Set("snap-setup", &snapsup)

return state.NewTaskSet(switchSnap), nil
ts := state.NewTaskSet(switchSnap)
ts.MarkEdge(switchSnap, SnapSetupEdge)

return ts, nil
}

// RevisionOptions control the selection of a snap revision.
Expand Down Expand Up @@ -3417,6 +3436,7 @@
ts.AddTask(linkSnap)
// prepare-snap is the last task that carries no system modifications
ts.MarkEdge(prepareSnap, LastBeforeLocalModificationsEdge)
ts.MarkEdge(prepareSnap, SnapSetupEdge)
return ts, nil
}

Expand Down Expand Up @@ -3540,6 +3560,7 @@
ts := state.NewTaskSet(prepareSnap, gadgetUpdate, gadgetCmdline)
// prepare-snap is the last task that carries no system modifications
ts.MarkEdge(prepareSnap, LastBeforeLocalModificationsEdge)
ts.MarkEdge(prepareSnap, SnapSetupEdge)
return ts, nil
}

Expand Down
5 changes: 5 additions & 0 deletions overlord/snapstate/snapstate_install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ func verifyInstallTasksWithComponents(c *C, typ snap.Type, opts, discards int, c
}
}

c.Assert(ts.MaybeEdge(snapstate.SnapSetupEdge), NotNil)

var count int
for _, t := range ts.Tasks() {
switch t.Kind() {
Expand All @@ -251,6 +253,9 @@ func verifyInstallTasksWithComponents(c *C, typ snap.Type, opts, discards int, c
c.Assert(err, IsNil)
c.Check(compsup.CompSideInfo.Component.ComponentName, Equals, components[count])
count++
case "setup-profiles":
c.Assert(t.Has("component-setup-task"), Equals, false)
c.Assert(t.Has("component-setup"), Equals, false)
}
}
c.Check(count, Equals, len(components), Commentf("expected %d components, found %d", len(components), count))
Expand Down
14 changes: 10 additions & 4 deletions overlord/snapstate/snapstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,7 @@ func (s *snapmgrTestSuite) TestSwitchTasks(c *C) {

c.Assert(s.state.TaskCount(), Equals, len(ts.Tasks()))
c.Assert(taskKinds(ts.Tasks()), DeepEquals, []string{"switch-snap"})
c.Assert(ts.MaybeEdge(snapstate.SnapSetupEdge), NotNil)
}

func (s *snapmgrTestSuite) TestSwitchConflict(c *C) {
Expand Down Expand Up @@ -10176,7 +10177,8 @@ func (s *snapmgrTestSuite) TestDownloadWithComponents(c *C) {
c.Assert(begin, NotNil)
c.Check(begin.Kind(), Equals, "download-snap")

verifySnapAndComponentSetupsForDownload(c, begin, ts, downloadDir)
const componentExclusive = false
verifySnapAndComponentSetupsForDownload(c, begin, ts, downloadDir, componentExclusive)
}

func (s *snapmgrTestSuite) TestDownloadWithComponentsWithMismatchValidationSets(c *C) {
Expand Down Expand Up @@ -10388,7 +10390,8 @@ func (s *snapmgrTestSuite) TestDownloadWithComponentsWithValidationSets(c *C) {
c.Assert(begin, NotNil)
c.Check(begin.Kind(), Equals, "download-snap")

verifySnapAndComponentSetupsForDownload(c, begin, ts, downloadDir)
const componentExclusive = false
verifySnapAndComponentSetupsForDownload(c, begin, ts, downloadDir, componentExclusive)
}

func (s *snapmgrTestSuite) TestDownloadComponents(c *C) {
Expand Down Expand Up @@ -10463,10 +10466,11 @@ func (s *snapmgrTestSuite) TestDownloadComponents(c *C) {
c.Assert(begin, NotNil)
c.Check(begin.Kind(), Equals, "download-component")

verifySnapAndComponentSetupsForDownload(c, begin, ts, downloadDir)
const componentExclusive = true
verifySnapAndComponentSetupsForDownload(c, begin, ts, downloadDir, componentExclusive)
}

func verifySnapAndComponentSetupsForDownload(c *C, begin *state.Task, ts *state.TaskSet, downloadDir string) {
func verifySnapAndComponentSetupsForDownload(c *C, begin *state.Task, ts *state.TaskSet, downloadDir string, componentExclusive bool) {
var snapsup snapstate.SnapSetup
err := begin.Get("snap-setup", &snapsup)
c.Assert(err, IsNil)
Expand All @@ -10482,6 +10486,8 @@ func verifySnapAndComponentSetupsForDownload(c *C, begin *state.Task, ts *state.
fmt.Sprintf("%s_%s.snap", snapsup.InstanceName(), snapsup.Revision()),
))

c.Assert(snapsup.ComponentExclusiveSetup, Equals, componentExclusive)

var compsupTaskIDs []string
err = begin.Get("component-setup-tasks", &compsupTaskIDs)
c.Assert(err, IsNil)
Expand Down
1 change: 1 addition & 0 deletions overlord/snapstate/snapstate_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3117,6 +3117,7 @@ func (s *snapmgrTestSuite) TestUpdateSameRevisionSwitchesChannel(c *C) {
c.Assert(err, IsNil)
c.Check(ts.Tasks(), HasLen, 1)
c.Check(ts.Tasks()[0].Kind(), Equals, "switch-snap-channel")
c.Check(ts.MaybeEdge(snapstate.SnapSetupEdge).Kind(), Equals, "switch-snap-channel")
}

func (s *snapmgrTestSuite) TestUpdateSameRevisionSwitchesChannelConflict(c *C) {
Expand Down
Loading
Loading