Skip to content

Commit

Permalink
Move task env into execcontext
Browse files Browse the repository at this point in the history
Also inject PATH into rkt commands since we're no longer appending host
env vars for it.
  • Loading branch information
schmichael committed May 23, 2017
1 parent aaa3acd commit 6268c17
Show file tree
Hide file tree
Showing 14 changed files with 118 additions and 94 deletions.
2 changes: 1 addition & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ func (c *Client) setupDrivers() error {

var avail []string
var skipped []string
driverCtx := driver.NewDriverContext("", "", c.config, c.config.Node, c.logger, nil, nil)
driverCtx := driver.NewDriverContext("", "", c.config, c.config.Node, c.logger, nil)
for name := range driver.BuiltinDrivers {
// Skip fingerprinting drivers that are not in the whitelist if it is
// enabled.
Expand Down
8 changes: 4 additions & 4 deletions client/driver/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ func (d *DockerDriver) getDockerCoordinator(client *docker.Client) (*dockerCoord
}

func (d *DockerDriver) Prestart(ctx *ExecContext, task *structs.Task) (*PrestartResponse, error) {
driverConfig, err := NewDockerDriverConfig(task, d.taskEnv)
driverConfig, err := NewDockerDriverConfig(task, ctx.TaskEnv)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -491,7 +491,7 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle
return nil, err
}
executorCtx := &executor.ExecutorContext{
TaskEnv: d.taskEnv,
TaskEnv: ctx.TaskEnv,
Task: task,
Driver: "docker",
AllocID: d.DriverContext.allocID,
Expand Down Expand Up @@ -898,7 +898,7 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas
config.ExposedPorts = exposedPorts
}

parsedArgs := d.taskEnv.ParseAndReplace(driverConfig.Args)
parsedArgs := ctx.TaskEnv.ParseAndReplace(driverConfig.Args)

// If the user specified a custom command to run, we'll inject it here.
if driverConfig.Command != "" {
Expand All @@ -922,7 +922,7 @@ func (d *DockerDriver) createContainerConfig(ctx *ExecContext, task *structs.Tas
d.logger.Printf("[DEBUG] driver.docker: applied labels on the container: %+v", config.Labels)
}

config.Env = d.taskEnv.List()
config.Env = ctx.TaskEnv.List()

containerName := fmt.Sprintf("%s-%s", task.Name, d.DriverContext.allocID)
d.logger.Printf("[DEBUG] driver.docker: setting container name to: %s", containerName)
Expand Down
27 changes: 15 additions & 12 deletions client/driver/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package driver
import (
"fmt"
"io/ioutil"
"log"
"math/rand"
"os"
"path/filepath"
Expand Down Expand Up @@ -108,12 +109,16 @@ func dockerSetupWithClient(t *testing.T, task *structs.Task, client *docker.Clie
driver := NewDockerDriver(tctx.DriverCtx)
copyImage(t, tctx.ExecCtx.TaskDir, "busybox.tar")

res, err := driver.Prestart(tctx.ExecCtx, task)
resp, err := driver.Prestart(tctx.ExecCtx, task)
if err != nil {
tctx.AllocDir.Destroy()
t.Fatalf("error in prestart: %v", err)
}

// At runtime this is handled by TaskRunner
tctx.EnvBuilder.SetPortMap(resp.PortMap)
tctx.ExecCtx.TaskEnv = tctx.EnvBuilder.Build()

handle, err := driver.Start(tctx.ExecCtx, task)
if err != nil {
tctx.AllocDir.Destroy()
Expand All @@ -126,7 +131,7 @@ func dockerSetupWithClient(t *testing.T, task *structs.Task, client *docker.Clie
}

cleanup := func() {
driver.Cleanup(tctx.ExecCtx, res)
driver.Cleanup(tctx.ExecCtx, resp.CreatedResources)
handle.Kill()
tctx.AllocDir.Destroy()
}
Expand Down Expand Up @@ -894,10 +899,12 @@ func TestDockerDriver_PortsMapping(t *testing.T) {
"NOMAD_HOST_PORT_main": strconv.Itoa(docker_reserved),
}

log.Println(strings.Join(container.Config.Env, "\n"))

for key, val := range expectedEnvironment {
search := fmt.Sprintf("%s=%s", key, val)
if !inSlice(search, container.Config.Env) {
t.Errorf("Expected to find %s in container environment: %+v", search, container.Config.Env)
t.Errorf("Expected to find %s in container environment:\n%s\n\n", search, strings.Join(container.Config.Env, "\n"))
}
}
}
Expand Down Expand Up @@ -1087,25 +1094,20 @@ func setupDockerVolumes(t *testing.T, cfg *config.Config, hostpath string) (*str
}

alloc := mock.Alloc()
execCtx := NewExecContext(taskDir)
envBuilder := env.NewBuilder(cfg.Node, alloc, task, cfg.Region)
execCtx := NewExecContext(taskDir, envBuilder.Build())
cleanup := func() {
allocDir.Destroy()
if filepath.IsAbs(hostpath) {
os.RemoveAll(hostpath)
}
}

taskEnv, err := GetTaskEnv(taskDir, cfg.Node, task, alloc, cfg, "")
if err != nil {
cleanup()
t.Fatalf("Failed to get task env: %v", err)
}

logger := testLogger()
emitter := func(m string, args ...interface{}) {
logger.Printf("[EVENT] "+m, args...)
}
driverCtx := NewDriverContext(task.Name, alloc.ID, cfg, cfg.Node, testLogger(), taskEnv, emitter)
driverCtx := NewDriverContext(task.Name, alloc.ID, cfg, cfg.Node, testLogger(), emitter)
driver := NewDockerDriver(driverCtx)
copyImage(t, taskDir, "busybox.tar")

Expand Down Expand Up @@ -1236,10 +1238,11 @@ func TestDockerDriver_Cleanup(t *testing.T) {

// Run Prestart
driver := NewDockerDriver(tctx.DriverCtx).(*DockerDriver)
res, err := driver.Prestart(tctx.ExecCtx, task)
resp, err := driver.Prestart(tctx.ExecCtx, task)
if err != nil {
t.Fatalf("error in prestart: %v", err)
}
res := resp.CreatedResources
if len(res.Resources) == 0 || len(res.Resources[dockerImageResKey]) == 0 {
t.Fatalf("no created resources: %#v", res)
}
Expand Down
10 changes: 6 additions & 4 deletions client/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,6 @@ type DriverContext struct {
config *config.Config
logger *log.Logger
node *structs.Node
taskEnv *env.TaskEnv

emitEvent LogEventFn
}
Expand All @@ -259,14 +258,13 @@ func NewEmptyDriverContext() *DriverContext {
// private to the driver. If we want to change this later we can gorename all of
// the fields in DriverContext.
func NewDriverContext(taskName, allocID string, config *config.Config, node *structs.Node,
logger *log.Logger, taskEnv *env.TaskEnv, eventEmitter LogEventFn) *DriverContext {
logger *log.Logger, eventEmitter LogEventFn) *DriverContext {
return &DriverContext{
taskName: taskName,
allocID: allocID,
config: config,
node: node,
logger: logger,
taskEnv: taskEnv,
emitEvent: eventEmitter,
}
}
Expand Down Expand Up @@ -308,12 +306,16 @@ type ScriptExecutor interface {
type ExecContext struct {
// TaskDir contains information about the task directory structure.
TaskDir *allocdir.TaskDir

// TaskEnv contains the task's environment variables.
TaskEnv *env.TaskEnv
}

// NewExecContext is used to create a new execution context
func NewExecContext(td *allocdir.TaskDir) *ExecContext {
func NewExecContext(td *allocdir.TaskDir, te *env.TaskEnv) *ExecContext {
return &ExecContext{
TaskDir: td,
TaskEnv: te,
}
}

Expand Down
35 changes: 16 additions & 19 deletions client/driver/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ func testConfig() *config.Config {
}

type testContext struct {
AllocDir *allocdir.AllocDir
DriverCtx *DriverContext
ExecCtx *ExecContext
AllocDir *allocdir.AllocDir
DriverCtx *DriverContext
ExecCtx *ExecContext
EnvBuilder *env.Builder
}

// testDriverContext sets up an alloc dir, task dir, DriverContext, and ExecContext.
Expand Down Expand Up @@ -112,23 +113,17 @@ func testDriverContexts(t *testing.T, task *structs.Task) *testContext {
t.Fatalf("TaskDir.Build(%#v, %q) failed: %v", config.DefaultChrootEnv, tmpdrv.FSIsolation(), err)
return nil
}

execCtx := NewExecContext(td)

taskEnv, err := GetTaskEnv(td, cfg.Node, task, alloc, cfg, "")
if err != nil {
allocDir.Destroy()
t.Fatalf("GetTaskEnv() failed: %v", err)
return nil
}
eb := env.NewBuilder(cfg.Node, alloc, task, cfg.Region)
SetEnvvars(eb, tmpdrv.FSIsolation(), td, cfg)
execCtx := NewExecContext(td, eb.Build())

logger := testLogger()
emitter := func(m string, args ...interface{}) {
logger.Printf("[EVENT] "+m, args...)
}
driverCtx := NewDriverContext(task.Name, alloc.ID, cfg, cfg.Node, logger, taskEnv, emitter)
driverCtx := NewDriverContext(task.Name, alloc.ID, cfg, cfg.Node, logger, emitter)

return &testContext{allocDir, driverCtx, execCtx}
return &testContext{allocDir, driverCtx, execCtx, eb}
}

// setupTaskEnv creates a test env for GetTaskEnv testing. Returns task dir,
Expand Down Expand Up @@ -165,10 +160,12 @@ func setupTaskEnv(t *testing.T, driver string) (*allocdir.TaskDir, map[string]st
conf := testConfig()
allocDir := allocdir.NewAllocDir(testLogger(), filepath.Join(conf.AllocDir, alloc.ID))
taskDir := allocDir.NewTaskDir(task.Name)
env, err := GetTaskEnv(taskDir, conf.Node, task, alloc, conf, "")
eb := env.NewBuilder(conf.Node, alloc, task, conf.Region)
tmpDriver, err := NewDriver(driver, NewEmptyDriverContext())
if err != nil {
t.Fatalf("GetTaskEnv() failed: %v", err)
t.Fatalf("unable to create driver %q: %v", driver, err)
}
SetEnvvars(eb, tmpDriver.FSIsolation(), taskDir, conf)
exp := map[string]string{
"NOMAD_CPU_LIMIT": "1000",
"NOMAD_MEMORY_LIMIT": "500",
Expand Down Expand Up @@ -216,7 +213,7 @@ func setupTaskEnv(t *testing.T, driver string) (*allocdir.TaskDir, map[string]st
"NOMAD_REGION": "global",
}

act := env.EnvMap()
act := eb.Build().Map()
return taskDir, exp, act
}

Expand Down Expand Up @@ -275,9 +272,9 @@ func TestDriver_GetTaskEnv_Chroot(t *testing.T) {
}
}

// TestDriver_GetTaskEnv_Image ensures host environment variables are not set
// TestDriver_TaskEnv_Image ensures host environment variables are not set
// for image based drivers. See #2211
func TestDriver_GetTaskEnv_Image(t *testing.T) {
func TestDriver_TaskEnv_Image(t *testing.T) {
_, exp, act := setupTaskEnv(t, "docker")

exp[env.AllocDir] = allocdir.SharedAllocContainerPath
Expand Down
8 changes: 7 additions & 1 deletion client/driver/env/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,11 @@ func (b *Builder) Build() *TaskEnv {
envMap[VaultToken] = b.vaultToken
}

// Copy task meta
for k, v := range b.taskMeta {
envMap[k] = v
}

// Copy node attributes
for k, v := range b.nodeAttrs {
nodeAttrs[k] = v
Expand Down Expand Up @@ -394,9 +399,10 @@ func (b *Builder) setAlloc(alloc *structs.Allocation) *Builder {
// setNode is called from NewBuilder to populate node attributes.
func (b *Builder) setNode(n *structs.Node) *Builder {
b.nodeAttrs[nodeIdKey] = n.ID
b.nodeAttrs[nodeDcKey] = n.Datacenter
b.nodeAttrs[nodeNameKey] = n.Name
b.nodeAttrs[nodeClassKey] = n.NodeClass
b.nodeAttrs[nodeDcKey] = n.Datacenter
b.datacenter = n.Datacenter

// Set up the attributes.
for k, v := range n.Attributes {
Expand Down
2 changes: 1 addition & 1 deletion client/driver/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle,
return nil, err
}
executorCtx := &executor.ExecutorContext{
TaskEnv: d.taskEnv,
TaskEnv: ctx.TaskEnv,
Driver: "exec",
AllocID: d.DriverContext.allocID,
LogDir: ctx.TaskDir.LogDir,
Expand Down
4 changes: 2 additions & 2 deletions client/driver/java.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func NewJavaDriverConfig(task *structs.Task, env *env.TaskEnv) (*JavaDriverConfi
}

func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, error) {
driverConfig, err := NewJavaDriverConfig(task, d.taskEnv)
driverConfig, err := NewJavaDriverConfig(task, ctx.TaskEnv)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -249,7 +249,7 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle,

// Set the context
executorCtx := &executor.ExecutorContext{
TaskEnv: d.taskEnv,
TaskEnv: ctx.TaskEnv,
Driver: "java",
AllocID: d.DriverContext.allocID,
Task: task,
Expand Down
2 changes: 1 addition & 1 deletion client/driver/qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func (d *QemuDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle,
return nil, err
}
executorCtx := &executor.ExecutorContext{
TaskEnv: d.taskEnv,
TaskEnv: ctx.TaskEnv,
Driver: "qemu",
AllocID: d.DriverContext.allocID,
Task: task,
Expand Down
6 changes: 3 additions & 3 deletions client/driver/raw_exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandl
return nil, err
}
executorCtx := &executor.ExecutorContext{
TaskEnv: d.taskEnv,
TaskEnv: ctx.TaskEnv,
Driver: "raw_exec",
AllocID: d.DriverContext.allocID,
Task: task,
Expand Down Expand Up @@ -169,7 +169,7 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandl
logger: d.logger,
doneCh: make(chan struct{}),
waitCh: make(chan *dstructs.WaitResult, 1),
taskEnv: d.taskEnv,
taskEnv: ctx.TaskEnv,
taskDir: ctx.TaskDir,
}
go h.run()
Expand Down Expand Up @@ -218,7 +218,7 @@ func (d *RawExecDriver) Open(ctx *ExecContext, handleID string) (DriverHandle, e
version: id.Version,
doneCh: make(chan struct{}),
waitCh: make(chan *dstructs.WaitResult, 1),
taskEnv: d.taskEnv,
taskEnv: ctx.TaskEnv,
taskDir: ctx.TaskDir,
}
go h.run()
Expand Down
Loading

0 comments on commit 6268c17

Please sign in to comment.