Skip to content

Commit

Permalink
Merge pull request #5553 from hashicorp/txn-check-serialization
Browse files Browse the repository at this point in the history 
Use the correct check duration fields when converting transaction ops
  • Loading branch information
@kyhavlov
kyhavlov authored Apr 2, 2019
2 parents 1f67b0a + 716a20d commit c2da314
Show file tree
Hide file tree
Showing 2 changed files with 195 additions and 8 deletions.
35 changes: 27 additions & 8 deletions agent/txn_endpoint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"strings"
"time"

"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api"
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
168 changes: 168 additions & 0 deletions agent/txn_endpoint_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}

0 comments on commit c2da314

Please sign in to comment.