diff --git a/overlord/devicestate/devicestate.go b/overlord/devicestate/devicestate.go index 088cdfc3dac..3b2d40a23d4 100644 --- a/overlord/devicestate/devicestate.go +++ b/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. diff --git a/overlord/devicestate/devicestate_test.go b/overlord/devicestate/devicestate_test.go index 4cfa393933f..dd9ec51a9ca 100644 --- a/overlord/devicestate/devicestate_test.go +++ b/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() diff --git a/overlord/snapstate/export_test.go b/overlord/snapstate/export_test.go index 8eacf715d71..4a24bc618cb 100644 --- a/overlord/snapstate/export_test.go +++ b/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) } diff --git a/overlord/snapstate/seed.go b/overlord/snapstate/seed.go index e2d36c0755b..322165e5a3c 100644 --- a/overlord/snapstate/seed.go +++ b/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 { diff --git a/overlord/snapstate/snapstate.go b/overlord/snapstate/snapstate.go index a75cffe7de8..7d7c2b1e97b 100644 --- a/overlord/snapstate/snapstate.go +++ b/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 { diff --git a/overlord/snapstate/snapstate_remove_test.go b/overlord/snapstate/snapstate_remove_test.go index 1fe86f34b3e..b47bf066141 100644 --- a/overlord/snapstate/snapstate_remove_test.go +++ b/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", diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index 935d23c2eff..5dbd065a5f5 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/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)