o/registrystate: create modify-registry tasks (canonical#14445)

* o/registrystate: create modify-registry tasks

Adds a helper that attaches the appropriate tasks to gather approval,
save, commit and notify of a change to a registry. The specific approval
gathering mechanism is described in SD133. This doesn't include the
task/hook handlers that deal with the specific but the whole picture can
be seen in canonical#14283

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

* o/registrystate: make task kinds more specific

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

* o/registrystate: minor fix

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

---------

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

overlord/registrystate/export_test.go

@@ -24,9 +24,10 @@ import (
 )
 
 var (
-	ReadDatabag             = readDatabag
-	WriteDatabag            = writeDatabag
-	GetPlugsAffectedByPaths = getPlugsAffectedByPaths
+	ReadDatabag               = readDatabag
+	WriteDatabag              = writeDatabag
+	GetPlugsAffectedByPaths   = getPlugsAffectedByPaths
+	CreateChangeRegistryTasks = createChangeRegistryTasks
 )
 
 func MockReadDatabag(f func(st *state.State, account, registryName string) (registry.JSONDataBag, error)) func() {

overlord/registrystate/registrymgr.go

@@ -0,0 +1,39 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+/*
+ * Copyright (C) 2024 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package registrystate
+
+import (
+	"fmt"
+
+	"github.com/snapcore/snapd/i18n"
+	"github.com/snapcore/snapd/overlord/hookstate"
+	"github.com/snapcore/snapd/overlord/state"
+)
+
+func setupRegistryHook(st *state.State, snapName, hookName string, ignoreError bool) *state.Task {
+	hookSup := &hookstate.HookSetup{
+		Snap:        snapName,
+		Hook:        hookName,
+		Optional:    true,
+		IgnoreError: ignoreError,
+	}
+	summary := fmt.Sprintf(i18n.G("Run hook %s of snap %q"), hookName, snapName)
+	task := hookstate.HookTask(st, summary, hookSup, nil)
+	return task
+}

overlord/registrystate/registrystate.go

@@ -20,6 +20,8 @@ package registrystate
 
 import (
 	"errors"
+	"fmt"
+	"sort"
 
 	"github.com/snapcore/snapd/overlord/assertstate"
 	"github.com/snapcore/snapd/overlord/hookstate"
@@ -226,6 +228,149 @@ func RegistryTransaction(ctx *hookstate.Context, reg *registry.Registry) (*Trans
 	return tx, nil
 }
 
+func createChangeRegistryTasks(st *state.State, chg *state.Change, tx *Transaction, view *registry.View, callingSnap string) error {
+	managerPlugs, err := getManagerPlugsForView(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)
+	}
+
+	managerNames := make([]string, 0, len(managerPlugs))
+	for name := range managerPlugs {
+		managerNames = append(managerNames, name)
+	}
+
+	// process the change/save hooks in a deterministic order (useful for testing
+	// and potentially for the snaps themselves)
+	sort.Strings(managerNames)
+
+	var tasks []*state.Task
+	linkTask := func(t *state.Task) {
+		if len(tasks) > 0 {
+			t.WaitFor(tasks[len(tasks)-1])
+		}
+		tasks = append(tasks, t)
+		chg.AddTask(t)
+	}
+
+	// if the transaction errors, clear the tx from the state
+	clearTxOnErrTask := st.NewTask("clear-registry-tx-on-error", "Clears the ongoing registry transaction from state (on error)")
+	linkTask(clearTxOnErrTask)
+
+	// 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 {
+			continue
+		}
+
+		const ignoreError = false
+		chgViewTask := setupRegistryHook(st, name, "change-view-"+plug.Name, ignoreError)
+		// run change-view-<plug> hooks in a sequential, deterministic order
+		linkTask(chgViewTask)
+	}
+
+	for _, name := range managerNames {
+		plug := managerPlugs[name]
+		manager := plug.Snap
+		if _, ok := manager.Hooks["save-view-"+plug.Name]; !ok {
+			continue
+		}
+
+		const ignoreError = false
+		saveViewTask := setupRegistryHook(st, name, "save-view-"+plug.Name, ignoreError)
+		// also run save-view hooks sequentially so, if one fails, we can determine
+		// which tasks need to be rolled back
+		linkTask(saveViewTask)
+	}
+
+	// run view-changed hooks for any plug that references a view that could have
+	// changed with this data modification
+	paths := tx.AlteredPaths()
+	affectedPlugs, err := getPlugsAffectedByPaths(st, view.Registry(), paths)
+	if err != nil {
+		return err
+	}
+
+	viewChangedSnaps := make([]string, 0, len(affectedPlugs))
+	for name := range affectedPlugs {
+		viewChangedSnaps = append(viewChangedSnaps, name)
+	}
+	sort.Strings(viewChangedSnaps)
+
+	for _, snapName := range viewChangedSnaps {
+		if snapName == callingSnap {
+			// the snap making the changes doesn't need to be notified
+			continue
+		}
+
+		for _, plug := range affectedPlugs[snapName] {
+			// TODO: run these concurrently or keep sequential for predictability?
+			const ignoreError = true
+			task := setupRegistryHook(st, snapName, plug.Name+"-view-changed", ignoreError)
+			linkTask(task)
+		}
+	}
+
+	// commit after managers 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
+	for _, t := range tasks {
+		t.Set("commit-task", commitTask.ID())
+	}
+	linkTask(commitTask)
+
+	// clear the ongoing tx from the state and unblock other writers waiting for it
+	clearTxTask := st.NewTask("clear-registry-tx", "Clears the ongoing registry transaction from state")
+	linkTask(clearTxTask)
+	clearTxTask.Set("commit-task", commitTask.ID())
+
+	return nil
+}
+
+func getManagerPlugsForView(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)
+	for _, plug := range plugs {
+		conns, err := repo.Connected(plug.Snap.InstanceName(), plug.Name)
+		if err != nil {
+			return nil, err
+		}
+		if len(conns) == 0 {
+			continue
+		}
+
+		if role, ok := plug.Attrs["role"]; !ok || role != "manager" {
+			continue
+		}
+
+		account, registryName, viewName, err := snap.RegistryPlugAttrs(plug)
+		if err != nil {
+			return nil, err
+		}
+
+		if view.Registry().Account != account || view.Registry().Name != registryName ||
+			view.Name != viewName {
+			continue
+		}
+
+		// 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
+	}
+
+	return managers, nil
+}
+
 func getPlugsAffectedByPaths(st *state.State, registry *registry.Registry, storagePaths []string) (map[string][]*snap.PlugInfo, error) {
 	var viewNames []string
 	for _, path := range storagePaths {
@@ -264,3 +409,32 @@ func getPlugsAffectedByPaths(st *state.State, registry *registry.Registry, stora
 
 	return affectedPlugs, nil
 }
+
+// GetStoredTransaction returns the registry transaction associate with the
+// task (even if indirectly) and the task in which it was stored.
+func GetStoredTransaction(t *state.Task) (*Transaction, *state.Task, error) {
+	var tx *Transaction
+	err := t.Get("registry-transaction", &tx)
+	if err == nil {
+		return tx, t, nil
+	} else if !errors.Is(err, &state.NoStateError{}) {
+		return nil, nil, err
+	}
+
+	var id string
+	err = t.Get("commit-task", &id)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	ct := t.State().Task(id)
+	if ct == nil {
+		return nil, nil, fmt.Errorf("cannot find task %s", id)
+	}
+
+	if err := ct.Get("registry-transaction", &tx); err != nil {
+		return nil, nil, err
+	}
+
+	return tx, ct, nil
+}