From 88ba0901ea23d4c620dbd200a174279b1cedf12a Mon Sep 17 00:00:00 2001 From: Andrew Phelps Date: Thu, 6 Feb 2025 16:14:13 -0500 Subject: [PATCH] o/snapstate, o/devicestate: support components in snapstate.LinkNewBaseOrKernel and snapstate.AddLinkNewBaseOrKernel --- .../devicestate/devicestate_remodel_test.go | 5 +- overlord/snapstate/snapstate.go | 120 ++++++++------ overlord/snapstate/snapstate_test.go | 146 ++++++++++++++++-- .../remodel-to-installed-kernel/task.yaml | 2 - 4 files changed, 210 insertions(+), 63 deletions(-) diff --git a/overlord/devicestate/devicestate_remodel_test.go b/overlord/devicestate/devicestate_remodel_test.go index 98446f0b515..bcd12e82a6d 100644 --- a/overlord/devicestate/devicestate_remodel_test.go +++ b/overlord/devicestate/devicestate_remodel_test.go @@ -3867,14 +3867,17 @@ func (s *deviceMgrRemodelSuite) testRemodelUC20SwitchKernelBaseGadgetSnapsInstal for _, alreadyInstalledName := range []string{"pc-kernel-new", "core24-new", "pc-new"} { snapYaml := "name: pc-kernel-new\nversion: 1\ntype: kernel\n" channel := "other/edge" + rev := snap.R(222) if alreadyInstalledName == "core24-new" { snapYaml = "name: core24-new\nversion: 1\ntype: base\n" + rev = snap.R(223) } else if alreadyInstalledName == "pc-new" { snapYaml = "name: pc-new\nversion: 1\ntype: gadget\nbase: core24-new\n" + rev = snap.R(224) } si := &snap.SideInfo{ RealName: alreadyInstalledName, - Revision: snap.R(222), + Revision: rev, SnapID: snaptest.AssertedSnapID(alreadyInstalledName), } info := snaptest.MakeSnapFileAndDir(c, snapYaml, nil, si) diff --git a/overlord/snapstate/snapstate.go b/overlord/snapstate/snapstate.go index 8d7739364ca..295b1bd703c 100644 --- a/overlord/snapstate/snapstate.go +++ b/overlord/snapstate/snapstate.go @@ -3404,10 +3404,41 @@ func LinkNewBaseOrKernel(st *state.State, name string, fromChange string) (*stat InstanceKey: snapst.InstanceKey, } + // note that prepare-snap doesn't actually do anything here, and is mostly + // used as a task to carry the snap-setup information. prepareSnap := st.NewTask("prepare-snap", fmt.Sprintf(i18n.G("Prepare snap %q (%s) for remodel"), snapsup.InstanceName(), snapst.Current)) prepareSnap.Set("snap-setup", &snapsup) - prev := prepareSnap + ts := state.NewTaskSet(prepareSnap) + ts.MarkEdge(prepareSnap, LastBeforeLocalModificationsEdge) + ts.MarkEdge(prepareSnap, SnapSetupEdge) + + if err := addLinkNewBaseOrKernelTasks(st, snapst, ts, prepareSnap); err != nil { + return nil, err + } + + return ts, nil +} + +func addLinkNewBaseOrKernelTasks(st *state.State, snapst SnapState, ts *state.TaskSet, snapsupTask *state.Task) error { + tasks := ts.Tasks() + if len(tasks) == 0 { + return errors.New("internal error: task set must be seeded with at least one task") + } + + prev := tasks[len(tasks)-1] + add := func(t *state.Task) { + t.Set("snap-setup-task", snapsupTask.ID()) + t.WaitFor(prev) + ts.AddTask(t) + prev = t + } + + info, err := snapst.CurrentInfo() + if err != nil { + return err + } + // preserve the same order as during the update if info.Type() == snap.TypeKernel { // this previously created a prepare-kernel-snap task, but this was not @@ -3417,41 +3448,35 @@ func LinkNewBaseOrKernel(st *state.State, name string, fromChange string) (*stat // installed, then it is expected that the drivers tree is present. // kernel snaps can carry boot assets - gadgetUpdate := st.NewTask("update-gadget-assets", fmt.Sprintf(i18n.G("Update assets from %s %q (%s) for remodel"), snapsup.Type, snapsup.InstanceName(), snapst.Current)) - gadgetUpdate.Set("snap-setup-task", prepareSnap.ID()) - gadgetUpdate.WaitFor(prev) - ts.AddTask(gadgetUpdate) - prev = gadgetUpdate + gadgetUpdate := st.NewTask("update-gadget-assets", fmt.Sprintf(i18n.G("Update assets from %s %q (%s) for remodel"), info.Type(), info.InstanceName(), snapst.Current)) + add(gadgetUpdate) } - linkSnap := st.NewTask("link-snap", fmt.Sprintf(i18n.G("Make snap %q (%s) available to the system during remodel"), snapsup.InstanceName(), snapst.Current)) - linkSnap.Set("snap-setup-task", prepareSnap.ID()) - linkSnap.WaitFor(prev) - ts.AddTask(linkSnap) + + linkSnap := st.NewTask("link-snap", fmt.Sprintf(i18n.G("Make snap %q (%s) available to the system during remodel"), info.InstanceName(), snapst.Current)) + add(linkSnap) ts.MarkEdge(linkSnap, MaybeRebootEdge) - for _, cs := range snapst.Sequence.ComponentsForRevision(snapst.Current) { + components := snapst.Sequence.ComponentsForRevision(snapst.Current) + compsupTasks := make([]string, 0, len(components)) + for _, cs := range components { compsup := ComponentSetup{ CompSideInfo: cs.SideInfo, CompType: cs.CompType, } - compName := compsup.CompSideInfo.Component + cref := compsup.CompSideInfo.Component compRev := compsup.CompSideInfo.Revision - prepare := st.NewTask("prepare-component", fmt.Sprintf(i18n.G("Prepare component %q (%s) for remodel"), compName, compRev)) - prepare.Set("component-setup", compsup) - ts.AddTask(prepare) + link := st.NewTask("link-component", fmt.Sprintf(i18n.G("Make component %q (%s) available to the system during remodel"), cref, compRev)) + link.Set("component-setup", compsup) + add(link) - link := st.NewTask("link-component", fmt.Sprintf(i18n.G("Make component %q (%s) available to the system during remodel"), compName, compRev)) - link.Set("component-setup-task", prepare.ID()) - link.WaitFor(prepare) - ts.AddTask(link) + compsupTasks = append(compsupTasks, link.ID()) } - // prepare-snap is the last task that carries no system modifications - ts.MarkEdge(prepareSnap, LastBeforeLocalModificationsEdge) - ts.MarkEdge(prepareSnap, SnapSetupEdge) - return ts, nil + snapsupTask.Set("component-setup-tasks", compsupTasks) + + return nil } func findSnapSetupTask(tasks []*state.Task) (*state.Task, *SnapSetup, error) { @@ -3470,40 +3495,37 @@ func findSnapSetupTask(tasks []*state.Task) (*state.Task, *SnapSetup, error) { // AddLinkNewBaseOrKernel creates the same tasks as LinkNewBaseOrKernel but adds // them to the provided task set. func AddLinkNewBaseOrKernel(st *state.State, ts *state.TaskSet) (*state.TaskSet, error) { + if ts.MaybeEdge(LastBeforeLocalModificationsEdge) != nil { + return nil, errors.New("internal error: cannot add tasks to link new base or kernel to task set that introduces local modifications") + } + allTasks := ts.Tasks() snapSetupTask, snapsup, err := findSnapSetupTask(allTasks) if err != nil { return nil, err } if snapSetupTask == nil { - return nil, fmt.Errorf("internal error: cannot identify task with snap-setup") + return nil, errors.New("internal error: cannot identify task with snap-setup") } - // the first task added here waits for the last task in the existing set - prev := allTasks[len(allTasks)-1] - // preserve the same order as during the update - if snapsup.Type == snap.TypeKernel { - // kernel snaps can carry boot assets - gadgetUpdate := st.NewTask("update-gadget-assets", fmt.Sprintf(i18n.G("Update assets from %s %q (%s) for remodel"), snapsup.Type, snapsup.InstanceName(), snapsup.Revision())) - gadgetUpdate.Set("snap-setup-task", snapSetupTask.ID()) - // wait for the last task in existing set - gadgetUpdate.WaitFor(prev) - ts.AddTask(gadgetUpdate) - prev = gadgetUpdate - } - linkSnap := st.NewTask("link-snap", - fmt.Sprintf(i18n.G("Make snap %q (%s) available to the system during remodel"), snapsup.InstanceName(), snapsup.SideInfo.Revision)) - linkSnap.Set("snap-setup-task", snapSetupTask.ID()) - linkSnap.WaitFor(prev) - ts.AddTask(linkSnap) - ts.MarkEdge(linkSnap, MaybeRebootEdge) - // make sure that remodel can identify which tasks introduce actual - // changes to the system and order them correctly - if edgeTask := ts.MaybeEdge(LastBeforeLocalModificationsEdge); edgeTask == nil { - // no task in the task set is marked as last before system - // modifications are introduced, so we need to mark the last - // task in the set, as tasks introduced here modify system state - ts.MarkEdge(allTasks[len(allTasks)-1], LastBeforeLocalModificationsEdge) + + var snapst SnapState + if err := Get(st, snapsup.InstanceName(), &snapst); err != nil { + return nil, err } + + if snapst.Current != snapsup.Revision() { + return nil, errors.New("internal error: cannot add tasks to link new base or kernel to task set that changes the snap revision") + } + + // no task in the task set is marked as last before system modifications are + // introduced, so we need to mark the last task in the original set, as + // tasks introduced here modify system state + ts.MarkEdge(allTasks[len(allTasks)-1], LastBeforeLocalModificationsEdge) + + if err := addLinkNewBaseOrKernelTasks(st, snapst, ts, snapSetupTask); err != nil { + return nil, err + } + return ts, nil } diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index f3f806ff129..072d709b61e 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/overlord/snapstate/snapstate_test.go @@ -8056,7 +8056,7 @@ version: 1`), &snap.SideInfo{Revision: snap.R("5")}) c.Check(ignoreValidation, DeepEquals, map[string]bool{"bar": true}) } -func (s *snapmgrTestSuite) addSnapsForRemodel(c *C) { +func (s *snapmgrTestSuite) addSnapsForRemodel(c *C, withComponents bool) { si := &snap.SideInfo{ RealName: "some-base", Revision: snap.R(1), } @@ -8072,12 +8072,25 @@ func (s *snapmgrTestSuite) addSnapsForRemodel(c *C) { RealName: "some-kernel", Revision: snap.R(2), } snaptest.MockSnapCurrent(c, "name: some-kernel\nversion: 1.0\ntype: kernel\n", si) + seq := snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{si}) + if withComponents { + seq.AddComponentForRevision(si.Revision, sequence.NewComponentState(&snap.ComponentSideInfo{ + Component: naming.NewComponentRef("some-kernel", "comp-1"), + Revision: snap.R(22), + }, snap.KernelModulesComponent)) + + seq.AddComponentForRevision(si.Revision, sequence.NewComponentState(&snap.ComponentSideInfo{ + Component: naming.NewComponentRef("some-kernel", "comp-2"), + Revision: snap.R(33), + }, snap.KernelModulesComponent)) + } snapstate.Set(s.state, "some-kernel", &snapstate.SnapState{ Active: true, - Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{si}), + Sequence: seq, Current: si.Revision, SnapType: "kernel", }) + si = &snap.SideInfo{ RealName: "some-gadget", Revision: snap.R(3), } @@ -8103,6 +8116,7 @@ var nonReLinkKinds = []string{ "run-hook[configure]", "run-hook[check-health]", "discard-old-kernel-snap-setup", + "mount-component", } func kindsToSet(kinds []string) map[string]bool { @@ -8121,6 +8135,10 @@ func (s *snapmgrTestSuite) TestRemodelLinkNewBaseOrUC20KernelHappy(c *C) { s.testRemodelLinkNewBaseOrKernelHappy(c, MakeModel20("brand-gadget", nil), 0) } +func (s *snapmgrTestSuite) TestRemodelLinkNewBaseOrUC20KernelHappyWithKmodComponent(c *C) { + s.testRemodelLinkNewBaseOrKernelHappy(c, MakeModel20("brand-gadget", nil), 0) +} + func (s *snapmgrTestSuite) TestRemodelLinkNewBaseOrUC24KernelHappy(c *C) { // UC24 model has additional tasks for the kernel s.testRemodelLinkNewBaseOrKernelHappy(c, @@ -8138,16 +8156,20 @@ func (s *snapmgrTestSuite) testRemodelLinkNewBaseOrKernelHappy(c *C, model *asse defer snapstatetest.MockDeviceModel(model)() - s.addSnapsForRemodel(c) + const withComponents = false + s.addSnapsForRemodel(c, withComponents) ts, err := snapstate.LinkNewBaseOrKernel(s.state, "some-kernel", "") c.Assert(err, IsNil) + tasks := ts.Tasks() c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeKernel, opts, 0, 0, []string{"prepare-snap"}, nil, kindsToSet(nonReLinkKinds))) - tPrepare := tasks[0] c.Assert(tasks, HasLen, 3) + + tPrepare := tasks[0] tUpdateGadgetAssets := tasks[1] tLink := tasks[2] + c.Assert(tPrepare.Kind(), Equals, "prepare-snap") c.Assert(tPrepare.Summary(), Equals, `Prepare snap "some-kernel" (2) for remodel`) c.Assert(tPrepare.Has("snap-setup"), Equals, true) @@ -8173,6 +8195,92 @@ func (s *snapmgrTestSuite) testRemodelLinkNewBaseOrKernelHappy(c *C, model *asse c.Assert(ts.MaybeEdge(snapstate.MaybeRebootEdge), Equals, tLink) } +func (s *snapmgrTestSuite) TestRemodelLinkNewBaseOrKernelWithComponent(c *C) { + model := MakeModel20("brand-gadget", nil) + + restore := release.MockOnClassic(false) + defer restore() + + s.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) + s.state.Lock() + defer s.state.Unlock() + + defer snapstatetest.MockDeviceModel(model)() + + const withComponents = true + s.addSnapsForRemodel(c, withComponents) + + ts, err := snapstate.LinkNewBaseOrKernel(s.state, "some-kernel", "") + c.Assert(err, IsNil) + + comps := []string{"comp-1", "comp-2"} + startTasks := []string{"prepare-snap"} + + tasks := ts.Tasks() + c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeKernel, 0, 0, 0, startTasks, comps, kindsToSet(nonReLinkKinds))) + c.Assert(tasks, HasLen, 5) + + prepare := ts.MaybeEdge(snapstate.SnapSetupEdge) + c.Assert(prepare, NotNil) + c.Assert(prepare.Kind(), Equals, "prepare-snap") + c.Assert(prepare.Has("snap-setup"), Equals, true) + + link := ts.MaybeEdge(snapstate.MaybeRebootEdge) + c.Assert(link, NotNil) + c.Assert(link.Kind(), Equals, "link-snap") + + for _, t := range ts.Tasks() { + if t.Kind() == "link-component" { + c.Assert(t.Has("component-setup"), Equals, true) + } + } +} + +func (s *snapmgrTestSuite) TestRemodelAddLinkNewBaseOrKernelWithComponent(c *C) { + model := MakeModel20("brand-gadget", nil) + + restore := release.MockOnClassic(false) + defer restore() + + s.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) + s.state.Lock() + defer s.state.Unlock() + + defer snapstatetest.MockDeviceModel(model)() + + const withComponents = true + s.addSnapsForRemodel(c, withComponents) + + switchSnap := s.state.NewTask("switch-snap", "switch snap") + switchSnap.Set("snap-setup", &snapstate.SnapSetup{ + SideInfo: &snap.SideInfo{RealName: "some-kernel", Revision: snap.R(2)}, + Type: "kernel", + Channel: "new-channel", + }) + + ts := state.NewTaskSet(switchSnap) + + ts, err := snapstate.AddLinkNewBaseOrKernel(s.state, ts) + c.Assert(err, IsNil) + + comps := []string{"comp-1", "comp-2"} + startTasks := []string{"switch-snap"} + + tasks := ts.Tasks() + c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeKernel, 0, 0, 0, startTasks, comps, kindsToSet(nonReLinkKinds))) + c.Assert(tasks, HasLen, 5) + + link := ts.MaybeEdge(snapstate.MaybeRebootEdge) + c.Assert(link, NotNil) + c.Assert(link.Kind(), Equals, "link-snap") + + for _, t := range ts.Tasks() { + if t.Kind() == "link-component" { + c.Assert(t.Has("component-setup"), Equals, true) + } + } +} + func (s *snapmgrTestSuite) TestRemodelLinkNewBaseOrKernelBadType(c *C) { restore := release.MockOnClassic(false) defer restore() @@ -8180,7 +8288,9 @@ func (s *snapmgrTestSuite) TestRemodelLinkNewBaseOrKernelBadType(c *C) { s.BaseTest.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) s.state.Lock() defer s.state.Unlock() - s.addSnapsForRemodel(c) + + const withComponents = false + s.addSnapsForRemodel(c, withComponents) si := &snap.SideInfo{RealName: "some-snap", Revision: snap.R(3)} snaptest.MockSnapCurrent(c, "name: snap-gadget\nversion: 1.0\n", si) @@ -8206,7 +8316,9 @@ func (s *snapmgrTestSuite) TestRemodelLinkNewBaseOrKernelNoRemodelConflict(c *C) s.BaseTest.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) s.state.Lock() defer s.state.Unlock() - s.addSnapsForRemodel(c) + + const withComponents = false + s.addSnapsForRemodel(c, withComponents) tugc := s.state.NewTask("update-managed-boot-config", "update managed boot config") chg := s.state.NewChange("remodel", "remodel") @@ -8241,6 +8353,8 @@ func (s *snapmgrTestSuite) testRemodelAddLinkNewBaseOrKernel(c *C, model *assert defer snapstatetest.MockDeviceModel(model)() + s.addSnapsForRemodel(c, false) + // try a kernel snap first si := &snap.SideInfo{RealName: "some-kernel", Revision: snap.R(2)} tPrepare := s.state.NewTask("prepare-snap", "test task") @@ -8313,7 +8427,9 @@ func (s *snapmgrTestSuite) TestRemodelSwitchNewGadget(c *C) { s.BaseTest.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) s.state.Lock() defer s.state.Unlock() - s.addSnapsForRemodel(c) + + const withComponents = false + s.addSnapsForRemodel(c, withComponents) ts, err := snapstate.SwitchToNewGadget(s.state, "some-gadget", "") c.Assert(err, IsNil) @@ -8343,7 +8459,9 @@ func (s *snapmgrTestSuite) TestRemodelSwitchNewGadgetNoRemodelConflict(c *C) { s.BaseTest.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) s.state.Lock() defer s.state.Unlock() - s.addSnapsForRemodel(c) + + const withComponents = false + s.addSnapsForRemodel(c, withComponents) tugc := s.state.NewTask("update-managed-boot-config", "update managed boot config") chg := s.state.NewChange("remodel", "remodel") @@ -8360,7 +8478,9 @@ func (s *snapmgrTestSuite) TestRemodelSwitchNewGadgetBadType(c *C) { s.BaseTest.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) s.state.Lock() defer s.state.Unlock() - s.addSnapsForRemodel(c) + + const withComponent = false + s.addSnapsForRemodel(c, withComponent) si := &snap.SideInfo{RealName: "some-snap", Revision: snap.R(3)} snaptest.MockSnapCurrent(c, "name: snap-gadget\nversion: 1.0\n", si) @@ -8388,7 +8508,9 @@ func (s *snapmgrTestSuite) TestRemodelSwitchNewGadgetConflict(c *C) { s.BaseTest.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) s.state.Lock() defer s.state.Unlock() - s.addSnapsForRemodel(c) + + const withComponents = false + s.addSnapsForRemodel(c, withComponents) tugc := s.state.NewTask("update-gadget-cmdline", "update gadget cmdline") chg := s.state.NewChange("optional-kernel-cmdline", "optional kernel cmdline") @@ -8414,7 +8536,9 @@ func (s *snapmgrTestSuite) TestRemodelSwitchNewGadgetConflictExclusiveKind(c *C) s.BaseTest.AddCleanup(snapstate.MockSnapReadInfo(snap.ReadInfo)) s.state.Lock() defer s.state.Unlock() - s.addSnapsForRemodel(c) + + const withComponent = false + s.addSnapsForRemodel(c, withComponent) tugc := s.state.NewTask("some-random-task", "...") chg := s.state.NewChange("transition-ubuntu-core", "...") diff --git a/tests/nested/manual/remodel-to-installed-kernel/task.yaml b/tests/nested/manual/remodel-to-installed-kernel/task.yaml index 778ffb32e9c..a7689b90105 100644 --- a/tests/nested/manual/remodel-to-installed-kernel/task.yaml +++ b/tests/nested/manual/remodel-to-installed-kernel/task.yaml @@ -187,5 +187,3 @@ execute: | remote.exec sudo modprobe mac80211_hwsim remote.exec ip link show wlan0 remote.exec modinfo --filename mac80211_hwsim | MATCH '/lib/modules/.*/updates/wifi-comp' - - false