From 69ced2a2bdbb11d97f81055954c0d18501c0be5e Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 10 Oct 2022 13:02:33 -0500 Subject: [PATCH] services: remove assertion on 'task' field being set (#14864) This PR removes the assertion around when the 'task' field of a check may be set. Starting in Nomad 1.4 we automatically set the task field on all checks in support of the NSD checks feature. This is causing validation problems elsewhere, e.g. when a group service using the Consul provider sets 'task' it will fail validation that worked previously. The assertion of leaving 'task' unset was only about making sure job submitters weren't expecting some behavior, but in practice is causing bugs now that we need the task field for more than it was originally added for. We can simply update the docs, noting when the task field set by job submitters actually has value. --- .changelog/14864.txt | 3 +++ nomad/structs/structs.go | 4 ---- nomad/structs/structs_test.go | 3 --- website/content/docs/job-specification/check.mdx | 8 ++++---- 4 files changed, 7 insertions(+), 11 deletions(-) create mode 100644 .changelog/14864.txt diff --git a/.changelog/14864.txt b/.changelog/14864.txt new file mode 100644 index 000000000..fc805d70f --- /dev/null +++ b/.changelog/14864.txt @@ -0,0 +1,3 @@ +```release-note:bug +services: Fixed a regression where check task validation stopped allowing some configurations +``` diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 24fd8d103..db2cdf2f9 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -6681,10 +6681,6 @@ func (tg *TaskGroup) validateServices() error { for _, check := range service.Checks { if check.TaskName != "" { - if check.Type != ServiceCheckScript && check.Type != ServiceCheckGRPC { - mErr.Errors = append(mErr.Errors, - fmt.Errorf("Check %s invalid: only script and gRPC checks should have tasks", check.Name)) - } if check.AddressMode == AddressModeDriver { mErr.Errors = append(mErr.Errors, fmt.Errorf("Check %q invalid: cannot use address_mode=\"driver\", only checks defined in a \"task\" service block can use this mode", service.Name)) } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index eeabbc04b..fdc174aa9 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -1278,9 +1278,6 @@ func TestTaskGroup_Validate(t *testing.T) { expected = `Check check-a invalid: refers to non-existent task task-b` require.Contains(t, err.Error(), expected) - expected = `Check check-a invalid: only script and gRPC checks should have tasks` - require.Contains(t, err.Error(), expected) - tg = &TaskGroup{ Name: "group-a", Services: []*Service{ diff --git a/website/content/docs/job-specification/check.mdx b/website/content/docs/job-specification/check.mdx index 62a32764a..70f8bf04e 100644 --- a/website/content/docs/job-specification/check.mdx +++ b/website/content/docs/job-specification/check.mdx @@ -134,11 +134,11 @@ job "example" { - `protocol` `(string: "http")` - Specifies the protocol for the http-based health checks. Valid options are `http` and `https`. -- `task` `(string: )` - Specifies the task associated with this +- `task` `(string: "")` - Specifies the task associated with this check. Scripts are executed within the task's environment, and - `check_restart` stanzas will apply to the specified task. For `checks` on group - level `services` only. Inherits the [`service.task`][service_task] value if not - set. May only be set for script or gRPC checks. + `check_restart` stanzas will apply to the specified task. Inherits + the [`service.task`][service_task] value if not set. Must be unset + or equivelent to `service.task` in task-level services. - `timeout` `(string: )` - Specifies how long to wait for a health check query to succeed. This is specified using a label suffix like "30s" or "1h". This