From 8d4d9645d84df375fa9bdafaf3c8c86ebf6aec56 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 9 Sep 2015 18:06:23 -0700 Subject: [PATCH] Replace logging and config with DriverContext, which allows us to expand the dependency injection without changing the interface --- client/client.go | 3 ++- client/driver/docker.go | 11 +++-------- client/driver/driver.go | 27 ++++++++++++++++++++++++--- client/driver/exec.go | 12 +++--------- client/driver/java.go | 12 +++--------- client/task_runner.go | 3 ++- 6 files changed, 37 insertions(+), 31 deletions(-) diff --git a/client/client.go b/client/client.go index 1135fbb0d..5a3323f8c 100644 --- a/client/client.go +++ b/client/client.go @@ -342,8 +342,9 @@ func (c *Client) fingerprint() error { // setupDrivers is used to find the available drivers func (c *Client) setupDrivers() error { var avail []string + driverCtx := driver.NewDriverContext(c.config, c.config.Node, c.logger) for name := range driver.BuiltinDrivers { - d, err := driver.NewDriver(name, c.logger, c.config) + d, err := driver.NewDriver(name, driverCtx) if err != nil { return err } diff --git a/client/driver/docker.go b/client/driver/docker.go index fad9a87b2..e4f238d5e 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -20,8 +20,7 @@ var ( ) type DockerDriver struct { - logger *log.Logger - config *config.Config + DriverContext } type dockerPID struct { @@ -37,12 +36,8 @@ type dockerHandle struct { doneCh chan struct{} } -func NewDockerDriver(logger *log.Logger, config *config.Config) Driver { - d := &DockerDriver{ - logger: logger, - config: config, - } - return d +func NewDockerDriver(ctx *DriverContext) Driver { + return &DockerDriver{*ctx} } func (d *DockerDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { diff --git a/client/driver/driver.go b/client/driver/driver.go index 1016ac411..aaab33f47 100644 --- a/client/driver/driver.go +++ b/client/driver/driver.go @@ -20,7 +20,7 @@ var BuiltinDrivers = map[string]Factory{ // NewDriver is used to instantiate and return a new driver // given the name and a logger -func NewDriver(name string, logger *log.Logger, config *config.Config) (Driver, error) { +func NewDriver(name string, ctx *DriverContext) (Driver, error) { // Lookup the factory function factory, ok := BuiltinDrivers[name] if !ok { @@ -28,12 +28,12 @@ func NewDriver(name string, logger *log.Logger, config *config.Config) (Driver, } // Instantiate the driver - f := factory(logger, config) + f := factory(ctx) return f, nil } // Factory is used to instantiate a new Driver -type Factory func(*log.Logger, *config.Config) Driver +type Factory func(*DriverContext) Driver // Driver is used for execution of tasks. This allows Nomad // to support many pluggable implementations of task drivers. @@ -49,6 +49,27 @@ type Driver interface { Open(ctx *ExecContext, handleID string) (DriverHandle, error) } +// DriverContext is a means to inject dependencies such as loggers, configs, and +// node attributes into a Driver without having to change the Driver interface +// each time we do it. Used in conjection with Factory, above. +type DriverContext struct { + config *config.Config + logger *log.Logger + node *structs.Node +} + +// NewDriverContext initializes a new DriverContext with the specified fields. +// This enables other packages to create DriverContexts but keeps the fields +// private to the driver. If we want to change this later we can gorename all of +// the fields in DriverContext. +func NewDriverContext(config *config.Config, node *structs.Node, logger *log.Logger) *DriverContext { + return &DriverContext{ + config: config, + node: node, + logger: logger, + } +} + // DriverHandle is an opaque handle into a driver used for task // manipulation type DriverHandle interface { diff --git a/client/driver/exec.go b/client/driver/exec.go index e13ad47c2..ddb57c135 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -2,7 +2,6 @@ package driver import ( "fmt" - "log" "os" "os/exec" "strconv" @@ -17,8 +16,7 @@ import ( // fork/execs tasks. It should probably not be used for most things, // but is useful for testing purposes or for very simple tasks. type ExecDriver struct { - logger *log.Logger - config *config.Config + DriverContext } // execHandle is returned from Start/Open as a handle to the PID @@ -29,12 +27,8 @@ type execHandle struct { } // NewExecDriver is used to create a new exec driver -func NewExecDriver(logger *log.Logger, config *config.Config) Driver { - d := &ExecDriver{ - logger: logger, - config: config, - } - return d +func NewExecDriver(ctx *DriverContext) Driver { + return &ExecDriver{*ctx} } func (d *ExecDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { diff --git a/client/driver/java.go b/client/driver/java.go index 01afc0061..9ff88c6d7 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "io" - "log" "net/http" "os" "os/exec" @@ -21,8 +20,7 @@ import ( // JavaDriver is a simple driver to execute applications packaged in Jars. // It literally just fork/execs tasks with the java command. type JavaDriver struct { - logger *log.Logger - config *config.Config + DriverContext } // javaHandle is returned from Start/Open as a handle to the PID @@ -33,12 +31,8 @@ type javaHandle struct { } // NewJavaDriver is used to create a new exec driver -func NewJavaDriver(logger *log.Logger, config *config.Config) Driver { - d := &JavaDriver{ - logger: logger, - config: config, - } - return d +func NewJavaDriver(ctx *DriverContext) Driver { + return &JavaDriver{*ctx} } func (d *JavaDriver) Fingerprint(cfg *config.Config, node *structs.Node) (bool, error) { diff --git a/client/task_runner.go b/client/task_runner.go index 954604008..6bdb6e5e1 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -129,7 +129,8 @@ func (r *TaskRunner) setStatus(status, desc string) { // createDriver makes a driver for the task func (r *TaskRunner) createDriver() (driver.Driver, error) { - driver, err := driver.NewDriver(r.task.Driver, r.logger, r.config) + driverCtx := driver.NewDriverContext(r.config, r.config.Node, r.logger) + driver, err := driver.NewDriver(r.task.Driver, driverCtx) if err != nil { err = fmt.Errorf("failed to create driver '%s' for alloc %s: %v", r.task.Driver, r.allocID, err)