> @@ -94,6 +97,12 @@ public boolean apply(FloatingIP arg0) {
>           // try to prevent multiple parallel launches from choosing the same 
> ip.
>           Collections.shuffle(unassignedIps);
>           ip = Iterables.getLast(unassignedIps);
> +         
> +         //if we are still unable to allocate IP, even after iterating 
> through all 
> +         //available, then re-throw IRE as there is nothing left we can do
> +         if(ip == null){
> +         throw new InsufficientResourcesException("Failed to allocate a 
> FloatingIP for node(" + node.getId() + ")",e);
> +         }
>        }
>        logger.debug(">> adding floatingIp(%s) to node(%s)", ip.getIp(), 
> node.getId());

Looking at the overall code, we should be able to have the null check in one 
place only
```
      FloatingIP ip = null;
      try {
         logger.debug(">> allocating or reassigning floating ip for node(%s)", 
node.getId());         
         ip = floatingIpApi.create();
      } catch (InsufficientResourcesException e) {
         ...
         ip = Iterables.getLast(unassignedIps);
      }
      if (ip == null) {
         throw new InsufficientResourcesException(String.format("Failed to 
allocate a FloatingIP for node %s", node.getId()), e);
      }
      logger.debug(">> adding floatingIp(%s) to node(%s)", ip.getIp(), 
node.getId());
```
A question on the exception: does it get caught and turned into a 
RunNodesException higher up? This is the only exception the [interface 
method](https://github.com/jclouds/jclouds/blob/master/compute/src/main/java/org/jclouds/compute/ComputeService.java#L160)
 is declared as throwing, so if this is **not** caught and converted we 
probably should be throwing an RNE here.

@nacx: Any insight on that?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14408908

Reply via email to