Rewrite rate limit retry tests

Rewrite rate limit retry tests

This branch uses the new service double control hooks introduced in the previous branch to rewrite the tests which check that rate limit exceeded retries are handled properly.
The kludge used previously to induce a retry error has been removed, and now an additional test can also be easily added to check the behaviour if too many rate limit retry responses are received.

So that the tests run fast, I've allowed for the Retry-After header value to be a float (even though a real instance only assigns an int). This means the tests can specify a really
short retry time (I used 1ms).

I've also re-added a rate limit retry test for use with the live instance, but improved it so that it exists as soon as the first rate limit exceeded response is received.

https://code.launchpad.net/~wallyworld/goose/rate-limit-retry-tests/+merge/144849

Requires: https://code.launchpad.net/~wallyworld/goose/control-test-doubles/+merge/144838

(do not edit description out of merge proposal)

[wallyworld, 2013-01-25 05:27:35]

Please take a look.

[jameinel, 2013-01-28 06:44:56]

Overall LGTM.

https://codereview.appspot.com/7200049/diff/1/http/client.go
File http/client.go (right):

https://codereview.appspot.com/7200049/diff/1/http/client.go#newcode195
http/client.go:195: for i := 0; i <= c.maxRetries; i++ {
<= ? Doesn't that mean we will try 4 times instead of 3?

https://codereview.appspot.com/7200049/diff/1/identity/userpass.go
File identity/userpass.go (right):

https://codereview.appspot.com/7200049/diff/1/identity/userpass.go#newcode109
identity/userpass.go:109: }
I think I have a more complete fix for this:
https://code.launchpad.net/~jameinel/goose/handle-missing-endpoint/+merge/145098

https://codereview.appspot.com/7200049/diff/1/nova/live_test.go
File nova/live_test.go (right):

https://codereview.appspot.com/7200049/diff/1/nova/live_test.go#newcode452
nova/live_test.go:452: return
Is it possible to use your hooks to check for a retry request, rather than
reading the output string? (I realize this probably didn't exist before, but it
seems like a really good use of it.)

https://codereview.appspot.com/7200049/diff/1/testservices/novaservice/servic...
File testservices/novaservice/service_http.go (right):

https://codereview.appspot.com/7200049/diff/1/testservices/novaservice/servic...
testservices/novaservice/service_http.go:216: // Retry-After is usually an int
but we don't want the tests taking too long to run.
Maybe: RFC says that Retry-After should be an int, but we don't want to wait an
entire second during the test suite.

[wallyworld, 2013-01-29 01:44:40]

Please take a look.

https://codereview.appspot.com/7200049/diff/1/http/client.go
File http/client.go (right):

https://codereview.appspot.com/7200049/diff/1/http/client.go#newcode195
http/client.go:195: for i := 0; i <= c.maxRetries; i++ {
On 2013/01/28 06:44:57, jameinel wrote:
> <= ? Doesn't that mean we will try 4 times instead of 3?

The logic was confusing retries with total attempts. Using "retries"
terminology, the client sends the first request and retries X times. With
"sendAttempts" terminology, the client tries "sendAttempts" requests in total. 

I've change the variable and constant to be maxSendAttempts to remove the
ambiguity.

https://codereview.appspot.com/7200049/diff/1/identity/userpass.go
File identity/userpass.go (right):

https://codereview.appspot.com/7200049/diff/1/identity/userpass.go#newcode109
identity/userpass.go:109: }
On 2013/01/28 06:44:57, jameinel wrote:
> I think I have a more complete fix for this:
>
https://code.launchpad.net/%7Ejameinel/goose/handle-missing-endpoint/+merge/1...

I had a quick look, the mp was missing the pre-req branch so was very large.
Looked ok at first glance. Perhaps we can land this branch now as is and follow
up with yours?

https://codereview.appspot.com/7200049/diff/1/nova/live_test.go
File nova/live_test.go (right):

https://codereview.appspot.com/7200049/diff/1/nova/live_test.go#newcode452
nova/live_test.go:452: return
On 2013/01/28 06:44:57, jameinel wrote:
> Is it possible to use your hooks to check for a retry request, rather than
> reading the output string? (I realize this probably didn't exist before, but
it
> seems like a really good use of it.)

Possibly, but the hook stuff has been set up to work with the test doubles
(manipulates the state of the test service back end) rather than a real live
instance. The nova client implementation would need to be retooled to implement
the ServiceControl interface. Perhaps this can be done in a followup branch.

https://codereview.appspot.com/7200049/diff/1/testservices/novaservice/servic...
File testservices/novaservice/service_http.go (right):

https://codereview.appspot.com/7200049/diff/1/testservices/novaservice/servic...
testservices/novaservice/service_http.go:216: // Retry-After is usually an int
but we don't want the tests taking too long to run.
On 2013/01/28 06:44:57, jameinel wrote:
> Maybe: RFC says that Retry-After should be an int, but we don't want to wait
an
> entire second during the test suite.

Done.

[jameinel, 2013-01-29 11:58:48]

LGTM

[wallyworld, 2013-01-29 13:23:52]

*** Submitted:

Rewrite rate limit retry tests

This branch uses the new service double control hooks introduced in the previous
branch to rewrite the tests which check that rate limit exceeded retries are
handled properly.
The kludge used previously to induce a retry error has been removed, and now an
additional test can also be easily added to check the behaviour if too many rate
limit retry responses are received.

So that the tests run fast, I've allowed for the Retry-After header value to be
a float (even though a real instance only assigns an int). This means the tests
can specify a really
short retry time (I used 1ms).

I've also re-added a rate limit retry test for use with the live instance, but
improved it so that it exists as soon as the first rate limit exceeded response
is received.

R=jameinel
CC=
https://codereview.appspot.com/7200049