From ffcb72bfe38acfea7facd70ddecabe74c35a7d24 Mon Sep 17 00:00:00 2001 From: nicoche <78445450+nicoche@users.noreply.github.com> Date: Mon, 10 Jun 2024 16:59:49 +0200 Subject: [PATCH] api: Add Notes field to service checks (#22397) Co-authored-by: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com> --- .changelog/22397.txt | 3 +++ api/services.go | 1 + client/taskenv/services.go | 1 + client/taskenv/services_test.go | 3 +++ command/agent/job_endpoint.go | 1 + command/agent/job_endpoint_test.go | 4 ++++ jobspec/parse_service.go | 1 + nomad/structs/diff_test.go | 14 ++++++++++++++ nomad/structs/services.go | 11 +++++++++++ nomad/structs/services_test.go | 21 +++++++++++++++++++++ 10 files changed, 60 insertions(+) create mode 100644 .changelog/22397.txt diff --git a/.changelog/22397.txt b/.changelog/22397.txt new file mode 100644 index 000000000..68b93b022 --- /dev/null +++ b/.changelog/22397.txt @@ -0,0 +1,3 @@ +```release-note:improvement +api: Add support for setting Notes field for Consul health checks +``` diff --git a/api/services.go b/api/services.go index e8f25e19a..149a83ddf 100644 --- a/api/services.go +++ b/api/services.go @@ -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"` diff --git a/client/taskenv/services.go b/client/taskenv/services.go index ddbc61376..a4e3241e9 100644 --- a/client/taskenv/services.go +++ b/client/taskenv/services.go @@ -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) diff --git a/client/taskenv/services_test.go b/client/taskenv/services_test.go index 69bb418e6..2995e9ff8 100644 --- a/client/taskenv/services_test.go +++ b/client/taskenv/services_test.go @@ -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}"}, @@ -71,6 +72,7 @@ func TestInterpolateServices(t *testing.T) { "checkproto": "checkproto", "checklabel": "checklabel", "checkstatus": "checkstatus", + "checknotes": "checknotes", "checkmethod": "checkmethod", "checkheaderk": "checkheaderk", "checkheaderv": "checkheaderv", @@ -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"}, diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index 63c9c43e0..53d1c0f37 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -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, diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 7f3b365be..f06c5767e 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index a9e3f2684..3d8504cd1 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -1224,6 +1224,7 @@ func parseChecks(service *api.Service, checkObjs *ast.ObjectList) error { "command", "args", "initial_status", + "notes", "tls_skip_verify", "header", "method", diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 05dc11b8b..9c8747b49 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -3838,6 +3838,12 @@ func TestTaskGroupDiff(t *testing.T) { Old: "foo", New: "foo", }, + { + Type: DiffTypeNone, + Name: "Notes", + Old: "", + New: "", + }, { Type: DiffTypeNone, Name: "OnUpdate", @@ -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"}, }, @@ -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"}, @@ -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", diff --git a/nomad/structs/services.go b/nomad/structs/services.go index a4e1b0c46..c155f6222 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -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 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) @@ -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 } @@ -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 } @@ -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") diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go index 338261939..28691dc75 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -5,6 +5,7 @@ package structs import ( "errors" + "strings" "testing" "time" @@ -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 {