overlord: accept a SeedRefreshCandidate to check if both snaps and components can be removed

Author: natibek

overlord/devicestate/devicestate.go

@@ -298,7 +298,6 @@ func delayedCrossMgrInit() {
 	snapstate.SeedRefreshTasks = SeedRefreshTasks
 	snapstate.UpdateSeedRefreshChange = UpdateSeedRefreshChange
 	snapstate.CheckSeedRefreshRemove = CheckSeedRefreshRemove
-	snapstate.CheckComponentSeedRefreshRemove = CheckComponentSeedRefreshRemove
 }
 
 // proxyStore returns the store assertion for the proxy store if one is set.
@@ -1862,40 +1861,21 @@ func appendSeedRefreshCandidate(create *state.Task, snapSetupTasks []string, com
 	return setTaskRecoverySystemSetup(create, setup)
 }
 
-// CheckSeedRefreshRemove prevents removing optional snaps that are still
-// present in the current seed while seed-refresh is enabled.
+// CheckSeedRefreshRemove prevents removing optional snaps and components 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 {
+func CheckSeedRefreshRemove(st *state.State, candidate snapstate.SeedRefreshCandidate, dctx snapstate.DeviceContext) error {
 	filter := seedRefreshFilter(st, dctx)
-	_, seedRefreshTriggered, err := filter(snapstate.SeedRefreshCandidate{
-		InstanceName: si.SnapName(),
-	})
-	if err != nil {
-		return err
-	}
+	_, seedRefreshTriggered, err := filter(candidate)
 
-	if seedRefreshTriggered {
-		return errors.New("cannot remove snap present in the current seed while seed-refresh is enabled")
-	}
-	return nil
-}
-
-// CheckComponentSeedRefreshRemove is set by devicestate to prevent removal of
-// components that must remain present for seed-refresh.
-var CheckComponentSeedRefreshRemove = func(st *state.State, si *snap.Info, componentName string, dctx snapstate.DeviceContext) error {
-	filter := seedRefreshFilter(st, dctx)
-	_, seedRefreshTriggered, err := filter(snapstate.SeedRefreshCandidate{
-		InstanceName:          si.SnapName(),
-		ComponentSetupTaskIDs: map[string]string{componentName: ""},
-	})
 	if err != nil {
 		return err
 	}
 
 	if seedRefreshTriggered {
-		return errors.New("cannot remove component present in the current seed while seed-refresh is enabled")
+		return errors.New("cannot remove snap present in the current seed while seed-refresh is enabled")
 	}
 	return nil
 }

overlord/devicestate/devicestate_test.go

@@ -2855,9 +2855,10 @@ func (s *deviceMgrSuite) TestCheckSeedRefreshRemoveBlocksOptionalSnapInCurrentSe
 		{"name": "pc", "type": "gadget", "default-channel": "24"},
 		{"name": "snap-2", "presence": "optional"},
 	}, nil, "snap-2")
-	info := snaptest.MockInfo(c, "name: snap-2\nversion: 1", nil)
-
-	err := devicestate.CheckSeedRefreshRemove(s.state, info, dctx)
+	candidate := snapstate.SeedRefreshCandidate{
+		InstanceName: "snap-2",
+	}
+	err := devicestate.CheckSeedRefreshRemove(s.state, candidate, dctx)
 	c.Assert(err, ErrorMatches, `cannot remove snap present in the current seed while seed-refresh is enabled`)
 }
 
@@ -2872,9 +2873,10 @@ func (s *deviceMgrSuite) TestCheckSeedRefreshRemoveAllowsOptionalSnapNotInCurren
 		{"name": "pc", "type": "gadget", "default-channel": "24"},
 		{"name": "snap-2", "presence": "optional"},
 	}, nil)
-	info := snaptest.MockInfo(c, "name: snap-2\nversion: 1", nil)
-
-	err := devicestate.CheckSeedRefreshRemove(s.state, info, dctx)
+	candidate := snapstate.SeedRefreshCandidate{
+		InstanceName: "snap-2",
+	}
+	err := devicestate.CheckSeedRefreshRemove(s.state, candidate, dctx)
 	c.Assert(err, IsNil)
 }
 

overlord/snapstate/component.go

