diff --git a/agent/txn_endpoint.go b/agent/txn_endpoint.go index b471c8a954..eaad624065 100644 --- a/agent/txn_endpoint.go +++ b/agent/txn_endpoint.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "strings" + "time" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" @@ -220,6 +221,24 @@ func (s *HTTPServer) convertOps(resp http.ResponseWriter, req *http.Request) (st } check := in.Check.Check + + // Check if the internal duration fields are set as well as the normal ones. This is + // to be backwards compatible with a bug where the internal duration fields were being + // deserialized from instead of the correct fields. + // See https://github.com/hashicorp/consul/issues/5477 for more details. + interval := check.Definition.IntervalDuration + if dur := time.Duration(check.Definition.Interval); dur != 0 { + interval = dur + } + timeout := check.Definition.TimeoutDuration + if dur := time.Duration(check.Definition.Timeout); dur != 0 { + timeout = dur + } + deregisterCriticalServiceAfter := check.Definition.DeregisterCriticalServiceAfterDuration + if dur := time.Duration(check.Definition.DeregisterCriticalServiceAfter); dur != 0 { + deregisterCriticalServiceAfter = dur + } + out := &structs.TxnOp{ Check: &structs.TxnCheckOp{ Verb: in.Check.Verb, @@ -234,14 +253,14 @@ func (s *HTTPServer) convertOps(resp http.ResponseWriter, req *http.Request) (st ServiceName: check.ServiceName, ServiceTags: check.ServiceTags, Definition: structs.HealthCheckDefinition{ - HTTP: check.Definition.HTTP, - TLSSkipVerify: check.Definition.TLSSkipVerify, - Header: check.Definition.Header, - Method: check.Definition.Method, - TCP: check.Definition.TCP, - Interval: check.Definition.IntervalDuration, - Timeout: check.Definition.TimeoutDuration, - DeregisterCriticalServiceAfter: check.Definition.DeregisterCriticalServiceAfterDuration, + HTTP: check.Definition.HTTP, + TLSSkipVerify: check.Definition.TLSSkipVerify, + Header: check.Definition.Header, + Method: check.Definition.Method, + TCP: check.Definition.TCP, + Interval: interval, + Timeout: timeout, + DeregisterCriticalServiceAfter: deregisterCriticalServiceAfter, }, RaftIndex: structs.RaftIndex{ ModifyIndex: check.ModifyIndex, diff --git a/agent/txn_endpoint_test.go b/agent/txn_endpoint_test.go index a9bacd8597..7a84bb04f7 100644 --- a/agent/txn_endpoint_test.go +++ b/agent/txn_endpoint_test.go @@ -8,8 +8,11 @@ import ( "reflect" "strings" "testing" + "time" + "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/testrpc" + "github.com/pascaldekloe/goe/verify" "github.com/hashicorp/consul/agent/structs" ) @@ -400,3 +403,168 @@ func TestTxnEndpoint_KV_Actions(t *testing.T) { } }) } + +func TestTxnEndpoint_UpdateCheck(t *testing.T) { + t.Parallel() + a := NewTestAgent(t, t.Name(), "") + defer a.Shutdown() + testrpc.WaitForTestAgent(t, a.RPC, "dc1") + + // Make sure the fields of a check are handled correctly when both creating and + // updating, and test both sets of duration fields to ensure backwards compatibility. + buf := bytes.NewBuffer([]byte(fmt.Sprintf(` +[ + { + "Check": { + "Verb": "set", + "Check": { + "Node": "%s", + "CheckID": "nodecheck", + "Name": "Node http check", + "Status": "critical", + "Notes": "Http based health check", + "Output": "", + "ServiceID": "", + "ServiceName": "", + "Definition": { + "Interval": "6s", + "Timeout": "6s", + "DeregisterCriticalServiceAfter": "6s", + "HTTP": "http://localhost:8000", + "TLSSkipVerify": true + } + } + } + }, + { + "Check": { + "Verb": "set", + "Check": { + "Node": "%s", + "CheckID": "nodecheck", + "Name": "Node http check", + "Status": "passing", + "Notes": "Http based health check", + "Output": "success", + "ServiceID": "", + "ServiceName": "", + "Definition": { + "Interval": "10s", + "Timeout": "10s", + "DeregisterCriticalServiceAfter": "15m", + "HTTP": "http://localhost:9000", + "TLSSkipVerify": false + } + } + } + }, + { + "Check": { + "Verb": "set", + "Check": { + "Node": "%s", + "CheckID": "nodecheck", + "Name": "Node http check", + "Status": "passing", + "Notes": "Http based health check", + "Output": "success", + "ServiceID": "", + "ServiceName": "", + "Definition": { + "IntervalDuration": "15s", + "TimeoutDuration": "15s", + "DeregisterCriticalServiceAfterDuration": "30m", + "HTTP": "http://localhost:9000", + "TLSSkipVerify": false + } + } + } + } +] +`, a.config.NodeName, a.config.NodeName, a.config.NodeName))) + req, _ := http.NewRequest("PUT", "/v1/txn", buf) + resp := httptest.NewRecorder() + obj, err := a.srv.Txn(resp, req) + if err != nil { + t.Fatalf("err: %v", err) + } + if resp.Code != 200 { + t.Fatalf("expected 200, got %d", resp.Code) + } + + txnResp, ok := obj.(structs.TxnResponse) + if !ok { + t.Fatalf("bad type: %T", obj) + } + if len(txnResp.Results) != 3 { + t.Fatalf("bad: %v", txnResp) + } + index := txnResp.Results[0].Check.ModifyIndex + expected := structs.TxnResponse{ + Results: structs.TxnResults{ + &structs.TxnResult{ + Check: &structs.HealthCheck{ + Node: a.config.NodeName, + CheckID: "nodecheck", + Name: "Node http check", + Status: api.HealthCritical, + Notes: "Http based health check", + Definition: structs.HealthCheckDefinition{ + Interval: 6 * time.Second, + Timeout: 6 * time.Second, + DeregisterCriticalServiceAfter: 6 * time.Second, + HTTP: "http://localhost:8000", + TLSSkipVerify: true, + }, + RaftIndex: structs.RaftIndex{ + CreateIndex: index, + ModifyIndex: index, + }, + }, + }, + &structs.TxnResult{ + Check: &structs.HealthCheck{ + Node: a.config.NodeName, + CheckID: "nodecheck", + Name: "Node http check", + Status: api.HealthPassing, + Notes: "Http based health check", + Output: "success", + Definition: structs.HealthCheckDefinition{ + Interval: 10 * time.Second, + Timeout: 10 * time.Second, + DeregisterCriticalServiceAfter: 15 * time.Minute, + HTTP: "http://localhost:9000", + TLSSkipVerify: false, + }, + RaftIndex: structs.RaftIndex{ + CreateIndex: index, + ModifyIndex: index, + }, + }, + }, + &structs.TxnResult{ + Check: &structs.HealthCheck{ + Node: a.config.NodeName, + CheckID: "nodecheck", + Name: "Node http check", + Status: api.HealthPassing, + Notes: "Http based health check", + Output: "success", + Definition: structs.HealthCheckDefinition{ + Interval: 15 * time.Second, + Timeout: 15 * time.Second, + DeregisterCriticalServiceAfter: 30 * time.Minute, + HTTP: "http://localhost:9000", + TLSSkipVerify: false, + }, + RaftIndex: structs.RaftIndex{ + CreateIndex: index, + ModifyIndex: index, + }, + }, + }, + }, + } + verify.Values(t, "", txnResp, expected) +}