Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
+ destroyNode(vm.getId());
Hey @demobox . The point of this code is that destroyNode needs to be called
before we rethrow the exception. Otherwise the code will have created an
instance on the cloud, but will tell the higher level code that creating the
instance failed. Please have a look at the discussion about this patch in
https://github.com/jclouds/jclouds/issues/184
I think the last consensus was to merge this patch in. I know it's not the best
solution, but next to implementing a transaction mechanism with rollbacks this
does solve the problems we are having.
The issue i'm facing is that due to ipv4 shortage allocating a static nat ip
address sometimes fails. When this happens the jcloud-jenkins plugin thinks the
creation of the instance fails (and rightfully so) but the instance was created
on the cloud. Leaving a lot of unmanaged stale instances around. I've been
running production with this fix for several months and have yet to see any
issues with it.
It would be really nice to have this in.
Cheers,
Hugo
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11014225
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
+ destroyNode(vm.getId());
the point of this code is that destroyNode needs to be called before we
rethrow the exception.
Oh, I agree. Sorry, I didn't express myself clearly. What I meant is the
following: with the code as currently written, what if `destroyNode` here
throws an exception? Then the original exception (the one that probably matters
to the user, which explains why the static NATs could not be created) will not
be visible, and the user will instead be stuck with the cleanup exception.
I was suggesting something like:
```
} catch (RuntimeException re) {
logger.error(-- exception after node has been created, trying to destroy the
created virtualMachine(%s), vm.getId());
try {
destroyNode(vm.getId());
} catch (RuntimeException re2) {
// or just ignore this exception
logger.debug(-- exception trying to destroy the created
virtualMachine(%s): %s, vm.getId(), re2);
}
throw re
}
```
Does that make more sense?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11014653
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+logger.trace( static NATed IPAddress(%s) to
virtualMachine(%s), ip.getId(), vm.getId());
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
Add the exception itself (or at least `re.getMessage()`) to the log line?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11014667
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
+ destroyNode(vm.getId());
Ahh. that makes perfect sense :-). I make the change.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11014695
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+logger.trace( static NATed IPAddress(%s) to
virtualMachine(%s), ip.getId(), vm.getId());
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
I didn't think that was necessary as the code rethrows the exception anyway,
might be confusing for the guys looking at the logs to get the same exception
twice in the logs?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11014882
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+logger.trace( static NATed IPAddress(%s) to
virtualMachine(%s), ip.getId(), vm.getId());
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
Great :)
The checkstyle fix and the exception handler are pushed.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11016457
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds-pull-requests #710](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/710/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38786336
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
jclouds-pull-requests #710 SUCCESS [Clean Checkstyle](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/710/violations/). +1 - good to go for me. @nacx @abayer: any open questions? Otherwise, please squash'n'rebase, and then we can merge! --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38786846
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds-pull-requests #711](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/711/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38786961
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds-java-7-pull-requests #1181](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1181/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38787331
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
pushed a squashed and rebased patch --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38787615
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds-pull-requests #713](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/713/) SUCCESS This pull request looks good --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38791689
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds-java-7-pull-requests #1182](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1182/) 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/328#issuecomment-38792131
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds-java-7-pull-requests #1183](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1183/) 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/328#issuecomment-38792151
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
jclouds-java-7-pull-requests #1183 UNSTABLE [Spurious](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1182/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/) [test failures](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/org.apache.jclouds$jclouds-compute/1183/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38794219
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
Committed to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;h=0401959). Thanks, @spark404! @nacx: Do we want to backport this? If so, to which branch(es)? --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38794590
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds » jclouds #967](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/967/) 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/328#issuecomment-38795172
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
Would be nice to have it backported to the 1.7.x branch. That would make it easier to get it in the next release for jenkins-jclouds i think, but thats up to @abayer. I'm using this patch with a jclouds 1.7.x build at the moment. --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38796156
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds » jclouds #968](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/968/) 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/328#issuecomment-38799809
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds » jclouds #969](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/969/) 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/328#issuecomment-38805427
[jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
As discussed in https://github.com/jclouds/jclouds/pull/184 here is a new patch for master. Cheers, Hugo You can merge this Pull Request by running: git pull https://github.com/spark404/jclouds JCLOUDS-347-1.8.x Or you can view, comment on it, or merge it online at: https://github.com/jclouds/jclouds/pull/328 -- Commit Summary -- * Implement a poor-mans rollback if static nat creation fails -- File Changes -- M apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/strategy/CloudStackComputeServiceAdapter.java (44) -- Patch Links -- https://github.com/jclouds/jclouds/pull/328.patch https://github.com/jclouds/jclouds/pull/328.diff --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds » jclouds #953](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/953/) 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/328#issuecomment-38699461
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
[jclouds-pull-requests #697](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/697/) 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/328#issuecomment-38700999
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+vm =
client.getVirtualMachineApi().getVirtualMachine(vm.getId());
+ListInteger ports =
Ints.asList(templateOptions.getInboundPorts());
+if (capabilities.getCloudStackVersion().startsWith(2)) {
+ logger.debug( setting up IP forwarding for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetIPForwardingRule rules =
setupPortForwardingRulesForIP.apply(ip, ports);
+ logger.trace( setup %d IP forwarding rules on
IPAddress(%s), rules.size(), ip.getId());
+} else {
+ logger.debug( setting up firewall rules for
IPAddress(%s) rules(%s), ip.getId(), ports);
+ SetFirewallRule rules =
setupFirewallRulesForIP.apply(ip, ports);
+ logger.trace( setup %d firewall rules on
IPAddress(%s), rules.size(), ip.getId());
+}
+ }
+ }
+ } catch (RuntimeException re) {
+ logger.error(-- exception after node has been created, trying to
destroy the created virtualMachine(%s), vm.getId());
+ destroyNode(vm.getId());
Do we need a `try...catch` around here, or can this not throw an exception?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/328/files#r11004547
Re: [jclouds] JCLOUDS-347 Implement a poor-mans rollback if static nat creation fails (#328)
+1 - seems saner to me. =) --- Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds/pull/328#issuecomment-38753246
