> + vm =
> client.getVirtualMachineApi().getVirtualMachine(vm.getId());
> + List<Integer> ports =
> Ints.asList(templateOptions.getInboundPorts());
> + if (capabilities.getCloudStackVersion().startsWith("2")) {
> + logger.debug(">> setting up IP forwarding for
> IPAddress(%s) rules(%s)", ip.getId(), ports);
> + Set<IPForwardingRule> 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);
> + Set<FirewallRule> 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
