diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java index 18964f9e73..e774d04754 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java @@ -41,6 +41,7 @@ import org.jclouds.cloudstack.compute.strategy.AdvancedNetworkOptionsConverter; import org.jclouds.cloudstack.compute.strategy.BasicNetworkOptionsConverter; import org.jclouds.cloudstack.compute.strategy.CloudStackComputeServiceAdapter; import org.jclouds.cloudstack.compute.strategy.OptionsConverter; +import org.jclouds.cloudstack.domain.FirewallRule; import org.jclouds.cloudstack.domain.IPForwardingRule; import org.jclouds.cloudstack.domain.Network; import org.jclouds.cloudstack.domain.NetworkType; @@ -53,6 +54,7 @@ import org.jclouds.cloudstack.domain.Zone; import org.jclouds.cloudstack.features.GuestOSClient; import org.jclouds.cloudstack.functions.StaticNATVirtualMachineInNetwork; import org.jclouds.cloudstack.functions.ZoneIdToZone; +import org.jclouds.cloudstack.options.ListFirewallRulesOptions; import org.jclouds.cloudstack.predicates.JobComplete; import org.jclouds.cloudstack.suppliers.GetCurrentUser; import org.jclouds.cloudstack.suppliers.NetworksForCurrentUser; @@ -108,6 +110,8 @@ public class CloudStackComputeServiceContextModule extends install(new FactoryModuleBuilder().build(StaticNATVirtualMachineInNetwork.Factory.class)); bind(new TypeLiteral>>() { }).to(GetIPForwardingRulesByVirtualMachine.class); + bind(new TypeLiteral>>() { + }).to(GetFirewallRulesByVirtualMachine.class); bind(new TypeLiteral>() { }).to(ZoneIdToZone.class); bind(new TypeLiteral>>() { @@ -204,6 +208,34 @@ public class CloudStackComputeServiceContextModule extends } } + @Provides + @Singleton + protected LoadingCache> getFirewallRulesByVirtualMachine( + CacheLoader> getFirewallRules) { + return CacheBuilder.newBuilder().build(getFirewallRules); + } + + @Singleton + public static class GetFirewallRulesByVirtualMachine extends CacheLoader> { + private final CloudStackClient client; + + @Inject + public GetFirewallRulesByVirtualMachine(CloudStackClient client) { + this.client = checkNotNull(client, "client"); + } + + /** + * @throws ResourceNotFoundException + * when there is no ip forwarding rule available for the VM + */ + @Override + public Set load(String input) { + String publicIPId = client.getVirtualMachineClient().getVirtualMachine(input).getPublicIPId(); + Set rules = client.getFirewallClient().listFirewallRules(ListFirewallRulesOptions.Builder.ipAddressId(publicIPId)); + return rules != null ? rules : ImmutableSet.of(); + } + } + @Provides @Singleton public Map optionsConverters(){ diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/functions/VirtualMachineToNodeMetadata.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/functions/VirtualMachineToNodeMetadata.java index 193f3812bd..59a4418385 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/functions/VirtualMachineToNodeMetadata.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/functions/VirtualMachineToNodeMetadata.java @@ -134,6 +134,9 @@ public class VirtualMachineToNodeMetadata implements Function> vmToRules; private final Map credentialStore; private final Map optionsConverters; @@ -99,7 +104,9 @@ public class CloudStackComputeServiceAdapter implements @Memoized Supplier> networkSupplier, BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult, StaticNATVirtualMachineInNetwork.Factory staticNATVMInNetwork, - CreatePortForwardingRulesForIP setupPortForwardingRulesForIP, LoadingCache> vmToRules, + CreatePortForwardingRulesForIP setupPortForwardingRulesForIP, + CreateFirewallRulesForIP setupFirewallRulesForIP, + LoadingCache> vmToRules, Map credentialStore, Map optionsConverters, Supplier> zoneIdToZone) { this.client = checkNotNull(client, "client"); @@ -109,6 +116,7 @@ public class CloudStackComputeServiceAdapter implements "blockUntilJobCompletesAndReturnResult"); this.staticNATVMInNetwork = checkNotNull(staticNATVMInNetwork, "staticNATVMInNetwork"); this.setupPortForwardingRulesForIP = checkNotNull(setupPortForwardingRulesForIP, "setupPortForwardingRulesForIP"); + this.setupFirewallRulesForIP = checkNotNull(setupFirewallRulesForIP, "setupFirewallRulesForIP"); this.vmToRules = checkNotNull(vmToRules, "vmToRules"); this.credentialStore = checkNotNull(credentialStore, "credentialStore"); this.optionsConverters = optionsConverters; @@ -166,6 +174,7 @@ public class CloudStackComputeServiceAdapter implements AsyncCreateResponse job = client.getVirtualMachineClient().deployVirtualMachineInZone(zoneId, serviceOfferingId, templateId, options); VirtualMachine vm = blockUntilJobCompletesAndReturnResult. apply(job); + logger.debug("--- virtualmachine: %s", vm); LoginCredentials credentials = null; if (vm.isPasswordEnabled()) { assert vm.getPassword() != null : vm; @@ -174,15 +183,23 @@ public class CloudStackComputeServiceAdapter implements credentials = LoginCredentials.fromCredentials(credentialStore.get("keypair#" + templateOptions.getKeyPair())); } if (templateOptions.shouldSetupStaticNat()) { + Capabilities capabilities = client.getConfigurationClient().listCapabilities(); // TODO: possibly not all network ids, do we want to do this for (String networkId : options.getNetworkIds()) { logger.debug(">> creating static NAT for virtualMachine(%s) in network(%s)", vm.getId(), networkId); PublicIPAddress ip = staticNATVMInNetwork.create(networks.get(networkId)).apply(vm); logger.trace("<< static NATed IPAddress(%s) to virtualMachine(%s)", ip.getId(), vm.getId()); + vm = client.getVirtualMachineClient().getVirtualMachine(vm.getId()); List ports = Ints.asList(templateOptions.getInboundPorts()); - logger.debug(">> setting up IP forwarding for IPAddress(%s) rules(%s)", ip.getId(), ports); - Set rules = setupPortForwardingRulesForIP.apply(ip, ports); - logger.trace("<< setup %d IP forwarding rules on IPAddress(%s)", rules.size(), ip.getId()); + if (capabilities.getCloudStackVersion().startsWith("2")) { + logger.debug(">> setting up IP forwarding for IPAddress(%s) rules(%s)", ip.getId(), ports); + Set 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 rules = setupFirewallRulesForIP.apply(ip, ports); + logger.trace("<< setup %d firewall rules on IPAddress(%s)", rules.size(), ip.getId()); + } } } return new NodeAndInitialCredentials(vm, vm.getId() + "", credentials); @@ -241,10 +258,13 @@ public class CloudStackComputeServiceAdapter implements // 1) Delete IP forwarding rules associated with IP. Set ipAddresses = deleteIPForwardingRulesForVMAndReturnDistinctIPs(virtualMachineId); - // 2) Disable static nat rule for the IP. + // 2) Delete firewall rules associated with IP. + ipAddresses.addAll(deleteFirewallRulesForVMAndReturnDistinctIPs(virtualMachineId)); + + // 3) Disable static nat rule for the IP. disableStaticNATOnIPAddresses(ipAddresses); - // 3) Only after 1 and 2 release the IP address. + // 4) Only after 1 and 2 release the IP address. disassociateIPAddresses(ipAddresses); destroyVirtualMachine(virtualMachineId); @@ -305,6 +325,26 @@ public class CloudStackComputeServiceAdapter implements return ipAddresses; } + public Set deleteFirewallRulesForVMAndReturnDistinctIPs(String virtualMachineId) { + // immutable doesn't permit duplicates + Set ipAddresses = Sets.newLinkedHashSet(); + + String publicIpId = client.getVirtualMachineClient().getVirtualMachine(virtualMachineId).getPublicIPId(); + if (publicIpId != null) { + Set firewallRules = client.getFirewallClient() + .listFirewallRules(ListFirewallRulesOptions.Builder.ipAddressId(client.getVirtualMachineClient().getVirtualMachine(virtualMachineId).getPublicIPId())); + + for (FirewallRule rule : firewallRules) { + if (!FirewallRule.State.fromValue("DELETING").equals(rule.getState())) { + ipAddresses.add(rule.getIpAddressId()); + client.getFirewallClient().deleteFirewallRule(rule.getId()); + logger.debug(">> deleting FirewallRule(%s)", rule.getId()); + } + } + } + return ipAddresses; + } + public void awaitCompletion(Iterable jobs) { logger.debug(">> awaiting completion of jobs(%s)", jobs); for (String job : jobs) diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/FirewallRule.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/FirewallRule.java index 9c3a188f49..8b94d6796a 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/FirewallRule.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/FirewallRule.java @@ -55,7 +55,7 @@ public class FirewallRule implements Comparable { // Rules in this state can not be sent to network elements. ADD, // Add means the rule has been created and has gone through network rule conflict detection. ACTIVE, // Rule has been sent to the network elements and reported to be active. - DELETEING, // Revoke means this rule has been revoked. If this rule has been sent to the + DELETING, // Revoke means this rule has been revoked. If this rule has been sent to the // network elements, the rule will be deleted from database. UNKNOWN; diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/GuestIPType.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/GuestIPType.java index 40a37186f6..60f378f9ce 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/GuestIPType.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/GuestIPType.java @@ -34,12 +34,19 @@ public enum GuestIPType { * network. Dhcp role is played by domain router. */ VIRTUAL, + /** * traffic directly to the network and VMs created here are assigned an IP * directly from the network as configured */ DIRECT, + /** + * TODO: add comments to explain the meaning (cs3 only) + */ + SHARED, + ISOLATED, + UNRECOGNIZED; @Override diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/PortForwardingRule.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/PortForwardingRule.java index 7d0daf2dcf..ee7f10c12d 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/PortForwardingRule.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/PortForwardingRule.java @@ -55,7 +55,7 @@ public class PortForwardingRule implements Comparable { // Rules in this state can not be sent to network elements. ADD, // Add means the rule has been created and has gone through network rule conflict detection. ACTIVE, // Rule has been sent to the network elements and reported to be active. - DELETEING, // Revoke means this rule has been revoked. If this rule has been sent to the + DELETING, // Revoke means this rule has been revoked. If this rule has been sent to the // network elements, the rule will be deleted from database. UNKNOWN; diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/Template.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/Template.java index 2a455212da..52fc1efcf5 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/Template.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/domain/Template.java @@ -62,6 +62,7 @@ public class Template implements Comparable