o, o/snapstate, tests: on classic auto-install snapd as prereq for non-essential snap install

overlord/managers_test.go

@@ -465,6 +465,19 @@ func (s *baseMgrsSuite) SetUpTest(c *C) {
 		},
 	})
 
+	// add snapd itself
+	snapstate.Set(st, "snapd", &snapstate.SnapState{
+		Active: true,
+		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{
+			{RealName: "snapd", SnapID: fakeSnapID("snapd"), Revision: snap.R(1)},
+		}),
+		Current:  snap.R(1),
+		SnapType: "snapd",
+		Flags: snapstate.Flags{
+			Required: true,
+		},
+	})
+
 	// commonly used core and snapd revisions in tests
 	defaultInfoFile := `
 VERSION=2.54.3+git1.g479e745-dirty
@@ -575,6 +588,13 @@ func (s *mgrsSuiteCore) SetUpTest(c *C) {
 	// it panicking.
 	restore := release.MockOnClassic(false)
 	s.baseMgrsSuite.SetUpTest(c)
+
+	// remove snapd snap added for baseMgrsSuite
+	st := s.o.State()
+	st.Lock()
+	snapstate.Set(st, "snapd", nil)
+	st.Unlock()
+
 	s.AddCleanup(restore)
 }
 
@@ -712,9 +732,11 @@ hooks:
 	// nothing in snaps
 	all, err := snapstate.All(st)
 	c.Assert(err, IsNil)
-	c.Check(all, HasLen, 1)
+	c.Check(all, HasLen, 2)
 	_, ok := all["core"]
 	c.Check(ok, Equals, true)
+	_, ok = all["snapd"]
+	c.Check(ok, Equals, true)
 
 	// nothing in config
 	var config map[string]*json.RawMessage
@@ -6061,6 +6083,9 @@ func (s *kernelSuite) SetUpTest(c *C) {
 	st.Lock()
 	defer st.Unlock()
 
+	// remove snapd snap added for baseMgrsSuite
+	snapstate.Set(st, "snapd", nil)
+
 	// create/set custom model assertion
 	model := s.brands.Model("can0nical", "my-model", modelDefaults)
 	devicestatetest.SetDevice(st, &auth.DeviceState{

overlord/snapstate/aliasesv2_test.go

@@ -248,6 +248,7 @@ func (s *snapmgrTestSuite) TestAutoAliasesDeltaAll(c *C) {
 
 	c.Check(seen, DeepEquals, map[string]bool{
 		"core":                    true,
+		"snapd":                   true,
 		"alias-snap":              true,
 		"other-snap":              true,
 		"alias-snap-apps-go-away": true,

overlord/snapstate/backend_test.go

@@ -261,7 +261,7 @@ func (f *fakeStore) snap(spec snapSpec) (*snap.Info, error) {
 	switch spec.Name {
 	case "core", "core16", "ubuntu-core", "some-core":
 		typ = snap.TypeOS
-	case "some-base", "other-base", "some-other-base", "yet-another-base", "core18", "core22":
+	case "some-base", "other-base", "some-other-base", "yet-another-base", "core18", "core20", "core22", "core24", "bare":
 		typ = snap.TypeBase
 	case "some-kernel":
 		typ = snap.TypeKernel

overlord/snapstate/handlers.go

@@ -560,18 +560,23 @@ func (m *SnapManager) installPrereqs(t *state.Task, base string, prereq map[stri
 		}
 	}
 
-	// on systems without core or snapd need to install snapd to
-	// make interfaces work - LP: 1819318
+	// On classic systems that are already seeded, automatically
+	// install snapd snap (covers LP: 1819318). Not allowed for
+	// Ubuntu Core systems - requires remodeling.
 	var tsSnapd *state.TaskSet
 	snapdSnapInstalled, err := isInstalled(st, "snapd")
 	if err != nil {
 		return err
 	}
-	coreSnapInstalled, err := isInstalled(st, "core")
-	if err != nil {
+
+	// consider the state of seeding to avoid seed conflict error
+	var seeded bool
+	err = st.Get("seeded", &seeded)
+	if err != nil && !errors.Is(err, state.ErrNoState) {
 		return err
 	}
-	if base != "core" && !snapdSnapInstalled && !coreSnapInstalled {
+
+	if release.OnClassic && seeded && !snapdSnapInstalled {
 		timings.Run(tm, "install-prereq", "install snapd", func(timings.Measurer) {
 			noTypeBaseCheck := false
 			tsSnapd, err = m.installOneBaseOrRequired(t, "snapd", nil, noTypeBaseCheck, defaultSnapdSnapsChannel(), onInFlightErr, userID, Flags{

overlord/snapstate/handlers_prereq_test.go

@@ -91,13 +91,45 @@ func (s *prereqSuite) SetUpTest(c *C) {
 func (s *prereqSuite) TestDoPrereqNothingToDo(c *C) {
 	s.state.Lock()
 
-	si1 := &snap.SideInfo{
-		RealName: "core",
-		Revision: snap.R(1),
-	}
+	// install snapd so that prerequisites handler won't try to install it
+	snapstate.Set(s.state, "snapd", &snapstate.SnapState{
+		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{
+			{RealName: "snapd", Revision: snap.R(1)},
+		}),
+		Current: snap.R(1),
+	})
+
+	t := s.state.NewTask("prerequisites", "test")
+	t.Set("snap-setup", &snapstate.SnapSetup{
+		SideInfo: &snap.SideInfo{
+			RealName: "foo",
+			Revision: snap.R(33),
+		},
+		Base: "none",
+	})
+	s.state.NewChange("sample", "...").AddTask(t)
+	s.state.Unlock()
+
+	s.se.Ensure()
+	s.se.Wait()
+
+	s.state.Lock()
+	defer s.state.Unlock()
+	c.Assert(s.fakeBackend.ops, HasLen, 0)
+	c.Check(t.Status(), Equals, state.DoneStatus)
+}
+
+func (s *prereqSuite) TestDoPrereqNothingToDoOnCore(c *C) {
+	restore := release.MockOnClassic(false)
+	defer restore()
+
+	s.state.Lock()
+
 	snapstate.Set(s.state, "core", &snapstate.SnapState{
-		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{si1}),
-		Current:  si1.Revision,
+		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{
+			{RealName: "core", Revision: snap.R(1)},
+		}),
+		Current: snap.R(1),
 	})
 
 	t := s.state.NewTask("prerequisites", "test")
@@ -106,6 +138,7 @@ func (s *prereqSuite) TestDoPrereqNothingToDo(c *C) {
 			RealName: "foo",
 			Revision: snap.R(33),
 		},
+		Base: "core",
 	})
 	s.state.NewChange("sample", "...").AddTask(t)
 	s.state.Unlock()
@@ -302,11 +335,23 @@ func (s *prereqSuite) TestDoPrereqTalksToStoreAndQueues(c *C) {
 			},
 			revno: snap.R(11),
 		},
+		{
+			op: "storesvc-snap-action",
+		},
+		{
+			op: "storesvc-snap-action:action",
+			action: store.SnapAction{
+				Action:       "install",
+				InstanceName: "snapd",
+				Channel:      "stable",
+			},
+			revno: snap.R(11),
+		},
 	})
 	c.Check(t.Status(), Equals, state.DoneStatus)
 
 	// check that the do-prereq task added all needed prereqs
-	expectedLinkedSnaps := []string{"prereq1", "prereq2", "some-base"}
+	expectedLinkedSnaps := []string{"prereq1", "prereq2", "some-base", "snapd"}
 	linkedSnaps := make([]string, 0, len(expectedLinkedSnaps))
 	for _, t := range chg.Tasks() {
 		if t.Kind() == "link-snap" {
@@ -363,6 +408,14 @@ func (s *prereqSuite) TestDoPrereqRetryWhenBaseInFlight(c *C) {
 		},
 	})
 
+	// install snapd so that prerequisites handler won't try to install it
+	snapstate.Set(s.state, "snapd", &snapstate.SnapState{
+		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{
+			{RealName: "snapd", Revision: snap.R(1)},
+		}),
+		Current: snap.R(1),
+	})
+
 	// pretend foo gets installed and needs core (which is in progress)
 	prereqTask = s.state.NewTask("prerequisites", "foo")
 	prereqTask.Set("snap-setup", &snapstate.SnapSetup{
@@ -480,10 +533,11 @@ func (s *prereqSuite) TestDoPrereqChannelEnvvars(c *C) {
 	defer os.Unsetenv("SNAPD_PREREQS_CHANNEL")
 	s.state.Lock()
 
-	snapstate.Set(s.state, "core", &snapstate.SnapState{
+	// install snapd so that prerequisites handler won't try to install it
+	snapstate.Set(s.state, "snapd", &snapstate.SnapState{
 		Active: true,
 		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{
-			{RealName: "core", Revision: snap.R(1)},
+			{RealName: "snapd", Revision: snap.R(1)},
 		}),
 		Current:  snap.R(1),
 		SnapType: "os",
@@ -602,7 +656,10 @@ func (s *prereqSuite) TestDoPrereqNothingToDoForSnapdSnap(c *C) {
 	s.state.Unlock()
 }
 
-func (s *prereqSuite) TestDoPrereqCore16wCoreNothingToDo(c *C) {
+func (s *prereqSuite) TestDoPrereqCore16WithCoreNothingToDoOnCore(c *C) {
+	restore := release.MockOnClassic(false)
+	defer restore()
+
 	s.state.Lock()
 
 	si1 := &snap.SideInfo{
@@ -634,12 +691,8 @@ func (s *prereqSuite) TestDoPrereqCore16wCoreNothingToDo(c *C) {
 	c.Check(t.Status(), Equals, state.DoneStatus)
 }
 
-func (s *prereqSuite) testDoPrereqNoCorePullsInSnaps(c *C, base string) {
-	restore := release.MockOnClassic(true)
-	defer restore()
-
+func (s *prereqSuite) testDoPrereqBasePullsInSnapd(c *C, base string) {
 	s.state.Lock()
-
 	t := s.state.NewTask("prerequisites", "test")
 	t.Set("snap-setup", &snapstate.SnapSetup{
 		SideInfo: &snap.SideInfo{
@@ -687,16 +740,32 @@ func (s *prereqSuite) testDoPrereqNoCorePullsInSnaps(c *C, base string) {
 	c.Check(t.Status(), Equals, state.DoneStatus)
 }
 
-func (s *prereqSuite) TestDoPrereqCore16noCore(c *C) {
-	s.testDoPrereqNoCorePullsInSnaps(c, "core16")
+func (s *prereqSuite) TestDoPrereqCorePullsInSnapd(c *C) {
+	s.testDoPrereqBasePullsInSnapd(c, "core")
+}
+
+func (s *prereqSuite) TestDoPrereqCore16PullsInSnapd(c *C) {
+	s.testDoPrereqBasePullsInSnapd(c, "core16")
 }
 
-func (s *prereqSuite) TestDoPrereqCore18NoCorePullsInSnapd(c *C) {
-	s.testDoPrereqNoCorePullsInSnaps(c, "core18")
+func (s *prereqSuite) TestDoPrereqCore18PullsInSnapd(c *C) {
+	s.testDoPrereqBasePullsInSnapd(c, "core18")
 }
 
-func (s *prereqSuite) TestDoPrereqOtherBaseNoCorePullsInSnapd(c *C) {
-	s.testDoPrereqNoCorePullsInSnaps(c, "some-base")
+func (s *prereqSuite) TestDoPrereqCore20PullsInSnapd(c *C) {
+	s.testDoPrereqBasePullsInSnapd(c, "core20")
+}
+
+func (s *prereqSuite) TestDoPrereqCore22PullsInSnapd(c *C) {
+	s.testDoPrereqBasePullsInSnapd(c, "core22")
+}
+
+func (s *prereqSuite) TestDoPrereqCore24PullsInSnapd(c *C) {
+	s.testDoPrereqBasePullsInSnapd(c, "core24")
+}
+
+func (s *prereqSuite) TestDoPrereqOtherBasePullsInSnapd(c *C) {
+	s.testDoPrereqBasePullsInSnapd(c, "other-base")
 }
 
 func (s *prereqSuite) TestDoPrereqBaseIsNotBase(c *C) {
@@ -914,17 +983,6 @@ func (s *prereqSuite) TestDoPrereqSkipDuringRemodel(c *C) {
 	// ChangeConflictError, which is then ignored, making this test invalid
 	snapstate.ReplaceStore(s.state, storetest.Store{})
 
-	// install snapd so that prerequisites handler won't try to install it
-	snapstate.Set(s.state, "snapd", &snapstate.SnapState{
-		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{
-			{
-				RealName: "snapd",
-				Revision: snap.R(1),
-			},
-		}),
-		Current: snap.R(1),
-	})
-
 	prereqTask := s.state.NewTask("prerequisites", "test")
 	prereqTask.Set("snap-setup", &snapstate.SnapSetup{
 		SideInfo: &snap.SideInfo{

overlord/snapstate/snapstate_install_test.go

@@ -386,6 +386,9 @@ func (s *snapmgrTestSuite) TestInstallSnapdSnapTypeOnClassic(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
 
+	// remove snapd snap added for snapmgrBaseTest
+	snapstate.Set(s.state, "snapd", nil)
+
 	// setup a classic model so the device context says we are on classic
 	defer snapstatetest.MockDeviceModel(ClassicModel())()
 
@@ -407,6 +410,9 @@ func (s *snapmgrTestSuite) TestInstallSnapdSnapTypeOnCore(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
 
+	// remove snapd snap added for snapmgrBaseTest
+	snapstate.Set(s.state, "snapd", nil)
+
 	opts := &snapstate.RevisionOptions{Channel: "some-channel"}
 	ts, err := snapstate.Install(context.Background(), s.state, "snapd", opts, 0, snapstate.Flags{})
 	c.Assert(err, IsNil)
@@ -530,6 +536,9 @@ func (s *snapmgrTestSuite) TestInstallWithDeviceContextNoRemodelConflict(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
 
+	// remove snapd snap added for snapmgrBaseTest
+	snapstate.Set(s.state, "snapd", nil)
+
 	// unset the global store, it will need to come via the device context
 	snapstate.ReplaceStore(s.state, nil)
 
@@ -926,6 +935,9 @@ func (s *snapmgrTestSuite) TestInstallSnapdConflict(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()
 
+	// remove snapd snap added for snapmgrBaseTest
+	snapstate.Set(s.state, "snapd", nil)
+
 	tugc := s.state.NewTask("update-gadget-cmdline", "update gadget cmdline")
 	chg := s.state.NewChange("optional-kernel-cmdline", "optional kernel cmdline")
 	chg.AddTask(tugc)