Skip to content

Commit

Permalink
o/snapstate, daemon: allow installing components for already installe…
Browse files Browse the repository at this point in the history 
…d snap (canonical#14260)

* o/snapstate: explicitly handle discard tasks in componentInstallTaskSet

* o/snapstate: allow installing components for a snap that is already installed

* daemon: move code around to create a place for installing components for already installed snaps

* o/snapstate: add TODO:COMPS about using transaction

* daemon: allow snapd API to install components from the store for already installed snaps

* tests: spread test installing a component for a snap that is already installed

* o/snapstate, store: do not return ErrNoUpdateAvailable for snaps that have had a resource revision change

* daemon: lift variable initialization out of loop

* daemon: rename some tests for clarity

* o/snapstate: move function closer to similar functions

* store: add comment explaining how we decide if a snap has no updates or not

* o/snapstate: add TODO:COMPS about testing branch when losing comps during refresh

* store: add doc comment on ResourceInstall field

* o/snapstate, snap: add and use snap.AlreadyInstalledComponentError

* o/snapstate: add tests for conflict behavior for installing components

* o/snapstate: add params for splicing setup-profiles and setup-kernel-modules-components tasks into chain built by doInstallComponent

* o/snapstate: manually add setup-profiles and setup-kernel-modules-components tasks into chain to avoid adding inaccurate data to the tasks

* o/snapstate: add some clarifying comments

* o/snapstate: fix typo in comment
  • Loading branch information
@andrewphelpsj
andrewphelpsj authored Aug 8, 2024
1 parent db09bc8 commit c999099
Show file tree
Hide file tree
Showing 16 changed files with 1,053 additions and 148 deletions.
2 changes: 1 addition & 1 deletion daemon/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,11 @@ func storeFrom(d *Daemon) snapstate.StoreService {

var (
snapstateStoreInstallGoal = snapstate.StoreInstallGoal
snapstateInstallOne = snapstate.InstallOne
snapstateInstallWithGoal = snapstate.InstallWithGoal
snapstateInstallPath = snapstate.InstallPath
snapstateInstallPathMany = snapstate.InstallPathMany
snapstateInstallComponentPath = snapstate.InstallComponentPath
snapstateInstallComponents = snapstate.InstallComponents
snapstateRefreshCandidates = snapstate.RefreshCandidates
snapstateTryPath = snapstate.TryPath
snapstateUpdate = snapstate.Update
Expand Down
12 changes: 6 additions & 6 deletions daemon/api_aliases_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,11 +590,11 @@ func (s *aliasesSuite) TestAliases(c *check.C) {
func (s *aliasesSuite) TestInstallUnaliased(c *check.C) {
var calledFlags snapstate.Flags

defer daemon.MockSnapstateInstallOne(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) (*snap.Info, *state.TaskSet, error) {
defer daemon.MockSnapstateInstallWithGoal(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) ([]*snap.Info, []*state.TaskSet, error) {
calledFlags = opts.Flags

t := st.NewTask("fake-install-snap", "Doing a fake install")
return &snap.Info{}, state.NewTaskSet(t), nil
return []*snap.Info{{}}, []*state.TaskSet{state.NewTaskSet(t)}, nil
})()

d := s.daemon(c)
Expand All @@ -617,11 +617,11 @@ func (s *aliasesSuite) TestInstallUnaliased(c *check.C) {
func (s *aliasesSuite) TestInstallIgnoreRunning(c *check.C) {
var calledFlags snapstate.Flags

defer daemon.MockSnapstateInstallOne(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) (*snap.Info, *state.TaskSet, error) {
defer daemon.MockSnapstateInstallWithGoal(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) ([]*snap.Info, []*state.TaskSet, error) {
calledFlags = opts.Flags

t := st.NewTask("fake-install-snap", "Doing a fake install")
return &snap.Info{}, state.NewTaskSet(t), nil
return []*snap.Info{{}}, []*state.TaskSet{state.NewTaskSet(t)}, nil
})()

d := s.daemon(c)
Expand All @@ -644,11 +644,11 @@ func (s *aliasesSuite) TestInstallIgnoreRunning(c *check.C) {
func (s *aliasesSuite) TestInstallPrefer(c *check.C) {
var calledFlags snapstate.Flags

defer daemon.MockSnapstateInstallOne(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) (*snap.Info, *state.TaskSet, error) {
defer daemon.MockSnapstateInstallWithGoal(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) ([]*snap.Info, []*state.TaskSet, error) {
calledFlags = opts.Flags

t := st.NewTask("fake-install-snap", "Doing a fake install")
return &snap.Info{}, state.NewTaskSet(t), nil
return []*snap.Info{{}}, []*state.TaskSet{state.NewTaskSet(t)}, nil
})()

d := s.daemon(c)
Expand Down
8 changes: 4 additions & 4 deletions daemon/api_sideload_n_try_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func (s *sideloadSuite) sideloadCheck(c *check.C, content string, head map[strin
return &snap.Info{SuggestedName: mockedName}, nil
})()

defer daemon.MockSnapstateInstallOne(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) (*snap.Info, *state.TaskSet, error) {
defer daemon.MockSnapstateInstallWithGoal(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) ([]*snap.Info, []*state.TaskSet, error) {
goal, ok := g.(*storeInstallGoalRecorder)
c.Assert(ok, check.Equals, true, check.Commentf("unexpected InstallGoal type %T", g))
c.Assert(goal.snaps, check.HasLen, 1)
Expand All @@ -214,7 +214,7 @@ func (s *sideloadSuite) sideloadCheck(c *check.C, content string, head map[strin
installQueue = append(installQueue, goal.snaps[0].InstanceName)

t := st.NewTask("fake-install-snap", "Doing a fake install")
return &snap.Info{}, state.NewTaskSet(t), nil
return []*snap.Info{{}}, []*state.TaskSet{state.NewTaskSet(t)}, nil
})()

defer daemon.MockSnapstateInstallPath(func(s *state.State, si *snap.SideInfo, path, name, channel string, flags snapstate.Flags, prqt snapstate.PrereqTracker) (*state.TaskSet, *snap.Info, error) {
Expand Down Expand Up @@ -1315,7 +1315,7 @@ func (s *trySuite) TestTrySnap(c *check.C) {
return state.NewTaskSet(t), nil
})()

defer daemon.MockSnapstateInstallOne(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) (*snap.Info, *state.TaskSet, error) {
defer daemon.MockSnapstateInstallWithGoal(func(ctx context.Context, st *state.State, g snapstate.InstallGoal, opts snapstate.Options) ([]*snap.Info, []*state.TaskSet, error) {
goal, ok := g.(*storeInstallGoalRecorder)
c.Assert(ok, check.Equals, true, check.Commentf("unexpected InstallGoal type %T", g))
c.Assert(goal.snaps, check.HasLen, 1)
Expand All @@ -1325,7 +1325,7 @@ func (s *trySuite) TestTrySnap(c *check.C) {
}

t := st.NewTask("fake-install-snap", "Doing a fake install")
return &snap.Info{}, state.NewTaskSet(t), nil
return []*snap.Info{{}}, []*state.TaskSet{state.NewTaskSet(t)}, nil
})()

// try the snap (without an installed core)
Expand Down
118 changes: 77 additions & 41 deletions daemon/api_snaps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,11 +463,6 @@ func snapInstall(ctx context.Context, inst *snapInstruction, st *state.State) (s
return "", nil, fmt.Errorf(i18n.G("cannot install snap with empty name"))
}

flags, err := inst.installFlags()
if err != nil {
return "", nil, err
}

var ckey string
if inst.CohortKey == "" {
logger.Noticef("Installing snap %q revision %s", inst.Snaps[0], inst.Revision)
Expand All @@ -476,18 +471,12 @@ func snapInstall(ctx context.Context, inst *snapInstruction, st *state.State) (s
logger.Noticef("Installing snap %q from cohort %q", inst.Snaps[0], ckey)
}

// TODO:COMPS: handle installing a component of a snap that is already
// installed
goal := storeInstallGoalFromInstruction(inst)
_, tset, err := snapstateInstallOne(ctx, st, goal, snapstate.Options{
UserID: inst.userID,
Flags: flags,
})
_, tss, err := installationTaskSets(ctx, st, inst)
if err != nil {
return "", nil, err
}

return installMessage(inst, ckey), []*state.TaskSet{tset}, nil
return installMessage(inst, ckey), tss, nil
}

func installMessage(inst *snapInstruction, cohort string) string {
Expand Down Expand Up @@ -848,58 +837,105 @@ func (inst *snapInstruction) dispatchForMany() (op snapManyActionFunc) {
return op
}

func storeInstallGoalFromInstruction(inst *snapInstruction) snapstate.InstallGoal {
snaps := make([]snapstate.StoreSnap, 0, len(inst.Snaps))
for _, sn := range inst.Snaps {
// we currently only allow revision options when installing one snap
opts := snapstate.RevisionOptions{}
if len(inst.Snaps) == 1 {
opts = *inst.revnoOpts()
func installationTaskSets(ctx context.Context, st *state.State, inst *snapInstruction) ([]*snap.Info, []*state.TaskSet, error) {
expectOneSnap := len(inst.Snaps) == 1
opts := snapstate.Options{
UserID: inst.userID,
ExpectOneSnap: expectOneSnap,
}

if expectOneSnap {
flags, err := inst.installFlags()
if err != nil {
return nil, nil, err
}
opts.Flags = flags
} else {
opts.Flags.Transaction = inst.Transaction
}

revOpts := snapstate.RevisionOptions{}
if expectOneSnap {
revOpts = *inst.revnoOpts()
}

var (
tss []*state.TaskSet
snaps []snapstate.StoreSnap
infos []*snap.Info
)
for _, name := range inst.Snaps {
var snapst snapstate.SnapState
if err := snapstate.Get(st, name, &snapst); err != nil && !errors.Is(err, state.ErrNoState) {
return nil, nil, err
}

comps := inst.CompsForSnaps[name]

if snapst.IsInstalled() && len(comps) > 0 {
info, err := snapst.CurrentInfo()
if err != nil {
return nil, nil, err
}

ts, err := snapstateInstallComponents(ctx, st, comps, info, opts)
if err != nil {
return nil, nil, err
}

infos = append(infos, info)
tss = append(tss, ts...)

continue
}

snaps = append(snaps, snapstate.StoreSnap{
InstanceName: sn,
Components: inst.CompsForSnaps[sn],
RevOpts: opts,
InstanceName: name,
Components: comps,
RevOpts: revOpts,
})
}

return snapstateStoreInstallGoal(snaps...)
// this means that we're installing a set of components for one snap that is
// already installed
if len(snaps) == 0 {
return infos, tss, nil
}

installed, ts, err := snapstateInstallWithGoal(ctx, st, snapstateStoreInstallGoal(snaps...), opts)
if err != nil {
return nil, nil, err
}

infos = append(infos, installed...)
tss = append(tss, ts...)

return infos, tss, nil
}

func snapInstallMany(ctx context.Context, inst *snapInstruction, st *state.State) (*snapInstructionResult, error) {
for _, name := range inst.Snaps {
if len(name) == 0 {
return nil, fmt.Errorf(i18n.G("cannot install snap with empty name"))
return nil, errors.New(i18n.G("cannot install snap with empty name"))
}
}

// TODO:COMPS: handle installing a component of a snap that is already
// installed
goal := storeInstallGoalFromInstruction(inst)
installed, tasksets, err := snapstateInstallWithGoal(ctx, st, goal, snapstate.Options{
UserID: inst.userID,
Flags: snapstate.Flags{
Transaction: inst.Transaction,
},
})
if err != nil {
return nil, err
}

if len(inst.Snaps) == 0 {
return nil, fmt.Errorf("cannot install zero snaps")
return nil, errors.New(i18n.G("cannot install zero snaps"))
}

msg := multiInstallMessage(inst)
installed, tasksets, err := installationTaskSets(ctx, st, inst)
if err != nil {
return nil, err
}

names := make([]string, 0, len(installed))
for _, sn := range installed {
names = append(names, sn.InstanceName())
}

return &snapInstructionResult{
Summary: msg,
Summary: multiInstallMessage(inst),
Affected: names,
Tasksets: tasksets,
}, nil
Expand Down
Loading

0 comments on commit c999099

Please sign in to comment.