From 0c499f78b30e0dc8b07ed1c655433789b587ed68 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 26 Nov 2015 14:13:19 -0800 Subject: [PATCH 1/8] Moving the args to helper --- client/driver/docker.go | 2 +- client/driver/executor/exec_basic.go | 2 +- client/driver/executor/exec_linux.go | 2 +- client/driver/rkt.go | 2 +- {client/driver => helper}/args/args.go | 0 {client/driver => helper}/args/args_test.go | 0 6 files changed, 4 insertions(+), 4 deletions(-) rename {client/driver => helper}/args/args.go (100%) rename {client/driver => helper}/args/args_test.go (100%) diff --git a/client/driver/docker.go b/client/driver/docker.go index ceaac4b4f77..8cf11dacefd 100644 --- a/client/driver/docker.go +++ b/client/driver/docker.go @@ -13,9 +13,9 @@ import ( "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/driver/args" cstructs "github.com/hashicorp/nomad/client/driver/structs" "github.com/hashicorp/nomad/client/fingerprint" + "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" "github.com/mitchellh/mapstructure" ) diff --git a/client/driver/executor/exec_basic.go b/client/driver/executor/exec_basic.go index d047733d4cc..39ce3f36bc7 100644 --- a/client/driver/executor/exec_basic.go +++ b/client/driver/executor/exec_basic.go @@ -11,9 +11,9 @@ import ( "strings" "github.com/hashicorp/nomad/client/allocdir" - "github.com/hashicorp/nomad/client/driver/args" "github.com/hashicorp/nomad/client/driver/environment" "github.com/hashicorp/nomad/client/driver/spawn" + "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" cstructs "github.com/hashicorp/nomad/client/driver/structs" diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index 43c9270e064..a5b33e06bd3 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -15,9 +15,9 @@ import ( "github.com/hashicorp/go-multierror" "github.com/hashicorp/nomad/client/allocdir" - "github.com/hashicorp/nomad/client/driver/args" "github.com/hashicorp/nomad/client/driver/environment" "github.com/hashicorp/nomad/client/driver/spawn" + "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" "github.com/opencontainers/runc/libcontainer/cgroups" diff --git a/client/driver/rkt.go b/client/driver/rkt.go index 1fb6e9e06ff..1d90ba71e45 100644 --- a/client/driver/rkt.go +++ b/client/driver/rkt.go @@ -16,9 +16,9 @@ import ( "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/config" - "github.com/hashicorp/nomad/client/driver/args" cstructs "github.com/hashicorp/nomad/client/driver/structs" "github.com/hashicorp/nomad/client/fingerprint" + "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/nomad/structs" "github.com/mitchellh/mapstructure" ) diff --git a/client/driver/args/args.go b/helper/args/args.go similarity index 100% rename from client/driver/args/args.go rename to helper/args/args.go diff --git a/client/driver/args/args_test.go b/helper/args/args_test.go similarity index 100% rename from client/driver/args/args_test.go rename to helper/args/args_test.go From 70a38fc8cf89b962de9c7eef465feb7b1c86f42e Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 26 Nov 2015 18:30:46 -0800 Subject: [PATCH 2/8] Added a method to expand service names --- api/tasks.go | 12 ++++++++++++ api/tasks_test.go | 34 ++++++++++++++++++++++++++++++++++ jobspec/parse.go | 2 -- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index c378c222d53..46ef0cf38b4 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -1,6 +1,8 @@ package api import ( + "fmt" + "github.com/hashicorp/nomad/helper/args" "time" ) @@ -42,6 +44,16 @@ type Service struct { Checks []ServiceCheck } +func (s *Service) ExpandName(job string, taskGroup string, task string) { + s.Name = args.ReplaceEnv(s.Name, map[string]string{ + "JOB": job, + "TASKGROUP": taskGroup, + "TASK": task, + "BASE": fmt.Sprintf("%s-%s-%s", job, taskGroup, task), + }, + ) +} + // TaskGroup is the unit of scheduling. type TaskGroup struct { Name string diff --git a/api/tasks_test.go b/api/tasks_test.go index 75f29996d84..0baa315ba1d 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -218,3 +218,37 @@ func TestTask_Constrain(t *testing.T) { t.Fatalf("expect: %#v, got: %#v", expect, task.Constraints) } } + +func TestService_Expand_Name(t *testing.T) { + job := "example" + taskGroup := "cache" + task := "redis" + + s := Service{ + Name: "${TASK}-db", + } + + s.ExpandName(job, taskGroup, task) + if s.Name != "redis-db" { + t.Fatalf("Expected name: %v, Actual: %v", "redis-db", s.Name) + } + + s.Name = "db" + s.ExpandName(job, taskGroup, task) + if s.Name != "db" { + t.Fatalf("Expected name: %v, Actual: %v", "redis-db", s.Name) + } + + s.Name = "${JOB}-${TASKGROUP}-${TASK}-db" + s.ExpandName(job, taskGroup, task) + if s.Name != "example-cache-redis-db" { + t.Fatalf("Expected name: %v, Actual: %v", "expample-cache-redis-db", s.Name) + } + + s.Name = "${BASE}-db" + s.ExpandName(job, taskGroup, task) + if s.Name != "example-cache-redis-db" { + t.Fatalf("Expected name: %v, Actual: %v", "expample-cache-redis-db", s.Name) + } + +} diff --git a/jobspec/parse.go b/jobspec/parse.go index 716ff9611f9..9146475a061 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -502,8 +502,6 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser if service.Name == "" { defaultServiceName = true service.Name = fmt.Sprintf("%s-%s-%s", jobName, taskGroupName, task.Name) - } else { - service.Name = fmt.Sprintf("%s-%s-%s-%s", jobName, taskGroupName, task.Name, service.Name) } // Fileter checks From 7b04700b3543f3511532d7cb141f18ccb39cd92d Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 26 Nov 2015 19:26:00 -0800 Subject: [PATCH 3/8] Making the Job expand all service names during registration --- api/tasks.go | 12 ------ api/tasks_test.go | 34 --------------- nomad/job_endpoint.go | 3 ++ nomad/job_endpoint_test.go | 4 ++ nomad/mock/mock.go | 6 +++ nomad/structs/structs.go | 35 ++++++++++++++++ nomad/structs/structs_test.go | 79 +++++++++++++++++++++++++++++++++++ 7 files changed, 127 insertions(+), 46 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 46ef0cf38b4..c378c222d53 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -1,8 +1,6 @@ package api import ( - "fmt" - "github.com/hashicorp/nomad/helper/args" "time" ) @@ -44,16 +42,6 @@ type Service struct { Checks []ServiceCheck } -func (s *Service) ExpandName(job string, taskGroup string, task string) { - s.Name = args.ReplaceEnv(s.Name, map[string]string{ - "JOB": job, - "TASKGROUP": taskGroup, - "TASK": task, - "BASE": fmt.Sprintf("%s-%s-%s", job, taskGroup, task), - }, - ) -} - // TaskGroup is the unit of scheduling. type TaskGroup struct { Name string diff --git a/api/tasks_test.go b/api/tasks_test.go index 0baa315ba1d..75f29996d84 100644 --- a/api/tasks_test.go +++ b/api/tasks_test.go @@ -218,37 +218,3 @@ func TestTask_Constrain(t *testing.T) { t.Fatalf("expect: %#v, got: %#v", expect, task.Constraints) } } - -func TestService_Expand_Name(t *testing.T) { - job := "example" - taskGroup := "cache" - task := "redis" - - s := Service{ - Name: "${TASK}-db", - } - - s.ExpandName(job, taskGroup, task) - if s.Name != "redis-db" { - t.Fatalf("Expected name: %v, Actual: %v", "redis-db", s.Name) - } - - s.Name = "db" - s.ExpandName(job, taskGroup, task) - if s.Name != "db" { - t.Fatalf("Expected name: %v, Actual: %v", "redis-db", s.Name) - } - - s.Name = "${JOB}-${TASKGROUP}-${TASK}-db" - s.ExpandName(job, taskGroup, task) - if s.Name != "example-cache-redis-db" { - t.Fatalf("Expected name: %v, Actual: %v", "expample-cache-redis-db", s.Name) - } - - s.Name = "${BASE}-db" - s.ExpandName(job, taskGroup, task) - if s.Name != "example-cache-redis-db" { - t.Fatalf("Expected name: %v, Actual: %v", "expample-cache-redis-db", s.Name) - } - -} diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index e961428e4ef..384398fd73e 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -28,6 +28,9 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis if err := args.Job.Validate(); err != nil { return err } + + args.Job.ExpandAllServiceNames() + if args.Job.Type == structs.JobTypeCore { return fmt.Errorf("job type cannot be core") } diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index c12e5b4630f..e38ae5a6a2d 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -47,6 +47,10 @@ func TestJobEndpoint_Register(t *testing.T) { if out.CreateIndex != resp.JobModifyIndex { t.Fatalf("index mis-match") } + serviceName := out.TaskGroups[0].Tasks[0].Services[0].Name + if serviceName != "web-frontend" { + t.Fatalf("Expected Service Name: %s, Actual: %s", serviceName) + } // Lookup the evaluation eval, err := state.EvalByID(resp.EvalID) diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go index 53f246700c0..0d2949d8045 100644 --- a/nomad/mock/mock.go +++ b/nomad/mock/mock.go @@ -90,6 +90,12 @@ func Job() *structs.Job { Env: map[string]string{ "FOO": "bar", }, + Services: []*structs.Service{ + { + Name: "${TASK}-frontend", + PortLabel: "http", + }, + }, Resources: &structs.Resources{ CPU: 500, MemoryMB: 256, diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 8aaf0e7efbe..44a55175c8d 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/go-msgpack/codec" "github.com/hashicorp/go-multierror" "github.com/hashicorp/go-version" + "github.com/hashicorp/nomad/helper/args" ) var ( @@ -774,6 +775,14 @@ type Job struct { ModifyIndex uint64 } +// ExpandAllServiceNames traverses all Task Groups and makes them +// interpolate Job, Task group and Task names in all Service names +func (j *Job) ExpandAllServiceNames() { + for _, tg := range j.TaskGroups { + tg.ExpandAllServiceNames(j.Name) + } +} + // Validate is used to sanity check a job input func (j *Job) Validate() error { var mErr multierror.Error @@ -942,6 +951,14 @@ type TaskGroup struct { Meta map[string]string } +// ExpandAllServiceNames traverses over all Tasks and makes them to interpolate +// values of Job, Task Group and Task names in all Service Names +func (tg *TaskGroup) ExpandAllServiceNames(job string) { + for _, task := range tg.Tasks { + task.ExpandAllServiceNames(job, tg.Name) + } +} + // Validate is used to sanity check a task group func (tg *TaskGroup) Validate() error { var mErr multierror.Error @@ -1025,6 +1042,16 @@ type ServiceCheck struct { Timeout time.Duration // Timeout of the response from the check before consul fails the check } +func (s *Service) ExpandName(job string, taskGroup string, task string) { + s.Name = args.ReplaceEnv(s.Name, map[string]string{ + "JOB": job, + "TASKGROUP": taskGroup, + "TASK": task, + "BASE": fmt.Sprintf("%s-%s-%s", job, taskGroup, task), + }, + ) +} + func (sc *ServiceCheck) Validate() error { t := strings.ToLower(sc.Type) if t != ServiceCheckTCP && t != ServiceCheckHTTP { @@ -1109,6 +1136,14 @@ type Task struct { Meta map[string]string } +// ExpandAllServiceNames interpolates values of Job, Task Group +// and Tasks in all the service Names of a Task +func (t *Task) ExpandAllServiceNames(job string, taskGroup string) { + for _, service := range t.Services { + service.ExpandName(job, taskGroup, t.Name) + } +} + func (t *Task) GoString() string { return fmt.Sprintf("*%#v", *t) } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index a7df0d52397..7d1c6a1f0d8 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -405,3 +405,82 @@ func TestDistinctCheckId(t *testing.T) { } } + +func TestService_Expand_Name(t *testing.T) { + job := "example" + taskGroup := "cache" + task := "redis" + + s := Service{ + Name: "${TASK}-db", + } + + s.ExpandName(job, taskGroup, task) + if s.Name != "redis-db" { + t.Fatalf("Expected name: %v, Actual: %v", "redis-db", s.Name) + } + + s.Name = "db" + s.ExpandName(job, taskGroup, task) + if s.Name != "db" { + t.Fatalf("Expected name: %v, Actual: %v", "redis-db", s.Name) + } + + s.Name = "${JOB}-${TASKGROUP}-${TASK}-db" + s.ExpandName(job, taskGroup, task) + if s.Name != "example-cache-redis-db" { + t.Fatalf("Expected name: %v, Actual: %v", "expample-cache-redis-db", s.Name) + } + + s.Name = "${BASE}-db" + s.ExpandName(job, taskGroup, task) + if s.Name != "example-cache-redis-db" { + t.Fatalf("Expected name: %v, Actual: %v", "expample-cache-redis-db", s.Name) + } + +} + +func TestJob_ExpandServiceNames(t *testing.T) { + j := &Job{ + Name: "my-job", + TaskGroups: []*TaskGroup{ + &TaskGroup{ + Name: "web", + Tasks: []*Task{ + { + Name: "frontend", + Services: []*Service{ + { + Name: "${BASE}-default", + }, + { + Name: "jmx", + }, + }, + }, + }, + }, + &TaskGroup{ + Name: "admin", + Tasks: []*Task{ + { + Name: "admin-web", + }, + }, + }, + }, + } + + j.ExpandAllServiceNames() + + service1Name := j.TaskGroups[0].Tasks[0].Services[0].Name + if service1Name != "my-job-web-frontend-default" { + t.Fatalf("Expected Service Name: %s, Actual: %s", "my-job-web-frontend-default", service1Name) + } + + service2Name := j.TaskGroups[0].Tasks[0].Services[1].Name + if service2Name != "jmx" { + t.Fatalf("Expected Service Name: %s, Actual: %s", "jmx", service2Name) + } + +} From 4650be73559945597350092da535346d1acd2cef Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 26 Nov 2015 19:29:11 -0800 Subject: [PATCH 4/8] Added docs to the ExpandName method --- nomad/structs/structs.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 44a55175c8d..1329c0af027 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1042,6 +1042,8 @@ type ServiceCheck struct { Timeout time.Duration // Timeout of the response from the check before consul fails the check } +// ExpandName interpolates values of Job, Task Group and Task in the Service +// Name func (s *Service) ExpandName(job string, taskGroup string, task string) { s.Name = args.ReplaceEnv(s.Name, map[string]string{ "JOB": job, From 22d82b9ee8df36ea8b6938447fe0f2726c3305aa Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 26 Nov 2015 19:37:52 -0800 Subject: [PATCH 5/8] Added docs and the changelog entry --- CHANGELOG.md | 1 + website/source/docs/jobspec/servicediscovery.html.md | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f173ff04d9..24eae5286a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ IMPROVEMENTS: * driver/docker: Added TLS client options to the config file [GH-480] * core/api: Can list all known regions in the cluster [GH-495] * client/discovery: Added more consul client api configuration options [GH-503] + * jobspec: More flexibility in naming Services [GH-509] BUG FIXES: diff --git a/website/source/docs/jobspec/servicediscovery.html.md b/website/source/docs/jobspec/servicediscovery.html.md index 4bb6062e8f6..f0e146449e1 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -79,8 +79,10 @@ group "database" { 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. + services have to be explicitly named. Users can add the following to the + service names: ```${JOB}```, ```${TASKGROUP}```, ```${TASK}```, ```${BASE}```. + Nomad will replace them with the appropriate value of the Job, Task Group and + Task Group name while registering the Job. ```${BASE}``` expands to ${JOB}-${TASKGROUP}-${TASK} * `tags`: A list of tags associated with this Service. From 7bc03351b9ae543681e0249856a00ba8cf0dbdd9 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 26 Nov 2015 19:43:02 -0800 Subject: [PATCH 6/8] Fixing the GetJob test --- nomad/job_endpoint_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nomad/job_endpoint_test.go b/nomad/job_endpoint_test.go index e38ae5a6a2d..1ad8a8264d4 100644 --- a/nomad/job_endpoint_test.go +++ b/nomad/job_endpoint_test.go @@ -351,7 +351,11 @@ func TestJobEndpoint_GetJob(t *testing.T) { t.Fatalf("Bad index: %d %d", resp2.Index, resp.Index) } - if !reflect.DeepEqual(job, resp2.Job) { + // Make a copy of the origin job and change the service name so that we can + // do a deep equal with the response from the GET JOB Api + j := job + j.TaskGroups[0].Tasks[0].Services[0].Name = "web-frontend" + if !reflect.DeepEqual(j, resp2.Job) { t.Fatalf("bad: %#v %#v", job, resp2.Job) } From e397b9b0ca32b37448c131bc9f8720371c0c656d Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 26 Nov 2015 19:45:16 -0800 Subject: [PATCH 7/8] Fixing the docs --- 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 f0e146449e1..c4dc4475793 100644 --- a/website/source/docs/jobspec/servicediscovery.html.md +++ b/website/source/docs/jobspec/servicediscovery.html.md @@ -82,7 +82,7 @@ group "database" { services have to be explicitly named. Users can add the following to the service names: ```${JOB}```, ```${TASKGROUP}```, ```${TASK}```, ```${BASE}```. Nomad will replace them with the appropriate value of the Job, Task Group and - Task Group name while registering the Job. ```${BASE}``` expands to ${JOB}-${TASKGROUP}-${TASK} + Task names while registering the Job. ```${BASE}``` expands to ${JOB}-${TASKGROUP}-${TASK} * `tags`: A list of tags associated with this Service. From fe5703450930827cedbb23105effe8c54fc1256d Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Thu, 26 Nov 2015 23:27:33 -0800 Subject: [PATCH 8/8] Updated the example nomad job spec to use task group name in the service name --- command/init.go | 2 +- nomad/job_endpoint.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/command/init.go b/command/init.go index 92554ce451d..39cfbc4cd92 100644 --- a/command/init.go +++ b/command/init.go @@ -129,7 +129,7 @@ job "example" { } service { - # name = redis + name = "${TASKGROUP}-redis" tags = ["global", "cache"] port = "db" check { diff --git a/nomad/job_endpoint.go b/nomad/job_endpoint.go index 384398fd73e..aef44cc7427 100644 --- a/nomad/job_endpoint.go +++ b/nomad/job_endpoint.go @@ -29,6 +29,7 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis return err } + // Expand the service names args.Job.ExpandAllServiceNames() if args.Job.Type == structs.JobTypeCore {