From 30903db4421530c5c18686e9768bb1e37feaa4d3 Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Fri, 12 Mar 2021 09:40:49 -0500 Subject: [PATCH] AutopilotServerHealth now handles the 429 status code (#8599) AutopilotServerHealthy now handles the 429 status code Previously we would error out and not parse the response. Now either a 200 or 429 status code are considered expected statuses and will result in the method returning the reply allowing API consumers to not only see if the system is healthy or not but which server is unhealthy. --- .changelog/8599.txt | 3 ++ api/go.sum | 1 + api/mock_api_test.go | 82 ++++++++++++++++++++++++++++++++++ api/operator_autopilot.go | 15 ++++++- api/operator_autopilot_test.go | 59 ++++++++++++++++++++++++ 5 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 .changelog/8599.txt create mode 100644 api/mock_api_test.go diff --git a/.changelog/8599.txt b/.changelog/8599.txt new file mode 100644 index 0000000000..ef289adefc --- /dev/null +++ b/.changelog/8599.txt @@ -0,0 +1,3 @@ +```release-note:improvement +api: `AutopilotServerHelath` now handles the 429 status code returned by the v1/operator/autopilot/health endpoint and still returned the parsed reply which will indicate server healthiness +``` diff --git a/api/go.sum b/api/go.sum index 57ef543992..0ee1dc2cd3 100644 --- a/api/go.sum +++ b/api/go.sum @@ -83,6 +83,7 @@ github.com/posener/complete v1.2.3/go.mod h1:WZIdtGGp+qx0sLrYKtIRAruyNpv6hFCicSg github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529 h1:nn5Wsu0esKSJiIVhscUtVbo7ada43DJhG55ua/hjS5I= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc= +github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= diff --git a/api/mock_api_test.go b/api/mock_api_test.go new file mode 100644 index 0000000000..dc4de0a3e4 --- /dev/null +++ b/api/mock_api_test.go @@ -0,0 +1,82 @@ +package api + +import ( + "encoding/json" + "io" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type mockAPI struct { + ts *httptest.Server + t *testing.T + mock.Mock +} + +func setupMockAPI(t *testing.T) (*mockAPI, *Client) { + mapi := mockAPI{t: t} + mapi.Test(t) + mapi.ts = httptest.NewServer(&mapi) + t.Cleanup(func() { + mapi.ts.Close() + mapi.Mock.AssertExpectations(t) + }) + + cfg := DefaultConfig() + cfg.Address = mapi.ts.URL + + client, err := NewClient(cfg) + require.NoError(t, err) + return &mapi, client +} + +func (m *mockAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { + var body interface{} + + if r.Body != nil { + bodyBytes, err := ioutil.ReadAll(r.Body) + if err == nil && len(bodyBytes) > 0 { + body = bodyBytes + + var bodyMap map[string]interface{} + if err := json.Unmarshal(bodyBytes, &bodyMap); err != nil { + body = bodyMap + } + } + } + + ret := m.Called(r.Method, r.URL.Path, body) + + if replyFn, ok := ret.Get(0).(func(http.ResponseWriter, *http.Request)); ok { + replyFn(w, r) + return + } +} + +func (m *mockAPI) static(method string, path string, body interface{}) *mock.Call { + return m.On("ServeHTTP", method, path, body) +} + +func (m *mockAPI) withReply(method, path string, body interface{}, status int, reply interface{}) *mock.Call { + return m.static(method, path, body).Return(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(status) + + if reply == nil { + return + } + + rdr, ok := reply.(io.Reader) + if ok { + io.Copy(w, rdr) + return + } + + enc := json.NewEncoder(w) + require.NoError(m.t, enc.Encode(reply)) + }) +} diff --git a/api/operator_autopilot.go b/api/operator_autopilot.go index 57876ee9f6..92e2c91cad 100644 --- a/api/operator_autopilot.go +++ b/api/operator_autopilot.go @@ -334,10 +334,23 @@ func (op *Operator) AutopilotCASConfiguration(conf *AutopilotConfiguration, q *W func (op *Operator) AutopilotServerHealth(q *QueryOptions) (*OperatorHealthReply, error) { r := op.c.newRequest("GET", "/v1/operator/autopilot/health") r.setQueryOptions(q) - _, resp, err := requireOK(op.c.doRequest(r)) + + // we cannot just use requireOK because this endpoint might use a 429 status to indicate + // that unhealthiness + _, resp, err := op.c.doRequest(r) if err != nil { + if resp != nil { + resp.Body.Close() + } return nil, err } + + // these are the only 2 status codes that would indicate that we should + // expect the body to contain the right format. + if resp.StatusCode != 200 && resp.StatusCode != 429 { + return nil, generateUnexpectedResponseCodeError(resp) + } + defer resp.Body.Close() var out OperatorHealthReply diff --git a/api/operator_autopilot_test.go b/api/operator_autopilot_test.go index b1b034cc57..720ae29413 100644 --- a/api/operator_autopilot_test.go +++ b/api/operator_autopilot_test.go @@ -2,9 +2,11 @@ package api import ( "testing" + "time" "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/consul/sdk/testutil/retry" + "github.com/stretchr/testify/require" ) func TestAPI_OperatorAutopilotGetSetConfiguration(t *testing.T) { @@ -123,3 +125,60 @@ func TestAPI_OperatorAutopilotState(t *testing.T) { } }) } + +func TestAPI_OperatorAutopilotServerHealth_429(t *testing.T) { + mapi, client := setupMockAPI(t) + + reply := OperatorHealthReply{ + Healthy: false, + FailureTolerance: 0, + Servers: []ServerHealth{ + { + ID: "d9fdded2-27ae-4db2-9232-9d8d0114ac98", + Name: "foo", + Address: "198.18.0.1:8300", + SerfStatus: "alive", + Version: "1.8.3", + Leader: true, + LastContact: NewReadableDuration(0), + LastTerm: 4, + LastIndex: 99, + Healthy: true, + Voter: true, + StableSince: time.Date(2020, 9, 2, 12, 0, 0, 0, time.UTC), + }, + { + ID: "1bcdda01-b896-41bc-a763-1a62b4260777", + Name: "bar", + Address: "198.18.0.2:8300", + SerfStatus: "alive", + Version: "1.8.3", + Leader: false, + LastContact: NewReadableDuration(10 * time.Millisecond), + LastTerm: 4, + LastIndex: 99, + Healthy: true, + Voter: true, + StableSince: time.Date(2020, 9, 2, 12, 0, 0, 0, time.UTC), + }, + { + ID: "661d1eac-81be-436b-bfe1-d51ffd665b9d", + Name: "baz", + Address: "198.18.0.3:8300", + SerfStatus: "failed", + Version: "1.8.3", + Leader: false, + LastContact: NewReadableDuration(10 * time.Millisecond), + LastTerm: 4, + LastIndex: 99, + Healthy: false, + Voter: true, + }, + }, + } + mapi.withReply("GET", "/v1/operator/autopilot/health", nil, 429, reply).Once() + + out, err := client.Operator().AutopilotServerHealth(nil) + require.NoError(t, err) + require.Equal(t, &reply, out) +}