Skip to content

Commit

Permalink
o/registrystate: more renames
Browse files Browse the repository at this point in the history 
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
  • Loading branch information
@miguelpires
miguelpires committed Sep 13, 2024
1 parent 3165d09 commit 8774c07
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 67 deletions.
42 changes: 21 additions & 21 deletions overlord/registrystate/registrystate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,23 +229,23 @@ func RegistryTransaction(ctx *hookstate.Context, reg *registry.Registry) (*Trans
}

func createChangeRegistryTasks(st *state.State, chg *state.Change, tx *Transaction, view *registry.View, callingSnap string) error {
managerPlugs, err := getManagerPlugsForView(st, view)
custodianPlugs, err := getCustodianPlugsForView(st, view)
if err != nil {
return err
}

if len(managerPlugs) == 0 {
return fmt.Errorf("cannot commit changes to registry %s/%s: no manager snap installed", view.Registry().Account, view.Registry().Name)
if len(custodianPlugs) == 0 {
return fmt.Errorf("cannot commit changes to registry %s/%s: no custodian snap installed", view.Registry().Account, view.Registry().Name)
}

managerNames := make([]string, 0, len(managerPlugs))
for name := range managerPlugs {
managerNames = append(managerNames, name)
custodianNames := make([]string, 0, len(custodianPlugs))
for name := range custodianPlugs {
custodianNames = append(custodianNames, name)
}

// process the change/save hooks in a deterministic order (useful for testing
// and potentially for the snaps themselves)
sort.Strings(managerNames)
sort.Strings(custodianNames)

var tasks []*state.Task
linkTask := func(t *state.Task) {
Expand All @@ -262,10 +262,10 @@ func createChangeRegistryTasks(st *state.State, chg *state.Change, tx *Transacti

// look for plugs that reference the relevant view and create run-hooks for
// them, if the snap has those hooks
for _, name := range managerNames {
plug := managerPlugs[name]
manager := plug.Snap
if _, ok := manager.Hooks["change-view-"+plug.Name]; !ok {
for _, name := range custodianNames {
plug := custodianPlugs[name]
custodian := plug.Snap
if _, ok := custodian.Hooks["change-view-"+plug.Name]; !ok {
continue
}

Expand All @@ -275,10 +275,10 @@ func createChangeRegistryTasks(st *state.State, chg *state.Change, tx *Transacti
linkTask(chgViewTask)
}

for _, name := range managerNames {
plug := managerPlugs[name]
manager := plug.Snap
if _, ok := manager.Hooks["save-view-"+plug.Name]; !ok {
for _, name := range custodianNames {
plug := custodianPlugs[name]
custodian := plug.Snap
if _, ok := custodian.Hooks["save-view-"+plug.Name]; !ok {
continue
}

Expand Down Expand Up @@ -317,7 +317,7 @@ func createChangeRegistryTasks(st *state.State, chg *state.Change, tx *Transacti
}
}

// commit after managers save ephemeral data
// commit after custodians save ephemeral data
commitTask := st.NewTask("commit-registry-tx", fmt.Sprintf("Commit changes to registry \"%s/%s\"", view.Registry().Account, view.Registry().Name))
commitTask.Set("registry-transaction", tx)
// link all previous tasks to the commit task that carries the transaction
Expand All @@ -334,11 +334,11 @@ func createChangeRegistryTasks(st *state.State, chg *state.Change, tx *Transacti
return nil
}

func getManagerPlugsForView(st *state.State, view *registry.View) (map[string]*snap.PlugInfo, error) {
func getCustodianPlugsForView(st *state.State, view *registry.View) (map[string]*snap.PlugInfo, error) {
repo := ifacerepo.Get(st)
plugs := repo.AllPlugs("registry")

managers := make(map[string]*snap.PlugInfo)
custodians := make(map[string]*snap.PlugInfo)
for _, plug := range plugs {
conns, err := repo.Connected(plug.Snap.InstanceName(), plug.Name)
if err != nil {
Expand All @@ -348,7 +348,7 @@ func getManagerPlugsForView(st *state.State, view *registry.View) (map[string]*s
continue
}

if role, ok := plug.Attrs["role"]; !ok || role != "manager" {
if role, ok := plug.Attrs["role"]; !ok || role != "custodian" {
continue
}

Expand All @@ -365,10 +365,10 @@ func getManagerPlugsForView(st *state.State, view *registry.View) (map[string]*s
// TODO: if a snap has more than one plug providing access to a view, then
// which plug we're getting here becomes unpredictable. We should check
// for this at some point (interface connection?)
managers[plug.Snap.SnapName()] = plug
custodians[plug.Snap.SnapName()] = plug
}

return managers, nil
return custodians, nil
}

func getPlugsAffectedByPaths(st *state.State, registry *registry.Registry, storagePaths []string) (map[string][]*snap.PlugInfo, error) {
Expand Down
92 changes: 46 additions & 46 deletions overlord/registrystate/registrystate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,12 +499,12 @@ slots:
c.Assert(plugNames, testutil.DeepUnsortedMatches, []string{"view-1", "view-3"})
}

func (s *registryTestSuite) TestRegistryTasksUserSetWithManagerInstalled(c *C) {
func (s *registryTestSuite) TestRegistryTasksUserSetWithCustodianInstalled(c *C) {
s.state.Lock()
defer s.state.Unlock()

// only one manager snap is installed
s.setupRegistryModificationScenario(c, []string{"manager-snap"}, nil)
// only one custodian snap is installed
s.setupRegistryModificationScenario(c, []string{"custodian-snap"}, nil)

tx, err := registrystate.NewTransaction(s.state, s.devAccID, "network")
c.Assert(err, IsNil)
Expand All @@ -519,23 +519,23 @@ func (s *registryTestSuite) TestRegistryTasksUserSetWithManagerInstalled(c *C) {
err = registrystate.CreateChangeRegistryTasks(s.state, chg, tx, view, "")
c.Assert(err, IsNil)

// the manager snap's hooks are run
// the custodian snap's hooks are run
tasks := []string{"clear-registry-tx-on-error", "run-hook", "run-hook", "run-hook", "commit-registry-tx", "clear-registry-tx"}
hooks := []*hookstate.HookSetup{
{
Snap: "manager-snap",
Snap: "custodian-snap",
Hook: "change-view-setup",
Optional: true,
IgnoreError: false,
},
{
Snap: "manager-snap",
Snap: "custodian-snap",
Hook: "save-view-setup",
Optional: true,
IgnoreError: false,
},
{
Snap: "manager-snap",
Snap: "custodian-snap",
Hook: "setup-view-changed",
Optional: true,
IgnoreError: true,
Expand All @@ -545,12 +545,12 @@ func (s *registryTestSuite) TestRegistryTasksUserSetWithManagerInstalled(c *C) {
checkModifyRegistryTasks(c, chg, tasks, hooks)
}

func (s *registryTestSuite) TestRegistryTasksManagerSnapSet(c *C) {
func (s *registryTestSuite) TestRegistryTasksCustodianSnapSet(c *C) {
s.state.Lock()
defer s.state.Unlock()

// only one manager snap is installed
s.setupRegistryModificationScenario(c, []string{"manager-snap"}, nil)
// only one custodian snap is installed
s.setupRegistryModificationScenario(c, []string{"custodian-snap"}, nil)

tx, err := registrystate.NewTransaction(s.state, s.devAccID, "network")
c.Assert(err, IsNil)
Expand All @@ -562,20 +562,20 @@ func (s *registryTestSuite) TestRegistryTasksManagerSnapSet(c *C) {
chg := s.state.NewChange("modify-registry", "")

// a user (not a snap) changes a registry
err = registrystate.CreateChangeRegistryTasks(s.state, chg, tx, view, "manager-snap")
err = registrystate.CreateChangeRegistryTasks(s.state, chg, tx, view, "custodian-snap")
c.Assert(err, IsNil)

// the manager snap's hooks are run
// the custodian snap's hooks are run
tasks := []string{"clear-registry-tx-on-error", "run-hook", "run-hook", "commit-registry-tx", "clear-registry-tx"}
hooks := []*hookstate.HookSetup{
{
Snap: "manager-snap",
Snap: "custodian-snap",
Hook: "change-view-setup",
Optional: true,
IgnoreError: false,
},
{
Snap: "manager-snap",
Snap: "custodian-snap",
Hook: "save-view-setup",
Optional: true,
IgnoreError: false,
Expand All @@ -585,12 +585,12 @@ func (s *registryTestSuite) TestRegistryTasksManagerSnapSet(c *C) {
checkModifyRegistryTasks(c, chg, tasks, hooks)
}

func (s *registryTestSuite) TestRegistryTasksObserverSnapSetWithManagerInstalled(c *C) {
func (s *registryTestSuite) TestRegistryTasksObserverSnapSetWithCustodianInstalled(c *C) {
s.state.Lock()
defer s.state.Unlock()

// one manager and several non-managers are installed
s.setupRegistryModificationScenario(c, []string{"manager-snap"}, []string{"test-snap-1", "test-snap-2"})
// one custodian and several non-custodians are installed
s.setupRegistryModificationScenario(c, []string{"custodian-snap"}, []string{"test-snap-1", "test-snap-2"})

tx, err := registrystate.NewTransaction(s.state, s.devAccID, "network")
c.Assert(err, IsNil)
Expand All @@ -601,28 +601,28 @@ func (s *registryTestSuite) TestRegistryTasksObserverSnapSetWithManagerInstalled
view := s.registry.View("setup-wifi")
chg := s.state.NewChange("modify-registry", "")

// a non-manager snap modifies a registry
// a non-custodian snap modifies a registry
err = registrystate.CreateChangeRegistryTasks(s.state, chg, tx, view, "test-snap-1")
c.Assert(err, IsNil)

// we trigger hooks for the manager snap and for the -view-changed for the
// we trigger hooks for the custodian snap and for the -view-changed for the
// observer snap that didn't trigger the change
tasks := []string{"clear-registry-tx-on-error", "run-hook", "run-hook", "run-hook", "run-hook", "commit-registry-tx", "clear-registry-tx"}
hooks := []*hookstate.HookSetup{
{
Snap: "manager-snap",
Snap: "custodian-snap",
Hook: "change-view-setup",
Optional: true,
IgnoreError: false,
},
{
Snap: "manager-snap",
Snap: "custodian-snap",
Hook: "save-view-setup",
Optional: true,
IgnoreError: false,
},
{
Snap: "manager-snap",
Snap: "custodian-snap",
Hook: "setup-view-changed",
Optional: true,
IgnoreError: true,
Expand All @@ -638,26 +638,26 @@ func (s *registryTestSuite) TestRegistryTasksObserverSnapSetWithManagerInstalled
checkModifyRegistryTasks(c, chg, tasks, hooks)
}

func (s *registryTestSuite) TestRegistryTasksDisconnectedManagerSnap(c *C) {
func (s *registryTestSuite) TestRegistryTasksDisconnectedCustodianSnap(c *C) {
s.state.Lock()
defer s.state.Unlock()

// mock and installed manager-snap but disconnect it
s.setupRegistryModificationScenario(c, []string{"test-manager-snap"}, []string{"test-snap"})
s.repo.Disconnect("test-manager-snap", "setup", "core", "registry-slot")
s.testRegistryTasksNoManager(c)
// mock and installed custodian-snap but disconnect it
s.setupRegistryModificationScenario(c, []string{"test-custodian-snap"}, []string{"test-snap"})
s.repo.Disconnect("test-custodian-snap", "setup", "core", "registry-slot")
s.testRegistryTasksNoCustodian(c)
}

func (s *registryTestSuite) TestRegistryTasksNoManagerSnapInstalled(c *C) {
func (s *registryTestSuite) TestRegistryTasksNoCustodianSnapInstalled(c *C) {
s.state.Lock()
defer s.state.Unlock()

// no manager snap is installed
// no custodian snap is installed
s.setupRegistryModificationScenario(c, nil, []string{"test-snap"})
s.testRegistryTasksNoManager(c)
s.testRegistryTasksNoCustodian(c)
}

func (s *registryTestSuite) testRegistryTasksNoManager(c *C) {
func (s *registryTestSuite) testRegistryTasksNoCustodian(c *C) {
tx, err := registrystate.NewTransaction(s.state, s.devAccID, "network")
c.Assert(err, IsNil)

Expand All @@ -667,12 +667,12 @@ func (s *registryTestSuite) testRegistryTasksNoManager(c *C) {
view := s.registry.View("setup-wifi")
chg := s.state.NewChange("modify-registry", "")

// a non-manager snap modifies a registry
// a non-custodian snap modifies a registry
err = registrystate.CreateChangeRegistryTasks(s.state, chg, tx, view, "test-snap-1")
c.Assert(err, ErrorMatches, fmt.Sprintf("cannot commit changes to registry %s/network: no manager snap installed", s.devAccID))
c.Assert(err, ErrorMatches, fmt.Sprintf("cannot commit changes to registry %s/network: no custodian snap installed", s.devAccID))
}

func (s *registryTestSuite) setupRegistryModificationScenario(c *C, managers, nonManagers []string) {
func (s *registryTestSuite) setupRegistryModificationScenario(c *C, custodians, nonCustodians []string) {
s.repo = interfaces.NewRepository()
ifacerepo.Replace(s.state, s.repo)

Expand All @@ -696,7 +696,7 @@ slots:
err = s.repo.AddAppSet(coreSet)
c.Assert(err, IsNil)

mockSnap := func(snapName string, isManager bool) {
mockSnap := func(snapName string, isCustodian bool) {
snapYaml := fmt.Sprintf(`name: %s
version: 1
type: app
Expand All @@ -707,14 +707,14 @@ plugs:
view: network/setup-wifi
`, snapName, s.devAccID)

if isManager {
if isCustodian {
snapYaml +=
` role: manager`
` role: custodian`
}

info := mockInstalledSnap(c, s.state, snapYaml, "")

// by default, mock all the hooks a managers can have
// by default, mock all the hooks a custodians can have
for _, hookName := range []string{"change-view-setup", "save-view-setup", "setup-view-changed"} {
info.Hooks[hookName] = &snap.HookInfo{
Name: hookName,
Expand All @@ -735,16 +735,16 @@ plugs:
c.Assert(err, IsNil)
}

// mock managers
for _, snap := range managers {
isManager := true
mockSnap(snap, isManager)
// mock custodians
for _, snap := range custodians {
isCustodian := true
mockSnap(snap, isCustodian)
}

// mock non-managers
for _, snap := range nonManagers {
isManager := false
mockSnap(snap, isManager)
// mock non-custodians
for _, snap := range nonCustodians {
isCustodian := false
mockSnap(snap, isCustodian)
}
}

Expand Down

0 comments on commit 8774c07

Please sign in to comment.