From 411ac6c5486c34408e9692aafa87348672567009 Mon Sep 17 00:00:00 2001 From: Andrew Phelps <136256549+andrewphelpsj@users.noreply.github.com> Date: Wed, 26 Jun 2024 13:30:49 -0400 Subject: [PATCH] o/snapstate: install components from the store (#14092) * o/snapstate: replace some args to doInstallComponent with a flags type * o/snapstate: create validate-component task when installing component from the store * o/snapstate: add arg to doInstall for components * o/snapstate: install components from store in StoreTarget * o/snapstate: add component fields to fakeSnappyBackend ops * o/snapstate: add helpers for checking tasks for installing components * o/snapstate: add testing channel that adds a component to snap that is being installed * o/snapstate: use instance name in path to that is set for remove ops in test backend * o/snapstate: extend doInstall to handle components * o/snapstate: refactor addTask in doInstall to update prev pointer to eliminate manual tracking of last task * o/snapstate: test tasks created when installing component from the store * o/snapstate: remove component files when snap file is also ephemeral * o/snapstate: change component types in error strings to be more clear * o/snapstate: always install components for each snap in a consistent order * o/snapstate: replace pointer params to doInstall with copies * o/snapstate: unexport componentTarget * o/snapstate: add TODOs about handling components * o/snapstate: add comment explaining why we do not create setup-profiles task for new components when installed with snap * o/snapstate: remove usage of new go stdlib function * o/snapstate: fully initialize ComponentSetup in store's InstallGoal implementation * o/snapstate: rename componentInstallFlags.SkipSecurity to SkipProfiles * o/snapstate: move componentTarget struct * o/snapstate: eliminate componentTarget type * store, o/snapstate: swap out resources slice in SnapAction for flag in RefreshOptions * o/snapstate: remove this change for now, it should not be a part of this PR * o/snapstate: add TODO about ordering component and snap hook tasks * o/snapstate: rename variable for readability --- overlord/snapstate/backend_test.go | 43 +- overlord/snapstate/component.go | 49 +- overlord/snapstate/component_install_test.go | 9 +- overlord/snapstate/snapstate.go | 107 ++-- overlord/snapstate/snapstate_install_test.go | 515 ++++++++++++++++++- overlord/snapstate/snapstate_remove_test.go | 8 +- overlord/snapstate/snapstate_test.go | 31 +- overlord/snapstate/snapstate_update_test.go | 14 +- overlord/snapstate/target.go | 106 +++- overlord/snapstate/target_test.go | 177 +++++++ store/store_action.go | 18 +- store/store_action_test.go | 3 +- 12 files changed, 950 insertions(+), 130 deletions(-) create mode 100644 overlord/snapstate/target_test.go diff --git a/overlord/snapstate/backend_test.go b/overlord/snapstate/backend_test.go index a597b276aeb..e8e5fc3836a 100644 --- a/overlord/snapstate/backend_test.go +++ b/overlord/snapstate/backend_test.go @@ -60,6 +60,11 @@ type fakeOp struct { sinfo snap.SideInfo stype snap.Type + componentName string + componentPath string + componentRev snap.Revision + componentSideInfo snap.ComponentSideInfo + curSnaps []store.CurrentSnap action store.SnapAction @@ -328,6 +333,17 @@ func (f *fakeStore) snap(spec snapSpec) (*snap.Info, error) { DaemonScope: "user", }, } + case "channel-for-components": + info.Components = map[string]*snap.Component{ + "test-component": { + Type: snap.TestComponent, + Name: "test-component", + }, + "kernel-modules-component": { + Type: snap.KernelModulesComponent, + Name: "kernel-modules-component", + }, + } case "channel-for-dbus-activation": slot := &snap.SlotInfo{ Snap: info, @@ -658,10 +674,14 @@ func (f *fakeStore) SnapAction(ctx context.Context, currentSnaps []*store.Curren info.Channel = "" } info.InstanceKey = instanceKey + sar := store.SnapActionResult{ - Info: info, - Resources: f.snapResources(info), + Info: info, + } + if opts.IncludeResources { + sar.Resources = f.snapResources(info) } + if strings.HasSuffix(snapName, "-with-default-track") && strutil.ListContains([]string{"stable", "candidate", "beta", "edge"}, a.Channel) { sar.RedirectChannel = "2.0/" + a.Channel } @@ -1371,7 +1391,7 @@ func (f *fakeSnappyBackend) RemoveSnapDataDir(info *snap.Info, otherInstances bo f.appendOp(&fakeOp{ op: "remove-snap-data-dir", name: info.InstanceName(), - path: snap.BaseDataDir(info.SnapName()), + path: snap.BaseDataDir(info.InstanceName()), otherInstances: otherInstances, }) return f.maybeErrForLastOp() @@ -1389,7 +1409,7 @@ func (f *fakeSnappyBackend) RemoveSnapDir(s snap.PlaceInfo, otherInstances bool) f.appendOp(&fakeOp{ op: "remove-snap-dir", name: s.InstanceName(), - path: snap.BaseDir(s.SnapName()), + path: snap.BaseDir(s.InstanceName()), otherInstances: otherInstances, }) return f.maybeErrForLastOp() @@ -1440,12 +1460,21 @@ func (f *fakeSnappyBackend) CurrentInfo(curInfo *snap.Info) { }) } -func (f *fakeSnappyBackend) ForeignTask(kind string, status state.Status, snapsup *snapstate.SnapSetup) error { - f.appendOp(&fakeOp{ +func (f *fakeSnappyBackend) ForeignTask(kind string, status state.Status, snapsup *snapstate.SnapSetup, compsup *snapstate.ComponentSetup) error { + op := &fakeOp{ op: kind + ":" + status.String(), name: snapsup.InstanceName(), revno: snapsup.Revision(), - }) + } + + if compsup != nil { + op.componentName = compsup.ComponentName() + op.componentPath = compsup.CompPath + op.componentRev = compsup.Revision() + op.componentSideInfo = *compsup.CompSideInfo + } + + f.appendOp(op) return f.maybeErrForLastOp() } diff --git a/overlord/snapstate/component.go b/overlord/snapstate/component.go index d27d71b0136..6a3314f0c6d 100644 --- a/overlord/snapstate/component.go +++ b/overlord/snapstate/component.go @@ -71,15 +71,21 @@ func InstallComponentPath(st *state.State, csi *snap.ComponentSideInfo, info *sn CompType: compInfo.Type, CompPath: path, } - // The file passed around is temporary, make sure it gets removed. - // TODO probably this should be part of a flags type in the future. - removeComponentPath := true - return doInstallComponent(st, &snapst, compSetup, snapsup, removeComponentPath, "") + + return doInstallComponent(st, &snapst, compSetup, snapsup, componentInstallFlags{ + // The file passed around is temporary, make sure it gets removed. + RemoveComponentPath: true, + }, "") +} + +type componentInstallFlags struct { + RemoveComponentPath bool + SkipProfiles bool } // doInstallComponent might be called with the owner snap installed or not. func doInstallComponent(st *state.State, snapst *SnapState, compSetup *ComponentSetup, - snapsup *SnapSetup, removeComponentPath bool, fromChange string) (*state.TaskSet, error) { + snapsup *SnapSetup, flags componentInstallFlags, fromChange string) (*state.TaskSet, error) { // TODO check for experimental flag that will hide temporarily components @@ -100,23 +106,23 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup *Component // Check if we already have the revision in the snaps folder (alters tasks). // Note that this will search for all snap revisions in the system. - revisionIsPresent := snapst.IsComponentRevPresent(compSi) == true + revisionIsPresent := snapst.IsComponentRevPresent(compSi) revisionStr := fmt.Sprintf(" (%s)", compSi.Revision) - var prepare, prev *state.Task + fromStore := compSetup.CompPath == "" && !revisionIsPresent + + var prepare *state.Task // if we have a local revision here we go back to that - if compSetup.CompPath != "" || revisionIsPresent { - prepare = st.NewTask("prepare-component", - fmt.Sprintf(i18n.G("Prepare component %q%s"), - compSetup.CompPath, revisionStr)) + if fromStore { + prepare = st.NewTask("download-component", fmt.Sprintf(i18n.G("Download component %q%s"), compSetup.ComponentName(), revisionStr)) } else { - prepare = st.NewTask("download-component", fmt.Sprintf(i18n.G("Download component %q%s"), compSetup, revisionStr)) + prepare = st.NewTask("prepare-component", fmt.Sprintf(i18n.G("Prepare component %q%s"), compSetup.CompPath, revisionStr)) } prepare.Set("component-setup", compSetup) prepare.Set("snap-setup", snapsup) tasks := []*state.Task{prepare} - prev = prepare + prev := prepare addTask := func(t *state.Task) { t.Set("component-setup-task", prepare.ID()) @@ -126,8 +132,12 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup *Component prev = t } - // TODO task to fetch and check assertions for component if from store - // (equivalent to "validate-snap") + if fromStore { + validate := st.NewTask("validate-component", fmt.Sprintf( + i18n.G("Fetch and check assertions for component %q%s"), compSetup.ComponentName(), revisionStr), + ) + addTask(validate) + } // Task that copies the file and creates mount units if !revisionIsPresent { @@ -136,7 +146,7 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup *Component compSi.Component, revisionStr)) addTask(mount) } else { - if removeComponentPath { + if flags.RemoveComponentPath { // If the revision is local, we will not need the // temporary snap. This can happen when e.g. // side-loading a local revision again. The path is @@ -167,9 +177,10 @@ func doInstallComponent(st *state.State, snapst *SnapState, compSetup *Component } // security - setupSecurity := st.NewTask("setup-profiles", fmt.Sprintf(i18n.G("Setup component %q%s security profiles"), compSi.Component, revisionStr)) - addTask(setupSecurity) - prev = setupSecurity + if !flags.SkipProfiles { + setupSecurity := st.NewTask("setup-profiles", fmt.Sprintf(i18n.G("Setup component %q%s security profiles"), compSi.Component, revisionStr)) + addTask(setupSecurity) + } // finalize (sets SnapState) linkSnap := st.NewTask("link-component", diff --git a/overlord/snapstate/component_install_test.go b/overlord/snapstate/component_install_test.go index 92d792701e0..10c99e4bc47 100644 --- a/overlord/snapstate/component_install_test.go +++ b/overlord/snapstate/component_install_test.go @@ -45,6 +45,8 @@ const ( compTypeIsKernMods // Current component is discarded at the end compCurrentIsDiscarded + // Component is being installed with a snap, so skip setup-profiles + compOptSkipSecurity ) // opts is a bitset with compOpt* as possible values. @@ -54,8 +56,9 @@ func expectedComponentInstallTasks(opts int) []string { if opts&compOptIsLocal != 0 { startTasks = []string{"prepare-component"} } else { - startTasks = []string{"download-component"} + startTasks = []string{"download-component", "validate-component"} } + // Revision is not the same as the current one installed if opts&compOptRevisionPresent == 0 { startTasks = append(startTasks, "mount-component") @@ -68,7 +71,9 @@ func expectedComponentInstallTasks(opts int) []string { startTasks = append(startTasks, "unlink-current-component") } - startTasks = append(startTasks, "setup-profiles") + if opts&compOptSkipSecurity == 0 { + startTasks = append(startTasks, "setup-profiles") + } // link-component is always present startTasks = append(startTasks, "link-component") diff --git a/overlord/snapstate/snapstate.go b/overlord/snapstate/snapstate.go index 8b63e6dc832..f8fa4bd16e5 100644 --- a/overlord/snapstate/snapstate.go +++ b/overlord/snapstate/snapstate.go @@ -371,7 +371,33 @@ func isCoreSnap(snapName string) bool { return snapName == defaultCoreSnapName } -func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int, fromChange string, inUseCheck func(snap.Type) (boot.InUseFunc, error)) (*state.TaskSet, error) { +func componentTasksForInstallWithSnap(ts *state.TaskSet) (beforeLink []*state.Task, linkAndAfter []*state.Task, err error) { + link, err := ts.Edge(MaybeRebootEdge) + if err != nil { + return nil, nil, err + } + + isBeforeLink := true + + // this depends on the order of the tasks in the set, but if we want to + // maintain a logical ordering of the snaps in the task set returned by + // doInstall, then requiring this order simplifies the code a lot + for _, t := range ts.Tasks() { + if t.ID() == link.ID() { + isBeforeLink = false + } + + if isBeforeLink { + beforeLink = append(beforeLink, t) + } else { + linkAndAfter = append(linkAndAfter, t) + } + } + + return beforeLink, linkAndAfter, nil +} + +func doInstall(st *state.State, snapst *SnapState, snapsup SnapSetup, compsups []ComponentSetup, flags int, fromChange string, inUseCheck func(snap.Type) (boot.InUseFunc, error)) (*state.TaskSet, error) { tr := config.NewTransaction(st) experimentalRefreshAppAwareness, err := features.Flag(tr, features.RefreshAppAwareness) if err != nil && !config.IsNoOption(err) { @@ -408,7 +434,7 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int } } - if err := isParallelInstallable(snapsup); err != nil { + if err := isParallelInstallable(&snapsup); err != nil { return nil, err } @@ -445,7 +471,7 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int // Note that because we are modifying the snap state inside // softCheckNothingRunningForRefresh, this block must be located // after the conflict check done above. - if err := softCheckNothingRunningForRefresh(st, snapst, snapsup, info); err != nil { + if err := softCheckNothingRunningForRefresh(st, snapst, &snapsup, info); err != nil { // snap is running; schedule its downloading before notifying to close var busyErr *timedBusySnapError if errors.As(err, &busyErr) && snapsup.IsAutoRefresh { @@ -508,6 +534,7 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int t.Set("snap-setup-task", prepare.ID()) t.WaitFor(prev) tasks = append(tasks, t) + prev = t } addTasksFromTaskSet := func(ts *state.TaskSet) { ts.WaitFor(prev) @@ -520,14 +547,12 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int // 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) - prev = checkAsserts } // mount if !revisionIsLocal { mount := st.NewTask("mount-snap", fmt.Sprintf(i18n.G("Mount snap %q%s"), snapsup.InstanceName(), revisionStr)) addTask(mount) - prev = mount } else { if snapsup.Flags.RemoveSnapPath { // If the revision is local, we will not need the @@ -546,7 +571,6 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int if runRefreshHooks { preRefreshHook := SetupPreRefreshHook(st, snapsup.InstanceName()) addTask(preRefreshHook) - prev = preRefreshHook } if snapst.IsInstalled() { @@ -554,17 +578,14 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int stop := st.NewTask("stop-snap-services", fmt.Sprintf(i18n.G("Stop snap %q services"), snapsup.InstanceName())) stop.Set("stop-reason", snap.StopReasonRefresh) addTask(stop) - prev = stop removeAliases := st.NewTask("remove-aliases", fmt.Sprintf(i18n.G("Remove aliases for snap %q"), snapsup.InstanceName())) removeAliases.Set("remove-reason", removeAliasesReasonRefresh) addTask(removeAliases) - prev = removeAliases unlink := st.NewTask("unlink-current-snap", fmt.Sprintf(i18n.G("Make current revision for snap %q unavailable"), snapsup.InstanceName())) unlink.Set("unlink-reason", unlinkReasonRefresh) addTask(unlink) - prev = unlink } // we need to know some of the characteristics of the device - it is @@ -581,14 +602,12 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int if snapsup.Type == snap.TypeKernel && NeedsKernelSetup(deviceCtx.Model()) { setupKernel := st.NewTask("prepare-kernel-snap", fmt.Sprintf(i18n.G("Prepare kernel driver tree for %q%s"), snapsup.InstanceName(), revisionStr)) addTask(setupKernel) - prev = setupKernel } if deviceCtx.IsCoreBoot() && (snapsup.Type == snap.TypeGadget || (snapsup.Type == snap.TypeKernel && !TestingLeaveOutKernelUpdateGadgetAssets)) { // gadget update currently for core boot systems only gadgetUpdate := st.NewTask("update-gadget-assets", fmt.Sprintf(i18n.G("Update assets from %s %q%s"), snapsup.Type, snapsup.InstanceName(), revisionStr)) addTask(gadgetUpdate) - prev = gadgetUpdate } // kernel command line from gadget is for core boot systems only if deviceCtx.IsCoreBoot() && snapsup.Type == snap.TypeGadget { @@ -598,20 +617,47 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int } gadgetCmdline := st.NewTask("update-gadget-cmdline", fmt.Sprintf(i18n.G("Update kernel command line from gadget %q%s"), snapsup.InstanceName(), revisionStr)) addTask(gadgetCmdline) - prev = gadgetCmdline } // copy-data (needs stopped services by unlink) if !snapsup.Flags.Revert { copyData := st.NewTask("copy-snap-data", fmt.Sprintf(i18n.G("Copy snap %q data"), snapsup.InstanceName())) addTask(copyData) - prev = copyData + } + + tasksAfterLinkSnap := make([]*state.Task, 0, len(compsups)) + for _, compsup := range compsups { + compTaskSet, err := doInstallComponent(st, snapst, &compsup, &snapsup, componentInstallFlags{ + // if we are removing the snap, we can assume that we should remove + // the component too + RemoveComponentPath: snapsup.RemoveSnapPath, + // the setup-profiles task that is created for the snap itself + // generates the security profiles for the snap and its new components + SkipProfiles: true, + }, "") + if err != nil { + return nil, fmt.Errorf("cannot install component %q: %v", compsup.CompSideInfo.Component, err) + } + + beforeLink, afterLink, err := componentTasksForInstallWithSnap(compTaskSet) + if err != nil { + return nil, err + } + + // TODO: once component hooks are fully, we will need to take care to + // correctly order the tasks that are created for running component + // and snap hooks + + for _, t := range beforeLink { + addTask(t) + } + + tasksAfterLinkSnap = append(tasksAfterLinkSnap, afterLink...) } // security setupSecurity := st.NewTask("setup-profiles", fmt.Sprintf(i18n.G("Setup snap %q%s security profiles"), snapsup.InstanceName(), revisionStr)) addTask(setupSecurity) - prev = setupSecurity // finalize (wrappers+current symlink) // @@ -626,7 +672,10 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int // additional tasks. linkSnap := st.NewTask("link-snap", fmt.Sprintf(i18n.G("Make snap %q%s available to the system"), snapsup.InstanceName(), revisionStr)) addTask(linkSnap) - prev = linkSnap + + for _, t := range tasksAfterLinkSnap { + addTask(t) + } // auto-connections // @@ -640,29 +689,24 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int // performs some reboot-verification code. autoConnect := st.NewTask("auto-connect", fmt.Sprintf(i18n.G("Automatically connect eligible plugs and slots of snap %q"), snapsup.InstanceName())) addTask(autoConnect) - prev = autoConnect if snapsup.Type == snap.TypeKernel && NeedsKernelSetup(deviceCtx.Model()) { // This task needs to run after we're back and running the new // kernel after a reboot was requested in link-snap handler. setupKernel := st.NewTask("discard-old-kernel-snap-setup", fmt.Sprintf(i18n.G("Discard previous kernel driver tree for %q%s"), snapsup.InstanceName(), revisionStr)) addTask(setupKernel) - prev = setupKernel } // setup aliases setAutoAliases := st.NewTask("set-auto-aliases", fmt.Sprintf(i18n.G("Set automatic aliases for snap %q"), snapsup.InstanceName())) addTask(setAutoAliases) - prev = setAutoAliases setupAliases := st.NewTask("setup-aliases", fmt.Sprintf(i18n.G("Setup snap %q aliases"), snapsup.InstanceName())) addTask(setupAliases) - prev = setupAliases if snapsup.Flags.Prefer { prefer := st.NewTask("prefer-aliases", fmt.Sprintf(i18n.G("Prefer aliases for snap %q"), snapsup.InstanceName())) addTask(prefer) - prev = prefer } if deviceCtx.IsCoreBoot() && snapsup.Type == snap.TypeSnapd { @@ -674,13 +718,11 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int // so that the task is executed by the new snapd bootConfigUpdate := st.NewTask("update-managed-boot-config", fmt.Sprintf(i18n.G("Update managed boot config assets from %q%s"), snapsup.InstanceName(), revisionStr)) addTask(bootConfigUpdate) - prev = bootConfigUpdate } if runRefreshHooks { postRefreshHook := SetupPostRefreshHook(st, snapsup.InstanceName()) addTask(postRefreshHook) - prev = postRefreshHook } var installHook *state.Task @@ -688,7 +730,6 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int if !snapst.IsInstalled() { installHook = SetupInstallHook(st, snapsup.InstanceName()) addTask(installHook) - prev = installHook } if snapsup.QuotaGroupName != "" { @@ -697,12 +738,11 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int return nil, err } addTask(quotaAddSnapTask) - prev = quotaAddSnapTask } // only run default-configure hook if installing the snap for the first time and // default-configure is allowed - if !snapst.IsInstalled() && isDefaultConfigureAllowed(snapsup) { + if !snapst.IsInstalled() && isDefaultConfigureAllowed(&snapsup) { defaultConfigureSet := DefaultConfigure(st, snapsup.InstanceName()) addTasksFromTaskSet(defaultConfigureSet) } @@ -710,7 +750,6 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int // run new services startSnapServices := st.NewTask("start-snap-services", fmt.Sprintf(i18n.G("Start snap %q%s services"), snapsup.InstanceName(), revisionStr)) addTask(startSnapServices) - prev = startSnapServices // Do not do that if we are reverting to a local revision var cleanupTask *state.Task @@ -805,8 +844,8 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int return installSet, nil } - if isConfigureAllowed(snapsup) { - confFlags := configureSnapFlags(snapst, snapsup) + if isConfigureAllowed(&snapsup) { + confFlags := configureSnapFlags(snapst, &snapsup) configSet := ConfigureSnap(st, snapsup.InstanceName(), confFlags) configSet.WaitAll(installSet) installSet.AddAll(configSet) @@ -2089,9 +2128,11 @@ func doUpdate(ctx context.Context, st *state.State, names []string, updates []mi return nil, nil, err } + // TODO: we need to handle components here too + // Do not set any default restart boundaries, we do it when we have access to all // the task-sets in preparation for single-reboot. - ts, err := doInstall(st, snapst, snapsup, noRestartBoundaries, fromChange, inUseFor(deviceCtx)) + ts, err := doInstall(st, snapst, *snapsup, nil, noRestartBoundaries, fromChange, inUseFor(deviceCtx)) if err != nil { if errors.Is(err, &timedBusySnapError{}) && ts != nil { // snap is busy and pre-download tasks were made for it @@ -3905,7 +3946,9 @@ func RevertToRevision(st *state.State, name string, rev snap.Revision, flags Fla PlugsOnly: len(info.Slots) == 0, InstanceKey: snapst.InstanceKey, } - return doInstall(st, &snapst, snapsup, 0, fromChange, nil) + + // TODO: we need to handle components here too + return doInstall(st, &snapst, *snapsup, nil, 0, fromChange, nil) } // TransitionCore transitions from an old core snap name to a new core @@ -3941,13 +3984,13 @@ func TransitionCore(st *state.State, oldName, newName string) ([]*state.TaskSet, } // start by installing the new snap - tsInst, err := doInstall(st, &newSnapst, &SnapSetup{ + tsInst, err := doInstall(st, &newSnapst, SnapSetup{ Channel: oldSnapst.TrackingChannel, DownloadInfo: &newInfo.DownloadInfo, SideInfo: &newInfo.SideInfo, Type: newInfo.Type(), Version: newInfo.Version, - }, 0, "", nil) + }, nil, 0, "", nil) if err != nil { return nil, err } diff --git a/overlord/snapstate/snapstate_install_test.go b/overlord/snapstate/snapstate_install_test.go index 7c46acb25d1..891cca8b805 100644 --- a/overlord/snapstate/snapstate_install_test.go +++ b/overlord/snapstate/snapstate_install_test.go @@ -26,6 +26,8 @@ import ( "os" "os/exec" "path/filepath" + "sort" + "strings" "time" . "gopkg.in/check.v1" @@ -58,12 +60,13 @@ import ( "github.com/snapcore/snapd/overlord/state" "github.com/snapcore/snapd/release" "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/naming" "github.com/snapcore/snapd/snap/snaptest" "github.com/snapcore/snapd/store" "github.com/snapcore/snapd/testutil" ) -func expectedDoInstallTasks(typ snap.Type, opts, discards int, startTasks []string, filterOut map[string]bool) []string { +func expectedDoInstallTasks(typ snap.Type, opts, discards int, startTasks []string, components []string, filterOut map[string]bool) []string { if !release.OnClassic || opts&isHybrid != 0 { switch typ { case snap.TypeGadget: @@ -108,11 +111,24 @@ func expectedDoInstallTasks(typ snap.Type, opts, discards int, startTasks []stri if opts&updatesGadget != 0 { expected = append(expected, "update-gadget-cmdline") } - expected = append(expected, - "copy-snap-data", - "setup-profiles", - "link-snap", - "auto-connect") + + expected = append(expected, "copy-snap-data") + + afterLinkSnap := make([]string, 0, len(components)) + for range components { + compTasks := expectedComponentInstallTasks(compOptSkipSecurity) + for i, t := range compTasks { + if t == "link-component" { + afterLinkSnap = append(afterLinkSnap, compTasks[i:]...) + break + } + expected = append(expected, t) + } + } + + expected = append(expected, "setup-profiles", "link-snap") + expected = append(expected, afterLinkSnap...) + expected = append(expected, "auto-connect") if opts&updatesGadgetAssets != 0 && opts&needsKernelSetup != 0 { expected = append(expected, "discard-old-kernel-snap-setup") } @@ -165,9 +181,13 @@ func expectedDoInstallTasks(typ snap.Type, opts, discards int, startTasks []stri } func verifyInstallTasks(c *C, typ snap.Type, opts, discards int, ts *state.TaskSet) { + verifyInstallTasksWithComponents(c, typ, opts, discards, nil, ts) +} + +func verifyInstallTasksWithComponents(c *C, typ snap.Type, opts, discards int, components []string, ts *state.TaskSet) { kinds := taskKinds(ts.Tasks()) - expected := expectedDoInstallTasks(typ, opts, discards, nil, nil) + expected := expectedDoInstallTasks(typ, opts, discards, nil, components, nil) c.Assert(kinds, DeepEquals, expected) @@ -180,6 +200,18 @@ func verifyInstallTasks(c *C, typ snap.Type, opts, discards int, ts *state.TaskS c.Assert(te.Kind(), Equals, "validate-snap") } } + + var count int + for _, t := range ts.Tasks() { + switch t.Kind() { + case "prepare-component", "download-component": + compsup, _, err := snapstate.TaskComponentSetup(t) + c.Assert(err, IsNil) + c.Check(compsup.CompSideInfo.Component.ComponentName, Equals, components[count]) + count++ + } + } + c.Check(count, Equals, len(components), Commentf("expected %d components, found %d", len(components), count)) } func (s *snapmgrTestSuite) TestInstallDevModeConfinementFiltering(c *C) { @@ -554,8 +586,8 @@ func (s *snapmgrTestSuite) TestInstallFailsOnDisabledSnap(c *C) { Current: snap.R(2), SnapType: "app", } - snapsup := &snapstate.SnapSetup{SideInfo: &snap.SideInfo{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(1)}} - _, err := snapstate.DoInstall(s.state, snapst, snapsup, 0, "", nil) + snapsup := snapstate.SnapSetup{SideInfo: &snap.SideInfo{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(1)}} + _, err := snapstate.DoInstall(s.state, snapst, snapsup, nil, 0, "", nil) c.Assert(err, ErrorMatches, `cannot update disabled snap "some-snap"`) } @@ -607,12 +639,12 @@ func (s *snapmgrTestSuite) TestInstallFailsOnBusySnap(c *C) { defer restore() // Attempt to install revision 2 of the snap. - snapsup := &snapstate.SnapSetup{ + snapsup := snapstate.SnapSetup{ SideInfo: &snap.SideInfo{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(2)}, } // And observe that we cannot refresh because the snap is busy. - _, err := snapstate.DoInstall(s.state, snapst, snapsup, 0, "", inUseCheck) + _, err := snapstate.DoInstall(s.state, snapst, snapsup, nil, 0, "", inUseCheck) c.Assert(err, ErrorMatches, `snap "some-snap" has running apps \(app\), pids: 1234`) // Don't record time since it wasn't a failed refresh @@ -671,14 +703,14 @@ func (s *snapmgrTestSuite) TestInstallWithIgnoreRunningProceedsOnBusySnap(c *C) defer restore() // Attempt to install revision 2 of the snap, with the IgnoreRunning flag set. - snapsup := &snapstate.SnapSetup{ + snapsup := snapstate.SnapSetup{ SideInfo: &snap.SideInfo{RealName: "pkg", SnapID: "pkg-id", Revision: snap.R(2)}, Flags: snapstate.Flags{IgnoreRunning: true}, Type: "app", } // And observe that we do so despite the running app. - _, err := snapstate.DoInstall(s.state, snapst, snapsup, 0, "", inUseCheck) + _, err := snapstate.DoInstall(s.state, snapst, snapsup, nil, 0, "", inUseCheck) c.Assert(err, IsNil) // The state confirms that the refresh operation was not postponed. @@ -734,12 +766,12 @@ func (s *snapmgrTestSuite) TestInstallDespiteBusySnap(c *C) { defer restore() // Attempt to install revision 2 of the snap. - snapsup := &snapstate.SnapSetup{ + snapsup := snapstate.SnapSetup{ SideInfo: &snap.SideInfo{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(2)}, } // And observe that refresh occurred regardless of the running process. - _, err := snapstate.DoInstall(s.state, snapst, snapsup, 0, "", inUseCheck) + _, err := snapstate.DoInstall(s.state, snapst, snapsup, nil, 0, "", inUseCheck) c.Assert(err, IsNil) } @@ -747,8 +779,8 @@ func (s *snapmgrTestSuite) TestInstallFailsOnSystem(c *C) { s.state.Lock() defer s.state.Unlock() - snapsup := &snapstate.SnapSetup{SideInfo: &snap.SideInfo{RealName: "system", SnapID: "some-snap-id", Revision: snap.R(1)}} - _, err := snapstate.DoInstall(s.state, nil, snapsup, 0, "", nil) + snapsup := snapstate.SnapSetup{SideInfo: &snap.SideInfo{RealName: "system", SnapID: "some-snap-id", Revision: snap.R(1)}} + _, err := snapstate.DoInstall(s.state, nil, snapsup, nil, 0, "", nil) c.Assert(err, ErrorMatches, `cannot install reserved snap name 'system'`) } @@ -832,7 +864,7 @@ func (s *snapmgrTestSuite) TestInstallNoRestartBoundaries(c *C) { r := snapstatetest.MockDeviceModel(DefaultModel()) defer r() - snapsup := &snapstate.SnapSetup{ + snapsup := snapstate.SnapSetup{ SideInfo: &snap.SideInfo{ RealName: "brand-gadget", SnapID: "brand-gadget", @@ -843,7 +875,7 @@ func (s *snapmgrTestSuite) TestInstallNoRestartBoundaries(c *C) { // Ensure that restart boundaries were set on 'link-snap' as a part of doInstall // when the flag noRestartBoundaries is not set - ts1, err := snapstate.DoInstall(s.state, &snapstate.SnapState{}, snapsup, 0, "", inUseCheck) + ts1, err := snapstate.DoInstall(s.state, &snapstate.SnapState{}, snapsup, nil, 0, "", inUseCheck) c.Assert(err, IsNil) linkSnap1 := ts1.MaybeEdge(snapstate.MaybeRebootEdge) @@ -853,7 +885,7 @@ func (s *snapmgrTestSuite) TestInstallNoRestartBoundaries(c *C) { c.Check(linkSnap1.Get("restart-boundary", &boundary), IsNil) // Ensure that restart boundaries are not set when we do provide the noRestartBoundaries flag - ts2, err := snapstate.DoInstall(s.state, &snapstate.SnapState{}, snapsup, snapstate.NoRestartBoundaries, "", inUseCheck) + ts2, err := snapstate.DoInstall(s.state, &snapstate.SnapState{}, snapsup, nil, snapstate.NoRestartBoundaries, "", inUseCheck) c.Assert(err, IsNil) linkSnap2 := ts2.MaybeEdge(snapstate.MaybeRebootEdge) @@ -6109,3 +6141,446 @@ func (s *snapmgrTestSuite) TestInstallOneSnapMisbehavingGoal(c *C) { }) c.Check(err, testutil.ErrorIs, snapstate.ErrExpectedOneSnap) } + +func (s *snapmgrTestSuite) TestInstallZeroComponentsRunThrough(c *C) { + const ( + undo = false + snapName = "test-snap" + instanceKey = "" + ) + s.testInstallComponentsRunThrough(c, snapName, instanceKey, nil, undo) +} + +func (s *snapmgrTestSuite) TestInstallOneComponentsRunThrough(c *C) { + const ( + undo = false + snapName = "test-snap" + instanceKey = "" + ) + s.testInstallComponentsRunThrough(c, snapName, instanceKey, []string{"test-component"}, undo) +} + +func (s *snapmgrTestSuite) TestInstallManyComponentsRunThrough(c *C) { + const ( + undo = false + snapName = "test-snap" + instanceKey = "" + ) + s.testInstallComponentsRunThrough(c, snapName, instanceKey, []string{"test-component", "kernel-modules-component"}, undo) +} + +func (s *snapmgrTestSuite) TestInstallInstanceManyComponentsRunThrough(c *C) { + const ( + undo = false + snapName = "test-snap" + instanceKey = "key" + ) + s.testInstallComponentsRunThrough(c, snapName, instanceKey, []string{"test-component", "kernel-modules-component"}, undo) +} + +func (s *snapmgrTestSuite) TestInstallManyComponentsUndoRunThrough(c *C) { + const ( + undo = false + snapName = "test-snap" + instanceKey = "" + ) + s.testInstallComponentsRunThrough(c, snapName, instanceKey, []string{"test-component", "kernel-modules-component"}, undo) +} + +func (s *snapmgrTestSuite) TestInstallInstanceManyComponentsUndoRunThrough(c *C) { + const ( + undo = true + snapName = "test-snap" + instanceKey = "key" + ) + s.testInstallComponentsRunThrough(c, snapName, instanceKey, []string{"test-component", "kernel-modules-component"}, undo) +} + +func undoInstallOps(snapName, instanceName string, snapRevision snap.Revision, components []string) []fakeOp { + snapMount := filepath.Join(dirs.SnapMountDir, filepath.Join(instanceName, snapRevision.String())) + ops := []fakeOp{{ + op: "update-aliases", + }, { + op: "auto-connect:Undoing", + name: instanceName, + revno: snapRevision, + }} + + for i := len(components) - 1; i >= 0; i-- { + ops = append(ops, fakeOp{ + op: "unlink-component", + path: snap.ComponentMountDir(components[i], snap.R(i+1), instanceName), + }) + } + + ops = append(ops, []fakeOp{{ + op: "discard-namespace", + name: instanceName, + }, { + op: "unlink-snap", + path: snapMount, + unlinkFirstInstallUndo: true, + }, { + op: "setup-profiles:Undoing", + name: instanceName, + revno: snapRevision, + }}...) + + for i := len(components) - 1; i >= 0; i-- { + csi := &snap.ComponentSideInfo{ + Component: naming.NewComponentRef(snapName, components[i]), + Revision: snap.R(i + 1), + } + + containerName := fmt.Sprintf("%s+%s", instanceName, components[i]) + filename := fmt.Sprintf("%s_%v.comp", containerName, csi.Revision) + + if strings.HasPrefix(components[i], string(snap.KernelModulesComponent)) { + ops = append(ops, fakeOp{ + op: "remove-kernel-modules-components-setup", + compsToRemove: []*snap.ComponentSideInfo{csi}, + finalComps: []*snap.ComponentSideInfo{}, + }) + } + + ops = append(ops, []fakeOp{{ + op: "undo-setup-component", + containerName: containerName, + containerFileName: filename, + }, { + op: "remove-component-dir", + containerName: containerName, + containerFileName: filename, + }}...) + } + + ops = append(ops, []fakeOp{{ + op: "undo-copy-snap-data", + path: snapMount, + old: "", + }, { + op: "undo-setup-snap-save-data", + path: filepath.Join(dirs.SnapDataSaveDir, instanceName), + old: "", + }, { + op: "remove-snap-data-dir", + name: instanceName, + path: filepath.Join(dirs.SnapDataDir, instanceName), + }, { + op: "undo-setup-snap", + name: instanceName, + stype: "app", + path: snapMount, + }, { + op: "remove-snap-dir", + name: instanceName, + path: filepath.Join(dirs.SnapMountDir, instanceName), + }}...) + + return ops +} + +func (s *snapmgrTestSuite) testInstallComponentsRunThrough(c *C, snapName, instanceKey string, components []string, undo bool) { + s.state.Lock() + defer s.state.Unlock() + + // sort these so that we can predict the order of the ops + sort.Strings(components) + + if instanceKey != "" { + tr := config.NewTransaction(s.state) + tr.Set("core", "experimental.parallel-instances", true) + tr.Commit() + } + + const ( + snapID = "test-snap-id" + channel = "channel-for-components" + ) + snapRevision := snap.R(11) + + sort.Strings(components) + + instanceName := snap.InstanceName(snapName, instanceKey) + + // we start without the auxiliary store info + c.Check(snapstate.AuxStoreInfoFilename(snapID), testutil.FileAbsent) + + 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 i, compName := range components { + results = append(results, store.SnapResourceResult{ + DownloadInfo: snap.DownloadInfo{ + DownloadURL: "http://example.com/" + compName, + }, + Name: compName, + Revision: i + 1, + Type: fmt.Sprintf("component/%s", compNameToType(compName)), + Version: "1.0", + CreatedAt: "2024-01-01T00:00:00Z", + }) + } + return results + } + + target := snapstate.StoreInstallGoal(snapstate.StoreSnap{ + InstanceName: instanceName, + Components: components, + RevOpts: snapstate.RevisionOptions{Channel: channel}, + }) + + info, ts, err := snapstate.InstallOne(context.Background(), s.state, target, snapstate.Options{ + UserID: 1, + }) + c.Assert(err, IsNil) + + c.Check(info.InstanceName(), Equals, instanceName) + c.Check(info.Channel, Equals, channel) + c.Check(info.Revision, Equals, snapRevision) + + s.AddCleanup(snapstate.MockReadComponentInfo(func( + compMntDir string, snapInfo *snap.Info, csi *snap.ComponentSideInfo, + ) (*snap.ComponentInfo, error) { + for i, compName := range components { + if compMntDir == snap.ComponentMountDir(compName, snap.R(i+1), instanceName) { + return &snap.ComponentInfo{}, nil + } + } + return nil, fmt.Errorf("unexpected component mount dir %q", compMntDir) + })) + + chg := s.state.NewChange("install", "install a snap") + chg.AddAll(ts) + + if undo { + last := ts.Tasks()[len(ts.Tasks())-1] + terr := s.state.NewTask("error-trigger", "provoking total undo") + terr.WaitFor(last) + chg.AddTask(terr) + } + + s.settle(c) + + // ensure all our tasks ran + if 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()))) + } + + c.Assert(chg.IsReady(), Equals, true, Commentf("change tasks:\n%s", printTasks(chg.Tasks()))) + c.Check(snapstate.Installing(s.state), Equals, false) + + downloads := []fakeDownload{{ + macaroon: s.user.StoreMacaroon, + name: snapName, + target: filepath.Join(dirs.SnapBlobDir, fmt.Sprintf("%s_%v.snap", instanceName, snapRevision)), + }} + for i, compName := range 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, i+1)), + }) + } + c.Check(s.fakeStore.downloads, DeepEquals, downloads) + c.Check(s.fakeStore.seenPrivacyKeys["privacy-key"], Equals, true, Commentf("salts seen: %v", s.fakeStore.seenPrivacyKeys)) + + snapFileName := fmt.Sprintf("%s_%v.snap", instanceName, snapRevision) + + // ops that happens before component tasks + expected := fakeOps{{ + op: "storesvc-snap-action", + userID: 1, + }, { + op: "storesvc-snap-action:action", + action: store.SnapAction{ + Action: "install", + InstanceName: instanceName, + Channel: channel, + }, + revno: snapRevision, + userID: 1, + }, { + op: "storesvc-download", + name: snapName, + }, { + op: "validate-snap:Doing", + name: instanceName, + revno: snapRevision, + }, { + op: "current", + old: "", + }, { + op: "open-snap-file", + path: filepath.Join(dirs.SnapBlobDir, snapFileName), + sinfo: snap.SideInfo{ + RealName: snapName, + SnapID: snapID, + Channel: channel, + Revision: snapRevision, + }, + }, { + op: "setup-snap", + name: instanceName, + path: filepath.Join(dirs.SnapBlobDir, snapFileName), + revno: snapRevision, + }, { + op: "copy-data", + path: filepath.Join(dirs.SnapMountDir, filepath.Join(instanceName, snapRevision.String())), + old: "", + }, { + op: "setup-snap-save-data", + path: filepath.Join(dirs.SnapDataSaveDir, instanceName), + }} + + // ops for mounting a component (but not yet linking it) + for i, compName := range components { + csi := snap.ComponentSideInfo{ + Component: naming.NewComponentRef(snapName, compName), + Revision: snap.R(i + 1), + } + + containerName := fmt.Sprintf("%s+%s", instanceName, compName) + filename := fmt.Sprintf("%s_%d.comp", containerName, i+1) + + expected = append(expected, []fakeOp{{ + op: "storesvc-download", + name: csi.Component.String(), + }, { + op: "validate-component:Doing", + name: instanceName, + revno: snapRevision, + componentName: compName, + componentPath: filepath.Join(dirs.SnapBlobDir, filename), + componentRev: csi.Revision, + componentSideInfo: csi, + }, { + op: "setup-component", + containerName: containerName, + containerFileName: filename, + }}...) + + if strings.HasPrefix(compName, string(snap.KernelModulesComponent)) { + expected = append(expected, fakeOp{ + op: "setup-kernel-modules-components", + currentComps: []*snap.ComponentSideInfo{}, + compsToInstall: []*snap.ComponentSideInfo{{ + Component: naming.NewComponentRef(snapName, compName), + Revision: snap.R(i + 1), + }}, + }) + } + } + + expected = append(expected, []fakeOp{{ + op: "setup-profiles:Doing", + name: instanceName, + revno: snapRevision, + }, { + op: "candidate", + sinfo: snap.SideInfo{ + RealName: snapName, + SnapID: snapID, + Channel: channel, + Revision: snapRevision, + }, + }, { + op: "link-snap", + path: filepath.Join(dirs.SnapMountDir, filepath.Join(instanceName, snapRevision.String())), + }}...) + + // ops for linking components + for i, compName := range components { + expected = append(expected, []fakeOp{{ + op: "link-component", + path: snap.ComponentMountDir(compName, snap.R(i+1), instanceName), + }}...) + } + + // ops that follow linking components + expected = append(expected, []fakeOp{{ + op: "auto-connect:Doing", + name: instanceName, + revno: snapRevision, + }, { + op: "update-aliases", + }}...) + + if undo { + expected = append(expected, undoInstallOps(snapName, instanceName, snapRevision, components)...) + } else { + expected = append(expected, fakeOp{ + op: "cleanup-trash", + name: instanceName, + revno: snapRevision, + }) + } + + // 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) + + if undo { + var snapst snapstate.SnapState + err = snapstate.Get(s.state, instanceName, &snapst) + c.Assert(err, testutil.ErrorIs, state.ErrNoState) + + c.Check(snapstate.AuxStoreInfoFilename(snapID), testutil.FileAbsent) + } else { + // verify snap in the system state + var snapst snapstate.SnapState + err = snapstate.Get(s.state, instanceName, &snapst) + c.Assert(err, IsNil) + + c.Check(snapst.Active, Equals, true) + c.Check(snapst.TrackingChannel, Equals, fmt.Sprintf("%s/stable", channel)) + c.Check(snapst.Required, Equals, false) + + c.Check(snapst.CurrentSideInfo(), DeepEquals, &snap.SideInfo{ + RealName: snapName, + Channel: channel, + Revision: snapRevision, + SnapID: snapID, + }) + + var compst []*sequence.ComponentState + for i, compName := range components { + compst = append(compst, &sequence.ComponentState{ + SideInfo: &snap.ComponentSideInfo{ + Component: naming.NewComponentRef(snapName, compName), + Revision: snap.R(i + 1), + }, + CompType: compNameToType(compName), + }) + } + + // make sure that all of our components are accounted for + c.Assert(snapst.Sequence.Revisions[0].Components, DeepEquals, compst) + + c.Check(snapstate.AuxStoreInfoFilename(snapID), testutil.FilePresent) + } +} + +func printTasks(ts []*state.Task) string { + var b strings.Builder + for _, t := range ts { + fmt.Fprintf(&b, "task %s (%s), %s, %s\n", t.Kind(), t.ID(), t.Summary(), t.Status()) + if t.Status() != state.DoneStatus { + fmt.Fprintln(&b, " waiting on:") + for _, w := range t.WaitTasks() { + fmt.Fprintf(&b, " - %s (%s), %s\n", w.Kind(), w.ID(), w.Status()) + } + } + } + return b.String() +} diff --git a/overlord/snapstate/snapstate_remove_test.go b/overlord/snapstate/snapstate_remove_test.go index 84ed352deb6..50ed482a49a 100644 --- a/overlord/snapstate/snapstate_remove_test.go +++ b/overlord/snapstate/snapstate_remove_test.go @@ -438,7 +438,7 @@ func (s *snapmgrTestSuite) TestParallelInstanceRemoveRunThrough(c *C) { { op: "remove-snap-data-dir", name: "some-snap_instance", - path: filepath.Join(dirs.SnapDataDir, "some-snap"), + path: filepath.Join(dirs.SnapDataDir, "some-snap_instance"), otherInstances: true, }, { @@ -461,7 +461,7 @@ func (s *snapmgrTestSuite) TestParallelInstanceRemoveRunThrough(c *C) { { op: "remove-snap-dir", name: "some-snap_instance", - path: filepath.Join(dirs.SnapMountDir, "some-snap"), + path: filepath.Join(dirs.SnapMountDir, "some-snap_instance"), otherInstances: true, }, } @@ -592,7 +592,7 @@ func (s *snapmgrTestSuite) TestParallelInstanceRemoveRunThroughOtherInstances(c { op: "remove-snap-data-dir", name: "some-snap_instance", - path: filepath.Join(dirs.SnapDataDir, "some-snap"), + path: filepath.Join(dirs.SnapDataDir, "some-snap_instance"), otherInstances: true, }, { @@ -615,7 +615,7 @@ func (s *snapmgrTestSuite) TestParallelInstanceRemoveRunThroughOtherInstances(c { op: "remove-snap-dir", name: "some-snap_instance", - path: filepath.Join(dirs.SnapMountDir, "some-snap"), + path: filepath.Join(dirs.SnapMountDir, "some-snap_instance"), otherInstances: true, }, } diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index dcdda6ba56c..7c5fbef413a 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/overlord/snapstate/snapstate_test.go @@ -352,22 +352,30 @@ func (s *snapmgrBaseTest) TearDownTest(c *C) { } type ForeignTaskTracker interface { - ForeignTask(kind string, status state.Status, snapsup *snapstate.SnapSetup) error + ForeignTask(kind string, status state.Status, snapsup *snapstate.SnapSetup, compsup *snapstate.ComponentSetup) error } func AddForeignTaskHandlers(runner *state.TaskRunner, tracker ForeignTaskTracker) { // Add fake handlers for tasks handled by interfaces manager fakeHandler := func(task *state.Task, _ *tomb.Tomb) error { task.State().Lock() + defer task.State().Unlock() kind := task.Kind() status := task.Status() snapsup, err := snapstate.TaskSnapSetup(task) - task.State().Unlock() if err != nil { return err } - return tracker.ForeignTask(kind, status, snapsup) + var compsup *snapstate.ComponentSetup + if task.Has("component-setup") || task.Has("component-setup-task") { + compsup, _, err = snapstate.TaskComponentSetup(task) + if err != nil { + return err + } + } + + return tracker.ForeignTask(kind, status, snapsup, compsup) } runner.AddHandler("setup-profiles", fakeHandler, fakeHandler) runner.AddHandler("auto-connect", fakeHandler, fakeHandler) @@ -375,6 +383,7 @@ func AddForeignTaskHandlers(runner *state.TaskRunner, tracker ForeignTaskTracker runner.AddHandler("remove-profiles", fakeHandler, fakeHandler) runner.AddHandler("discard-conns", fakeHandler, fakeHandler) runner.AddHandler("validate-snap", fakeHandler, nil) + runner.AddHandler("validate-component", fakeHandler, nil) runner.AddHandler("transition-ubuntu-core", fakeHandler, nil) runner.AddHandler("transition-to-snapd-snap", fakeHandler, nil) runner.AddHandler("update-gadget-assets", fakeHandler, nil) @@ -8093,7 +8102,7 @@ func (s *snapmgrTestSuite) testRemodelLinkNewBaseOrKernelHappy(c *C, model *asse 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, []string{"prepare-snap"}, kindsToSet(nonReLinkKinds))) + c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeKernel, opts, 0, []string{"prepare-snap"}, nil, kindsToSet(nonReLinkKinds))) tPrepare := tasks[0] var tLink, tUpdateGadgetAssets *state.Task if opts&needsKernelSetup != 0 { @@ -8121,7 +8130,7 @@ func (s *snapmgrTestSuite) testRemodelLinkNewBaseOrKernelHappy(c *C, model *asse ts, err = snapstate.LinkNewBaseOrKernel(s.state, "some-base", "") c.Assert(err, IsNil) tasks = ts.Tasks() - c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeBase, 0, 0, []string{"prepare-snap"}, kindsToSet(nonReLinkKinds))) + c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeBase, 0, 0, []string{"prepare-snap"}, nil, kindsToSet(nonReLinkKinds))) c.Assert(tasks, HasLen, 2) tPrepare = tasks[0] tLink = tasks[1] @@ -8215,7 +8224,7 @@ func (s *snapmgrTestSuite) testRemodelAddLinkNewBaseOrKernel(c *C, model *assert c.Assert(err, IsNil) c.Assert(tsNew, NotNil) tasks := tsNew.Tasks() - c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeKernel, opts, 0, []string{"prepare-snap", "test-task"}, kindsToSet(nonReLinkKinds))) + c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeKernel, opts, 0, []string{"prepare-snap", "test-task"}, nil, kindsToSet(nonReLinkKinds))) // since this is the kernel, we have our task + test task + update-gadget-assets + link-snap var tLink, tUpdateGadgetAssets *state.Task if opts&needsKernelSetup != 0 { @@ -8259,7 +8268,7 @@ func (s *snapmgrTestSuite) testRemodelAddLinkNewBaseOrKernel(c *C, model *assert c.Assert(err, IsNil) c.Assert(tsNew, NotNil) tasks = tsNew.Tasks() - c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeBase, 0, 0, []string{"prepare-snap"}, kindsToSet(nonReLinkKinds))) + c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeBase, 0, 0, []string{"prepare-snap"}, nil, kindsToSet(nonReLinkKinds))) // since this is the base, we have our task + link-snap only c.Assert(tasks, HasLen, 2) tLink = tasks[1] @@ -8288,7 +8297,9 @@ func (s *snapmgrTestSuite) TestRemodelSwitchNewGadget(c *C) { ts, err := snapstate.SwitchToNewGadget(s.state, "some-gadget", "") c.Assert(err, IsNil) tasks := ts.Tasks() - c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeGadget, 0, 0, []string{"prepare-snap"}, kindsToSet(append(nonReLinkKinds, "link-snap")))) + c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks( + snap.TypeGadget, 0, 0, []string{"prepare-snap"}, nil, kindsToSet(append(nonReLinkKinds, "link-snap"))), + ) c.Assert(tasks, HasLen, 3) tPrepare := tasks[0] tUpdateGadgetAssets := tasks[1] @@ -8423,7 +8434,9 @@ func (s *snapmgrTestSuite) TestRemodelAddGadgetAssetTasks(c *C) { c.Assert(err, IsNil) c.Assert(tsNew, NotNil) tasks := tsNew.Tasks() - c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks(snap.TypeGadget, 0, 0, []string{"prepare-snap", "test-task"}, kindsToSet(append(nonReLinkKinds, "link-snap")))) + c.Check(taskKinds(tasks), DeepEquals, expectedDoInstallTasks( + snap.TypeGadget, 0, 0, []string{"prepare-snap", "test-task"}, nil, kindsToSet(append(nonReLinkKinds, "link-snap"))), + ) // since this is the gadget, we have our task + test task + update assets + update cmdline c.Assert(tasks, HasLen, 4) tUpdateGadgetAssets := tasks[2] diff --git a/overlord/snapstate/snapstate_update_test.go b/overlord/snapstate/snapstate_update_test.go index 027b8461eb9..8bb11cda664 100644 --- a/overlord/snapstate/snapstate_update_test.go +++ b/overlord/snapstate/snapstate_update_test.go @@ -68,7 +68,7 @@ import ( func verifyUpdateTasks(c *C, typ snap.Type, opts, discards int, ts *state.TaskSet) { kinds := taskKinds(ts.Tasks()) - expected := expectedDoInstallTasks(typ, unlinkBefore|cleanupAfter|opts, discards, nil, nil) + expected := expectedDoInstallTasks(typ, unlinkBefore|cleanupAfter|opts, discards, nil, nil, nil) if opts&doesReRefresh != 0 { expected = append(expected, "check-rerefresh") } @@ -10224,12 +10224,12 @@ func (s *snapmgrTestSuite) TestAutoRefreshCreatePreDownload(c *C) { SnapType: string(snap.TypeApp), } snapstate.Set(s.state, "some-snap", snapst) - snapsup := &snapstate.SnapSetup{ + snapsup := snapstate.SnapSetup{ SideInfo: &snap.SideInfo{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(2)}, Flags: snapstate.Flags{IsAutoRefresh: true}, } - ts, err := snapstate.DoInstall(s.state, snapst, snapsup, 0, "", inUseCheck) + ts, err := snapstate.DoInstall(s.state, snapst, snapsup, nil, 0, "", inUseCheck) var busyErr *snapstate.TimedBusySnapError c.Assert(errors.As(err, &busyErr), Equals, true) @@ -10642,7 +10642,7 @@ func (s *snapmgrTestSuite) TestAutoRefreshBusySnapButOngoingPreDownload(c *C) { SnapType: string(snap.TypeApp), } snapstate.Set(s.state, "some-snap", snapst) - snapsup := &snapstate.SnapSetup{ + snapsup := snapstate.SnapSetup{ SideInfo: &snap.SideInfo{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(2)}, Flags: snapstate.Flags{IsAutoRefresh: true}, } @@ -10657,7 +10657,7 @@ func (s *snapmgrTestSuite) TestAutoRefreshBusySnapButOngoingPreDownload(c *C) { // don't create a pre-download task if one exists w/ these statuses for _, status := range []state.Status{state.DoStatus, state.DoingStatus} { task.SetStatus(status) - ts, err := snapstate.DoInstall(s.state, snapst, snapsup, 0, "", inUseCheck) + ts, err := snapstate.DoInstall(s.state, snapst, snapsup, nil, 0, "", inUseCheck) var busyErr *snapstate.TimedBusySnapError c.Assert(errors.As(err, &busyErr), Equals, true) @@ -10675,7 +10675,7 @@ func (s *snapmgrTestSuite) TestAutoRefreshBusySnapButOngoingPreDownload(c *C) { // a "Done" pre-download is ignored since the auto-refresh it causes might also be done task.SetStatus(state.DoneStatus) - ts, err := snapstate.DoInstall(s.state, snapst, snapsup, 0, "", inUseCheck) + ts, err := snapstate.DoInstall(s.state, snapst, snapsup, nil, 0, "", inUseCheck) c.Assert(err, FitsTypeOf, &snapstate.TimedBusySnapError{}) c.Assert(ts.Tasks(), HasLen, 1) } @@ -12463,7 +12463,7 @@ type: snapd panic(err) } } - return s.fakeBackend.ForeignTask(kind, status, snapsup) + return s.fakeBackend.ForeignTask(kind, status, snapsup, nil) } s.o.TaskRunner().AddHandler("auto-connect", fakeAutoConnect, fakeAutoConnect) diff --git a/overlord/snapstate/target.go b/overlord/snapstate/target.go index ff598cd2b7f..aa51cf80a12 100644 --- a/overlord/snapstate/target.go +++ b/overlord/snapstate/target.go @@ -23,6 +23,7 @@ import ( "context" "errors" "fmt" + "sort" "github.com/snapcore/snapd/asserts/snapasserts" "github.com/snapcore/snapd/client" @@ -30,6 +31,7 @@ import ( "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/snap/naming" "github.com/snapcore/snapd/store" + "github.com/snapcore/snapd/strutil" ) // Options contains optional parameters for the snapstate operations. All of @@ -67,29 +69,28 @@ type Options struct { type target struct { // setup is a partially initialized SnapSetup that contains the data needed // to find the snap file that will be installed. - setup *SnapSetup - // components is a list of partially initialized ComponentSetup structs that - // contain the data needed to find the component files that will be - // installed. - components []*ComponentSetup + setup SnapSetup // info contains the snap.info for the snap to be installed. info *snap.Info // snapst is the current state of the target snap, prior to installation. // This must be retrieved prior to unlocking the state for any reason (for // example, talking to the store). snapst SnapState + // components is a list of components to install with this snap. + components []ComponentSetup } -// snapsup returns the completed SnapSetup for the target snap. -func (t *target) snapsup(st *state.State, opts Options) (SnapSetup, error) { +// setups returns the completed SnapSetup and slice of ComponentSetup structs +// for the target snap. +func (t *target) setups(st *state.State, opts Options) (SnapSetup, []ComponentSetup, error) { snapUserID, err := userIDForSnap(st, &t.snapst, opts.UserID) if err != nil { - return SnapSetup{}, err + return SnapSetup{}, nil, err } flags, err := earlyChecks(st, &t.snapst, t.info, opts.Flags) if err != nil { - return SnapSetup{}, err + return SnapSetup{}, nil, err } providerContentAttrs := defaultProviderContentAttrs(st, t.info, opts.PrereqTracker) @@ -110,7 +111,7 @@ func (t *target) snapsup(st *state.State, opts Options) (SnapSetup, error) { PlugsOnly: len(t.info.Slots) == 0, InstanceKey: t.info.InstanceKey, ExpectedProvenance: t.info.SnapProvenance, - }, nil + }, t.components, nil } // InstallGoal represents a single snap or a group of snaps to be installed. @@ -164,6 +165,10 @@ func StoreInstallGoal(snaps ...StoreSnap) InstallGoal { sn.RevOpts.Channel = "stable" } + if len(sn.Components) > 0 { + sn.Components = strutil.Deduplicate(sn.Components) + } + seen[sn.InstanceName] = true unique = append(unique, sn) } @@ -216,6 +221,7 @@ func (s *storeInstallGoal) toInstall(ctx context.Context, st *state.State, opts return vsets, nil } + includeResources := false actions := make([]*store.SnapAction, 0, len(s.snaps)) for _, sn := range s.snaps { action, err := installActionForStoreTarget(sn, opts, enforcedSets) @@ -223,6 +229,10 @@ func (s *storeInstallGoal) toInstall(ctx context.Context, st *state.State, opts return nil, err } + if len(sn.Components) > 0 { + includeResources = true + } + actions = append(actions, action) } @@ -231,7 +241,9 @@ func (s *storeInstallGoal) toInstall(ctx context.Context, st *state.State, opts return nil, err } - refreshOpts, err := refreshOptions(st, nil) + refreshOpts, err := refreshOptions(st, &store.RefreshOptions{ + IncludeResources: includeResources, + }) if err != nil { return nil, err } @@ -266,8 +278,6 @@ func (s *storeInstallGoal) toInstall(ctx context.Context, st *state.State, opts snapst = &SnapState{} } - // TODO: extract components from resources - // TODO: is it safe to pull the channel from here? i'm not sure what // this will actually look like as a response from the real store channel := r.RedirectChannel @@ -275,21 +285,73 @@ func (s *storeInstallGoal) toInstall(ctx context.Context, st *state.State, opts channel = sn.RevOpts.Channel } + comps, err := requestedComponentsFromActionResult(sn, r) + if err != nil { + return nil, fmt.Errorf("cannot extract components from snap resources: %w", err) + } + installs = append(installs, target{ - setup: &SnapSetup{ + setup: SnapSetup{ DownloadInfo: &r.DownloadInfo, Channel: channel, CohortKey: sn.RevOpts.CohortKey, }, info: r.Info, snapst: *snapst, - components: nil, + components: comps, }) } return installs, err } +func requestedComponentsFromActionResult(sn StoreSnap, sar store.SnapActionResult) ([]ComponentSetup, error) { + mapping := make(map[string]store.SnapResourceResult, len(sar.Resources)) + for _, res := range sar.Resources { + mapping[res.Name] = res + } + + setups := make([]ComponentSetup, 0, len(sn.Components)) + for _, comp := range sn.Components { + res, ok := mapping[comp] + if !ok { + return nil, fmt.Errorf("cannot find component %q in snap resources", comp) + } + + setup, err := componentSetupFromResource(comp, res, sar.Info) + if err != nil { + return nil, err + } + + setups = append(setups, setup) + } + return setups, nil +} + +func componentSetupFromResource(name string, sar store.SnapResourceResult, info *snap.Info) (ComponentSetup, error) { + comp, ok := info.Components[name] + if !ok { + return ComponentSetup{}, fmt.Errorf("%q is not a component for snap %q", name, info.SnapName()) + } + + if typ := fmt.Sprintf("component/%s", comp.Type); typ != sar.Type { + return ComponentSetup{}, fmt.Errorf("inconsistent component type (%q in snap, %q in component)", typ, sar.Type) + } + + cref := naming.NewComponentRef(info.SnapName(), name) + + csi := snap.ComponentSideInfo{ + Component: cref, + Revision: snap.R(sar.Revision), + } + + return ComponentSetup{ + DownloadInfo: &sar.DownloadInfo, + CompSideInfo: &csi, + CompType: comp.Type, + }, nil +} + func installActionForStoreTarget(t StoreSnap, opts Options, enforcedSets func() (*snapasserts.ValidationSets, error)) (*store.SnapAction, error) { action := &store.SnapAction{ Action: "install", @@ -458,6 +520,14 @@ func InstallWithGoal(ctx context.Context, st *state.State, goal InstallGoal, opt return nil, nil, ErrExpectedOneSnap } + for _, t := range targets { + // sort the components by name to ensure we always install components in the + // same order + sort.Slice(t.components, func(i, j int) bool { + return t.components[i].ComponentName() < t.components[j].ComponentName() + }) + } + installInfos := make([]minimalInstallInfo, 0, len(targets)) for _, t := range targets { installInfos = append(installInfos, installSnapInfo{t.info}) @@ -480,7 +550,7 @@ func InstallWithGoal(ctx context.Context, st *state.State, goal InstallGoal, opt opts.PrereqTracker.Add(t.info) - snapsup, err := t.snapsup(st, opts) + snapsup, compsups, err := t.setups(st, opts) if err != nil { return nil, nil, err } @@ -490,7 +560,7 @@ func InstallWithGoal(ctx context.Context, st *state.State, goal InstallGoal, opt instFlags |= skipConfigure } - ts, err := doInstall(st, &t.snapst, &snapsup, instFlags, opts.FromChange, inUseFor(opts.DeviceCtx)) + ts, err := doInstall(st, &t.snapst, snapsup, compsups, instFlags, opts.FromChange, inUseFor(opts.DeviceCtx)) if err != nil { return nil, nil, err } @@ -608,7 +678,7 @@ func (p *pathInstallGoal) toInstall(ctx context.Context, st *state.State, opts O } inst := target{ - setup: &SnapSetup{ + setup: SnapSetup{ SnapPath: p.path, Channel: channel, CohortKey: p.revOpts.CohortKey, diff --git a/overlord/snapstate/target_test.go b/overlord/snapstate/target_test.go new file mode 100644 index 00000000000..83be6f47f8e --- /dev/null +++ b/overlord/snapstate/target_test.go @@ -0,0 +1,177 @@ +package snapstate_test + +import ( + "context" + "fmt" + + "github.com/snapcore/snapd/overlord/snapstate" + "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/store" + . "gopkg.in/check.v1" +) + +type TargetTestSuite struct { + snapmgrBaseTest +} + +var _ = Suite(&TargetTestSuite{}) + +func (s *TargetTestSuite) TestInstallWithComponents(c *C) { + s.state.Lock() + defer s.state.Unlock() + + const ( + snapName = "test-snap" + compName = "test-component" + channel = "channel-for-components" + ) + s.fakeStore.snapResourcesFn = func(info *snap.Info) []store.SnapResourceResult { + c.Assert(info.SnapName(), DeepEquals, snapName) + + return []store.SnapResourceResult{ + { + DownloadInfo: snap.DownloadInfo{ + DownloadURL: fmt.Sprintf("http://example.com/%s", snapName), + }, + Name: compName, + Revision: 1, + Type: fmt.Sprintf("component/%s", snap.TestComponent), + Version: "1.0", + CreatedAt: "2024-01-01T00:00:00Z", + }, + } + } + + goal := snapstate.StoreInstallGoal(snapstate.StoreSnap{ + InstanceName: snapName, + Components: []string{compName}, + RevOpts: snapstate.RevisionOptions{ + Channel: channel, + }, + }) + + info, ts, err := snapstate.InstallOne(context.Background(), s.state, goal, snapstate.Options{}) + c.Assert(err, IsNil) + + c.Check(info.InstanceName(), Equals, snapName) + c.Check(info.Channel, Equals, channel) + c.Check(info.Components[compName].Name, Equals, compName) + + verifyInstallTasksWithComponents(c, snap.TypeApp, 0, 0, []string{compName}, ts) +} + +func (s *TargetTestSuite) TestInstallWithComponentsMissingResource(c *C) { + s.state.Lock() + defer s.state.Unlock() + + const ( + snapName = "test-snap" + compName = "test-component" + channel = "channel-for-components" + ) + s.fakeStore.snapResourcesFn = func(info *snap.Info) []store.SnapResourceResult { + c.Assert(info.SnapName(), DeepEquals, snapName) + + return []store.SnapResourceResult{ + { + DownloadInfo: snap.DownloadInfo{ + DownloadURL: fmt.Sprintf("http://example.com/%s", snapName), + }, + Name: "missing-component", + Revision: 1, + Type: fmt.Sprintf("component/%s", snap.TestComponent), + Version: "1.0", + CreatedAt: "2024-01-01T00:00:00Z", + }, + } + } + + goal := snapstate.StoreInstallGoal(snapstate.StoreSnap{ + InstanceName: snapName, + Components: []string{compName}, + RevOpts: snapstate.RevisionOptions{ + Channel: channel, + }, + }) + + _, _, err := snapstate.InstallOne(context.Background(), s.state, goal, snapstate.Options{}) + c.Assert(err, ErrorMatches, fmt.Sprintf(`.*cannot find component "%s" in snap resources`, compName)) +} + +func (s *TargetTestSuite) TestInstallWithComponentsWrongType(c *C) { + s.state.Lock() + defer s.state.Unlock() + + const ( + snapName = "test-snap" + compName = "test-component" + channel = "channel-for-components" + ) + s.fakeStore.snapResourcesFn = func(info *snap.Info) []store.SnapResourceResult { + c.Assert(info.SnapName(), DeepEquals, snapName) + + return []store.SnapResourceResult{ + { + DownloadInfo: snap.DownloadInfo{ + DownloadURL: fmt.Sprintf("http://example.com/%s", snapName), + }, + Name: compName, + Revision: 1, + Type: fmt.Sprintf("component/%s", snap.KernelModulesComponent), + Version: "1.0", + CreatedAt: "2024-01-01T00:00:00Z", + }, + } + } + + goal := snapstate.StoreInstallGoal(snapstate.StoreSnap{ + InstanceName: snapName, + Components: []string{compName}, + RevOpts: snapstate.RevisionOptions{ + Channel: channel, + }, + }) + + _, _, err := snapstate.InstallOne(context.Background(), s.state, goal, snapstate.Options{}) + c.Assert(err, ErrorMatches, fmt.Sprintf( + `.*inconsistent component type \("component/%s" in snap, "component/%s" in component\)`, snap.TestComponent, snap.KernelModulesComponent, + )) +} + +func (s *TargetTestSuite) TestInstallWithComponentsMissingInInfo(c *C) { + s.state.Lock() + defer s.state.Unlock() + + const ( + snapName = "test-snap" + compName = "test-missing-component" + channel = "channel-for-components" + ) + s.fakeStore.snapResourcesFn = func(info *snap.Info) []store.SnapResourceResult { + c.Assert(info.SnapName(), DeepEquals, snapName) + + return []store.SnapResourceResult{ + { + DownloadInfo: snap.DownloadInfo{ + DownloadURL: fmt.Sprintf("http://example.com/%s", snapName), + }, + Name: compName, + Revision: 1, + Type: fmt.Sprintf("component/%s", snap.TestComponent), + Version: "1.0", + CreatedAt: "2024-01-01T00:00:00Z", + }, + } + } + + goal := snapstate.StoreInstallGoal(snapstate.StoreSnap{ + InstanceName: snapName, + Components: []string{compName}, + RevOpts: snapstate.RevisionOptions{ + Channel: channel, + }, + }) + + _, _, err := snapstate.InstallOne(context.Background(), s.state, goal, snapstate.Options{}) + c.Assert(err, ErrorMatches, fmt.Sprintf(`.*"%s" is not a component for snap "%s"`, compName, snapName)) +} diff --git a/store/store_action.go b/store/store_action.go index e8acae1c0ae..6ebda7f6594 100644 --- a/store/store_action.go +++ b/store/store_action.go @@ -37,6 +37,8 @@ import ( "github.com/snapcore/snapd/snap" ) +// TODO: rename this type to something more general, since it is used for more +// than just refreshes type RefreshOptions struct { // RefreshManaged indicates to the store that the refresh is // managed via snapd-control. @@ -44,6 +46,10 @@ type RefreshOptions struct { Scheduled bool PrivacyKey string + + // IncludeResources indicates to the store that resources should be included + // in the response. + IncludeResources bool } // snap action: install/refresh @@ -108,10 +114,6 @@ type SnapAction struct { // ValidationSets is an optional array of validation set primary keys // (relevant for install and refresh actions). ValidationSets []snapasserts.ValidationSetKey - // Resources is an optional array of component names. If non-empty, this - // indicates that we should request that the store include snap resource - // information in responses. - Resources []string } func isValidAction(action string) bool { @@ -541,14 +543,10 @@ func (s *Store) snapAction(ctx context.Context, currentSnaps []*CurrentSnap, act } } - // include resources field if any action has requested resources fields := make([]string, len(snapActionFields)) copy(fields, snapActionFields) - for _, a := range actions { - if len(a.Resources) > 0 { - fields = append(fields, "resources") - break - } + if opts.IncludeResources { + fields = append(fields, "resources") } // build input for the install/refresh endpoint diff --git a/store/store_action_test.go b/store/store_action_test.go index f8d2b9bc0a8..7b245400ae2 100644 --- a/store/store_action_test.go +++ b/store/store_action_test.go @@ -206,9 +206,8 @@ func (s *storeActionSuite) testSnapAction(c *C, resources []string) { SnapID: helloWorldSnapID, InstanceName: "hello-world", CohortKey: helloCohortKey, - Resources: resources, }, - }, nil, nil, nil) + }, nil, nil, &store.RefreshOptions{IncludeResources: len(resources) > 0}) c.Assert(err, IsNil) c.Assert(aresults, HasLen, 0) c.Assert(results, HasLen, 1)