From 6cb5fd369414872feb64f96c29c4f7ec37972617 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Fri, 8 Jul 2016 14:09:27 -0700 Subject: [PATCH 1/2] Allowing ports to be overriden in check definitions --- api/tasks.go | 19 +++++++++-------- command/agent/consul/syncer.go | 14 +++++++----- command/agent/consul/syncer_test.go | 33 ++++++++++++++++++++++++++++- jobspec/parse.go | 1 + jobspec/parse_test.go | 9 ++++---- jobspec/test-fixtures/basic.hcl | 1 + nomad/structs/structs.go | 18 +++++++++------- 7 files changed, 68 insertions(+), 27 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index ca1ff41d8..1ab7a2e88 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -60,15 +60,16 @@ type RestartPolicy struct { // The ServiceCheck data model represents the consul health check that // Nomad registers for a Task type ServiceCheck struct { - Id string - Name string - Type string - Command string - Args []string - Path string - Protocol string - Interval time.Duration - Timeout time.Duration + Id string + Name string + Type string + Command string + Args []string + Path string + Protocol string `mapstructure:"port"` + PortLabel string `mapstructure:"port"` + Interval time.Duration + Timeout time.Duration } // The Service model represents a Consul service definition diff --git a/command/agent/consul/syncer.go b/command/agent/consul/syncer.go index 4c000b1e9..14475ed4a 100644 --- a/command/agent/consul/syncer.go +++ b/command/agent/consul/syncer.go @@ -684,14 +684,18 @@ func (c *Syncer) registerCheck(chkReg *consul.AgentCheckRegistration) error { // createCheckReg creates a Check that can be registered with Nomad. It also // creates a Nomad check for the check types that it can handle. -func (c *Syncer) createCheckReg(check *structs.ServiceCheck, service *consul.AgentServiceRegistration) (*consul.AgentCheckRegistration, error) { +func (c *Syncer) createCheckReg(check *structs.ServiceCheck, serviceReg *consul.AgentServiceRegistration) (*consul.AgentCheckRegistration, error) { chkReg := consul.AgentCheckRegistration{ - ID: check.Hash(service.ID), + ID: check.Hash(serviceReg.ID), Name: check.Name, - ServiceID: service.ID, + ServiceID: serviceReg.ID, } chkReg.Timeout = check.Timeout.String() chkReg.Interval = check.Interval.String() + host, port := serviceReg.Address, serviceReg.Port + if check.PortLabel != "" { + host, port = c.addrFinder(check.PortLabel) + } switch check.Type { case structs.ServiceCheckHTTP: if check.Protocol == "" { @@ -699,12 +703,12 @@ func (c *Syncer) createCheckReg(check *structs.ServiceCheck, service *consul.Age } url := url.URL{ Scheme: check.Protocol, - Host: fmt.Sprintf("%s:%d", service.Address, service.Port), + Host: fmt.Sprintf("%s:%d", host, port), Path: check.Path, } chkReg.HTTP = url.String() case structs.ServiceCheckTCP: - chkReg.TCP = fmt.Sprintf("%s:%d", service.Address, service.Port) + chkReg.TCP = fmt.Sprintf("%s:%d", host, port) case structs.ServiceCheckScript: chkReg.TTL = (check.Interval + ttlCheckBuffer).String() default: diff --git a/command/agent/consul/syncer_test.go b/command/agent/consul/syncer_test.go index 3f644accd..94325b33d 100644 --- a/command/agent/consul/syncer_test.go +++ b/command/agent/consul/syncer_test.go @@ -26,12 +26,19 @@ var ( Interval: 30 * time.Second, Timeout: 5 * time.Second, } + check2 = structs.ServiceCheck{ + Name: "check1", + Type: "tcp", + PortLabel: "port2", + Interval: 3 * time.Second, + Timeout: 1 * time.Second, + } service1 = structs.Service{ Name: "foo-1", Tags: []string{"tag1", "tag2"}, PortLabel: "port1", Checks: []*structs.ServiceCheck{ - &check1, + &check1, &check2, }, } @@ -42,6 +49,30 @@ var ( } ) +func TestCheckRegistration(t *testing.T) { + cs, err := NewSyncer(config.DefaultConsulConfig(), make(chan struct{}), logger) + if err != nil { + t.Fatalf("Err: %v", err) + } + + task := mockTask() + cs.SetAddrFinder(task.FindHostAndPortFor) + + srvReg, _ := cs.createService(&service1, "domain", "key") + check1Reg, _ := cs.createCheckReg(&check1, srvReg) + check2Reg, _ := cs.createCheckReg(&check2, srvReg) + + expected := "10.10.11.5:20002" + if check1Reg.TCP != expected { + t.Fatalf("expected: %v, actual: %v", expected, check1Reg.TCP) + } + + expected = "10.10.11.5:20003" + if check2Reg.TCP != expected { + t.Fatalf("expected: %v, actual: %v", expected, check1Reg.TCP) + } +} + func TestConsulServiceRegisterServices(t *testing.T) { t.Skip() diff --git a/jobspec/parse.go b/jobspec/parse.go index 1ccfd8617..81639d4cb 100644 --- a/jobspec/parse.go +++ b/jobspec/parse.go @@ -767,6 +767,7 @@ func parseChecks(service *structs.Service, checkObjs *ast.ObjectList) error { "timeout", "path", "protocol", + "port", "command", "args", } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index e73a1e5c2..80ca8c4ed 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -104,10 +104,11 @@ func TestParse(t *testing.T) { PortLabel: "http", Checks: []*structs.ServiceCheck{ { - Name: "check-name", - Type: "tcp", - Interval: 10 * time.Second, - Timeout: 2 * time.Second, + Name: "check-name", + Type: "tcp", + PortLabel: "admin", + Interval: 10 * time.Second, + Timeout: 2 * time.Second, }, }, }, diff --git a/jobspec/test-fixtures/basic.hcl b/jobspec/test-fixtures/basic.hcl index 835e53f07..f1df91ba5 100644 --- a/jobspec/test-fixtures/basic.hcl +++ b/jobspec/test-fixtures/basic.hcl @@ -72,6 +72,7 @@ job "binstore-storagelocker" { type = "tcp" interval = "10s" timeout = "2s" + port = "admin" } } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index b4cc1a237..1a927f0ce 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -1504,14 +1504,15 @@ const ( // The ServiceCheck data model represents the consul health check that // Nomad registers for a Task type ServiceCheck struct { - Name string // Name of the check, defaults to id - Type string // Type of the check - tcp, http, docker and script - Command string // Command is the command to run for script checks - Args []string // Args is a list of argumes for script checks - 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 + Name string // Name of the check, defaults to id + Type string // Type of the check - tcp, http, docker and script + Command string // Command is the command to run for script checks + Args []string // Args is a list of argumes for script checks + Path string // path of the health check url for http type check + Protocol string // Protocol to use if check is http, defaults to http + PortLabel string `mapstructure:"port"` // The port to use for tcp/http checks + 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) Copy() *ServiceCheck { @@ -1575,6 +1576,7 @@ func (sc *ServiceCheck) Hash(serviceID string) string { io.WriteString(h, strings.Join(sc.Args, "")) io.WriteString(h, sc.Path) io.WriteString(h, sc.Protocol) + io.WriteString(h, sc.PortLabel) io.WriteString(h, sc.Interval.String()) io.WriteString(h, sc.Timeout.String()) return fmt.Sprintf("%x", h.Sum(nil)) From 473d165c0da7c3308171a97c707fd362fc54a8a6 Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Fri, 8 Jul 2016 16:31:54 -0700 Subject: [PATCH 2/2] Using net.JoinHostPort instead of handcrafting addrs --- command/agent/consul/syncer.go | 6 ++++-- nomad/structs/diff_test.go | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/command/agent/consul/syncer.go b/command/agent/consul/syncer.go index 14475ed4a..dd8e8e88c 100644 --- a/command/agent/consul/syncer.go +++ b/command/agent/consul/syncer.go @@ -27,7 +27,9 @@ package consul import ( "fmt" "log" + "net" "net/url" + "strconv" "strings" "sync" "time" @@ -703,12 +705,12 @@ func (c *Syncer) createCheckReg(check *structs.ServiceCheck, serviceReg *consul. } url := url.URL{ Scheme: check.Protocol, - Host: fmt.Sprintf("%s:%d", host, port), + Host: net.JoinHostPort(host, strconv.Itoa(port)), Path: check.Path, } chkReg.HTTP = url.String() case structs.ServiceCheckTCP: - chkReg.TCP = fmt.Sprintf("%s:%d", host, port) + chkReg.TCP = net.JoinHostPort(host, strconv.Itoa(port)) case structs.ServiceCheckScript: chkReg.TTL = (check.Interval + ttlCheckBuffer).String() default: diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 5555a2c33..fc97e0502 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -2781,6 +2781,12 @@ func TestTaskDiff(t *testing.T) { Old: "foo", New: "foo", }, + { + Type: DiffTypeNone, + Name: "PortLabel", + Old: "", + New: "", + }, { Type: DiffTypeNone, Name: "Protocol",