Merge pull request #1395 from hashicorp/f-check-port

Allowing ports to be overriden in check definitions

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

command/agent/consul/syncer.go

@@ -27,7 +27,9 @@ package consul
 import (
 	"fmt"
 	"log"
+	"net"
 	"net/url"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -684,14 +686,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 +705,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:   net.JoinHostPort(host, strconv.Itoa(port)),
 			Path:   check.Path,
 		}
 		chkReg.HTTP = url.String()
 	case structs.ServiceCheckTCP:
-		chkReg.TCP = fmt.Sprintf("%s:%d", service.Address, service.Port)
+		chkReg.TCP = net.JoinHostPort(host, strconv.Itoa(port))
 	case structs.ServiceCheckScript:
 		chkReg.TTL = (check.Interval + ttlCheckBuffer).String()
 	default:

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

jobspec/parse.go

@@ -767,6 +767,7 @@ func parseChecks(service *structs.Service, checkObjs *ast.ObjectList) error {
 			"timeout",
 			"path",
 			"protocol",
+			"port",
 			"command",
 			"args",
 		}

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,
 											},
 										},
 									},

jobspec/test-fixtures/basic.hcl

@@ -72,6 +72,7 @@ job "binstore-storagelocker" {
           type     = "tcp"
           interval = "10s"
           timeout  = "2s"
+          port     = "admin"
         }
       }
 

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

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