many: add default-configure hook

overlord/configstate/configmgr.go

@@ -44,6 +44,15 @@ func MockConfigcoreRun(f func(sysconfig.Device, configcore.RunTransaction) error
 func Init(st *state.State, hookManager *hookstate.HookManager) error {
 	delayedCrossMgrInit()
 
+	// Most default configuration is handled via the "default-configure" hook of the
+	// snaps, however, some default configuration is internally handled.
+	hookManager.Register(regexp.MustCompile("^default-configure$"), newDefaultConfigureHandler)
+	// Ensure that we run default-configure for the core snap internally.
+	hookManager.RegisterHijack("default-configure", "core", func(ctx *hookstate.Context) error {
+		// TODO: Find out if complete implementation is required and if so add this.
+		return nil
+	})
+
 	// Most configuration is handled via the "configure" hook of the
 	// snaps. However some configuration is internally handled
 	hookManager.Register(regexp.MustCompile("^configure$"), newConfigureHandler)

overlord/configstate/configstate.go

@@ -43,6 +43,7 @@ import (
 
 func init() {
 	snapstate.Configure = Configure
+	snapstate.DefaultConfigure = DefaultConfigure
 }
 
 func ConfigureHookTimeout() time.Duration {
@@ -105,19 +106,19 @@ func ConfigureInstalled(st *state.State, snapName string, patch map[string]inter
 	return taskset, nil
 }
 
-// Configure returns a taskset to apply the given configuration patch.
-func Configure(st *state.State, snapName string, patch map[string]interface{}, flags int) *state.TaskSet {
-	summary := fmt.Sprintf(i18n.G("Run configure hook of %q snap"), snapName)
-	// regular configuration hook
+func configureHook(st *state.State, configureHook string, snapName string, patch map[string]interface{}, flags int) *state.TaskSet {
+	summary := fmt.Sprintf(i18n.G("Run %s hook of %q snap"), configureHook, snapName)
+	// setup configure hook variant specified by confHook
 	hooksup := &hookstate.HookSetup{
 		Snap:        snapName,
-		Hook:        "configure",
+		Hook:        configureHook,
 		Optional:    len(patch) == 0,
 		IgnoreError: flags&snapstate.IgnoreHookError != 0,
 		TrackError:  flags&snapstate.TrackHookError != 0,
 		// all configure hooks must finish within this timeout
 		Timeout: ConfigureHookTimeout(),
 	}
+
 	var contextData map[string]interface{}
 	if flags&snapstate.UseConfigDefaults != 0 {
 		contextData = map[string]interface{}{"use-defaults": true}
@@ -126,13 +127,23 @@ func Configure(st *state.State, snapName string, patch map[string]interface{}, f
 	}
 
 	if hooksup.Optional {
-		summary = fmt.Sprintf(i18n.G("Run configure hook of %q snap if present"), snapName)
+		summary = fmt.Sprintf(i18n.G("Run %s hook of %q snap if present"), configureHook, snapName)
 	}
 
 	task := hookstate.HookTask(st, summary, hooksup, contextData)
 	return state.NewTaskSet(task)
 }
 
+// Configure returns a taskset to apply the given configuration patch.
+func Configure(st *state.State, snapName string, patch map[string]interface{}, flags int) *state.TaskSet {
+	return configureHook(st, "configure", snapName, patch, flags)
+}
+
+// DefaultConfigure returns a taskset to apply the given default-configuration patch.
+func DefaultConfigure(st *state.State, snapName string, patch map[string]interface{}, flags int) *state.TaskSet {
+	return configureHook(st, "default-configure", snapName, patch, flags)
+}
+
 // RemapSnapFromRequest renames a snap as received from an API request
 func RemapSnapFromRequest(snapName string) string {
 	if snapName == "system" {

overlord/configstate/hooks.go

@@ -34,36 +34,6 @@ type configureHandler struct {
 	context *hookstate.Context
 }
 
-// cachedTransaction is the index into the context cache where the initialized
-// transaction is stored.
-type cachedTransaction struct{}
-
-// ContextTransaction retrieves the transaction cached within the context (and
-// creates one if it hasn't already been cached).
-func ContextTransaction(context *hookstate.Context) *config.Transaction {
-	// Check for one already cached
-	tr, ok := context.Cached(cachedTransaction{}).(*config.Transaction)
-	if ok {
-		return tr
-	}
-
-	// It wasn't already cached, so create and cache a new one
-	tr = config.NewTransaction(context.State())
-
-	context.OnDone(func() error {
-		tr.Commit()
-		if context.InstanceName() == "core" {
-			// make sure the Ensure logic can process
-			// system configuration changes as soon as possible
-			context.State().EnsureBefore(0)
-		}
-		return nil
-	})
-
-	context.Cache(cachedTransaction{}, tr)
-	return tr
-}
-
 func newConfigureHandler(context *hookstate.Context) hookstate.Handler {
 	return &configureHandler{context: context}
 }
@@ -108,6 +78,11 @@ func (h *configureHandler) Before() error {
 			if info.Hooks["configure"] == nil {
 				return fmt.Errorf("cannot apply gadget config defaults for snap %q, no configure hook", instanceName)
 			}
+			// If default-configure hook is present it should have already applied the default configuration. Only apply
+			// default configuration using the configure hook when default-configure hook is not present.
+			if info.Hooks["default-configure"] != nil {
+				patch = nil
+			}
 		}
 	} else {
 		if err := h.context.Get("patch", &patch); err != nil && !errors.Is(err, state.ErrNoState) {
@@ -133,3 +108,113 @@ func (h *configureHandler) Done() error {
 func (h *configureHandler) Error(err error) (bool, error) {
 	return false, nil
 }
+
+// defaultConfigureHandler is the handler for the default-configure hook.
+type defaultConfigureHandler struct {
+	context *hookstate.Context
+}
+
+func newDefaultConfigureHandler(context *hookstate.Context) hookstate.Handler {
+	return &defaultConfigureHandler{context: context}
+}
+
+// Before is called by the HookManager before the default-configure hook is run.
+func (h *defaultConfigureHandler) Before() error {
+	h.context.Lock()
+	defer h.context.Unlock()
+
+	tr := ContextTransaction(h.context)
+
+	// Initialize the transaction if there's a patch provided in the
+	// context or useDefaults is set in which case gadget defaults are used.
+
+	var patch map[string]interface{}
+	var useDefaults bool
+	if err := h.context.Get("use-defaults", &useDefaults); err != nil && !errors.Is(err, state.ErrNoState) {
+		return err
+	}
+
+	instanceName := h.context.InstanceName()
+	st := h.context.State()
+	if useDefaults {
+		task, _ := h.context.Task()
+		deviceCtx, err := snapstate.DeviceCtx(st, task, nil)
+		if err != nil {
+			return err
+		}
+
+		patch, err = snapstate.ConfigDefaults(st, deviceCtx, instanceName)
+		if err != nil && !errors.Is(err, state.ErrNoState) {
+			return err
+		}
+		// core is handled internally and does not need a default-configure and configure
+		// hook, for other snaps double check that the hook is present
+		if len(patch) != 0 && instanceName != "core" {
+			// TODO: helper on context?
+			info, err := snapstate.CurrentInfo(st, instanceName)
+			if err != nil {
+				return err
+			}
+			if info.Hooks["default-configure"] == nil {
+				return fmt.Errorf("cannot apply gadget config defaults for snap %q, no default-configure hook", instanceName)
+			}
+			// the default-configure hook exists to support the configure hook and is therefore disallowed if the
+			// configure hook is unavailable
+			if info.Hooks["configure"] == nil {
+				return fmt.Errorf("cannot apply gadget config defaults for snap %q, no configure hook", instanceName)
+			}
+		}
+	} else {
+		if err := h.context.Get("patch", &patch); err != nil && !errors.Is(err, state.ErrNoState) {
+			return err
+		}
+	}
+
+	if err := config.Patch(tr, instanceName, patch); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// Done is called by the HookManager after the default-configure hook has exited
+// successfully.
+func (h *defaultConfigureHandler) Done() error {
+	return nil
+}
+
+// Error is called by the HookManager after the default-configure hook has exited
+// non-zero, and includes the error.
+func (h *defaultConfigureHandler) Error(err error) (bool, error) {
+	return false, nil
+}
+
+// cachedTransaction is the index into the context cache where the initialized
+// transaction is stored.
+type cachedTransaction struct{}
+
+// ContextTransaction retrieves the transaction cached within the context (and
+// creates one if it hasn't already been cached).
+func ContextTransaction(context *hookstate.Context) *config.Transaction {
+	// Check for one already cached
+	tr, ok := context.Cached(cachedTransaction{}).(*config.Transaction)
+	if ok {
+		return tr
+	}
+
+	// It wasn't already cached, so create and cache a new one
+	tr = config.NewTransaction(context.State())
+
+	context.OnDone(func() error {
+		tr.Commit()
+		if context.InstanceName() == "core" {
+			// make sure the Ensure logic can process
+			// system configuration changes as soon as possible
+			context.State().EnsureBefore(0)
+		}
+		return nil
+	})
+
+	context.Cache(cachedTransaction{}, tr)
+	return tr
+}

overlord/hookstate/ctlcmd/services_test.go

@@ -336,6 +336,7 @@ var (
 		"set-auto-aliases",
 		"setup-aliases",
 		"run-hook[install]",
+		"run-hook[default-configure]",
 		"start-snap-services",
 		"run-hook[configure]",
 		"run-hook[check-health]",
@@ -403,10 +404,10 @@ func (s *servicectlSuite) TestQueuedCommands(c *C) {
 	checkLaneTasks := func(lane int) {
 		laneTasks := chg.LaneTasks(lane)
 		c.Assert(taskKinds(laneTasks), DeepEquals, expectedTaskKinds)
-		c.Check(laneTasks[12].Summary(), Matches, `Run configure hook of .* snap if present`)
-		c.Check(laneTasks[14].Summary(), Equals, "stop of [test-snap.test-service]")
-		c.Check(laneTasks[16].Summary(), Equals, "start of [test-snap.test-service]")
-		c.Check(laneTasks[18].Summary(), Equals, "restart of [test-snap.test-service]")
+		c.Check(laneTasks[13].Summary(), Matches, `Run configure hook of .* snap if present`)
+		c.Check(laneTasks[15].Summary(), Equals, "stop of [test-snap.test-service]")
+		c.Check(laneTasks[17].Summary(), Equals, "start of [test-snap.test-service]")
+		c.Check(laneTasks[19].Summary(), Equals, "restart of [test-snap.test-service]")
 	}
 	checkLaneTasks(1)
 	checkLaneTasks(2)
@@ -591,10 +592,10 @@ func (s *servicectlSuite) TestQueuedCommandsSingleLane(c *C) {
 
 	laneTasks := chg.LaneTasks(0)
 	c.Assert(taskKinds(laneTasks), DeepEquals, append(installTaskKinds, "exec-command", "service-control", "exec-command", "service-control", "exec-command", "service-control"))
-	c.Check(laneTasks[12].Summary(), Matches, `Run configure hook of .* snap if present`)
-	c.Check(laneTasks[14].Summary(), Equals, "stop of [test-snap.test-service]")
-	c.Check(laneTasks[16].Summary(), Equals, "start of [test-snap.test-service]")
-	c.Check(laneTasks[18].Summary(), Equals, "restart of [test-snap.test-service]")
+	c.Check(laneTasks[13].Summary(), Matches, `Run configure hook of .* snap if present`)
+	c.Check(laneTasks[15].Summary(), Equals, "stop of [test-snap.test-service]")
+	c.Check(laneTasks[17].Summary(), Equals, "start of [test-snap.test-service]")
+	c.Check(laneTasks[19].Summary(), Equals, "restart of [test-snap.test-service]")
 }
 
 func (s *servicectlSuite) TestTwoServices(c *C) {

overlord/managers_test.go

@@ -4808,6 +4808,10 @@ func validateInstallTasks(c *C, tasks []*state.Task, name, revno string, flags i
 	i++
 	c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Run install hook of "%s" snap if present`, name))
 	i++
+	if flags&noConfigure == 0 {
+		c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Run default-configure hook of "%s" snap if present`, name))
+		i++
+	}
 	c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Start snap "%s" (%s) services`, name, revno))
 	i++
 	if flags&noConfigure == 0 {