Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(overlord): allow adding external managers #273

Merged
merged 20 commits into from
Aug 25, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .github/.golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
max-same-issues: 0

run:
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
timeout: 5m

2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
exit 1
11 changes: 10 additions & 1 deletion internals/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
}

// A Daemon listens for requests and routes them to the right command
Expand Down Expand Up @@ -217,6 +222,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()
Expand Down Expand Up @@ -783,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.
Expand Down
36 changes: 36 additions & 0 deletions internals/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved

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)
}

paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
func (s *daemonSuite) TestAddCommand(c *C) {
const endpoint = "/v1/addedendpoint"
var handler fakeHandler
Expand Down
2 changes: 1 addition & 1 deletion internals/overlord/managers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
61 changes: 49 additions & 12 deletions internals/overlord/overlord.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
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)
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved

// New creates a Overlord with all its state managers.
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
// 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) {
Expand Down Expand Up @@ -130,12 +151,28 @@ 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!
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)
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
}

func (o *Overlord) GetExternalManager(tag any) StateManager {
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
return o.externalManagers[tag]
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
}

func (o *Overlord) addManager(mgr StateManager) {
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
o.stateEng.AddManager(mgr)
}
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
28 changes: 14 additions & 14 deletions internals/overlord/overlord_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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()
Expand Down Expand Up @@ -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")
}

Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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")
Expand Down
Loading