Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Header and Method support for HTTP checks #3031

Merged
merged 14 commits into from
Aug 18, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ IMPROVEMENTS:
* deployment: Emit task events explaining unhealthy allocations[GH-3025]
* deployment: Better description when a deployment should auto-revert but there
is no target [GH-3024]
* discovery: Add HTTP header and method support to checks [GH-3031]
* driver/docker: Added DNS options [GH-2992]
* driver/rkt: support read-only volume mounts [GH-2883]
* jobspec: Add `shutdown_delay` so tasks can delay shutdown after
Expand Down
2 changes: 2 additions & 0 deletions api/tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ type ServiceCheck struct {
Timeout time.Duration
InitialStatus string `mapstructure:"initial_status"`
TLSSkipVerify bool `mapstructure:"tls_skip_verify"`
Header map[string][]string
Method string
}

// The Service model represents a Consul service definition
Expand Down
12 changes: 12 additions & 0 deletions client/task_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -1450,6 +1450,18 @@ func interpolateServices(taskEnv *env.TaskEnv, task *structs.Task) *structs.Task
check.Protocol = taskEnv.ReplaceEnv(check.Protocol)
check.PortLabel = taskEnv.ReplaceEnv(check.PortLabel)
check.InitialStatus = taskEnv.ReplaceEnv(check.InitialStatus)
check.Method = taskEnv.ReplaceEnv(check.Method)
if len(check.Header) > 0 {
header := make(map[string][]string, len(check.Header))
for k, vs := range check.Header {
newVals := make([]string, len(vs))
for i, v := range vs {
newVals[i] = taskEnv.ReplaceEnv(v)
}
header[taskEnv.ReplaceEnv(k)] = newVals
}
check.Header = header
}
}
service.Name = taskEnv.ReplaceEnv(service.Name)
service.PortLabel = taskEnv.ReplaceEnv(service.PortLabel)
Expand Down
84 changes: 84 additions & 0 deletions client/task_runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"os"
"path/filepath"
"reflect"
"strings"
"syscall"
"testing"
"time"
Expand All @@ -17,11 +18,13 @@ import (
"github.com/golang/snappy"
"github.com/hashicorp/nomad/client/allocdir"
"github.com/hashicorp/nomad/client/config"
"github.com/hashicorp/nomad/client/driver/env"
cstructs "github.com/hashicorp/nomad/client/structs"
"github.com/hashicorp/nomad/client/vaultclient"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/testutil"
"github.com/kr/pretty"
)

func testLogger() *log.Logger {
Expand Down Expand Up @@ -1615,6 +1618,87 @@ func TestTaskRunner_Pre06ScriptCheck(t *testing.T) {
t.Run(run("0.5.6", "mock_driver", "tcp", false))
}

func TestTaskRunner_interpolateServices(t *testing.T) {
t.Parallel()
task := &structs.Task{
Services: []*structs.Service{
{
Name: "${name}",
PortLabel: "${portlabel}",
Tags: []string{"${tags}"},
Checks: []*structs.ServiceCheck{
{
Name: "${checkname}",
Type: "${checktype}",
Command: "${checkcmd}",
Args: []string{"${checkarg}"},
Path: "${checkstr}",
Protocol: "${checkproto}",
PortLabel: "${checklabel}",
InitialStatus: "${checkstatus}",
Method: "${checkmethod}",
Header: map[string][]string{
"${checkheaderk}": {"${checkheaderv}"},
},
},
},
},
},
}

env := &env.TaskEnv{
EnvMap: map[string]string{
"name": "name",
"portlabel": "portlabel",
"tags": "tags",
"checkname": "checkname",
"checktype": "checktype",
"checkcmd": "checkcmd",
"checkarg": "checkarg",
"checkstr": "checkstr",
"checkpath": "checkpath",
"checkproto": "checkproto",
"checklabel": "checklabel",
"checkstatus": "checkstatus",
"checkmethod": "checkmethod",
"checkheaderk": "checkheaderk",
"checkheaderv": "checkheaderv",
},
}

interpTask := interpolateServices(env, task)

exp := &structs.Task{
Services: []*structs.Service{
{
Name: "name",
PortLabel: "portlabel",
Tags: []string{"tags"},
Checks: []*structs.ServiceCheck{
{
Name: "checkname",
Type: "checktype",
Command: "checkcmd",
Args: []string{"checkarg"},
Path: "checkstr",
Protocol: "checkproto",
PortLabel: "checklabel",
InitialStatus: "checkstatus",
Method: "checkmethod",
Header: map[string][]string{
"checkheaderk": {"checkheaderv"},
},
},
},
},
},
}

if diff := pretty.Diff(interpTask, exp); len(diff) > 0 {
t.Fatalf("diff:\n%s\n", strings.Join(diff, "\n"))
}
}

func TestTaskRunner_ShutdownDelay(t *testing.T) {
t.Parallel()

Expand Down
2 changes: 2 additions & 0 deletions command/agent/consul/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,8 @@ func createCheckReg(serviceID, checkID string, check *structs.ServiceCheck, host
}
url := base.ResolveReference(relative)
chkReg.HTTP = url.String()
chkReg.Method = check.Method
chkReg.Header = check.Header
case structs.ServiceCheckTCP:
chkReg.TCP = net.JoinHostPort(host, strconv.Itoa(port))
case structs.ServiceCheckScript:
Expand Down
46 changes: 46 additions & 0 deletions command/agent/consul/unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import (
"log"
"os"
"reflect"
"strings"
"sync"
"testing"
"time"

"github.com/hashicorp/consul/api"
cstructs "github.com/hashicorp/nomad/client/structs"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/kr/pretty"
)

const (
Expand Down Expand Up @@ -1352,3 +1354,47 @@ func TestIsNomadService(t *testing.T) {
})
}
}

