o/snapstate: prevent removal of seed-refresh snaps when seed-refresh is enabled

overlord/devicestate/devicestate.go

@@ -300,6 +300,7 @@ func delayedCrossMgrInit() {
 	snapstate.RemodelingChange = RemodelingChange
 	snapstate.SeedRefreshTasks = SeedRefreshTasks
 	snapstate.UpdateSeedRefreshChange = UpdateSeedRefreshChange
+	snapstate.CheckSeedRefreshRemove = CheckSeedRefreshRemove
 }
 
 // proxyStore returns the store assertion for the proxy store if one is set.
@@ -1879,6 +1880,24 @@ func appendSeedRefreshCandidate(create *state.Task, snapSetupTasks, compSetupTas
 	return setTaskRecoverySystemSetup(create, setup)
 }
 
+// CheckSeedRefreshRemove prevents removing optional snaps that are still
+// present in the current seed while seed-refresh is enabled.
+//
+// TODO:SEEDREFRESH: remove this once we support seed-refresh seeds
+// gaining/losing snaps
+func CheckSeedRefreshRemove(st *state.State, si *snap.Info, dctx snapstate.DeviceContext) error {
+	triggers := seedRefreshTriggers(st, dctx)
+	ok, err := triggers(si.SnapName())
+	if err != nil {
+		return err
+	}
+
+	if ok {
+		return errors.New("cannot remove snap present in the current seed while seed-refresh is enabled")
+	}
+	return nil
+}
+
 // seedRefreshTriggers returns a closure that reports whether the given snap
 // should trigger a seed refresh. The seed is lazily loaded, and only opened
 // when required.

overlord/devicestate/devicestate_test.go

@@ -2517,6 +2517,40 @@ func (s *deviceMgrSuite) TestUpdateSeedRefreshChangeSkipsOptionalSnapNotInCurren
 	c.Check(setup.SnapSetupTasks, DeepEquals, []string{snap1Task.ID()})
 }
 
