Skip to content

Commit

Permalink
Merge pull request #101 from r00ta/lp-2034014-add-retry-policy-for-40…
Browse files Browse the repository at this point in the history
…9-responses

#101

This PR aims to retry on MAAS responses with status code equal to 409. 

In particular we observed that in some cases it is possible that the client calls MAAS endpoints in parallel and due to some MAAS internals the requests need to be remade by the clients. 

[Link to the original bug](https://bugs.launchpad.net/maas/+bug/2034014)

[Link to MAAS core merge proposal to add the `Retry-After` header in 409 responses](https://code.launchpad.net/~r00ta/maas/+git/maas/+merge/450867)
  • Loading branch information
jujubot authored Sep 11, 2023
2 parents 5277155 + 8b71f2b commit 7b1b159
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
2 changes: 1 addition & 1 deletion client.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (client Client) dispatchRequest(request *http.Request) ([]byte, error) {
// as instructed and retry the request.
if err != nil {
serverError, ok := errors.Cause(err).(ServerError)
if ok && serverError.StatusCode == http.StatusServiceUnavailable {
if ok && (serverError.StatusCode == http.StatusServiceUnavailable || serverError.StatusCode == http.StatusConflict) {
retryTimeInt, errConv := strconv.Atoi(serverError.Header.Get(RetryAfterHeaderName))
if errConv == nil {
select {
Expand Down
16 changes: 16 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,22 @@ func (suite *ClientSuite) TestClientDispatchRequestRetries503(c *gc.C) {
c.Check(*server.requests, jc.DeepEquals, expectedRequestsContent)
}

func (suite *ClientSuite) TestClientDispatchRequestRetries409(c *gc.C) {
URI := "/some/url/?param1=test"
server := newFlakyServer(URI, 409, NumberOfRetries)
defer server.Close()
client, err := NewAnonymousClient(server.URL, "1.0")
c.Assert(err, jc.ErrorIsNil)
content := "content"
request, err := http.NewRequest("GET", server.URL+URI, io.NopCloser(strings.NewReader(content)))
c.Assert(err, jc.ErrorIsNil)

_, err = client.dispatchRequest(request)

c.Assert(err, jc.ErrorIsNil)
c.Check(*server.nbRequests, gc.Equals, NumberOfRetries+1)
}

func (suite *ClientSuite) TestTLSClientDispatchRequestRetries503NilBody(c *gc.C) {
URI := "/some/path"
server := newFlakyTLSServer(URI, 503, NumberOfRetries)
Expand Down
2 changes: 1 addition & 1 deletion testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func newFlakyServer(uri string, code int, nbFlakyResponses int) *flakyServer {
errorMsg := fmt.Sprintf("Error 404: page not found (expected '%v', got '%v').", uri, request.URL.String())
http.Error(writer, errorMsg, http.StatusNotFound)
} else if nbRequests <= nbFlakyResponses {
if code == http.StatusServiceUnavailable {
if code == http.StatusServiceUnavailable || code == http.StatusConflict {
writer.Header().Set("Retry-After", "0")
}
writer.WriteHeader(code)
Expand Down

0 comments on commit 7b1b159

Please sign in to comment.