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.
This commit is contained in:
Seth Hoenig 2022-10-10 13:02:33 -05:00 committed by GitHub
parent 5e38a0e82c
commit 69ced2a2bd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 7 additions and 11 deletions

3
.changelog/14864.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
services: Fixed a regression where check task validation stopped allowing some configurations
```

View File

@ -6681,10 +6681,6 @@ func (tg *TaskGroup) validateServices() error {
for _, check := range service.Checks { for _, check := range service.Checks {
if check.TaskName != "" { 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 { 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)) 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))
} }

View File

@ -1278,9 +1278,6 @@ func TestTaskGroup_Validate(t *testing.T) {
expected = `Check check-a invalid: refers to non-existent task task-b` expected = `Check check-a invalid: refers to non-existent task task-b`
require.Contains(t, err.Error(), expected) 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{ tg = &TaskGroup{
Name: "group-a", Name: "group-a",
Services: []*Service{ Services: []*Service{

View File

@ -134,11 +134,11 @@ job "example" {
- `protocol` `(string: "http")` - Specifies the protocol for the http-based - `protocol` `(string: "http")` - Specifies the protocol for the http-based
health checks. Valid options are `http` and `https`. health checks. Valid options are `http` and `https`.
- `task` `(string: <required>)` - 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. Scripts are executed within the task's environment, and
`check_restart` stanzas will apply to the specified task. For `checks` on group `check_restart` stanzas will apply to the specified task. Inherits
level `services` only. Inherits the [`service.task`][service_task] value if not the [`service.task`][service_task] value if not set. Must be unset
set. May only be set for script or gRPC checks. or equivelent to `service.task` in task-level services.
- `timeout` `(string: <required>)` - Specifies how long to wait for a health check - `timeout` `(string: <required>)` - Specifies how long to wait for a health check
query to succeed. This is specified using a label suffix like "30s" or "1h". This query to succeed. This is specified using a label suffix like "30s" or "1h". This