Merge lp://staging/~rvb/gomaasapi/gomaasapi-bug-1384001 into lp://staging/gomaasapi

Proposed by Raphaël Badin
Status: Merged
Approved by: Ian Booth
Approved revision: 63
Merged at revision: 58
Proposed branch: lp://staging/~rvb/gomaasapi/gomaasapi-bug-1384001
Merge into: lp://staging/gomaasapi
Diff against target: 195 lines (+137/-3)
3 files modified
client.go (+50/-3)
client_test.go (+50/-0)
testing.go (+37/-0)
To merge this branch: bzr merge lp://staging/~rvb/gomaasapi/gomaasapi-bug-1384001
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Juju Engineering Pending
Review via email: mp+239738@code.staging.launchpad.net

Commit message

Retry requests that result in a 503 response with the Retry-After header set.

Description of the change

To post a comment you must log in.
60. By Raphaël Badin

Lint fixes.

61. By Raphaël Badin

Update comment.

Revision history for this message
Gavin Panella (allenap) wrote :

My eyes are bleeding, but otherwise it seems okay.

review: Approve
62. By Raphaël Badin

Review fixes.

63. By Raphaël Badin

Retry more.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Small point of order - MAAS doesn't set the Retry-After header (yet).

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Ah my bad, saw the maas change you made after I had written that.

Revision history for this message
Andrew Wilkins (axwalk) wrote :

I think it'd be good to have an option to handle this outside of gomaasapi as well. In Juju, the provisioner runs in a single goroutine (there are good reasons for that) and so retrying a single request will block others. It'd be good to just reschedule the provisioning for the failed one.

For other operations (e.g. releasing all nodes) I think this behaviour would be fine.

64. By Raphaël Badin

Restore body before issuing request.

Revision history for this message
Ian Booth (wallyworld) wrote :

@Andrew,

The new behaviour in here now matches what goose does. When Openstack's rate limit is exceeded, a 403 response comes back and goose will retry after the time set by the retry-after header. This is independent of what request is being made. I think this is ok given that machine provisioning generally takes a while anyway, and adding in a few retries hopefully won't materially affect the total time. Not that there's not scope to discuss if this could be improved across the board.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: