nova: floating ip errors

nova: floating ip errors

Adds control points for addFloatingIP so the response can be tweaked
and error conditions tested. Added two errors - NoMoreFloatingIPs and
IPLimitExceeded, and added tests to handle them.

This is closely related to the other CL in juju-core:
https://codereview.appspot.com/7226068/

https://code.launchpad.net/~dimitern/goose/nova-floating-ips-errors/+merge/146182

(do not edit description out of merge proposal)

[dimitern, 2013-02-01 17:28:19]

Please take a look.

[rog, 2013-02-01 18:11:21]

LGTM.
i have misgivings about the approach to errors here, but that's nothing new :-)
if you can act on the below remarks, so much the better.

https://codereview.appspot.com/7265043/diff/1/nova/local_test.go
File nova/local_test.go (right):

https://codereview.appspot.com/7265043/diff/1/nova/local_test.go#newcode142
nova/local_test.go:142: return &testservices.NoMoreFloatingIPs{fmt.Errorf("zero
floating ips available")}
aside: it seems odd that these very specific error types don't know how to
describe themselves.

https://codereview.appspot.com/7265043/diff/1/testservices/service.go
File testservices/service.go (right):

https://codereview.appspot.com/7265043/diff/1/testservices/service.go#newcode35
testservices/service.go:35: type NoMoreFloatingIPs struct {
i don't see why these embed an error, or even why they're types and not
variables.

can't they just be variables like this?

var NoMoreFloatingIPs = errors.New("no floating ips available")

[dimitern, 2013-02-04 15:53:13]

Please take a look.

https://codereview.appspot.com/7265043/diff/1/nova/local_test.go
File nova/local_test.go (right):

https://codereview.appspot.com/7265043/diff/1/nova/local_test.go#newcode142
nova/local_test.go:142: return &testservices.NoMoreFloatingIPs{fmt.Errorf("zero
floating ips available")}
On 2013/02/01 18:11:21, rog wrote:
> aside: it seems odd that these very specific error types don't know how to
> describe themselves.

Yeah, it's tedious, but a lot of error handling in goose is now. Probably a
general refactoring or error handling is in order.

https://codereview.appspot.com/7265043/diff/1/testservices/service.go
File testservices/service.go (right):

https://codereview.appspot.com/7265043/diff/1/testservices/service.go#newcode35
testservices/service.go:35: type NoMoreFloatingIPs struct {
On 2013/02/01 18:11:21, rog wrote:
> i don't see why these embed an error, or even why they're types and not
> variables.
> 
> can't they just be variables like this?
> 
> var NoMoreFloatingIPs = errors.New("no floating ips available")
> 

Good point! Done.

[rog, 2013-02-04 15:57:32]

On 2013/02/04 15:53:13, dimitern wrote:
> Please take a look.
> 
> https://codereview.appspot.com/7265043/diff/1/nova/local_test.go
> File nova/local_test.go (right):
> 
> https://codereview.appspot.com/7265043/diff/1/nova/local_test.go#newcode142
> nova/local_test.go:142: return
&testservices.NoMoreFloatingIPs{fmt.Errorf("zero
> floating ips available")}
> On 2013/02/01 18:11:21, rog wrote:
> > aside: it seems odd that these very specific error types don't know how to
> > describe themselves.
> 
> Yeah, it's tedious, but a lot of error handling in goose is now. Probably a
> general refactoring or error handling is in order.
> 
> https://codereview.appspot.com/7265043/diff/1/testservices/service.go
> File testservices/service.go (right):
> 
>
https://codereview.appspot.com/7265043/diff/1/testservices/service.go#newcode35
> testservices/service.go:35: type NoMoreFloatingIPs struct {
> On 2013/02/01 18:11:21, rog wrote:
> > i don't see why these embed an error, or even why they're types and not
> > variables.
> > 
> > can't they just be variables like this?
> > 
> > var NoMoreFloatingIPs = errors.New("no floating ips available")
> > 
> 
> Good point! Done.

LGTM

[fwereade, 2013-02-04 15:59:14]

LGTM

[dimitern, 2013-02-04 16:11:27]

Submitted through Goose bot (we should link it with lbox)