From 716a20d8a68b01a4842b323a2ac72948f08cc3f6 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 26 Mar 2019 10:44:02 -0700 Subject: [PATCH] Re-add logic to handle the undocumented duration fields --- agent/txn_endpoint.go | 34 +++++++++++++++----- agent/txn_endpoint_test.go | 66 +++++++++++++++++++++++++++++++------- 2 files changed, 81 insertions(+), 19 deletions(-) diff --git a/agent/txn_endpoint.go b/agent/txn_endpoint.go index 664a34fd6f..eaad624065 100644 --- a/agent/txn_endpoint.go +++ b/agent/txn_endpoint.go @@ -221,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, @@ -235,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: time.Duration(check.Definition.Interval), - Timeout: time.Duration(check.Definition.Timeout), - DeregisterCriticalServiceAfter: time.Duration(check.Definition.DeregisterCriticalServiceAfter), + 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 2667f798be..7a84bb04f7 100644 --- a/agent/txn_endpoint_test.go +++ b/agent/txn_endpoint_test.go @@ -410,7 +410,8 @@ func TestTxnEndpoint_UpdateCheck(t *testing.T) { defer a.Shutdown() testrpc.WaitForTestAgent(t, a.RPC, "dc1") - // Make sure the fields of a check are handled correctly when both creating and updating. + // 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(` [ { @@ -456,9 +457,31 @@ func TestTxnEndpoint_UpdateCheck(t *testing.T) { } } } + }, + { + "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, a.config.NodeName, a.config.NodeName))) req, _ := http.NewRequest("PUT", "/v1/txn", buf) resp := httptest.NewRecorder() obj, err := a.srv.Txn(resp, req) @@ -473,7 +496,7 @@ func TestTxnEndpoint_UpdateCheck(t *testing.T) { if !ok { t.Fatalf("bad type: %T", obj) } - if len(txnResp.Results) != 2 { + if len(txnResp.Results) != 3 { t.Fatalf("bad: %v", txnResp) } index := txnResp.Results[0].Check.ModifyIndex @@ -487,11 +510,11 @@ func TestTxnEndpoint_UpdateCheck(t *testing.T) { Status: api.HealthCritical, Notes: "Http based health check", Definition: structs.HealthCheckDefinition{ - Interval: 6 * time.Second, - Timeout: 6 * time.Second, + Interval: 6 * time.Second, + Timeout: 6 * time.Second, DeregisterCriticalServiceAfter: 6 * time.Second, - HTTP: "http://localhost:8000", - TLSSkipVerify: true, + HTTP: "http://localhost:8000", + TLSSkipVerify: true, }, RaftIndex: structs.RaftIndex{ CreateIndex: index, @@ -508,11 +531,32 @@ func TestTxnEndpoint_UpdateCheck(t *testing.T) { Notes: "Http based health check", Output: "success", Definition: structs.HealthCheckDefinition{ - Interval: 10 * time.Second, - Timeout: 10 * time.Second, + Interval: 10 * time.Second, + Timeout: 10 * time.Second, DeregisterCriticalServiceAfter: 15 * time.Minute, - HTTP: "http://localhost:9000", - TLSSkipVerify: false, + 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,