You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Christopher Dancy <no...@github.com> on 2014/06/30 22:08:32 UTC
[jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE
on FloatingIPA... (#425)
Addresses possible returned null value from 'FloatingIPApi.create()'. If upon check we find null then throw InsufficientResourcesException. This in turn will make a second check at IP allocation by iterating through 'FloatingIPApi.list()' and grabbing an available IP. If this subsequent call fails then re-throw InsufficientResourcesException.
You can merge this Pull Request by running:
git pull https://github.com/cdancy/jclouds JCLOUDS-607
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/425
-- Commit Summary --
* JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPApi.create()
-- File Changes --
M apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNode.java (13)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/425.patch
https://github.com/jclouds/jclouds/pull/425.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #967](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/967/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48058547
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
>jclouds-java-7-pull-requests #1425 UNSTABLE
Unrelated test failure.
Thanks for the fixes! Code lgtm. @demobox good to go once squashed and rebased?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47717364
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
NP! I'll do later today.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48363551
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1338](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1338/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48251436
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -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());
Uops, just had the page opened for a while and just seen your comment after refreshing :) Thanks @cdancy!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14410501
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
i know @nacx is in the same boat and while I think in theory the design is not bad practice, and has its use-cases, I'm willing to concede that in this context it may not be the best fit. I'll revert now and lets see where that gets us.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48074096
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> @@ -68,41 +71,83 @@ public AllocateAndAddFloatingIpToNode(@Named(TIMEOUT_NODE_RUNNING) Predicate<Ato
> }
>
> @Override
> - public AtomicReference<NodeMetadata> apply(AtomicReference<NodeMetadata> input) {
> - checkState(nodeRunning.apply(input), "node never achieved state running %s", input.get());
> - NodeMetadata node = input.get();
> + public AtomicReference<NodeMetadata> apply(AtomicReference<NodeAndNovaTemplateOptions> input) {
> + checkState(nodeRunning.apply(input.get().getNodeMetadata()), "node never achieved state running %s", input.get().getNodeMetadata());
> + NodeMetadata node = input.get().getNodeMetadata().get();
I think `NodeAndNovaTemplateOptions` may be missing, but is there any reason it takes two AtomicRefs, rather than simply a Node and a NovaTemplateOptions object? I think having the NodeAndNovaTemplateOptions object in an AtomicRef should be enough?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14536682
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -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());
@cdancy I mean, it should be already caught in an upper jclouds layer. It would be great if you could just run a test and confirm that the final exception being thrown is already a RunNodesException.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14410382
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
Is `NodeAndNovaTemplateOptions` missing from the PR?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47981723
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1425](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1425/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47716831
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
Committed to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=3659a5f). Does this need to be backported to 1.7.x?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48255973
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1436](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1436/) ABORTED
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48077363
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> ip = floatingIpApi.create();
> + if(ip == null){
> + throw new InsufficientResourcesException();
> + }
Please use a 3-space indent and spaces around the condition:
```
if (ip == null) {
throw new...
}
```
Also, if the IRE allows you to pass a message, please pass one, e.g. "Unable to create floating IP for node + ..." or so?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14383367
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
It was easier to keep the wrapped version inside ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet which is why I did so. Let me see if I can clean things up a bit
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47983671
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
Only one last comment: remove the `if (ip == null)` in steps 2 and 3 in the "allocate ip" method, now that each step has a `return` statement?
Apart from this, lgtm!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48247514
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1335](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1335/) ABORTED
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48076028
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Have not done a squash and rebase on my end yet. Just let me know when things look good enough to do so.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48254007
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
@demobox we can sneak that in
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47667676
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> IMO this is a reasonable tradeoff.
OK, then let's keep as-is.
> Also I see jenkins/cloudbees failing due to checkstyle issues.
Checkstyle should never cause these builds to fail red. [This one](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1434/console) looks like a compilation error?
```
Waiting for Jenkins to finish collecting data
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project openstack-nova: Compilation failure: Compilation failure:
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNode.java:[36,55] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] location: package org.jclouds.openstack.nova.v2_0.compute.options
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNode.java:[55,35] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNode.java:[74,63] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[JENKINS] Archiving /scratch/jenkins/workspace/jclouds-java-7-pull-requests/providers/aws-ec2/pom.xml to org.apache.jclouds.provider/aws-ec2/1.8.0-SNAPSHOT/aws-ec2-1.8.0-SNAPSHOT.pom
[ERROR] location: class org.jclouds.openstack.nova.v2_0.compute.functions.AllocateAndAddFloatingIpToNode
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[45,55] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] location: package org.jclouds.openstack.nova.v2_0.compute.options
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[157,43] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] location: class org.jclouds.openstack.nova.v2_0.compute.strategy.ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[158,76] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] location: class org.jclouds.openstack.nova.v2_0.compute.strategy.ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[161,42] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[162,22] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[162,82] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[163,49] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[157,118] cannot infer type arguments for <O>com.google.common.util.concurrent.ListenableFuture<O>;
[ERROR] reason: no unique maximal instance exists for type variable O with upper bounds java.lang.Object
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project openstack-nova: Compilation failure
```
And [the other](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/963/console) was barfing because of the missing class:
```
[ERROR] /scratch/jenkins/workspace/jclouds-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[163,49] cannot find symbol
[ERROR] symbol: class NodeAndNovaTemplateOptions
[ERROR] -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.1:compile (default-compile) on project openstack-nova: Compilation failure
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47987218
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> ip = floatingIpApi.create();
> + if(ip == null){
> + throw new InsufficientResourcesException();
> + }
You’ve got it right on controlling execution flow with exceptions but I opted to go this route to keep as much existing code in place (whether for good or bad). But yea I think putting the functionality out to a method makes sense. Let me see what I can put together …
From: Ignasi Barrera [mailto:notifications@github.com]
Sent: Tuesday, July 01, 2014 3:20 AM
To: jclouds/jclouds
Cc: Dancy, Chris
Subject: Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)
In apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNode.java:
> ip = floatingIpApi.create();
> + if(ip == null){
> + throw new InsufficientResourcesException();
> + }
I wouldn't use exceptions to control the execution flow. I understand it might be trivial here, but it is in general a bad practice.
Could the code inside the catch be moved out to a method, call it from the catch and when the returned ip is null, and remove that throws?
—
Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/425/files#r14391427>.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14404173
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1328](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1328/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47981614
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #964](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/964/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47989800
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> @@ -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());
It does NOT get turned into a RunNodesException higher up as it fails before the point the RNE is created (which happens in BaseComputeService around line 222). Let me play around a bit and see what I can make happen. I still think this is the right exception to throw but we should be able to capture it and embed it within an RNE.
As far as putting code out to a method: doing that now ;)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14409632
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
Closed #425.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#event-139000679
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
As an aside: I know it's not related to this particular issue, but there's a bit of a nasty declaration of `unassignedIps` as an ArrayList (rather than simply a List) in the [affected class](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNode.java#L85).
Is this something we could clean up here, or should we leave that for a separate PR? Perhaps we can sneak a bit of boy-scouting in here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47667289
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #969](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/969/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48075403
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> @@ -77,8 +77,11 @@ public AllocateAndAddFloatingIpToNode(@Named(TIMEOUT_NODE_RUNNING) Predicate<Ato
>
> FloatingIP ip = null;
> try {
> - logger.debug(">> allocating or reassigning floating ip for node(%s)", node.getId());
> + logger.debug(">> allocating or reassigning floating ip for node(%s)", node.getId());
[minor] Please fix the indent?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14408920
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1426](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1426/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47718289
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1441](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1441/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48223749
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -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());
It should be caught and wrapped into a `RunNodesException`, but confirmation would be good. @cdancy?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14410311
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1339](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1339/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48255572
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Ok. Let me see what I can put together
From: Ignasi Barrera [mailto:notifications@github.com]
Sent: Tuesday, July 01, 2014 11:14 AM
To: jclouds/jclouds
Cc: Dancy, Chris
Subject: Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws NPE on FloatingIPA... (#425)
@nacx<https://github.com/nacx> not sure what unit tests I could write that's not already covered. I've not actually written any new functionality just checked for a possible NPE.
The AllocateAndAddFloatingIpToNodeExpectTest<https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNodeExpectTest.java> currently unit tests the catch clause<https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNodeExpectTest.java#L100>, which is reached when the servers returns a 400 response (the NovaErrorhandler<https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/handlers/NovaErrorHandler.java#L83-L90> is the responsible of automatically translating that 400 response into the InsufficientResourcesException).
That test covers only the case when the exception is thrown, but not when the API returns null. To properly verify that the class behaves properly even if the API returns null, a new test should be added.
It should be pretty straightforward: just copy the existing test for the 400 response, but configure the response to be a 404. That will translate into a null return value. The rest of the test could be the same, as with your change, a null value should reach the same execution path than the InsufficientResourcesException.
—
Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/425#issuecomment-47668739>.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47669038
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> // node's location is a host
> String zoneId = node.getLocation().getParent().getId();
> FloatingIPApi floatingIpApi = novaApi.getFloatingIPExtensionForZone(zoneId).get();
> + Optional<Set<String>> poolNames = input.get().getNovaTemplateOptions().get().getFloatingIpPoolNames();
> +
> + Optional<FloatingIP> ip = allocateFloatingIPForNode(floatingIpApi, poolNames, node.getId());
> + if (!ip.isPresent()){
[minor] Space before the `{`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14536697
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> I'd like it in 1.7.x.
@nacx I tried to apply [the patch](https://issues.apache.org/jira/secure/attachment/12654411/JCLOUDS-607.patch) but it doesn't work on master. Could you submit a PR for 1.7.x..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48360571
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #946](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/946/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47611598
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Upon further thought I wonder if we should remove/deprecate the call to "create()" altogether? Searching the internets I'm not the first to come across this issue openstack. The "create()" call fails because it's not passing a pool-name which I assume is why the "allocate(String poolName)" call was added ... but we're not using it. Because the call will always fail, at least the way it's setup now, it seems to make more sense, if only to stop the unnecessary http traffic for every request, to use ONLY the "list()" route that I've taken here. Thoughts?
JCLOUDS-LOG
==================
2014-07-02 13:40:34,089 DEBUG [org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod] [user thread 1] >> invoking floatingip:create
2014-07-02 13:40:34,090 DEBUG [org.jclouds.http.internal.JavaUrlHttpCommandExecutorService] [user thread 1] Sending request -1217628840: POST http://10.1.4.211:8774/v2/6f0dcd273b264ac38336c3d9cbd49a13/os-floating-ips HTTP/1.1
2014-07-02 13:40:34,144 DEBUG [org.jclouds.http.internal.JavaUrlHttpCommandExecutorService] [user thread 1] Receiving response -1217628840: HTTP/1.1 404 Not Found
2014-07-02 13:40:34,154 DEBUG [org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod] [user thread 1] >> invoking floatingip:list
2014-07-02 13:40:34,155 DEBUG [org.jclouds.http.internal.JavaUrlHttpCommandExecutorService] [user thread 1] Sending request 779197165: GET http://10.1.4.211:8774/v2/6f0dcd273b264ac38336c3d9cbd49a13/os-floating-ips HTTP/1.1
2014-07-02 13:40:34,261 DEBUG [org.jclouds.http.internal.JavaUrlHttpCommandExecutorService] [user thread 1] Receiving response 779197165: HTTP/1.1 200 OK
2014-07-02 13:40:34,275 DEBUG [org.jclouds.rest.internal.InvokeSyncToAsyncHttpMethod] [user thread 1] >> invoking floatingip:add
JCLOUDS-WIRE-LOG
===========================
2014-07-02 13:40:34,090 DEBUG [jclouds.headers] [user thread 1] >> POST http://10.1.4.211:8774/v2/6f0dcd273b264ac38336c3d9cbd49a13/os-floating-ips HTTP/1.1
2014-07-02 13:40:34,090 DEBUG [jclouds.headers] [user thread 1] >> Accept: application/json
2014-07-02 13:40:34,090 DEBUG [jclouds.headers] [user thread 1] >> X-Auth-Token: MIIQeAYJKoZIhvcNAQcCoIIQaTCCEGUCAQExCTAHBgUrDgMCGjCCDs4GCSqGSIb3DQEHAaCCDr8Egg67eyJhY2Nlc3MiOiB7InRva2VuIjogeyJpc3N1ZWRfYXQiOiAiMjAxNC0wNy0wMlQxNzo0MjowMy44MDIzMDEiLCAiZXhwaXJlcyI6ICIyMDE0LTA3LTAzVDE3OjQyOjAzWiIsICJpZCI6ICJwbGFjZWhvbGRlciIsICJ0ZW5hbnQiOiB7ImRlc2NyaXB0aW9uIjogIiIsICJlbmFibGVkIjogdHJ1ZSwgImlkIjogIjZmMGRjZDI3M2IyNjRhYzM4MzM2YzNkOWNiZDQ5YTEzIiwgIm5hbWUiOiAiVGVzdCJ9fSwgInNlcnZpY2VDYXRhbG9nIjogW3siZW5kcG9pbnRzIjogW3siYWRtaW5VUkwiOiAiaHR0cDovLzEwLjEuNC4yMTE6ODc3NC92Mi82ZjBkY2QyNzNiMjY0YWMzODMzNmMzZDljYmQ0OWExMyIsICJyZWdpb24iOiAiUmVnaW9uT25lIiwgImludGVybmFsVVJMIjogImh0dHA6Ly8xMC4xLjQuMjExOjg3NzQvdjIvNmYwZGNkMjczYjI2NGFjMzgzMzZjM2Q5Y2JkNDlhMTMiLCAiaWQiOiAiMTFjNTU0MGRkMjU1NDA3YzkxODQzMjUyYjk1ODY0ODMiLCAicHVibGljVVJMIjogImh0dHA6Ly8xMC4xLjQuMjExOjg3NzQvdjIvNmYwZGNkMjczYjI2NGFjMzgzMzZjM2Q5Y2JkNDlhMTMifV0sICJlbmRwb2ludHNfbGlua3MiOiBbXSwgInR5cGUiOiAiY29tcHV0ZSIsICJuYW1lIjogIm5vdmEifSwgeyJlbmRwb2ludHMiO
iBbeyJhZG1pblVSTCI6ICJodHRwOi8vMTAuMS40LjIxMTo5Njk2IiwgInJlZ2lvbiI6ICJSZWdpb25PbmUiLCAiaW50ZXJuYWxVUkwiOiAiaHR0cDovLzEwLjEuNC4yMTE6OTY5NiIsICJpZCI6ICIxZGY0MDAwNzYzZDM0NGYwODI2OWJiMzQ4YThmNTNmZCIsICJwdWJsaWNVUkwiOiAiaHR0cDovLzEwLjEuNC4yMTE6OTY5NiJ9XSwgImVuZHBvaW50c19saW5rcyI6IFtdLCAidHlwZSI6ICJuZXR3b3JrIiwgIm5hbWUiOiAibmV1dHJvbiJ9LCB7ImVuZHBvaW50cyI6IFt7ImFkbWluVVJMIjogImh0dHA6Ly8xMC4xLjQuMjExOjkyOTIiLCAicmVnaW9uIjogIlJlZ2lvbk9uZSIsICJpbnRlcm5hbFVSTCI6ICJodHRwOi8vMTAuMS40LjIxMTo5MjkyIiwgImlkIjogIjY0MzlhYTcyYmY5YzQ4NjRiZTlmYmEwNzM2MDU2OWUxIiwgInB1YmxpY1VSTCI6ICJodHRwOi8vMTAuMS40LjIxMTo5MjkyIn1dLCAiZW5kcG9pbnRzX2xpbmtzIjogW10sICJ0eXBlIjogImltYWdlIiwgIm5hbWUiOiAiZ2xhbmNlIn0sIHsiZW5kcG9pbnRzIjogW3siYWRtaW5VUkwiOiAiaHR0cDovLzEwLjEuNC4yMTE6ODc3NyIsICJyZWdpb24iOiAiUmVnaW9uT25lIiwgImludGVybmFsVVJMIjogImh0dHA6Ly8xMC4xLjQuMjExOjg3NzciLCAiaWQiOiAiMmNiODk2OTIzY2Y2NDc3MGIwMTY1ZmNlZTg0YTI1ZGIiLCAicHVibGljVVJMIjogImh0dHA6Ly8xMC4xLjQuMjExOjg3NzcifV0sICJlbmRwb2ludHNfbGlua3MiOiBbXSwgInR5cGUiOiAibWV0ZX
JpbmciLCAibmFtZSI6ICJjZWlsb21ldGVyIn0sIHsiZW5kcG9pbnRzIjogW3siYWRtaW5VUkwiOiAiaHR0cDovLzEwLjEuNC4yMDA6ODAwMC92MS82ZjBkY2QyNzNiMjY0YWMzODMzNmMzZDljYmQ0OWExMyIsICJyZWdpb24iOiAiUmVnaW9uT25lIiwgImludGVybmFsVVJMIjogImh0dHA6Ly8xMC4xLjQuMjAwOjgwMDAvdjEvNmYwZGNkMjczYjI2NGFjMzgzMzZjM2Q5Y2JkNDlhMTMiLCAiaWQiOiAiMjI3ZjQ4NmE5M2FhNGNjNmJhNDc1ODU5NDkzZGNiNWEiLCAicHVibGljVVJMIjogImh0dHA6Ly8xMC4xLjQuMjAwOjgwMDAvdjEvNmYwZGNkMjczYjI2NGFjMzgzMzZjM2Q5Y2JkNDlhMTMifV0sICJlbmRwb2ludHNfbGlua3MiOiBbXSwgInR5cGUiOiAiY2xvdWRmb3JtYXRpb24iLCAibmFtZSI6ICJoZWF0LWNmbiJ9LCB7ImVuZHBvaW50cyI6IFt7ImFkbWluVVJMIjogImh0dHA6Ly8xMC4xLjQuMjExOjg3NzYvdjEvNmYwZGNkMjczYjI2NGFjMzgzMzZjM2Q5Y2JkNDlhMTMiLCAicmVnaW9uIjogIlJlZ2lvbk9uZSIsICJpbnRlcm5hbFVSTCI6ICJodHRwOi8vMTAuMS40LjIxMTo4Nzc2L3YxLzZmMGRjZDI3M2IyNjRhYzM4MzM2YzNkOWNiZDQ5YTEzIiwgImlkIjogIjRjOWQ2OGUwODgyYjRkNTdhZGFkMDhmYzIzNDhiODVlIiwgInB1YmxpY1VSTCI6ICJodHRwOi8vMTAuMS40LjIxMTo4Nzc2L3YxLzZmMGRjZDI3M2IyNjRhYzM4MzM2YzNkOWNiZDQ5YTEzIn1dLCAiZW5kcG9pbnRzX2xpbmtzIjogW10sICJ0eXBlIjo
gInZvbHVtZSIsICJuYW1lIjogImNpbmRlciJ9LCB7ImVuZHBvaW50cyI6IFt7ImFkbWluVVJMIjogImh0dHA6Ly8xMC4xLjQuMjExOjg3NzMvc2VydmljZXMvQWRtaW4iLCAicmVnaW9uIjogIlJlZ2lvbk9uZSIsICJpbnRlcm5hbFVSTCI6ICJodHRwOi8vMTAuMS40LjIxMTo4NzczL3NlcnZpY2VzL0FkbWluIiwgImlkIjogIjhhMDU0MDY3ODVmYjQzOGY5YzQ3ZjdiZTc3NGMzNjY4IiwgInB1YmxpY1VSTCI6ICJodHRwOi8vMTAuMS40LjIxMTo4NzczL3NlcnZpY2VzL0Nsb3VkIn1dLCAiZW5kcG9pbnRzX2xpbmtzIjogW10sICJ0eXBlIjogImVjMiIsICJuYW1lIjogImVjMiJ9LCB7ImVuZHBvaW50cyI6IFt7ImFkbWluVVJMIjogImh0dHA6Ly8xMC4xLjQuMjAwOjgwMDQvdjEvNmYwZGNkMjczYjI2NGFjMzgzMzZjM2Q5Y2JkNDlhMTMiLCAicmVnaW9uIjogIlJlZ2lvbk9uZSIsICJpbnRlcm5hbFVSTCI6ICJodHRwOi8vMTAuMS40LjIwMDo4MDA0L3YxLzZmMGRjZDI3M2IyNjRhYzM4MzM2YzNkOWNiZDQ5YTEzIiwgImlkIjogIjFjY2ZhMmQ0YTVkODQyMzI4ZmM5NmI3MDY2NjA1NzhhIiwgInB1YmxpY1VSTCI6ICJodHRwOi8vMTAuMS40LjIwMDo4MDA0L3YxLzZmMGRjZDI3M2IyNjRhYzM4MzM2YzNkOWNiZDQ5YTEzIn1dLCAiZW5kcG9pbnRzX2xpbmtzIjogW10sICJ0eXBlIjogIm9yY2hlc3RyYXRpb24iLCAibmFtZSI6ICJoZWF0In0sIHsiZW5kcG9pbnRzIjogW3siYWRtaW5VUkwiOiAiaHR0cDovLzEwLjEuNC4y
MDU6ODA4MC92MS9BVVRIXzZmMGRjZDI3M2IyNjRhYzM4MzM2YzNkOWNiZDQ5YTEzIiwgInJlZ2lvbiI6ICJSZWdpb25PbmUiLCAiaW50ZXJuYWxVUkwiOiAiaHR0cDovLzEwLjEuNC4yMDU6ODA4MC92MS9BVVRIXzZmMGRjZDI3M2IyNjRhYzM4MzM2YzNkOWNiZDQ5YTEzIiwgImlkIjogIjNmYmNhMGVhZTkzNzQxMzA4OWExMmYwMDQwNmYxODBhIiwgInB1YmxpY1VSTCI6ICJodHRwOi8vMTAuMS40LjIwNTo4MDgwL3YxL0FVVEhfNmYwZGNkMjczYjI2NGFjMzgzMzZjM2Q5Y2JkNDlhMTMifV0sICJlbmRwb2ludHNfbGlua3MiOiBbXSwgInR5cGUiOiAib2JqZWN0LXN0b3JlIiwgIm5hbWUiOiAic3dpZnQifSwgeyJlbmRwb2ludHMiOiBbeyJhZG1pblVSTCI6ICJodHRwOi8vMTAuMS40LjIxMTozNTM1Ny92Mi4wIiwgInJlZ2lvbiI6ICJSZWdpb25PbmUiLCAiaW50ZXJuYWxVUkwiOiAiaHR0cDovLzEwLjEuNC4yMTE6NTAwMC92Mi4wIiwgImlkIjogIjJjNDZhMTEwZTY5ODRlNDFiZGZiZTYwZTBiNjRkN2IxIiwgInB1YmxpY1VSTCI6ICJodHRwOi8vMTAuMS40LjIxMTo1MDAwL3YyLjAifV0sICJlbmRwb2ludHNfbGlua3MiOiBbXSwgInR5cGUiOiAiaWRlbnRpdHkiLCAibmFtZSI6ICJrZXlzdG9uZSJ9XSwgInVzZXIiOiB7InVzZXJuYW1lIjogIkNocmlzIiwgInJvbGVzX2xpbmtzIjogW10sICJpZCI6ICI4YTY5NzY1ZmI0NGQ0MWEwODdiYmYxYTE2YmQxNjM1YSIsICJyb2xlcyI6IFt7Im5hbWUiOiAiX21lbWJlcl8if
SwgeyJuYW1lIjogImFkbWluIn1dLCAibmFtZSI6ICJDaHJpcyJ9LCAibWV0YWRhdGEiOiB7ImlzX2FkbWluIjogMCwgInJvbGVzIjogWyI5ZmUyZmY5ZWU0Mzg0YjE4OTRhOTA4NzhkM2U5MmJhYiIsICIzMTFiMDNhZTU1NDU0YjkzYTBmNjBjNWNmZWYyMzAwZCJdfX19MYIBgTCCAX0CAQEwXDBXMQswCQYDVQQGEwJVUzEOMAwGA1UECAwFVW5zZXQxDjAMBgNVBAcMBVVuc2V0MQ4wDAYDVQQKDAVVbnNldDEYMBYGA1UEAwwPd3d3LmV4YW1wbGUuY29tAgEBMAcGBSsOAwIaMA0GCSqGSIb3DQEBAQUABIIBAIoc+b3JfyFXSmZ8H4HtgSeOzXi90J9-Yb+M1ewvVSIPYt+HszLiIUBzO6P8vvWxP7oLTzXCC8bAjoZajRWjYYrcQiW1Gr3TydpuZEQrTyRPiY13IImjbgT8ErrMrALJh0krjSaSgFHqHSt0Nb+UaYS1XtFkwm93-Arpr1W-BhdQ6ekqdduv5eI5l9LIZzMwUUFGc4LZY8DoSzIJI-aqNgTFzxcEh0z7OvVnymW0m8OjONXpMN+Ncf2sKdfTfBD0RbBf2VQp2BBBx5R9vJkO0ZWBkfVAqPZCaETkO9-aS0Q20ArKNa1ui6Lvk6cGWcjwbgGYjTjyTU-eOstRiXiRELM=
2014-07-02 13:40:34,090 DEBUG [jclouds.headers] [user thread 1] >> Content-Type: application/json
2014-07-02 13:40:34,090 DEBUG [jclouds.headers] [user thread 1] >> Content-Length: 2
2014-07-02 13:40:34,144 DEBUG [jclouds.headers] [user thread 1] << HTTP/1.1 404 Not Found
2014-07-02 13:40:34,145 DEBUG [jclouds.headers] [user thread 1] << Date: Wed, 02 Jul 2014 17:42:12 GMT
2014-07-02 13:40:34,145 DEBUG [jclouds.headers] [user thread 1] << Connection: keep-alive
2014-07-02 13:40:34,145 DEBUG [jclouds.headers] [user thread 1] << X-Compute-Request-Id: req-95142310-92a2-4543-9a8b-c7e4b2cb2d8d
2014-07-02 13:40:34,145 DEBUG [jclouds.headers] [user thread 1] << Content-Type: application/json; charset=UTF-8
2014-07-02 13:40:34,145 DEBUG [jclouds.headers] [user thread 1] << Content-Length: 97
2014-07-02 13:40:34,145 DEBUG [jclouds.wire] [user thread 1] << "{"itemNotFound": {"message": "FloatingIpPoolNotFound: Floating ip pool not found.", "code": 404}}"
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47811752
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Not sure. It's definitely a bug but the fix was to provide extra functionality to address the bug. If it's easy enough to do so I would say yes otherwise as long as it makes it to 1.7.4/1.8.0 I'm good.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48260914
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1434](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1434/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47982995
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1439](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1439/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48068442
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #953](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/953/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47699369
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Made minor commit to address formatting issues. I promise now that @nacx has pointed me to the link below I will be less likely to submit spaghetti code ;)
https://wiki.apache.org/jclouds/Coding%20Standards
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47710524
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1414](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1414/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47588400
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
done and done ;)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48246534
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
@demobox looking over things I'm actually going to argue FOR keeping the AtomicReferences inside NodeAndNovaTemplateOptions. The reason being is that we are constantly passing around AtomicReference<NodeMetadata> and so it makes things immensely easier to retrieve it rather than constantly unpacking/packing it back up again. Mainly the classes:
AllocateAndAddFloatingIpToNode
ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet
currently these are the only 2 classes using this data-structure but they both heavily use AtomicReference<NodeMetadata> and so packing/unpacking would turn the code ugly. IMO this is a reasonable tradeoff.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47985531
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #955](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/955/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47718039
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> Also did you need me to do a squash/rebase? It appears not but just checking
I squashed it all down as part of the merge. I felt you'd done enough back and forth on this PR already ;-) Thanks for asking, though!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48263449
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1435](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1435/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47990078
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
I'd like it in 1.7.x. This issue affects also the Jenkins plugin and backporting will help implementing the changes on the Jenkins side.
Thanks for your work here @cdancy!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48276983
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
Minor general comment: many of the files contain `if (...){...` - please change that to `if (...) {` (note the additional space).
Thanks for bearing with this one, @cdancy!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48245215
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Also did you need me to do a squash/rebase? It appears not but just checking
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48261261
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1297](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1297/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47586541
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1312](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1312/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47719896
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @cdancy! Could you also add unit tests for the change so we don't break it accidentally in the future?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47624593
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
I'd love some feedback on latest commit. Instead of pushing off the second PR noted above I've included those proposed changed here as it's the proper way to solve the problem instead of pushing it off to some later date. This will allow us to do the following:
NovaTemplateOptions options = NovaTemplateOptions.Builder.
autoAssignFloatingIp(true).
floatingIpPoolNames(ipPoolNames). //this is the noted change
blockUntilRunning(true).
securityGroupNames(securityGroups);
@everett-toews @jdaggett @nacx thoughts?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47979279
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> ip = floatingIpApi.create();
> + if(ip == null){
> + throw new InsufficientResourcesException();
> + }
And therein lies the million dollar question! Because most of the code to allocate, based on "listed" IP's, is inside the catch I left it there and I think it makes sense to do so. Should something pop we'd like a last-effort-attempt to allocate an IP by iterating through the listed IP's. This code is basically a short-circuit to get us there should the call to ".create()" give us a null. Why the call to ".create()" returns null (though I don't believe for all cases/providers) but the call to ".list()" succeeds ... I've no idea.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14385723
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> @@ -77,8 +77,11 @@ public AllocateAndAddFloatingIpToNode(@Named(TIMEOUT_NODE_RUNNING) Predicate<Ato
>
> FloatingIP ip = null;
> try {
> - logger.debug(">> allocating or reassigning floating ip for node(%s)", node.getId());
> + logger.debug(">> allocating or reassigning floating ip for node(%s)", node.getId());
Actually I think we want to leave this in as it matches with formatting for the rest of the file: that being all enclosed code (whether from if/else or try/catch) is starting on column 10
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14409912
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1330](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1330/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48007345
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> Upon further thought I wonder if we should remove/deprecate the call to "create()" altogether?
I'm not familiar enough with this part of the code to comment - perhaps @zack-shoylev, @jdaggett or @everett-toews have some thoughts here?
In the meantime, would you prefer to leave this PR open or merge it if it's ready to go, @cdancy?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47813143
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> @@ -94,6 +99,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",e);
> + }
Will do and will do
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14385731
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
@nacx not sure what unit tests I could write that's not already covered. I've not actually written any new functionality just checked for a possible NPE.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47665259
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> @@ -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());
If we don't throw the NPE here it gets caught downstream and could be mis-construed as something else whereas here we can force the NPE at the point it pops. If we throw an IRE we are back in the same boat we started in ...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14415520
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #963](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/963/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47982976
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> @@ -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());
Here is what I'm talking about. Is this the solution we want to go with?
FloatingIP ip = null;
try {
logger.debug(">> allocating or reassigning floating ip for node(%s)", node.getId());
// throw InsufficientResourcesException on 400 from server or null if 404 from server
ip = floatingIpApi.create();
if(ip == null){
ip = allocateIPFromFloatingIPApiList(floatingIpApi);
if(ip == null){
throw new NullPointerException("Failed to allocate a FloatingIP for node(" + node.getId() + ")");
}
}
} catch (InsufficientResourcesException e) {
logger.trace("<< [%s] allocating a new floating ip for node(%s)", e.getMessage(), node.getId());
logger.trace(">> searching for existing, unassigned floating ip for node(%s)", node.getId());
ip = allocateIPFromFloatingIPApiList(floatingIpApi);
// 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);
}
}
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14415348
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -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());
Jclouds implements APIs in a declarative way, and automatically generates the request based on the method declaration. For the `create` method, there is [this fallback](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/extensions/FloatingIPAsyncApi.java#L105) defined, which will make the method return `null` when the server returns a 404 response. For responses codes that are not explicitly mapped in the API method, the [error handler](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/handlers/NovaErrorHandler.java#L73) is invoked, so:
* The API will return `null` when the server returns a 404.
* It will throw the IRE when the server returns a 400 with a `quota exceeded` message.
This is related to the test I also mentioned mentioned. It currently covers [the 400 response](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNodeExpectTest.java#L97-L130) path, but not the 404 one (that's the one I asked to add by copying the existing 400 test).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14413173
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #968](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/968/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48068268
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #972](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/972/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48253959
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> done and done ;)
Pending PR builders, looks good to me. @nacx: any further comments?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48246957
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> NP! I'll do later today.
Thanks! ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48367854
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> @@ -77,8 +77,13 @@ public AllocateAndAddFloatingIpToNode(@Named(TIMEOUT_NODE_RUNNING) Predicate<Ato
>
> FloatingIP ip = null;
> try {
> - logger.debug(">> allocating or reassigning floating ip for node(%s)", node.getId());
> +
> + logger.debug(">> allocating or reassigning floating ip for node(%s)", node.getId());
> +
[minor] remove new blank lines here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14383320
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
@nacx @demobox take a look at the latest changes and let me know what you think. I refactored the code a bit and think this is a clean compromise/solution given this scenario.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47695399
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> @@ -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());
Gotcha. I guess my next thought would then be: if we don't want to throw an IRE when "FloatingIPApi.create()" returns null but instead search through listed IP's (much like the caught IRE does now) then what do we want to do should both fail? It seems to me to throw an IRE!!! Do we want to let the null fall through which eventually gets us an NPE (wrapped in a RunNodesException)? It seems the IRE is more descriptive but maybe throwing an NPE with a detailed message will suffice?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14413990
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1443](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1443/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48254801
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
>
> + floatingIpApi.addToServer(ip.get().getIp(), node.getProviderId());
> + input.get().getNodeMetadata().set(NodeMetadataBuilder.fromNodeMetadata(node).publicAddresses(ImmutableSet.of(ip.get().getIp())).build());
> + floatingIpCache.invalidate(ZoneAndId.fromSlashEncoded(node.getId()));
> + return input.get().getNodeMetadata();
> + }
> +
> + /**
> + * Allocate a FloatingIP for a given Node
[minor] "Allocates"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14536724
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1424](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1424/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47700121
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> @@ -94,6 +99,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",e);
> + }
[minor] please add a space after the `//` and add complete the "...after iterating through all available _" comment. Also, for the `if`, please use a 3-space indent and add spaces around the expression:
```
if (ip == null) {
throw new InsufficientResourcesException("Failed to allocate a FloatingIP for node", e);
}
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14383451
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
@demobox The patch did not apply, but I squashed the commits in this PR into a single one, and then cherry-picked to 1.7.x and there were no conflicts, so I pushed it directly after building locally: https://github.com/jclouds/jclouds/commit/7641bd61cd56f7f5a4da5ac89769c1d200abe79f
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48494892
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1300](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1300/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47611087
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
@demobox clicking that link above gives this exception:
[ERROR] /scratch/jenkins/workspace/jclouds-java-7-pull-requests/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java:[157,118] cannot infer type arguments for <O>com.google.common.util.concurrent.ListenableFuture<O>;
reason: no unique maximal instance exists for type variable O with upper bounds java.lang.Object
I'm relatively new to working with guava and subsequently ListenableFuture (this is actually the first project I've done so) so the solution to this issue is not jumping out at me. Compiles and executes fine locally so I'm not exactly sure what the issue is. @jdaggett @nacx @ccuistine any ideas?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47994725
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> if (templateOptions.shouldAutoAssignFloatingIp()) {
> - return Futures.transform(future, createAndAddFloatingIpToNode, userExecutor);
> +
> + ListenableFuture<AtomicReference<NodeAndNovaTemplateOptions>> nodeAndNovaTemplateOptions = Futures.transform(future,
> + new Function<AtomicReference<NodeMetadata>, AtomicReference<NodeAndNovaTemplateOptions>>(){
> +
> + @Override
> + public AtomicReference<NodeAndNovaTemplateOptions> apply(AtomicReference<NodeMetadata> input) {
> + return NodeAndNovaTemplateOptions.newAtomicReference(input, Atomics.newReference(templateOptions));
> + }
> + }
> + );
> + return Futures.transform(nodeAndNovaTemplateOptions, createAndAddFloatingIpToNode, userExecutor);
[minor] Better use Guava's `Functions.compose`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14586112
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> * Specifies the keypair used to run instances with
> * @return the keypair to be used
> */
> public String getKeyPairName() {
> return keyPairName;
> }
> -
> +
[minor] Remove the whitespace here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14536515
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> @@ -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
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Let's leave it open a while longer to see what others have to say. In testing the 'create()' method is just making unnecessary http calls and filling up the logs with the above. If we can agree on the above, meaning to deprecate, or at the least don't use it at all, then I'll create a second PR to address using the "allocate(String poolName)" method and how to get the pool name down into this context.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47813704
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
>
> + floatingIpApi.addToServer(ip.get().getIp(), node.getProviderId());
> + input.get().getNodeMetadata().set(NodeMetadataBuilder.fromNodeMetadata(node).publicAddresses(ImmutableSet.of(ip.get().getIp())).build());
> + floatingIpCache.invalidate(ZoneAndId.fromSlashEncoded(node.getId()));
> + return input.get().getNodeMetadata();
> + }
> +
> + /**
> + * Allocate a FloatingIP for a given Node
On this one too
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14537109
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #954](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/954/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47716663
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> so I pushed it directly after building locally:
Thanks, @nacx! Did you also get a bunch of whitespace errors? I recall I had some cleaning up to do when creating the patch. I've added comments to the commit...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48498705
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -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());
In both cases, when falling back to the list because of the `null` or the IRE, I'd end up throwing an IRE if no ip could be allocated. I assume the NPE was an issue and that it should never got propagated, so I'd throw the IRE if no ip can be allocated, regardless of the cause (404 or 400).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14414233
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Good catch on the "if" statements.
As far as the Functions.compose is concerned: seems like we would lose the ListenableFuture, that method createNodeInGroupWithNameAndTemplate returns, if we go the Functions.compose route instead of Futures.transfrom?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48209442
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1334](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1334/) ABORTED
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48070023
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #965](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/965/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48007875
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> i guess the exception seems to have cleared itself up!?!
Yes...weird! But perhaps adding the missing file fixed things? There's also one minor [Checkstyle violation now](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1438/org.apache.jclouds.api$openstack-nova/violations/file/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/BinaryTransport.java/).
See also https://github.com/cdancy/jclouds/commit/5cfba305ce2eb0692679b47aa2187930f06e3564#commitcomment-6901005?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48065601
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> - try {
> - logger.debug(">> allocating or reassigning floating ip for node(%s)", node.getId());
> - ip = floatingIpApi.create();
> - } catch (InsufficientResourcesException e) {
> - logger.trace("<< [%s] allocating a new floating ip for node(%s)", e.getMessage(), node.getId());
> - logger.trace(">> searching for existing, unassigned floating ip for node(%s)", node.getId());
> - ArrayList<FloatingIP> unassignedIps = Lists.newArrayList(Iterables.filter(floatingIpApi.list(),
> - new Predicate<FloatingIP>() {
> +
> + // 1.) Attempt to allocate from optionally passed poolNames
> + if (poolNames.isPresent()){
> + for (String poolName : poolNames.get()){
> + try {
> + logger.debug(">> allocating floating IP from pool %s for node(%s)", poolName, nodeID);
> + ip = floatingIpApi.allocateFromPool(poolName);
> + if (ip != null){
Just `return` here to avoid having so much "if" statements? Code may look cleaner
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14585898
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
>@nacx not sure what unit tests I could write that's not already covered. I've not actually written any new functionality just checked for a possible NPE.
The [AllocateAndAddFloatingIpToNodeExpectTest](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNodeExpectTest.java) currently [unit tests the catch clause](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/AllocateAndAddFloatingIpToNodeExpectTest.java#L100), which is reached when the servers returns a 400 response (the [NovaErrorhandler](https://github.com/jclouds/jclouds/blob/master/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/handlers/NovaErrorHandler.java#L83-L90) is the responsible of automatically translating that 400 response into the `InsufficientResourcesException`).
That test covers only the case when the exception is thrown, but not when the API returns `null`. To properly verify that the class behaves properly even if the API returns `null`, a new test should be added.
It should be pretty straightforward: just copy the existing test for the 400 response, but configure the response to be a 404. That will translate into a `null` return value. The rest of the test could be the same, as with your change, a `null` value should reach the same execution path than the `InsufficientResourcesException`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47668739
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1438](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1438/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48058770
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> Should be good now
Great, thanks @cdancy! Once the PR builders come back I'll look to get this squashed and merged...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48249979
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Aaahhh how I forgot those 2, considering I was just in there poking around for this very issue, I've no idea. Should be good now
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48248928
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1311](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1311/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47714953
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> }
> -
> +
[minor] Remove the whitespace here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14536596
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
Just to verify: from what I can see in the code, we now have three possible branches for assigning IPs: find from pool, create, find fixed unused (or something like that). Are all three scenarios covered by tests?
Thanks for the update, @cdancy!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47982562
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #943](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/943/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47587524
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @cdancy! Some minor comments.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47603668
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1333](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1333/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48057141
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> @@ -184,13 +207,25 @@ public boolean shouldAutoAssignFloatingIp() {
> }
>
> /**
> + * The floating IP pool name(s) to use when allocating a FloatingIP. Applicable
> + * only if If #shouldGenerateKeyPair() returns true. If not set will attempt to
[minor] "if If"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14536858
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
i guess the exception seems to have cleared itself up!?!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48035792
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> @@ -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());
Do we know if 'FloatingIPApi.create()' throws an 'InsufficientResourcesException' or will always return null upon unsuccessful allocation? I've no idea how to go about tracking the impl down which is why I ask. If the later we can do away with the try/catch block altogether and only throw the IRE should all attempts to allocate an IP fail. Thoughts?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14412381
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #971](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/971/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48253060
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #970](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/970/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48220801
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> @@ -184,13 +207,25 @@ public boolean shouldAutoAssignFloatingIp() {
> }
>
> /**
> + * The floating IP pool name(s) to use when allocating a FloatingIP. Applicable
> + * only if If #shouldGenerateKeyPair() returns true. If not set will attempt to
Yes and yes. Fixing now ;)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14536959
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1337](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1337/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48218451
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> + logger.debug(">> allocating floating IP from pool %s for node(%s)", poolName, nodeID);
> + ip = floatingIpApi.allocateFromPool(poolName);
> + if (ip != null){
> + break;
> + }
> + } catch (InsufficientResourcesException ire){
> + logger.trace("<< [%s] failed to allocate floating IP from pool %s for node(%s)", ire.getMessage(), poolName, nodeID);
> + }
> + }
> + }
> +
> + // 2.) Attempt to allocate, if necessary, via 'create()' call
> + if (ip == null){
> + try {
> + logger.debug(">> creating floating IP for node(%s)", nodeID);
> + ip = floatingIpApi.create();
Same as above?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14585903
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> ip = floatingIpApi.create();
> + if(ip == null){
> + throw new InsufficientResourcesException();
> + }
Thanks @cdancy!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14406679
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Ignasi Barrera <no...@github.com>.
> ip = floatingIpApi.create();
> + if(ip == null){
> + throw new InsufficientResourcesException();
> + }
I wouldn't use exceptions to control the execution flow. I understand it might be trivial here, but it is in general a bad practice.
Could the code inside the catch be moved out to a method, call it from the catch and when the returned ip is `null`, and remove that `throws`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14391427
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1417](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1417/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47611802
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
Also I see jenkins/cloudbees failing due to checkstyle issues. I'm using the below checkstyle command on my code base and it's running fine.
mvn checkstyle:checkstyle --quiet -Dcheckstyle.output.file=/dev/stdout -Dcheckstyle.output.format=plain
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47986496
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1309](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1309/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47701892
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Christopher Dancy <no...@github.com>.
> }
> -
> +
on it
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14537102
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by Andrew Phillips <no...@github.com>.
> + * corresponding NovaTemplateOptions object.
> + */
> +public class NodeAndNovaTemplateOptions {
> +
> + private final AtomicReference<NodeMetadata> nodeMetadata;
> + private final AtomicReference<NovaTemplateOptions> novaTemplateOptions;
> +
> + public NodeAndNovaTemplateOptions(NodeMetadata nodeMetadata, NovaTemplateOptions novaTemplateOptions){
> + this.nodeMetadata = Atomics.newReference(nodeMetadata);
> + this.novaTemplateOptions = Atomics.newReference(novaTemplateOptions);
> + }
> +
> + public NodeAndNovaTemplateOptions(AtomicReference<NodeMetadata> nodeMetadata, AtomicReference<NovaTemplateOptions> novaTemplateOptions){
> + this.nodeMetadata = nodeMetadata;
> + this.novaTemplateOptions = novaTemplateOptions;
> + }
[minor] Are we ever going to create these directly, rather than with the static helpers? Can we reduce this to one private or protected constructor, and two static builders
```
public static NodeAndNovaTemplateOptions newNodeAndNovaTemplateOptions(AtomicReference<NodeMetadata> node, AtomicReference<NovaTemplateOptions> options) {
return new NodeAndNovaTemplateOptions(node, options);
}
public static NodeAndNovaTemplateOptions newNodeAndNovaTemplateOptions(NodeMetadata node, NovaTemplateOptions options) {
return newNodeAndNovaTemplateOptions(Atomics.newReference(node), Atomics.newReference(options));
// or return new NodeAndNovaTemplateOptions(Atomics.newReference(node), Atomics.newReference(options));
// if we prefer that
}
```
?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425/files#r14624518
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1329](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1329/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-47988879
Re: [jclouds] JCLOUDS-607: ComputeService.createNodesInGroup throws
NPE on FloatingIPA... (#425)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1440](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1440/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/425#issuecomment-48075446