Actually, and I hope no one has gotten their hopes up here, this solution may 
be masking a bigger issue and I think we need to take a step back. The original 
reason for making this change was due to a bug I found when calling 
'ComputeService.createNodesInGroup' in that, if a user requested to have 
autoAssignedFloatingIP's for each of the nodes, it would fail throwing an NPE. 
So I think we can hold off this functionality for another PR and instead focus 
first on fixing the bug at hand. The problematic code is below which returns 
NULL and subsequently and NPE is thrown when trying to access the 'ip' 
reference:

        Class:  AllocateAndAddFloatingipToNode
        
                Ip = FloatingIPApi.create() //about line 83 or so. This can 
return NULL

However, you can use the same FloatingIPApi reference to call "list()" and it 
will successfully return all IP's available to be allocated. One would think 
that if the 'list()' call returns something than one of those IP's could be 
used in the 'create()' call ... but this is not the case. This code is 
obviously broken but there does not seem to be a clear/clean solution, without 
a good amount of code refactoring, to solve the problem. So a solution that 
I've proposed is to do the below:

                Ip = FloatingIPApi.create();
                If(ip == null)
                        throw new InsufficientResourcesException();

This has the added benefit of being caught and subsequent second attempt is 
made at IP allocation, by cycling through the "FloatingIPApi.list()" call, to 
acquire a floating IP. If after both of these attempts we still don't have an 
IP then re-throw the IRE. PR has been sent in here:

https://github.com/jclouds/jclouds/pull/425


-----Original Message-----
From: Ignasi Barrera [mailto:n...@apache.org] 
Sent: Monday, June 30, 2014 2:57 PM
To: dev@jclouds.apache.org
Subject: Re: On JIRA issue JCLOUDS-607

That would be fine by me.

What I wanted to highlight is that having a Set as a parameter may cause the 
impression that N ips will be allocated. At least it is the impression that Set 
would give to me :)

If the javadoc and name of the method properly reflect the behavior, then it is 
ok to me.
El 30/06/2014 19:09, "Everett Toews" <everett.to...@rackspace.com> escribió:

> On Jun 30, 2014, at 2:35 AM, Ignasi Barrera <n...@apache.org> wrote:
>
> > Regarding the proposed implementation, you'll be passing a Set but 
> > allocating only one Ip. Couldn't this cause confusion to users? 
> > Also, the Ip in the set that will actually be allocated is not predictable.
>
> Unless I’m mistaken, I believe the intention is to allocate 1 IP 
> address from any the Set of IP address pools passed in.
>
> It’s a way for the user to express that they need an IP address from 
> some pool. Because it’s a Set and the iterator order is guaranteed, I 
> think the Javadoc on the param for floatingIpPoolNames would need to 
> be something like
>
> “The IP Address is allocated from any one of the specified pools.”
>
> Or something like that.
>
> Everett

Reply via email to