From 6f17902027c0c2c187acbe475f7dc15d3f2bc5fb Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Tue, 17 Nov 2015 17:29:27 -0800 Subject: [PATCH 01/53] Added some missing dependencies for linux x-compile --- scripts/deps.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/deps.sh b/scripts/deps.sh index d2352b782..2c2899ff2 100755 --- a/scripts/deps.sh +++ b/scripts/deps.sh @@ -7,6 +7,8 @@ GOOS=linux go get $DEP_ARGS github.com/docker/docker/pkg/units GOOS=linux go get $DEP_ARGS github.com/docker/docker/pkg/mount GOOS=linux go get $DEP_ARGS github.com/opencontainers/runc/libcontainer/cgroups/fs GOOS=linux go get $DEP_ARGS github.com/opencontainers/runc/libcontainer/configs +GOOS=linux go get $DEP_ARGS github.com/coreos/go-systemd/util +GOOS=linux go get $DEP_ARGS github.com/coreos/go-systemd/dbus # Get the rest of the deps DEPS=$(go list -f '{{range .TestImports}}{{.}} {{end}}' ./...) From 9a578c5c0dba6957c2f5b639147d491b9ccd7134 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 00:50:45 -0800 Subject: [PATCH 02/53] Added the implementation of consul client --- client/alloc_runner.go | 37 +++++++++++---------- client/alloc_runner_test.go | 6 ++-- client/client.go | 25 ++++++++++----- client/consul.go | 64 +++++++++++++++++++++++++++++++++++++ client/task_runner.go | 23 ++++++++++++- client/task_runner_test.go | 8 +++-- jobspec/parse.go | 4 +-- nomad/structs/structs.go | 2 +- 8 files changed, 136 insertions(+), 33 deletions(-) create mode 100644 client/consul.go diff --git a/client/alloc_runner.go b/client/alloc_runner.go index d723478db..0bee8103a 100644 --- a/client/alloc_runner.go +++ b/client/alloc_runner.go @@ -33,9 +33,10 @@ type AllocStateUpdater func(alloc *structs.Allocation) error // AllocRunner is used to wrap an allocation and provide the execution context. type AllocRunner struct { - config *config.Config - updater AllocStateUpdater - logger *log.Logger + config *config.Config + updater AllocStateUpdater + logger *log.Logger + consulClient *ConsulClient alloc *structs.Allocation @@ -66,18 +67,20 @@ type allocRunnerState struct { } // NewAllocRunner is used to create a new allocation context -func NewAllocRunner(logger *log.Logger, config *config.Config, updater AllocStateUpdater, alloc *structs.Allocation) *AllocRunner { +func NewAllocRunner(logger *log.Logger, config *config.Config, updater AllocStateUpdater, + alloc *structs.Allocation, consulClient *ConsulClient) *AllocRunner { ar := &AllocRunner{ - config: config, - updater: updater, - logger: logger, - alloc: alloc, - dirtyCh: make(chan struct{}, 1), - tasks: make(map[string]*TaskRunner), - restored: make(map[string]struct{}), - updateCh: make(chan *structs.Allocation, 8), - destroyCh: make(chan struct{}), - waitCh: make(chan struct{}), + config: config, + updater: updater, + logger: logger, + alloc: alloc, + consulClient: consulClient, + dirtyCh: make(chan struct{}, 1), + tasks: make(map[string]*TaskRunner), + restored: make(map[string]struct{}), + updateCh: make(chan *structs.Allocation, 8), + destroyCh: make(chan struct{}), + waitCh: make(chan struct{}), } return ar } @@ -109,7 +112,8 @@ func (r *AllocRunner) RestoreState() error { task := &structs.Task{Name: name} restartTracker := newRestartTracker(r.alloc.Job.Type, r.RestartPolicy) tr := NewTaskRunner(r.logger, r.config, r.setTaskState, r.ctx, - r.alloc.ID, task, r.alloc.TaskStates[task.Name], restartTracker) + r.alloc.ID, task, r.alloc.TaskStates[task.Name], restartTracker, + r.consulClient) r.tasks[name] = tr // Skip tasks in terminal states. @@ -320,7 +324,8 @@ func (r *AllocRunner) Run() { task.Resources = alloc.TaskResources[task.Name] restartTracker := newRestartTracker(r.alloc.Job.Type, r.RestartPolicy) tr := NewTaskRunner(r.logger, r.config, r.setTaskState, r.ctx, - r.alloc.ID, task, r.alloc.TaskStates[task.Name], restartTracker) + r.alloc.ID, task, r.alloc.TaskStates[task.Name], restartTracker, + r.consulClient) r.tasks[task.Name] = tr go tr.Run() } diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index 9abe6b8a2..3b9828493 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -31,12 +31,13 @@ func testAllocRunner(restarts bool) (*MockAllocStateUpdater, *AllocRunner) { conf.AllocDir = os.TempDir() upd := &MockAllocStateUpdater{} alloc := mock.Alloc() + consulClient, _ := NewConsulClient() if !restarts { alloc.Job.Type = structs.JobTypeBatch *alloc.Job.LookupTaskGroup(alloc.TaskGroup).RestartPolicy = structs.RestartPolicy{Attempts: 0} } - ar := NewAllocRunner(logger, conf, upd.Update, alloc) + ar := NewAllocRunner(logger, conf, upd.Update, alloc, consulClient) return upd, ar } @@ -141,8 +142,9 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { } // Create a new alloc runner + consulClient, err := NewConsulClient() ar2 := NewAllocRunner(ar.logger, ar.config, upd.Update, - &structs.Allocation{ID: ar.alloc.ID}) + &structs.Allocation{ID: ar.alloc.ID}, consulClient) err = ar2.RestoreState() if err != nil { t.Fatalf("err: %v", err) diff --git a/client/client.go b/client/client.go index 1255e7d10..527d43041 100644 --- a/client/client.go +++ b/client/client.go @@ -70,6 +70,8 @@ type Client struct { logger *log.Logger + consulClient *ConsulClient + lastServer net.Addr lastRPCTime time.Time lastServerLock sync.Mutex @@ -96,14 +98,21 @@ func NewClient(cfg *config.Config) (*Client, error) { // Create a logger logger := log.New(cfg.LogOutput, "", log.LstdFlags) + // Create the consul client + consulClient, err := NewConsulClient() + if err != nil { + return nil, fmt.Errorf("failed to create the consul client: %v", err) + } + // Create the client c := &Client{ - config: cfg, - start: time.Now(), - connPool: nomad.NewPool(cfg.LogOutput, clientRPCCache, clientMaxStreams, nil), - logger: logger, - allocs: make(map[string]*AllocRunner), - shutdownCh: make(chan struct{}), + config: cfg, + start: time.Now(), + consulClient: consulClient, + connPool: nomad.NewPool(cfg.LogOutput, clientRPCCache, clientMaxStreams, nil), + logger: logger, + allocs: make(map[string]*AllocRunner), + shutdownCh: make(chan struct{}), } // Initialize the client @@ -335,7 +344,7 @@ func (c *Client) restoreState() error { for _, entry := range list { id := entry.Name() alloc := &structs.Allocation{ID: id} - ar := NewAllocRunner(c.logger, c.config, c.updateAllocStatus, alloc) + ar := NewAllocRunner(c.logger, c.config, c.updateAllocStatus, alloc, c.consulClient) c.allocs[id] = ar if err := ar.RestoreState(); err != nil { c.logger.Printf("[ERR] client: failed to restore state for alloc %s: %v", id, err) @@ -749,7 +758,7 @@ func (c *Client) updateAlloc(exist, update *structs.Allocation) error { func (c *Client) addAlloc(alloc *structs.Allocation) error { c.allocLock.Lock() defer c.allocLock.Unlock() - ar := NewAllocRunner(c.logger, c.config, c.updateAllocStatus, alloc) + ar := NewAllocRunner(c.logger, c.config, c.updateAllocStatus, alloc, c.consulClient) c.allocs[alloc.ID] = ar go ar.Run() return nil diff --git a/client/consul.go b/client/consul.go new file mode 100644 index 000000000..188816859 --- /dev/null +++ b/client/consul.go @@ -0,0 +1,64 @@ +package client + +import ( + "fmt" + "github.com/hashicorp/consul/api" + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/nomad/nomad/structs" +) + +const ( + consulPort = 8080 +) + +type ConsulClient struct { + client *api.Client +} + +func NewConsulClient() (*ConsulClient, error) { + var err error + var c *api.Client + if c, err = api.NewClient(api.DefaultConfig()); err != nil { + return nil, err + } + + consulClient := ConsulClient{ + client: c, + } + + return &consulClient, nil +} + +func (c *ConsulClient) Register(task *structs.Task, allocID string, port int, host string) error { + var mErr multierror.Error + serviceDefns := make([]*api.AgentServiceRegistration, len(task.Services)) + for idx, service := range task.Services { + service.Id = fmt.Sprintf("%s-%s", allocID, task.Name) + asr := &api.AgentServiceRegistration{ + ID: service.Id, + Name: service.Name, + Tags: service.Tags, + Port: port, + Address: host, + } + serviceDefns[idx] = asr + } + + for _, serviceDefn := range serviceDefns { + if err := c.client.Agent().ServiceRegister(serviceDefn); err != nil { + mErr.Errors = append(mErr.Errors, err) + } + } + + return mErr.ErrorOrNil() +} + +func (c *ConsulClient) DeRegister(task *structs.Task) error { + var mErr multierror.Error + for _, service := range task.Services { + if err := c.client.Agent().ServiceDeregister(service.Id); err != nil { + mErr.Errors = append(mErr.Errors, err) + } + } + return mErr.ErrorOrNil() +} diff --git a/client/task_runner.go b/client/task_runner.go index 6a4e6ed46..456df9e09 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -25,6 +25,7 @@ type TaskRunner struct { ctx *driver.ExecContext allocID string restartTracker restartTracker + consulClient *ConsulClient task *structs.Task state *structs.TaskState @@ -52,13 +53,14 @@ type TaskStateUpdater func(taskName string) func NewTaskRunner(logger *log.Logger, config *config.Config, updater TaskStateUpdater, ctx *driver.ExecContext, allocID string, task *structs.Task, state *structs.TaskState, - restartTracker restartTracker) *TaskRunner { + restartTracker restartTracker, consulClient *ConsulClient) *TaskRunner { tc := &TaskRunner{ config: config, updater: updater, logger: logger, restartTracker: restartTracker, + consulClient: consulClient, ctx: ctx, allocID: allocID, task: task, @@ -231,6 +233,22 @@ func (r *TaskRunner) run() { var destroyErr error destroyed := false + // Register the services defined by the task with Consil + for _, service := range r.task.Services { + portLabel := service.PortLabel + var port int + var host string + for _, network := range r.task.Resources.Networks { + if p, ok := network.MapLabelToValues()[portLabel]; ok { + port = p + host = network.IP + break + } + } + + r.consulClient.Register(r.task, r.allocID, port, host) + } + OUTER: // Wait for updates for { @@ -303,6 +321,9 @@ func (r *TaskRunner) run() { // Set force start because we are restarting the task. forceStart = true } + + // De-Register the services belonging to the task from consul + r.consulClient.DeRegister(r.task) return } diff --git a/client/task_runner_test.go b/client/task_runner_test.go index f38cf045a..2c3eb8928 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -32,7 +32,7 @@ func testTaskRunner(restarts bool) (*MockTaskStateUpdater, *TaskRunner) { upd := &MockTaskStateUpdater{} alloc := mock.Alloc() task := alloc.Job.TaskGroups[0].Tasks[0] - + consulClient, _ := NewConsulClient() // Initialize the port listing. This should be done by the offer process but // we have a mock so that doesn't happen. task.Resources.Networks[0].ReservedPorts = []structs.Port{{"", 80}} @@ -48,7 +48,7 @@ func testTaskRunner(restarts bool) (*MockTaskStateUpdater, *TaskRunner) { } state := alloc.TaskStates[task.Name] - tr := NewTaskRunner(logger, conf, upd.Update, ctx, alloc.ID, task, state, restartTracker) + tr := NewTaskRunner(logger, conf, upd.Update, ctx, alloc.ID, task, state, restartTracker, consulClient) return upd, tr } @@ -164,8 +164,10 @@ func TestTaskRunner_SaveRestoreState(t *testing.T) { } // Create a new task runner + consulClient, _ := NewConsulClient() tr2 := NewTaskRunner(tr.logger, tr.config, upd.Update, - tr.ctx, tr.allocID, &structs.Task{Name: tr.task.Name}, tr.state, tr.restartTracker) + tr.ctx, tr.allocID, &structs.Task{Name: tr.task.Name}, tr.state, tr.restartTracker, + consulClient) if err := tr2.RestoreState(); err != nil { t.Fatalf("err: %v", err) } diff --git a/jobspec/parse.go b/jobspec/parse.go index 92f3c5048..58dbd6c9c 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -463,7 +463,7 @@ func parseTasks(jobName string, taskGroupName string, result *[]*structs.Task, l } func parseServices(jobName string, taskGroupName string, task *structs.Task, serviceObjs *ast.ObjectList) error { - task.Services = make([]structs.Service, len(serviceObjs.Items)) + task.Services = make([]*structs.Service, len(serviceObjs.Items)) var defaultServiceName bool for idx, o := range serviceObjs.Items { var service structs.Service @@ -503,7 +503,7 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser } } - task.Services[idx] = service + task.Services[idx] = &service } return nil diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 4a0f29f88..c4dbc5c45 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1064,7 +1064,7 @@ type Task struct { Env map[string]string // List of service definitions exposed by the Task - Services []Service + Services []*Service // Constraints can be specified at a task level and apply only to // the particular task. From 392a80730525938a66e99e4611e61d3bb4ae90b1 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 01:18:29 -0800 Subject: [PATCH 03/53] Moving the logic to find port and host inside consul client --- client/consul.go | 22 ++++++++++++++++++++-- client/task_runner.go | 15 +-------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/client/consul.go b/client/consul.go index 188816859..27ea17735 100644 --- a/client/consul.go +++ b/client/consul.go @@ -29,11 +29,15 @@ func NewConsulClient() (*ConsulClient, error) { return &consulClient, nil } -func (c *ConsulClient) Register(task *structs.Task, allocID string, port int, host string) error { +func (c *ConsulClient) Register(task *structs.Task, allocID string) error { var mErr multierror.Error - serviceDefns := make([]*api.AgentServiceRegistration, len(task.Services)) + var serviceDefns []*api.AgentServiceRegistration for idx, service := range task.Services { service.Id = fmt.Sprintf("%s-%s", allocID, task.Name) + host, port := c.findPortAndHostForLabel(service.PortLabel, task) + if host == "" || port == 0 { + continue + } asr := &api.AgentServiceRegistration{ ID: service.Id, Name: service.Name, @@ -62,3 +66,17 @@ func (c *ConsulClient) DeRegister(task *structs.Task) error { } return mErr.ErrorOrNil() } + +func (c *ConsulClient) findPortAndHostForLabel(portLabel string, task *structs.Task) (string, int) { + var host string + var port int + for _, network := range task.Resources.Networks { + if p, ok := network.MapLabelToValues()[portLabel]; ok { + host = network.IP + port = p + break + } + } + + return host, port +} diff --git a/client/task_runner.go b/client/task_runner.go index 456df9e09..bcf1f1e58 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -234,20 +234,7 @@ func (r *TaskRunner) run() { destroyed := false // Register the services defined by the task with Consil - for _, service := range r.task.Services { - portLabel := service.PortLabel - var port int - var host string - for _, network := range r.task.Resources.Networks { - if p, ok := network.MapLabelToValues()[portLabel]; ok { - port = p - host = network.IP - break - } - } - - r.consulClient.Register(r.task, r.allocID, port, host) - } + r.consulClient.Register(r.task, r.allocID) OUTER: // Wait for updates From df074a54e37f65db49b9489b3b0a2a58f74fbf7e Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 01:20:53 -0800 Subject: [PATCH 04/53] DRYed the code --- client/consul.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/client/consul.go b/client/consul.go index 27ea17735..a7f411941 100644 --- a/client/consul.go +++ b/client/consul.go @@ -68,15 +68,10 @@ func (c *ConsulClient) DeRegister(task *structs.Task) error { } func (c *ConsulClient) findPortAndHostForLabel(portLabel string, task *structs.Task) (string, int) { - var host string - var port int for _, network := range task.Resources.Networks { if p, ok := network.MapLabelToValues()[portLabel]; ok { - host = network.IP - port = p - break + return network.IP, p } } - - return host, port + return "", 0 } From 028ad505cfb4d8a96ca40b35d9ef0f34c808bd56 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 02:07:07 -0800 Subject: [PATCH 05/53] Fixed the test errors --- jobspec/parse_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 6eb19af11..d452d1a70 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -94,7 +94,7 @@ func TestParse(t *testing.T) { Config: map[string]interface{}{ "image": "hashicorp/binstore", }, - Services: []structs.Service{ + Services: []*structs.Service{ { Id: "", Name: "binstore-storagelocker-binsl-binstore", From 4221cda6f771ac94eca9a228168aea0d1d6b1342 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 02:14:07 -0800 Subject: [PATCH 06/53] Added a logger to consul client --- client/alloc_runner_test.go | 4 ++-- client/client.go | 2 +- client/consul.go | 6 +++++- client/task_runner_test.go | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index 3b9828493..148a33a42 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -31,7 +31,7 @@ func testAllocRunner(restarts bool) (*MockAllocStateUpdater, *AllocRunner) { conf.AllocDir = os.TempDir() upd := &MockAllocStateUpdater{} alloc := mock.Alloc() - consulClient, _ := NewConsulClient() + consulClient, _ := NewConsulClient(logger) if !restarts { alloc.Job.Type = structs.JobTypeBatch *alloc.Job.LookupTaskGroup(alloc.TaskGroup).RestartPolicy = structs.RestartPolicy{Attempts: 0} @@ -142,7 +142,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { } // Create a new alloc runner - consulClient, err := NewConsulClient() + consulClient, err := NewConsulClient(ar.logger) ar2 := NewAllocRunner(ar.logger, ar.config, upd.Update, &structs.Allocation{ID: ar.alloc.ID}, consulClient) err = ar2.RestoreState() diff --git a/client/client.go b/client/client.go index 527d43041..e33872782 100644 --- a/client/client.go +++ b/client/client.go @@ -99,7 +99,7 @@ func NewClient(cfg *config.Config) (*Client, error) { logger := log.New(cfg.LogOutput, "", log.LstdFlags) // Create the consul client - consulClient, err := NewConsulClient() + consulClient, err := NewConsulClient(logger) if err != nil { return nil, fmt.Errorf("failed to create the consul client: %v", err) } diff --git a/client/consul.go b/client/consul.go index a7f411941..85fad12bc 100644 --- a/client/consul.go +++ b/client/consul.go @@ -5,6 +5,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/nomad/structs" + "log" ) const ( @@ -13,9 +14,11 @@ const ( type ConsulClient struct { client *api.Client + + logger *log.Logger } -func NewConsulClient() (*ConsulClient, error) { +func NewConsulClient(logger *log.Logger) (*ConsulClient, error) { var err error var c *api.Client if c, err = api.NewClient(api.DefaultConfig()); err != nil { @@ -24,6 +27,7 @@ func NewConsulClient() (*ConsulClient, error) { consulClient := ConsulClient{ client: c, + logger: logger, } return &consulClient, nil diff --git a/client/task_runner_test.go b/client/task_runner_test.go index 2c3eb8928..788b0b1d5 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -32,7 +32,7 @@ func testTaskRunner(restarts bool) (*MockTaskStateUpdater, *TaskRunner) { upd := &MockTaskStateUpdater{} alloc := mock.Alloc() task := alloc.Job.TaskGroups[0].Tasks[0] - consulClient, _ := NewConsulClient() + consulClient, _ := NewConsulClient(logger) // Initialize the port listing. This should be done by the offer process but // we have a mock so that doesn't happen. task.Resources.Networks[0].ReservedPorts = []structs.Port{{"", 80}} @@ -164,7 +164,7 @@ func TestTaskRunner_SaveRestoreState(t *testing.T) { } // Create a new task runner - consulClient, _ := NewConsulClient() + consulClient, _ := NewConsulClient(tr.logger) tr2 := NewTaskRunner(tr.logger, tr.config, upd.Update, tr.ctx, tr.allocID, &structs.Task{Name: tr.task.Name}, tr.state, tr.restartTracker, consulClient) From 929882426c4bebd83dd5cd2a5a43c663effb6e0a Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 02:37:34 -0800 Subject: [PATCH 07/53] Defering calling the de-register from consul call when a service is not running --- client/consul.go | 10 +++++++--- client/task_runner.go | 5 +++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/client/consul.go b/client/consul.go index 85fad12bc..b0ad31893 100644 --- a/client/consul.go +++ b/client/consul.go @@ -36,7 +36,7 @@ func NewConsulClient(logger *log.Logger) (*ConsulClient, error) { func (c *ConsulClient) Register(task *structs.Task, allocID string) error { var mErr multierror.Error var serviceDefns []*api.AgentServiceRegistration - for idx, service := range task.Services { + for _, service := range task.Services { service.Id = fmt.Sprintf("%s-%s", allocID, task.Name) host, port := c.findPortAndHostForLabel(service.PortLabel, task) if host == "" || port == 0 { @@ -49,11 +49,13 @@ func (c *ConsulClient) Register(task *structs.Task, allocID string) error { Port: port, Address: host, } - serviceDefns[idx] = asr + serviceDefns = append(serviceDefns, asr) } for _, serviceDefn := range serviceDefns { + c.logger.Printf("[INFO] Registering service %v with Consul", serviceDefn.Name) if err := c.client.Agent().ServiceRegister(serviceDefn); err != nil { + c.logger.Printf("[ERROR] Error while registering service %v with Consul: %v", serviceDefn.Name, err) mErr.Errors = append(mErr.Errors, err) } } @@ -61,10 +63,12 @@ func (c *ConsulClient) Register(task *structs.Task, allocID string) error { return mErr.ErrorOrNil() } -func (c *ConsulClient) DeRegister(task *structs.Task) error { +func (c *ConsulClient) Deregister(task *structs.Task) error { var mErr multierror.Error for _, service := range task.Services { + c.logger.Printf("[INFO] De-Registering service %v with Consul", service.Name) if err := c.client.Agent().ServiceDeregister(service.Id); err != nil { + c.logger.Printf("[ERROR] Error in de-registering service %v from Consul", service.Name) mErr.Errors = append(mErr.Errors, err) } } diff --git a/client/task_runner.go b/client/task_runner.go index bcf1f1e58..cc4bddab8 100644 --- a/client/task_runner.go +++ b/client/task_runner.go @@ -236,6 +236,9 @@ func (r *TaskRunner) run() { // Register the services defined by the task with Consil r.consulClient.Register(r.task, r.allocID) + // De-Register the services belonging to the task from consul + defer r.consulClient.Deregister(r.task) + OUTER: // Wait for updates for { @@ -309,8 +312,6 @@ func (r *TaskRunner) run() { forceStart = true } - // De-Register the services belonging to the task from consul - r.consulClient.DeRegister(r.task) return } From 992b6709e29c0293b75eae24f5d2897b9f40d36f Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 03:08:53 -0800 Subject: [PATCH 08/53] Registering the checks with consul --- client/consul.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/client/consul.go b/client/consul.go index b0ad31893..26a9c0d7b 100644 --- a/client/consul.go +++ b/client/consul.go @@ -42,12 +42,14 @@ func (c *ConsulClient) Register(task *structs.Task, allocID string) error { if host == "" || port == 0 { continue } + checks := c.makeChecks(service, host, port) asr := &api.AgentServiceRegistration{ ID: service.Id, Name: service.Name, Tags: service.Tags, Port: port, Address: host, + Checks: checks, } serviceDefns = append(serviceDefns, asr) } @@ -83,3 +85,23 @@ func (c *ConsulClient) findPortAndHostForLabel(portLabel string, task *structs.T } return "", 0 } + +func (c *ConsulClient) makeChecks(service *structs.Service, ip string, port int) []*api.AgentServiceCheck { + var checks []*api.AgentServiceCheck + for _, check := range service.Checks { + c := &api.AgentServiceCheck{ + Interval: check.Interval.String(), + Timeout: check.Timeout.String(), + } + switch check.Type { + case structs.ServiceCheckHTTP: + c.HTTP = fmt.Sprintf("%s://%s:%d/%s", check.Protocol, ip, port, check.Http) + case structs.ServiceCheckTCP: + c.TCP = fmt.Sprintf("%s:%d", ip, port) + case structs.ServiceCheckScript: + c.Script = check.Script // TODO This needs to include the path of the alloc dir and based on driver types + } + checks = append(checks, c) + } + return checks +} From 7efac5dae8f3ecfbdb9dc8e0ea8567568c2b76ec Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 04:34:23 -0800 Subject: [PATCH 09/53] Added the logic to retry services which needs to be tracked if consul doesn't respond --- client/consul.go | 130 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 95 insertions(+), 35 deletions(-) diff --git a/client/consul.go b/client/consul.go index 26a9c0d7b..5814450f9 100644 --- a/client/consul.go +++ b/client/consul.go @@ -2,32 +2,43 @@ package client import ( "fmt" - "github.com/hashicorp/consul/api" + consul "github.com/hashicorp/consul/api" "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/nomad/structs" "log" + "time" ) const ( - consulPort = 8080 + syncInterval = 5 * time.Second ) -type ConsulClient struct { - client *api.Client +type trackedService struct { + allocId string + task *structs.Task + service *structs.Service +} - logger *log.Logger +type ConsulClient struct { + client *consul.Client + logger *log.Logger + shutdownCh chan struct{} + + trackedServices map[string]*trackedService } func NewConsulClient(logger *log.Logger) (*ConsulClient, error) { var err error - var c *api.Client - if c, err = api.NewClient(api.DefaultConfig()); err != nil { + var c *consul.Client + ts := make(map[string]*trackedService) + if c, err = consul.NewClient(consul.DefaultConfig()); err != nil { return nil, err } consulClient := ConsulClient{ - client: c, - logger: logger, + client: c, + logger: logger, + trackedServices: ts, } return &consulClient, nil @@ -35,31 +46,16 @@ func NewConsulClient(logger *log.Logger) (*ConsulClient, error) { func (c *ConsulClient) Register(task *structs.Task, allocID string) error { var mErr multierror.Error - var serviceDefns []*api.AgentServiceRegistration for _, service := range task.Services { - service.Id = fmt.Sprintf("%s-%s", allocID, task.Name) - host, port := c.findPortAndHostForLabel(service.PortLabel, task) - if host == "" || port == 0 { - continue - } - checks := c.makeChecks(service, host, port) - asr := &api.AgentServiceRegistration{ - ID: service.Id, - Name: service.Name, - Tags: service.Tags, - Port: port, - Address: host, - Checks: checks, - } - serviceDefns = append(serviceDefns, asr) - } - - for _, serviceDefn := range serviceDefns { - c.logger.Printf("[INFO] Registering service %v with Consul", serviceDefn.Name) - if err := c.client.Agent().ServiceRegister(serviceDefn); err != nil { - c.logger.Printf("[ERROR] Error while registering service %v with Consul: %v", serviceDefn.Name, err) + if err := c.registerService(service, task, allocID); err != nil { mErr.Errors = append(mErr.Errors, err) } + ts := &trackedService{ + allocId: allocID, + task: task, + } + c.trackedServices[service.Id] = ts + } return mErr.ErrorOrNil() @@ -69,10 +65,11 @@ func (c *ConsulClient) Deregister(task *structs.Task) error { var mErr multierror.Error for _, service := range task.Services { c.logger.Printf("[INFO] De-Registering service %v with Consul", service.Name) - if err := c.client.Agent().ServiceDeregister(service.Id); err != nil { + if err := c.deregisterService(service.Id); err != nil { c.logger.Printf("[ERROR] Error in de-registering service %v from Consul", service.Name) mErr.Errors = append(mErr.Errors, err) } + delete(c.trackedServices, service.Id) } return mErr.ErrorOrNil() } @@ -86,10 +83,73 @@ func (c *ConsulClient) findPortAndHostForLabel(portLabel string, task *structs.T return "", 0 } -func (c *ConsulClient) makeChecks(service *structs.Service, ip string, port int) []*api.AgentServiceCheck { - var checks []*api.AgentServiceCheck +func (c *ConsulClient) SyncWithConsul() { + sync := time.After(syncInterval) + agent := c.client.Agent() + + for { + select { + case <-sync: + var consulServices map[string]*consul.AgentService + var err error + if consulServices, err = agent.Services(); err != nil { + c.logger.Printf("[DEBUG] Error while syncing services with Consul: %v", err) + continue + } + for serviceId := range c.trackedServices { + if _, ok := consulServices[serviceId]; !ok { + ts := c.trackedServices[serviceId] + c.registerService(ts.service, ts.task, ts.allocId) + } + } + + for serviceId := range consulServices { + if _, ok := c.trackedServices[serviceId]; !ok { + if err := c.deregisterService(serviceId); err != nil { + c.logger.Printf("[DEBUG] Error while de-registering service with ID: %s", serviceId) + } + } + } + case <-c.shutdownCh: + return + } + } +} + +func (c *ConsulClient) registerService(service *structs.Service, task *structs.Task, allocID string) error { + var mErr multierror.Error + service.Id = fmt.Sprintf("%s-%s", allocID, task.Name) + host, port := c.findPortAndHostForLabel(service.PortLabel, task) + if host == "" || port == 0 { + return fmt.Errorf("The port:%s marked for registration of service: %s couldn't be found", service.PortLabel, service.Name) + } + checks := c.makeChecks(service, host, port) + asr := &consul.AgentServiceRegistration{ + ID: service.Id, + Name: service.Name, + Tags: service.Tags, + Port: port, + Address: host, + Checks: checks, + } + if err := c.client.Agent().ServiceRegister(asr); err != nil { + c.logger.Printf("[ERROR] Error while registering service %v with Consul: %v", service.Name, err) + mErr.Errors = append(mErr.Errors, err) + } + return mErr.ErrorOrNil() +} + +func (c *ConsulClient) deregisterService(serviceId string) error { + if err := c.client.Agent().ServiceDeregister(serviceId); err != nil { + return err + } + return nil +} + +func (c *ConsulClient) makeChecks(service *structs.Service, ip string, port int) []*consul.AgentServiceCheck { + var checks []*consul.AgentServiceCheck for _, check := range service.Checks { - c := &api.AgentServiceCheck{ + c := &consul.AgentServiceCheck{ Interval: check.Interval.String(), Timeout: check.Timeout.String(), } From 3866d4ed4f7f57389f1e50c6a1db71336603c123 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 04:59:57 -0800 Subject: [PATCH 10/53] Shutting down consul an not trying to de-register the consul service --- client/client.go | 6 ++++++ client/consul.go | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/client/client.go b/client/client.go index e33872782..8e605f4a5 100644 --- a/client/client.go +++ b/client/client.go @@ -145,6 +145,9 @@ func NewClient(cfg *config.Config) (*Client, error) { // Start the client! go c.run() + + // Start the consul client + go c.consulClient.SyncWithConsul() return c, nil } @@ -209,6 +212,9 @@ func (c *Client) Shutdown() error { } } + // Stop the consul client + c.consulClient.ShutDown() + c.shutdown = true close(c.shutdownCh) c.connPool.Shutdown() diff --git a/client/consul.go b/client/consul.go index 5814450f9..db22eea56 100644 --- a/client/consul.go +++ b/client/consul.go @@ -30,7 +30,6 @@ type ConsulClient struct { func NewConsulClient(logger *log.Logger) (*ConsulClient, error) { var err error var c *consul.Client - ts := make(map[string]*trackedService) if c, err = consul.NewClient(consul.DefaultConfig()); err != nil { return nil, err } @@ -38,7 +37,8 @@ func NewConsulClient(logger *log.Logger) (*ConsulClient, error) { consulClient := ConsulClient{ client: c, logger: logger, - trackedServices: ts, + trackedServices: make(map[string]*trackedService), + shutdownCh: make(chan struct{}), } return &consulClient, nil @@ -53,6 +53,7 @@ func (c *ConsulClient) Register(task *structs.Task, allocID string) error { ts := &trackedService{ allocId: allocID, task: task, + service: service, } c.trackedServices[service.Id] = ts @@ -74,6 +75,10 @@ func (c *ConsulClient) Deregister(task *structs.Task) error { return mErr.ErrorOrNil() } +func (c *ConsulClient) ShutDown() { + close(c.shutdownCh) +} + func (c *ConsulClient) findPortAndHostForLabel(portLabel string, task *structs.Task) (string, int) { for _, network := range task.Resources.Networks { if p, ok := network.MapLabelToValues()[portLabel]; ok { @@ -90,6 +95,8 @@ func (c *ConsulClient) SyncWithConsul() { for { select { case <-sync: + sync = time.After(syncInterval) + c.logger.Printf("[DEBUG] Syncing with consul") var consulServices map[string]*consul.AgentService var err error if consulServices, err = agent.Services(); err != nil { @@ -104,6 +111,9 @@ func (c *ConsulClient) SyncWithConsul() { } for serviceId := range consulServices { + if serviceId == "consul" { + continue + } if _, ok := c.trackedServices[serviceId]; !ok { if err := c.deregisterService(serviceId); err != nil { c.logger.Printf("[DEBUG] Error while de-registering service with ID: %s", serviceId) @@ -111,6 +121,7 @@ func (c *ConsulClient) SyncWithConsul() { } } case <-c.shutdownCh: + c.logger.Printf("[INFO] Shutting down Consul Client") return } } From 6d81111995464152d4e97817c466fc7abd1a01c3 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 05:15:52 -0800 Subject: [PATCH 11/53] Added the option to configure consul address --- client/alloc_runner_test.go | 4 ++-- client/client.go | 3 ++- client/consul.go | 6 ++++-- client/task_runner_test.go | 4 ++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index 148a33a42..15077c76c 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -31,7 +31,7 @@ func testAllocRunner(restarts bool) (*MockAllocStateUpdater, *AllocRunner) { conf.AllocDir = os.TempDir() upd := &MockAllocStateUpdater{} alloc := mock.Alloc() - consulClient, _ := NewConsulClient(logger) + consulClient, _ := NewConsulClient(logger, "127.0.0.1:8500") if !restarts { alloc.Job.Type = structs.JobTypeBatch *alloc.Job.LookupTaskGroup(alloc.TaskGroup).RestartPolicy = structs.RestartPolicy{Attempts: 0} @@ -142,7 +142,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { } // Create a new alloc runner - consulClient, err := NewConsulClient(ar.logger) + consulClient, err := NewConsulClient(ar.logger, "127.0.0.1:8500") ar2 := NewAllocRunner(ar.logger, ar.config, upd.Update, &structs.Allocation{ID: ar.alloc.ID}, consulClient) err = ar2.RestoreState() diff --git a/client/client.go b/client/client.go index 8e605f4a5..040d80bbf 100644 --- a/client/client.go +++ b/client/client.go @@ -99,7 +99,8 @@ func NewClient(cfg *config.Config) (*Client, error) { logger := log.New(cfg.LogOutput, "", log.LstdFlags) // Create the consul client - consulClient, err := NewConsulClient(logger) + consulAddr := cfg.ReadDefault("consul.address", "127.0.0.1:8500") + consulClient, err := NewConsulClient(logger, consulAddr) if err != nil { return nil, fmt.Errorf("failed to create the consul client: %v", err) } diff --git a/client/consul.go b/client/consul.go index db22eea56..0e122f13a 100644 --- a/client/consul.go +++ b/client/consul.go @@ -27,10 +27,12 @@ type ConsulClient struct { trackedServices map[string]*trackedService } -func NewConsulClient(logger *log.Logger) (*ConsulClient, error) { +func NewConsulClient(logger *log.Logger, consulAddr string) (*ConsulClient, error) { var err error var c *consul.Client - if c, err = consul.NewClient(consul.DefaultConfig()); err != nil { + cfg := consul.DefaultConfig() + cfg.Address = consulAddr + if c, err = consul.NewClient(cfg); err != nil { return nil, err } diff --git a/client/task_runner_test.go b/client/task_runner_test.go index 788b0b1d5..f8bc9e466 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -32,7 +32,7 @@ func testTaskRunner(restarts bool) (*MockTaskStateUpdater, *TaskRunner) { upd := &MockTaskStateUpdater{} alloc := mock.Alloc() task := alloc.Job.TaskGroups[0].Tasks[0] - consulClient, _ := NewConsulClient(logger) + consulClient, _ := NewConsulClient(logger, "127.0.0.1:8500") // Initialize the port listing. This should be done by the offer process but // we have a mock so that doesn't happen. task.Resources.Networks[0].ReservedPorts = []structs.Port{{"", 80}} @@ -164,7 +164,7 @@ func TestTaskRunner_SaveRestoreState(t *testing.T) { } // Create a new task runner - consulClient, _ := NewConsulClient(tr.logger) + consulClient, _ := NewConsulClient(tr.logger, "127.0.0.1:8500") tr2 := NewTaskRunner(tr.logger, tr.config, upd.Update, tr.ctx, tr.allocID, &structs.Task{Name: tr.task.Name}, tr.state, tr.restartTracker, consulClient) From 112e385066ab8a80e18e1548d752b1d483955234 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 05:20:57 -0800 Subject: [PATCH 12/53] Removed a debug log --- client/consul.go | 1 - 1 file changed, 1 deletion(-) diff --git a/client/consul.go b/client/consul.go index 0e122f13a..ca0feedc8 100644 --- a/client/consul.go +++ b/client/consul.go @@ -98,7 +98,6 @@ func (c *ConsulClient) SyncWithConsul() { select { case <-sync: sync = time.After(syncInterval) - c.logger.Printf("[DEBUG] Syncing with consul") var consulServices map[string]*consul.AgentService var err error if consulServices, err = agent.Services(); err != nil { From 90f589fdc432ac6120ec2262b67da0e92ac6e6b2 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 05:49:40 -0800 Subject: [PATCH 13/53] Added some docs --- client/consul.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/client/consul.go b/client/consul.go index ca0feedc8..0573364cf 100644 --- a/client/consul.go +++ b/client/consul.go @@ -100,10 +100,15 @@ func (c *ConsulClient) SyncWithConsul() { sync = time.After(syncInterval) var consulServices map[string]*consul.AgentService var err error + + // Get the list of the services that Consul knows about if consulServices, err = agent.Services(); err != nil { c.logger.Printf("[DEBUG] Error while syncing services with Consul: %v", err) continue } + + // See if we have services that Consul doesn't know about yet. + // Register with Consul the services which are not registered for serviceId := range c.trackedServices { if _, ok := consulServices[serviceId]; !ok { ts := c.trackedServices[serviceId] @@ -111,6 +116,8 @@ func (c *ConsulClient) SyncWithConsul() { } } + // See if consul thinks we have some services which are not running + // anymore on the node. We de-register those services for serviceId := range consulServices { if serviceId == "consul" { continue From 61b3b706f019c03cd034a2d0dab17f599fa790c9 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 09:36:37 -0800 Subject: [PATCH 14/53] Added a lock around modification of tracked services map --- client/consul.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/client/consul.go b/client/consul.go index 0573364cf..2bb194af3 100644 --- a/client/consul.go +++ b/client/consul.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/nomad/structs" "log" + "sync" "time" ) @@ -25,6 +26,7 @@ type ConsulClient struct { shutdownCh chan struct{} trackedServices map[string]*trackedService + trackedSrvLock sync.Mutex } func NewConsulClient(logger *log.Logger, consulAddr string) (*ConsulClient, error) { @@ -57,8 +59,9 @@ func (c *ConsulClient) Register(task *structs.Task, allocID string) error { task: task, service: service, } + c.trackedSrvLock.Lock() c.trackedServices[service.Id] = ts - + c.trackedSrvLock.Unlock() } return mErr.ErrorOrNil() @@ -72,7 +75,9 @@ func (c *ConsulClient) Deregister(task *structs.Task) error { c.logger.Printf("[ERROR] Error in de-registering service %v from Consul", service.Name) mErr.Errors = append(mErr.Errors, err) } + c.trackedSrvLock.Lock() delete(c.trackedServices, service.Id) + c.trackedSrvLock.Unlock() } return mErr.ErrorOrNil() } From 10c0e7f3eca3cb4b904386c18c4a40cda96c7efe Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 10:26:24 -0800 Subject: [PATCH 15/53] Removing support for scrpt checks --- nomad/structs/structs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index c4dbc5c45..7fde437cc 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1024,7 +1024,7 @@ func (sc *ServiceCheck) Validate() error { if sc.Type == ServiceCheckScript && sc.Script == "" { return fmt.Errorf("Script checks need the script to invoke") } - if t != ServiceCheckTCP && t != ServiceCheckHTTP && t != ServiceCheckDocker && t != ServiceCheckScript { + if t != ServiceCheckTCP && t != ServiceCheckHTTP { return fmt.Errorf("Check with name %v has invalid check type: %s ", sc.Name, sc.Type) } return nil From 32d9d9f4bfa1f985ca64e723d2cc25c5f5642ff8 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 10:32:31 -0800 Subject: [PATCH 16/53] Added a log line to indicate we are registering a service with Consul --- client/consul.go | 1 + 1 file changed, 1 insertion(+) diff --git a/client/consul.go b/client/consul.go index 2bb194af3..b3fdfe6c1 100644 --- a/client/consul.go +++ b/client/consul.go @@ -51,6 +51,7 @@ func NewConsulClient(logger *log.Logger, consulAddr string) (*ConsulClient, erro func (c *ConsulClient) Register(task *structs.Task, allocID string) error { var mErr multierror.Error for _, service := range task.Services { + c.logger.Printf("[INFO] Registering service %s with Consul.", service.Name) if err := c.registerService(service, task, allocID); err != nil { mErr.Errors = append(mErr.Errors, err) } From 25484421d9ac2445d17208ffb3e422bba19e3b23 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 11:13:03 -0800 Subject: [PATCH 17/53] increased test timeout to 80s so docker tests don't timeout and panic --- scripts/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/test.sh b/scripts/test.sh index 7071669a7..cbb2d016d 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -10,4 +10,4 @@ go build -o $TEMPDIR/nomad || exit 1 # Run the tests echo "--> Running tests" -go list ./... | PATH=$TEMPDIR:$PATH xargs -n1 go test -cover -timeout=40s +go list ./... | PATH=$TEMPDIR:$PATH xargs -n1 go test -cover -timeout=80s From f4acdfd035c2ffa196035cc574d8b40a1795abb3 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 11:28:07 -0800 Subject: [PATCH 18/53] Added missing %s to error format string --- client/driver/docker.go | 2 +- client/driver/docker_test.go | 6 ++++++ website/Gemfile | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index ab0ac96e0..6fa1af600 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -431,7 +431,7 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle if len(containers) != 1 { log.Printf("[ERR] driver.docker: failed to get id for container %s", config.Name) - return nil, fmt.Errorf("Failed to get id for container %s", config.Name, err) + return nil, fmt.Errorf("Failed to get id for container %s: %s", config.Name, err) } log.Printf("[INFO] driver.docker: a container with the name %s already exists; will attempt to purge and re-create", config.Name) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 4c9300579..5f4167e4a 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -545,3 +545,9 @@ func TestDockerDNS(t *testing.T) { t.Errorf("DNS Servers don't match.\nExpected:\n%s\nGot:\n%s\n", task.Config["dns_search_domains"], container.HostConfig.DNSSearch) } } + +func TestDockerAuth(t *testing.T) { + if !dockerIsConnected(t) { + t.SkipNow() + } +} diff --git a/website/Gemfile b/website/Gemfile index 2b35e2810..d0e5a286b 100644 --- a/website/Gemfile +++ b/website/Gemfile @@ -1,5 +1,5 @@ source "https://rubygems.org" -ruby "2.2.2" +ruby "2.2.3" gem "middleman-hashicorp", github: "hashicorp/middleman-hashicorp" From 85d07e72bf9654c51d2c283613c0b8f69fc1125e Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 11:28:38 -0800 Subject: [PATCH 19/53] Revert "Added missing %s to error format string" This reverts commit f4acdfd035c2ffa196035cc574d8b40a1795abb3. --- client/driver/docker.go | 2 +- client/driver/docker_test.go | 6 ------ website/Gemfile | 2 +- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index 6fa1af600..ab0ac96e0 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -431,7 +431,7 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle if len(containers) != 1 { log.Printf("[ERR] driver.docker: failed to get id for container %s", config.Name) - return nil, fmt.Errorf("Failed to get id for container %s: %s", config.Name, err) + return nil, fmt.Errorf("Failed to get id for container %s", config.Name, err) } log.Printf("[INFO] driver.docker: a container with the name %s already exists; will attempt to purge and re-create", config.Name) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 5f4167e4a..4c9300579 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -545,9 +545,3 @@ func TestDockerDNS(t *testing.T) { t.Errorf("DNS Servers don't match.\nExpected:\n%s\nGot:\n%s\n", task.Config["dns_search_domains"], container.HostConfig.DNSSearch) } } - -func TestDockerAuth(t *testing.T) { - if !dockerIsConnected(t) { - t.SkipNow() - } -} diff --git a/website/Gemfile b/website/Gemfile index d0e5a286b..2b35e2810 100644 --- a/website/Gemfile +++ b/website/Gemfile @@ -1,5 +1,5 @@ source "https://rubygems.org" -ruby "2.2.3" +ruby "2.2.2" gem "middleman-hashicorp", github: "hashicorp/middleman-hashicorp" From c375000f08b821c33d174e804271da1d48f8bd35 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 11:29:49 -0800 Subject: [PATCH 20/53] Added missing %s to error format string --- client/driver/docker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index ab0ac96e0..6fa1af600 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -431,7 +431,7 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle if len(containers) != 1 { log.Printf("[ERR] driver.docker: failed to get id for container %s", config.Name) - return nil, fmt.Errorf("Failed to get id for container %s", config.Name, err) + return nil, fmt.Errorf("Failed to get id for container %s: %s", config.Name, err) } log.Printf("[INFO] driver.docker: a container with the name %s already exists; will attempt to purge and re-create", config.Name) From 9666f450e7fea4521fe3d9f9e5462745a555245f Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Wed, 18 Nov 2015 14:46:56 -0500 Subject: [PATCH 21/53] Move test code to *_test.go file. --- client/driver/executor/{test_harness.go => test_harness_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename client/driver/executor/{test_harness.go => test_harness_test.go} (100%) diff --git a/client/driver/executor/test_harness.go b/client/driver/executor/test_harness_test.go similarity index 100% rename from client/driver/executor/test_harness.go rename to client/driver/executor/test_harness_test.go From 8917fd8408a822aba958e092def42f0ba3f5db71 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 12:14:28 -0800 Subject: [PATCH 22/53] Added the consul docs --- website/source/docs/jobspec/index.html.md | 26 +++- .../docs/jobspec/servicediscovery.html.md | 123 ++++++++++++++++++ 2 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 website/source/docs/jobspec/servicediscovery.html.md diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index cffc04ed1..1a3a6b216 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -47,6 +47,15 @@ job "my-service" { config { image = "hashicorp/web-frontend" } + service { + port = "http" + check { + type = "http" + path = "/health" + interval = "10s" + timeout = "2s" + } + } env { DB_HOST = "db01.example.com" DB_USER = "web" @@ -57,10 +66,13 @@ job "my-service" { memory = 128 network { mbits = 100 - dynamic_ports = [ - "http", - "https", - ] + # Request for a dynamic port + port "http" { + } + # Request for a static port + port "https" { + static = 443 + } } } } @@ -178,6 +190,12 @@ The `task` object supports the following keys: to start the task. The details of configurations are specific to each driver. +* `service` - Nomad integrates with Consul for Service Discovery. A service + block represents a rotable and discoverable service on the network. Nomad + automatically registers when a Task is started and de-registers it when the + Task transitons to the DEAD state. See the Service Discovery section + for more details. To learn more about Services please visit [here](/docs/jobspec/servicediscovery.html.md) + * `env` - A map of key/value representing environment variables that will be passed along to the running process. diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md new file mode 100644 index 000000000..08297a4dc --- /dev/null +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -0,0 +1,123 @@ +--- +layout: "docs" +page_title: "Service Discovery in Nomad" +sidebar_current: "docs-service-discovery" +description: |- + Learn how to add service discovery to jobs + --- + +# Service Discovery + +Nomad enables dynamic scheduling on compute resources to run services at scale +on compute nodes. Compute nodes, size of an application cluster varies depending +on volume of traffic etc, thereby the network topology of services constanly +changing. This introduces the challenge of discovery of services, Nomad solves +this problem by integrating with [Consul](https://consul.io) to provide service +discovery and health checks of services. + +Operators have to run Consul agents on a Nomad compute node.Nomad also makes +running an application like Consul agent on every single node in a data centre +simple by providing system jobs. Nomad connects to Consul on it's default http +port but operators can override it. + +## Configuration + +* `consul.address`: This is a Nomad client config which can be used to override + the default Consul Agent HTTP port that Nomad uses to connect to Consul. The + default for this is "127.0.0.1:8500" + +## Service Deginition Syntax + +The service blocks in a Task definition defines a service which Nomad will +register with Consul. Multiple Service blocks are allowed in a Task deginition, +which makes it usable for cases when an application listens on multiple ports +each exposing a specific service. + +### Example + +A brief example of a service definition in a Task +``` +group "database" { + task "mysql" { + driver = "docker" + service { + tags = ["master", "mysql"] + port = "db" + check { + type = "tcp" + delay = "10s" + timeout = "2s" + } + } + reources { + cpu = 500 + memory = 1024 + network { + mbits = 10 + port "db" { + } + } + } + } +} + +``` + +* `name`: Nomad automatically determines the name of a Task By default the name + of a service is $(job-name)-$(task-group)-$(task-name). Users can explictly + name the service by specifying this option. + +* `tags`: A list of tags associated with this Service. + +* `port`: The port indicated the port associated with the Service. Users are + reruired to specify a valid port label here which they have defined in the + resources block. This could be a label to either a dynamic or static port. If + an incorrect port label is specified, Nomad doesn't register the service with + Consul. + +* `check`: A check block defines a health check associated with the service. + Mutliple check blocks are allowed for a service. Nomad currently supports only + the `http` and `tcp` Consul Checks. + +### Check Syntax +* `type`: This indicates the check types supported by Nomad. Valid options are + currently `http` and `tcp`. In the future Nomad will add support for more + Consul checks. + +* `delay`: This indicates the frequency of the health checks that Consul with + perform. + +* `timeout`: This indicates how long Consul will wait for a health check query + to succeed. + +* `path`: The path of the http endpoint which Consul will query to query the + health of a service if the type of the check is `http`. Nomad will add the ip + of the service and the port, users are only required to add the relative url + of the health check endpoint. + +* `protocol`: This indicates the protocol for the http checks. Valid options are + `http` and `https`. + + +## Assumptions + +* The Service Discovery feature in Nomad depends on Operators making sure that the + Nomad client can reach the consul agent. + +* Nomad assumes that it controls the life cycle of all the externally + discoverable services running on a host. + +* Tasks running inside Nomad also needs to reach out to the Consul agent if they + want to use any Consul APIs. Ex: A task running inside a docker container in + the bridge mode won't be able to talk to a Consul Agent running on the + loopback interface of the host since the container in the bridge mode has it's + own network interface and doesn't see interfaces on the global network + namespaces. There are a couple of ways to solve this, one way is to run the + container in the host networking mode, or make the Consul agent listen on an + interface on the network namespace of the container. + + + + + + From d4c7a834bd405bb5fdd30ce662a2b8e70960c2cc Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 12:16:03 -0800 Subject: [PATCH 23/53] Fixed some typo --- website/source/docs/jobspec/index.html.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index 1a3a6b216..5a99bf636 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -191,10 +191,9 @@ The `task` object supports the following keys: each driver. * `service` - Nomad integrates with Consul for Service Discovery. A service - block represents a rotable and discoverable service on the network. Nomad + block represents a routable and discoverable service on the network. Nomad automatically registers when a Task is started and de-registers it when the - Task transitons to the DEAD state. See the Service Discovery section - for more details. To learn more about Services please visit [here](/docs/jobspec/servicediscovery.html.md) + Task transitons to the DEAD state. To learn more about Services please visit [here](/docs/jobspec/servicediscovery.html.md) * `env` - A map of key/value representing environment variables that will be passed along to the running process. From 3d3a7b5cf0865903fc01c894734cc59b0135d47c Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 12:19:20 -0800 Subject: [PATCH 24/53] Fixed more typos and changed some sentences --- .../source/docs/jobspec/servicediscovery.html.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index 08297a4dc..91a848d22 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -9,13 +9,13 @@ description: |- # Service Discovery Nomad enables dynamic scheduling on compute resources to run services at scale -on compute nodes. Compute nodes, size of an application cluster varies depending -on volume of traffic etc, thereby the network topology of services constanly -changing. This introduces the challenge of discovery of services, Nomad solves -this problem by integrating with [Consul](https://consul.io) to provide service +on compute nodes. Size of an application cluster varies depending +on volume of traffic, health of services, etc thereby the network topology of +services are constanly changing. This introduces the challenge of discovery of services, +Nomad solves this problem by integrating with [Consul](https://consul.io) to provide service discovery and health checks of services. -Operators have to run Consul agents on a Nomad compute node.Nomad also makes +Operators have to run Consul agents on a Nomad compute node. Nomad also makes running an application like Consul agent on every single node in a data centre simple by providing system jobs. Nomad connects to Consul on it's default http port but operators can override it. @@ -29,7 +29,7 @@ port but operators can override it. ## Service Deginition Syntax The service blocks in a Task definition defines a service which Nomad will -register with Consul. Multiple Service blocks are allowed in a Task deginition, +register with Consul. Multiple Service blocks are allowed in a Task definition, which makes it usable for cases when an application listens on multiple ports each exposing a specific service. @@ -69,7 +69,7 @@ group "database" { * `tags`: A list of tags associated with this Service. -* `port`: The port indicated the port associated with the Service. Users are +* `port`: The port indicates the port associated with the Service. Users are reruired to specify a valid port label here which they have defined in the resources block. This could be a label to either a dynamic or static port. If an incorrect port label is specified, Nomad doesn't register the service with From fe521dbf94cdb02f8e187c9042cc5c6b07ecb165 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 12:22:48 -0800 Subject: [PATCH 25/53] Added more explanation to name --- website/source/docs/jobspec/servicediscovery.html.md | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index 91a848d22..af3d84a59 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -63,15 +63,18 @@ group "database" { ``` -* `name`: Nomad automatically determines the name of a Task By default the name +* `name`: Nomad automatically determines the name of a Task. By default the name of a service is $(job-name)-$(task-group)-$(task-name). Users can explictly - name the service by specifying this option. + name the service by specifying this option. If multiple services are defined + for a Task then all the services have to be explictly named. Nomad will add + the prefix $(job-name)-${task-group}-${task-name} prefix to each user defined + names. * `tags`: A list of tags associated with this Service. * `port`: The port indicates the port associated with the Service. Users are reruired to specify a valid port label here which they have defined in the - resources block. This could be a label to either a dynamic or static port. If + resources block. This could be a label to either a dynamic or a static port. If an incorrect port label is specified, Nomad doesn't register the service with Consul. @@ -101,6 +104,8 @@ group "database" { ## Assumptions +* Consul 0.6 is needed for using the TCP checks. + * The Service Discovery feature in Nomad depends on Operators making sure that the Nomad client can reach the consul agent. From e55517d0c8ae2e7b99bfceb4aa96e0db384e6c10 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 12:24:25 -0800 Subject: [PATCH 26/53] More grammer fixes --- website/source/docs/jobspec/servicediscovery.html.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index af3d84a59..4c23225f5 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -113,11 +113,11 @@ group "database" { discoverable services running on a host. * Tasks running inside Nomad also needs to reach out to the Consul agent if they - want to use any Consul APIs. Ex: A task running inside a docker container in + want to use any of the Consul APIs. Ex: A task running inside a docker container in the bridge mode won't be able to talk to a Consul Agent running on the loopback interface of the host since the container in the bridge mode has it's own network interface and doesn't see interfaces on the global network - namespaces. There are a couple of ways to solve this, one way is to run the + namespace of the host. There are a couple of ways to solve this, one way is to run the container in the host networking mode, or make the Consul agent listen on an interface on the network namespace of the container. From d20be7b58cbc4f86008683bff0c7ae7823a90cbb Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 09:41:05 -0800 Subject: [PATCH 27/53] Update ParseAndReplace to take a list of args and remove shell style parsing --- client/driver/args/args.go | 27 ++++++--------------------- client/driver/args/args_test.go | 26 +++----------------------- 2 files changed, 9 insertions(+), 44 deletions(-) diff --git a/client/driver/args/args.go b/client/driver/args/args.go index 51793bd8b..e40be3f29 100644 --- a/client/driver/args/args.go +++ b/client/driver/args/args.go @@ -1,11 +1,6 @@ package args -import ( - "fmt" - "regexp" - - "github.com/mattn/go-shellwords" -) +import "regexp" var ( envRe = regexp.MustCompile(`\$({[a-zA-Z0-9_]+}|[a-zA-Z0-9_]+)`) @@ -13,27 +8,17 @@ var ( // ParseAndReplace takes the user supplied args and a map of environment // variables. It replaces any instance of an environment variable in the args -// with the actual value and does correct splitting of the arg list. -func ParseAndReplace(args string, env map[string]string) ([]string, error) { - // Set up parser. - p := shellwords.NewParser() - p.ParseEnv = false - p.ParseBacktick = false - - parsed, err := p.Parse(args) - if err != nil { - return nil, fmt.Errorf("Couldn't parse args %v: %v", args, err) - } - - replaced := make([]string, len(parsed)) - for i, arg := range parsed { +// with the actual value. +func ParseAndReplace(args []string, env map[string]string) ([]string, error) { + replaced := make([]string, len(args)) + for i, arg := range args { replaced[i] = ReplaceEnv(arg, env) } return replaced, nil } -// replaceEnv takes an arg and replaces all occurences of environment variables. +// ReplaceEnv takes an arg and replaces all occurences of environment variables. // If the variable is found in the passed map it is replaced, otherwise the // original string is returned. func ReplaceEnv(arg string, env map[string]string) string { diff --git a/client/driver/args/args_test.go b/client/driver/args/args_test.go index 4e1d88b59..e11b2cc8b 100644 --- a/client/driver/args/args_test.go +++ b/client/driver/args/args_test.go @@ -21,7 +21,7 @@ var ( ) func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { - input := "invalid $FOO" + input := []string{"invalid", "$FOO"} exp := []string{"invalid", "$FOO"} act, err := ParseAndReplace(input, envVars) if err != nil { @@ -34,7 +34,7 @@ func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { } func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { - input := fmt.Sprintf("nomad_ip \\\"$%v\\\"!", ipKey) + input := []string{"nomad_ip", fmt.Sprintf(`"$%v"!`, ipKey)} exp := []string{"nomad_ip", fmt.Sprintf("\"%s\"!", ipVal)} act, err := ParseAndReplace(input, envVars) if err != nil { @@ -47,7 +47,7 @@ func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { } func TestDriverArgs_ParseAndReplaceChainedEnv(t *testing.T) { - input := fmt.Sprintf("-foo $%s$%s", ipKey, portKey) + input := []string{"-foo", fmt.Sprintf("$%s$%s", ipKey, portKey)} exp := []string{"-foo", fmt.Sprintf("%s%s", ipVal, portVal)} act, err := ParseAndReplace(input, envVars) if err != nil { @@ -58,23 +58,3 @@ func TestDriverArgs_ParseAndReplaceChainedEnv(t *testing.T) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) } } - -func TestDriverArgs_ParseAndReplaceInvalidArgEscape(t *testing.T) { - input := "-c \"echo \"foo\\\" > bar.txt\"" - if _, err := ParseAndReplace(input, envVars); err == nil { - t.Fatalf("ParseAndReplace(%v, %v) should have failed", input, envVars) - } -} - -func TestDriverArgs_ParseAndReplaceValidArgEscape(t *testing.T) { - input := "-c \"echo \\\"foo\\\" > bar.txt\"" - exp := []string{"-c", "echo \"foo\" > bar.txt"} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } - - if !reflect.DeepEqual(act, exp) { - t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) - } -} From ee1887e609e9360c2129b61d0288f77e2ca5ff1f Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 11:27:59 -0800 Subject: [PATCH 28/53] Rebase --- client/driver/docker.go | 6 +++--- client/driver/docker_test.go | 10 +++++++--- client/driver/exec.go | 16 +++++----------- client/driver/exec_test.go | 16 +++++++++++----- client/driver/executor/exec_basic.go | 3 +-- client/driver/executor/exec_linux.go | 3 +-- client/driver/java.go | 12 ++++++------ client/driver/raw_exec.go | 8 +------- client/driver/raw_exec_test.go | 16 +++++++++++----- client/driver/rkt.go | 6 +++--- client/driver/rkt_test.go | 6 +++--- 11 files changed, 52 insertions(+), 50 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index 6fa1af600..a27da8fc4 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -35,7 +35,7 @@ type DockerDriverAuth struct { type DockerDriverConfig struct { ImageName string `mapstructure:"image"` // Container's Image Name Command string `mapstructure:"command"` // The Command/Entrypoint to run when the container starts up - Args string `mapstructure:"args"` // The arguments to the Command/Entrypoint + Args []string `mapstructure:"args"` // The arguments to the Command/Entrypoint NetworkMode string `mapstructure:"network_mode"` // The network mode of the container - host, net and none PortMap []map[string]int `mapstructure:"port_map"` // A map of host port labels and the ports exposed on the container Privileged bool `mapstructure:"privileged"` // Flag to run the container in priviledged mode @@ -302,12 +302,12 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri // inject it here. if driverConfig.Command != "" { cmd := []string{driverConfig.Command} - if driverConfig.Args != "" { + if len(driverConfig.Args) != 0 { cmd = append(cmd, parsedArgs...) } d.logger.Printf("[DEBUG] driver.docker: setting container startup command to: %s", strings.Join(cmd, " ")) config.Cmd = cmd - } else if driverConfig.Args != "" { + } else if len(driverConfig.Args) != 0 { d.logger.Println("[DEBUG] driver.docker: ignoring command arguments because command is not specified") } diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 4c9300579..00ffbe61b 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -137,7 +137,7 @@ func TestDockerDriver_Start_Wait(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "redis-server", - "args": "-v", + "args": []string{"-v"}, }, Resources: &structs.Resources{ MemoryMB: 256, @@ -190,7 +190,11 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "/bin/bash", - "args": fmt.Sprintf(`-c "sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`"sleep 1; echo -n %s > $%s/%s"`, + string(exp), environment.AllocDir, file), + }, }, Resources: &structs.Resources{ MemoryMB: 256, @@ -243,7 +247,7 @@ func TestDockerDriver_Start_Kill_Wait(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "/bin/sleep", - "args": "10", + "args": []string{"10"}, }, Resources: basicResources, } diff --git a/client/driver/exec.go b/client/driver/exec.go index 1f9e5b7d9..a0a136523 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -24,10 +24,10 @@ type ExecDriver struct { fingerprint.StaticFingerprinter } type ExecDriverConfig struct { - ArtifactSource string `mapstructure:"artifact_source"` - Checksum string `mapstructure:"checksum"` - Command string `mapstructure:"command"` - Args string `mapstructure:"args"` + ArtifactSource string `mapstructure:"artifact_source"` + Checksum string `mapstructure:"checksum"` + Command string `mapstructure:"command"` + Args []string `mapstructure:"args"` } // execHandle is returned from Start/Open as a handle to the PID @@ -91,14 +91,8 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, // Get the environment variables. envVars := TaskEnvironmentVariables(ctx, task) - // Look for arguments - var args []string - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) - } - // Setup the command - cmd := executor.Command(command, args...) + cmd := executor.Command(command, driverConfig.Args...) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } diff --git a/client/driver/exec_test.go b/client/driver/exec_test.go index 7e6554e1d..34b71095b 100644 --- a/client/driver/exec_test.go +++ b/client/driver/exec_test.go @@ -39,7 +39,7 @@ func TestExecDriver_StartOpen_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "5", + "args": []string{"5"}, }, Resources: basicResources, } @@ -73,7 +73,7 @@ func TestExecDriver_Start_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "2", + "args": []string{"2"}, }, Resources: basicResources, } @@ -161,7 +161,10 @@ func TestExecDriver_Start_Artifact_expanded(t *testing.T) { Config: map[string]interface{}{ "artifact_source": fmt.Sprintf("https://dl.dropboxusercontent.com/u/47675/jar_thing/%s", file), "command": "/bin/bash", - "args": fmt.Sprintf("-c '/bin/sleep 1 && %s'", filepath.Join("$NOMAD_TASK_DIR", file)), + "args": []string{ + "-c", + fmt.Sprintf(`'/bin/sleep 1 && %s'`, filepath.Join("$NOMAD_TASK_DIR", file)), + }, }, Resources: basicResources, } @@ -204,7 +207,10 @@ func TestExecDriver_Start_Wait_AllocDir(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/bash", - "args": fmt.Sprintf("-c \"sleep 1; echo -n %s > $%s/%s\"", string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`"sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + }, }, Resources: basicResources, } @@ -250,7 +256,7 @@ func TestExecDriver_Start_Kill_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 438ae6b92..7c97760ea 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -63,8 +63,7 @@ func (e *BasicExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - combined := strings.Join(e.cmd.Args, " ") - parsed, err := args.ParseAndReplace(combined, envVars.Map()) + parsed, err := args.ParseAndReplace(e.cmd.Args, envVars.Map()) if err != nil { return err } diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index 14f4f809c..c2f2931c0 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -167,8 +167,7 @@ func (e *LinuxExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - combined := strings.Join(e.cmd.Args, " ") - parsed, err := args.ParseAndReplace(combined, envVars.Map()) + parsed, err := args.ParseAndReplace(e.cmd.Args, envVars.Map()) if err != nil { return err } diff --git a/client/driver/java.go b/client/driver/java.go index eb2930a28..f408c6087 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -28,10 +28,10 @@ type JavaDriver struct { } type JavaDriverConfig struct { - JvmOpts string `mapstructure:"jvm_options"` - ArtifactSource string `mapstructure:"artifact_source"` - Checksum string `mapstructure:"checksum"` - Args string `mapstructure:"args"` + JvmOpts string `mapstructure:"jvm_options"` + ArtifactSource string `mapstructure:"artifact_source"` + Checksum string `mapstructure:"checksum"` + Args []string `mapstructure:"args"` } // javaHandle is returned from Start/Open as a handle to the PID @@ -133,8 +133,8 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, // Build the argument list. args = append(args, "-jar", filepath.Join(allocdir.TaskLocal, jarName)) - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) + if len(driverConfig.Args) != 0 { + args = append(args, driverConfig.Args...) } // Setup the command diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index d5202fc39..5b1d98269 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -93,15 +93,9 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandl // Get the environment variables. envVars := TaskEnvironmentVariables(ctx, task) - // Look for arguments - var args []string - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) - } - // Setup the command cmd := executor.NewBasicExecutor() - executor.SetCommand(cmd, command, args) + executor.SetCommand(cmd, command, driverConfig.Args) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } diff --git a/client/driver/raw_exec_test.go b/client/driver/raw_exec_test.go index 1b7a0c8db..9bf38ad6d 100644 --- a/client/driver/raw_exec_test.go +++ b/client/driver/raw_exec_test.go @@ -53,7 +53,7 @@ func TestRawExecDriver_StartOpen_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } @@ -151,7 +151,10 @@ func TestRawExecDriver_Start_Artifact_expanded(t *testing.T) { Config: map[string]interface{}{ "artifact_source": fmt.Sprintf("https://dl.dropboxusercontent.com/u/47675/jar_thing/%s", file), "command": "/bin/bash", - "args": fmt.Sprintf("-c '/bin/sleep 1 && %s'", filepath.Join("$NOMAD_TASK_DIR", file)), + "args": []string{ + "-c", + fmt.Sprintf(`'/bin/sleep 1 && %s'`, filepath.Join("$NOMAD_TASK_DIR", file)), + }, }, Resources: basicResources, } @@ -190,7 +193,7 @@ func TestRawExecDriver_Start_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } @@ -232,7 +235,10 @@ func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/bash", - "args": fmt.Sprintf(`-c "sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`"sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + }, }, Resources: basicResources, } @@ -277,7 +283,7 @@ func TestRawExecDriver_Start_Kill_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } diff --git a/client/driver/rkt.go b/client/driver/rkt.go index d09eac1db..d86129656 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -37,8 +37,8 @@ type RktDriver struct { } type RktDriverConfig struct { - ImageName string `mapstructure:"image"` - Args string `mapstructure:"args"` + ImageName string `mapstructure:"image"` + Args []string `mapstructure:"args"` } // rktHandle is returned from Start/Open as a handle to the PID @@ -150,7 +150,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e } // Add user passed arguments. - if driverConfig.Args != "" { + if len(driverConfig.Args) != 0 { parsed, err := args.ParseAndReplace(driverConfig.Args, envVars.Map()) if err != nil { return nil, err diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index a6b3dfb78..15db27b27 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -119,7 +119,7 @@ func TestRktDriver_Start_Wait(t *testing.T) { "trust_prefix": "coreos.com/etcd", "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } @@ -160,7 +160,7 @@ func TestRktDriver_Start_Wait_Skip_Trust(t *testing.T) { Config: map[string]interface{}{ "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } @@ -202,7 +202,7 @@ func TestRktDriver_Start_Wait_Logs(t *testing.T) { "trust_prefix": "coreos.com/etcd", "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } From b7e84bc124023b3913456f114c9278c4fb5a0fed Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 11:04:27 -0800 Subject: [PATCH 29/53] Docs --- website/source/docs/drivers/docker.html.md | 6 +++--- website/source/docs/drivers/exec.html.md | 22 ++++++++++++-------- website/source/docs/drivers/java.html.md | 18 +++++++++------- website/source/docs/drivers/qemu.html.md | 9 +++++--- website/source/docs/drivers/raw_exec.html.md | 22 ++++++++++++-------- website/source/docs/drivers/rkt.html.md | 17 ++++++++------- 6 files changed, 55 insertions(+), 39 deletions(-) diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index d1c6bafbe..a50bc1cec 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -19,13 +19,13 @@ and cleaning up after containers. The `docker` driver supports the following configuration in the job specification: -* `image` - (Required) The Docker image to run. The image may include a tag or +* `image` - The Docker image to run. The image may include a tag or custom URL. By default it will be fetched from Docker Hub. * `command` - (Optional) The command to run when starting the container. -* `args` - (Optional) Arguments to the optional `command`. If no `command` is - present, `args` are ignored. +* `args` - (Optional) A list of arguments to the optional `command`. If no + `command` is present, `args` are ignored. * `network_mode` - (Optional) The network mode to be used for the container. In order to support userspace networking plugins in Docker 1.9 this accepts any diff --git a/website/source/docs/drivers/exec.html.md b/website/source/docs/drivers/exec.html.md index f897b1ea4..3d921aa6d 100644 --- a/website/source/docs/drivers/exec.html.md +++ b/website/source/docs/drivers/exec.html.md @@ -20,15 +20,19 @@ scripts or other wrappers which provide higher level features. The `exec` driver supports the following configuration in the job spec: -* `command` - (Required) The command to execute. Must be provided. -* `artifact_source` – (Optional) Source location of an executable artifact. Must be accessible -from the Nomad client. If you specify an `artifact_source` to be executed, you -must reference it in the `command` as show in the examples below -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start -* `args` - The argument list to the command, space seperated. Optional. +* `command` - The command to execute. Must be provided. + +* `artifact_source` – (Optional) Source location of an executable artifact. Must + be accessible from the Nomad client. If you specify an `artifact_source` to be + executed, you must reference it in the `command` as show in the examples below + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start + +* `args` - (Optional) A list of arguments to the `command`. ## Client Requirements diff --git a/website/source/docs/drivers/java.html.md b/website/source/docs/drivers/java.html.md index f2bbd2b76..3d74fc751 100644 --- a/website/source/docs/drivers/java.html.md +++ b/website/source/docs/drivers/java.html.md @@ -18,16 +18,18 @@ HTTP from the Nomad client. The `java` driver supports the following configuration in the job spec: -* `artifact_source` - **(Required)** The hosted location of the source Jar file. Must be accessible -from the Nomad client -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start +* `artifact_source` - The hosted location of the source Jar file. Must be + accessible from the Nomad client -* `args` - **(Optional)** The argument list for the `java` command, space separated. +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start -* `jvm_options` - **(Optional)** JVM options to be passed while invoking java. These options +* `args` - (Optional) A list of arguments to the `java` command. + +* `jvm_options` - (Optional) JVM options to be passed while invoking java. These options are passed not validated in any way in Nomad. ## Client Requirements diff --git a/website/source/docs/drivers/qemu.html.md b/website/source/docs/drivers/qemu.html.md index 84909e331..d329c7a8e 100644 --- a/website/source/docs/drivers/qemu.html.md +++ b/website/source/docs/drivers/qemu.html.md @@ -23,16 +23,19 @@ The `Qemu` driver can execute any regular `qemu` image (e.g. `qcow`, `img`, The `Qemu` driver supports the following configuration in the job spec: -* `artifact_source` - **(Required)** The hosted location of the source Qemu image. Must be accessible +* `artifact_source` - The hosted location of the source Qemu image. Must be accessible from the Nomad client, via HTTP. -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` image. The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, and the value is the computed checksum. If a checksum is supplied and does not match the downloaded artifact, the driver will fail to start + * `accelerator` - (Optional) The type of accelerator to use in the invocation. If the host machine has `Qemu` installed with KVM support, users can specify `kvm` for the `accelerator`. Default is `tcg` -* `port_map` - **(Optional)** A `map[string]int` that maps port labels to ports + +* `port_map` - (Optional) A `map[string]int` that maps port labels to ports on the guest. This forwards the host port to the guest vm. For example, `port_map { db = 6539 }` would forward the host port with label `db` to the guest vm's port 6539. diff --git a/website/source/docs/drivers/raw_exec.html.md b/website/source/docs/drivers/raw_exec.html.md index 2dc741887..c76825f25 100644 --- a/website/source/docs/drivers/raw_exec.html.md +++ b/website/source/docs/drivers/raw_exec.html.md @@ -18,15 +18,19 @@ As such, it should be used with extreme care and is disabled by default. The `raw_exec` driver supports the following configuration in the job spec: -* `command` - (Required) The command to execute. Must be provided. -* `artifact_source` – (Optional) Source location of an executable artifact. Must be accessible -from the Nomad client. If you specify an `artifact_source` to be executed, you -must reference it in the `command` as show in the examples below -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start -* `args` - The argument list to the command, space seperated. Optional. +* `command` - The command to execute. Must be provided. + +* `artifact_source` – (Optional) Source location of an executable artifact. Must + be accessible from the Nomad client. If you specify an `artifact_source` to be + executed, you must reference it in the `command` as show in the examples below + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start + +* `args` - (Optional) A list of arguments to the `command`. ## Client Requirements diff --git a/website/source/docs/drivers/rkt.html.md b/website/source/docs/drivers/rkt.html.md index ddbb76601..7ce3e1a49 100644 --- a/website/source/docs/drivers/rkt.html.md +++ b/website/source/docs/drivers/rkt.html.md @@ -20,13 +20,16 @@ being marked as experimental and should be used with care. The `rkt` driver supports the following configuration in the job spec: -* `trust_prefix` - **(Optional)** The trust prefix to be passed to rkt. Must be reachable from -the box running the nomad agent. If not specified, the image is run without -verifying the image signature. -* `image` - **(Required)** The image to run which may be specified by name, -hash, ACI address or docker registry. -* `command` - **(Optional**) A command to execute on the ACI. -* `args` - **(Optional**) A string of args to pass into the image. +* `image` - The image to run which may be specified by name, hash, ACI address + or docker registry. + +* `command` - (Optional) A command to execute on the ACI. + +* `args` - (Optional) A list of arguments to the image. + +* `trust_prefix` - (Optional) The trust prefix to be passed to rkt. Must be + reachable from the box running the nomad agent. If not specified, the image is + run without verifying the image signature. ## Task Directories From c6a895aff77a3fd5295ec1de7d4ca804e48bb411 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 11:22:08 -0800 Subject: [PATCH 30/53] Fix executor tests --- client/driver/executor/exec_basic.go | 1 - client/driver/executor/test_harness_test.go | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 7c97760ea..8bf75c2ee 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -29,7 +29,6 @@ type BasicExecutor struct { allocDir string } -// TODO: Have raw_exec use this as well. func NewBasicExecutor() Executor { return &BasicExecutor{} } diff --git a/client/driver/executor/test_harness_test.go b/client/driver/executor/test_harness_test.go index 10cbac371..b5413700c 100644 --- a/client/driver/executor/test_harness_test.go +++ b/client/driver/executor/test_harness_test.go @@ -112,7 +112,7 @@ func Executor_Start_Wait(t *testing.T, command buildExecCommand) { expected := "hello world" file := filepath.Join(allocdir.TaskLocal, "output.txt") absFilePath := filepath.Join(taskDir, file) - cmd := fmt.Sprintf(`"%v \"%v\" > %v"`, "/bin/sleep 1 ; echo -n", expected, file) + cmd := fmt.Sprintf(`/bin/sleep 1 ; echo -n %v > %v`, expected, file) e := command("/bin/bash", "-c", cmd) if err := e.Limit(constraint); err != nil { @@ -190,7 +190,7 @@ func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func() Ex expected := "hello world" file := filepath.Join(allocdir.TaskLocal, "output.txt") absFilePath := filepath.Join(taskDir, file) - cmd := fmt.Sprintf(`"%v \"%v\" > %v"`, "/bin/sleep 1 ; echo -n", expected, file) + cmd := fmt.Sprintf(`/bin/sleep 1 ; echo -n %v > %v`, expected, file) e := command("/bin/bash", "-c", cmd) if err := e.Limit(constraint); err != nil { From 62a0c5d14c80c912056921430978a9a8a617c0fb Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 12:30:35 -0800 Subject: [PATCH 31/53] Rebase --- client/driver/docker.go | 2 +- client/driver/docker_test.go | 2 +- client/driver/exec_test.go | 4 ++-- client/driver/java.go | 6 +++--- client/driver/java_test.go | 3 +-- client/driver/raw_exec_test.go | 2 +- website/source/docs/drivers/java.html.md | 4 ++-- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/client/driver/docker.go b/client/driver/docker.go index a27da8fc4..69aea2b73 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -431,7 +431,7 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle if len(containers) != 1 { log.Printf("[ERR] driver.docker: failed to get id for container %s", config.Name) - return nil, fmt.Errorf("Failed to get id for container %s: %s", config.Name, err) + return nil, fmt.Errorf("Failed to get id for container %s", config.Name) } log.Printf("[INFO] driver.docker: a container with the name %s already exists; will attempt to purge and re-create", config.Name) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 00ffbe61b..a61de381c 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -192,7 +192,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { "command": "/bin/bash", "args": []string{ "-c", - fmt.Sprintf(`"sleep 1; echo -n %s > $%s/%s"`, + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), }, }, diff --git a/client/driver/exec_test.go b/client/driver/exec_test.go index 34b71095b..f5470074a 100644 --- a/client/driver/exec_test.go +++ b/client/driver/exec_test.go @@ -163,7 +163,7 @@ func TestExecDriver_Start_Artifact_expanded(t *testing.T) { "command": "/bin/bash", "args": []string{ "-c", - fmt.Sprintf(`'/bin/sleep 1 && %s'`, filepath.Join("$NOMAD_TASK_DIR", file)), + fmt.Sprintf(`/bin/sleep 1 && %s`, filepath.Join("$NOMAD_TASK_DIR", file)), }, }, Resources: basicResources, @@ -209,7 +209,7 @@ func TestExecDriver_Start_Wait_AllocDir(t *testing.T) { "command": "/bin/bash", "args": []string{ "-c", - fmt.Sprintf(`"sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), }, }, Resources: basicResources, diff --git a/client/driver/java.go b/client/driver/java.go index f408c6087..eb475db32 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -28,7 +28,7 @@ type JavaDriver struct { } type JavaDriverConfig struct { - JvmOpts string `mapstructure:"jvm_options"` + JvmOpts []string `mapstructure:"jvm_options"` ArtifactSource string `mapstructure:"artifact_source"` Checksum string `mapstructure:"checksum"` Args []string `mapstructure:"args"` @@ -126,9 +126,9 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, args := []string{} // Look for jvm options - if driverConfig.JvmOpts != "" { + if len(driverConfig.JvmOpts) != 0 { d.logger.Printf("[DEBUG] driver.java: found JVM options: %s", driverConfig.JvmOpts) - args = append(args, driverConfig.JvmOpts) + args = append(args, driverConfig.JvmOpts...) } // Build the argument list. diff --git a/client/driver/java_test.go b/client/driver/java_test.go index a0c6d3b80..af6e44d6d 100644 --- a/client/driver/java_test.go +++ b/client/driver/java_test.go @@ -51,7 +51,7 @@ func TestJavaDriver_StartOpen_Wait(t *testing.T) { Name: "demo-app", Config: map[string]interface{}{ "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/demoapp.jar", - "jvm_options": "-Xmx2048m -Xms256m", + "jvm_options": []string{"-Xmx64m", "-Xms32m"}, "checksum": "sha256:58d6e8130308d32e197c5108edd4f56ddf1417408f743097c2e662df0f0b17c8", }, Resources: basicResources, @@ -97,7 +97,6 @@ func TestJavaDriver_Start_Wait(t *testing.T) { Name: "demo-app", Config: map[string]interface{}{ "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/demoapp.jar", - "jvm_options": "-Xmx2048m -Xms256m", "checksum": "sha256:58d6e8130308d32e197c5108edd4f56ddf1417408f743097c2e662df0f0b17c8", }, Resources: basicResources, diff --git a/client/driver/raw_exec_test.go b/client/driver/raw_exec_test.go index 9bf38ad6d..8b6e0c649 100644 --- a/client/driver/raw_exec_test.go +++ b/client/driver/raw_exec_test.go @@ -237,7 +237,7 @@ func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { "command": "/bin/bash", "args": []string{ "-c", - fmt.Sprintf(`"sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), }, }, Resources: basicResources, diff --git a/website/source/docs/drivers/java.html.md b/website/source/docs/drivers/java.html.md index 3d74fc751..45baacb72 100644 --- a/website/source/docs/drivers/java.html.md +++ b/website/source/docs/drivers/java.html.md @@ -29,8 +29,8 @@ The `java` driver supports the following configuration in the job spec: * `args` - (Optional) A list of arguments to the `java` command. -* `jvm_options` - (Optional) JVM options to be passed while invoking java. These options - are passed not validated in any way in Nomad. +* `jvm_options` - (Optional) A list of JVM options to be passed while invoking + java. These options are passed not validated in any way in Nomad. ## Client Requirements From d0ebac151345040203b5ff9cc45bf47d76478f7a Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 12:28:34 -0800 Subject: [PATCH 32/53] More test fixes --- client/alloc_runner_test.go | 6 +++--- client/client_test.go | 2 +- client/task_runner_test.go | 6 +++--- nomad/mock/mock.go | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index 15077c76c..bc4a7aa4f 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -65,7 +65,7 @@ func TestAllocRunner_Destroy(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() start := time.Now() @@ -97,7 +97,7 @@ func TestAllocRunner_Update(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() defer ar.Destroy() start := time.Now() @@ -130,7 +130,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() defer ar.Destroy() diff --git a/client/client_test.go b/client/client_test.go index 90fecad6b..935c63dea 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -336,7 +336,7 @@ func TestClient_SaveRestoreState(t *testing.T) { alloc1.NodeID = c1.Node().ID task := alloc1.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} state := s1.State() err := state.UpsertAllocs(100, diff --git a/client/task_runner_test.go b/client/task_runner_test.go index f8bc9e466..1ada5060b 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -89,7 +89,7 @@ func TestTaskRunner_Destroy(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() // Begin the tear down @@ -128,7 +128,7 @@ func TestTaskRunner_Update(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() defer tr.Destroy() defer tr.ctx.AllocDir.Destroy() @@ -153,7 +153,7 @@ func TestTaskRunner_SaveRestoreState(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() defer tr.Destroy() diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 12a8484c0..25ca60ef2 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -86,7 +86,7 @@ func Job() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": "+%s", + "args": []string{"+%s"}, }, Env: map[string]string{ "FOO": "bar", @@ -151,7 +151,7 @@ func SystemJob() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": "+%s", + "args": []string{"+%s"}, }, Resources: &structs.Resources{ CPU: 500, From 5aaa0553cf65a8cdccc0544aa1368665d76974b5 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 12:40:34 -0800 Subject: [PATCH 33/53] Remove returning the error --- client/driver/args/args.go | 4 ++-- client/driver/args/args_test.go | 15 +++------------ client/driver/docker.go | 5 +---- client/driver/executor/exec_basic.go | 6 +----- client/driver/executor/exec_linux.go | 6 +----- client/driver/rkt.go | 5 +---- 6 files changed, 9 insertions(+), 32 deletions(-) diff --git a/client/driver/args/args.go b/client/driver/args/args.go index e40be3f29..9e8a9980c 100644 --- a/client/driver/args/args.go +++ b/client/driver/args/args.go @@ -9,13 +9,13 @@ var ( // ParseAndReplace takes the user supplied args and a map of environment // variables. It replaces any instance of an environment variable in the args // with the actual value. -func ParseAndReplace(args []string, env map[string]string) ([]string, error) { +func ParseAndReplace(args []string, env map[string]string) []string { replaced := make([]string, len(args)) for i, arg := range args { replaced[i] = ReplaceEnv(arg, env) } - return replaced, nil + return replaced } // ReplaceEnv takes an arg and replaces all occurences of environment variables. diff --git a/client/driver/args/args_test.go b/client/driver/args/args_test.go index e11b2cc8b..5e7cbca4a 100644 --- a/client/driver/args/args_test.go +++ b/client/driver/args/args_test.go @@ -23,10 +23,7 @@ var ( func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { input := []string{"invalid", "$FOO"} exp := []string{"invalid", "$FOO"} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) @@ -36,10 +33,7 @@ func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { input := []string{"nomad_ip", fmt.Sprintf(`"$%v"!`, ipKey)} exp := []string{"nomad_ip", fmt.Sprintf("\"%s\"!", ipVal)} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) @@ -49,10 +43,7 @@ func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { func TestDriverArgs_ParseAndReplaceChainedEnv(t *testing.T) { input := []string{"-foo", fmt.Sprintf("$%s$%s", ipKey, portKey)} exp := []string{"-foo", fmt.Sprintf("%s%s", ipVal, portVal)} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) diff --git a/client/driver/docker.go b/client/driver/docker.go index 69aea2b73..4aa97e13b 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -293,10 +293,7 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri config.ExposedPorts = exposedPorts } - parsedArgs, err := args.ParseAndReplace(driverConfig.Args, env.Map()) - if err != nil { - return c, err - } + parsedArgs := args.ParseAndReplace(driverConfig.Args, env.Map()) // If the user specified a custom command to run as their entrypoint, we'll // inject it here. diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 8bf75c2ee..2ef4fbedc 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -62,11 +62,7 @@ func (e *BasicExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - parsed, err := args.ParseAndReplace(e.cmd.Args, envVars.Map()) - if err != nil { - return err - } - e.cmd.Args = parsed + e.cmd.Args = args.ParseAndReplace(e.cmd.Args, envVars.Map()) spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index c2f2931c0..43c9270e0 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -167,11 +167,7 @@ func (e *LinuxExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - parsed, err := args.ParseAndReplace(e.cmd.Args, envVars.Map()) - if err != nil { - return err - } - e.cmd.Args = parsed + e.cmd.Args = args.ParseAndReplace(e.cmd.Args, envVars.Map()) spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) diff --git a/client/driver/rkt.go b/client/driver/rkt.go index d86129656..1fb6e9e06 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -151,10 +151,7 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e // Add user passed arguments. if len(driverConfig.Args) != 0 { - parsed, err := args.ParseAndReplace(driverConfig.Args, envVars.Map()) - if err != nil { - return nil, err - } + parsed := args.ParseAndReplace(driverConfig.Args, envVars.Map()) // Need to start arguments with "--" if len(parsed) > 0 { From f5b35e5475b51c2ec7b69d6988a3136929bc3bfa Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 13:14:08 -0800 Subject: [PATCH 34/53] Update version to 0.2.0-dev --- version.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/version.go b/version.go index 884805b31..80560f6db 100644 --- a/version.go +++ b/version.go @@ -5,9 +5,9 @@ var GitCommit string var GitDescribe string // The main version number that is being run at the moment. -const Version = "0.1.2" +const Version = "0.2.0" // A pre-release marker for the version. If this is "" (empty string) // then it means that it is a final release. Otherwise, this is a pre-release // such as "dev" (in development), "beta", "rc1", etc. -const VersionPrerelease = "" +const VersionPrerelease = "dev" From 1309a849151daa6f936098e66506af14dbc1c893 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 13:15:13 -0800 Subject: [PATCH 35/53] Use one msgpack codec and have it decode []string in nil interfaces --- nomad/fsm.go | 11 ++--------- nomad/rpc.go | 13 ++----------- nomad/structs/structs.go | 7 ++++--- nomad/timetable_test.go | 5 +++-- 4 files changed, 11 insertions(+), 25 deletions(-) diff --git a/nomad/fsm.go b/nomad/fsm.go index 21b3538e4..71c40d68f 100644 --- a/nomad/fsm.go +++ b/nomad/fsm.go @@ -13,13 +13,6 @@ import ( "github.com/hashicorp/raft" ) -var ( - msgpackHandle = &codec.MsgpackHandle{ - RawToString: true, - WriteExt: true, - } -) - const ( // timeTableGranularity is the granularity of index to time tracking timeTableGranularity = 5 * time.Minute @@ -328,7 +321,7 @@ func (n *nomadFSM) Restore(old io.ReadCloser) error { defer restore.Abort() // Create a decoder - dec := codec.NewDecoder(old, msgpackHandle) + dec := codec.NewDecoder(old, structs.MsgpackHandle) // Read in the header var header snapshotHeader @@ -412,7 +405,7 @@ func (n *nomadFSM) Restore(old io.ReadCloser) error { func (s *nomadSnapshot) Persist(sink raft.SnapshotSink) error { defer metrics.MeasureSince([]string{"nomad", "fsm", "persist"}, time.Now()) // Register the nodes - encoder := codec.NewEncoder(sink, msgpackHandle) + encoder := codec.NewEncoder(sink, structs.MsgpackHandle) // Write the header header := snapshotHeader{} diff --git a/nomad/rpc.go b/nomad/rpc.go index 123c35028..e52e258f0 100644 --- a/nomad/rpc.go +++ b/nomad/rpc.go @@ -11,7 +11,6 @@ import ( "time" "github.com/armon/go-metrics" - "github.com/hashicorp/go-msgpack/codec" "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" @@ -53,24 +52,16 @@ const ( enqueueLimit = 30 * time.Second ) -var ( - // rpcHandle is the MsgpackHandle to be used by both Client and Server codecs. - rpcHandle = &codec.MsgpackHandle{ - // Enables proper encoding of strings within nil interfaces. - RawToString: true, - } -) - // NewClientCodec returns a new rpc.ClientCodec to be used to make RPC calls to // the Nomad Server. func NewClientCodec(conn io.ReadWriteCloser) rpc.ClientCodec { - return msgpackrpc.NewCodecFromHandle(true, true, conn, rpcHandle) + return msgpackrpc.NewCodecFromHandle(true, true, conn, structs.MsgpackHandle) } // NewServerCodec returns a new rpc.ServerCodec to be used by the Nomad Server // to handle rpcs. func NewServerCodec(conn io.ReadWriteCloser) rpc.ServerCodec { - return msgpackrpc.NewCodecFromHandle(true, true, conn, rpcHandle) + return msgpackrpc.NewCodecFromHandle(true, true, conn, structs.MsgpackHandle) } // listen is used to listen for incoming RPC connections diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 7fde437cc..5ff8d8c3b 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1704,25 +1704,26 @@ func (p *PlanResult) FullCommit(plan *Plan) (bool, int, int) { } // msgpackHandle is a shared handle for encoding/decoding of structs -var msgpackHandle = func() *codec.MsgpackHandle { +var MsgpackHandle = func() *codec.MsgpackHandle { h := &codec.MsgpackHandle{RawToString: true} // Sets the default type for decoding a map into a nil interface{}. // This is necessary in particular because we store the driver configs as a // nil interface{}. h.MapType = reflect.TypeOf(map[string]interface{}(nil)) + h.SliceType = reflect.TypeOf([]string{}) return h }() // Decode is used to decode a MsgPack encoded object func Decode(buf []byte, out interface{}) error { - return codec.NewDecoder(bytes.NewReader(buf), msgpackHandle).Decode(out) + return codec.NewDecoder(bytes.NewReader(buf), MsgpackHandle).Decode(out) } // Encode is used to encode a MsgPack object with type prefix func Encode(t MessageType, msg interface{}) ([]byte, error) { var buf bytes.Buffer buf.WriteByte(uint8(t)) - err := codec.NewEncoder(&buf, msgpackHandle).Encode(msg) + err := codec.NewEncoder(&buf, MsgpackHandle).Encode(msg) return buf.Bytes(), err } diff --git a/nomad/timetable_test.go b/nomad/timetable_test.go index 3a771d6fa..a76446a13 100644 --- a/nomad/timetable_test.go +++ b/nomad/timetable_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/hashicorp/go-msgpack/codec" + "github.com/hashicorp/nomad/nomad/structs" ) func TestTimeTable(t *testing.T) { @@ -104,14 +105,14 @@ func TestTimeTable_SerializeDeserialize(t *testing.T) { tt.Witness(50, plusHour) var buf bytes.Buffer - enc := codec.NewEncoder(&buf, msgpackHandle) + enc := codec.NewEncoder(&buf, structs.MsgpackHandle) err := tt.Serialize(enc) if err != nil { t.Fatalf("err: %v", err) } - dec := codec.NewDecoder(&buf, msgpackHandle) + dec := codec.NewDecoder(&buf, structs.MsgpackHandle) tt2 := NewTimeTable(time.Second, time.Minute) err = tt2.Deserialize(dec) From 035cf7ed305b7b7dd980a118b7fa7f2575843b84 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 13:15:47 -0800 Subject: [PATCH 36/53] changed the intro --- .../docs/jobspec/servicediscovery.html.md | 39 +++++++++---------- website/source/layouts/docs.erb | 3 ++ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index 4c23225f5..6a0266587 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -8,30 +8,27 @@ description: |- # Service Discovery -Nomad enables dynamic scheduling on compute resources to run services at scale -on compute nodes. Size of an application cluster varies depending -on volume of traffic, health of services, etc thereby the network topology of -services are constanly changing. This introduces the challenge of discovery of services, -Nomad solves this problem by integrating with [Consul](https://consul.io) to provide service -discovery and health checks of services. +Nomad schedules workloads of various types across a cluster of generic hosts. +Because of this, placement is not known in advance and you will need to use +service discovery to connect tasks to other services deployed across your +cluster. Nomad integrates with [Consul](https://consul.io) to provide service +discovery and monitoring. -Operators have to run Consul agents on a Nomad compute node. Nomad also makes -running an application like Consul agent on every single node in a data centre -simple by providing system jobs. Nomad connects to Consul on it's default http -port but operators can override it. +Note that in order to use Consul with Nomad, you will need to configure and +install Consul on your nodes alongside Nomad, or schedule it as a system job. +Nomad does not currently run Consul for you. ## Configuration -* `consul.address`: This is a Nomad client config which can be used to override +* `consul.address`: This is a Nomad client configuration which can be used to override the default Consul Agent HTTP port that Nomad uses to connect to Consul. The default for this is "127.0.0.1:8500" -## Service Deginition Syntax +## Service Definition Syntax The service blocks in a Task definition defines a service which Nomad will register with Consul. Multiple Service blocks are allowed in a Task definition, -which makes it usable for cases when an application listens on multiple ports -each exposing a specific service. +which allow registering multiple services for a task that exposes multiple ports. ### Example @@ -49,7 +46,7 @@ group "database" { timeout = "2s" } } - reources { + resources { cpu = 500 memory = 1024 network { @@ -64,22 +61,22 @@ group "database" { ``` * `name`: Nomad automatically determines the name of a Task. By default the name - of a service is $(job-name)-$(task-group)-$(task-name). Users can explictly + of a service is $(job-name)-$(task-group)-$(task-name). Users can explicitly name the service by specifying this option. If multiple services are defined - for a Task then all the services have to be explictly named. Nomad will add - the prefix $(job-name)-${task-group}-${task-name} prefix to each user defined - names. + for a Task then only one task can have the default name, all the services have + to be explicitly named. Nomad will add the prefix ```$(job-name)-${task-group}-${task-name}``` + prefix to each user defined name. * `tags`: A list of tags associated with this Service. * `port`: The port indicates the port associated with the Service. Users are - reruired to specify a valid port label here which they have defined in the + required to specify a valid port label here which they have defined in the resources block. This could be a label to either a dynamic or a static port. If an incorrect port label is specified, Nomad doesn't register the service with Consul. * `check`: A check block defines a health check associated with the service. - Mutliple check blocks are allowed for a service. Nomad currently supports only + Multiple check blocks are allowed for a service. Nomad currently supports only the `http` and `tcp` Consul Checks. ### Check Syntax diff --git a/website/source/layouts/docs.erb b/website/source/layouts/docs.erb index 115623599..37769709f 100644 --- a/website/source/layouts/docs.erb +++ b/website/source/layouts/docs.erb @@ -41,6 +41,9 @@ > Scheduler Types + > + Service Discovery + From b2ac5a31c48a3dd6c5197ba097265fa01733995a Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 13:20:23 -0800 Subject: [PATCH 37/53] Reformat --- .../docs/jobspec/servicediscovery.html.md | 69 +++++++++---------- 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index 6a0266587..c156ec574 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -20,17 +20,18 @@ Nomad does not currently run Consul for you. ## Configuration -* `consul.address`: This is a Nomad client configuration which can be used to override - the default Consul Agent HTTP port that Nomad uses to connect to Consul. The - default for this is "127.0.0.1:8500" +* `consul.address`: This is a Nomad client configuration which can be used to + override the default Consul Agent HTTP port that Nomad uses to connect to + Consul. The default for this is `127.0.0.1:8500`. ## Service Definition Syntax The service blocks in a Task definition defines a service which Nomad will register with Consul. Multiple Service blocks are allowed in a Task definition, -which allow registering multiple services for a task that exposes multiple ports. +which allow registering multiple services for a task that exposes multiple +ports. -### Example +### Example A brief example of a service definition in a Task ``` @@ -60,26 +61,27 @@ group "database" { ``` -* `name`: Nomad automatically determines the name of a Task. By default the name - of a service is $(job-name)-$(task-group)-$(task-name). Users can explicitly - name the service by specifying this option. If multiple services are defined - for a Task then only one task can have the default name, all the services have - to be explicitly named. Nomad will add the prefix ```$(job-name)-${task-group}-${task-name}``` - prefix to each user defined name. +* `name`: Nomad automatically determines the name of a Task. By default the + name of a service is $(job-name)-$(task-group)-$(task-name). Users can + explicitly name the service by specifying this option. If multiple services + are defined for a Task then only one task can have the default name, all the + services have to be explicitly named. Nomad will add the prefix ```$(job-name + )-${task-group}-${task-name}``` prefix to each user defined name. * `tags`: A list of tags associated with this Service. * `port`: The port indicates the port associated with the Service. Users are required to specify a valid port label here which they have defined in the - resources block. This could be a label to either a dynamic or a static port. If - an incorrect port label is specified, Nomad doesn't register the service with - Consul. + resources block. This could be a label to either a dynamic or a static port. + If an incorrect port label is specified, Nomad doesn't register the service + with Consul. * `check`: A check block defines a health check associated with the service. - Multiple check blocks are allowed for a service. Nomad currently supports only - the `http` and `tcp` Consul Checks. + Multiple check blocks are allowed for a service. Nomad currently supports + only the `http` and `tcp` Consul Checks. + +### Check Syntax -### Check Syntax * `type`: This indicates the check types supported by Nomad. Valid options are currently `http` and `tcp`. In the future Nomad will add support for more Consul checks. @@ -95,31 +97,24 @@ group "database" { of the service and the port, users are only required to add the relative url of the health check endpoint. -* `protocol`: This indicates the protocol for the http checks. Valid options are - `http` and `https`. +* `protocol`: This indicates the protocol for the http checks. Valid options + are `http` and `https`. - -## Assumptions +## Assumptions * Consul 0.6 is needed for using the TCP checks. -* The Service Discovery feature in Nomad depends on Operators making sure that the - Nomad client can reach the consul agent. +* The Service Discovery feature in Nomad depends on Operators making sure that + the Nomad client can reach the consul agent. * Nomad assumes that it controls the life cycle of all the externally discoverable services running on a host. -* Tasks running inside Nomad also needs to reach out to the Consul agent if they - want to use any of the Consul APIs. Ex: A task running inside a docker container in - the bridge mode won't be able to talk to a Consul Agent running on the - loopback interface of the host since the container in the bridge mode has it's - own network interface and doesn't see interfaces on the global network - namespace of the host. There are a couple of ways to solve this, one way is to run the - container in the host networking mode, or make the Consul agent listen on an - interface on the network namespace of the container. - - - - - - +* Tasks running inside Nomad also needs to reach out to the Consul agent if + they want to use any of the Consul APIs. Ex: A task running inside a docker + container in the bridge mode won't be able to talk to a Consul Agent running + on the loopback interface of the host since the container in the bridge mode + has it's own network interface and doesn't see interfaces on the global + network namespace of the host. There are a couple of ways to solve this, one + way is to run the container in the host networking mode, or make the Consul + agent listen on an interface on the network namespace of the container. From f313dd53f8386023d07ca454bedf2e907fd77a42 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 13:22:43 -0800 Subject: [PATCH 38/53] Removed blank lines --- website/source/docs/jobspec/servicediscovery.html.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index 6a0266587..799412ce6 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -117,9 +117,3 @@ group "database" { namespace of the host. There are a couple of ways to solve this, one way is to run the container in the host networking mode, or make the Consul agent listen on an interface on the network namespace of the container. - - - - - - From e3583e4001ed1dc4686c63fa3a34e0ab7d2649a3 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 13:35:16 -0800 Subject: [PATCH 39/53] Fixed the sidebar-current --- website/source/docs/jobspec/servicediscovery.html.md | 2 +- website/source/layouts/docs.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index 799412ce6..212b688a7 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -1,7 +1,7 @@ --- layout: "docs" page_title: "Service Discovery in Nomad" -sidebar_current: "docs-service-discovery" +sidebar_current: "docs-jobspec-service-discovery" description: |- Learn how to add service discovery to jobs --- diff --git a/website/source/layouts/docs.erb b/website/source/layouts/docs.erb index 37769709f..5d4fe4f36 100644 --- a/website/source/layouts/docs.erb +++ b/website/source/layouts/docs.erb @@ -41,7 +41,7 @@ > Scheduler Types - > + > Service Discovery From d666f5bde1b025effe66342e2795bd668b6ffd1d Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 13:46:43 -0800 Subject: [PATCH 40/53] Revert "Make drivers take arguments as a list and not as a string" --- client/alloc_runner_test.go | 6 +-- client/client_test.go | 2 +- client/driver/args/args.go | 29 ++++++++++---- client/driver/args/args_test.go | 41 +++++++++++++++++--- client/driver/docker.go | 13 ++++--- client/driver/docker_test.go | 10 ++--- client/driver/exec.go | 16 +++++--- client/driver/exec_test.go | 16 +++----- client/driver/executor/exec_basic.go | 8 +++- client/driver/executor/exec_linux.go | 7 +++- client/driver/executor/test_harness_test.go | 4 +- client/driver/java.go | 16 ++++---- client/driver/java_test.go | 3 +- client/driver/raw_exec.go | 8 +++- client/driver/raw_exec_test.go | 16 +++----- client/driver/rkt.go | 11 ++++-- client/driver/rkt_test.go | 6 +-- client/task_runner_test.go | 6 +-- nomad/fsm.go | 11 +++++- nomad/mock/mock.go | 4 +- nomad/rpc.go | 13 ++++++- nomad/structs/structs.go | 7 ++-- nomad/timetable_test.go | 5 +-- website/source/docs/drivers/docker.html.md | 6 +-- website/source/docs/drivers/exec.html.md | 22 +++++------ website/source/docs/drivers/java.html.md | 20 +++++----- website/source/docs/drivers/qemu.html.md | 9 ++--- website/source/docs/drivers/raw_exec.html.md | 22 +++++------ website/source/docs/drivers/rkt.html.md | 17 ++++---- 29 files changed, 205 insertions(+), 149 deletions(-) diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index bc4a7aa4f..15077c76c 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -65,7 +65,7 @@ func TestAllocRunner_Destroy(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = []string{"10"} + task.Config["args"] = "10" go ar.Run() start := time.Now() @@ -97,7 +97,7 @@ func TestAllocRunner_Update(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = []string{"10"} + task.Config["args"] = "10" go ar.Run() defer ar.Destroy() start := time.Now() @@ -130,7 +130,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = []string{"10"} + task.Config["args"] = "10" go ar.Run() defer ar.Destroy() diff --git a/client/client_test.go b/client/client_test.go index 935c63dea..90fecad6b 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -336,7 +336,7 @@ func TestClient_SaveRestoreState(t *testing.T) { alloc1.NodeID = c1.Node().ID task := alloc1.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = []string{"10"} + task.Config["args"] = "10" state := s1.State() err := state.UpsertAllocs(100, diff --git a/client/driver/args/args.go b/client/driver/args/args.go index 9e8a9980c..51793bd8b 100644 --- a/client/driver/args/args.go +++ b/client/driver/args/args.go @@ -1,6 +1,11 @@ package args -import "regexp" +import ( + "fmt" + "regexp" + + "github.com/mattn/go-shellwords" +) var ( envRe = regexp.MustCompile(`\$({[a-zA-Z0-9_]+}|[a-zA-Z0-9_]+)`) @@ -8,17 +13,27 @@ var ( // ParseAndReplace takes the user supplied args and a map of environment // variables. It replaces any instance of an environment variable in the args -// with the actual value. -func ParseAndReplace(args []string, env map[string]string) []string { - replaced := make([]string, len(args)) - for i, arg := range args { +// with the actual value and does correct splitting of the arg list. +func ParseAndReplace(args string, env map[string]string) ([]string, error) { + // Set up parser. + p := shellwords.NewParser() + p.ParseEnv = false + p.ParseBacktick = false + + parsed, err := p.Parse(args) + if err != nil { + return nil, fmt.Errorf("Couldn't parse args %v: %v", args, err) + } + + replaced := make([]string, len(parsed)) + for i, arg := range parsed { replaced[i] = ReplaceEnv(arg, env) } - return replaced + return replaced, nil } -// ReplaceEnv takes an arg and replaces all occurences of environment variables. +// replaceEnv takes an arg and replaces all occurences of environment variables. // If the variable is found in the passed map it is replaced, otherwise the // original string is returned. func ReplaceEnv(arg string, env map[string]string) string { diff --git a/client/driver/args/args_test.go b/client/driver/args/args_test.go index 5e7cbca4a..4e1d88b59 100644 --- a/client/driver/args/args_test.go +++ b/client/driver/args/args_test.go @@ -21,9 +21,12 @@ var ( ) func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { - input := []string{"invalid", "$FOO"} + input := "invalid $FOO" exp := []string{"invalid", "$FOO"} - act := ParseAndReplace(input, envVars) + act, err := ParseAndReplace(input, envVars) + if err != nil { + t.Fatalf("Failed to parse valid input args %v: %v", input, err) + } if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) @@ -31,9 +34,12 @@ func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { } func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { - input := []string{"nomad_ip", fmt.Sprintf(`"$%v"!`, ipKey)} + input := fmt.Sprintf("nomad_ip \\\"$%v\\\"!", ipKey) exp := []string{"nomad_ip", fmt.Sprintf("\"%s\"!", ipVal)} - act := ParseAndReplace(input, envVars) + act, err := ParseAndReplace(input, envVars) + if err != nil { + t.Fatalf("Failed to parse valid input args %v: %v", input, err) + } if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) @@ -41,9 +47,32 @@ func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { } func TestDriverArgs_ParseAndReplaceChainedEnv(t *testing.T) { - input := []string{"-foo", fmt.Sprintf("$%s$%s", ipKey, portKey)} + input := fmt.Sprintf("-foo $%s$%s", ipKey, portKey) exp := []string{"-foo", fmt.Sprintf("%s%s", ipVal, portVal)} - act := ParseAndReplace(input, envVars) + act, err := ParseAndReplace(input, envVars) + if err != nil { + t.Fatalf("Failed to parse valid input args %v: %v", input, err) + } + + if !reflect.DeepEqual(act, exp) { + t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) + } +} + +func TestDriverArgs_ParseAndReplaceInvalidArgEscape(t *testing.T) { + input := "-c \"echo \"foo\\\" > bar.txt\"" + if _, err := ParseAndReplace(input, envVars); err == nil { + t.Fatalf("ParseAndReplace(%v, %v) should have failed", input, envVars) + } +} + +func TestDriverArgs_ParseAndReplaceValidArgEscape(t *testing.T) { + input := "-c \"echo \\\"foo\\\" > bar.txt\"" + exp := []string{"-c", "echo \"foo\" > bar.txt"} + act, err := ParseAndReplace(input, envVars) + if err != nil { + t.Fatalf("Failed to parse valid input args %v: %v", input, err) + } if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) diff --git a/client/driver/docker.go b/client/driver/docker.go index 4aa97e13b..6fa1af600 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -35,7 +35,7 @@ type DockerDriverAuth struct { type DockerDriverConfig struct { ImageName string `mapstructure:"image"` // Container's Image Name Command string `mapstructure:"command"` // The Command/Entrypoint to run when the container starts up - Args []string `mapstructure:"args"` // The arguments to the Command/Entrypoint + Args string `mapstructure:"args"` // The arguments to the Command/Entrypoint NetworkMode string `mapstructure:"network_mode"` // The network mode of the container - host, net and none PortMap []map[string]int `mapstructure:"port_map"` // A map of host port labels and the ports exposed on the container Privileged bool `mapstructure:"privileged"` // Flag to run the container in priviledged mode @@ -293,18 +293,21 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri config.ExposedPorts = exposedPorts } - parsedArgs := args.ParseAndReplace(driverConfig.Args, env.Map()) + parsedArgs, err := args.ParseAndReplace(driverConfig.Args, env.Map()) + if err != nil { + return c, err + } // If the user specified a custom command to run as their entrypoint, we'll // inject it here. if driverConfig.Command != "" { cmd := []string{driverConfig.Command} - if len(driverConfig.Args) != 0 { + if driverConfig.Args != "" { cmd = append(cmd, parsedArgs...) } d.logger.Printf("[DEBUG] driver.docker: setting container startup command to: %s", strings.Join(cmd, " ")) config.Cmd = cmd - } else if len(driverConfig.Args) != 0 { + } else if driverConfig.Args != "" { d.logger.Println("[DEBUG] driver.docker: ignoring command arguments because command is not specified") } @@ -428,7 +431,7 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle if len(containers) != 1 { log.Printf("[ERR] driver.docker: failed to get id for container %s", config.Name) - return nil, fmt.Errorf("Failed to get id for container %s", config.Name) + return nil, fmt.Errorf("Failed to get id for container %s: %s", config.Name, err) } log.Printf("[INFO] driver.docker: a container with the name %s already exists; will attempt to purge and re-create", config.Name) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index a61de381c..4c9300579 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -137,7 +137,7 @@ func TestDockerDriver_Start_Wait(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "redis-server", - "args": []string{"-v"}, + "args": "-v", }, Resources: &structs.Resources{ MemoryMB: 256, @@ -190,11 +190,7 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "/bin/bash", - "args": []string{ - "-c", - fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, - string(exp), environment.AllocDir, file), - }, + "args": fmt.Sprintf(`-c "sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), }, Resources: &structs.Resources{ MemoryMB: 256, @@ -247,7 +243,7 @@ func TestDockerDriver_Start_Kill_Wait(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "/bin/sleep", - "args": []string{"10"}, + "args": "10", }, Resources: basicResources, } diff --git a/client/driver/exec.go b/client/driver/exec.go index a0a136523..1f9e5b7d9 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -24,10 +24,10 @@ type ExecDriver struct { fingerprint.StaticFingerprinter } type ExecDriverConfig struct { - ArtifactSource string `mapstructure:"artifact_source"` - Checksum string `mapstructure:"checksum"` - Command string `mapstructure:"command"` - Args []string `mapstructure:"args"` + ArtifactSource string `mapstructure:"artifact_source"` + Checksum string `mapstructure:"checksum"` + Command string `mapstructure:"command"` + Args string `mapstructure:"args"` } // execHandle is returned from Start/Open as a handle to the PID @@ -91,8 +91,14 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, // Get the environment variables. envVars := TaskEnvironmentVariables(ctx, task) + // Look for arguments + var args []string + if driverConfig.Args != "" { + args = append(args, driverConfig.Args) + } + // Setup the command - cmd := executor.Command(command, driverConfig.Args...) + cmd := executor.Command(command, args...) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } diff --git a/client/driver/exec_test.go b/client/driver/exec_test.go index f5470074a..7e6554e1d 100644 --- a/client/driver/exec_test.go +++ b/client/driver/exec_test.go @@ -39,7 +39,7 @@ func TestExecDriver_StartOpen_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": []string{"5"}, + "args": "5", }, Resources: basicResources, } @@ -73,7 +73,7 @@ func TestExecDriver_Start_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": []string{"2"}, + "args": "2", }, Resources: basicResources, } @@ -161,10 +161,7 @@ func TestExecDriver_Start_Artifact_expanded(t *testing.T) { Config: map[string]interface{}{ "artifact_source": fmt.Sprintf("https://dl.dropboxusercontent.com/u/47675/jar_thing/%s", file), "command": "/bin/bash", - "args": []string{ - "-c", - fmt.Sprintf(`/bin/sleep 1 && %s`, filepath.Join("$NOMAD_TASK_DIR", file)), - }, + "args": fmt.Sprintf("-c '/bin/sleep 1 && %s'", filepath.Join("$NOMAD_TASK_DIR", file)), }, Resources: basicResources, } @@ -207,10 +204,7 @@ func TestExecDriver_Start_Wait_AllocDir(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/bash", - "args": []string{ - "-c", - fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), - }, + "args": fmt.Sprintf("-c \"sleep 1; echo -n %s > $%s/%s\"", string(exp), environment.AllocDir, file), }, Resources: basicResources, } @@ -256,7 +250,7 @@ func TestExecDriver_Start_Kill_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": []string{"1"}, + "args": "1", }, Resources: basicResources, } diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 2ef4fbedc..438ae6b92 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -29,6 +29,7 @@ type BasicExecutor struct { allocDir string } +// TODO: Have raw_exec use this as well. func NewBasicExecutor() Executor { return &BasicExecutor{} } @@ -62,7 +63,12 @@ func (e *BasicExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - e.cmd.Args = args.ParseAndReplace(e.cmd.Args, envVars.Map()) + combined := strings.Join(e.cmd.Args, " ") + parsed, err := args.ParseAndReplace(combined, envVars.Map()) + if err != nil { + return err + } + e.cmd.Args = parsed spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index 43c9270e0..14f4f809c 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -167,7 +167,12 @@ func (e *LinuxExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - e.cmd.Args = args.ParseAndReplace(e.cmd.Args, envVars.Map()) + combined := strings.Join(e.cmd.Args, " ") + parsed, err := args.ParseAndReplace(combined, envVars.Map()) + if err != nil { + return err + } + e.cmd.Args = parsed spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) diff --git a/client/driver/executor/test_harness_test.go b/client/driver/executor/test_harness_test.go index b5413700c..10cbac371 100644 --- a/client/driver/executor/test_harness_test.go +++ b/client/driver/executor/test_harness_test.go @@ -112,7 +112,7 @@ func Executor_Start_Wait(t *testing.T, command buildExecCommand) { expected := "hello world" file := filepath.Join(allocdir.TaskLocal, "output.txt") absFilePath := filepath.Join(taskDir, file) - cmd := fmt.Sprintf(`/bin/sleep 1 ; echo -n %v > %v`, expected, file) + cmd := fmt.Sprintf(`"%v \"%v\" > %v"`, "/bin/sleep 1 ; echo -n", expected, file) e := command("/bin/bash", "-c", cmd) if err := e.Limit(constraint); err != nil { @@ -190,7 +190,7 @@ func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func() Ex expected := "hello world" file := filepath.Join(allocdir.TaskLocal, "output.txt") absFilePath := filepath.Join(taskDir, file) - cmd := fmt.Sprintf(`/bin/sleep 1 ; echo -n %v > %v`, expected, file) + cmd := fmt.Sprintf(`"%v \"%v\" > %v"`, "/bin/sleep 1 ; echo -n", expected, file) e := command("/bin/bash", "-c", cmd) if err := e.Limit(constraint); err != nil { diff --git a/client/driver/java.go b/client/driver/java.go index eb475db32..eb2930a28 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -28,10 +28,10 @@ type JavaDriver struct { } type JavaDriverConfig struct { - JvmOpts []string `mapstructure:"jvm_options"` - ArtifactSource string `mapstructure:"artifact_source"` - Checksum string `mapstructure:"checksum"` - Args []string `mapstructure:"args"` + JvmOpts string `mapstructure:"jvm_options"` + ArtifactSource string `mapstructure:"artifact_source"` + Checksum string `mapstructure:"checksum"` + Args string `mapstructure:"args"` } // javaHandle is returned from Start/Open as a handle to the PID @@ -126,15 +126,15 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, args := []string{} // Look for jvm options - if len(driverConfig.JvmOpts) != 0 { + if driverConfig.JvmOpts != "" { d.logger.Printf("[DEBUG] driver.java: found JVM options: %s", driverConfig.JvmOpts) - args = append(args, driverConfig.JvmOpts...) + args = append(args, driverConfig.JvmOpts) } // Build the argument list. args = append(args, "-jar", filepath.Join(allocdir.TaskLocal, jarName)) - if len(driverConfig.Args) != 0 { - args = append(args, driverConfig.Args...) + if driverConfig.Args != "" { + args = append(args, driverConfig.Args) } // Setup the command diff --git a/client/driver/java_test.go b/client/driver/java_test.go index af6e44d6d..a0c6d3b80 100644 --- a/client/driver/java_test.go +++ b/client/driver/java_test.go @@ -51,7 +51,7 @@ func TestJavaDriver_StartOpen_Wait(t *testing.T) { Name: "demo-app", Config: map[string]interface{}{ "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/demoapp.jar", - "jvm_options": []string{"-Xmx64m", "-Xms32m"}, + "jvm_options": "-Xmx2048m -Xms256m", "checksum": "sha256:58d6e8130308d32e197c5108edd4f56ddf1417408f743097c2e662df0f0b17c8", }, Resources: basicResources, @@ -97,6 +97,7 @@ func TestJavaDriver_Start_Wait(t *testing.T) { Name: "demo-app", Config: map[string]interface{}{ "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/demoapp.jar", + "jvm_options": "-Xmx2048m -Xms256m", "checksum": "sha256:58d6e8130308d32e197c5108edd4f56ddf1417408f743097c2e662df0f0b17c8", }, Resources: basicResources, diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index 5b1d98269..d5202fc39 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -93,9 +93,15 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandl // Get the environment variables. envVars := TaskEnvironmentVariables(ctx, task) + // Look for arguments + var args []string + if driverConfig.Args != "" { + args = append(args, driverConfig.Args) + } + // Setup the command cmd := executor.NewBasicExecutor() - executor.SetCommand(cmd, command, driverConfig.Args) + executor.SetCommand(cmd, command, args) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } diff --git a/client/driver/raw_exec_test.go b/client/driver/raw_exec_test.go index 8b6e0c649..1b7a0c8db 100644 --- a/client/driver/raw_exec_test.go +++ b/client/driver/raw_exec_test.go @@ -53,7 +53,7 @@ func TestRawExecDriver_StartOpen_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": []string{"1"}, + "args": "1", }, Resources: basicResources, } @@ -151,10 +151,7 @@ func TestRawExecDriver_Start_Artifact_expanded(t *testing.T) { Config: map[string]interface{}{ "artifact_source": fmt.Sprintf("https://dl.dropboxusercontent.com/u/47675/jar_thing/%s", file), "command": "/bin/bash", - "args": []string{ - "-c", - fmt.Sprintf(`'/bin/sleep 1 && %s'`, filepath.Join("$NOMAD_TASK_DIR", file)), - }, + "args": fmt.Sprintf("-c '/bin/sleep 1 && %s'", filepath.Join("$NOMAD_TASK_DIR", file)), }, Resources: basicResources, } @@ -193,7 +190,7 @@ func TestRawExecDriver_Start_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": []string{"1"}, + "args": "1", }, Resources: basicResources, } @@ -235,10 +232,7 @@ func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/bash", - "args": []string{ - "-c", - fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), - }, + "args": fmt.Sprintf(`-c "sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), }, Resources: basicResources, } @@ -283,7 +277,7 @@ func TestRawExecDriver_Start_Kill_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": []string{"1"}, + "args": "1", }, Resources: basicResources, } diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 1fb6e9e06..d09eac1db 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -37,8 +37,8 @@ type RktDriver struct { } type RktDriverConfig struct { - ImageName string `mapstructure:"image"` - Args []string `mapstructure:"args"` + ImageName string `mapstructure:"image"` + Args string `mapstructure:"args"` } // rktHandle is returned from Start/Open as a handle to the PID @@ -150,8 +150,11 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e } // Add user passed arguments. - if len(driverConfig.Args) != 0 { - parsed := args.ParseAndReplace(driverConfig.Args, envVars.Map()) + if driverConfig.Args != "" { + parsed, err := args.ParseAndReplace(driverConfig.Args, envVars.Map()) + if err != nil { + return nil, err + } // Need to start arguments with "--" if len(parsed) > 0 { diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index 15db27b27..a6b3dfb78 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -119,7 +119,7 @@ func TestRktDriver_Start_Wait(t *testing.T) { "trust_prefix": "coreos.com/etcd", "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": []string{"--version"}, + "args": "--version", }, } @@ -160,7 +160,7 @@ func TestRktDriver_Start_Wait_Skip_Trust(t *testing.T) { Config: map[string]interface{}{ "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": []string{"--version"}, + "args": "--version", }, } @@ -202,7 +202,7 @@ func TestRktDriver_Start_Wait_Logs(t *testing.T) { "trust_prefix": "coreos.com/etcd", "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": []string{"--version"}, + "args": "--version", }, } diff --git a/client/task_runner_test.go b/client/task_runner_test.go index 1ada5060b..f8bc9e466 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -89,7 +89,7 @@ func TestTaskRunner_Destroy(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = []string{"10"} + tr.task.Config["args"] = "10" go tr.Run() // Begin the tear down @@ -128,7 +128,7 @@ func TestTaskRunner_Update(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = []string{"10"} + tr.task.Config["args"] = "10" go tr.Run() defer tr.Destroy() defer tr.ctx.AllocDir.Destroy() @@ -153,7 +153,7 @@ func TestTaskRunner_SaveRestoreState(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = []string{"10"} + tr.task.Config["args"] = "10" go tr.Run() defer tr.Destroy() diff --git a/nomad/fsm.go b/nomad/fsm.go index 71c40d68f..21b3538e4 100644 --- a/nomad/fsm.go +++ b/nomad/fsm.go @@ -13,6 +13,13 @@ import ( "github.com/hashicorp/raft" ) +var ( + msgpackHandle = &codec.MsgpackHandle{ + RawToString: true, + WriteExt: true, + } +) + const ( // timeTableGranularity is the granularity of index to time tracking timeTableGranularity = 5 * time.Minute @@ -321,7 +328,7 @@ func (n *nomadFSM) Restore(old io.ReadCloser) error { defer restore.Abort() // Create a decoder - dec := codec.NewDecoder(old, structs.MsgpackHandle) + dec := codec.NewDecoder(old, msgpackHandle) // Read in the header var header snapshotHeader @@ -405,7 +412,7 @@ func (n *nomadFSM) Restore(old io.ReadCloser) error { func (s *nomadSnapshot) Persist(sink raft.SnapshotSink) error { defer metrics.MeasureSince([]string{"nomad", "fsm", "persist"}, time.Now()) // Register the nodes - encoder := codec.NewEncoder(sink, structs.MsgpackHandle) + encoder := codec.NewEncoder(sink, msgpackHandle) // Write the header header := snapshotHeader{} diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 25ca60ef2..12a8484c0 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -86,7 +86,7 @@ func Job() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": []string{"+%s"}, + "args": "+%s", }, Env: map[string]string{ "FOO": "bar", @@ -151,7 +151,7 @@ func SystemJob() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": []string{"+%s"}, + "args": "+%s", }, Resources: &structs.Resources{ CPU: 500, diff --git a/nomad/rpc.go b/nomad/rpc.go index e52e258f0..123c35028 100644 --- a/nomad/rpc.go +++ b/nomad/rpc.go @@ -11,6 +11,7 @@ import ( "time" "github.com/armon/go-metrics" + "github.com/hashicorp/go-msgpack/codec" "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" @@ -52,16 +53,24 @@ const ( enqueueLimit = 30 * time.Second ) +var ( + // rpcHandle is the MsgpackHandle to be used by both Client and Server codecs. + rpcHandle = &codec.MsgpackHandle{ + // Enables proper encoding of strings within nil interfaces. + RawToString: true, + } +) + // NewClientCodec returns a new rpc.ClientCodec to be used to make RPC calls to // the Nomad Server. func NewClientCodec(conn io.ReadWriteCloser) rpc.ClientCodec { - return msgpackrpc.NewCodecFromHandle(true, true, conn, structs.MsgpackHandle) + return msgpackrpc.NewCodecFromHandle(true, true, conn, rpcHandle) } // NewServerCodec returns a new rpc.ServerCodec to be used by the Nomad Server // to handle rpcs. func NewServerCodec(conn io.ReadWriteCloser) rpc.ServerCodec { - return msgpackrpc.NewCodecFromHandle(true, true, conn, structs.MsgpackHandle) + return msgpackrpc.NewCodecFromHandle(true, true, conn, rpcHandle) } // listen is used to listen for incoming RPC connections diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 5ff8d8c3b..7fde437cc 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1704,26 +1704,25 @@ func (p *PlanResult) FullCommit(plan *Plan) (bool, int, int) { } // msgpackHandle is a shared handle for encoding/decoding of structs -var MsgpackHandle = func() *codec.MsgpackHandle { +var msgpackHandle = func() *codec.MsgpackHandle { h := &codec.MsgpackHandle{RawToString: true} // Sets the default type for decoding a map into a nil interface{}. // This is necessary in particular because we store the driver configs as a // nil interface{}. h.MapType = reflect.TypeOf(map[string]interface{}(nil)) - h.SliceType = reflect.TypeOf([]string{}) return h }() // Decode is used to decode a MsgPack encoded object func Decode(buf []byte, out interface{}) error { - return codec.NewDecoder(bytes.NewReader(buf), MsgpackHandle).Decode(out) + return codec.NewDecoder(bytes.NewReader(buf), msgpackHandle).Decode(out) } // Encode is used to encode a MsgPack object with type prefix func Encode(t MessageType, msg interface{}) ([]byte, error) { var buf bytes.Buffer buf.WriteByte(uint8(t)) - err := codec.NewEncoder(&buf, MsgpackHandle).Encode(msg) + err := codec.NewEncoder(&buf, msgpackHandle).Encode(msg) return buf.Bytes(), err } diff --git a/nomad/timetable_test.go b/nomad/timetable_test.go index a76446a13..3a771d6fa 100644 --- a/nomad/timetable_test.go +++ b/nomad/timetable_test.go @@ -7,7 +7,6 @@ import ( "time" "github.com/hashicorp/go-msgpack/codec" - "github.com/hashicorp/nomad/nomad/structs" ) func TestTimeTable(t *testing.T) { @@ -105,14 +104,14 @@ func TestTimeTable_SerializeDeserialize(t *testing.T) { tt.Witness(50, plusHour) var buf bytes.Buffer - enc := codec.NewEncoder(&buf, structs.MsgpackHandle) + enc := codec.NewEncoder(&buf, msgpackHandle) err := tt.Serialize(enc) if err != nil { t.Fatalf("err: %v", err) } - dec := codec.NewDecoder(&buf, structs.MsgpackHandle) + dec := codec.NewDecoder(&buf, msgpackHandle) tt2 := NewTimeTable(time.Second, time.Minute) err = tt2.Deserialize(dec) diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index a50bc1cec..d1c6bafbe 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -19,13 +19,13 @@ and cleaning up after containers. The `docker` driver supports the following configuration in the job specification: -* `image` - The Docker image to run. The image may include a tag or +* `image` - (Required) The Docker image to run. The image may include a tag or custom URL. By default it will be fetched from Docker Hub. * `command` - (Optional) The command to run when starting the container. -* `args` - (Optional) A list of arguments to the optional `command`. If no - `command` is present, `args` are ignored. +* `args` - (Optional) Arguments to the optional `command`. If no `command` is + present, `args` are ignored. * `network_mode` - (Optional) The network mode to be used for the container. In order to support userspace networking plugins in Docker 1.9 this accepts any diff --git a/website/source/docs/drivers/exec.html.md b/website/source/docs/drivers/exec.html.md index 3d921aa6d..f897b1ea4 100644 --- a/website/source/docs/drivers/exec.html.md +++ b/website/source/docs/drivers/exec.html.md @@ -20,19 +20,15 @@ scripts or other wrappers which provide higher level features. The `exec` driver supports the following configuration in the job spec: -* `command` - The command to execute. Must be provided. - -* `artifact_source` – (Optional) Source location of an executable artifact. Must - be accessible from the Nomad client. If you specify an `artifact_source` to be - executed, you must reference it in the `command` as show in the examples below - -* `checksum` - (Optional) The checksum type and value for the `artifact_source` - image. The format is `type:value`, where type is any of `md5`, `sha1`, - `sha256`, or `sha512`, and the value is the computed checksum. If a checksum - is supplied and does not match the downloaded artifact, the driver will fail - to start - -* `args` - (Optional) A list of arguments to the `command`. +* `command` - (Required) The command to execute. Must be provided. +* `artifact_source` – (Optional) Source location of an executable artifact. Must be accessible +from the Nomad client. If you specify an `artifact_source` to be executed, you +must reference it in the `command` as show in the examples below +* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. +The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, +and the value is the computed checksum. If a checksum is supplied and does not +match the downloaded artifact, the driver will fail to start +* `args` - The argument list to the command, space seperated. Optional. ## Client Requirements diff --git a/website/source/docs/drivers/java.html.md b/website/source/docs/drivers/java.html.md index 45baacb72..f2bbd2b76 100644 --- a/website/source/docs/drivers/java.html.md +++ b/website/source/docs/drivers/java.html.md @@ -18,19 +18,17 @@ HTTP from the Nomad client. The `java` driver supports the following configuration in the job spec: -* `artifact_source` - The hosted location of the source Jar file. Must be - accessible from the Nomad client +* `artifact_source` - **(Required)** The hosted location of the source Jar file. Must be accessible +from the Nomad client +* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. +The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, +and the value is the computed checksum. If a checksum is supplied and does not +match the downloaded artifact, the driver will fail to start -* `checksum` - (Optional) The checksum type and value for the `artifact_source` - image. The format is `type:value`, where type is any of `md5`, `sha1`, - `sha256`, or `sha512`, and the value is the computed checksum. If a checksum - is supplied and does not match the downloaded artifact, the driver will fail - to start +* `args` - **(Optional)** The argument list for the `java` command, space separated. -* `args` - (Optional) A list of arguments to the `java` command. - -* `jvm_options` - (Optional) A list of JVM options to be passed while invoking - java. These options are passed not validated in any way in Nomad. +* `jvm_options` - **(Optional)** JVM options to be passed while invoking java. These options + are passed not validated in any way in Nomad. ## Client Requirements diff --git a/website/source/docs/drivers/qemu.html.md b/website/source/docs/drivers/qemu.html.md index d329c7a8e..84909e331 100644 --- a/website/source/docs/drivers/qemu.html.md +++ b/website/source/docs/drivers/qemu.html.md @@ -23,19 +23,16 @@ The `Qemu` driver can execute any regular `qemu` image (e.g. `qcow`, `img`, The `Qemu` driver supports the following configuration in the job spec: -* `artifact_source` - The hosted location of the source Qemu image. Must be accessible +* `artifact_source` - **(Required)** The hosted location of the source Qemu image. Must be accessible from the Nomad client, via HTTP. - -* `checksum` - (Optional) The checksum type and value for the `artifact_source` image. +* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, and the value is the computed checksum. If a checksum is supplied and does not match the downloaded artifact, the driver will fail to start - * `accelerator` - (Optional) The type of accelerator to use in the invocation. If the host machine has `Qemu` installed with KVM support, users can specify `kvm` for the `accelerator`. Default is `tcg` - -* `port_map` - (Optional) A `map[string]int` that maps port labels to ports +* `port_map` - **(Optional)** A `map[string]int` that maps port labels to ports on the guest. This forwards the host port to the guest vm. For example, `port_map { db = 6539 }` would forward the host port with label `db` to the guest vm's port 6539. diff --git a/website/source/docs/drivers/raw_exec.html.md b/website/source/docs/drivers/raw_exec.html.md index c76825f25..2dc741887 100644 --- a/website/source/docs/drivers/raw_exec.html.md +++ b/website/source/docs/drivers/raw_exec.html.md @@ -18,19 +18,15 @@ As such, it should be used with extreme care and is disabled by default. The `raw_exec` driver supports the following configuration in the job spec: -* `command` - The command to execute. Must be provided. - -* `artifact_source` – (Optional) Source location of an executable artifact. Must - be accessible from the Nomad client. If you specify an `artifact_source` to be - executed, you must reference it in the `command` as show in the examples below - -* `checksum` - (Optional) The checksum type and value for the `artifact_source` - image. The format is `type:value`, where type is any of `md5`, `sha1`, - `sha256`, or `sha512`, and the value is the computed checksum. If a checksum - is supplied and does not match the downloaded artifact, the driver will fail - to start - -* `args` - (Optional) A list of arguments to the `command`. +* `command` - (Required) The command to execute. Must be provided. +* `artifact_source` – (Optional) Source location of an executable artifact. Must be accessible +from the Nomad client. If you specify an `artifact_source` to be executed, you +must reference it in the `command` as show in the examples below +* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. +The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, +and the value is the computed checksum. If a checksum is supplied and does not +match the downloaded artifact, the driver will fail to start +* `args` - The argument list to the command, space seperated. Optional. ## Client Requirements diff --git a/website/source/docs/drivers/rkt.html.md b/website/source/docs/drivers/rkt.html.md index 7ce3e1a49..ddbb76601 100644 --- a/website/source/docs/drivers/rkt.html.md +++ b/website/source/docs/drivers/rkt.html.md @@ -20,16 +20,13 @@ being marked as experimental and should be used with care. The `rkt` driver supports the following configuration in the job spec: -* `image` - The image to run which may be specified by name, hash, ACI address - or docker registry. - -* `command` - (Optional) A command to execute on the ACI. - -* `args` - (Optional) A list of arguments to the image. - -* `trust_prefix` - (Optional) The trust prefix to be passed to rkt. Must be - reachable from the box running the nomad agent. If not specified, the image is - run without verifying the image signature. +* `trust_prefix` - **(Optional)** The trust prefix to be passed to rkt. Must be reachable from +the box running the nomad agent. If not specified, the image is run without +verifying the image signature. +* `image` - **(Required)** The image to run which may be specified by name, +hash, ACI address or docker registry. +* `command` - **(Optional**) A command to execute on the ACI. +* `args` - **(Optional**) A string of args to pass into the image. ## Task Directories From bd22e8d33c0d557d8897f37245e7e6e036173e50 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 14:11:23 -0800 Subject: [PATCH 41/53] Fix errant spaces in frontmatter --- website/source/docs/jobspec/servicediscovery.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index f22e6e1e7..fb02a7387 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -4,7 +4,7 @@ page_title: "Service Discovery in Nomad" sidebar_current: "docs-jobspec-service-discovery" description: |- Learn how to add service discovery to jobs - --- +--- # Service Discovery From 6e6c7258c02ccb8893702a97322449b1dbc01fed Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 14:22:42 -0800 Subject: [PATCH 42/53] Fix formatting issue that made most of the page a pre-formatted block --- website/source/docs/jobspec/servicediscovery.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index fb02a7387..447ae1f1e 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -34,6 +34,7 @@ ports. ### Example A brief example of a service definition in a Task + ``` group "database" { task "mysql" { @@ -58,7 +59,6 @@ group "database" { } } } - ``` * `name`: Nomad automatically determines the name of a Task. By default the From b422b8eaa1a1072716c4af79c7e3a7807ad9838b Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 14:29:52 -0800 Subject: [PATCH 43/53] Wrap literal strings in backticks so they don't turn into URLs --- website/helpers/command_helpers.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/helpers/command_helpers.rb b/website/helpers/command_helpers.rb index 45d437599..0718f2d48 100644 --- a/website/helpers/command_helpers.rb +++ b/website/helpers/command_helpers.rb @@ -2,8 +2,8 @@ module CommandHelpers # Returns the markdown text for the general options usage. def general_options_usage() <`: The address of the Nomad server. Overrides the "NOMAD_ADDR" - environment variable if set. Defaults to "http://127.0.0.1:4646". +* `-address=`: The address of the Nomad server. Overrides the `NOMAD_ADDR` + environment variable if set. Defaults to `http://127.0.0.1:4646`. EOF end end From 197e755506c5c9299af3e4ffaec9b387fd1a23ab Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 13:02:25 -0800 Subject: [PATCH 44/53] Changed the http field to path --- api/tasks.go | 2 +- client/consul.go | 4 +++- nomad/structs/structs.go | 10 +++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 2990b5433..c378c222d 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -27,7 +27,7 @@ type ServiceCheck struct { Name string Type string Script string - Http string + Path string Protocol string Interval time.Duration Timeout time.Duration diff --git a/client/consul.go b/client/consul.go index b3fdfe6c1..0c2988a52 100644 --- a/client/consul.go +++ b/client/consul.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/nomad/structs" "log" + "path" "sync" "time" ) @@ -180,7 +181,8 @@ func (c *ConsulClient) makeChecks(service *structs.Service, ip string, port int) } switch check.Type { case structs.ServiceCheckHTTP: - c.HTTP = fmt.Sprintf("%s://%s:%d/%s", check.Protocol, ip, port, check.Http) + baseUrl := fmt.Sprintf("%s://%s:%d", check.Protocol, ip, port) + c.HTTP = path.Join(baseUrl, check.Path) case structs.ServiceCheckTCP: c.TCP = fmt.Sprintf("%s:%d", ip, port) case structs.ServiceCheckScript: diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 5ff8d8c3b..876bb0c39 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1009,7 +1009,7 @@ type ServiceCheck struct { Name string // Name of the check, defaults to id Type string // Type of the check - tcp, http, docker and script Script string // Script to invoke for script check - Http string // path of the health check url for http type check + Path string // path of the health check url for http type check Protocol string // Protocol to use if check is http, defaults to http Interval time.Duration // Interval of the check Timeout time.Duration // Timeout of the response from the check before consul fails the check @@ -1017,16 +1017,16 @@ type ServiceCheck struct { func (sc *ServiceCheck) Validate() error { t := strings.ToLower(sc.Type) - if sc.Type == ServiceCheckHTTP && sc.Http == "" { + if t != ServiceCheckTCP && t != ServiceCheckHTTP { + return fmt.Errorf("Check with name %v has invalid check type: %s ", sc.Name, sc.Type) + } + if sc.Type == ServiceCheckHTTP && sc.Path == "" { return fmt.Errorf("http checks needs the Http path information.") } if sc.Type == ServiceCheckScript && sc.Script == "" { return fmt.Errorf("Script checks need the script to invoke") } - if t != ServiceCheckTCP && t != ServiceCheckHTTP { - return fmt.Errorf("Check with name %v has invalid check type: %s ", sc.Name, sc.Type) - } return nil } From 0c5ddfae6bb0558a6c3783b77ad6d22fa4a6c322 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 14:19:58 -0800 Subject: [PATCH 45/53] Added a test to check check creation --- client/consul.go | 13 ++++++++++--- client/consul_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 client/consul_test.go diff --git a/client/consul.go b/client/consul.go index 0c2988a52..5761321af 100644 --- a/client/consul.go +++ b/client/consul.go @@ -6,7 +6,7 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/nomad/structs" "log" - "path" + "net/url" "sync" "time" ) @@ -179,10 +179,17 @@ func (c *ConsulClient) makeChecks(service *structs.Service, ip string, port int) Interval: check.Interval.String(), Timeout: check.Timeout.String(), } + if check.Protocol == "" { + check.Protocol = "http" + } switch check.Type { case structs.ServiceCheckHTTP: - baseUrl := fmt.Sprintf("%s://%s:%d", check.Protocol, ip, port) - c.HTTP = path.Join(baseUrl, check.Path) + url := url.URL{ + Scheme: check.Protocol, + Host: fmt.Sprintf("%s:%d", ip, port), + Path: check.Path, + } + c.HTTP = url.String() case structs.ServiceCheckTCP: c.TCP = fmt.Sprintf("%s:%d", ip, port) case structs.ServiceCheckScript: diff --git a/client/consul_test.go b/client/consul_test.go new file mode 100644 index 000000000..097bb1bcc --- /dev/null +++ b/client/consul_test.go @@ -0,0 +1,42 @@ +package client + +import ( + "github.com/hashicorp/nomad/nomad/structs" + "log" + "os" + "testing" + "time" +) + +func TestMakeChecks(t *testing.T) { + service := &structs.Service{ + Id: "Foo", + Name: "Bar", + Checks: []structs.ServiceCheck{ + { + Type: "http", + Path: "/foo/bar", + Interval: 10 * time.Second, + Timeout: 2 * time.Second, + }, + { + Type: "tcp", + Interval: 10 * time.Second, + Timeout: 2 * time.Second, + }, + }, + } + + logger := log.New(os.Stdout, "logger: ", log.Lshortfile) + + c, _ := NewConsulClient(logger, "") + checks := c.makeChecks(service, "10.10.0.1", 8090) + + if checks[0].HTTP != "http://10.10.0.1:8090/foo/bar" { + t.Fatalf("Invalid http url for check: %v", checks[0].HTTP) + } + + if checks[1].TCP != "10.10.0.1:8090" { + t.Fatalf("Invalid tcp check: %v", checks[0].TCP) + } +} From d4b1b9240ea431bbc0d4941ec17397ee6d79a79f Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 14:20:45 -0800 Subject: [PATCH 46/53] Added a test to check check creation --- client/consul.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/consul.go b/client/consul.go index 5761321af..a87c9dad1 100644 --- a/client/consul.go +++ b/client/consul.go @@ -179,11 +179,11 @@ func (c *ConsulClient) makeChecks(service *structs.Service, ip string, port int) Interval: check.Interval.String(), Timeout: check.Timeout.String(), } - if check.Protocol == "" { - check.Protocol = "http" - } switch check.Type { case structs.ServiceCheckHTTP: + if check.Protocol == "" { + check.Protocol = "http" + } url := url.URL{ Scheme: check.Protocol, Host: fmt.Sprintf("%s:%d", ip, port), From bcfda2687ed9b9604cafcc0fc1a463604d050498 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 14:22:46 -0800 Subject: [PATCH 47/53] Added a check for accpeting protocol information in consul check --- client/consul_test.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/client/consul_test.go b/client/consul_test.go index 097bb1bcc..fb844859e 100644 --- a/client/consul_test.go +++ b/client/consul_test.go @@ -19,6 +19,13 @@ func TestMakeChecks(t *testing.T) { Interval: 10 * time.Second, Timeout: 2 * time.Second, }, + { + Type: "http", + Protocol: "https", + Path: "/foo/bar", + Interval: 10 * time.Second, + Timeout: 2 * time.Second, + }, { Type: "tcp", Interval: 10 * time.Second, @@ -36,7 +43,11 @@ func TestMakeChecks(t *testing.T) { t.Fatalf("Invalid http url for check: %v", checks[0].HTTP) } - if checks[1].TCP != "10.10.0.1:8090" { + if checks[1].HTTP != "https://10.10.0.1:8090/foo/bar" { + t.Fatalf("Invalid http url for check: %v", checks[0].HTTP) + } + + if checks[2].TCP != "10.10.0.1:8090" { t.Fatalf("Invalid tcp check: %v", checks[0].TCP) } } From e22926904a3c1dc4996087088a54ecb9e3c64b3d Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 14:31:56 -0800 Subject: [PATCH 48/53] Added info for default value of check protocol --- website/source/docs/jobspec/servicediscovery.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index f22e6e1e7..83c87fa62 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -98,7 +98,7 @@ group "database" { of the health check endpoint. * `protocol`: This indicates the protocol for the http checks. Valid options - are `http` and `https`. + are `http` and `https`. We default it to `http` ## Assumptions From 021b09974b6b05a7757bb57390e08a2c8c2de9e8 Mon Sep 17 00:00:00 2001 From: Chris Bednarski Date: Wed, 18 Nov 2015 14:39:33 -0800 Subject: [PATCH 49/53] Change quotes to backticks to highlight literal values --- .../source/docs/internals/architecture.html.md | 4 ++-- website/source/docs/jobspec/index.html.md | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/website/source/docs/internals/architecture.html.md b/website/source/docs/internals/architecture.html.md index 9c0079f1b..92102405a 100644 --- a/website/source/docs/internals/architecture.html.md +++ b/website/source/docs/internals/architecture.html.md @@ -64,8 +64,8 @@ clarify what is being discussed: Regions may contain multiple datacenters. Servers are assigned to regions and manage all state for the region and make scheduling decisions within that region. Requests that are made between regions are forwarded to the appropriate servers. As an example, you may - have a "US" region with the "us-east-1" and "us-west-1" datacenters, connected to the - "EU" region with the "eu-fr-1" and "eu-uk-1" datacenters. + have a `US` region with the `us-east-1` and `us-west-1` datacenters, connected to the + `EU` region with the `eu-fr-1` and `eu-uk-1` datacenters. * **Bin Packing** - Bin Packing is the process of filling bins with items in a way that maximizes the utilization of bins. This extends to Nomad, where the clients are "bins" diff --git a/website/source/docs/jobspec/index.html.md b/website/source/docs/jobspec/index.html.md index 5a99bf636..f34680de2 100644 --- a/website/source/docs/jobspec/index.html.md +++ b/website/source/docs/jobspec/index.html.md @@ -181,7 +181,7 @@ The `task` object supports the following keys: * `driver` - Specifies the task driver that should be used to run the task. See the [driver documentation](/docs/drivers/index.html) for what - is available. Examples include "docker", "qemu", "java", and "exec". + is available. Examples include `docker`, `qemu`, `java`, and `exec`. * `constraint` - This can be provided multiple times to define additional constraints. See the constraint reference for more details. @@ -240,14 +240,14 @@ The `restart` object supports the following keys: number of restarts allowed in an `interval` before a restart delay is added. * `interval` - `interval` is only valid on non-batch jobs and is a time duration - that can be specified using the "s", "m", and "h" suffixes, such as "30s". + that can be specified using the `s`, `m`, and `h` suffixes, such as `30s`. The `interval` begins when the first task starts and ensures that only `attempts` number of restarts happens within it. If more than `attempts` number of failures happen, the restart is delayed till after the `interval`, which is then reset. * `delay` - A duration to wait before restarting a task. It is specified as a - time duration using the "s", "m", and "h" suffixes, such as "30s". + time duration using the `s`, `m`, and `h` suffixes, such as `30s`. The default `batch` restart policy is: @@ -283,7 +283,7 @@ The `constraint` object supports the following keys: This can be a literal value or another attribute. * `version` - Specifies a version constraint against the attribute. - This sets the operator to "version" and the `value` to what is + This sets the operator to `version` and the `value` to what is specified. This supports a comma seperated list of constraints, including the pessimistic operator. See the [go-version](https://github.com/hashicorp/go-version) repository @@ -343,7 +343,7 @@ Below is a table documenting common node attributes: arch - CPU architecture of the client. Examples: "amd64", "386" + CPU architecture of the client. Examples: `amd64`, `386` consul.datacenter @@ -363,11 +363,11 @@ Below is a table documenting common node attributes: kernel.name - Kernel of the client. Examples: "linux", "darwin" + Kernel of the client. Examples: `linux`, `darwin` kernel.version - Version of the client kernel. Examples: "3.19.0-25-generic", "15.0.0" + Version of the client kernel. Examples: `3.19.0-25-generic`, `15.0.0` platform.aws.ami-id @@ -379,7 +379,7 @@ Below is a table documenting common node attributes: os.name - Operating system of the client. Examples: "ubuntu", "windows", "darwin" + Operating system of the client. Examples: `ubuntu`, `windows`, `darwin` os.version From 7357979089be685cf476e2ab2ca824b91f7c4e11 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 15:16:42 -0800 Subject: [PATCH 50/53] Revert "Revert "Make drivers take arguments as a list and not as a string"" --- client/alloc_runner_test.go | 6 +-- client/client_test.go | 2 +- client/driver/args/args.go | 29 ++++---------- client/driver/args/args_test.go | 41 +++----------------- client/driver/docker.go | 13 +++---- client/driver/docker_test.go | 10 +++-- client/driver/exec.go | 16 +++----- client/driver/exec_test.go | 16 +++++--- client/driver/executor/exec_basic.go | 8 +--- client/driver/executor/exec_linux.go | 7 +--- client/driver/executor/test_harness_test.go | 4 +- client/driver/java.go | 16 ++++---- client/driver/java_test.go | 3 +- client/driver/raw_exec.go | 8 +--- client/driver/raw_exec_test.go | 16 +++++--- client/driver/rkt.go | 11 ++---- client/driver/rkt_test.go | 6 +-- client/task_runner_test.go | 6 +-- nomad/fsm.go | 11 +----- nomad/mock/mock.go | 4 +- nomad/rpc.go | 13 +------ nomad/structs/structs.go | 7 ++-- nomad/timetable_test.go | 5 ++- website/source/docs/drivers/docker.html.md | 6 +-- website/source/docs/drivers/exec.html.md | 22 ++++++----- website/source/docs/drivers/java.html.md | 20 +++++----- website/source/docs/drivers/qemu.html.md | 9 +++-- website/source/docs/drivers/raw_exec.html.md | 22 ++++++----- website/source/docs/drivers/rkt.html.md | 17 ++++---- 29 files changed, 149 insertions(+), 205 deletions(-) diff --git a/client/alloc_runner_test.go b/client/alloc_runner_test.go index 15077c76c..bc4a7aa4f 100644 --- a/client/alloc_runner_test.go +++ b/client/alloc_runner_test.go @@ -65,7 +65,7 @@ func TestAllocRunner_Destroy(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() start := time.Now() @@ -97,7 +97,7 @@ func TestAllocRunner_Update(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() defer ar.Destroy() start := time.Now() @@ -130,7 +130,7 @@ func TestAllocRunner_SaveRestoreState(t *testing.T) { // Ensure task takes some time task := ar.alloc.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} go ar.Run() defer ar.Destroy() diff --git a/client/client_test.go b/client/client_test.go index 90fecad6b..935c63dea 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -336,7 +336,7 @@ func TestClient_SaveRestoreState(t *testing.T) { alloc1.NodeID = c1.Node().ID task := alloc1.Job.TaskGroups[0].Tasks[0] task.Config["command"] = "/bin/sleep" - task.Config["args"] = "10" + task.Config["args"] = []string{"10"} state := s1.State() err := state.UpsertAllocs(100, diff --git a/client/driver/args/args.go b/client/driver/args/args.go index 51793bd8b..9e8a9980c 100644 --- a/client/driver/args/args.go +++ b/client/driver/args/args.go @@ -1,11 +1,6 @@ package args -import ( - "fmt" - "regexp" - - "github.com/mattn/go-shellwords" -) +import "regexp" var ( envRe = regexp.MustCompile(`\$({[a-zA-Z0-9_]+}|[a-zA-Z0-9_]+)`) @@ -13,27 +8,17 @@ var ( // ParseAndReplace takes the user supplied args and a map of environment // variables. It replaces any instance of an environment variable in the args -// with the actual value and does correct splitting of the arg list. -func ParseAndReplace(args string, env map[string]string) ([]string, error) { - // Set up parser. - p := shellwords.NewParser() - p.ParseEnv = false - p.ParseBacktick = false - - parsed, err := p.Parse(args) - if err != nil { - return nil, fmt.Errorf("Couldn't parse args %v: %v", args, err) - } - - replaced := make([]string, len(parsed)) - for i, arg := range parsed { +// with the actual value. +func ParseAndReplace(args []string, env map[string]string) []string { + replaced := make([]string, len(args)) + for i, arg := range args { replaced[i] = ReplaceEnv(arg, env) } - return replaced, nil + return replaced } -// replaceEnv takes an arg and replaces all occurences of environment variables. +// ReplaceEnv takes an arg and replaces all occurences of environment variables. // If the variable is found in the passed map it is replaced, otherwise the // original string is returned. func ReplaceEnv(arg string, env map[string]string) string { diff --git a/client/driver/args/args_test.go b/client/driver/args/args_test.go index 4e1d88b59..5e7cbca4a 100644 --- a/client/driver/args/args_test.go +++ b/client/driver/args/args_test.go @@ -21,12 +21,9 @@ var ( ) func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { - input := "invalid $FOO" + input := []string{"invalid", "$FOO"} exp := []string{"invalid", "$FOO"} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) @@ -34,12 +31,9 @@ func TestDriverArgs_ParseAndReplaceInvalidEnv(t *testing.T) { } func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { - input := fmt.Sprintf("nomad_ip \\\"$%v\\\"!", ipKey) + input := []string{"nomad_ip", fmt.Sprintf(`"$%v"!`, ipKey)} exp := []string{"nomad_ip", fmt.Sprintf("\"%s\"!", ipVal)} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) @@ -47,32 +41,9 @@ func TestDriverArgs_ParseAndReplaceValidEnv(t *testing.T) { } func TestDriverArgs_ParseAndReplaceChainedEnv(t *testing.T) { - input := fmt.Sprintf("-foo $%s$%s", ipKey, portKey) + input := []string{"-foo", fmt.Sprintf("$%s$%s", ipKey, portKey)} exp := []string{"-foo", fmt.Sprintf("%s%s", ipVal, portVal)} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } - - if !reflect.DeepEqual(act, exp) { - t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) - } -} - -func TestDriverArgs_ParseAndReplaceInvalidArgEscape(t *testing.T) { - input := "-c \"echo \"foo\\\" > bar.txt\"" - if _, err := ParseAndReplace(input, envVars); err == nil { - t.Fatalf("ParseAndReplace(%v, %v) should have failed", input, envVars) - } -} - -func TestDriverArgs_ParseAndReplaceValidArgEscape(t *testing.T) { - input := "-c \"echo \\\"foo\\\" > bar.txt\"" - exp := []string{"-c", "echo \"foo\" > bar.txt"} - act, err := ParseAndReplace(input, envVars) - if err != nil { - t.Fatalf("Failed to parse valid input args %v: %v", input, err) - } + act := ParseAndReplace(input, envVars) if !reflect.DeepEqual(act, exp) { t.Fatalf("ParseAndReplace(%v, %v) returned %#v; want %#v", input, envVars, act, exp) diff --git a/client/driver/docker.go b/client/driver/docker.go index 6fa1af600..4aa97e13b 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -35,7 +35,7 @@ type DockerDriverAuth struct { type DockerDriverConfig struct { ImageName string `mapstructure:"image"` // Container's Image Name Command string `mapstructure:"command"` // The Command/Entrypoint to run when the container starts up - Args string `mapstructure:"args"` // The arguments to the Command/Entrypoint + Args []string `mapstructure:"args"` // The arguments to the Command/Entrypoint NetworkMode string `mapstructure:"network_mode"` // The network mode of the container - host, net and none PortMap []map[string]int `mapstructure:"port_map"` // A map of host port labels and the ports exposed on the container Privileged bool `mapstructure:"privileged"` // Flag to run the container in priviledged mode @@ -293,21 +293,18 @@ func (d *DockerDriver) createContainer(ctx *ExecContext, task *structs.Task, dri config.ExposedPorts = exposedPorts } - parsedArgs, err := args.ParseAndReplace(driverConfig.Args, env.Map()) - if err != nil { - return c, err - } + parsedArgs := args.ParseAndReplace(driverConfig.Args, env.Map()) // If the user specified a custom command to run as their entrypoint, we'll // inject it here. if driverConfig.Command != "" { cmd := []string{driverConfig.Command} - if driverConfig.Args != "" { + if len(driverConfig.Args) != 0 { cmd = append(cmd, parsedArgs...) } d.logger.Printf("[DEBUG] driver.docker: setting container startup command to: %s", strings.Join(cmd, " ")) config.Cmd = cmd - } else if driverConfig.Args != "" { + } else if len(driverConfig.Args) != 0 { d.logger.Println("[DEBUG] driver.docker: ignoring command arguments because command is not specified") } @@ -431,7 +428,7 @@ func (d *DockerDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle if len(containers) != 1 { log.Printf("[ERR] driver.docker: failed to get id for container %s", config.Name) - return nil, fmt.Errorf("Failed to get id for container %s: %s", config.Name, err) + return nil, fmt.Errorf("Failed to get id for container %s", config.Name) } log.Printf("[INFO] driver.docker: a container with the name %s already exists; will attempt to purge and re-create", config.Name) diff --git a/client/driver/docker_test.go b/client/driver/docker_test.go index 4c9300579..a61de381c 100644 --- a/client/driver/docker_test.go +++ b/client/driver/docker_test.go @@ -137,7 +137,7 @@ func TestDockerDriver_Start_Wait(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "redis-server", - "args": "-v", + "args": []string{"-v"}, }, Resources: &structs.Resources{ MemoryMB: 256, @@ -190,7 +190,11 @@ func TestDockerDriver_Start_Wait_AllocDir(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "/bin/bash", - "args": fmt.Sprintf(`-c "sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, + string(exp), environment.AllocDir, file), + }, }, Resources: &structs.Resources{ MemoryMB: 256, @@ -243,7 +247,7 @@ func TestDockerDriver_Start_Kill_Wait(t *testing.T) { Config: map[string]interface{}{ "image": "redis", "command": "/bin/sleep", - "args": "10", + "args": []string{"10"}, }, Resources: basicResources, } diff --git a/client/driver/exec.go b/client/driver/exec.go index 1f9e5b7d9..a0a136523 100644 --- a/client/driver/exec.go +++ b/client/driver/exec.go @@ -24,10 +24,10 @@ type ExecDriver struct { fingerprint.StaticFingerprinter } type ExecDriverConfig struct { - ArtifactSource string `mapstructure:"artifact_source"` - Checksum string `mapstructure:"checksum"` - Command string `mapstructure:"command"` - Args string `mapstructure:"args"` + ArtifactSource string `mapstructure:"artifact_source"` + Checksum string `mapstructure:"checksum"` + Command string `mapstructure:"command"` + Args []string `mapstructure:"args"` } // execHandle is returned from Start/Open as a handle to the PID @@ -91,14 +91,8 @@ func (d *ExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, // Get the environment variables. envVars := TaskEnvironmentVariables(ctx, task) - // Look for arguments - var args []string - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) - } - // Setup the command - cmd := executor.Command(command, args...) + cmd := executor.Command(command, driverConfig.Args...) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } diff --git a/client/driver/exec_test.go b/client/driver/exec_test.go index 7e6554e1d..f5470074a 100644 --- a/client/driver/exec_test.go +++ b/client/driver/exec_test.go @@ -39,7 +39,7 @@ func TestExecDriver_StartOpen_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "5", + "args": []string{"5"}, }, Resources: basicResources, } @@ -73,7 +73,7 @@ func TestExecDriver_Start_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "2", + "args": []string{"2"}, }, Resources: basicResources, } @@ -161,7 +161,10 @@ func TestExecDriver_Start_Artifact_expanded(t *testing.T) { Config: map[string]interface{}{ "artifact_source": fmt.Sprintf("https://dl.dropboxusercontent.com/u/47675/jar_thing/%s", file), "command": "/bin/bash", - "args": fmt.Sprintf("-c '/bin/sleep 1 && %s'", filepath.Join("$NOMAD_TASK_DIR", file)), + "args": []string{ + "-c", + fmt.Sprintf(`/bin/sleep 1 && %s`, filepath.Join("$NOMAD_TASK_DIR", file)), + }, }, Resources: basicResources, } @@ -204,7 +207,10 @@ func TestExecDriver_Start_Wait_AllocDir(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/bash", - "args": fmt.Sprintf("-c \"sleep 1; echo -n %s > $%s/%s\"", string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), + }, }, Resources: basicResources, } @@ -250,7 +256,7 @@ func TestExecDriver_Start_Kill_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index 438ae6b92..2ef4fbedc 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -29,7 +29,6 @@ type BasicExecutor struct { allocDir string } -// TODO: Have raw_exec use this as well. func NewBasicExecutor() Executor { return &BasicExecutor{} } @@ -63,12 +62,7 @@ func (e *BasicExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - combined := strings.Join(e.cmd.Args, " ") - parsed, err := args.ParseAndReplace(combined, envVars.Map()) - if err != nil { - return err - } - e.cmd.Args = parsed + e.cmd.Args = args.ParseAndReplace(e.cmd.Args, envVars.Map()) spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index 14f4f809c..43c9270e0 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -167,12 +167,7 @@ func (e *LinuxExecutor) Start() error { } e.cmd.Path = args.ReplaceEnv(e.cmd.Path, envVars.Map()) - combined := strings.Join(e.cmd.Args, " ") - parsed, err := args.ParseAndReplace(combined, envVars.Map()) - if err != nil { - return err - } - e.cmd.Args = parsed + e.cmd.Args = args.ParseAndReplace(e.cmd.Args, envVars.Map()) spawnState := filepath.Join(e.allocDir, fmt.Sprintf("%s_%s", e.taskName, "exit_status")) e.spawn = spawn.NewSpawner(spawnState) diff --git a/client/driver/executor/test_harness_test.go b/client/driver/executor/test_harness_test.go index 10cbac371..b5413700c 100644 --- a/client/driver/executor/test_harness_test.go +++ b/client/driver/executor/test_harness_test.go @@ -112,7 +112,7 @@ func Executor_Start_Wait(t *testing.T, command buildExecCommand) { expected := "hello world" file := filepath.Join(allocdir.TaskLocal, "output.txt") absFilePath := filepath.Join(taskDir, file) - cmd := fmt.Sprintf(`"%v \"%v\" > %v"`, "/bin/sleep 1 ; echo -n", expected, file) + cmd := fmt.Sprintf(`/bin/sleep 1 ; echo -n %v > %v`, expected, file) e := command("/bin/bash", "-c", cmd) if err := e.Limit(constraint); err != nil { @@ -190,7 +190,7 @@ func Executor_Open(t *testing.T, command buildExecCommand, newExecutor func() Ex expected := "hello world" file := filepath.Join(allocdir.TaskLocal, "output.txt") absFilePath := filepath.Join(taskDir, file) - cmd := fmt.Sprintf(`"%v \"%v\" > %v"`, "/bin/sleep 1 ; echo -n", expected, file) + cmd := fmt.Sprintf(`/bin/sleep 1 ; echo -n %v > %v`, expected, file) e := command("/bin/bash", "-c", cmd) if err := e.Limit(constraint); err != nil { diff --git a/client/driver/java.go b/client/driver/java.go index eb2930a28..eb475db32 100644 --- a/client/driver/java.go +++ b/client/driver/java.go @@ -28,10 +28,10 @@ type JavaDriver struct { } type JavaDriverConfig struct { - JvmOpts string `mapstructure:"jvm_options"` - ArtifactSource string `mapstructure:"artifact_source"` - Checksum string `mapstructure:"checksum"` - Args string `mapstructure:"args"` + JvmOpts []string `mapstructure:"jvm_options"` + ArtifactSource string `mapstructure:"artifact_source"` + Checksum string `mapstructure:"checksum"` + Args []string `mapstructure:"args"` } // javaHandle is returned from Start/Open as a handle to the PID @@ -126,15 +126,15 @@ func (d *JavaDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, args := []string{} // Look for jvm options - if driverConfig.JvmOpts != "" { + if len(driverConfig.JvmOpts) != 0 { d.logger.Printf("[DEBUG] driver.java: found JVM options: %s", driverConfig.JvmOpts) - args = append(args, driverConfig.JvmOpts) + args = append(args, driverConfig.JvmOpts...) } // Build the argument list. args = append(args, "-jar", filepath.Join(allocdir.TaskLocal, jarName)) - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) + if len(driverConfig.Args) != 0 { + args = append(args, driverConfig.Args...) } // Setup the command diff --git a/client/driver/java_test.go b/client/driver/java_test.go index a0c6d3b80..af6e44d6d 100644 --- a/client/driver/java_test.go +++ b/client/driver/java_test.go @@ -51,7 +51,7 @@ func TestJavaDriver_StartOpen_Wait(t *testing.T) { Name: "demo-app", Config: map[string]interface{}{ "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/demoapp.jar", - "jvm_options": "-Xmx2048m -Xms256m", + "jvm_options": []string{"-Xmx64m", "-Xms32m"}, "checksum": "sha256:58d6e8130308d32e197c5108edd4f56ddf1417408f743097c2e662df0f0b17c8", }, Resources: basicResources, @@ -97,7 +97,6 @@ func TestJavaDriver_Start_Wait(t *testing.T) { Name: "demo-app", Config: map[string]interface{}{ "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/demoapp.jar", - "jvm_options": "-Xmx2048m -Xms256m", "checksum": "sha256:58d6e8130308d32e197c5108edd4f56ddf1417408f743097c2e662df0f0b17c8", }, Resources: basicResources, diff --git a/client/driver/raw_exec.go b/client/driver/raw_exec.go index d5202fc39..5b1d98269 100644 --- a/client/driver/raw_exec.go +++ b/client/driver/raw_exec.go @@ -93,15 +93,9 @@ func (d *RawExecDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandl // Get the environment variables. envVars := TaskEnvironmentVariables(ctx, task) - // Look for arguments - var args []string - if driverConfig.Args != "" { - args = append(args, driverConfig.Args) - } - // Setup the command cmd := executor.NewBasicExecutor() - executor.SetCommand(cmd, command, args) + executor.SetCommand(cmd, command, driverConfig.Args) if err := cmd.Limit(task.Resources); err != nil { return nil, fmt.Errorf("failed to constrain resources: %s", err) } diff --git a/client/driver/raw_exec_test.go b/client/driver/raw_exec_test.go index 1b7a0c8db..8b6e0c649 100644 --- a/client/driver/raw_exec_test.go +++ b/client/driver/raw_exec_test.go @@ -53,7 +53,7 @@ func TestRawExecDriver_StartOpen_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } @@ -151,7 +151,10 @@ func TestRawExecDriver_Start_Artifact_expanded(t *testing.T) { Config: map[string]interface{}{ "artifact_source": fmt.Sprintf("https://dl.dropboxusercontent.com/u/47675/jar_thing/%s", file), "command": "/bin/bash", - "args": fmt.Sprintf("-c '/bin/sleep 1 && %s'", filepath.Join("$NOMAD_TASK_DIR", file)), + "args": []string{ + "-c", + fmt.Sprintf(`'/bin/sleep 1 && %s'`, filepath.Join("$NOMAD_TASK_DIR", file)), + }, }, Resources: basicResources, } @@ -190,7 +193,7 @@ func TestRawExecDriver_Start_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } @@ -232,7 +235,10 @@ func TestRawExecDriver_Start_Wait_AllocDir(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/bash", - "args": fmt.Sprintf(`-c "sleep 1; echo -n %s > $%s/%s"`, string(exp), environment.AllocDir, file), + "args": []string{ + "-c", + fmt.Sprintf(`sleep 1; echo -n %s > $%s/%s`, string(exp), environment.AllocDir, file), + }, }, Resources: basicResources, } @@ -277,7 +283,7 @@ func TestRawExecDriver_Start_Kill_Wait(t *testing.T) { Name: "sleep", Config: map[string]interface{}{ "command": "/bin/sleep", - "args": "1", + "args": []string{"1"}, }, Resources: basicResources, } diff --git a/client/driver/rkt.go b/client/driver/rkt.go index d09eac1db..1fb6e9e06 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -37,8 +37,8 @@ type RktDriver struct { } type RktDriverConfig struct { - ImageName string `mapstructure:"image"` - Args string `mapstructure:"args"` + ImageName string `mapstructure:"image"` + Args []string `mapstructure:"args"` } // rktHandle is returned from Start/Open as a handle to the PID @@ -150,11 +150,8 @@ func (d *RktDriver) Start(ctx *ExecContext, task *structs.Task) (DriverHandle, e } // Add user passed arguments. - if driverConfig.Args != "" { - parsed, err := args.ParseAndReplace(driverConfig.Args, envVars.Map()) - if err != nil { - return nil, err - } + if len(driverConfig.Args) != 0 { + parsed := args.ParseAndReplace(driverConfig.Args, envVars.Map()) // Need to start arguments with "--" if len(parsed) > 0 { diff --git a/client/driver/rkt_test.go b/client/driver/rkt_test.go index a6b3dfb78..15db27b27 100644 --- a/client/driver/rkt_test.go +++ b/client/driver/rkt_test.go @@ -119,7 +119,7 @@ func TestRktDriver_Start_Wait(t *testing.T) { "trust_prefix": "coreos.com/etcd", "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } @@ -160,7 +160,7 @@ func TestRktDriver_Start_Wait_Skip_Trust(t *testing.T) { Config: map[string]interface{}{ "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } @@ -202,7 +202,7 @@ func TestRktDriver_Start_Wait_Logs(t *testing.T) { "trust_prefix": "coreos.com/etcd", "image": "coreos.com/etcd:v2.0.4", "command": "/etcd", - "args": "--version", + "args": []string{"--version"}, }, } diff --git a/client/task_runner_test.go b/client/task_runner_test.go index f8bc9e466..1ada5060b 100644 --- a/client/task_runner_test.go +++ b/client/task_runner_test.go @@ -89,7 +89,7 @@ func TestTaskRunner_Destroy(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() // Begin the tear down @@ -128,7 +128,7 @@ func TestTaskRunner_Update(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() defer tr.Destroy() defer tr.ctx.AllocDir.Destroy() @@ -153,7 +153,7 @@ func TestTaskRunner_SaveRestoreState(t *testing.T) { // Change command to ensure we run for a bit tr.task.Config["command"] = "/bin/sleep" - tr.task.Config["args"] = "10" + tr.task.Config["args"] = []string{"10"} go tr.Run() defer tr.Destroy() diff --git a/nomad/fsm.go b/nomad/fsm.go index 21b3538e4..71c40d68f 100644 --- a/nomad/fsm.go +++ b/nomad/fsm.go @@ -13,13 +13,6 @@ import ( "github.com/hashicorp/raft" ) -var ( - msgpackHandle = &codec.MsgpackHandle{ - RawToString: true, - WriteExt: true, - } -) - const ( // timeTableGranularity is the granularity of index to time tracking timeTableGranularity = 5 * time.Minute @@ -328,7 +321,7 @@ func (n *nomadFSM) Restore(old io.ReadCloser) error { defer restore.Abort() // Create a decoder - dec := codec.NewDecoder(old, msgpackHandle) + dec := codec.NewDecoder(old, structs.MsgpackHandle) // Read in the header var header snapshotHeader @@ -412,7 +405,7 @@ func (n *nomadFSM) Restore(old io.ReadCloser) error { func (s *nomadSnapshot) Persist(sink raft.SnapshotSink) error { defer metrics.MeasureSince([]string{"nomad", "fsm", "persist"}, time.Now()) // Register the nodes - encoder := codec.NewEncoder(sink, msgpackHandle) + encoder := codec.NewEncoder(sink, structs.MsgpackHandle) // Write the header header := snapshotHeader{} diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 12a8484c0..25ca60ef2 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -86,7 +86,7 @@ func Job() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": "+%s", + "args": []string{"+%s"}, }, Env: map[string]string{ "FOO": "bar", @@ -151,7 +151,7 @@ func SystemJob() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": "+%s", + "args": []string{"+%s"}, }, Resources: &structs.Resources{ CPU: 500, diff --git a/nomad/rpc.go b/nomad/rpc.go index 123c35028..e52e258f0 100644 --- a/nomad/rpc.go +++ b/nomad/rpc.go @@ -11,7 +11,6 @@ import ( "time" "github.com/armon/go-metrics" - "github.com/hashicorp/go-msgpack/codec" "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/hashicorp/nomad/nomad/state" "github.com/hashicorp/nomad/nomad/structs" @@ -53,24 +52,16 @@ const ( enqueueLimit = 30 * time.Second ) -var ( - // rpcHandle is the MsgpackHandle to be used by both Client and Server codecs. - rpcHandle = &codec.MsgpackHandle{ - // Enables proper encoding of strings within nil interfaces. - RawToString: true, - } -) - // NewClientCodec returns a new rpc.ClientCodec to be used to make RPC calls to // the Nomad Server. func NewClientCodec(conn io.ReadWriteCloser) rpc.ClientCodec { - return msgpackrpc.NewCodecFromHandle(true, true, conn, rpcHandle) + return msgpackrpc.NewCodecFromHandle(true, true, conn, structs.MsgpackHandle) } // NewServerCodec returns a new rpc.ServerCodec to be used by the Nomad Server // to handle rpcs. func NewServerCodec(conn io.ReadWriteCloser) rpc.ServerCodec { - return msgpackrpc.NewCodecFromHandle(true, true, conn, rpcHandle) + return msgpackrpc.NewCodecFromHandle(true, true, conn, structs.MsgpackHandle) } // listen is used to listen for incoming RPC connections diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index b7453b512..876bb0c39 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1704,25 +1704,26 @@ func (p *PlanResult) FullCommit(plan *Plan) (bool, int, int) { } // msgpackHandle is a shared handle for encoding/decoding of structs -var msgpackHandle = func() *codec.MsgpackHandle { +var MsgpackHandle = func() *codec.MsgpackHandle { h := &codec.MsgpackHandle{RawToString: true} // Sets the default type for decoding a map into a nil interface{}. // This is necessary in particular because we store the driver configs as a // nil interface{}. h.MapType = reflect.TypeOf(map[string]interface{}(nil)) + h.SliceType = reflect.TypeOf([]string{}) return h }() // Decode is used to decode a MsgPack encoded object func Decode(buf []byte, out interface{}) error { - return codec.NewDecoder(bytes.NewReader(buf), msgpackHandle).Decode(out) + return codec.NewDecoder(bytes.NewReader(buf), MsgpackHandle).Decode(out) } // Encode is used to encode a MsgPack object with type prefix func Encode(t MessageType, msg interface{}) ([]byte, error) { var buf bytes.Buffer buf.WriteByte(uint8(t)) - err := codec.NewEncoder(&buf, msgpackHandle).Encode(msg) + err := codec.NewEncoder(&buf, MsgpackHandle).Encode(msg) return buf.Bytes(), err } diff --git a/nomad/timetable_test.go b/nomad/timetable_test.go index 3a771d6fa..a76446a13 100644 --- a/nomad/timetable_test.go +++ b/nomad/timetable_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/hashicorp/go-msgpack/codec" + "github.com/hashicorp/nomad/nomad/structs" ) func TestTimeTable(t *testing.T) { @@ -104,14 +105,14 @@ func TestTimeTable_SerializeDeserialize(t *testing.T) { tt.Witness(50, plusHour) var buf bytes.Buffer - enc := codec.NewEncoder(&buf, msgpackHandle) + enc := codec.NewEncoder(&buf, structs.MsgpackHandle) err := tt.Serialize(enc) if err != nil { t.Fatalf("err: %v", err) } - dec := codec.NewDecoder(&buf, msgpackHandle) + dec := codec.NewDecoder(&buf, structs.MsgpackHandle) tt2 := NewTimeTable(time.Second, time.Minute) err = tt2.Deserialize(dec) diff --git a/website/source/docs/drivers/docker.html.md b/website/source/docs/drivers/docker.html.md index d1c6bafbe..a50bc1cec 100644 --- a/website/source/docs/drivers/docker.html.md +++ b/website/source/docs/drivers/docker.html.md @@ -19,13 +19,13 @@ and cleaning up after containers. The `docker` driver supports the following configuration in the job specification: -* `image` - (Required) The Docker image to run. The image may include a tag or +* `image` - The Docker image to run. The image may include a tag or custom URL. By default it will be fetched from Docker Hub. * `command` - (Optional) The command to run when starting the container. -* `args` - (Optional) Arguments to the optional `command`. If no `command` is - present, `args` are ignored. +* `args` - (Optional) A list of arguments to the optional `command`. If no + `command` is present, `args` are ignored. * `network_mode` - (Optional) The network mode to be used for the container. In order to support userspace networking plugins in Docker 1.9 this accepts any diff --git a/website/source/docs/drivers/exec.html.md b/website/source/docs/drivers/exec.html.md index f897b1ea4..3d921aa6d 100644 --- a/website/source/docs/drivers/exec.html.md +++ b/website/source/docs/drivers/exec.html.md @@ -20,15 +20,19 @@ scripts or other wrappers which provide higher level features. The `exec` driver supports the following configuration in the job spec: -* `command` - (Required) The command to execute. Must be provided. -* `artifact_source` – (Optional) Source location of an executable artifact. Must be accessible -from the Nomad client. If you specify an `artifact_source` to be executed, you -must reference it in the `command` as show in the examples below -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start -* `args` - The argument list to the command, space seperated. Optional. +* `command` - The command to execute. Must be provided. + +* `artifact_source` – (Optional) Source location of an executable artifact. Must + be accessible from the Nomad client. If you specify an `artifact_source` to be + executed, you must reference it in the `command` as show in the examples below + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start + +* `args` - (Optional) A list of arguments to the `command`. ## Client Requirements diff --git a/website/source/docs/drivers/java.html.md b/website/source/docs/drivers/java.html.md index f2bbd2b76..45baacb72 100644 --- a/website/source/docs/drivers/java.html.md +++ b/website/source/docs/drivers/java.html.md @@ -18,17 +18,19 @@ HTTP from the Nomad client. The `java` driver supports the following configuration in the job spec: -* `artifact_source` - **(Required)** The hosted location of the source Jar file. Must be accessible -from the Nomad client -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start +* `artifact_source` - The hosted location of the source Jar file. Must be + accessible from the Nomad client -* `args` - **(Optional)** The argument list for the `java` command, space separated. +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start -* `jvm_options` - **(Optional)** JVM options to be passed while invoking java. These options - are passed not validated in any way in Nomad. +* `args` - (Optional) A list of arguments to the `java` command. + +* `jvm_options` - (Optional) A list of JVM options to be passed while invoking + java. These options are passed not validated in any way in Nomad. ## Client Requirements diff --git a/website/source/docs/drivers/qemu.html.md b/website/source/docs/drivers/qemu.html.md index 84909e331..d329c7a8e 100644 --- a/website/source/docs/drivers/qemu.html.md +++ b/website/source/docs/drivers/qemu.html.md @@ -23,16 +23,19 @@ The `Qemu` driver can execute any regular `qemu` image (e.g. `qcow`, `img`, The `Qemu` driver supports the following configuration in the job spec: -* `artifact_source` - **(Required)** The hosted location of the source Qemu image. Must be accessible +* `artifact_source` - The hosted location of the source Qemu image. Must be accessible from the Nomad client, via HTTP. -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` image. The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, and the value is the computed checksum. If a checksum is supplied and does not match the downloaded artifact, the driver will fail to start + * `accelerator` - (Optional) The type of accelerator to use in the invocation. If the host machine has `Qemu` installed with KVM support, users can specify `kvm` for the `accelerator`. Default is `tcg` -* `port_map` - **(Optional)** A `map[string]int` that maps port labels to ports + +* `port_map` - (Optional) A `map[string]int` that maps port labels to ports on the guest. This forwards the host port to the guest vm. For example, `port_map { db = 6539 }` would forward the host port with label `db` to the guest vm's port 6539. diff --git a/website/source/docs/drivers/raw_exec.html.md b/website/source/docs/drivers/raw_exec.html.md index 2dc741887..c76825f25 100644 --- a/website/source/docs/drivers/raw_exec.html.md +++ b/website/source/docs/drivers/raw_exec.html.md @@ -18,15 +18,19 @@ As such, it should be used with extreme care and is disabled by default. The `raw_exec` driver supports the following configuration in the job spec: -* `command` - (Required) The command to execute. Must be provided. -* `artifact_source` – (Optional) Source location of an executable artifact. Must be accessible -from the Nomad client. If you specify an `artifact_source` to be executed, you -must reference it in the `command` as show in the examples below -* `checksum` - **(Optional)** The checksum type and value for the `artifact_source` image. -The format is `type:value`, where type is any of `md5`, `sha1`, `sha256`, or `sha512`, -and the value is the computed checksum. If a checksum is supplied and does not -match the downloaded artifact, the driver will fail to start -* `args` - The argument list to the command, space seperated. Optional. +* `command` - The command to execute. Must be provided. + +* `artifact_source` – (Optional) Source location of an executable artifact. Must + be accessible from the Nomad client. If you specify an `artifact_source` to be + executed, you must reference it in the `command` as show in the examples below + +* `checksum` - (Optional) The checksum type and value for the `artifact_source` + image. The format is `type:value`, where type is any of `md5`, `sha1`, + `sha256`, or `sha512`, and the value is the computed checksum. If a checksum + is supplied and does not match the downloaded artifact, the driver will fail + to start + +* `args` - (Optional) A list of arguments to the `command`. ## Client Requirements diff --git a/website/source/docs/drivers/rkt.html.md b/website/source/docs/drivers/rkt.html.md index ddbb76601..7ce3e1a49 100644 --- a/website/source/docs/drivers/rkt.html.md +++ b/website/source/docs/drivers/rkt.html.md @@ -20,13 +20,16 @@ being marked as experimental and should be used with care. The `rkt` driver supports the following configuration in the job spec: -* `trust_prefix` - **(Optional)** The trust prefix to be passed to rkt. Must be reachable from -the box running the nomad agent. If not specified, the image is run without -verifying the image signature. -* `image` - **(Required)** The image to run which may be specified by name, -hash, ACI address or docker registry. -* `command` - **(Optional**) A command to execute on the ACI. -* `args` - **(Optional**) A string of args to pass into the image. +* `image` - The image to run which may be specified by name, hash, ACI address + or docker registry. + +* `command` - (Optional) A command to execute on the ACI. + +* `args` - (Optional) A list of arguments to the image. + +* `trust_prefix` - (Optional) The trust prefix to be passed to rkt. Must be + reachable from the box running the nomad agent. If not specified, the image is + run without verifying the image signature. ## Task Directories From 42b664a3245412196f16550002429166221a9371 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 15:19:18 -0800 Subject: [PATCH 51/53] remove args from mock --- nomad/mock/mock.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 25ca60ef2..53f246700 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -86,7 +86,6 @@ func Job() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": []string{"+%s"}, }, Env: map[string]string{ "FOO": "bar", @@ -151,7 +150,6 @@ func SystemJob() *structs.Job { Driver: "exec", Config: map[string]interface{}{ "command": "/bin/date", - "args": []string{"+%s"}, }, Resources: &structs.Resources{ CPU: 500, From 48f4db09aa9cc6a7bd832a396d67d6205caa6cbd Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 15:26:38 -0800 Subject: [PATCH 52/53] Register gob type and remove slicetype --- command/run.go | 1 + nomad/structs/structs.go | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/command/run.go b/command/run.go index 0ec0212e8..5b674b81b 100644 --- a/command/run.go +++ b/command/run.go @@ -124,6 +124,7 @@ func (c *RunCommand) Run(args []string) int { // This function is just a hammer and probably needs to be revisited. func convertJob(in *structs.Job) (*api.Job, error) { gob.Register([]map[string]interface{}{}) + gob.Register([]interface{}{}) var apiJob *api.Job buf := new(bytes.Buffer) if err := gob.NewEncoder(buf).Encode(in); err != nil { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 876bb0c39..a601fde29 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1711,7 +1711,6 @@ var MsgpackHandle = func() *codec.MsgpackHandle { // This is necessary in particular because we store the driver configs as a // nil interface{}. h.MapType = reflect.TypeOf(map[string]interface{}(nil)) - h.SliceType = reflect.TypeOf([]string{}) return h }() From 35b6284d65957db8c36af802bd2b24f54d1da28c Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Wed, 18 Nov 2015 16:02:36 -0800 Subject: [PATCH 53/53] Update changelog to reflect backwards incompatibility for ports --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 428d401d5..b43855b93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ BACKWARDS INCOMPATIBILITIES: * core: Removed weight and hard/soft fields in constraints [GH-351] * drivers: Qemu and Java driver configurations have been updated to both use `artifact_source` as the source for external images/jars to be ran + * New reserved and dynamic port specification [GH-415] FEATURES: