From 6c7589a2a53a73d7f954ce50f06a20c4cc4e20c9 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Tue, 8 Aug 2023 17:18:54 +0200
Subject: [PATCH 01/19] overlord: allow adding custom managers

This commit introduces StateEngine.SetTaskRunner and therefore relaxes
the constraint on the ordering of the managers in the StateEngine.
This change has made the inited field of Overlord obsolete: there is
no reason to guard against adding managers after Overlord.New anymore.
So the inited field has been removed too.

In turn, the relaxing of the ordering allows sister projects to add
managers to the Overlord. The Overlord of a given Daemon can now be
reached using a method of Daemon, so the sister project can use this
functionality on their own Daemon.
---
 internals/daemon/daemon.go        |  4 ++++
 internals/overlord/overlord.go    | 11 +----------
 internals/overlord/stateengine.go | 32 ++++++++++++++++++++++++++-----
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 5782f974..67f0a984 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -217,6 +217,10 @@ func userFromRequest(state interface{}, r *http.Request) (*UserState, error) {
 	return nil, nil
 }
 
+func (d *Daemon) Overlord() *overlord.Overlord {
+	return d.overlord
+}
+
 func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	st := c.d.state
 	st.Lock()
diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index 771a670d..8e9ac76e 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -64,7 +64,6 @@ type Overlord struct {
 	pruneTicker *time.Ticker
 
 	// managers
-	inited     bool
 	runner     *state.TaskRunner
 	serviceMgr *servstate.ServiceManager
 	commandMgr *cmdstate.CommandManager
@@ -78,7 +77,6 @@ func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writ
 	o := &Overlord{
 		pebbleDir: pebbleDir,
 		loopTomb:  new(tomb.Tomb),
-		inited:    true,
 	}
 
 	if !filepath.IsAbs(pebbleDir) {
@@ -130,8 +128,7 @@ func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writ
 	// Tell service manager about check failures.
 	o.checkMgr.NotifyCheckFailed(o.serviceMgr.CheckFailed)
 
-	// the shared task runner should be added last!
-	o.stateEng.AddManager(o.runner)
+	o.stateEng.SetTaskRunner(o.runner)
 
 	return o, nil
 }
@@ -427,7 +424,6 @@ func Fake() *Overlord {
 func FakeWithState(handleRestart func(restart.RestartType)) *Overlord {
 	o := &Overlord{
 		loopTomb: new(tomb.Tomb),
-		inited:   false,
 	}
 	s := state.New(fakeBackend{o: o})
 	o.stateEng = NewStateEngine(s)
@@ -435,12 +431,7 @@ func FakeWithState(handleRestart func(restart.RestartType)) *Overlord {
 	return o
 }
 
-// AddManager adds a manager to a fake overlord. It cannot be used for
-// a normally initialized overlord those are already fully populated.
 func (o *Overlord) AddManager(mgr StateManager) {
-	if o.inited {
-		panic("internal error: cannot add managers to a fully initialized Overlord")
-	}
 	o.addManager(mgr)
 }
 
diff --git a/internals/overlord/stateengine.go b/internals/overlord/stateengine.go
index 25710333..42e8dca8 100644
--- a/internals/overlord/stateengine.go
+++ b/internals/overlord/stateengine.go
@@ -56,8 +56,24 @@ type StateEngine struct {
 	state   *state.State
 	stopped bool
 	// managers in use
-	mgrLock  sync.Mutex
-	managers []StateManager
+	mgrLock    sync.Mutex
+	managers   []StateManager
+	taskRunner StateManager
+}
+
+// orderedManagers returns all the managers added to this engine.
+//
+// The managers are ordered by their precedence when executing Ensure, Stop
+// and Wait: the task runner is the last item in the result.
+func (se *StateEngine) orderedManagers() []StateManager {
+	result := make([]StateManager, 0, len(se.managers)+1)
+	for _, m := range se.managers {
+		result = append(result, m)
+	}
+	if se.taskRunner != nil {
+		result = append(result, se.taskRunner)
+	}
+	return result
 }
 
 // NewStateEngine returns a new state engine.
@@ -95,7 +111,7 @@ func (se *StateEngine) Ensure() error {
 		return fmt.Errorf("state engine already stopped")
 	}
 	var errs []error
-	for _, m := range se.managers {
+	for _, m := range se.orderedManagers() {
 		err := m.Ensure()
 		if err != nil {
 			logger.Noticef("state ensure error: %v", err)
@@ -115,6 +131,12 @@ func (se *StateEngine) AddManager(m StateManager) {
 	se.managers = append(se.managers, m)
 }
 
+func (se *StateEngine) SetTaskRunner(taskRunner StateManager) {
+	se.mgrLock.Lock()
+	defer se.mgrLock.Unlock()
+	se.taskRunner = taskRunner
+}
+
 // Wait waits for all managers current activities.
 func (se *StateEngine) Wait() {
 	se.mgrLock.Lock()
@@ -122,7 +144,7 @@ func (se *StateEngine) Wait() {
 	if se.stopped {
 		return
 	}
-	for _, m := range se.managers {
+	for _, m := range se.orderedManagers() {
 		if waiter, ok := m.(StateWaiter); ok {
 			waiter.Wait()
 		}
@@ -136,7 +158,7 @@ func (se *StateEngine) Stop() {
 	if se.stopped {
 		return
 	}
-	for _, m := range se.managers {
+	for _, m := range se.orderedManagers() {
 		if stopper, ok := m.(StateStopper); ok {
 			stopper.Stop()
 		}

From df46e416a437ecc5d0354307b73204dfe216c0c9 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Mon, 21 Aug 2023 18:06:39 +0200
Subject: [PATCH 02/19] pr fix: increase lint timeout

---
 .github/workflows/lint.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 6122d1ea..0c3d4c3f 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -17,7 +17,7 @@ jobs:
         id: lint
         with:
           version: latest
-          args: '-c .github/.golangci.yml --out-format=colored-line-number'
+          args: '-c .github/.golangci.yml --out-format=colored-line-number --timeout 2m0s'
           skip-cache: true
 
       - name: Print error message
@@ -27,4 +27,4 @@ jobs:
           Linting failed. On your local machine, please run
               golangci-lint run -c .github/.golangci.yml --fix
           and check in the changes.'
-          exit 1
\ No newline at end of file
+          exit 1

From f599ab1cccdac606ff298a6eedabb6dcfa1f8168 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Mon, 21 Aug 2023 18:22:14 +0200
Subject: [PATCH 03/19] pr fix: undo removal of inited

---
 internals/overlord/overlord.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index 8e9ac76e..e35812cc 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -64,6 +64,7 @@ type Overlord struct {
 	pruneTicker *time.Ticker
 
 	// managers
+	inited     bool
 	runner     *state.TaskRunner
 	serviceMgr *servstate.ServiceManager
 	commandMgr *cmdstate.CommandManager
@@ -77,6 +78,7 @@ func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writ
 	o := &Overlord{
 		pebbleDir: pebbleDir,
 		loopTomb:  new(tomb.Tomb),
+		inited:    true,
 	}
 
 	if !filepath.IsAbs(pebbleDir) {
@@ -424,6 +426,7 @@ func Fake() *Overlord {
 func FakeWithState(handleRestart func(restart.RestartType)) *Overlord {
 	o := &Overlord{
 		loopTomb: new(tomb.Tomb),
+		inited:   false,
 	}
 	s := state.New(fakeBackend{o: o})
 	o.stateEng = NewStateEngine(s)
@@ -431,7 +434,12 @@ func FakeWithState(handleRestart func(restart.RestartType)) *Overlord {
 	return o
 }
 
+// AddManager adds a manager to a fake overlord. It cannot be used for
+// a normally initialized overlord those are already fully populated.
 func (o *Overlord) AddManager(mgr StateManager) {
+	if o.inited {
+		panic("internal error: cannot add managers to a fully initialized Overlord")
+	}
 	o.addManager(mgr)
 }
 

From 093034c1e6f3cd4505d98f3f1f25aa6ea1035056 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Tue, 22 Aug 2023 17:17:02 +0200
Subject: [PATCH 04/19] pr fix: pass manager generator functions

---
 internals/daemon/daemon.go          |  7 +++-
 internals/daemon/daemon_test.go     | 36 +++++++++++++++++
 internals/overlord/managers_test.go |  2 +-
 internals/overlord/overlord.go      | 62 +++++++++++++++++++++++------
 internals/overlord/overlord_test.go | 28 ++++++-------
 internals/overlord/stateengine.go   | 32 +++------------
 6 files changed, 112 insertions(+), 55 deletions(-)

diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 67f0a984..fd612397 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -71,6 +71,11 @@ type Options struct {
 	// ServiceOuput is an optional io.Writer for the service log output, if set, all services
 	// log output will be written to the writer.
 	ServiceOutput io.Writer
+
+	// ManagerGenerators is an optional slice of ManagerGenerator which will
+	// be passed to the overlord.New to create custom managers at overlord
+	// creation.
+	ManagerGenerators []overlord.ManagerGenerator
 }
 
 // A Daemon listens for requests and routes them to the right command
@@ -787,7 +792,7 @@ func New(opts *Options) (*Daemon, error) {
 		httpAddress:         opts.HTTPAddress,
 	}
 
-	ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput)
+	ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput, opts.ManagerGenerators)
 	if err == errExpectedReboot {
 		// we proceed without overlord until we reach Stop
 		// where we will schedule and wait again for a system restart.
diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index c31f2cc5..1c12cbb8 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -33,6 +33,7 @@ import (
 	. "gopkg.in/check.v1"
 
 	"github.com/canonical/pebble/internals/osutil"
+	"github.com/canonical/pebble/internals/overlord"
 	"github.com/canonical/pebble/internals/overlord/patch"
 	"github.com/canonical/pebble/internals/overlord/restart"
 	"github.com/canonical/pebble/internals/overlord/standby"
@@ -99,6 +100,41 @@ func (h *fakeHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	h.lastMethod = r.Method
 }
 
+type fakeManager struct {
+	id          string
+	ensureCalls int
+}
+
+func (m *fakeManager) Ensure() error {
+	m.ensureCalls++
+	return nil
+}
+
+func (s *daemonSuite) TestExternalManager(c *C) {
+	generators := []overlord.ManagerGenerator{
+		func(o overlord.ManagerProvider) (any, overlord.StateManager) {
+			return fakeManager{}, &fakeManager{id: "expected"}
+		},
+	}
+
+	d, err := New(&Options{
+		Dir:               s.pebbleDir,
+		SocketPath:        s.socketPath,
+		HTTPAddress:       s.httpAddress,
+		ManagerGenerators: generators,
+	})
+	c.Assert(err, IsNil)
+
+	err = d.overlord.StateEngine().Ensure()
+	c.Assert(err, IsNil)
+	managerItf := d.overlord.GetExternalManager(fakeManager{})
+	c.Assert(managerItf, NotNil)
+	concrete, ok := managerItf.(*fakeManager)
+	c.Assert(ok, Equals, true)
+	c.Assert(concrete.id, Equals, "expected")
+	c.Assert(concrete.ensureCalls, Equals, 1)
+}
+
 func (s *daemonSuite) TestAddCommand(c *C) {
 	const endpoint = "/v1/addedendpoint"
 	var handler fakeHandler
diff --git a/internals/overlord/managers_test.go b/internals/overlord/managers_test.go
index 7b59191f..cc5b1a41 100644
--- a/internals/overlord/managers_test.go
+++ b/internals/overlord/managers_test.go
@@ -42,7 +42,7 @@ func (s *mgrsSuite) SetUpTest(c *C) {
 
 	s.dir = c.MkDir()
 
-	o, err := overlord.New(s.dir, nil, nil)
+	o, err := overlord.New(s.dir, nil, nil, nil)
 	c.Assert(err, IsNil)
 	s.o = o
 }
diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index e35812cc..4575a21a 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -64,21 +64,42 @@ type Overlord struct {
 	pruneTicker *time.Ticker
 
 	// managers
-	inited     bool
-	runner     *state.TaskRunner
-	serviceMgr *servstate.ServiceManager
-	commandMgr *cmdstate.CommandManager
-	checkMgr   *checkstate.CheckManager
-	logMgr     *logstate.LogManager
+	inited           bool
+	runner           *state.TaskRunner
+	serviceMgr       *servstate.ServiceManager
+	commandMgr       *cmdstate.CommandManager
+	checkMgr         *checkstate.CheckManager
+	logMgr           *logstate.LogManager
+	externalManagers map[any]StateManager
 }
 
-// New creates a new Overlord with all its state managers.
+// ManagerProvider is the interface that ManagerGenerator depends on
+//
+// Overlord implements ManagerProvider as it provides the necessary
+// handles to hook an external manager into the overlord's environment.
+type ManagerProvider interface {
+	State() *state.State
+	TaskRunner() *state.TaskRunner
+}
+
+// ManagerGenerator is passed to Overlord to create a manager
+// The return value is a key, value pair where the key has to be a unique
+// identifier for the manager being created.
+type ManagerGenerator func(ManagerProvider) (key any, manager StateManager)
+
+// New creates a  Overlord with all its state managers.
 // It can be provided with an optional restart.Handler.
-func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writer) (*Overlord, error) {
+func New(
+	pebbleDir string,
+	restartHandler restart.Handler,
+	serviceOutput io.Writer,
+	generators []ManagerGenerator) (*Overlord, error) {
+
 	o := &Overlord{
-		pebbleDir: pebbleDir,
-		loopTomb:  new(tomb.Tomb),
-		inited:    true,
+		pebbleDir:        pebbleDir,
+		loopTomb:         new(tomb.Tomb),
+		inited:           true,
+		externalManagers: make(map[any]StateManager, len(generators)),
 	}
 
 	if !filepath.IsAbs(pebbleDir) {
@@ -130,11 +151,28 @@ func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writ
 	// Tell service manager about check failures.
 	o.checkMgr.NotifyCheckFailed(o.serviceMgr.CheckFailed)
 
-	o.stateEng.SetTaskRunner(o.runner)
+	for _, gen := range generators {
+		tag, manager := gen(o)
+		o.tagManager(tag, manager)
+	}
+
+	// TaskRunner must be the last manager added to the StateEngine,
+	// because TaskRunner runs all the tasks required by the managers that ran
+	// before it.
+	o.stateEng.AddManager(o.runner)
 
 	return o, nil
 }
 
+func (o *Overlord) tagManager(tag any, mgr StateManager) {
+	o.externalManagers[tag] = mgr
+	o.addManager(mgr)
+}
+
+func (o *Overlord) GetExternalManager(tag any) StateManager {
+	return o.externalManagers[tag]
+}
+
 func (o *Overlord) addManager(mgr StateManager) {
 	o.stateEng.AddManager(mgr)
 }
diff --git a/internals/overlord/overlord_test.go b/internals/overlord/overlord_test.go
index c6b9cc7e..3de056f6 100644
--- a/internals/overlord/overlord_test.go
+++ b/internals/overlord/overlord_test.go
@@ -58,7 +58,7 @@ func (ovs *overlordSuite) TestNew(c *C) {
 	restore := patch.Fake(42, 2, nil)
 	defer restore()
 
-	o, err := overlord.New(ovs.dir, nil, nil)
+	o, err := overlord.New(ovs.dir, nil, nil, nil)
 	c.Assert(err, IsNil)
 	c.Check(o, NotNil)
 
@@ -83,7 +83,7 @@ func (ovs *overlordSuite) TestNewWithGoodState(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	o, err := overlord.New(ovs.dir, nil, nil)
+	o, err := overlord.New(ovs.dir, nil, nil, nil)
 	c.Assert(err, IsNil)
 
 	state := o.State()
@@ -111,7 +111,7 @@ func (ovs *overlordSuite) TestNewWithInvalidState(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	_, err = overlord.New(ovs.dir, nil, nil)
+	_, err = overlord.New(ovs.dir, nil, nil, nil)
 	c.Assert(err, ErrorMatches, "cannot read state: EOF")
 }
 
@@ -130,7 +130,7 @@ func (ovs *overlordSuite) TestNewWithPatches(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	o, err := overlord.New(ovs.dir, nil, nil)
+	o, err := overlord.New(ovs.dir, nil, nil, nil)
 	c.Assert(err, IsNil)
 
 	state := o.State()
@@ -175,7 +175,7 @@ func (wm *witnessManager) Ensure() error {
 }
 
 func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) {
-	o, err := overlord.New(ovs.dir, nil, nil)
+	o, err := overlord.New(ovs.dir, nil, nil, nil)
 	c.Assert(err, IsNil)
 
 	o.Loop()
@@ -185,7 +185,7 @@ func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) {
 }
 
 func (ovs *overlordSuite) TestUnknownTasks(c *C) {
-	o, err := overlord.New(ovs.dir, nil, nil)
+	o, err := overlord.New(ovs.dir, nil, nil, nil)
 	c.Assert(err, IsNil)
 
 	// unknown tasks are ignored and succeed
@@ -486,7 +486,7 @@ func (ovs *overlordSuite) TestCheckpoint(c *C) {
 	oldUmask := syscall.Umask(0)
 	defer syscall.Umask(oldUmask)
 
-	o, err := overlord.New(ovs.dir, nil, nil)
+	o, err := overlord.New(ovs.dir, nil, nil, nil)
 	c.Assert(err, IsNil)
 
 	s := o.State()
@@ -727,7 +727,7 @@ func (ovs *overlordSuite) TestSettleExplicitEnsureBefore(c *C) {
 }
 
 func (ovs *overlordSuite) TestRequestRestartNoHandler(c *C) {
-	o, err := overlord.New(ovs.dir, nil, nil)
+	o, err := overlord.New(ovs.dir, nil, nil, nil)
 	c.Assert(err, IsNil)
 
 	st := o.State()
@@ -760,7 +760,7 @@ func (rb *testRestartHandler) RebootIsMissing(_ *state.State) error {
 func (ovs *overlordSuite) TestRequestRestartHandler(c *C) {
 	rb := &testRestartHandler{}
 
-	o, err := overlord.New(ovs.dir, rb, nil)
+	o, err := overlord.New(ovs.dir, rb, nil, nil)
 	c.Assert(err, IsNil)
 
 	st := o.State()
@@ -779,7 +779,7 @@ func (ovs *overlordSuite) TestVerifyRebootNoPendingReboot(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(ovs.dir, rb, nil)
+	_, err = overlord.New(ovs.dir, rb, nil, nil)
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -792,7 +792,7 @@ func (ovs *overlordSuite) TestVerifyRebootOK(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(ovs.dir, rb, nil)
+	_, err = overlord.New(ovs.dir, rb, nil, nil)
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -806,7 +806,7 @@ func (ovs *overlordSuite) TestVerifyRebootOKButError(c *C) {
 	e := errors.New("boom")
 	rb := &testRestartHandler{rebootVerifiedErr: e}
 
-	_, err = overlord.New(ovs.dir, rb, nil)
+	_, err = overlord.New(ovs.dir, rb, nil, nil)
 	c.Assert(err, Equals, e)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -822,7 +822,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissing(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(ovs.dir, rb, nil)
+	_, err = overlord.New(ovs.dir, rb, nil, nil)
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "did-not-happen")
@@ -839,7 +839,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissingError(c *C) {
 	e := errors.New("boom")
 	rb := &testRestartHandler{rebootVerifiedErr: e}
 
-	_, err = overlord.New(ovs.dir, rb, nil)
+	_, err = overlord.New(ovs.dir, rb, nil, nil)
 	c.Assert(err, Equals, e)
 
 	c.Check(rb.rebootState, Equals, "did-not-happen")
diff --git a/internals/overlord/stateengine.go b/internals/overlord/stateengine.go
index 42e8dca8..25710333 100644
--- a/internals/overlord/stateengine.go
+++ b/internals/overlord/stateengine.go
@@ -56,24 +56,8 @@ type StateEngine struct {
 	state   *state.State
 	stopped bool
 	// managers in use
-	mgrLock    sync.Mutex
-	managers   []StateManager
-	taskRunner StateManager
-}
-
-// orderedManagers returns all the managers added to this engine.
-//
-// The managers are ordered by their precedence when executing Ensure, Stop
-// and Wait: the task runner is the last item in the result.
-func (se *StateEngine) orderedManagers() []StateManager {
-	result := make([]StateManager, 0, len(se.managers)+1)
-	for _, m := range se.managers {
-		result = append(result, m)
-	}
-	if se.taskRunner != nil {
-		result = append(result, se.taskRunner)
-	}
-	return result
+	mgrLock  sync.Mutex
+	managers []StateManager
 }
 
 // NewStateEngine returns a new state engine.
@@ -111,7 +95,7 @@ func (se *StateEngine) Ensure() error {
 		return fmt.Errorf("state engine already stopped")
 	}
 	var errs []error
-	for _, m := range se.orderedManagers() {
+	for _, m := range se.managers {
 		err := m.Ensure()
 		if err != nil {
 			logger.Noticef("state ensure error: %v", err)
@@ -131,12 +115,6 @@ func (se *StateEngine) AddManager(m StateManager) {
 	se.managers = append(se.managers, m)
 }
 
-func (se *StateEngine) SetTaskRunner(taskRunner StateManager) {
-	se.mgrLock.Lock()
-	defer se.mgrLock.Unlock()
-	se.taskRunner = taskRunner
-}
-
 // Wait waits for all managers current activities.
 func (se *StateEngine) Wait() {
 	se.mgrLock.Lock()
@@ -144,7 +122,7 @@ func (se *StateEngine) Wait() {
 	if se.stopped {
 		return
 	}
-	for _, m := range se.orderedManagers() {
+	for _, m := range se.managers {
 		if waiter, ok := m.(StateWaiter); ok {
 			waiter.Wait()
 		}
@@ -158,7 +136,7 @@ func (se *StateEngine) Stop() {
 	if se.stopped {
 		return
 	}
-	for _, m := range se.orderedManagers() {
+	for _, m := range se.managers {
 		if stopper, ok := m.(StateStopper); ok {
 			stopper.Stop()
 		}

From bbba226342a090a2aa80344d04d2bf38fac84d6f Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Tue, 22 Aug 2023 17:22:04 +0200
Subject: [PATCH 05/19] pr fix: conflict-free lint timeout fix

---
 .github/.golangci.yml      | 6 +++++-
 .github/workflows/lint.yml | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/.github/.golangci.yml b/.github/.golangci.yml
index 3f49df42..2e368060 100644
--- a/.github/.golangci.yml
+++ b/.github/.golangci.yml
@@ -11,4 +11,8 @@ linters-settings:
 issues:
   # these values ensure that all issues will be surfaced
   max-issues-per-linter: 0
-  max-same-issues: 0
\ No newline at end of file
+  max-same-issues: 0
+
+run:
+  timeout: 5m
+
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 0c3d4c3f..b2e2f189 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -17,7 +17,7 @@ jobs:
         id: lint
         with:
           version: latest
-          args: '-c .github/.golangci.yml --out-format=colored-line-number --timeout 2m0s'
+          args: '-c .github/.golangci.yml --out-format=colored-line-number'
           skip-cache: true
 
       - name: Print error message

From 56d45ef0e3c312df36b7851b448c9c33160b724c Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Wed, 23 Aug 2023 09:07:51 +0200
Subject: [PATCH 06/19] pr fix: inline tagManager, panic on tag not found

---
 internals/overlord/overlord.go | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index 4575a21a..80395887 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -153,7 +153,8 @@ func New(
 
 	for _, gen := range generators {
 		tag, manager := gen(o)
-		o.tagManager(tag, manager)
+		o.externalManagers[tag] = manager
+		o.addManager(manager)
 	}
 
 	// TaskRunner must be the last manager added to the StateEngine,
@@ -164,13 +165,12 @@ func New(
 	return o, nil
 }
 
-func (o *Overlord) tagManager(tag any, mgr StateManager) {
-	o.externalManagers[tag] = mgr
-	o.addManager(mgr)
-}
-
 func (o *Overlord) GetExternalManager(tag any) StateManager {
-	return o.externalManagers[tag]
+	result, ok := o.externalManagers[tag]
+	if !ok {
+		panic("Manager tag not found")
+	}
+	return result
 }
 
 func (o *Overlord) addManager(mgr StateManager) {

From a552e09f9cd864fcdd35977eff72cc695579fb57 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Wed, 23 Aug 2023 09:11:23 +0200
Subject: [PATCH 07/19] pr fix: external manager creation error handling

---
 internals/daemon/daemon_test.go | 4 ++--
 internals/overlord/overlord.go  | 7 +++++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 1c12cbb8..0d0c43f7 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -112,8 +112,8 @@ func (m *fakeManager) Ensure() error {
 
 func (s *daemonSuite) TestExternalManager(c *C) {
 	generators := []overlord.ManagerGenerator{
-		func(o overlord.ManagerProvider) (any, overlord.StateManager) {
-			return fakeManager{}, &fakeManager{id: "expected"}
+		func(o overlord.ManagerProvider) (any, overlord.StateManager, error) {
+			return fakeManager{}, &fakeManager{id: "expected"}, nil
 		},
 	}
 
diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index 80395887..9439f00d 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -85,7 +85,7 @@ type ManagerProvider interface {
 // ManagerGenerator is passed to Overlord to create a manager
 // The return value is a key, value pair where the key has to be a unique
 // identifier for the manager being created.
-type ManagerGenerator func(ManagerProvider) (key any, manager StateManager)
+type ManagerGenerator func(ManagerProvider) (key any, manager StateManager, err error)
 
 // New creates a  Overlord with all its state managers.
 // It can be provided with an optional restart.Handler.
@@ -152,7 +152,10 @@ func New(
 	o.checkMgr.NotifyCheckFailed(o.serviceMgr.CheckFailed)
 
 	for _, gen := range generators {
-		tag, manager := gen(o)
+		tag, manager, err := gen(o)
+		if err != nil {
+			return nil, err
+		}
 		o.externalManagers[tag] = manager
 		o.addManager(manager)
 	}

From 4f0f98aa9eb61ae70b08468befdd48fd2cfe6f50 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Wed, 23 Aug 2023 10:38:09 +0200
Subject: [PATCH 08/19] pr fix: refactor generators

---
 internals/daemon/daemon.go      |  6 +++---
 internals/daemon/daemon_test.go | 14 +++++++-------
 internals/overlord/overlord.go  | 14 +++++++-------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index fd612397..dfcc7e25 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -72,10 +72,10 @@ type Options struct {
 	// log output will be written to the writer.
 	ServiceOutput io.Writer
 
-	// ManagerGenerators is an optional slice of ManagerGenerator which will
+	// NewManagerFuncs is an optional slice of NewManagerFunc which will
 	// be passed to the overlord.New to create custom managers at overlord
 	// creation.
-	ManagerGenerators []overlord.ManagerGenerator
+	NewManagerFuncs []overlord.NewManagerFunc
 }
 
 // A Daemon listens for requests and routes them to the right command
@@ -792,7 +792,7 @@ func New(opts *Options) (*Daemon, error) {
 		httpAddress:         opts.HTTPAddress,
 	}
 
-	ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput, opts.ManagerGenerators)
+	ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput, opts.NewManagerFuncs)
 	if err == errExpectedReboot {
 		// we proceed without overlord until we reach Stop
 		// where we will schedule and wait again for a system restart.
diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 0d0c43f7..11ba9610 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -111,23 +111,23 @@ func (m *fakeManager) Ensure() error {
 }
 
 func (s *daemonSuite) TestExternalManager(c *C) {
-	generators := []overlord.ManagerGenerator{
-		func(o overlord.ManagerProvider) (any, overlord.StateManager, error) {
+	generators := []overlord.NewManagerFunc{
+		func(o overlord.ManagerEnvironment) (any, overlord.StateManager, error) {
 			return fakeManager{}, &fakeManager{id: "expected"}, nil
 		},
 	}
 
 	d, err := New(&Options{
-		Dir:               s.pebbleDir,
-		SocketPath:        s.socketPath,
-		HTTPAddress:       s.httpAddress,
-		ManagerGenerators: generators,
+		Dir:             s.pebbleDir,
+		SocketPath:      s.socketPath,
+		HTTPAddress:     s.httpAddress,
+		NewManagerFuncs: generators,
 	})
 	c.Assert(err, IsNil)
 
 	err = d.overlord.StateEngine().Ensure()
 	c.Assert(err, IsNil)
-	managerItf := d.overlord.GetExternalManager(fakeManager{})
+	managerItf := d.overlord.ExternalManager(fakeManager{})
 	c.Assert(managerItf, NotNil)
 	concrete, ok := managerItf.(*fakeManager)
 	c.Assert(ok, Equals, true)
diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index 9439f00d..a11734c7 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -73,19 +73,19 @@ type Overlord struct {
 	externalManagers map[any]StateManager
 }
 
-// ManagerProvider is the interface that ManagerGenerator depends on
+// ManagerEnvironment is the interface that NewManagerFunc depends on
 //
-// Overlord implements ManagerProvider as it provides the necessary
+// Overlord implements ManagerEnvironment as it provides the necessary
 // handles to hook an external manager into the overlord's environment.
-type ManagerProvider interface {
+type ManagerEnvironment interface {
 	State() *state.State
 	TaskRunner() *state.TaskRunner
 }
 
-// ManagerGenerator is passed to Overlord to create a manager
+// NewManagerFunc is passed to Overlord to create a manager
 // The return value is a key, value pair where the key has to be a unique
 // identifier for the manager being created.
-type ManagerGenerator func(ManagerProvider) (key any, manager StateManager, err error)
+type NewManagerFunc func(ManagerEnvironment) (key any, manager StateManager, err error)
 
 // New creates a  Overlord with all its state managers.
 // It can be provided with an optional restart.Handler.
@@ -93,7 +93,7 @@ func New(
 	pebbleDir string,
 	restartHandler restart.Handler,
 	serviceOutput io.Writer,
-	generators []ManagerGenerator) (*Overlord, error) {
+	generators []NewManagerFunc) (*Overlord, error) {
 
 	o := &Overlord{
 		pebbleDir:        pebbleDir,
@@ -168,7 +168,7 @@ func New(
 	return o, nil
 }
 
-func (o *Overlord) GetExternalManager(tag any) StateManager {
+func (o *Overlord) ExternalManager(tag any) StateManager {
 	result, ok := o.externalManagers[tag]
 	if !ok {
 		panic("Manager tag not found")

From da8c2c339d4f8378582eeb4f56fedab9e53622d0 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Thu, 24 Aug 2023 09:58:08 +0200
Subject: [PATCH 09/19] pr fix: overlord.Extension

---
 internals/daemon/daemon.go      |  9 +++---
 internals/daemon/daemon_test.go | 34 +++++++++++++----------
 internals/overlord/overlord.go  | 49 ++++++++++++++++++---------------
 3 files changed, 50 insertions(+), 42 deletions(-)

diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index dfcc7e25..817b56f0 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -72,10 +72,9 @@ type Options struct {
 	// log output will be written to the writer.
 	ServiceOutput io.Writer
 
-	// NewManagerFuncs is an optional slice of NewManagerFunc which will
-	// be passed to the overlord.New to create custom managers at overlord
-	// creation.
-	NewManagerFuncs []overlord.NewManagerFunc
+	// OverlordExtension is an optional struct used to extend the capabilities
+	// of the Overlord.
+	OverlordExtension overlord.Extension
 }
 
 // A Daemon listens for requests and routes them to the right command
@@ -792,7 +791,7 @@ func New(opts *Options) (*Daemon, error) {
 		httpAddress:         opts.HTTPAddress,
 	}
 
-	ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput, opts.NewManagerFuncs)
+	ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput, opts.OverlordExtension)
 	if err == errExpectedReboot {
 		// we proceed without overlord until we reach Stop
 		// where we will schedule and wait again for a system restart.
diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index 11ba9610..dbe9d40d 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -110,29 +110,33 @@ func (m *fakeManager) Ensure() error {
 	return nil
 }
 
-func (s *daemonSuite) TestExternalManager(c *C) {
-	generators := []overlord.NewManagerFunc{
-		func(o overlord.ManagerEnvironment) (any, overlord.StateManager, error) {
-			return fakeManager{}, &fakeManager{id: "expected"}, nil
-		},
-	}
+type fakeExtension struct {
+	mgr fakeManager
+}
 
+func (f *fakeExtension) ExtraManagers(o *overlord.Overlord) ([]overlord.StateManager, error) {
+	f.mgr = fakeManager{id: "expected", ensureCalls: 0}
+	result := []overlord.StateManager{&f.mgr}
+	return result, nil
+}
+
+func (s *daemonSuite) TestExternalManager(c *C) {
 	d, err := New(&Options{
-		Dir:             s.pebbleDir,
-		SocketPath:      s.socketPath,
-		HTTPAddress:     s.httpAddress,
-		NewManagerFuncs: generators,
+		Dir:               s.pebbleDir,
+		SocketPath:        s.socketPath,
+		HTTPAddress:       s.httpAddress,
+		OverlordExtension: &fakeExtension{},
 	})
 	c.Assert(err, IsNil)
 
 	err = d.overlord.StateEngine().Ensure()
 	c.Assert(err, IsNil)
-	managerItf := d.overlord.ExternalManager(fakeManager{})
-	c.Assert(managerItf, NotNil)
-	concrete, ok := managerItf.(*fakeManager)
+	extension, ok := d.overlord.Extension().(*fakeExtension)
 	c.Assert(ok, Equals, true)
-	c.Assert(concrete.id, Equals, "expected")
-	c.Assert(concrete.ensureCalls, Equals, 1)
+	c.Assert(extension, NotNil)
+	manager := extension.mgr
+	c.Assert(manager.id, Equals, "expected")
+	c.Assert(manager.ensureCalls, Equals, 1)
 }
 
 func (s *daemonSuite) TestAddCommand(c *C) {
diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index a11734c7..f2855150 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -49,6 +49,12 @@ var (
 	defaultCachedDownloads = 5
 )
 
+// Extension represents an extension of the Overlord
+type Extension interface {
+	// ExtraManagers is called when building an Overlord
+	ExtraManagers(o *Overlord) ([]StateManager, error)
+}
+
 // Overlord is the central manager of the system, keeping track
 // of all available state managers and related helpers.
 type Overlord struct {
@@ -64,13 +70,15 @@ type Overlord struct {
 	pruneTicker *time.Ticker
 
 	// managers
-	inited           bool
-	runner           *state.TaskRunner
-	serviceMgr       *servstate.ServiceManager
-	commandMgr       *cmdstate.CommandManager
-	checkMgr         *checkstate.CheckManager
-	logMgr           *logstate.LogManager
-	externalManagers map[any]StateManager
+	inited     bool
+	runner     *state.TaskRunner
+	serviceMgr *servstate.ServiceManager
+	commandMgr *cmdstate.CommandManager
+	checkMgr   *checkstate.CheckManager
+	logMgr     *logstate.LogManager
+
+	// Overlord extension
+	extension Extension
 }
 
 // ManagerEnvironment is the interface that NewManagerFunc depends on
@@ -93,13 +101,13 @@ func New(
 	pebbleDir string,
 	restartHandler restart.Handler,
 	serviceOutput io.Writer,
-	generators []NewManagerFunc) (*Overlord, error) {
+	extension Extension) (*Overlord, error) {
 
 	o := &Overlord{
-		pebbleDir:        pebbleDir,
-		loopTomb:         new(tomb.Tomb),
-		inited:           true,
-		externalManagers: make(map[any]StateManager, len(generators)),
+		pebbleDir: pebbleDir,
+		loopTomb:  new(tomb.Tomb),
+		inited:    true,
+		extension: extension,
 	}
 
 	if !filepath.IsAbs(pebbleDir) {
@@ -151,13 +159,14 @@ func New(
 	// Tell service manager about check failures.
 	o.checkMgr.NotifyCheckFailed(o.serviceMgr.CheckFailed)
 
-	for _, gen := range generators {
-		tag, manager, err := gen(o)
+	if extension != nil {
+		extraManagers, err := o.extension.ExtraManagers(o)
 		if err != nil {
 			return nil, err
 		}
-		o.externalManagers[tag] = manager
-		o.addManager(manager)
+		for _, manager := range extraManagers {
+			o.addManager(manager)
+		}
 	}
 
 	// TaskRunner must be the last manager added to the StateEngine,
@@ -168,12 +177,8 @@ func New(
 	return o, nil
 }
 
-func (o *Overlord) ExternalManager(tag any) StateManager {
-	result, ok := o.externalManagers[tag]
-	if !ok {
-		panic("Manager tag not found")
-	}
-	return result
+func (o *Overlord) Extension() Extension {
+	return o.extension
 }
 
 func (o *Overlord) addManager(mgr StateManager) {

From f0f1c80e2e97ead0cdce30917ffbdc34bd5cfd49 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Thu, 24 Aug 2023 10:19:20 +0200
Subject: [PATCH 10/19] pr fix: cleanup

---
 internals/overlord/overlord.go | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index f2855150..a97097db 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -81,20 +81,6 @@ type Overlord struct {
 	extension Extension
 }
 
-// ManagerEnvironment is the interface that NewManagerFunc depends on
-//
-// Overlord implements ManagerEnvironment as it provides the necessary
-// handles to hook an external manager into the overlord's environment.
-type ManagerEnvironment interface {
-	State() *state.State
-	TaskRunner() *state.TaskRunner
-}
-
-// NewManagerFunc is passed to Overlord to create a manager
-// The return value is a key, value pair where the key has to be a unique
-// identifier for the manager being created.
-type NewManagerFunc func(ManagerEnvironment) (key any, manager StateManager, err error)
-
 // New creates a  Overlord with all its state managers.
 // It can be provided with an optional restart.Handler.
 func New(

From 56032ce2899e9c6877dbd536c76522ad4bc1372b Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Thu, 24 Aug 2023 10:39:51 +0200
Subject: [PATCH 11/19] pr fix: stateEng.AddManager()

---
 internals/overlord/overlord.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index a97097db..8c5db573 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -151,7 +151,7 @@ func New(
 			return nil, err
 		}
 		for _, manager := range extraManagers {
-			o.addManager(manager)
+			o.stateEng.AddManager(manager)
 		}
 	}
 

From 7a5225b11a63400a195bbf1ecbb3c9918735d2af Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Thu, 24 Aug 2023 10:45:03 +0200
Subject: [PATCH 12/19] pr fix: comments

---
 internals/daemon/daemon.go     | 2 +-
 internals/overlord/overlord.go | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 817b56f0..1401c67e 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -72,7 +72,7 @@ type Options struct {
 	// log output will be written to the writer.
 	ServiceOutput io.Writer
 
-	// OverlordExtension is an optional struct used to extend the capabilities
+	// OverlordExtension is an optional interface used to extend the capabilities
 	// of the Overlord.
 	OverlordExtension overlord.Extension
 }
diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index 8c5db573..f12cdf0a 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -77,11 +77,10 @@ type Overlord struct {
 	checkMgr   *checkstate.CheckManager
 	logMgr     *logstate.LogManager
 
-	// Overlord extension
 	extension Extension
 }
 
-// New creates a  Overlord with all its state managers.
+// New creates an Overlord with all its state managers.
 // It can be provided with an optional restart.Handler.
 func New(
 	pebbleDir string,

From 2228d725f3098d3a34f0a411355c92ae8e539c49 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Thu, 24 Aug 2023 13:24:26 +0200
Subject: [PATCH 13/19] pr fix: overlord.Options

---
 internals/daemon/daemon.go          |  9 +++++-
 internals/overlord/managers_test.go |  2 +-
 internals/overlord/overlord.go      | 50 ++++++++++++++++++-----------
 internals/overlord/overlord_test.go | 28 ++++++++--------
 4 files changed, 55 insertions(+), 34 deletions(-)

diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 1401c67e..72ec2c56 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -791,7 +791,14 @@ func New(opts *Options) (*Daemon, error) {
 		httpAddress:         opts.HTTPAddress,
 	}
 
-	ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput, opts.OverlordExtension)
+	ovldOptions := overlord.Options{
+		PebbleDir:      opts.Dir,
+		RestartHandler: d,
+		ServiceOutput:  opts.ServiceOutput,
+		Extension:      opts.OverlordExtension,
+	}
+
+	ovld, err := overlord.New(ovldOptions)
 	if err == errExpectedReboot {
 		// we proceed without overlord until we reach Stop
 		// where we will schedule and wait again for a system restart.
diff --git a/internals/overlord/managers_test.go b/internals/overlord/managers_test.go
index cc5b1a41..c883b0ee 100644
--- a/internals/overlord/managers_test.go
+++ b/internals/overlord/managers_test.go
@@ -42,7 +42,7 @@ func (s *mgrsSuite) SetUpTest(c *C) {
 
 	s.dir = c.MkDir()
 
-	o, err := overlord.New(s.dir, nil, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: s.dir})
 	c.Assert(err, IsNil)
 	s.o = o
 }
diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index f12cdf0a..7bb0b383 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -49,12 +49,25 @@ var (
 	defaultCachedDownloads = 5
 )
 
-// Extension represents an extension of the Overlord
+// Extension represents an extension of the Overlord.
 type Extension interface {
-	// ExtraManagers is called when building an Overlord
+	// ExtraManagers is called when building an Overlord.
 	ExtraManagers(o *Overlord) ([]StateManager, error)
 }
 
+// Options is the arguments passed to construct an Overlord.
+type Options struct {
+	// PebbleDir is the path to the pebble directory. It must be provided.
+	PebbleDir string
+	// RestartHandler is an optional structure to handle restart requests.
+	RestartHandler restart.Handler
+	// ServiceOutput is an optional output for the logging manager.
+	ServiceOutput io.Writer
+	// Extension is an optional struct defining additional components to add to
+	// the overlord.
+	Extension Extension
+}
+
 // Overlord is the central manager of the system, keeping track
 // of all available state managers and related helpers.
 type Overlord struct {
@@ -81,33 +94,28 @@ type Overlord struct {
 }
 
 // New creates an Overlord with all its state managers.
-// It can be provided with an optional restart.Handler.
-func New(
-	pebbleDir string,
-	restartHandler restart.Handler,
-	serviceOutput io.Writer,
-	extension Extension) (*Overlord, error) {
+func New(opts Options) (*Overlord, error) {
 
 	o := &Overlord{
-		pebbleDir: pebbleDir,
+		pebbleDir: opts.PebbleDir,
 		loopTomb:  new(tomb.Tomb),
 		inited:    true,
-		extension: extension,
+		extension: opts.Extension,
 	}
 
-	if !filepath.IsAbs(pebbleDir) {
-		return nil, fmt.Errorf("directory %q must be absolute", pebbleDir)
+	if !filepath.IsAbs(o.pebbleDir) {
+		return nil, fmt.Errorf("directory %q must be absolute", o.pebbleDir)
 	}
-	if !osutil.IsDir(pebbleDir) {
-		return nil, fmt.Errorf("directory %q does not exist", pebbleDir)
+	if !osutil.IsDir(o.pebbleDir) {
+		return nil, fmt.Errorf("directory %q does not exist", o.pebbleDir)
 	}
-	statePath := filepath.Join(pebbleDir, ".pebble.state")
+	statePath := filepath.Join(o.pebbleDir, ".pebble.state")
 
 	backend := &overlordStateBackend{
 		path:         statePath,
 		ensureBefore: o.ensureBefore,
 	}
-	s, err := loadState(statePath, restartHandler, backend)
+	s, err := loadState(statePath, opts.RestartHandler, backend)
 	if err != nil {
 		return nil, err
 	}
@@ -124,7 +132,13 @@ func New(
 	o.logMgr = logstate.NewLogManager()
 	o.addManager(o.logMgr)
 
-	o.serviceMgr, err = servstate.NewManager(s, o.runner, o.pebbleDir, serviceOutput, restartHandler, o.logMgr)
+	o.serviceMgr, err = servstate.NewManager(
+		s,
+		o.runner,
+		o.pebbleDir,
+		opts.ServiceOutput,
+		opts.RestartHandler,
+		o.logMgr)
 	if err != nil {
 		return nil, err
 	}
@@ -144,7 +158,7 @@ func New(
 	// Tell service manager about check failures.
 	o.checkMgr.NotifyCheckFailed(o.serviceMgr.CheckFailed)
 
-	if extension != nil {
+	if o.extension != nil {
 		extraManagers, err := o.extension.ExtraManagers(o)
 		if err != nil {
 			return nil, err
diff --git a/internals/overlord/overlord_test.go b/internals/overlord/overlord_test.go
index 3de056f6..703ee0af 100644
--- a/internals/overlord/overlord_test.go
+++ b/internals/overlord/overlord_test.go
@@ -58,7 +58,7 @@ func (ovs *overlordSuite) TestNew(c *C) {
 	restore := patch.Fake(42, 2, nil)
 	defer restore()
 
-	o, err := overlord.New(ovs.dir, nil, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 	c.Check(o, NotNil)
 
@@ -83,7 +83,7 @@ func (ovs *overlordSuite) TestNewWithGoodState(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	o, err := overlord.New(ovs.dir, nil, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	state := o.State()
@@ -111,7 +111,7 @@ func (ovs *overlordSuite) TestNewWithInvalidState(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	_, err = overlord.New(ovs.dir, nil, nil, nil)
+	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, ErrorMatches, "cannot read state: EOF")
 }
 
@@ -130,7 +130,7 @@ func (ovs *overlordSuite) TestNewWithPatches(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	o, err := overlord.New(ovs.dir, nil, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	state := o.State()
@@ -175,7 +175,7 @@ func (wm *witnessManager) Ensure() error {
 }
 
 func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) {
-	o, err := overlord.New(ovs.dir, nil, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	o.Loop()
@@ -185,7 +185,7 @@ func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) {
 }
 
 func (ovs *overlordSuite) TestUnknownTasks(c *C) {
-	o, err := overlord.New(ovs.dir, nil, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	// unknown tasks are ignored and succeed
@@ -486,7 +486,7 @@ func (ovs *overlordSuite) TestCheckpoint(c *C) {
 	oldUmask := syscall.Umask(0)
 	defer syscall.Umask(oldUmask)
 
-	o, err := overlord.New(ovs.dir, nil, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	s := o.State()
@@ -727,7 +727,7 @@ func (ovs *overlordSuite) TestSettleExplicitEnsureBefore(c *C) {
 }
 
 func (ovs *overlordSuite) TestRequestRestartNoHandler(c *C) {
-	o, err := overlord.New(ovs.dir, nil, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	st := o.State()
@@ -760,7 +760,7 @@ func (rb *testRestartHandler) RebootIsMissing(_ *state.State) error {
 func (ovs *overlordSuite) TestRequestRestartHandler(c *C) {
 	rb := &testRestartHandler{}
 
-	o, err := overlord.New(ovs.dir, rb, nil, nil)
+	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, IsNil)
 
 	st := o.State()
@@ -779,7 +779,7 @@ func (ovs *overlordSuite) TestVerifyRebootNoPendingReboot(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(ovs.dir, rb, nil, nil)
+	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -792,7 +792,7 @@ func (ovs *overlordSuite) TestVerifyRebootOK(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(ovs.dir, rb, nil, nil)
+	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -806,7 +806,7 @@ func (ovs *overlordSuite) TestVerifyRebootOKButError(c *C) {
 	e := errors.New("boom")
 	rb := &testRestartHandler{rebootVerifiedErr: e}
 
-	_, err = overlord.New(ovs.dir, rb, nil, nil)
+	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, Equals, e)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -822,7 +822,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissing(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(ovs.dir, rb, nil, nil)
+	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "did-not-happen")
@@ -839,7 +839,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissingError(c *C) {
 	e := errors.New("boom")
 	rb := &testRestartHandler{rebootVerifiedErr: e}
 
-	_, err = overlord.New(ovs.dir, rb, nil, nil)
+	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, Equals, e)
 
 	c.Check(rb.rebootState, Equals, "did-not-happen")

From b3654efdab9de79e3014a83bcc36125234282f57 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Thu, 24 Aug 2023 13:31:00 +0200
Subject: [PATCH 14/19] pr fix: missing tests

---
 internals/daemon/daemon_test.go | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/internals/daemon/daemon_test.go b/internals/daemon/daemon_test.go
index dbe9d40d..0d747c55 100644
--- a/internals/daemon/daemon_test.go
+++ b/internals/daemon/daemon_test.go
@@ -120,6 +120,12 @@ func (f *fakeExtension) ExtraManagers(o *overlord.Overlord) ([]overlord.StateMan
 	return result, nil
 }
 
+type otherFakeExtension struct{}
+
+func (otherFakeExtension) ExtraManagers(o *overlord.Overlord) ([]overlord.StateManager, error) {
+	return nil, nil
+}
+
 func (s *daemonSuite) TestExternalManager(c *C) {
 	d, err := New(&Options{
 		Dir:               s.pebbleDir,
@@ -133,12 +139,36 @@ func (s *daemonSuite) TestExternalManager(c *C) {
 	c.Assert(err, IsNil)
 	extension, ok := d.overlord.Extension().(*fakeExtension)
 	c.Assert(ok, Equals, true)
-	c.Assert(extension, NotNil)
 	manager := extension.mgr
 	c.Assert(manager.id, Equals, "expected")
 	c.Assert(manager.ensureCalls, Equals, 1)
 }
 
+func (s *daemonSuite) TestNoExtension(c *C) {
+	d, err := New(&Options{
+		Dir:         s.pebbleDir,
+		SocketPath:  s.socketPath,
+		HTTPAddress: s.httpAddress,
+	})
+	c.Assert(err, IsNil)
+
+	extension := d.overlord.Extension()
+	c.Assert(extension, IsNil)
+}
+
+func (s *daemonSuite) TestWrongExtension(c *C) {
+	d, err := New(&Options{
+		Dir:               s.pebbleDir,
+		SocketPath:        s.socketPath,
+		HTTPAddress:       s.httpAddress,
+		OverlordExtension: &fakeExtension{},
+	})
+	c.Assert(err, IsNil)
+
+	_, ok := d.overlord.Extension().(*otherFakeExtension)
+	c.Assert(ok, Equals, false)
+}
+
 func (s *daemonSuite) TestAddCommand(c *C) {
 	const endpoint = "/v1/addedendpoint"
 	var handler fakeHandler

From a5bb04aa43ffdd1fed6473e06d40359df93303bf Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Fri, 25 Aug 2023 09:22:22 +0200
Subject: [PATCH 15/19] pr fix: remove overlord.addManager()

---
 internals/overlord/overlord.go | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index 7bb0b383..b1e04589 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -130,7 +130,7 @@ func New(opts Options) (*Overlord, error) {
 	o.runner.AddOptionalHandler(matchAnyUnknownTask, handleUnknownTask, nil)
 
 	o.logMgr = logstate.NewLogManager()
-	o.addManager(o.logMgr)
+	o.stateEng.AddManager(o.logMgr)
 
 	o.serviceMgr, err = servstate.NewManager(
 		s,
@@ -142,10 +142,10 @@ func New(opts Options) (*Overlord, error) {
 	if err != nil {
 		return nil, err
 	}
-	o.addManager(o.serviceMgr)
+	o.stateEng.AddManager(o.serviceMgr)
 
 	o.commandMgr = cmdstate.NewManager(o.runner)
-	o.addManager(o.commandMgr)
+	o.stateEng.AddManager(o.commandMgr)
 
 	o.checkMgr = checkstate.NewManager()
 
@@ -180,10 +180,6 @@ func (o *Overlord) Extension() Extension {
 	return o.extension
 }
 
-func (o *Overlord) addManager(mgr StateManager) {
-	o.stateEng.AddManager(mgr)
-}
-
 func loadState(statePath string, restartHandler restart.Handler, backend state.Backend) (*state.State, error) {
 	timings := timing.Start("", "", map[string]string{"startup": "load-state"})
 
@@ -485,7 +481,7 @@ func (o *Overlord) AddManager(mgr StateManager) {
 	if o.inited {
 		panic("internal error: cannot add managers to a fully initialized Overlord")
 	}
-	o.addManager(mgr)
+	o.stateEng.AddManager(mgr)
 }
 
 type fakeBackend struct {

From 9f29faf0000186fd67843cba5a581d1ddd10fa96 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Fri, 25 Aug 2023 09:24:45 +0200
Subject: [PATCH 16/19] pr fix: use pointer to Options

---
 internals/daemon/daemon.go          |  2 +-
 internals/overlord/managers_test.go |  2 +-
 internals/overlord/overlord.go      |  2 +-
 internals/overlord/overlord_test.go | 28 ++++++++++++++--------------
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/internals/daemon/daemon.go b/internals/daemon/daemon.go
index 72ec2c56..b4400b29 100644
--- a/internals/daemon/daemon.go
+++ b/internals/daemon/daemon.go
@@ -798,7 +798,7 @@ func New(opts *Options) (*Daemon, error) {
 		Extension:      opts.OverlordExtension,
 	}
 
-	ovld, err := overlord.New(ovldOptions)
+	ovld, err := overlord.New(&ovldOptions)
 	if err == errExpectedReboot {
 		// we proceed without overlord until we reach Stop
 		// where we will schedule and wait again for a system restart.
diff --git a/internals/overlord/managers_test.go b/internals/overlord/managers_test.go
index c883b0ee..834b4c93 100644
--- a/internals/overlord/managers_test.go
+++ b/internals/overlord/managers_test.go
@@ -42,7 +42,7 @@ func (s *mgrsSuite) SetUpTest(c *C) {
 
 	s.dir = c.MkDir()
 
-	o, err := overlord.New(overlord.Options{PebbleDir: s.dir})
+	o, err := overlord.New(&overlord.Options{PebbleDir: s.dir})
 	c.Assert(err, IsNil)
 	s.o = o
 }
diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index b1e04589..be4fb633 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -94,7 +94,7 @@ type Overlord struct {
 }
 
 // New creates an Overlord with all its state managers.
-func New(opts Options) (*Overlord, error) {
+func New(opts *Options) (*Overlord, error) {
 
 	o := &Overlord{
 		pebbleDir: opts.PebbleDir,
diff --git a/internals/overlord/overlord_test.go b/internals/overlord/overlord_test.go
index 703ee0af..fd9d7682 100644
--- a/internals/overlord/overlord_test.go
+++ b/internals/overlord/overlord_test.go
@@ -58,7 +58,7 @@ func (ovs *overlordSuite) TestNew(c *C) {
 	restore := patch.Fake(42, 2, nil)
 	defer restore()
 
-	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
+	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 	c.Check(o, NotNil)
 
@@ -83,7 +83,7 @@ func (ovs *overlordSuite) TestNewWithGoodState(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
+	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	state := o.State()
@@ -111,7 +111,7 @@ func (ovs *overlordSuite) TestNewWithInvalidState(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir})
+	_, err = overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, ErrorMatches, "cannot read state: EOF")
 }
 
@@ -130,7 +130,7 @@ func (ovs *overlordSuite) TestNewWithPatches(c *C) {
 	err := ioutil.WriteFile(ovs.statePath, fakeState, 0600)
 	c.Assert(err, IsNil)
 
-	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
+	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	state := o.State()
@@ -175,7 +175,7 @@ func (wm *witnessManager) Ensure() error {
 }
 
 func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) {
-	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
+	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	o.Loop()
@@ -185,7 +185,7 @@ func (ovs *overlordSuite) TestTrivialRunAndStop(c *C) {
 }
 
 func (ovs *overlordSuite) TestUnknownTasks(c *C) {
-	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
+	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	// unknown tasks are ignored and succeed
@@ -486,7 +486,7 @@ func (ovs *overlordSuite) TestCheckpoint(c *C) {
 	oldUmask := syscall.Umask(0)
 	defer syscall.Umask(oldUmask)
 
-	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
+	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	s := o.State()
@@ -727,7 +727,7 @@ func (ovs *overlordSuite) TestSettleExplicitEnsureBefore(c *C) {
 }
 
 func (ovs *overlordSuite) TestRequestRestartNoHandler(c *C) {
-	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir})
+	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir})
 	c.Assert(err, IsNil)
 
 	st := o.State()
@@ -760,7 +760,7 @@ func (rb *testRestartHandler) RebootIsMissing(_ *state.State) error {
 func (ovs *overlordSuite) TestRequestRestartHandler(c *C) {
 	rb := &testRestartHandler{}
 
-	o, err := overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
+	o, err := overlord.New(&overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, IsNil)
 
 	st := o.State()
@@ -779,7 +779,7 @@ func (ovs *overlordSuite) TestVerifyRebootNoPendingReboot(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
+	_, err = overlord.New(&overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -792,7 +792,7 @@ func (ovs *overlordSuite) TestVerifyRebootOK(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
+	_, err = overlord.New(&overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -806,7 +806,7 @@ func (ovs *overlordSuite) TestVerifyRebootOKButError(c *C) {
 	e := errors.New("boom")
 	rb := &testRestartHandler{rebootVerifiedErr: e}
 
-	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
+	_, err = overlord.New(&overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, Equals, e)
 
 	c.Check(rb.rebootState, Equals, "as-expected")
@@ -822,7 +822,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissing(c *C) {
 
 	rb := &testRestartHandler{}
 
-	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
+	_, err = overlord.New(&overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, IsNil)
 
 	c.Check(rb.rebootState, Equals, "did-not-happen")
@@ -839,7 +839,7 @@ func (ovs *overlordSuite) TestVerifyRebootIsMissingError(c *C) {
 	e := errors.New("boom")
 	rb := &testRestartHandler{rebootVerifiedErr: e}
 
-	_, err = overlord.New(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
+	_, err = overlord.New(&overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
 	c.Assert(err, Equals, e)
 
 	c.Check(rb.rebootState, Equals, "did-not-happen")

From 5c7c1057776b841df4c88c55458b6528f034db9b Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Fri, 25 Aug 2023 10:00:56 +0200
Subject: [PATCH 17/19] pr fix: doc

---
 internals/overlord/overlord.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index be4fb633..bc919036 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -51,7 +51,8 @@ var (
 
 // Extension represents an extension of the Overlord.
 type Extension interface {
-	// ExtraManagers is called when building an Overlord.
+	// ExtraManagers is called when building an Overlord, it allows additional
+	// managers to be added to the Overlord.
 	ExtraManagers(o *Overlord) ([]StateManager, error)
 }
 
@@ -63,8 +64,7 @@ type Options struct {
 	RestartHandler restart.Handler
 	// ServiceOutput is an optional output for the logging manager.
 	ServiceOutput io.Writer
-	// Extension is an optional struct defining additional components to add to
-	// the overlord.
+	// Extension, if set, defines additional components to add to the overlord.
 	Extension Extension
 }
 

From 29f8edceed3cfbf2f865d07631739780e60b7939 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Fri, 25 Aug 2023 12:33:47 +0200
Subject: [PATCH 18/19] pr fix: docs

---
 internals/overlord/overlord.go | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index c0308a93..87c624d8 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -51,8 +51,7 @@ var (
 
 // Extension represents an extension of the Overlord.
 type Extension interface {
-	// ExtraManagers is called when building an Overlord, it allows additional
-	// managers to be added to the Overlord.
+	// ExtraManagers  allows additional managers to be used.
 	ExtraManagers(o *Overlord) ([]StateManager, error)
 }
 
@@ -64,7 +63,7 @@ type Options struct {
 	RestartHandler restart.Handler
 	// ServiceOutput is an optional output for the logging manager.
 	ServiceOutput io.Writer
-	// Extension, if set, defines additional components to add to the overlord.
+	// Extension allows extending the overlord with externally defined features.
 	Extension Extension
 }
 

From 714323f53d2a52913bb72bdde4ed048aa4730f82 Mon Sep 17 00:00:00 2001
From: Paul Rodriguez <paul.rodriguez@canonical.com>
Date: Fri, 25 Aug 2023 15:03:56 +0200
Subject: [PATCH 19/19] pr fix: typo

---
 internals/overlord/overlord.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internals/overlord/overlord.go b/internals/overlord/overlord.go
index 87c624d8..ba3ca2ad 100644
--- a/internals/overlord/overlord.go
+++ b/internals/overlord/overlord.go
@@ -51,7 +51,7 @@ var (
 
 // Extension represents an extension of the Overlord.
 type Extension interface {
-	// ExtraManagers  allows additional managers to be used.
+	// ExtraManagers allows additional StateManagers to be used.
 	ExtraManagers(o *Overlord) ([]StateManager, error)
 }