diff --git a/internals/daemon/api_test.go b/internals/daemon/api_test.go index c7b624d98..79e87ec18 100644 --- a/internals/daemon/api_test.go +++ b/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 } diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go index 15517549a..862fb7cd1 100644 --- a/internals/daemon/daemon.go +++ b/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. diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go index b0ab87479..cd3081fd3 100644 --- a/internals/daemon/daemon_test.go +++ b/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"]}`) diff --git a/internals/overlord/managers_test.go b/internals/overlord/managers_test.go index 834b4c933..afabcd5e6 100644 --- a/internals/overlord/managers_test.go +++ b/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 } diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go index ba3ca2ada..d6406e243 100644 --- a/internals/overlord/overlord.go +++ b/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) } diff --git a/internals/overlord/overlord_test.go b/internals/overlord/overlord_test.go index 2d257a8d3..57765a049 100644 --- a/internals/overlord/overlord_test.go +++ b/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) diff --git a/internals/overlord/stateengine.go b/internals/overlord/stateengine.go index 6ed822548..ee5a9ec1d 100644 --- a/internals/overlord/stateengine.go +++ b/internals/overlord/stateengine.go @@ -30,6 +30,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 +60,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,6 +80,37 @@ func (se *StateEngine) State() *state.State { return se.state } +type startupError struct { + errs []error +} + +func (e *startupError) Error() string { + return fmt.Sprintf("state startup 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 &startupError{errs} + } + return nil +} + type ensureError struct { errs []error } @@ -91,6 +130,9 @@ func (e *ensureError) Error() string { func (se *StateEngine) Ensure() error { se.mgrLock.Lock() defer se.mgrLock.Unlock() + if !se.startedUp { + return fmt.Errorf("state engine skipped startup") + } if se.stopped { return fmt.Errorf("state engine already stopped") } diff --git a/internals/overlord/stateengine_test.go b/internals/overlord/stateengine_test.go index f12ed98b9..f70aa3475 100644 --- a/internals/overlord/stateengine_test.go +++ b/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,48 @@ 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.Error(), DeepEquals, "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) @@ -68,6 +115,11 @@ func (ses *stateEngineSuite) TestEnsure(c *C) { se.AddManager(mgr2) err := se.Ensure() + c.Check(err, ErrorMatches, "state engine skipped startup") + c.Assert(se.StartUp(), IsNil) + calls = []string{} + + err = se.Ensure() c.Assert(err, IsNil) c.Check(calls, DeepEquals, []string{"ensure:mgr1", "ensure:mgr2"}) @@ -91,6 +143,9 @@ func (ses *stateEngineSuite) TestEnsureError(c *C) { se.AddManager(mgr1) se.AddManager(mgr2) + c.Assert(se.StartUp(), IsNil) + calls = []string{} + err := se.Ensure() c.Check(err.Error(), DeepEquals, "state ensure errors: [boom1 boom2]") c.Check(calls, DeepEquals, []string{"ensure:mgr1", "ensure:mgr2"}) @@ -108,6 +163,9 @@ func (ses *stateEngineSuite) TestStop(c *C) { se.AddManager(mgr1) se.AddManager(mgr2) + c.Assert(se.StartUp(), IsNil) + calls = []string{} + se.Stop() c.Check(calls, DeepEquals, []string{"stop:mgr1", "stop:mgr2"}) se.Stop()