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 14 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
17 changes: 16 additions & 1 deletion internals/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ 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

// OverlordExtension is an optional interface used to extend the capabilities
// of the Overlord.
OverlordExtension overlord.Extension
}

// A Daemon listens for requests and routes them to the right command
Expand Down Expand Up @@ -217,6 +221,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 +791,14 @@ func New(opts *Options) (*Daemon, error) {
httpAddress: opts.HTTPAddress,
}

ovld, err := overlord.New(opts.Dir, d, opts.ServiceOutput)
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.
Expand Down
70 changes: 70 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,75 @@ 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
}

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
}

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,
SocketPath: s.socketPath,
HTTPAddress: s.httpAddress,
OverlordExtension: &fakeExtension{},
})
c.Assert(err, IsNil)
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved

err = d.overlord.StateEngine().Ensure()
c.Assert(err, IsNil)
extension, ok := d.overlord.Extension().(*fakeExtension)
c.Assert(ok, Equals, true)
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)
}

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(overlord.Options{PebbleDir: s.dir})
c.Assert(err, IsNil)
s.o = o
}
Expand Down
68 changes: 56 additions & 12 deletions internals/overlord/overlord.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,25 @@ var (
defaultCachedDownloads = 5
)

// Extension represents an extension of the Overlord.
type Extension interface {
// ExtraManagers is called when building an Overlord.
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
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.
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved
Extension Extension
}

// Overlord is the central manager of the system, keeping track
// of all available state managers and related helpers.
type Overlord struct {
Expand All @@ -70,30 +89,33 @@ type Overlord struct {
commandMgr *cmdstate.CommandManager
checkMgr *checkstate.CheckManager
logMgr *logstate.LogManager

extension Extension
}

// New creates a new 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) {
// New creates an Overlord with all its state managers.
func New(opts Options) (*Overlord, error) {
paul-rodriguez marked this conversation as resolved.
Show resolved Hide resolved

o := &Overlord{
pebbleDir: pebbleDir,
pebbleDir: opts.PebbleDir,
loopTomb: new(tomb.Tomb),
inited: true,
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
}
Expand All @@ -110,7 +132,13 @@ func New(pebbleDir string, restartHandler restart.Handler, serviceOutput io.Writ
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
}
Expand All @@ -130,12 +158,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!
if o.extension != nil {
extraManagers, err := o.extension.ExtraManagers(o)
if err != nil {
return nil, err
}
for _, manager := range extraManagers {
o.stateEng.AddManager(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) Extension() Extension {
return o.extension
}

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(overlord.Options{PebbleDir: ovs.dir})
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(overlord.Options{PebbleDir: ovs.dir})
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(overlord.Options{PebbleDir: ovs.dir})
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(overlord.Options{PebbleDir: ovs.dir})
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(overlord.Options{PebbleDir: ovs.dir})
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(overlord.Options{PebbleDir: ovs.dir})
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(overlord.Options{PebbleDir: ovs.dir})
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(overlord.Options{PebbleDir: ovs.dir})
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(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
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(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
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(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
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(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
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(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
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(overlord.Options{PebbleDir: ovs.dir, RestartHandler: rb})
c.Assert(err, Equals, e)

c.Check(rb.rebootState, Equals, "did-not-happen")
Expand Down
Loading