feat(stateengine): merge StateStarterUp interface from snapd

internals/overlord/overlord.go

@@ -16,6 +16,7 @@
 package overlord
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -83,6 +84,7 @@ type Overlord struct {
 
 	// managers
 	inited     bool
+	startedUp  bool
 	runner     *state.TaskRunner
 	serviceMgr *servstate.ServiceManager
 	commandMgr *cmdstate.CommandManager
@@ -252,6 +254,15 @@ func initRestart(s *state.State, curBootID string, restartHandler restart.Handle
 	return restart.Init(s, curBootID, restartHandler)
 }
 
+// StartUp proceeds to run any expensive Overlord or managers initialization. After this is done once it is a noop.
+func (o *Overlord) StartUp() error {
+	if o.startedUp {
+		return nil
+	}
+	o.startedUp = true
+	return o.stateEng.StartUp()
+}
+
 func (o *Overlord) ensureTimerSetup() {
 	o.ensureLock.Lock()
 	defer o.ensureLock.Unlock()
@@ -358,17 +369,16 @@ func (o *Overlord) settle(timeout time.Duration, beforeCleanups func()) error {
 	var errs []error
 	for !done {
 		if timeout > 0 && time.Since(t0) > timeout {
-			err := fmt.Errorf("Settle is not converging")
 			if len(errs) != 0 {
-				return &ensureError{append(errs, err)}
+				return newMultiError("settle is not converging", errs)
 			}
-			return err
+			return errors.New("settle is not converging")
 		}
 		next := o.ensureTimerReset()
 		err := o.stateEng.Ensure()
 		switch ee := err.(type) {
 		case nil:
-		case *ensureError:
+		case *multiError:
 			errs = append(errs, ee.errs...)
 		default:
 			errs = append(errs, err)
@@ -395,7 +405,7 @@ func (o *Overlord) settle(timeout time.Duration, beforeCleanups func()) error {
 		}
 	}
 	if len(errs) != 0 {
-		return &ensureError{errs}
+		return newMultiError("state ensure errors", errs)
 	}
 	return nil
 }

internals/overlord/overlord_test.go

@@ -169,6 +169,12 @@ type witnessManager struct {
 	expectedEnsure int
 	ensureCalled   chan struct{}
 	ensureCallback func(s *state.State) error
+	startedUp      int
+}
+
+func (wm *witnessManager) StartUp() error {
+	wm.startedUp++
+	return nil
 }
 
 func (wm *witnessManager) Ensure() error {
@@ -186,6 +192,9 @@ func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) {
 	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
+	err = o.StartUp()
+	c.Assert(err, IsNil)
+
 	o.Loop()
 
 	err = o.Stop()
@@ -224,6 +233,9 @@ func (ovs *overlordSuite) TestEnsureLoopRunAndStop(c *C) {
 	}
 	o.AddManager(witness)
 
+	err := o.StartUp()
+	c.Assert(err, IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -235,7 +247,7 @@ func (ovs *overlordSuite) TestEnsureLoopRunAndStop(c *C) {
 	}
 	c.Check(time.Since(t0) >= 10*time.Millisecond, Equals, true)
 
-	err := o.Stop()
+	err = o.Stop()
 	c.Assert(err, IsNil)
 }
 
@@ -257,6 +269,8 @@ func (ovs *overlordSuite) TestEnsureLoopMediatedEnsureBeforeImmediate(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -285,6 +299,8 @@ func (ovs *overlordSuite) TestEnsureLoopMediatedEnsureBefore(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -314,6 +330,8 @@ func (ovs *overlordSuite) TestEnsureBeforeSleepy(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -343,6 +361,8 @@ func (ovs *overlordSuite) TestEnsureBeforeLater(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -372,6 +392,8 @@ func (ovs *overlordSuite) TestEnsureLoopMediatedEnsureBeforeOutsideEnsure(c *C)
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -428,6 +450,8 @@ func (ovs *overlordSuite) TestEnsureLoopPrune(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 
 	select {
@@ -497,6 +521,8 @@ func (ovs *overlordSuite) TestCheckpoint(c *C) {
 	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	s := o.State()
 	s.Lock()
 	s.Set("mark", 1)
@@ -621,7 +647,7 @@ func (ovs *overlordSuite) TestSettleNotConverging(c *C) {
 	err := o.Settle(250 * time.Millisecond)
 	s.Lock()
 
-	c.Check(err, ErrorMatches, `Settle is not converging`)
+	c.Check(err, ErrorMatches, `settle is not converging`)
 
 }
 
@@ -864,6 +890,8 @@ func (ovs *overlordSuite) TestOverlordCanStandby(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	// can only standby after loop ran once
 	c.Assert(o.CanStandby(), Equals, false)
 

internals/overlord/stateengine.go

@@ -15,7 +15,9 @@
 package overlord
 
 import (
+	"bytes"
 	"fmt"
+	"strings"
 	"sync"
 
 	"github.com/canonical/pebble/internals/logger"
@@ -30,6 +32,13 @@ type StateManager interface {
 	Ensure() error
 }
 
+// StateStarterUp is optionally implemented by StateManager that have expensive
+// initialization to perform before the main Overlord loop.
+type StateStarterUp interface {
+	// StartUp asks manager to perform any expensive initialization.
+	StartUp() error
+}
+
 // StateWaiter is optionally implemented by StateManagers that have running
 // activities that can be waited.
 type StateWaiter interface {
@@ -53,8 +62,9 @@ type StateStopper interface {
 // cope with Ensure calls in any order, coordinating among themselves
 // solely via the state.
 type StateEngine struct {
-	state   *state.State
-	stopped bool
+	state     *state.State
+	stopped   bool
+	startedUp bool
 	// managers in use
 	mgrLock  sync.Mutex
 	managers []StateManager
@@ -72,12 +82,67 @@ func (se *StateEngine) State() *state.State {
 	return se.state
 }
 
-type ensureError struct {
-	errs []error
+// multiError collects multiple errors that affected an operation.
+type multiError struct {
+	header string
+	errs   []error
+}
+
+// newMultiError returns a new multiError struct initialized with
+// the given format string that explains what operation potentially
+// went wrong. multiError can be nested and will render correctly
+// in these cases.
+func newMultiError(header string, errs []error) error {
+	return &multiError{header: header, errs: errs}
+}
+
+// Error formats the error string.
+func (me *multiError) Error() string {
+	return me.nestedError(0)
+}
+
+// helper to ensure formating of nested multiErrors works.
+func (me *multiError) nestedError(level int) string {
+	indent := strings.Repeat(" ", level)
+	buf := bytes.NewBufferString(fmt.Sprintf("%s:\n", me.header))
+	if level > 8 {
+		return "circular or too deep error nesting (max 8)?!"
+	}
+	for i, err := range me.errs {
+		switch v := err.(type) {
+		case *multiError:
+			fmt.Fprintf(buf, "%s- %v", indent, v.nestedError(level+1))
+		default:
+			fmt.Fprintf(buf, "%s- %v", indent, err)
+		}
+		if i < len(me.errs)-1 {
+			fmt.Fprintf(buf, "\n")
+		}
+	}
+	return buf.String()
 }
 
-func (e *ensureError) Error() string {
-	return fmt.Sprintf("state ensure errors: %v", e.errs)
+// StartUp asks all managers to perform any expensive initialization. It is a noop after the first invocation.
+func (se *StateEngine) StartUp() error {
+	se.mgrLock.Lock()
+	defer se.mgrLock.Unlock()
+	if se.startedUp {
+		return nil
+	}
+	se.startedUp = true
+	var errs []error
+	for _, m := range se.managers {
+		if starterUp, ok := m.(StateStarterUp); ok {
+			err := starterUp.StartUp()
+			if err != nil {
+				errs = append(errs, err)
+			}
+		}
+	}
+	if len(errs) != 0 {
+		return newMultiError("state startup errors", errs)
+	}
+	return nil
 }
 
 // Ensure asks every manager to ensure that they are doing the necessary
@@ -103,7 +168,7 @@ func (se *StateEngine) Ensure() error {
 		}
 	}
 	if len(errs) != 0 {
-		return &ensureError{errs}
+		return newMultiError("state ensure errors", errs)
 	}
 	return nil
 }

internals/overlord/stateengine_test.go

@@ -35,9 +35,14 @@ func (ses *stateEngineSuite) TestNewAndState(c *C) {
 }
 
 type fakeManager struct {
-	name                   string
-	calls                  *[]string
-	ensureError, stopError error
+	name                      string
+	calls                     *[]string
+	ensureError, startupError error
+}
+
+func (fm *fakeManager) StartUp() error {
+	*fm.calls = append(*fm.calls, "startup:"+fm.name)
+	return fm.startupError
 }
 
 func (fm *fakeManager) Ensure() error {
@@ -55,6 +60,50 @@ func (fm *fakeManager) Wait() {
 
 var _ overlord.StateManager = (*fakeManager)(nil)
 
+func (ses *stateEngineSuite) TestStartUp(c *C) {
+	s := state.New(nil)
+	se := overlord.NewStateEngine(s)
+
+	calls := []string{}
+
+	mgr1 := &fakeManager{name: "mgr1", calls: &calls}
+	mgr2 := &fakeManager{name: "mgr2", calls: &calls}
+
+	se.AddManager(mgr1)
+	se.AddManager(mgr2)
+
+	err := se.StartUp()
+	c.Assert(err, IsNil)
+	c.Check(calls, DeepEquals, []string{"startup:mgr1", "startup:mgr2"})
+
+	// noop
+	err = se.StartUp()
+	c.Assert(err, IsNil)
+	c.Check(calls, HasLen, 2)
+}
+
+func (ses *stateEngineSuite) TestStartUpError(c *C) {
+	s := state.New(nil)
+	se := overlord.NewStateEngine(s)
+
+	calls := []string{}
+
+	err1 := errors.New("boom1")
+	err2 := errors.New("boom2")
+
+	mgr1 := &fakeManager{name: "mgr1", calls: &calls, startupError: err1}
+	mgr2 := &fakeManager{name: "mgr2", calls: &calls, startupError: err2}
+
+	se.AddManager(mgr1)
+	se.AddManager(mgr2)
+
+	err := se.StartUp()
+	c.Check(err, ErrorMatches, `state startup errors:
+- boom1
+- boom2`)
+	c.Check(calls, DeepEquals, []string{"startup:mgr1", "startup:mgr2"})
+}
+
 func (ses *stateEngineSuite) TestEnsure(c *C) {
 	s := state.New(nil)
 	se := overlord.NewStateEngine(s)
@@ -92,7 +141,9 @@ func (ses *stateEngineSuite) TestEnsureError(c *C) {
 	se.AddManager(mgr2)
 
 	err := se.Ensure()
-	c.Check(err.Error(), DeepEquals, "state ensure errors: [boom1 boom2]")
+	c.Check(err, ErrorMatches, `state ensure errors:
+- boom1
+- boom2`)
 	c.Check(calls, DeepEquals, []string{"ensure:mgr1", "ensure:mgr2"})
 }