Add StateStarterUp to move expensive init ops to dedicated methods

Introduce the StateStarterUp interface alongside its implementations for
Overlord and StateEngine.

These changes were cherry-picked and adopted for pebble from the
original snapd PR: canonical/snapd#7132

internals/daemon/api_test.go

@@ -59,6 +59,9 @@ func (s *apiSuite) daemon(c *check.C) *Daemon {
 	d, err := New(&Options{Dir: s.pebbleDir})
 	c.Assert(err, check.IsNil)
 	d.addRoutes()
+
+	c.Assert(d.overlord.StartUp(), check.IsNil)
+
 	s.d = d
 	return d
 }

internals/daemon/daemon.go

@@ -466,18 +466,23 @@ func (d *Daemon) initStandbyHandling() {
 	d.standbyOpinions.Start()
 }
 
-func (d *Daemon) Start() {
+func (d *Daemon) Start() error {
 	if d.rebootIsMissing {
 		// we need to schedule and wait for a system restart
 		d.tomb.Kill(nil)
 		// avoid systemd killing us again while we wait
 		systemdSdNotify("READY=1")
-		return
+		return nil
 	}
 	if d.overlord == nil {
 		panic("internal error: no Overlord")
 	}
 
+	// now perform expensive overlord/manages initialisation
+	if err := d.overlord.StartUp(); err != nil {
+		return err
+	}
+
 	d.StartTime = time.Now()
 
 	d.connTracker = &connTracker{conns: make(map[net.Conn]struct{})}
@@ -519,6 +524,7 @@ func (d *Daemon) Start() {
 
 	// notify systemd that we are ready
 	systemdSdNotify("READY=1")
+	return nil
 }
 
 // HandleRestart implements overlord.RestartBehavior.

internals/daemon/daemon_test.go

@@ -130,7 +130,8 @@ func (s *daemonSuite) TestExternalManager(c *C) {
 		OverlordExtension: &fakeExtension{},
 	})
 	c.Assert(err, IsNil)
-
+	err = d.overlord.StartUp()
+	c.Assert(err, IsNil)
 	err = d.overlord.StateEngine().Ensure()
 	c.Assert(err, IsNil)
 	extension, ok := d.overlord.Extension().(*fakeExtension)
@@ -185,7 +186,7 @@ func (s *daemonSuite) TestAddCommand(c *C) {
 
 	d := s.newDaemon(c)
 	d.Init()
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 	defer d.Stop(nil)
 
 	result := d.router.Get(endpoint).GetHandler()
@@ -197,7 +198,7 @@ func (s *daemonSuite) TestExplicitPaths(c *C) {
 
 	d := s.newDaemon(c)
 	d.Init()
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 	defer d.Stop(nil)
 
 	info, err := os.Stat(s.socketPath)
@@ -553,7 +554,7 @@ func (s *daemonSuite) TestStartStop(c *C) {
 	untrustedAccept := make(chan struct{})
 	d.untrustedListener = &witnessAcceptListener{Listener: l2, accept: untrustedAccept}
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 
 	generalDone := make(chan struct{})
 	go func() {
@@ -594,7 +595,7 @@ func (s *daemonSuite) TestRestartWiring(c *C) {
 	untrustedAccept := make(chan struct{})
 	d.untrustedListener = &witnessAcceptListener{Listener: l, accept: untrustedAccept}
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 	defer d.Stop(nil)
 
 	generalDone := make(chan struct{})
@@ -661,7 +662,7 @@ func (s *daemonSuite) TestGracefulStop(c *C) {
 	untrustedAccept := make(chan struct{})
 	d.untrustedListener = &witnessAcceptListener{Listener: untrustedL, accept: untrustedAccept}
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 
 	generalAccepting := make(chan struct{})
 	go func() {
@@ -728,7 +729,7 @@ func (s *daemonSuite) TestRestartSystemWiring(c *C) {
 	untrustedAccept := make(chan struct{})
 	d.untrustedListener = &witnessAcceptListener{Listener: l, accept: untrustedAccept}
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 	defer d.Stop(nil)
 
 	st := d.overlord.State()
@@ -875,7 +876,7 @@ func (s *daemonSuite) TestRestartShutdownWithSigtermInBetween(c *C) {
 	d := s.newDaemon(c)
 	makeDaemonListeners(c, d)
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 	st := d.overlord.State()
 
 	st.Lock()
@@ -907,7 +908,7 @@ func (s *daemonSuite) TestRestartShutdown(c *C) {
 	d := s.newDaemon(c)
 	makeDaemonListeners(c, d)
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 	st := d.overlord.State()
 
 	st.Lock()
@@ -954,7 +955,7 @@ func (s *daemonSuite) TestRestartExpectedRebootIsMissing(c *C) {
 	c.Check(err, IsNil)
 	c.Check(n, Equals, 1)
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 
 	c.Check(s.notified, DeepEquals, []string{"READY=1"})
 
@@ -1030,11 +1031,11 @@ func (s *daemonSuite) TestRestartIntoSocketModeNoNewChanges(c *C) {
 	d := s.newDaemon(c)
 	makeDaemonListeners(c, d)
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 
 	// pretend some ensure happened
 	for i := 0; i < 5; i++ {
-		d.overlord.StateEngine().Ensure()
+		c.Check(d.overlord.StateEngine().Ensure(), IsNil)
 		time.Sleep(5 * time.Millisecond)
 	}
 
@@ -1066,10 +1067,10 @@ func (s *daemonSuite) TestRestartIntoSocketModePendingChanges(c *C) {
 
 	st := d.overlord.State()
 
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 	// pretend some ensure happened
 	for i := 0; i < 5; i++ {
-		d.overlord.StateEngine().Ensure()
+		c.Check(d.overlord.StateEngine().Ensure(), IsNil)
 		time.Sleep(5 * time.Millisecond)
 	}
 
@@ -1152,7 +1153,7 @@ func (s *daemonSuite) TestHTTPAPI(c *C) {
 	s.httpAddress = ":0" // Go will choose port (use listener.Addr() to find it)
 	d := s.newDaemon(c)
 	d.Init()
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 	port := d.httpListener.Addr().(*net.TCPAddr).Port
 
 	request, err := http.NewRequest("GET", fmt.Sprintf("http://localhost:%d/v1/health", port), nil)
@@ -1195,7 +1196,7 @@ services:
 	d := s.newDaemon(c)
 	err := d.Init()
 	c.Assert(err, IsNil)
-	d.Start()
+	c.Assert(d.Start(), IsNil)
 
 	// Start the test service.
 	payload := bytes.NewBufferString(`{"action": "start", "services": ["test1"]}`)

internals/overlord/managers_test.go

@@ -44,6 +44,8 @@ func (s *mgrsSuite) SetUpTest(c *C) {
 
 	o, err := overlord.New(&overlord.Options{PebbleDir: s.dir})
 	c.Assert(err, IsNil)
+	err = o.StartUp()
+	c.Assert(err, IsNil)
 	s.o = o
 }
 

internals/overlord/overlord.go

@@ -83,6 +83,7 @@ type Overlord struct {
 
 	// managers
 	inited     bool
+	startedUp  bool
 	runner     *state.TaskRunner
 	serviceMgr *servstate.ServiceManager
 	commandMgr *cmdstate.CommandManager
@@ -252,6 +253,14 @@ func initRestart(s *state.State, curBootID string, restartHandler restart.Handle
 	return restart.Init(s, curBootID, restartHandler)
 }
 
+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()
@@ -337,6 +346,10 @@ func (o *Overlord) Stop() error {
 }
 
 func (o *Overlord) settle(timeout time.Duration, beforeCleanups func()) error {
+	if err := o.StartUp(); err != nil {
+		return err
+	}
+
 	func() {
 		o.ensureLock.Lock()
 		defer o.ensureLock.Unlock()
@@ -406,7 +419,7 @@ func (o *Overlord) settle(timeout time.Duration, beforeCleanups func()) error {
 // is scheduled. It then waits similarly for all ready changes to
 // reach the clean state. Chiefly for tests. Cannot be used in
 // conjunction with Loop. If timeout is non-zero and settling takes
-// longer than timeout, returns an error.
+// longer than timeout, returns an error. Calls StartUp as well.
 func (o *Overlord) Settle(timeout time.Duration) error {
 	return o.settle(timeout, nil)
 }
@@ -418,7 +431,7 @@ func (o *Overlord) Settle(timeout time.Duration) error {
 // changes to reach the clean state, but calls once the provided
 // callback before doing that. Chiefly for tests. Cannot be used in
 // conjunction with Loop. If timeout is non-zero and settling takes
-// longer than timeout, returns an error.
+// longer than timeout, returns an error. Calls StartUp as well.
 func (o *Overlord) SettleObserveBeforeCleanups(timeout time.Duration, beforeCleanups func()) error {
 	return o.settle(timeout, beforeCleanups)
 }

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,8 +247,10 @@ 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)
+
+	c.Check(witness.startedUp, Equals, 1)
 }
 
 func (ovs *overlordSuite) TestEnsureLoopMediatedEnsureBeforeImmediate(c *C) {
@@ -257,6 +271,8 @@ func (ovs *overlordSuite) TestEnsureLoopMediatedEnsureBeforeImmediate(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -285,6 +301,8 @@ func (ovs *overlordSuite) TestEnsureLoopMediatedEnsureBefore(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -314,6 +332,9 @@ func (ovs *overlordSuite) TestEnsureBeforeSleepy(c *C) {
 	}
 	o.AddManager(witness)
 
+	err := o.StartUp()
+	c.Assert(err, IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -343,6 +364,8 @@ func (ovs *overlordSuite) TestEnsureBeforeLater(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -372,6 +395,8 @@ func (ovs *overlordSuite) TestEnsureLoopMediatedEnsureBeforeOutsideEnsure(c *C)
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 	defer o.Stop()
 
@@ -428,6 +453,8 @@ func (ovs *overlordSuite) TestEnsureLoopPrune(c *C) {
 	}
 	o.AddManager(witness)
 
+	c.Assert(o.StartUp(), IsNil)
+
 	o.Loop()
 
 	select {
@@ -864,6 +891,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)