From d209388d64354d6d03bb7b986f7bac411cdc825f Mon Sep 17 00:00:00 2001 From: Andrei Savu Date: Tue, 31 Jan 2012 17:59:26 +0200 Subject: [PATCH] Collect all IP addresses before building the NodeMetadata object --- .../VirtualMachineToNodeMetadata.java | 99 ++++++++------ .../VirtualMachineToNodeMetadataTest.java | 124 ++++++++++++++---- 2 files changed, 151 insertions(+), 72 deletions(-) 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 1328f0fb76..8166526b33 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 @@ -18,19 +18,19 @@ */ package org.jclouds.cloudstack.compute.functions; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.collect.Iterables.filter; -import static com.google.common.collect.Iterables.transform; -import static org.jclouds.compute.util.ComputeServiceUtils.parseGroupFromName; - -import java.util.Map; -import java.util.Set; - -import javax.annotation.Nullable; -import javax.inject.Inject; -import javax.inject.Singleton; - +import com.google.common.base.Function; +import com.google.common.base.Predicate; +import com.google.common.base.Supplier; +import com.google.common.base.Throwables; +import com.google.common.cache.LoadingCache; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.Sets; +import com.google.common.util.concurrent.UncheckedExecutionException; import org.jclouds.cloudstack.domain.IPForwardingRule; +import org.jclouds.cloudstack.domain.NIC; import org.jclouds.cloudstack.domain.VirtualMachine; import org.jclouds.collect.FindResourceInSet; import org.jclouds.collect.Memoized; @@ -46,18 +46,21 @@ import org.jclouds.rest.ResourceNotFoundException; import org.jclouds.util.InetAddresses2; import org.jclouds.util.Throwables2; -import com.google.common.base.Function; -import com.google.common.base.Predicate; -import com.google.common.base.Supplier; -import com.google.common.base.Throwables; -import com.google.common.cache.LoadingCache; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.util.concurrent.UncheckedExecutionException; +import javax.annotation.Nullable; +import javax.inject.Inject; +import javax.inject.Singleton; +import java.util.Map; +import java.util.Set; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.Iterables.filter; +import static com.google.common.collect.Iterables.transform; +import static com.google.common.collect.Sets.newHashSet; +import static org.jclouds.compute.util.ComputeServiceUtils.parseGroupFromName; +import static org.jclouds.util.InetAddresses2.isPrivateIPAddress; /** - * @author Adrian Cole + * @author Adrian Cole, Andrei Savu */ @Singleton public class VirtualMachineToNodeMetadata implements Function { @@ -108,7 +111,7 @@ public class VirtualMachineToNodeMetadata implements Function publicAddresses = newHashSet(), privateAddresses = newHashSet(); if (from.getIPAddress() != null) { - boolean isPrivate = InetAddresses2.isPrivateIPAddress(from.getIPAddress()); - Set addresses = ImmutableSet. of(from.getIPAddress()); - if (isPrivate) - builder.privateAddresses(addresses); - else - builder.publicAddresses(addresses); + boolean isPrivate = isPrivateIPAddress(from.getIPAddress()); + if (isPrivate) { + privateAddresses.add(from.getIPAddress()); + } else { + publicAddresses.add(from.getIPAddress()); + } + } + for (NIC nic : from.getNICs()) { + if (nic.getIPAddress() != null) { + if (isPrivateIPAddress(nic.getIPAddress())) { + privateAddresses.add(nic.getIPAddress()); + } else { + publicAddresses.add(nic.getIPAddress()); + } + } } try { - - builder.publicAddresses(transform( - filter(getIPForwardingRulesByVirtualMachine.getUnchecked(from.getId()), - new Predicate() { - @Override - public boolean apply(@Nullable IPForwardingRule rule) { - return !"Deleting".equals(rule.getState()); - } - }), new Function() { + /* Also add to the list of public IPs any public IP address that has a + forwarding rule that links to this machine */ + Iterables.addAll(publicAddresses, transform( + filter(getIPForwardingRulesByVirtualMachine.getUnchecked(from.getId()), + new Predicate() { @Override - public String apply(@Nullable IPForwardingRule rule) { - return rule.getIPAddress(); + public boolean apply(@Nullable IPForwardingRule rule) { + return !"Deleting".equals(rule.getState()); } - })); + }), new Function() { + @Override + public String apply(@Nullable IPForwardingRule rule) { + return rule.getIPAddress(); + } + })); } catch (UncheckedExecutionException e) { if (Throwables2.getFirstThrowableOfType(e, ResourceNotFoundException.class) == null) { Throwables.propagateIfPossible(e.getCause()); throw e; } } - return builder.build(); + return builder.privateAddresses(privateAddresses).publicAddresses(publicAddresses).build(); } @Singleton diff --git a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/functions/VirtualMachineToNodeMetadataTest.java b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/functions/VirtualMachineToNodeMetadataTest.java index 0b953d43a5..25fb7baade 100644 --- a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/functions/VirtualMachineToNodeMetadataTest.java +++ b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/functions/VirtualMachineToNodeMetadataTest.java @@ -18,14 +18,18 @@ */ package org.jclouds.cloudstack.compute.functions; -import static org.testng.Assert.assertEquals; - -import java.net.UnknownHostException; -import java.util.Set; - +import com.google.common.base.Supplier; +import com.google.common.base.Suppliers; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import org.jclouds.cloudstack.compute.functions.VirtualMachineToNodeMetadata.FindImageForVirtualMachine; import org.jclouds.cloudstack.compute.functions.VirtualMachineToNodeMetadata.FindLocationForVirtualMachine; +import org.jclouds.cloudstack.domain.GuestIPType; import org.jclouds.cloudstack.domain.IPForwardingRule; +import org.jclouds.cloudstack.domain.NIC; +import org.jclouds.cloudstack.domain.TrafficType; import org.jclouds.cloudstack.domain.VirtualMachine; import org.jclouds.cloudstack.parse.ListVirtualMachinesResponseTest; import org.jclouds.compute.domain.Hardware; @@ -34,19 +38,18 @@ import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.NodeMetadata; import org.jclouds.compute.domain.NodeMetadataBuilder; import org.jclouds.compute.domain.NodeState; +import org.jclouds.date.internal.SimpleDateFormatDateService; import org.jclouds.domain.Location; import org.jclouds.rest.ResourceNotFoundException; import org.testng.annotations.Test; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; +import java.net.UnknownHostException; +import java.util.Set; + +import static org.testng.Assert.assertEquals; /** - * @author Adrian Cole + * @author Adrian Cole, Andrei Savu */ @Test(groups = "unit", testName = "VirtualMachineToNodeMetadataTest") public class VirtualMachineToNodeMetadataTest { @@ -55,21 +58,21 @@ public class VirtualMachineToNodeMetadataTest { public void testApplyWhereVirtualMachineWithIPForwardingRule() throws UnknownHostException { Supplier> locationSupplier = Suppliers.> ofInstance(ImmutableSet - . of(ZoneToLocationTest.one, ZoneToLocationTest.two)); + .of(ZoneToLocationTest.one, ZoneToLocationTest.two)); Supplier> imageSupplier = Suppliers.> ofInstance(ImmutableSet - . of(TemplateToImageTest.one, TemplateToImageTest.two)); + .of(TemplateToImageTest.one, TemplateToImageTest.two)); VirtualMachineToNodeMetadata parser = new VirtualMachineToNodeMetadata(new FindLocationForVirtualMachine( locationSupplier), new FindImageForVirtualMachine( imageSupplier), CacheBuilder.newBuilder().> build( - new CacheLoader>() { + new CacheLoader>() { - @Override - public Set load(Long arg0) throws Exception { - return ImmutableSet.of(IPForwardingRule.builder().id(1234l).IPAddress("1.1.1.1").build()); - } + @Override + public Set load(Long arg0) throws Exception { + return ImmutableSet.of(IPForwardingRule.builder().id(1234l).IPAddress("1.1.1.1").build()); + } - })); + })); // notice if we've already parsed this properly here, we can rely on it. VirtualMachine guest = Iterables.get(new ListVirtualMachinesResponseTest().expected(), 0); @@ -87,25 +90,88 @@ public class VirtualMachineToNodeMetadataTest { } + @Test + public void testApplyWhereVirtualMachineHasNoIpForwardingRuleAndAPublicIP() throws UnknownHostException { + + Supplier> locationSupplier = Suppliers.> ofInstance(ImmutableSet + .of(ZoneToLocationTest.one, ZoneToLocationTest.two)); + + Supplier> imageSupplier = Suppliers.> ofInstance(ImmutableSet + .of(TemplateToImageTest.one, TemplateToImageTest.two)); + + VirtualMachineToNodeMetadata parser = new VirtualMachineToNodeMetadata(new FindLocationForVirtualMachine( + locationSupplier), new FindImageForVirtualMachine( + imageSupplier), CacheBuilder.newBuilder().> build( + new CacheLoader>() { + @Override + public Set load(Long arg0) throws Exception { + return ImmutableSet.of(); + } + + })); + + VirtualMachine guest =VirtualMachine.builder() + .id(54) + .name("i-3-54-VM") + .displayName("i-3-54-VM") + .account("adrian") + .domainId(1) + .domain("ROOT") + .created(new SimpleDateFormatDateService().iso8601SecondsDateParse("2011-02-16T14:28:37-0800")) + .state(VirtualMachine.State.STARTING) + .isHAEnabled(false) + .zoneId(1) + .zoneName("San Jose 1") + .templateId(2) + .templateName("CentOS 5.3(64-bit) no GUI (XenServer)") + .templateDisplayText("CentOS 5.3(64-bit) no GUI (XenServer)") + .passwordEnabled(false) + .serviceOfferingId(1) + .serviceOfferingName("Small Instance") + .cpuCount(1) + .cpuSpeed(500) + .memory(512) + .guestOSId(11) + .rootDeviceId(0) + .rootDeviceType("NetworkFilesystem") + .jobId(63l) + .jobStatus(0) + .nics(ImmutableSet.of(NIC.builder().id(72).networkId(204).netmask("255.255.255.0").gateway("1.1.1.1") + .IPAddress("1.1.1.5").trafficType(TrafficType.GUEST).guestIPType(GuestIPType.VIRTUAL) + .isDefault(true).build())).hypervisor("XenServer").build(); + + NodeMetadata node = parser.apply(guest); + + assertEquals( + node.toString(), + new NodeMetadataBuilder().id("54").providerId("54").name("i-3-54-VM").group("i-3") + .location(ZoneToLocationTest.one).state(NodeState.PENDING).hostname("i-3-54-VM") + .privateAddresses(ImmutableSet.of()) + .publicAddresses(ImmutableSet.of("1.1.1.5")) + .hardware(addHypervisor(ServiceOfferingToHardwareTest.one, "XenServer")) + .imageId(TemplateToImageTest.one.getId()) + .operatingSystem(TemplateToImageTest.one.getOperatingSystem()).build().toString()); + } + @Test public void testApplyWhereVirtualMachineWithNoPassword() throws UnknownHostException { Supplier> locationSupplier = Suppliers.> ofInstance(ImmutableSet - . of(ZoneToLocationTest.one, ZoneToLocationTest.two)); + .of(ZoneToLocationTest.one, ZoneToLocationTest.two)); Supplier> imageSupplier = Suppliers.> ofInstance(ImmutableSet - . of(TemplateToImageTest.one, TemplateToImageTest.two)); + .of(TemplateToImageTest.one, TemplateToImageTest.two)); VirtualMachineToNodeMetadata parser = new VirtualMachineToNodeMetadata(new FindLocationForVirtualMachine( locationSupplier), new FindImageForVirtualMachine( imageSupplier), CacheBuilder.newBuilder().> build( - new CacheLoader>() { + new CacheLoader>() { - @Override - public Set load(Long arg0) throws Exception { - throw new ResourceNotFoundException("no ip forwarding rule for: " + arg0); - } + @Override + public Set load(Long arg0) throws Exception { + throw new ResourceNotFoundException("no ip forwarding rule for: " + arg0); + } - })); + })); // notice if we've already parsed this properly here, we can rely on it. VirtualMachine guest = Iterables.get(new ListVirtualMachinesResponseTest().expected(), 0); @@ -121,7 +187,7 @@ public class VirtualMachineToNodeMetadataTest { .imageId(TemplateToImageTest.one.getId()) .operatingSystem(TemplateToImageTest.one.getOperatingSystem()).build().toString()); } - + protected Hardware addHypervisor(Hardware in, String hypervisor) { return HardwareBuilder.fromHardware(in).hypervisor(hypervisor).build(); }