@@ -827,7 +827,16 @@ func canRemoveComponent(st *state.State, compst *sequence.ComponentState, info *
 		return err
 	}
 	if seedRefresh {
-		if err := CheckComponentSeedRefreshRemove(st, info, compst.SideInfo.Component.ComponentName, deviceCtx); err != nil {
+		// Construct a component exclusive candidate with the component
+		// to test if the component would trigger a seed refresh.
+		// The task id is not involved in the filtering when checking
+		// if a component triggers a seed, so it is fine to leave
+		// the value in the ComponentSetupTaskIDs field empty.
+		candidate := SeedRefreshCandidate{
+			InstanceName:          info.InstanceName(),
+			ComponentSetupTaskIDs: map[string]string{compst.SideInfo.Component.ComponentName: ""},
+		}
+		if err := CheckSeedRefreshRemove(st, candidate, deviceCtx); err != nil {
 			return err
 		}
 	}

overlord/snapstate/component_remove_test.go

@@ -480,8 +480,8 @@ func (s *snapmgrTestSuite) TestRemoveComponentInSeedRefresh(c *C) {
 	c.Assert(tr.Set("core", "experimental.seed-refresh", true), IsNil)
 	tr.Commit()
 
-	s.AddCleanup(snapstate.MockCheckComponentSeedRefreshRemove(func(st *state.State,
-		si *snap.Info, componentName string, dctx snapstate.DeviceContext) error {
+	s.AddCleanup(snapstate.MockCheckSeedRefreshRemove(func(*state.State,
+		snapstate.SeedRefreshCandidate, snapstate.DeviceContext) error {
 		return fmt.Errorf("blocked by seed refresh")
 	}))
 

overlord/snapstate/export_test.go

@@ -80,14 +80,6 @@ func MockReadComponentInfo(mock func(compMntDir string, snapInfo *snap.Info, csi
 	return func() { readComponentInfoAt = old }
 }
 
-func MockCheckComponentSeedRefreshRemove(mock func(st *state.State, si *snap.Info, componentName string, dctx DeviceContext) error) (restore func()) {
-	old := CheckComponentSeedRefreshRemove
-	CheckComponentSeedRefreshRemove = mock
-	return func() {
-		CheckComponentSeedRefreshRemove = old
-	}
-}
-
 func MockMountPollInterval(intv time.Duration) (restore func()) {
 	old := mountPollInterval
 	mountPollInterval = intv
@@ -439,7 +431,7 @@ 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()) {
+func MockCheckSeedRefreshRemove(fn func(st *state.State, candidate SeedRefreshCandidate, dctx DeviceContext) error) (restore func()) {
 	r := testutil.Backup(&CheckSeedRefreshRemove)
 	CheckSeedRefreshRemove = fn
 	return r

overlord/snapstate/seed.go

@@ -26,7 +26,6 @@ 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.
@@ -76,16 +75,10 @@ var UpdateSeedRefreshChange = func(chg *state.Change, dctx DeviceContext, candid
 //
 // 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 {
+var CheckSeedRefreshRemove = func(st *state.State, candidate SeedRefreshCandidate, dctx DeviceContext) error {
 	panic("internal error: snapstate.CheckSeedRefreshRemove is unset")
 }
 
-// CheckComponentSeedRefreshRemove is set by devicestate to prevent removal of
-// components that must remain present for seed-refresh.
-var CheckComponentSeedRefreshRemove = func(st *state.State, si *snap.Info, componentName string, dctx DeviceContext) error {
-	panic("internal error: snapstate.CheckComponentSeedRefreshRemove is unset")
-}
-
 func seedRefreshCandidateForTaskSet(ts *state.TaskSet) (SeedRefreshCandidate, error) {
 	t, err := ts.Edge(SnapSetupEdge)
 	if err != nil {

overlord/snapstate/snapstate.go

@@ -3094,7 +3094,8 @@ func canRemove(st *state.State, si *snap.Info, snapst *SnapState, removeAll bool
 		return err
 	}
 	if seedRefresh && removeAll {
-		if err := CheckSeedRefreshRemove(st, si, deviceCtx); err != nil {
+		candidate := SeedRefreshCandidate{InstanceName: si.InstanceName()}
+		if err := CheckSeedRefreshRemove(st, candidate, deviceCtx); err != nil {
 			return err
 		}
 	}

overlord/snapstate/snapstate_remove_test.go

@@ -1361,7 +1361,7 @@ func (s *snapmgrTestSuite) TestRemoveConsultsSeedRefreshRemoveHookOnlyWhenEnable
 	})
 
 	called := false
-	restore := snapstate.MockCheckSeedRefreshRemove(func(*state.State, *snap.Info, snapstate.DeviceContext) error {
+	restore := snapstate.MockCheckSeedRefreshRemove(func(*state.State, snapstate.SeedRefreshCandidate, snapstate.DeviceContext) error {
 		called = true
 		return errors.New("blocked by test hook")
 	})
@@ -1405,7 +1405,7 @@ func (s *snapmgrTestSuite) TestRemoveSpecificRevisionDoesNotConsultSeedRefreshRe
 	tr.Commit()
 
 	called := false
-	restore := snapstate.MockCheckSeedRefreshRemove(func(*state.State, *snap.Info, snapstate.DeviceContext) error {
+	restore := snapstate.MockCheckSeedRefreshRemove(func(*state.State, snapstate.SeedRefreshCandidate, snapstate.DeviceContext) error {
 		called = true
 		return errors.New("blocked by test hook")
 	})

overlord/snapstate/snapstate_test.go

@@ -303,7 +303,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 }))
+	s.AddCleanup(snapstate.MockCheckSeedRefreshRemove(func(*state.State, snapstate.SeedRefreshCandidate, snapstate.DeviceContext) error { return nil }))
 	_, restore := mockSeedRefreshHooks(nil)
 	s.AddCleanup(restore)