From c9f10f75c43437f807de2ec0dabaecfb2d1127e9 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Mon, 16 Nov 2015 22:43:22 -0500 Subject: [PATCH 01/17] Added the example for service block --- command/init.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/command/init.go b/command/init.go index c911c95cf..668b33b26 100644 --- a/command/init.go +++ b/command/init.go @@ -128,6 +128,19 @@ job "example" { } } + service { + # name = redis + tags = ["global", "cache"] + port = "db" + check { + id = "id-alive-check" + name = "alive" + type = "tcp" + interval = "10s" + timeout = "2s" + } + } + # We must specify the resources required for # this task to ensure it runs on a machine with # enough capacity. From 01ef4469d992c883f99751fa1d6dccac4986a621 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 01:37:09 -0500 Subject: [PATCH 02/17] Added the parsling logic for service blocks --- command/init.go | 5 ++-- jobspec/parse.go | 64 ++++++++++++++++++++++++++++++++++++++++ nomad/structs/structs.go | 18 +++++++++++ 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/command/init.go b/command/init.go index 668b33b26..6bff4990e 100644 --- a/command/init.go +++ b/command/init.go @@ -128,12 +128,11 @@ job "example" { } } - service { + service "id-redis-check" { # name = redis tags = ["global", "cache"] port = "db" - check { - id = "id-alive-check" + check "id-alive-check" { name = "alive" type = "tcp" interval = "10s" diff --git a/jobspec/parse.go b/jobspec/parse.go index 24772364f..de7d9314c 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -378,6 +378,7 @@ func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { delete(m, "config") delete(m, "env") delete(m, "constraint") + delete(m, "service") delete(m, "meta") delete(m, "resources") @@ -401,6 +402,69 @@ func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { } } + if o := listVal.Filter("service"); len(o.Items) > 0 { + t.Services = make([]structs.Service, len(o.Items)) + for idx, o := range o.Items { + var service structs.Service + label := o.Keys[0].Token.Value().(string) + service.Id = label + + var m map[string]interface{} + if err := hcl.DecodeObject(&m, o.Val); err != nil { + return err + } + + delete(m, "check") + + if err := mapstructure.WeakDecode(m, &service); err != nil { + return err + } + + if service.Name == "" { + service.Name = service.Id + } + + // Fileter checks + var checkList *ast.ObjectList + if ot, ok := o.Val.(*ast.ObjectType); ok { + checkList = ot.List + } else { + return fmt.Errorf("service '%s': should be an object", label) + } + + if co := checkList.Filter("check"); len(co.Items) > 0 { + service.Checks = make([]structs.ServiceCheck, len(co.Items)) + for idx, co := range co.Items { + var check structs.ServiceCheck + label := o.Keys[0].Token.Value().(string) + var cm map[string]interface{} + if err := hcl.DecodeObject(&cm, co.Val); err != nil { + return err + } + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.StringToTimeDurationHookFunc(), + WeaklyTypedInput: true, + Result: &check, + }) + if err != nil { + return err + } + if err := dec.Decode(cm); err != nil { + return err + } + + check.Id = label + if check.Name == "" { + check.Name = label + } + service.Checks[idx] = check + } + } + + t.Services[idx] = service + } + } + // If we have config, then parse that if o := listVal.Filter("config"); len(o.Items) > 0 { for _, o := range o.Elem().Items { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 746b9b6c8..f021525fb 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -906,6 +906,22 @@ func NewRestartPolicy(jobType string) *RestartPolicy { return nil } +type ServiceCheck struct { + Id string + Name string + Type string + Interval time.Duration + Timeout time.Duration +} + +type Service struct { + Id string + Name string + Tags []string + PortLabel string `mapstructure:"port"` + Checks []ServiceCheck +} + // TaskGroup is an atomic unit of placement. Each task group belongs to // a job and may contain any number of tasks. A task group support running // in many replicas using the same configuration.. @@ -1009,6 +1025,8 @@ type Task struct { // Map of environment variables to be used by the driver Env map[string]string + Services []Service + // Constraints can be specified at a task level and apply only to // the particular task. Constraints []*Constraint From fce0031c9a6b346d91d1363d5bf719b7e3ed0544 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 01:40:39 -0500 Subject: [PATCH 03/17] Added comments --- nomad/structs/structs.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index f021525fb..ef6793770 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -906,6 +906,8 @@ func NewRestartPolicy(jobType string) *RestartPolicy { return nil } +// The ServiceCheck data model represents the consul health check that +// Nomad registers for a Task type ServiceCheck struct { Id string Name string @@ -914,6 +916,7 @@ type ServiceCheck struct { Timeout time.Duration } +// The Service model represents a Consul service defintion type Service struct { Id string Name string From 5a0777fccbd9b28a5fdf5d388d41caf6bc1da354 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 01:51:08 -0500 Subject: [PATCH 04/17] Added tests for service block parsing --- jobspec/parse.go | 2 +- jobspec/parse_test.go | 17 +++++++++++++++++ jobspec/test-fixtures/basic.hcl | 11 +++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/jobspec/parse.go b/jobspec/parse.go index de7d9314c..e1932cfaa 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -436,7 +436,7 @@ func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { service.Checks = make([]structs.ServiceCheck, len(co.Items)) for idx, co := range co.Items { var check structs.ServiceCheck - label := o.Keys[0].Token.Value().(string) + label := co.Keys[0].Token.Value().(string) var cm map[string]interface{} if err := hcl.DecodeObject(&cm, co.Val); err != nil { return err diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 125127de5..1c993caa2 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -94,6 +94,23 @@ func TestParse(t *testing.T) { Config: map[string]interface{}{ "image": "hashicorp/binstore", }, + Services: []structs.Service{ + { + Id: "service-id", + Name: "service-name", + Tags: []string{"foo", "bar"}, + PortLabel: "http", + Checks: []structs.ServiceCheck{ + { + Id: "check-id", + Name: "check-name", + Type: "tcp", + Interval: 10 * time.Second, + Timeout: 2 * time.Second, + }, + }, + }, + }, Env: map[string]string{ "HELLO": "world", "LOREM": "ipsum", diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index 236f4829a..609c8f2f5 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -45,6 +45,17 @@ job "binstore-storagelocker" { HELLO = "world" LOREM = "ipsum" } + service "service-id" { + name = "service-name" + tags = ["foo", "bar"] + port = "http" + check "check-id" { + name = "check-name" + type = "tcp" + interval = "10s" + timeout = "2s" + } + } resources { cpu = 500 memory = 128 From 53e9d21ac061210e94d141630f2dbefbe204bbf1 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 02:20:35 -0500 Subject: [PATCH 05/17] Added the service definitions in api structs too --- api/tasks.go | 20 ++++++++++++++++++++ nomad/structs/structs.go | 39 ++++++++++++++++++++------------------- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index e5ae46b5c..27b712f3d 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -20,6 +20,25 @@ func NewRestartPolicy() *RestartPolicy { } } +// The ServiceCheck data model represents the consul health check that +// Nomad registers for a Task +type ServiceCheck struct { + Id string + Name string + Type string + Interval time.Duration + Timeout time.Duration +} + +// The Service model represents a Consul service defintion +type Service struct { + Id string + Name string + Tags []string + PortLabel string `mapstructure:"port"` + Checks []ServiceCheck +} + // TaskGroup is the unit of scheduling. type TaskGroup struct { Name string @@ -68,6 +87,7 @@ type Task struct { Config map[string]interface{} Constraints []*Constraint Env map[string]string + Services []Service Resources *Resources Meta map[string]string } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index ef6793770..b720b3c4e 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -906,25 +906,6 @@ func NewRestartPolicy(jobType string) *RestartPolicy { return nil } -// The ServiceCheck data model represents the consul health check that -// Nomad registers for a Task -type ServiceCheck struct { - Id string - Name string - Type string - Interval time.Duration - Timeout time.Duration -} - -// The Service model represents a Consul service defintion -type Service struct { - Id string - Name string - Tags []string - PortLabel string `mapstructure:"port"` - Checks []ServiceCheck -} - // TaskGroup is an atomic unit of placement. Each task group belongs to // a job and may contain any number of tasks. A task group support running // in many replicas using the same configuration.. @@ -1014,6 +995,25 @@ func (tg *TaskGroup) GoString() string { return fmt.Sprintf("*%#v", *tg) } +// The ServiceCheck data model represents the consul health check that +// Nomad registers for a Task +type ServiceCheck struct { + Id string + Name string + Type string + Interval time.Duration + Timeout time.Duration +} + +// The Service model represents a Consul service defintion +type Service struct { + Id string + Name string + Tags []string + PortLabel string `mapstructure:"port"` + Checks []ServiceCheck +} + // Task is a single process typically that is executed as part of a task group. type Task struct { // Name of the task @@ -1028,6 +1028,7 @@ type Task struct { // Map of environment variables to be used by the driver Env map[string]string + // List of service definitions exposed by the Task Services []Service // Constraints can be specified at a task level and apply only to From f0ab1fcfaee966a774f03ad3cc2bb101f4cbc1a0 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 12:16:58 -0800 Subject: [PATCH 06/17] Added the fields for http and script checks --- nomad/structs/structs.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index b720b3c4e..8fe568d0d 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1001,6 +1001,8 @@ type ServiceCheck struct { Id string Name string Type string + Script string + Http string Interval time.Duration Timeout time.Duration } From acf673f111cd6f0e69f158a4cec6138cf0b9f850 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 12:38:34 -0800 Subject: [PATCH 07/17] Exctracted the logic of parsing services in a method --- jobspec/parse.go | 126 +++++++++++++++++++++++++---------------------- 1 file changed, 67 insertions(+), 59 deletions(-) diff --git a/jobspec/parse.go b/jobspec/parse.go index e1932cfaa..915a7d94b 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -403,65 +403,8 @@ func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { } if o := listVal.Filter("service"); len(o.Items) > 0 { - t.Services = make([]structs.Service, len(o.Items)) - for idx, o := range o.Items { - var service structs.Service - label := o.Keys[0].Token.Value().(string) - service.Id = label - - var m map[string]interface{} - if err := hcl.DecodeObject(&m, o.Val); err != nil { - return err - } - - delete(m, "check") - - if err := mapstructure.WeakDecode(m, &service); err != nil { - return err - } - - if service.Name == "" { - service.Name = service.Id - } - - // Fileter checks - var checkList *ast.ObjectList - if ot, ok := o.Val.(*ast.ObjectType); ok { - checkList = ot.List - } else { - return fmt.Errorf("service '%s': should be an object", label) - } - - if co := checkList.Filter("check"); len(co.Items) > 0 { - service.Checks = make([]structs.ServiceCheck, len(co.Items)) - for idx, co := range co.Items { - var check structs.ServiceCheck - label := co.Keys[0].Token.Value().(string) - var cm map[string]interface{} - if err := hcl.DecodeObject(&cm, co.Val); err != nil { - return err - } - dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - DecodeHook: mapstructure.StringToTimeDurationHookFunc(), - WeaklyTypedInput: true, - Result: &check, - }) - if err != nil { - return err - } - if err := dec.Decode(cm); err != nil { - return err - } - - check.Id = label - if check.Name == "" { - check.Name = label - } - service.Checks[idx] = check - } - } - - t.Services[idx] = service + if err := parseServices(&t, o); err != nil { + return err } } @@ -516,6 +459,71 @@ func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { return nil } +func parseServices(task *structs.Task, serviceObjs *ast.ObjectList) error { + task.Services = make([]structs.Service, len(serviceObjs.Items)) + for idx, o := range serviceObjs.Items { + var service structs.Service + label := o.Keys[0].Token.Value().(string) + service.Id = label + + var m map[string]interface{} + if err := hcl.DecodeObject(&m, o.Val); err != nil { + return err + } + + delete(m, "check") + + if err := mapstructure.WeakDecode(m, &service); err != nil { + return err + } + + if service.Name == "" { + service.Name = service.Id + } + + // Fileter checks + var checkList *ast.ObjectList + if ot, ok := o.Val.(*ast.ObjectType); ok { + checkList = ot.List + } else { + return fmt.Errorf("service '%s': should be an object", label) + } + + if co := checkList.Filter("check"); len(co.Items) > 0 { + service.Checks = make([]structs.ServiceCheck, len(co.Items)) + for idx, co := range co.Items { + var check structs.ServiceCheck + label := co.Keys[0].Token.Value().(string) + var cm map[string]interface{} + if err := hcl.DecodeObject(&cm, co.Val); err != nil { + return err + } + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.StringToTimeDurationHookFunc(), + WeaklyTypedInput: true, + Result: &check, + }) + if err != nil { + return err + } + if err := dec.Decode(cm); err != nil { + return err + } + + check.Id = label + if check.Name == "" { + check.Name = label + } + service.Checks[idx] = check + } + } + + task.Services[idx] = service + } + + return nil +} + func parseResources(result *structs.Resources, list *ast.ObjectList) error { list = list.Elem() if len(list.Items) == 0 { From 50086cc60ee2e202cb95dc5428e56536451ec9fd Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 13:36:59 -0800 Subject: [PATCH 08/17] Added validation to the checks --- nomad/structs/structs.go | 31 +++++++++++++++++++++++++++++++ nomad/structs/structs_test.go | 20 ++++++++++++++++---- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 8fe568d0d..6e851b3a3 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -995,6 +995,13 @@ func (tg *TaskGroup) GoString() string { return fmt.Sprintf("*%#v", *tg) } +const ( + ServiceCheckHTTP = "http" + ServiceCheckTCP = "tcp" + ServiceCheckDocker = "docker" + ServiceCheckScript = "script" +) + // The ServiceCheck data model represents the consul health check that // Nomad registers for a Task type ServiceCheck struct { @@ -1007,6 +1014,14 @@ type ServiceCheck struct { Timeout time.Duration } +func (sc *ServiceCheck) Validate() error { + t := strings.ToLower(sc.Type) + if t != ServiceCheckTCP && t != ServiceCheckHTTP && t != ServiceCheckDocker && t != ServiceCheckScript { + return fmt.Errorf("Check with name %v has invalid check type: %s ", sc.Name, sc.Type) + } + return nil +} + // The Service model represents a Consul service defintion type Service struct { Id string @@ -1016,6 +1031,16 @@ type Service struct { Checks []ServiceCheck } +func (s *Service) Validate() error { + var mErr multierror.Error + for _, c := range s.Checks { + if err := c.Validate(); err != nil { + mErr.Errors = append(mErr.Errors, err) + } + } + return mErr.ErrorOrNil() +} + // Task is a single process typically that is executed as part of a task group. type Task struct { // Name of the task @@ -1156,6 +1181,12 @@ func (t *Task) Validate() error { mErr.Errors = append(mErr.Errors, outer) } } + + for _, service := range t.Services { + if err := service.Validate(); err != nil { + mErr.Errors = append(mErr.Errors, err) + } + } return mErr.ErrorOrNil() } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 8221c40fd..84af2a198 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -357,9 +357,21 @@ func TestEncodeDecode(t *testing.T) { } } -func TestBatchRestartPolicyValidate(t *testing.T) { - rp := RestartPolicy{Attempts: 10, Delay: 25 * time.Second} - if err := rp.Validate(); err != nil { - t.Fatalf("err: %v", err) +func TestInvalidServiceCheck(t *testing.T) { + s := Service{ + Id: "service-id", + Name: "service-name", + PortLabel: "bar", + Checks: []ServiceCheck{ + { + + Id: "check-id", + Name: "check-name", + Type: "lol", + }, + }, + } + if err := s.Validate(); err == nil { + t.Fatalf("Service should be invalid") } } From fb3d2be6b6a3ebd3e21c24670fa96983d08341c7 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 13:38:41 -0800 Subject: [PATCH 09/17] Added the protocol field for check --- nomad/structs/structs.go | 1 + 1 file changed, 1 insertion(+) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 6e851b3a3..d2a2660d1 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1010,6 +1010,7 @@ type ServiceCheck struct { Type string Script string Http string + Protocol string Interval time.Duration Timeout time.Duration } From acc8ca73f7eab2f58ed356e3c4bacd54f2c2e9f0 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 13:42:24 -0800 Subject: [PATCH 10/17] Added some docs for the service and servicecheck type --- nomad/structs/structs.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index d2a2660d1..f7dde55b1 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1005,14 +1005,14 @@ const ( // The ServiceCheck data model represents the consul health check that // Nomad registers for a Task type ServiceCheck struct { - Id string - Name string - Type string - Script string - Http string - Protocol string - Interval time.Duration - Timeout time.Duration + Id string // If of the check + 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 + Protocol string // Protocol to use if check is http + Interval time.Duration // Interval of the check + Timeout time.Duration // Timeout of the response from the check before consul fails the check } func (sc *ServiceCheck) Validate() error { @@ -1025,11 +1025,11 @@ func (sc *ServiceCheck) Validate() error { // The Service model represents a Consul service defintion type Service struct { - Id string - Name string - Tags []string - PortLabel string `mapstructure:"port"` - Checks []ServiceCheck + Id string // Id of the service + Name string // Name of the service, defaults to id + Tags []string // List of tags for the service + PortLabel string `mapstructure:"port"` // port for the service + Checks []ServiceCheck // List of checks associated with the service } func (s *Service) Validate() error { From 3a250a179b803dfa45e95ec4bbbc2673784b9714 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 14:21:14 -0800 Subject: [PATCH 11/17] Exctacted a method for parsing checks --- api/tasks.go | 3 +++ jobspec/parse.go | 58 +++++++++++++++++++++++----------------- nomad/structs/structs.go | 2 +- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 27b712f3d..2990b5433 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -26,6 +26,9 @@ type ServiceCheck struct { Id string Name string Type string + Script string + Http string + Protocol string Interval time.Duration Timeout time.Duration } diff --git a/jobspec/parse.go b/jobspec/parse.go index 915a7d94b..725006be8 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -490,31 +490,8 @@ func parseServices(task *structs.Task, serviceObjs *ast.ObjectList) error { } if co := checkList.Filter("check"); len(co.Items) > 0 { - service.Checks = make([]structs.ServiceCheck, len(co.Items)) - for idx, co := range co.Items { - var check structs.ServiceCheck - label := co.Keys[0].Token.Value().(string) - var cm map[string]interface{} - if err := hcl.DecodeObject(&cm, co.Val); err != nil { - return err - } - dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ - DecodeHook: mapstructure.StringToTimeDurationHookFunc(), - WeaklyTypedInput: true, - Result: &check, - }) - if err != nil { - return err - } - if err := dec.Decode(cm); err != nil { - return err - } - - check.Id = label - if check.Name == "" { - check.Name = label - } - service.Checks[idx] = check + if err := parseChecks(&service, co); err != nil { + return err } } @@ -524,6 +501,37 @@ func parseServices(task *structs.Task, serviceObjs *ast.ObjectList) error { return nil } +func parseChecks(service *structs.Service, checkObjs *ast.ObjectList) error { + service.Checks = make([]structs.ServiceCheck, len(checkObjs.Items)) + for idx, co := range checkObjs.Items { + var check structs.ServiceCheck + label := co.Keys[0].Token.Value().(string) + var cm map[string]interface{} + if err := hcl.DecodeObject(&cm, co.Val); err != nil { + return err + } + dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: mapstructure.StringToTimeDurationHookFunc(), + WeaklyTypedInput: true, + Result: &check, + }) + if err != nil { + return err + } + if err := dec.Decode(cm); err != nil { + return err + } + + check.Id = label + if check.Name == "" { + check.Name = label + } + service.Checks[idx] = check + } + + return nil +} + func parseResources(result *structs.Resources, list *ast.ObjectList) error { list = list.Elem() if len(list.Items) == 0 { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index f7dde55b1..c76c3fe23 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1025,7 +1025,7 @@ func (sc *ServiceCheck) Validate() error { // The Service model represents a Consul service defintion type Service struct { - Id string // Id of the service + Id string // Id of the service, this needs to be unique on a local machine Name string // Name of the service, defaults to id Tags []string // List of tags for the service PortLabel string `mapstructure:"port"` // port for the service From 7ba5117a866dd9b577ea341a4c4a7319a6adacd1 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 14:25:23 -0800 Subject: [PATCH 12/17] Adding some sanity checks to checks --- nomad/structs/structs.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index c76c3fe23..794c91b39 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1010,13 +1010,20 @@ type ServiceCheck struct { 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 - Protocol string // Protocol to use if check is http + 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 } func (sc *ServiceCheck) Validate() error { t := strings.ToLower(sc.Type) + if sc.Type == ServiceCheckHTTP && sc.Http == "" { + 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 && t != ServiceCheckDocker && t != ServiceCheckScript { return fmt.Errorf("Check with name %v has invalid check type: %s ", sc.Name, sc.Type) } From 2ce78767a85dbaa9dfd3ac5a97a1e9f6b782ed8e Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 16:05:03 -0800 Subject: [PATCH 13/17] Autogenerating names of service --- command/init.go | 4 ++-- jobspec/parse.go | 25 ++++++++++--------------- jobspec/parse_test.go | 6 +++--- jobspec/test-fixtures/basic.hcl | 5 ++--- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/command/init.go b/command/init.go index 6bff4990e..92554ce45 100644 --- a/command/init.go +++ b/command/init.go @@ -128,11 +128,11 @@ job "example" { } } - service "id-redis-check" { + service { # name = redis tags = ["global", "cache"] port = "db" - check "id-alive-check" { + check { name = "alive" type = "tcp" interval = "10s" diff --git a/jobspec/parse.go b/jobspec/parse.go index 725006be8..ebf7f250f 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -144,7 +144,7 @@ func parseJob(result *structs.Job, list *ast.ObjectList) error { // If we have tasks outside, create TaskGroups for them if o := listVal.Filter("task"); len(o.Items) > 0 { var tasks []*structs.Task - if err := parseTasks(&tasks, o); err != nil { + if err := parseTasks(result.Name, "", &tasks, o); err != nil { return err } @@ -247,7 +247,7 @@ func parseGroups(result *structs.Job, list *ast.ObjectList) error { // Parse tasks if o := listVal.Filter("task"); len(o.Items) > 0 { - if err := parseTasks(&g.Tasks, o); err != nil { + if err := parseTasks(result.Name, g.Name, &g.Tasks, o); err != nil { return err } } @@ -346,7 +346,7 @@ func parseConstraints(result *[]*structs.Constraint, list *ast.ObjectList) error return nil } -func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { +func parseTasks(jobName string, taskGroupName string, result *[]*structs.Task, list *ast.ObjectList) error { list = list.Children() if len(list.Items) == 0 { return nil @@ -385,6 +385,9 @@ func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { // Build the task var t structs.Task t.Name = n + if taskGroupName == "" { + taskGroupName = n + } if err := mapstructure.WeakDecode(m, &t); err != nil { return err } @@ -403,7 +406,7 @@ func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { } if o := listVal.Filter("service"); len(o.Items) > 0 { - if err := parseServices(&t, o); err != nil { + if err := parseServices(jobName, taskGroupName, &t, o); err != nil { return err } } @@ -459,13 +462,10 @@ func parseTasks(result *[]*structs.Task, list *ast.ObjectList) error { return nil } -func parseServices(task *structs.Task, serviceObjs *ast.ObjectList) error { +func parseServices(jobName string, taskGroupName string, task *structs.Task, serviceObjs *ast.ObjectList) error { task.Services = make([]structs.Service, len(serviceObjs.Items)) for idx, o := range serviceObjs.Items { var service structs.Service - label := o.Keys[0].Token.Value().(string) - service.Id = label - var m map[string]interface{} if err := hcl.DecodeObject(&m, o.Val); err != nil { return err @@ -478,7 +478,7 @@ func parseServices(task *structs.Task, serviceObjs *ast.ObjectList) error { } if service.Name == "" { - service.Name = service.Id + service.Name = fmt.Sprintf("%s-%s-%s", jobName, taskGroupName, task.Name) } // Fileter checks @@ -486,7 +486,7 @@ func parseServices(task *structs.Task, serviceObjs *ast.ObjectList) error { if ot, ok := o.Val.(*ast.ObjectType); ok { checkList = ot.List } else { - return fmt.Errorf("service '%s': should be an object", label) + return fmt.Errorf("service '%s': should be an object", service.Name) } if co := checkList.Filter("check"); len(co.Items) > 0 { @@ -505,7 +505,6 @@ func parseChecks(service *structs.Service, checkObjs *ast.ObjectList) error { service.Checks = make([]structs.ServiceCheck, len(checkObjs.Items)) for idx, co := range checkObjs.Items { var check structs.ServiceCheck - label := co.Keys[0].Token.Value().(string) var cm map[string]interface{} if err := hcl.DecodeObject(&cm, co.Val); err != nil { return err @@ -522,10 +521,6 @@ func parseChecks(service *structs.Service, checkObjs *ast.ObjectList) error { return err } - check.Id = label - if check.Name == "" { - check.Name = label - } service.Checks[idx] = check } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 1c993caa2..398c95e0b 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -96,13 +96,13 @@ func TestParse(t *testing.T) { }, Services: []structs.Service{ { - Id: "service-id", - Name: "service-name", + Id: "", + Name: "binstore-storagelocker-binsl-binstore", Tags: []string{"foo", "bar"}, PortLabel: "http", Checks: []structs.ServiceCheck{ { - Id: "check-id", + Id: "", Name: "check-name", Type: "tcp", Interval: 10 * time.Second, diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index 609c8f2f5..9696fdef8 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -45,11 +45,10 @@ job "binstore-storagelocker" { HELLO = "world" LOREM = "ipsum" } - service "service-id" { - name = "service-name" + service { tags = ["foo", "bar"] port = "http" - check "check-id" { + check { name = "check-name" type = "tcp" interval = "10s" From 44a03b944c47f0de686a4a48cada41888422bfc9 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 16:08:03 -0800 Subject: [PATCH 14/17] Validating that services are named explicitly if there is more than one Service defn --- jobspec/parse.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/jobspec/parse.go b/jobspec/parse.go index ebf7f250f..6a9df3425 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -477,6 +477,10 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser return err } + if idx > 0 && service.Name == "" { + return fmt.Errorf("More than one service block is declared, please name each service explicitly") + } + if service.Name == "" { service.Name = fmt.Sprintf("%s-%s-%s", jobName, taskGroupName, task.Name) } From 169bb0f99c60555d2d11b9de097befb8abcb6b1f Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 16:44:05 -0800 Subject: [PATCH 15/17] Adding prefix to user defined name and forcing id to be blank during parsing --- jobspec/parse.go | 8 +++++++- nomad/structs/structs.go | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/jobspec/parse.go b/jobspec/parse.go index 6a9df3425..0f1ea8d0f 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -464,6 +464,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)) + var defaultServiceName bool for idx, o := range serviceObjs.Items { var service structs.Service var m map[string]interface{} @@ -477,14 +478,19 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser return err } - if idx > 0 && service.Name == "" { + if defaultServiceName && service.Name == "" { return fmt.Errorf("More than one service block is declared, please name each service explicitly") } 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) } + service.Id = "" // Forcing this to be blank while parsing since we will autogenerate this + // Fileter checks var checkList *ast.ObjectList if ot, ok := o.Val.(*ast.ObjectType); ok { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 794c91b39..4a0f29f88 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1005,7 +1005,7 @@ const ( // The ServiceCheck data model represents the consul health check that // Nomad registers for a Task type ServiceCheck struct { - Id string // If of the check + Id string // Id of the check, must be unique and it is autogenrated 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 From 5c3a6585256565d0bb0c70add418bd587d18847a Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 17:06:29 -0800 Subject: [PATCH 16/17] Added a test which makes sure parsing fails if more than one service block has non empty names --- jobspec/parse.go | 4 +- jobspec/parse_test.go | 17 ++++ .../test-fixtures/incorrect-service-def.hcl | 77 +++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 jobspec/test-fixtures/incorrect-service-def.hcl diff --git a/jobspec/parse.go b/jobspec/parse.go index 0f1ea8d0f..92f3c5048 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -479,7 +479,7 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser } if defaultServiceName && service.Name == "" { - return fmt.Errorf("More than one service block is declared, please name each service explicitly") + return fmt.Errorf("Only one service block may omit the Name field") } if service.Name == "" { @@ -489,8 +489,6 @@ func parseServices(jobName string, taskGroupName string, task *structs.Task, ser service.Name = fmt.Sprintf("%s-%s-%s-%s", jobName, taskGroupName, task.Name, service.Name) } - service.Id = "" // Forcing this to be blank while parsing since we will autogenerate this - // Fileter checks var checkList *ast.ObjectList if ot, ok := o.Val.(*ast.ObjectType); ok { diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 398c95e0b..90f6f4c62 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -331,3 +331,20 @@ func TestOverlappingPorts(t *testing.T) { t.Fatalf("Expected collision error; got %v", err) } } + +func TestIncompleteServiceDefn(t *testing.T) { + path, err := filepath.Abs(filepath.Join("./test-fixtures", "incorrect-service-def.hcl")) + if err != nil { + t.Fatalf("Can't get absoluate path for file: %s", err) + } + + _, err = ParseFile(path) + + if err == nil { + t.Fatalf("Expected an error") + } + + if !strings.Contains(err.Error(), "Only one service block may omit the Name field") { + t.Fatalf("Expected collision error; got %v", err) + } +} diff --git a/jobspec/test-fixtures/incorrect-service-def.hcl b/jobspec/test-fixtures/incorrect-service-def.hcl new file mode 100644 index 000000000..8a0029842 --- /dev/null +++ b/jobspec/test-fixtures/incorrect-service-def.hcl @@ -0,0 +1,77 @@ +job "binstore-storagelocker" { + region = "global" + type = "service" + priority = 50 + all_at_once = true + datacenters = ["us2", "eu1"] + + meta { + foo = "bar" + } + + constraint { + attribute = "kernel.os" + value = "windows" + } + + update { + stagger = "60s" + max_parallel = 2 + } + + task "outside" { + driver = "java" + config { + jar = "s3://my-cool-store/foo.jar" + } + meta { + my-cool-key = "foobar" + } + } + + group "binsl" { + count = 5 + restart { + attempts = 5 + interval = "10m" + delay = "15s" + } + task "binstore" { + driver = "docker" + config { + image = "hashicorp/binstore" + } + env { + HELLO = "world" + LOREM = "ipsum" + } + service { + tags = ["foo", "bar"] + port = "http" + check { + name = "check-name" + type = "http" + interval = "10s" + timeout = "2s" + } + } + service { + port = "one" + } + resources { + cpu = 500 + memory = 128 + + network { + mbits = "100" + port "one" { + static = 1 + } + port "three" { + static = 3 + } + port "http" {} + } + } + } +} From 1701ff240cf04db90a9f19d15f20682b4fdb5248 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 17 Nov 2015 17:12:21 -0800 Subject: [PATCH 17/17] Fixed typo --- jobspec/parse_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 90f6f4c62..6eb19af11 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -318,7 +318,7 @@ func TestBadPorts(t *testing.T) { func TestOverlappingPorts(t *testing.T) { path, err := filepath.Abs(filepath.Join("./test-fixtures", "overlapping-ports.hcl")) if err != nil { - t.Fatalf("Can't get absoluate path for file: %s", err) + t.Fatalf("Can't get absolute path for file: %s", err) } _, err = ParseFile(path) @@ -335,7 +335,7 @@ func TestOverlappingPorts(t *testing.T) { func TestIncompleteServiceDefn(t *testing.T) { path, err := filepath.Abs(filepath.Join("./test-fixtures", "incorrect-service-def.hcl")) if err != nil { - t.Fatalf("Can't get absoluate path for file: %s", err) + t.Fatalf("Can't get absolute path for file: %s", err) } _, err = ParseFile(path)