o/devicestate: refactor remodeling code to use new snapstate API (can…

…onical#14898)

* o/devicestate, overlord, daemon: refactor devicestate.Remodel api to take in a type that can wrap local snaps and components

* o/devicestate: remove usage of type that can be replaced by type that contains components

* o/devicestate: change remodeling to use new snapstate API and refactor in preparation for components

The refactor makes it so all of the decisions about what to do for a
snap in the incoming model are all in the same place. I feel that this
is pretty useful now, and it will simplify adding the logic that is
needed for handling components as well.

* o/snapstate, o/devicestate, daemon: introduce snapstate.PathComponent that is used in snapstate.PathSnap

* o/devicestate: eliminate some types that can be replaced with types from snapstate

* o/devicestate: update some comments on the remodeler and methods

* o/devicestate, o/snapstate: use snapstate.PathSnap as part of snapstate.PathInstallGoal api

* o/devicestate: rename remodelTarget to remodelSnapTarget

* o/snapstate: move PathComponent definition

* o/devicestate: do not change a snap's channel during a remodel if the current snap is not tracking a channel

* o/devicestate: remove unused function

daemon/api_model.go

@@ -38,7 +38,6 @@ import (
 	"github.com/snapcore/snapd/overlord/auth"
 	"github.com/snapcore/snapd/overlord/devicestate"
 	"github.com/snapcore/snapd/overlord/state"
-	"github.com/snapcore/snapd/snap"
 )
 
 var (
@@ -118,7 +117,7 @@ func remodelJSON(c *Command, r *http.Request) Response {
 	st.Lock()
 	defer st.Unlock()
 
-	chg, err := devicestateRemodel(st, newModel, nil, nil, devicestate.RemodelOptions{
+	chg, err := devicestateRemodel(st, newModel, nil, devicestate.RemodelOptions{
 		Offline: data.Offline,
 	})
 	if err != nil {
@@ -187,8 +186,7 @@ func startOfflineRemodelChange(st *state.State, newModel *asserts.Model,
 	}
 
 	*pathsToNotRemove = make([]string, 0, len(slInfo.snaps))
-	sideInfos := make([]*snap.SideInfo, 0, len(slInfo.snaps))
-	paths := make([]string, 0, len(slInfo.snaps))
+	localSnaps := make([]devicestate.LocalSnap, 0, len(slInfo.snaps))
 	for _, psi := range slInfo.snaps {
 		// Move file to the same name of what a downloaded one would have
 		dest := filepath.Join(dirs.SnapBlobDir,
@@ -197,12 +195,14 @@ func startOfflineRemodelChange(st *state.State, newModel *asserts.Model,
 		// Avoid trying to remove a file that does not exist anymore
 		*pathsToNotRemove = append(*pathsToNotRemove, psi.tmpPath)
 
-		sideInfos = append(sideInfos, &psi.info.SideInfo)
-		paths = append(paths, dest)
+		localSnaps = append(localSnaps, devicestate.LocalSnap{
+			SideInfo: &psi.info.SideInfo,
+			Path:     dest,
+		})
 	}
 
 	// Now create and start the remodel change
-	chg, err := devicestateRemodel(st, newModel, sideInfos, paths, devicestate.RemodelOptions{
+	chg, err := devicestateRemodel(st, newModel, localSnaps, devicestate.RemodelOptions{
 		// since this is the codepath that parses the form, offline is implcit
 		// because local snaps are being provided.
 		Offline: true,

daemon/api_model_test.go

@@ -133,7 +133,7 @@ func (s *modelSuite) testPostRemodel(c *check.C, offline bool) {
 	defer restore()
 
 	var devicestateRemodelGotModel *asserts.Model
-	defer daemon.MockDevicestateRemodel(func(st *state.State, nm *asserts.Model, localSnaps []*snap.SideInfo, paths []string, opts devicestate.RemodelOptions) (*state.Change, error) {
+	defer daemon.MockDevicestateRemodel(func(st *state.State, nm *asserts.Model, localSnaps []devicestate.LocalSnap, opts devicestate.RemodelOptions) (*state.Change, error) {
 		c.Check(opts.Offline, check.Equals, offline)
 		devicestateRemodelGotModel = nm
 		chg := st.NewChange("remodel", "...")
@@ -600,12 +600,12 @@ func (s *modelSuite) testPostOfflineRemodel(c *check.C, params *testPostOfflineR
 	snapRev := 1001
 	var devicestateRemodelGotModel *asserts.Model
 	defer daemon.MockDevicestateRemodel(func(st *state.State, nm *asserts.Model,
-		localSnaps []*snap.SideInfo, paths []string, opts devicestate.RemodelOptions) (*state.Change, error) {
+		localSnaps []devicestate.LocalSnap, opts devicestate.RemodelOptions) (*state.Change, error) {
 		c.Check(opts.Offline, check.Equals, true)
 		c.Check(len(localSnaps), check.Equals, 1)
-		c.Check(localSnaps[0].RealName, check.Equals, snapName)
-		c.Check(localSnaps[0].Revision, check.Equals, snap.Revision{N: snapRev})
-		c.Check(strings.HasSuffix(paths[0],
+		c.Check(localSnaps[0].SideInfo.RealName, check.Equals, snapName)
+		c.Check(localSnaps[0].SideInfo.Revision, check.Equals, snap.Revision{N: snapRev})
+		c.Check(strings.HasSuffix(localSnaps[0].Path,
 			"/var/lib/snapd/snaps/"+snapName+"_"+strconv.Itoa(snapRev)+".snap"),
 			check.Equals, true)
 

daemon/api_sideload_n_try.go

@@ -375,9 +375,12 @@ func sideloadTaskSets(ctx context.Context, st *state.State, sideload *sideloaded
 	// handle everything else
 	var pathSnaps []snapstate.PathSnap
 	for _, sn := range sideload.snaps {
-		comps := make(map[*snap.ComponentSideInfo]string, len(sn.components))
+		comps := make([]snapstate.PathComponent, 0, len(sn.components))
 		for _, ci := range sn.components {
-			comps[ci.sideInfo] = ci.tmpPath
+			comps = append(comps, snapstate.PathComponent{
+				SideInfo: ci.sideInfo,
+				Path:     ci.tmpPath,
+			})
 		}
 
 		pathSnaps = append(pathSnaps, snapstate.PathSnap{

daemon/api_sideload_n_try_test.go

@@ -1566,9 +1566,9 @@ func (s *sideloadSuite) testSideloadManySnapsAndComponents(c *check.C, opts side
 			comps, ok := expectedSnapsToComps[sn.SideInfo.RealName]
 			c.Assert(ok, check.Equals, true, check.Commentf("unexpected snap name %q", sn.SideInfo.RealName))
 			foundComps := make([]string, 0, len(comps))
-			for csi := range sn.Components {
-				c.Check(csi.Revision.Unset(), check.Equals, true)
-				foundComps = append(foundComps, csi.Component.ComponentName)
+			for _, comp := range sn.Components {
+				c.Check(comp.SideInfo.Revision.Unset(), check.Equals, true)
+				foundComps = append(foundComps, comp.SideInfo.Component.ComponentName)
 			}
 			c.Check(foundComps, testutil.DeepUnsortedMatches, comps)
 
@@ -1733,16 +1733,16 @@ func (s *sideloadSuite) TestSideloadManyAssertedSnapsAndComponents(c *check.C) {
 			comps, ok := snapsToComps[sn.SideInfo.RealName]
 			c.Assert(ok, check.Equals, true, check.Commentf("unexpected snap name %q", sn.SideInfo.RealName))
 			foundComps := make([]string, 0, len(comps))
-			for csi, compPath := range sn.Components {
-				c.Check(csi.Revision.Unset(), check.Equals, false)
-				foundComps = append(foundComps, csi.Component.ComponentName)
+			for _, comp := range sn.Components {
+				c.Check(comp.SideInfo.Revision.Unset(), check.Equals, false)
+				foundComps = append(foundComps, comp.SideInfo.Component.ComponentName)
 
-				container, err := snapfile.Open(compPath)
+				container, err := snapfile.Open(comp.Path)
 				c.Assert(err, check.IsNil)
 				ci, err := snap.ReadComponentInfoFromContainer(container, nil, nil)
 				c.Assert(err, check.IsNil)
 
-				c.Check(ci.Component, check.Equals, csi.Component)
+				c.Check(ci.Component, check.Equals, comp.SideInfo.Component)
 			}
 			c.Check(foundComps, testutil.DeepUnsortedMatches, comps)
 

daemon/export_api_model_test.go

@@ -23,10 +23,9 @@ import (
 	"github.com/snapcore/snapd/asserts"
 	"github.com/snapcore/snapd/overlord/devicestate"
 	"github.com/snapcore/snapd/overlord/state"
-	"github.com/snapcore/snapd/snap"
 )
 
-func MockDevicestateRemodel(mock func(*state.State, *asserts.Model, []*snap.SideInfo, []string, devicestate.RemodelOptions) (*state.Change, error)) (restore func()) {
+func MockDevicestateRemodel(mock func(*state.State, *asserts.Model, []devicestate.LocalSnap, devicestate.RemodelOptions) (*state.Change, error)) (restore func()) {
 	oldDevicestateRemodel := devicestateRemodel
 	devicestateRemodel = mock
 	return func() {