Merge pull request #509 from hashicorp/f-service-name

Implemented interpolation of Job, Task group and Task Names in Service Name

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:
 

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"
 )

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"

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"

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"
 )

command/init.go

@@ -129,7 +129,7 @@ job "example" {
 			}
 
 			service {
-				# name = redis
+				name = "${TASKGROUP}-redis"
 				tags = ["global", "cache"]
 				port = "db"
 				check {

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

nomad/job_endpoint.go

@@ -28,6 +28,10 @@ func (j *Job) Register(args *structs.JobRegisterRequest, reply *structs.JobRegis
 	if err := args.Job.Validate(); err != nil {
 		return err
 	}
+
+	// Expand the service names
+	args.Job.ExpandAllServiceNames()
+
 	if args.Job.Type == structs.JobTypeCore {
 		return fmt.Errorf("job type cannot be core")
 	}

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)
@@ -347,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)
 	}
 

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,

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,18 @@ 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,
+		"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 {
@@ -1110,6 +1139,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)
 }

nomad/structs/structs_test.go

@@ -409,3 +409,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)
+	}
+
+}

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 names while registering the Job. ```${BASE}``` expands to ${JOB}-${TASKGROUP}-${TASK}
 
 * `tags`: A list of tags associated with this Service.