Skip to content

Commit

Permalink
services: remove assertion on 'task' field being set
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
shoenig committed Oct 10, 2022
1 parent fed3298 commit 589b6d5
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 11 deletions.
3 changes: 3 additions & 0 deletions .changelog/14864.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
services: Fixed a regression where check task validation stopped allowing some configurations
```
4 changes: 0 additions & 4 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
3 changes: 0 additions & 3 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
8 changes: 4 additions & 4 deletions website/content/docs/job-specification/check.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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: <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_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: <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
Expand Down

0 comments on commit 589b6d5

Please sign in to comment.