From 197e755506c5c9299af3e4ffaec9b387fd1a23ab Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Wed, 18 Nov 2015 13:02:25 -0800 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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