// TestCreateCheckReg asserts Nomad ServiceCheck structs are properly converted
// to Consul API AgentCheckRegistrations.
func TestCreateCheckReg(t *testing.T) {
check := &structs.ServiceCheck{
Name: "name",
Type: "http",
Path: "/path",
PortLabel: "label",
Method: "POST",
Header: map[string][]string{
"Foo": {"bar"},
},
}

serviceID := "testService"
checkID := check.Hash(serviceID)
host := "localhost"
port := 41111

expected := &api.AgentCheckRegistration{
ID: checkID,
Name: "name",
ServiceID: serviceID,
AgentServiceCheck: api.AgentServiceCheck{
Timeout: "0s",
Interval: "0s",
HTTP: fmt.Sprintf("http://%s:%d/path", host, port),
Method: "POST",
Header: map[string][]string{
"Foo": {"bar"},
},
},
}

actual, err := createCheckReg(serviceID, checkID, check, host, port)
if err != nil {
t.Fatalf("err: %v", err)
}

if diff := pretty.Diff(actual, expected); len(diff) > 0 {
t.Fatalf("diff:\n%s\n", strings.Join(diff, "\n"))
}
}
2 changes: 2 additions & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,8 @@ func ApiTaskToStructsTask(apiTask *api.Task, structsTask *structs.Task) {
Timeout: check.Timeout,
InitialStatus: check.InitialStatus,
TLSSkipVerify: check.TLSSkipVerify,
Header: check.Header,
Method: check.Method,
}
}
}
Expand Down
15 changes: 15 additions & 0 deletions helper/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,21 @@ func CopyMapStringFloat64(m map[string]float64) map[string]float64 {
return c
}

// CopyMapStringSliceString copies a map of strings to string slices such as
// http.Header
func CopyMapStringSliceString(m map[string][]string) map[string][]string {
l := len(m)
if l == 0 {
return nil
}

c := make(map[string][]string, l)
for k, v := range m {
c[k] = CopySliceString(v)
}
return c
}

func CopySliceString(s []string) []string {
l := len(s)
if l == 0 {
Expand Down
18 changes: 18 additions & 0 deletions helper/funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,24 @@ func TestMapStringStringSliceValueSet(t *testing.T) {
}
}

func TestCopyMapStringSliceString(t *testing.T) {
m := map[string][]string{
"x": []string{"a", "b", "c"},
"y": []string{"1", "2", "3"},
"z": nil,
}

c := CopyMapStringSliceString(m)
if !reflect.DeepEqual(c, m) {
t.Fatalf("%#v != %#v", m, c)
}

c["x"][1] = "---"
if reflect.DeepEqual(c, m) {
t.Fatalf("Shared slices: %#v == %#v", m["x"], c["x"])
}
}

func TestClearEnvVar(t *testing.T) {
type testCase struct {
input string
Expand Down
33 changes: 33 additions & 0 deletions jobspec/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,8 @@ func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error {
"args",
"initial_status",
"tls_skip_verify",
"header",
"method",
}
if err := checkHCLKeys(co.Val, valid); err != nil {
return multierror.Prefix(err, "check ->")
Expand All @@ -972,6 +974,37 @@ func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error {
if err := hcl.DecodeObject(&cm, co.Val); err != nil {
return err
}

// HCL allows repeating stanzas so merge 'header' into a single
// map[string][]string.
if headerI, ok := cm["header"]; ok {
headerRaw, ok := headerI.([]map[string]interface{})
if !ok {
return fmt.Errorf("check -> header -> expected a []map[string][]string but found %T", headerI)
}
m := map[string][]string{}
for _, rawm := range headerRaw {
for k, vI := range rawm {
vs, ok := vI.([]interface{})
if !ok {
return fmt.Errorf("check -> header -> %q expected a []string but found %T", k, vI)
}
for _, vI := range vs {
v, ok := vI.(string)
if !ok {
return fmt.Errorf("check -> header -> %q expected a string but found %T", k, vI)
}
m[k] = append(m[k], v)
}
}
}

check.Header = m

// Remove "header" as it has been parsed
delete(cm, "header")
}

dec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
DecodeHook: mapstructure.StringToTimeDurationHookFunc(),
WeaklyTypedInput: true,
Expand Down
15 changes: 15 additions & 0 deletions jobspec/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,9 +450,14 @@ func TestParse(t *testing.T) {
{
Name: "check-name",
Type: "http",
Path: "/",
Interval: 10 * time.Second,
Timeout: 2 * time.Second,
InitialStatus: capi.HealthPassing,
Method: "POST",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a failing case? With numbers or something.

Header: map[string][]string{
"Authorization": {"Basic ZWxhc3RpYzpjaGFuZ2VtZQ=="},
},
},
},
},
Expand All @@ -464,6 +469,16 @@ func TestParse(t *testing.T) {
},
false,
},
{
"service-check-bad-header.hcl",
nil,
true,
},
{
"service-check-bad-header-2.hcl",
nil,
true,
},
{
// TODO This should be pushed into the API
"vault_inheritance.hcl",
Expand Down
28 changes: 28 additions & 0 deletions jobspec/test-fixtures/service-check-bad-header-2.hcl
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
job "check_bad_header" {
type = "service"
group "group" {
count = 1

task "task" {
service {
tags = ["bar"]
port = "http"

check {
name = "check-name"
type = "http"
path = "/"
method = "POST"
interval = "10s"
timeout = "2s"
initial_status = "passing"

header {
Authorization = ["ok", 840]
}
}
}
}
}
}

Loading