many: introduce and use snap.SelfContainedSetPrereqTracker (canonical…

…#13340)

* o/snapstate,snap: introduce snap.SelfContainedSetPrereqTracker

It is meant to be used when dealing with a self-contained set of snaps, with no
desire to fetch further snaps, so all prerequisites must be present in the set
itself. This applies to first boot seeding and remodeling for example.

* many: use snap.SelfContainedSetPrereqTracker

also in snap.ValidateBasesAndProviders

these now can produce warnings, OTOH the relaxed checks allow to build/seed an
image even if a content requirement is fulfilled by an alternative provider

notice that with the relaxed checks seeding might fail or the system not work
if the right auto-connections or connections are not in-place

snaps used as default-providers usually have been taken care of already but it
might be up to the user to ask to set that up for alternative ones

* snap: clarify some names

* many: rename and clarify to PrereqTracker.MissingProviderContentTags

* seed/seedwriter: clarify TODO

---------

Co-authored-by: Ernest Lotter <ernest.lotter@canonical.com>

image/image_test.go

@@ -2800,7 +2800,8 @@ func (s *imageSuite) TestSetupSeedMissingContentProvider(c *C) {
 	}
 
 	err := image.SetupSeed(s.tsto, model, opts)
-	c.Check(err, ErrorMatches, `prerequisites need to be added explicitly: cannot use snap "snap-req-content-provider": default provider "gtk-common-themes" is missing`)
+	// XXX content is empty because we disable SanitizePlugsSlots
+	c.Check(err, ErrorMatches, `prerequisites need to be added explicitly: cannot use snap "snap-req-content-provider": default provider "gtk-common-themes" or any alternative provider for content "" is missing`)
 }
 
 func (s *imageSuite) TestSetupSeedClassic(c *C) {

overlord/devicestate/firstboot.go

@@ -262,10 +262,7 @@ func (m *DeviceManager) populateStateFromSeedImpl(tm timings.Measurer) ([]*state
 
 	infoToTs := make(map[*snap.Info]*state.TaskSet, len(essentialSeedSnaps))
 
-	// XXX TODO: switch to a prereq tracker that supports properly
-	// a self-contained set of snaps, here we cannot nor want
-	// to go back to the store for prereqs anyway
-	prqt := snap.SimplePrereqTracker{}
+	prqt := snap.NewSelfContainedSetPrereqTracker()
 
 	if len(essentialSeedSnaps) != 0 {
 		// we *always* configure "core" here even if bases are used
@@ -336,13 +333,18 @@ func (m *DeviceManager) populateStateFromSeedImpl(tm timings.Measurer) ([]*state
 		infoToTs[info] = ts
 	}
 
-	// validate that all snaps have bases
-	// XXX this will use the PrereqTracker
-	errs := snap.ValidateBasesAndProviders(infos)
+	// validate that all snaps have bases and providers are fulfilled
+	// using the PrereqTracker
+	warns, errs := prqt.Check()
 	if errs != nil {
 		// only report the first error encountered
 		return nil, errs[0]
 	}
+	// XXX do better, use the warnings to setup checks at end of the seeding
+	// and log onlys plug not connected or explicitly disconnected there
+	for _, w := range warns {
+		logger.Noticef("seed prerequisites: %v", w)
+	}
 
 	// now add/chain the tasksets in the right order, note that we
 	// only have tasksets that we did not already seeded

overlord/devicestate/firstboot_test.go

@@ -40,6 +40,7 @@ import (
 	"github.com/snapcore/snapd/bootloader/bootloadertest"
 	"github.com/snapcore/snapd/dirs"
 	"github.com/snapcore/snapd/gadget"
+	"github.com/snapcore/snapd/logger"
 	"github.com/snapcore/snapd/osutil"
 	"github.com/snapcore/snapd/overlord"
 	"github.com/snapcore/snapd/overlord/assertstate"
@@ -1681,6 +1682,103 @@ snaps:
 	c.Check(conn.(map[string]interface{})["interface"], Equals, "content")
 }
 
+func (s *firstBoot16Suite) TestPopulateFromSeedAlternativeContentProviderAndOrder(c *C) {
+	logbuf, restore := logger.MockLogger()
+	defer restore()
+
+	bloader := boottest.MockUC16Bootenv(bootloadertest.Mock("mock", c.MkDir()))
+	bootloader.Force(bloader)
+	defer bootloader.Force(nil)
+	bloader.SetBootKernel("pc-kernel_1.snap")
+	bloader.SetBootBase("core_1.snap")
+
+	coreFname, kernelFname, gadgetFname := s.makeCoreSnaps(c, "")
+
+	// a snap that uses content providers
+	snapYaml := `name: gnome-calculator
+version: 1.0
+plugs:
+ gtk-3-themes:
+  interface: content
+  default-provider: gtk-common-themes
+  target: $SNAP/data-dir/themes
+`
+	calcFname, calcDecl, calcRev := s.MakeAssertedSnap(c, snapYaml, nil, snap.R(128), "developerid")
+	s.WriteAssertions("calc.asserts", s.devAcct, calcRev, calcDecl)
+
+	// put a 2nd firstboot snap into the SnapBlobDir
+	snapYaml = `name: gtk-common-themes-alt
+version: 1.0
+slots:
+ gtk-3-themes:
+  interface: content
+  source:
+   read:
+    - $SNAP/share/themes/Adawaita
+`
+	themesFname, themesDecl, themesRev := s.MakeAssertedSnap(c, snapYaml, nil, snap.R(65), "developerid")
+	s.WriteAssertions("themes.asserts", s.devAcct, themesDecl, themesRev)
+
+	// add a model assertion and its chain
+	assertsChain := s.makeModelAssertionChain(c, "my-model", nil)
+	s.WriteAssertions("model.asserts", assertsChain...)
+
+	// create a seed.yaml
+	content := []byte(fmt.Sprintf(`
+snaps:
+ - name: core
+   file: %s
+ - name: pc-kernel
+   file: %s
+ - name: pc
+   file: %s
+ - name: gnome-calculator
+   file: %s
+ - name: gtk-common-themes-alt
+   file: %s
+`, coreFname, kernelFname, gadgetFname, calcFname, themesFname))
+	err := os.WriteFile(filepath.Join(dirs.SnapSeedDir, "seed.yaml"), content, 0644)
+	c.Assert(err, IsNil)
+
+	// run the firstboot stuff
+	s.startOverlord(c)
+	st := s.overlord.State()
+	st.Lock()
+	defer st.Unlock()
+
+	tsAll, err := devicestate.PopulateStateFromSeedImpl(s.overlord.DeviceManager(), s.perfTimings)
+	c.Assert(err, IsNil)
+	// use the expected kind otherwise settle with start another one
+	chg := st.NewChange("seed", "run the populate from seed changes")
+	for _, ts := range tsAll {
+		chg.AddAll(ts)
+	}
+	c.Assert(st.Changes(), HasLen, 1)
+
+	// avoid device reg
+	chg1 := st.NewChange("become-operational", "init device")
+	chg1.SetStatus(state.DoingStatus)
+
+	st.Unlock()
+	err = s.overlord.Settle(settleTimeout)
+	st.Lock()
+
+	c.Assert(chg.Err(), IsNil)
+	c.Assert(err, IsNil)
+
+	// verify the result
+	var conns map[string]interface{}
+	err = st.Get("conns", &conns)
+	c.Assert(err, IsNil)
+	c.Check(conns, HasLen, 1)
+	conn, hasConn := conns["gnome-calculator:gtk-3-themes gtk-common-themes-alt:gtk-3-themes"]
+	c.Check(hasConn, Equals, true)
+	c.Check(conn.(map[string]interface{})["auto"], Equals, true)
+	c.Check(conn.(map[string]interface{})["interface"], Equals, "content")
+
+	c.Check(logbuf.String(), Matches, `(?sm).*seed prerequisites: snap "gnome-calculator" requires a provider for content "gtk-3-themes", a candidate slot is available \(gtk-common-themes-alt:gtk-3-themes\) but not the default-provider, ensure a single auto-connection \(or possibly a connection\) is in-place.*`)
+}
+
 func (s *firstBoot16Suite) TestPopulateFromSeedMissingBase(c *C) {
 	s.WriteAssertions("developer.account", s.devAcct)
 

overlord/snapstate/snapstate.go

@@ -973,13 +973,13 @@ func IsErrAndNotWait(err error) bool {
 
 // defaultProviderContentAttrs takes a snap.Info and returns a map of
 // default providers to the value of content attributes they should
-// provide. Content attributes already provided by a snap in the system are omitted.
+// provide. Content attributes already provided by a snap in the system are omitted. What is returned depends on the behavior of the passed PrereqTracker.
 func defaultProviderContentAttrs(st *state.State, info *snap.Info, prqt PrereqTracker) map[string][]string {
 	if prqt == nil {
 		prqt = snap.SimplePrereqTracker{}
 	}
 	repo := ifacerepo.Get(st)
-	return prqt.DefaultProviderContentAttrs(info, repo)
+	return prqt.MissingProviderContentTags(info, repo)
 }
 
 func getKeys(kv map[string][]string) []string {
@@ -1100,13 +1100,19 @@ func ensureInstallPreconditions(st *state.State, info *snap.Info, flags Flags, s
 type PrereqTracker interface {
 	// Add adds a snap for tracking.
 	Add(*snap.Info)
-	// DefaultProviderContentAttrs returns a map keyed by the names of all
-	// default-providers for the content plugs that the given snap.Info
-	// needs. The map values are the corresponding content tags.
-	// If repo is not nil, any content tag provided by an existing slot in it
-	// is considered already available and filtered out from the result.
-	// info might or might have not been passed already to Add.
-	DefaultProviderContentAttrs(info *snap.Info, repo snap.InterfaceRepo) map[string][]string
+	// MissingProviderContetTags returns a map keyed by the names of all
+	// missing default-providers for the content plugs that the given
+	// snap.Info needs. The map values are the corresponding content tags.
+	// Different prerequisites trackers might decide in different
+	// ways which providers are missing. Either making assumptions about
+	// the snap operations that are being set up or considering
+	// just the snap info and repo.
+	// In the latter case if repo is not nil, any content tag provided by
+	// an existing slot in it should be considered already available and
+	// filtered out from the result. info might or might have not been
+	// passed already to Add. snapstate uses the result to decide to
+	// install providers automatically.
+	MissingProviderContentTags(info *snap.Info, repo snap.InterfaceRepo) map[string][]string
 }
 
 // addPrereq adds the given prerequisite snap to the tracker, if the tracker is

overlord/snapstate/snapstate_install_test.go

@@ -363,7 +363,7 @@ func (s *snapmgrTestSuite) TestInstallWithDeviceContext(c *C) {
 
 	c.Assert(prqt.infos, HasLen, 1)
 	c.Check(prqt.infos[0].SnapName(), Equals, "some-snap")
-	c.Check(prqt.defaultProviderContentAttrsCalls, Equals, 1)
+	c.Check(prqt.missingProviderContentTagsCalls, Equals, 1)
 }
 
 func (s *snapmgrTestSuite) TestInstallPathWithDeviceContext(c *C) {
@@ -391,7 +391,7 @@ version: 1.0
 
 	c.Assert(prqt.infos, HasLen, 1)
 	c.Check(prqt.infos[0].SnapName(), Equals, "some-snap")
-	c.Check(prqt.defaultProviderContentAttrsCalls, Equals, 1)
+	c.Check(prqt.missingProviderContentTagsCalls, Equals, 1)
 }
 
 func (s *snapmgrTestSuite) TestInstallPathWithDeviceContextBadFile(c *C) {
@@ -2116,7 +2116,7 @@ version: 1.0`)
 
 	c.Assert(prqt.infos, HasLen, 1)
 	c.Check(prqt.infos[0], DeepEquals, info)
-	c.Check(prqt.defaultProviderContentAttrsCalls, Equals, 1)
+	c.Check(prqt.missingProviderContentTagsCalls, Equals, 1)
 
 	chg.AddAll(ts)
 

overlord/snapstate/snapstate_update_test.go

@@ -4257,20 +4257,20 @@ func (s *snapmgrTestSuite) TestUpdateWithDeviceContext(c *C) {
 
 	c.Assert(prqt.infos, HasLen, 1)
 	c.Check(prqt.infos[0].SnapName(), Equals, "some-snap")
-	c.Check(prqt.defaultProviderContentAttrsCalls, Equals, 1)
+	c.Check(prqt.missingProviderContentTagsCalls, Equals, 1)
 }
 
 type testPrereqTracker struct {
-	infos                            []*snap.Info
-	defaultProviderContentAttrsCalls int
+	infos                           []*snap.Info
+	missingProviderContentTagsCalls int
 }
 
 func (prqt *testPrereqTracker) Add(info *snap.Info) {
 	prqt.infos = append(prqt.infos, info)
 }
 
-func (prqt *testPrereqTracker) DefaultProviderContentAttrs(*snap.Info, snap.InterfaceRepo) map[string][]string {
-	prqt.defaultProviderContentAttrsCalls++
+func (prqt *testPrereqTracker) MissingProviderContentTags(*snap.Info, snap.InterfaceRepo) map[string][]string {
+	prqt.missingProviderContentTagsCalls++
 	return nil
 }
 
@@ -4306,7 +4306,7 @@ version: 1.0
 	c.Assert(s.state.TaskCount(), Equals, len(ts.Tasks()))
 	c.Assert(prqt.infos, HasLen, 1)
 	c.Check(prqt.infos[0].SnapName(), Equals, "some-snap")
-	c.Check(prqt.defaultProviderContentAttrsCalls, Equals, 1)
+	c.Check(prqt.missingProviderContentTagsCalls, Equals, 1)
 }
 
 func (s *snapmgrTestSuite) TestUpdatePathWithDeviceContextSwitchChannel(c *C) {

seed/seedwriter/writer.go

@@ -1277,16 +1277,24 @@ func (w *Writer) checkPrereqsInMode(mode string) error {
 	for _, sn := range w.byModeSnaps[mode] {
 		snaps = append(snaps, sn.Info)
 	}
-	if errs := snap.ValidateBasesAndProviders(snaps); errs != nil {
+	warns, errs := snap.ValidateBasesAndProviders(snaps)
+	if errs != nil {
 		var errPrefix string
-		// XXX do better in terms of error structuring
+		// XXX TODO: return an error that subsumes all the errors
 		if mode == "run" {
 			errPrefix = "prerequisites need to be added explicitly"
 		} else {
 			errPrefix = fmt.Sprintf("prerequisites need to be added explicitly for relevant mode %s", mode)
 		}
 		return fmt.Errorf("%s: %v", errPrefix, errs[0])
 	}
+	wfmt := "%v"
+	if mode != "run" {
+		wfmt = fmt.Sprintf("prerequisites for mode %s: %%v", mode)
+	}
+	for _, warn := range warns {
+		w.warningf(wfmt, warn)
+	}
 	return nil
 }
 

seed/seedwriter/writer_test.go

@@ -168,6 +168,15 @@ plugs:
 type: app
 version: 1
 confinement: devmode
+`,
+	"alt-cont-producer": `name: alt-cont-producer
+type: app
+base: core18
+version: 1.1
+slots:
+   serve-cont:
+     interface: content
+     content: cont
 `,
 })
 
@@ -724,7 +733,7 @@ func (s *writerSuite) TestDownloadedMissingDefaultProvider(c *C) {
 	s.makeSnap(c, "cont-consumer", "developerid")
 
 	_, _, err := s.upToDownloaded(c, model, s.fillDownloadedSnap, s.fetchAsserts(c))
-	c.Check(err, ErrorMatches, `prerequisites need to be added explicitly: cannot use snap "cont-consumer": default provider "cont-producer" is missing`)
+	c.Check(err, ErrorMatches, `prerequisites need to be added explicitly: cannot use snap "cont-consumer": default provider "cont-producer" or any alternative provider for content "cont" is missing`)
 }
 
 func (s *writerSuite) TestDownloadedCheckType(c *C) {
@@ -2685,7 +2694,70 @@ func (s *writerSuite) TestDownloadedCore20MissingDefaultProviderModes(c *C) {
 
 	s.opts.Label = "20191003"
 	_, _, err := s.upToDownloaded(c, model, s.fillDownloadedSnap, s.fetchAsserts(c))
-	c.Check(err, ErrorMatches, `prerequisites need to be added explicitly for relevant mode recover: cannot use snap "cont-consumer": default provider "cont-producer" is missing`)
+	c.Check(err, ErrorMatches, `prerequisites need to be added explicitly for relevant mode recover: cannot use snap "cont-consumer": default provider "cont-producer" or any alternative provider for content "cont" is missing`)
+}
+
+func (s *writerSuite) TestDownloadedCore20AlternativeProviderModes(c *C) {
+	model := s.Brands.Model("my-brand", "my-model", map[string]interface{}{
+		"display-name": "my model",
+		"architecture": "amd64",
+		"store":        "my-store",
+		"base":         "core20",
+		"snaps": []interface{}{
+			map[string]interface{}{
+				"name":            "pc-kernel",
+				"id":              s.AssertedSnapID("pc-kernel"),
+				"type":            "kernel",
+				"default-channel": "20",
+			},
+			map[string]interface{}{
+				"name":            "pc",
+				"id":              s.AssertedSnapID("pc"),
+				"type":            "gadget",
+				"default-channel": "20",
+			},
+			map[string]interface{}{
+				"name":  "core18",
+				"id":    s.AssertedSnapID("core18"),
+				"type":  "base",
+				"modes": []interface{}{"run", "ephemeral"},
+			},
+			map[string]interface{}{
+				"name": "cont-producer",
+				"id":   s.AssertedSnapID("cont-producer"),
+			},
+			map[string]interface{}{
+				"name":  "cont-consumer",
+				"id":    s.AssertedSnapID("cont-consumer"),
+				"modes": []interface{}{"recover"},
+			},
+			map[string]interface{}{
+				"name":  "alt-cont-producer",
+				"id":    s.AssertedSnapID("alt-cont-producer"),
+				"modes": []interface{}{"recover"},
+			},
+		},
+	})
+
+	// validity
+	c.Assert(model.Grade(), Equals, asserts.ModelSigned)
+
+	s.makeSnap(c, "snapd", "")
+	s.makeSnap(c, "core20", "")
+	s.makeSnap(c, "core18", "")
+	s.makeSnap(c, "pc-kernel=20", "")
+	s.makeSnap(c, "pc=20", "")
+	s.makeSnap(c, "cont-producer", "developerid")
+	s.makeSnap(c, "cont-consumer", "developerid")
+	s.makeSnap(c, "alt-cont-producer", "developerid")
+
+	s.opts.Label = "20191003"
+	complete, w, err := s.upToDownloaded(c, model, s.fillDownloadedSnap, s.fetchAsserts(c))
+	c.Assert(err, IsNil)
+	c.Check(complete, Equals, true)
+	warns := w.Warnings()
+	c.Assert(warns, HasLen, 1)
+	c.Check(warns[0], Matches, `prerequisites for mode recover: snap "cont-consumer" requires a provider for content "cont", a candidate slot is available \(alt-cont-producer:serve-cont\) but not the default-provider, ensure a single auto-connection \(or possibly a connection\) is in-place`)
 }
 
 func (s *writerSuite) TestCore20NonDangerousDisallowedDevmodeSnaps(c *C) {

seed/validate.go

@@ -124,7 +124,8 @@ func ValidateFromYaml(seedYamlFile string) error {
 		return nil
 	})
 
-	if errs2 := snap.ValidateBasesAndProviders(snapInfos); errs2 != nil {
+	// TODO: surface the warnings too, like we do for snap container checks
+	if _, errs2 := snap.ValidateBasesAndProviders(snapInfos); errs2 != nil {
 		ve.addErr("", errs2...)
 	}
 	if ve.hasErrors() {