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

api: Add Notes field to service checks #22397

Merged
merged 7 commits into from
Jun 10, 2024
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
3 changes: 3 additions & 0 deletions .changelog/22397.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
api: Add support for setting Notes field for Consul health checks
```
1 change: 1 addition & 0 deletions api/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ type ServiceCheck struct {
Interval time.Duration `hcl:"interval,optional"`
Timeout time.Duration `hcl:"timeout,optional"`
InitialStatus string `mapstructure:"initial_status" hcl:"initial_status,optional"`
Notes string `hcl:"notes,optional"`
TLSServerName string `mapstructure:"tls_server_name" hcl:"tls_server_name,optional"`
TLSSkipVerify bool `mapstructure:"tls_skip_verify" hcl:"tls_skip_verify,optional"`
Header map[string][]string `hcl:"header,block"`
Expand Down
1 change: 1 addition & 0 deletions client/taskenv/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func InterpolateService(taskEnv *TaskEnv, origService *structs.Service) *structs
check.Protocol = taskEnv.ReplaceEnv(check.Protocol)
check.PortLabel = taskEnv.ReplaceEnv(check.PortLabel)
check.InitialStatus = taskEnv.ReplaceEnv(check.InitialStatus)
check.Notes = taskEnv.ReplaceEnv(check.Notes)
check.Method = taskEnv.ReplaceEnv(check.Method)
check.GRPCService = taskEnv.ReplaceEnv(check.GRPCService)
check.Header = interpolateMapStringSliceString(taskEnv, check.Header)
Expand Down
3 changes: 3 additions & 0 deletions client/taskenv/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ func TestInterpolateServices(t *testing.T) {
Protocol: "${checkproto}",
PortLabel: "${checklabel}",
InitialStatus: "${checkstatus}",
Notes: "${checknotes}",
Method: "${checkmethod}",
Header: map[string][]string{
"${checkheaderk}": {"${checkheaderv}"},
Expand Down Expand Up @@ -71,6 +72,7 @@ func TestInterpolateServices(t *testing.T) {
"checkproto": "checkproto",
"checklabel": "checklabel",
"checkstatus": "checkstatus",
"checknotes": "checknotes",
"checkmethod": "checkmethod",
"checkheaderk": "checkheaderk",
"checkheaderv": "checkheaderv",
Expand Down Expand Up @@ -104,6 +106,7 @@ func TestInterpolateServices(t *testing.T) {
Protocol: "checkproto",
PortLabel: "checklabel",
InitialStatus: "checkstatus",
Notes: "checknotes",
Method: "checkmethod",
Header: map[string][]string{
"checkheaderk": {"checkheaderv"},
Expand Down
1 change: 1 addition & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -1624,6 +1624,7 @@ func ApiServicesToStructs(in []*api.Service, group bool) []*structs.Service {
Interval: check.Interval,
Timeout: check.Timeout,
InitialStatus: check.InitialStatus,
Notes: check.Notes,
TLSServerName: check.TLSServerName,
TLSSkipVerify: check.TLSSkipVerify,
Header: check.Header,
Expand Down
4 changes: 4 additions & 0 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2729,6 +2729,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Interval: 4 * time.Second,
Timeout: 2 * time.Second,
InitialStatus: "ok",
Notes: "this is a check",
CheckRestart: &api.CheckRestart{
Limit: 3,
IgnoreWarnings: true,
Expand Down Expand Up @@ -2838,6 +2839,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Interval: 4 * time.Second,
Timeout: 2 * time.Second,
InitialStatus: "ok",
Notes: "this is a check",
SuccessBeforePassing: 3,
FailuresBeforeCritical: 4,
FailuresBeforeWarning: 2,
Expand Down Expand Up @@ -3163,6 +3165,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Interval: 4 * time.Second,
Timeout: 2 * time.Second,
InitialStatus: "ok",
Notes: "this is a check",
CheckRestart: &structs.CheckRestart{
Grace: 11 * time.Second,
Limit: 3,
Expand Down Expand Up @@ -3274,6 +3277,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
Interval: 4 * time.Second,
Timeout: 2 * time.Second,
InitialStatus: "ok",
Notes: "this is a check",
GRPCService: "foo.Bar",
GRPCUseTLS: true,
SuccessBeforePassing: 3,
Expand Down
1 change: 1 addition & 0 deletions jobspec/parse_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,7 @@ func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error {
"command",
"args",
"initial_status",
"notes",
"tls_skip_verify",
"header",
"method",
Expand Down
14 changes: 14 additions & 0 deletions nomad/structs/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3838,6 +3838,12 @@ func TestTaskGroupDiff(t *testing.T) {
Old: "foo",
New: "foo",
},
{
Type: DiffTypeNone,
Name: "Notes",
Old: "",
New: "",
},
{
Type: DiffTypeNone,
Name: "OnUpdate",
Expand Down Expand Up @@ -7607,6 +7613,7 @@ func TestTaskDiff(t *testing.T) {
Interval: 1 * time.Second,
Timeout: 1 * time.Second,
InitialStatus: "critical",
Notes: "a note",
Header: map[string][]string{
"Foo": {"bar"},
},
Expand Down Expand Up @@ -7635,6 +7642,7 @@ func TestTaskDiff(t *testing.T) {
Interval: 1 * time.Second,
Timeout: 1 * time.Second,
InitialStatus: "passing",
Notes: "another note",
Method: "POST",
Header: map[string][]string{
"Foo": {"bar", "baz"},
Expand Down Expand Up @@ -7788,6 +7796,12 @@ func TestTaskDiff(t *testing.T) {
Old: "foo",
New: "foo",
},
{
Type: DiffTypeEdited,
Name: "Notes",
Old: "a note",
New: "another note",
},
{
Type: DiffTypeEdited,
Name: "OnUpdate",
Expand Down
11 changes: 11 additions & 0 deletions nomad/structs/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ type ServiceCheck struct {
Interval time.Duration // Interval of the check
Timeout time.Duration // Timeout of the response from the check before consul fails the check
InitialStatus string // Initial status of the check
Notes string // Specifies arbitrary information for humans. This is not used by Consul internally
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we could add a little validation on this field? Maybe we could at least limit the length of notes to 250 characters? There's already a method for validating Consul fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, pushed a fix

TLSServerName string // ServerName to use for SNI and TLS verification when (Type=https and Protocol=https) or (Type=grpc and GRPCUseTLS=true)
TLSSkipVerify bool // Skip TLS verification when (type=https and Protocol=https) or (type=grpc and grpc_use_tls=true)
Method string // HTTP Method to use (GET by default)
Expand Down Expand Up @@ -161,6 +162,10 @@ func (sc *ServiceCheck) Equal(o *ServiceCheck) bool {
return false
}

if sc.Notes != o.Notes {
return false
}

if sc.Interval != o.Interval {
return false
}
Expand Down Expand Up @@ -454,6 +459,11 @@ func (sc *ServiceCheck) validateConsul() error {
return fmt.Errorf("failures_before_warning not supported for check of type %q", sc.Type)
}

// Arbitrary value, we could bump it if needed
if len(sc.Notes) > 255 {
return fmt.Errorf("notes must not be longer than 255 characters")
}

return nil
}

Expand Down Expand Up @@ -492,6 +502,7 @@ func (sc *ServiceCheck) Hash(serviceID string) string {
hashString(h, sc.Method)
hashString(h, sc.Body)
hashString(h, sc.OnUpdate)
hashString(h, sc.Notes)

// use name "true" to maintain ID stability
hashBool(h, sc.TLSSkipVerify, "true")
Expand Down
21 changes: 21 additions & 0 deletions nomad/structs/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package structs

import (
"errors"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -1992,6 +1993,26 @@ func TestService_Validate(t *testing.T) {
},
expErr: false,
},
{
name: "provider consul with notes too long",
input: &Service{
Name: "testservice",
Provider: "consul",
PortLabel: "port",
Checks: []*ServiceCheck{
{
Name: "servicecheck",
Type: "http",
Path: "/",
Interval: 1 * time.Second,
Timeout: 3 * time.Second,
Notes: strings.Repeat("A", 256),
},
},
},
expErr: true,
expErrStr: "notes must not be longer than 255 characters",
},
}

for _, tc := range testCases {
Expand Down
Loading