+func (s *deviceMgrSuite) TestCheckSeedRefreshRemoveBlocksOptionalSnapInCurrentSeed(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	dctx := s.setupSeedRefreshSeedAndContext(c, []map[string]string{
+		{"name": "snapd", "type": "snapd"},
+		{"name": "core24", "type": "base", "default-channel": "24"},
+		{"name": "pc-kernel", "type": "kernel", "default-channel": "24"},
+		{"name": "pc", "type": "gadget", "default-channel": "24"},
+		{"name": "snap-2", "presence": "optional"},
+	}, "snap-2")
+	info := snaptest.MockInfo(c, "name: snap-2\nversion: 1", nil)
+
+	err := devicestate.CheckSeedRefreshRemove(s.state, info, dctx)
+	c.Assert(err, ErrorMatches, `cannot remove snap present in the current seed while seed-refresh is enabled`)
+}
+
+func (s *deviceMgrSuite) TestCheckSeedRefreshRemoveAllowsOptionalSnapNotInCurrentSeed(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	dctx := s.setupSeedRefreshSeedAndContext(c, []map[string]string{
+		{"name": "snapd", "type": "snapd"},
+		{"name": "core24", "type": "base", "default-channel": "24"},
+		{"name": "pc-kernel", "type": "kernel", "default-channel": "24"},
+		{"name": "pc", "type": "gadget", "default-channel": "24"},
+		{"name": "snap-2", "presence": "optional"},
+	})
+	info := snaptest.MockInfo(c, "name: snap-2\nversion: 1", nil)
+
+	err := devicestate.CheckSeedRefreshRemove(s.state, info, dctx)
+	c.Assert(err, IsNil)
+}
+
 func (s *deviceMgrSuite) TestUpdateSeedRefreshChangeUsesPendingSeedRefreshTasks(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()

overlord/snapstate/export_test.go

@@ -432,6 +432,12 @@ func MockRefreshAppsCheck(fn func(info *snap.Info) error) (restore func()) {
 	return func() { refreshAppsCheck = old }
 }
 
+func MockCheckSeedRefreshRemove(fn func(st *state.State, si *snap.Info, dctx DeviceContext) error) (restore func()) {
+	r := testutil.Backup(&CheckSeedRefreshRemove)
+	CheckSeedRefreshRemove = fn
+	return r
+}
+
 func (m *autoRefresh) EnsureRefreshHoldAtLeast(d time.Duration) error {
 	return m.ensureRefreshHoldAtLeast(d)
 }

overlord/snapstate/seed.go

@@ -26,6 +26,7 @@ import (
 	"github.com/snapcore/snapd/features"
 	"github.com/snapcore/snapd/overlord/configstate/config"
 	"github.com/snapcore/snapd/overlord/state"
+	"github.com/snapcore/snapd/snap"
 )
 
 // SeedRefreshTaskSet carries the tasks needed to perform a seed refresh.
@@ -61,6 +62,15 @@ var UpdateSeedRefreshChange = func(chg *state.Change, dctx DeviceContext, candid
 	panic("internal error: snapstate.UpdateSeedRefreshChange is unset")
 }
 
+// CheckSeedRefreshRemove is set by devicestate to prevent removal of snaps that
+// must remain present for seed-refresh.
+//
+// TODO:SEEDREFRESH: remove this hook once seed-refresh supports seeds
+// gaining/losing snaps
+var CheckSeedRefreshRemove = func(st *state.State, si *snap.Info, dctx DeviceContext) error {
+	panic("internal error: snapstate.CheckSeedRefreshRemove is unset")
+}
+
 func seedRefreshCandidateForTaskSet(ts *state.TaskSet) (SeedRefreshCandidate, error) {
 	t, err := ts.Edge(SnapSetupEdge)
 	if err != nil {

overlord/snapstate/snapstate.go

@@ -3089,6 +3089,16 @@ func canRemove(st *state.State, si *snap.Info, snapst *SnapState, removeAll bool
 		return err
 	}
 
+	seedRefresh, err := seedRefreshEnabled(st)
+	if err != nil {
+		return err
+	}
+	if seedRefresh && removeAll {
+		if err := CheckSeedRefreshRemove(st, si, deviceCtx); err != nil {
+			return err
+		}
+	}
+
 	// check if this snap is required by any validation set in enforcing mode
 	enforcedSets, err := EnforcedValidationSets(st)
 	if err != nil {

overlord/snapstate/snapstate_remove_test.go

@@ -1298,6 +1298,78 @@ func (s *snapmgrTestSuite) TestRemoveRefusedLastRevision(c *C) {
 	c.Check(err, ErrorMatches, `snap "brand-gadget" is not removable: snap is used by the model`)
 }
 
+func (s *snapmgrTestSuite) TestRemoveConsultsSeedRefreshRemoveHookOnlyWhenEnabled(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	si := snap.SideInfo{
+		RealName: "some-snap",
+		Revision: snap.R(7),
+	}
+
+	snapstate.Set(s.state, "some-snap", &snapstate.SnapState{
+		Active:   true,
+		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{&si}),
+		Current:  si.Revision,
+		SnapType: "app",
+	})
+
+	called := false
+	restore := snapstate.MockCheckSeedRefreshRemove(func(*state.State, *snap.Info, snapstate.DeviceContext) error {
+		called = true
+		return errors.New("blocked by test hook")
+	})
+	defer restore()
+
+	_, err := snapstate.Remove(s.state, "some-snap", snap.R(0), nil)
+	c.Assert(err, IsNil)
+	c.Check(called, Equals, false)
+
+	tr := config.NewTransaction(s.state)
+	c.Assert(tr.Set("core", "experimental.seed-refresh", true), IsNil)
+	tr.Commit()
+
+	_, err = snapstate.Remove(s.state, "some-snap", snap.R(0), nil)
+	c.Assert(err, ErrorMatches, `snap "some-snap" is not removable: blocked by test hook`)
+	c.Check(called, Equals, true)
+}
+
+func (s *snapmgrTestSuite) TestRemoveSpecificRevisionDoesNotConsultSeedRefreshRemoveHook(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	si2 := snap.SideInfo{
+		RealName: "some-snap",
+		Revision: snap.R(2),
+	}
+	si1 := snap.SideInfo{
+		RealName: "some-snap",
+		Revision: snap.R(1),
+	}
+
+	snapstate.Set(s.state, "some-snap", &snapstate.SnapState{
+		Active:   true,
+		Sequence: snapstatetest.NewSequenceFromSnapSideInfos([]*snap.SideInfo{&si2, &si1}),
+		Current:  si2.Revision,
+		SnapType: "app",
+	})
+
+	tr := config.NewTransaction(s.state)
+	c.Assert(tr.Set("core", "experimental.seed-refresh", true), IsNil)
+	tr.Commit()
+
+	called := false
+	restore := snapstate.MockCheckSeedRefreshRemove(func(*state.State, *snap.Info, snapstate.DeviceContext) error {
+		called = true
+		return errors.New("blocked by test hook")
+	})
+	defer restore()
+
+	_, err := snapstate.Remove(s.state, "some-snap", snap.R(1), nil)
+	c.Assert(err, IsNil)
+	c.Check(called, Equals, false)
+}
+
 func (s *snapmgrTestSuite) TestRemoveDeletesConfigOnLastRevision(c *C) {
 	si := snap.SideInfo{
 		RealName: "some-snap",

overlord/snapstate/snapstate_test.go

@@ -301,6 +301,7 @@ func (s *snapmgrBaseTest) SetUpTest(c *C) {
 	snapstate.SetupRemoveHook = hookstate.SetupRemoveHook
 	snapstate.SnapServiceOptions = servicestate.SnapServiceOptions
 	snapstate.EnsureSnapAbsentFromQuotaGroup = servicestate.EnsureSnapAbsentFromQuota
+	s.AddCleanup(snapstate.MockCheckSeedRefreshRemove(func(*state.State, *snap.Info, snapstate.DeviceContext) error { return nil }))
 	_, restore := mockSeedRefreshHooks(nil)
 	s.AddCleanup(restore)