From d1cdf4c4899ea69bb0136a6a372d7f6c72ab2718 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Thu, 25 Mar 2021 12:51:13 +1100 Subject: [PATCH 1/8] Add a 'body' field to the check stanza Consul allows specifying the HTTP body to send in a health check. Nomad uses Consul for health checking so this just plumbs the value through to where the Consul API is called. There is no validation that `body` is not used with an incompatible check method like GET. --- api/services.go | 1 + command/agent/consul/service_client.go | 1 + command/agent/job_endpoint.go | 2 ++ command/agent/job_endpoint_test.go | 4 ++++ nomad/structs/diff_test.go | 12 ++++++++++++ nomad/structs/services.go | 5 +++++ vendor/github.com/hashicorp/nomad/api/services.go | 1 + 7 files changed, 26 insertions(+) diff --git a/api/services.go b/api/services.go index d7cb87e57af..50932ccf043 100644 --- a/api/services.go +++ b/api/services.go @@ -95,6 +95,7 @@ type ServiceCheck struct { TaskName string `mapstructure:"task" hcl:"task,optional"` SuccessBeforePassing int `mapstructure:"success_before_passing" hcl:"success_before_passing,optional"` FailuresBeforeCritical int `mapstructure:"failures_before_critical" hcl:"failures_before_critical,optional"` + Body string `hcl:"body,optional"` OnUpdate string `mapstructure:"on_update" hcl:"on_update,optional"` } diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go index 6da8097acbb..ae421555d1a 100644 --- a/command/agent/consul/service_client.go +++ b/command/agent/consul/service_client.go @@ -1487,6 +1487,7 @@ func createCheckReg(serviceID, checkID string, check *structs.ServiceCheck, host chkReg.HTTP = checkURL.String() chkReg.Method = check.Method chkReg.Header = check.Header + chkReg.Body = check.Body case structs.ServiceCheckTCP: chkReg.TCP = net.JoinHostPort(host, strconv.Itoa(port)) diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index b4fc379aee5..bc544f2a2ef 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1082,6 +1082,7 @@ func ApiTaskToStructsTask(job *structs.Job, group *structs.TaskGroup, TLSSkipVerify: check.TLSSkipVerify, Header: check.Header, Method: check.Method, + Body: check.Body, GRPCService: check.GRPCService, GRPCUseTLS: check.GRPCUseTLS, SuccessBeforePassing: check.SuccessBeforePassing, @@ -1316,6 +1317,7 @@ func ApiServicesToStructs(in []*api.Service) []*structs.Service { TLSSkipVerify: check.TLSSkipVerify, Header: check.Header, Method: check.Method, + Body: check.Body, GRPCService: check.GRPCService, GRPCUseTLS: check.GRPCUseTLS, TaskName: check.TaskName, diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 650c32bb54b..5831860caa9 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -1954,6 +1954,8 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Args: []string{"a", "b"}, Path: "/check", Protocol: "http", + Method: "POST", + Body: "{\"check\":\"mem\"}", PortLabel: "foo", AddressMode: "driver", GRPCService: "foo.Bar", @@ -2330,6 +2332,8 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { Args: []string{"a", "b"}, Path: "/check", Protocol: "http", + Method: "POST", + Body: "{\"check\":\"mem\"}", PortLabel: "foo", AddressMode: "driver", GRPCService: "foo.Bar", diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 76133767495..cc6a18d9804 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -2895,6 +2895,12 @@ func TestTaskGroupDiff(t *testing.T) { Old: "", New: "", }, + { + Type: DiffTypeNone, + Name: "Body", + Old: "", + New: "", + }, { Type: DiffTypeEdited, Name: "Command", @@ -6236,6 +6242,12 @@ func TestTaskDiff(t *testing.T) { Old: "", New: "", }, + { + Type: DiffTypeNone, + Name: "Body", + Old: "", + New: "", + }, { Type: DiffTypeNone, Name: "Command", diff --git a/nomad/structs/services.go b/nomad/structs/services.go index b30839ad61c..51651ab1dcc 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -61,6 +61,7 @@ type ServiceCheck struct { TaskName string // What task to execute this check in SuccessBeforePassing int // Number of consecutive successes required before considered healthy FailuresBeforeCritical int // Number of consecutive failures required before considered unhealthy + Body string // Body to use in HTTP check OnUpdate string } @@ -168,6 +169,10 @@ func (sc *ServiceCheck) Equals(o *ServiceCheck) bool { return false } + if sc.Body != o.Body { + return false + } + if sc.OnUpdate != o.OnUpdate { return false } diff --git a/vendor/github.com/hashicorp/nomad/api/services.go b/vendor/github.com/hashicorp/nomad/api/services.go index d7cb87e57af..50932ccf043 100644 --- a/vendor/github.com/hashicorp/nomad/api/services.go +++ b/vendor/github.com/hashicorp/nomad/api/services.go @@ -95,6 +95,7 @@ type ServiceCheck struct { TaskName string `mapstructure:"task" hcl:"task,optional"` SuccessBeforePassing int `mapstructure:"success_before_passing" hcl:"success_before_passing,optional"` FailuresBeforeCritical int `mapstructure:"failures_before_critical" hcl:"failures_before_critical,optional"` + Body string `hcl:"body,optional"` OnUpdate string `mapstructure:"on_update" hcl:"on_update,optional"` } From 07be2a44e01dc23f58c3c0e72703d188825c628f Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Thu, 18 Mar 2021 22:27:48 +1100 Subject: [PATCH 2/8] Document usage of 'body' field --- website/content/api-docs/json-jobs.mdx | 2 ++ website/content/docs/job-specification/service.mdx | 2 ++ 2 files changed, 4 insertions(+) diff --git a/website/content/api-docs/json-jobs.mdx b/website/content/api-docs/json-jobs.mdx index 960e855983a..8021756c89c 100644 --- a/website/content/api-docs/json-jobs.mdx +++ b/website/content/api-docs/json-jobs.mdx @@ -496,6 +496,8 @@ The `Task` object supports the following keys: - `Method`: The HTTP method to use for HTTP checks. Defaults to GET. + - `Body`: The HTTP body to use for HTTP checks. Defaults to an empty string. + - `Path`: The path of the HTTP endpoint which Consul will query to query the health of a service if the type of the check is `http`. Nomad will add the IP of the service and the port, users are only required diff --git a/website/content/docs/job-specification/service.mdx b/website/content/docs/job-specification/service.mdx index 1a08cc00c5d..464d4d565de 100644 --- a/website/content/docs/job-specification/service.mdx +++ b/website/content/docs/job-specification/service.mdx @@ -246,6 +246,8 @@ scripts. - `method` `(string: "GET")` - Specifies the HTTP method to use for HTTP checks. +- `body` `(string: "")` - Specifies the HTTP body to use for HTTP checks. + - `name` `(string: "service: check")` - Specifies the name of the health check. If the name is not specified Nomad generates one based on the service name. If you have more than one check you must specify the name. From 199f7522b51749588c8f120b0fd6c39686fea85c Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Thu, 18 Mar 2021 22:28:45 +1100 Subject: [PATCH 3/8] Update changelog to mention adding the 'body' field --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20e4cdc01d4..1509b6756d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ IMPROVEMENTS: * api: Removed unimplemented `CSIVolumes.PluginList` API. [[GH-10158](https://github.com/hashicorp/nomad/issues/10158)] * cli: Update defaults for `nomad operator debug` flags `-interval` and `-server-id` to match common usage. [[GH-10121](https://github.com/hashicorp/nomad/issues/10121)] * cli: Added `nomad ui -authenticate` flag to generate a one-time token for authenticating to the web UI when ACLs are enabled. [[GH-10097](https://github.com/hashicorp/nomad/issues/10097)] + * consul: Allow setting `body` field on service/check Consul health checks. [[GH-10186](https://github.com/hashicorp/nomad/issues/10186)] * consul/connect: Enable setting `local_bind_address` field on connect upstreams [[GH-6248](https://github.com/hashicorp/nomad/issues/6248)] * consul/connect: Automatically populate `CONSUL_HTTP_ADDR` for connect native tasks in host networking mode. [[GH-10239](https://github.com/hashicorp/nomad/issues/10239)] * csi: Added support for jobs to request a unique volume ID per allocation. [[GH-10136](https://github.com/hashicorp/nomad/issues/10136)] @@ -20,6 +21,7 @@ IMPROVEMENTS: * driver/docker: Added support for configuring default logger behavior in the client configuration. [[GH-10156](https://github.com/hashicorp/nomad/issues/10156)] * nomad/structs: Removed deprecated Node.Drain field, added API extensions to restore it [[GH-10202](https://github.com/hashicorp/nomad/issues/10202)] + BUG FIXES: * agent: Only allow querying Prometheus formatted metrics if Prometheus is enabled within the config [[GH-10140](https://github.com/hashicorp/nomad/pull/10140)] * api: Fixed a panic that may occur on concurrent access to an SDK client [[GH-10302](https://github.com/hashicorp/nomad/issues/10302)] From 262670c6752e57320bde6d698a1997d8d22596fc Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Thu, 25 Mar 2021 12:21:59 +1100 Subject: [PATCH 4/8] Hash Body field as part of ServiceCheck --- nomad/structs/services.go | 1 + 1 file changed, 1 insertion(+) diff --git a/nomad/structs/services.go b/nomad/structs/services.go index 51651ab1dcc..e4bb039bccb 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -357,6 +357,7 @@ func (sc *ServiceCheck) Hash(serviceID string) string { hashString(h, sc.Interval.String()) hashString(h, sc.Timeout.String()) hashString(h, sc.Method) + hashString(h, sc.Body) hashString(h, sc.OnUpdate) // use name "true" to maintain ID stability From 9be0fd8afbf73dc5299154b2ae034bcdf7a93ebc Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Thu, 25 Mar 2021 12:36:35 +1100 Subject: [PATCH 5/8] Update TaskGroup services edited diff test to actually check Body --- nomad/structs/diff_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index cc6a18d9804..18017900377 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -2684,6 +2684,8 @@ func TestTaskGroupDiff(t *testing.T) { Args: []string{"foo"}, Path: "foo", Protocol: "http", + Method: "POST", + Body: "{\"key\": \"value\"}", Expose: true, Interval: 1 * time.Second, Timeout: 1 * time.Second, @@ -2896,9 +2898,9 @@ func TestTaskGroupDiff(t *testing.T) { New: "", }, { - Type: DiffTypeNone, + Type: DiffTypeDeleted, Name: "Body", - Old: "", + Old: "{\"key\": \"value\"}", New: "", }, { @@ -2944,9 +2946,9 @@ func TestTaskGroupDiff(t *testing.T) { New: "2000000000", }, { - Type: DiffTypeNone, + Type: DiffTypeDeleted, Name: "Method", - Old: "", + Old: "POST", New: "", }, { From e9d5990af7f4977b420a86afc8e796bd6bbaafda Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Thu, 25 Mar 2021 12:48:11 +1100 Subject: [PATCH 6/8] Test parsing of body field in jobspec2 --- jobspec2/parse_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/jobspec2/parse_test.go b/jobspec2/parse_test.go index 409001b34b3..17492d6abfd 100644 --- a/jobspec2/parse_test.go +++ b/jobspec2/parse_test.go @@ -874,3 +874,52 @@ func TestParse_UndefinedVariables(t *testing.T) { }) } } + +func TestParseServiceCheck(t *testing.T) { + hcl := ` job "group_service_check_script" { + group "group" { + service { + name = "foo-service" + port = "http" + check { + name = "check-name" + type = "http" + method = "POST" + body = "{\"check\":\"mem\"}" + } + } + } +} +` + parsedJob, err := ParseWithConfig(&ParseConfig{ + Path: "input.hcl", + Body: []byte(hcl), + }) + require.NoError(t, err) + + expectedJob := &api.Job{ + ID: stringToPtr("group_service_check_script"), + Name: stringToPtr("group_service_check_script"), + TaskGroups: []*api.TaskGroup{ + { + Name: stringToPtr("group"), + Services: []*api.Service{ + { + Name: "foo-service", + PortLabel: "http", + Checks: []api.ServiceCheck{ + { + Name: "check-name", + Type: "http", + Method: "POST", + Body: "{\"check\":\"mem\"}", + }, + }, + }, + }, + }, + }, + } + + require.Equal(t, expectedJob, parsedJob) +} From 6f21be49e3bb772fe3a5e4d7b7716fc92b0963f3 Mon Sep 17 00:00:00 2001 From: Nick Spain Date: Wed, 31 Mar 2021 21:37:56 +1100 Subject: [PATCH 7/8] Fix indentation of service check API documentation --- website/content/api-docs/json-jobs.mdx | 46 +++++++++++++------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/website/content/api-docs/json-jobs.mdx b/website/content/api-docs/json-jobs.mdx index 8021756c89c..4ceaefc00a0 100644 --- a/website/content/api-docs/json-jobs.mdx +++ b/website/content/api-docs/json-jobs.mdx @@ -466,27 +466,27 @@ The `Task` object supports the following keys: - `Name`: The name of the health check. - - `AddressMode`: Same as `AddressMode` on `Service`. Unlike services, - checks do not have an `auto` address mode as there's no way for - Nomad to know which is the best address to use for checks. Consul - needs access to the address for any HTTP or TCP checks. Added in - Nomad 0.7.1. Unlike `PortLabel`, this setting is _not_ inherited - from the `Service`. - - - `PortLabel`: Specifies the label of the port on which the check will - be performed. Note this is the _label_ of the port and not the port - number unless `AddressMode: "driver"`. The port label must match one - defined in the Network stanza. If a port value was declared on the - `Service`, this will inherit from that value if not supplied. If - supplied, this value takes precedence over the `Service.PortLabel` - value. This is useful for services which operate on multiple ports. - `http` and `tcp` checks require a port while `script` checks do not. - Checks will use the host IP and ports by default. In Nomad 0.7.1 or - later numeric ports may be used if `AddressMode: "driver"` is set on - the check. - - - `Header`: Headers for HTTP checks. Should be an object where the values are an - array of values. Headers will be written once for each value. + - `AddressMode`: Same as `AddressMode` on `Service`. Unlike services, + checks do not have an `auto` address mode as there's no way for + Nomad to know which is the best address to use for checks. Consul + needs access to the address for any HTTP or TCP checks. Added in + Nomad 0.7.1. Unlike `PortLabel`, this setting is _not_ inherited + from the `Service`. + + - `PortLabel`: Specifies the label of the port on which the check will + be performed. Note this is the _label_ of the port and not the port + number unless `AddressMode: "driver"`. The port label must match one + defined in the Network stanza. If a port value was declared on the + `Service`, this will inherit from that value if not supplied. If + supplied, this value takes precedence over the `Service.PortLabel` + value. This is useful for services which operate on multiple ports. + `http` and `tcp` checks require a port while `script` checks do not. + Checks will use the host IP and ports by default. In Nomad 0.7.1 or + later numeric ports may be used if `AddressMode: "driver"` is set on + the check. + + - `Header`: Headers for HTTP checks. Should be an object where the values + are an array of values. Headers will be written once for each value. - `Interval`: This indicates the frequency of the health checks that Consul will perform. @@ -513,8 +513,8 @@ The `Task` object supports the following keys: - `Args`: Additional arguments to the `command` for script based health checks. - - `TLSSkipVerify`: If true, Consul will not attempt to verify the - certificate when performing HTTPS checks. Requires Consul >= 0.7.2. + - `TLSSkipVerify`: If true, Consul will not attempt to verify the + certificate when performing HTTPS checks. Requires Consul >= 0.7.2. - `CheckRestart`: `CheckRestart` is an object which enables restarting of tasks based upon Consul health checks. From 26cfe0e126b6bd4de7820da1b83f3fafb19c48f8 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 13 Apr 2021 08:36:11 -0400 Subject: [PATCH 8/8] fixup changelog --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1509b6756d0..0a4cd3de71e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,6 @@ IMPROVEMENTS: * driver/docker: Added support for configuring default logger behavior in the client configuration. [[GH-10156](https://github.com/hashicorp/nomad/issues/10156)] * nomad/structs: Removed deprecated Node.Drain field, added API extensions to restore it [[GH-10202](https://github.com/hashicorp/nomad/issues/10202)] - BUG FIXES: * agent: Only allow querying Prometheus formatted metrics if Prometheus is enabled within the config [[GH-10140](https://github.com/hashicorp/nomad/pull/10140)] * api: Fixed a panic that may occur on concurrent access to an SDK client [[GH-10302](https://github.com/hashicorp/nomad/issues/10302)]