diff --git a/internals/cli/cmd_run.go b/internals/cli/cmd_run.go index 4ba5774f..b5c6febb 100644 --- a/internals/cli/cmd_run.go +++ b/internals/cli/cmd_run.go @@ -48,6 +48,7 @@ type sharedRunEnterOpts struct { Hold bool `long:"hold"` HTTP string `long:"http"` Verbose bool `short:"v" long:"verbose"` + Dry bool `long:"dry"` Args [][]string `long:"args" terminator:";"` } @@ -56,6 +57,7 @@ var sharedRunEnterArgsHelp = map[string]string{ "--hold": "Do not start default services automatically", "--http": `Start HTTP API listening on this address (e.g., ":4000")`, "--verbose": "Log all output from services to stdout", + "--dry": "Attempt to run without actually running", "--args": `Provide additional arguments to a service`, } @@ -162,6 +164,9 @@ func runDaemon(rcmd *cmdRun, ch chan os.Signal, ready chan<- func()) error { if err != nil { return err } + if rcmd.Dry { + return nil + } if err := d.Init(); err != nil { return err } diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go index b0ab8747..b980e07c 100644 --- a/internals/daemon/daemon_test.go +++ b/internals/daemon/daemon_test.go @@ -97,8 +97,13 @@ func (h *fakeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } type fakeManager struct { - id string - ensureCalls int + id string + ensureCalls, dryStartCalls int +} + +func (m *fakeManager) DryStart() error { + m.dryStartCalls++ + return nil } func (m *fakeManager) Ensure() error { @@ -111,7 +116,7 @@ type fakeExtension struct { } func (f *fakeExtension) ExtraManagers(o *overlord.Overlord) ([]overlord.StateManager, error) { - f.mgr = fakeManager{id: "expected", ensureCalls: 0} + f.mgr = fakeManager{id: "expected", ensureCalls: 0, dryStartCalls: 0} result := []overlord.StateManager{&f.mgr} return result, nil } diff --git a/internals/overlord/cmdstate/manager.go b/internals/overlord/cmdstate/manager.go index 2fdd06b4..f2abd4d8 100644 --- a/internals/overlord/cmdstate/manager.go +++ b/internals/overlord/cmdstate/manager.go @@ -37,6 +37,11 @@ func NewManager(runner *state.TaskRunner) *CommandManager { return manager } +// DryStart is part of the overlord.StateManager interface. +func (m *CommandManager) DryStart() error { + return nil +} + // Ensure is part of the overlord.StateManager interface. func (m *CommandManager) Ensure() error { return nil diff --git a/internals/overlord/logstate/manager.go b/internals/overlord/logstate/manager.go index 46582899..ec3edf30 100644 --- a/internals/overlord/logstate/manager.go +++ b/internals/overlord/logstate/manager.go @@ -109,6 +109,11 @@ func (m *LogManager) ServiceStarted(service *plan.Service, buffer *servicelog.Ri } } +// DryStart implements overlord.StateManager. +func (m *LogManager) DryStart() error { + return nil +} + // Ensure implements overlord.StateManager. func (m *LogManager) Ensure() error { return nil diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go index ba3ca2ad..9978e048 100644 --- a/internals/overlord/overlord.go +++ b/internals/overlord/overlord.go @@ -16,6 +16,7 @@ package overlord import ( + "errors" "fmt" "io" "os" @@ -94,7 +95,6 @@ type Overlord struct { // New creates an Overlord with all its state managers. func New(opts *Options) (*Overlord, error) { - o := &Overlord{ pebbleDir: opts.PebbleDir, loopTomb: new(tomb.Tomb), @@ -175,6 +175,10 @@ func New(opts *Options) (*Overlord, error) { // before it. o.stateEng.AddManager(o.runner) + // Dry start all managers. + if err := o.stateEng.DryStart(); err != nil { + return nil, err + } return o, nil } @@ -358,17 +362,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 +398,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 } @@ -474,6 +477,7 @@ func FakeWithState(handleRestart func(restart.RestartType)) *Overlord { s := state.New(fakeBackend{o: o}) o.stateEng = NewStateEngine(s) o.runner = state.NewTaskRunner(s) + o.stateEng.DryStart() return o } diff --git a/internals/overlord/overlord_test.go b/internals/overlord/overlord_test.go index 2d257a8d..affa4c50 100644 --- a/internals/overlord/overlord_test.go +++ b/internals/overlord/overlord_test.go @@ -171,6 +171,10 @@ type witnessManager struct { ensureCallback func(s *state.State) error } +func (wm *witnessManager) DryStart() error { + return nil +} + func (wm *witnessManager) Ensure() error { if wm.expectedEnsure--; wm.expectedEnsure == 0 { close(wm.ensureCalled) @@ -562,6 +566,10 @@ func newSampleManager(s *state.State, runner *state.TaskRunner) *sampleManager { return sm } +func (sm *sampleManager) DryStart() error { + return nil +} + func (sm *sampleManager) Ensure() error { if sm.ensureCallback != nil { sm.ensureCallback() @@ -621,7 +629,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`) } diff --git a/internals/overlord/servstate/manager.go b/internals/overlord/servstate/manager.go index f85bfeb5..cb2b5bf9 100644 --- a/internals/overlord/servstate/manager.go +++ b/internals/overlord/servstate/manager.go @@ -244,6 +244,12 @@ func (m *ServiceManager) acquirePlan() (release func(), err error) { return release, nil } +// DryStart implements StateManager.DryStart. +func (m *ServiceManager) DryStart() error { + _, err := m.Plan() + return err +} + // Ensure implements StateManager.Ensure. func (m *ServiceManager) Ensure() error { return nil diff --git a/internals/overlord/state/taskrunner.go b/internals/overlord/state/taskrunner.go index 5d9ff244..9895a2b7 100644 --- a/internals/overlord/state/taskrunner.go +++ b/internals/overlord/state/taskrunner.go @@ -331,6 +331,10 @@ func (r *TaskRunner) tryUndo(t *Task) { } } +func (r *TaskRunner) DryStart() error { + return nil +} + // Ensure starts new goroutines for all known tasks with no pending // dependencies. // Note that Ensure will lock the state. diff --git a/internals/overlord/stateengine.go b/internals/overlord/stateengine.go index 6ed82254..0ebc2aa7 100644 --- a/internals/overlord/stateengine.go +++ b/internals/overlord/stateengine.go @@ -15,7 +15,10 @@ package overlord import ( + "bytes" + "errors" "fmt" + "strings" "sync" "github.com/canonical/pebble/internals/logger" @@ -25,6 +28,10 @@ import ( // StateManager is implemented by types responsible for observing // the system and manipulating it to reflect the desired state. type StateManager interface { + // DryStart prepares the manager to run its activities without + // incurring unwanted side effects. + DryStart() error + // Ensure forces a complete evaluation of the current state. // See StateEngine.Ensure for more details. Ensure() error @@ -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 + dryStarted bool + stopped bool // managers in use mgrLock sync.Mutex managers []StateManager @@ -72,12 +80,68 @@ 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} } -func (e *ensureError) Error() string { - return fmt.Sprintf("state ensure errors: %v", e.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() +} + +var errStateEngineStopped = errors.New("state engine already stopped") + +// DryStart ensures all managers are ready to run their activities +// before the first call to Ensure() is made. +func (se *StateEngine) DryStart() error { + se.mgrLock.Lock() + defer se.mgrLock.Unlock() + if se.stopped { + return errStateEngineStopped + } + var errs []error + for _, m := range se.managers { + err := m.DryStart() + if err != nil { + errs = append(errs, err) + } + } + if len(errs) != 0 { + return newMultiError("dry-start failed", errs) + } + se.dryStarted = true + return nil } // Ensure asks every manager to ensure that they are doing the necessary @@ -91,8 +155,11 @@ func (e *ensureError) Error() string { func (se *StateEngine) Ensure() error { se.mgrLock.Lock() defer se.mgrLock.Unlock() + if !se.dryStarted { + return errors.New("state engine did not dry-start") + } if se.stopped { - return fmt.Errorf("state engine already stopped") + return errStateEngineStopped } var errs []error for _, m := range se.managers { @@ -103,7 +170,7 @@ func (se *StateEngine) Ensure() error { } } if len(errs) != 0 { - return &ensureError{errs} + return newMultiError("state ensure errors", errs) } return nil } diff --git a/internals/overlord/stateengine_test.go b/internals/overlord/stateengine_test.go index f12ed98b..f056f918 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, dryStartError error +} + +func (fm *fakeManager) DryStart() error { + *fm.calls = append(*fm.calls, "drystart:"+fm.name) + return fm.dryStartError } func (fm *fakeManager) Ensure() error { @@ -67,13 +72,45 @@ func (ses *stateEngineSuite) TestEnsure(c *C) { se.AddManager(mgr1) se.AddManager(mgr2) - err := se.Ensure() + err := se.DryStart() c.Assert(err, IsNil) - c.Check(calls, DeepEquals, []string{"ensure:mgr1", "ensure:mgr2"}) + c.Check(calls, DeepEquals, []string{"drystart:mgr1", "drystart:mgr2"}) err = se.Ensure() c.Assert(err, IsNil) - c.Check(calls, DeepEquals, []string{"ensure:mgr1", "ensure:mgr2", "ensure:mgr1", "ensure:mgr2"}) + c.Check(calls, DeepEquals, []string{"drystart:mgr1", "drystart:mgr2", "ensure:mgr1", "ensure:mgr2"}) +} + +func (ses *stateEngineSuite) TestDryStartError(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, dryStartError: err1} + mgr2 := &fakeManager{name: "mgr2", calls: &calls, dryStartError: err2} + + se.AddManager(mgr1) + se.AddManager(mgr2) + + err := se.DryStart() + c.Check(err, ErrorMatches, `dry-start failed: +- boom1 +- boom2`) + c.Check(calls, DeepEquals, []string{"drystart:mgr1", "drystart:mgr2"}) +} + +func (ses *stateEngineSuite) TestEnsureFailsNoDryStart(c *C) { + s := state.New(nil) + se := overlord.NewStateEngine(s) + mgr := &fakeManager{name: "mgr"} + se.AddManager(mgr) + + err := se.Ensure() + c.Check(err, ErrorMatches, `state engine did not dry-start`) } func (ses *stateEngineSuite) TestEnsureError(c *C) { @@ -91,9 +128,14 @@ func (ses *stateEngineSuite) TestEnsureError(c *C) { se.AddManager(mgr1) se.AddManager(mgr2) - err := se.Ensure() - c.Check(err.Error(), DeepEquals, "state ensure errors: [boom1 boom2]") - c.Check(calls, DeepEquals, []string{"ensure:mgr1", "ensure:mgr2"}) + err := se.DryStart() + c.Check(err, IsNil) + + err = se.Ensure() + c.Check(err, ErrorMatches, `state ensure errors: +- boom1 +- boom2`) + c.Check(calls, DeepEquals, []string{"drystart:mgr1", "drystart:mgr2", "ensure:mgr1", "ensure:mgr2"}) } func (ses *stateEngineSuite) TestStop(c *C) { @@ -108,11 +150,14 @@ func (ses *stateEngineSuite) TestStop(c *C) { se.AddManager(mgr1) se.AddManager(mgr2) + err := se.DryStart() + c.Check(err, IsNil) + se.Stop() - c.Check(calls, DeepEquals, []string{"stop:mgr1", "stop:mgr2"}) + c.Check(calls, DeepEquals, []string{"drystart:mgr1", "drystart:mgr2", "stop:mgr1", "stop:mgr2"}) se.Stop() - c.Check(calls, HasLen, 2) + c.Check(calls, HasLen, 4) - err := se.Ensure() + err = se.Ensure() c.Check(err, ErrorMatches, "state engine already stopped") }