normalized image parsing across canonical and alestic; added tests to kickout non-amis and test images

aws/core/src/main/java/org/jclouds/aws/ec2/EC2PropertiesBuilder.java

@@ -41,7 +41,8 @@ public class EC2PropertiesBuilder extends PropertiesBuilder {
       Properties properties = super.defaultProperties();
       properties.setProperty(PROPERTY_EC2_ENDPOINT, "https://ec2.us-east-1.amazonaws.com");
       properties.setProperty(PROPERTY_AWS_EXPIREINTERVAL, "60");
-      properties.setProperty(PROPERTY_EC2_AMI_OWNERS, "063491364108");
+      // alestic and canonical
+      properties.setProperty(PROPERTY_EC2_AMI_OWNERS, "063491364108,099720109477");
       return properties;
    }
 

aws/core/src/main/java/org/jclouds/aws/ec2/compute/config/EC2ComputeServiceContextModule.java

@@ -55,6 +55,7 @@ import org.jclouds.aws.ec2.domain.RunningInstance;
 import org.jclouds.aws.ec2.services.InstanceClient;
 import org.jclouds.compute.ComputeService;
 import org.jclouds.compute.ComputeServiceContext;
+import org.jclouds.compute.domain.Architecture;
 import org.jclouds.compute.domain.ComputeMetadata;
 import org.jclouds.compute.domain.Image;
 import org.jclouds.compute.domain.NodeMetadata;
@@ -109,7 +110,8 @@ public class EC2ComputeServiceContextModule extends EC2ContextModule {
    TemplateBuilder provideTemplate(Map<String, ? extends Location> locations,
             Map<String, ? extends Image> images, Map<String, ? extends Size> sizes,
             Location defaultLocation) {
-      return new TemplateBuilderImpl(locations, images, sizes, defaultLocation).osFamily(UBUNTU);
+      return new TemplateBuilderImpl(locations, images, sizes, defaultLocation).architecture(
+               Architecture.X86_32).osFamily(UBUNTU);
    }
 
    @Singleton
@@ -291,7 +293,7 @@ public class EC2ComputeServiceContextModule extends EC2ContextModule {
    @Singleton
    @Named(PROPERTY_EC2_AMI_OWNERS)
    String[] amiOwners(@Named(PROPERTY_EC2_AMI_OWNERS) String amiOwners) {
-      return Iterables.toArray(Splitter.on('.').split(amiOwners), String.class);
+      return Iterables.toArray(Splitter.on(',').split(amiOwners), String.class);
    }
 
    @Provides
@@ -311,7 +313,7 @@ public class EC2ComputeServiceContextModule extends EC2ContextModule {
             if (image != null)
                images.add(image);
             else
-               holder.logger.debug("<< images(%s) didn't parse", from.getId());
+               holder.logger.trace("<< image(%s) didn't parse", from.getId());
          }
       }
       holder.logger.debug("<< images(%d)", images.size());

aws/core/src/main/java/org/jclouds/aws/ec2/compute/functions/ImageParser.java

@@ -18,6 +18,7 @@
  */
 package org.jclouds.aws.ec2.compute.functions;
 
+import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -25,6 +26,7 @@ import javax.annotation.Resource;
 import javax.inject.Named;
 import javax.inject.Singleton;
 
+import org.jclouds.aws.ec2.domain.Image.ImageType;
 import org.jclouds.compute.domain.Architecture;
 import org.jclouds.compute.domain.Image;
 import org.jclouds.compute.domain.OsFamily;
@@ -35,32 +37,48 @@ import org.jclouds.logging.Logger;
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableMap;
 
+/**
+ * @author Adrian Cole
+ */
 @Singleton
-public class ImageParser implements
+public class ImageParser implements Function<org.jclouds.aws.ec2.domain.Image, Image> {
-         Function<org.jclouds.aws.ec2.domain.Image, Image> {
    @Resource
    @Named(ComputeServiceConstants.COMPUTE_LOGGER)
    protected Logger logger = Logger.NULL;
 
-   // alestic-32-eu-west-1/debian-4.0-etch-base-20081130
+   public static final Pattern CANONICAL_PATTERN = Pattern
-   public static final Pattern ALESTIC_PATTERN = Pattern
             .compile(".*/([^-]*)-([^-]*)-.*-(.*)(\\.manifest.xml)?");
 
+   public static final Map<String, String> NAME_VERSION_MAP = ImmutableMap
+            .<String, String> builder().put("hardy", "8.04").put("intrepid", "8.10").put("jaunty",
+                     "9.04").put("karmic", "9.10").put("lucid", "10.04").put("maverick", "10.10")
+            .build();
+
    @Override
    public Image apply(org.jclouds.aws.ec2.domain.Image from) {
+      if (from.getImageLocation().indexOf("test") != -1) {
+         logger.trace("skipping test image(%s)", from.getId());
+         return null;
+      }
+      if (from.getImageType() != ImageType.MACHINE) {
+         logger.trace("skipping as not a machine image(%s)", from.getId());
+         return null;
+      }
       OsFamily os = null;
       String name = "";
       String description = from.getDescription() != null ? from.getDescription() : from
                .getImageLocation();
       String osDescription = from.getImageLocation();
       String version = "";
-
+      Matcher matcher = CANONICAL_PATTERN.matcher(from.getImageLocation());
-      Matcher matcher = ALESTIC_PATTERN.matcher(from.getImageLocation());
       if (matcher.find()) {
          try {
             os = OsFamily.fromValue(matcher.group(1));
             name = matcher.group(2);// TODO no field for os version
-            version = matcher.group(3).replace("\\.manifest.xml", "");
+            // normalize versions across ubuntu from alestic and canonical
+            if (NAME_VERSION_MAP.containsKey(name))
+               name = NAME_VERSION_MAP.get(name);
+            version = matcher.group(3).replace(".manifest.xml", "");
          } catch (IllegalArgumentException e) {
             logger.debug("<< didn't match os(%s)", matcher.group(1));
          }

aws/core/src/main/java/org/jclouds/aws/ec2/xml/DescribeImagesResponseHandler.java

@@ -50,7 +50,7 @@ public class DescribeImagesResponseHandler extends ParseSax.HandlerWithResult<Se
    @Resource
    protected Logger logger = Logger.NULL;
 
-   private Set<Image> contents = Sets.newHashSet();
+   private Set<Image> contents = Sets.newLinkedHashSet();
    private StringBuilder currentText = new StringBuilder();
 
    private Architecture architecture;

aws/core/src/test/java/org/jclouds/aws/ec2/compute/EC2ComputeServiceLiveTest.java

@@ -50,10 +50,16 @@ public class EC2ComputeServiceLiveTest extends BaseComputeServiceLiveTest {
    @Test
    public void testTemplateBuilder() {
       Template defaultTemplate = client.templateBuilder().build();
+      assert (defaultTemplate.getImage().getId().startsWith("ami-")) : defaultTemplate;
+      assertEquals(defaultTemplate.getImage().getName(), "9.10");
       assertEquals(defaultTemplate.getImage().getArchitecture(), Architecture.X86_32);
       assertEquals(defaultTemplate.getImage().getOsFamily(), OsFamily.UBUNTU);
       assertEquals(defaultTemplate.getLocation().getId(), "us-east-1");
       assertEquals(defaultTemplate.getSize().getCores(), 1.0d);
+      client.templateBuilder().osFamily(OsFamily.UBUNTU).smallest().architecture(
+               Architecture.X86_32).imageId("ami-7e28ca17").build();
+      client.templateBuilder().osFamily(OsFamily.UBUNTU).smallest().architecture(
+               Architecture.X86_32).imageId("ami-bb709dd2").build();
    }
 
    @Override

aws/core/src/test/java/org/jclouds/aws/ec2/functions/ImageParserTest.java

@@ -30,6 +30,7 @@ import org.jclouds.aws.domain.Region;
 import org.jclouds.aws.ec2.compute.functions.ImageParser;
 import org.jclouds.aws.ec2.domain.Image;
 import org.jclouds.aws.ec2.xml.DescribeImagesResponseHandler;
+import org.jclouds.compute.domain.OsFamily;
 import org.jclouds.http.functions.BaseHandlerTest;
 import org.jclouds.http.functions.ParseSax;
 import org.jclouds.rest.internal.GeneratedHttpRequest;
@@ -44,6 +45,80 @@ import com.google.common.collect.Iterables;
 @Test(groups = "unit", testName = "compute.PropertiesTest")
 public class ImageParserTest extends BaseHandlerTest {
 
+   public void testParseAlesticCanonicalImage() {
+      InputStream is = getClass().getResourceAsStream("/ec2/alestic_canonical.xml");
+
+      Set<Image> result = parseImages(is);
+      assertEquals(result.size(), 6);
+
+      ImageParser parser = new ImageParser();
+      org.jclouds.compute.domain.Image ubuntuHardy = parser.apply(Iterables.get(result, 0));
+
+      assertEquals(ubuntuHardy.getArchitecture(), org.jclouds.compute.domain.Architecture.X86_32);
+      assertEquals(ubuntuHardy.getDescription(),
+               "ubuntu-images-us/ubuntu-hardy-8.04-i386-server-20091130.manifest.xml");
+      assertEquals(ubuntuHardy.getId(), "ami-7e28ca17");
+      assertEquals(ubuntuHardy.getLocationId(), "default");
+      assertEquals(ubuntuHardy.getName(), "8.04");
+      assertEquals(ubuntuHardy.getOsDescription(),
+               "ubuntu-images-us/ubuntu-hardy-8.04-i386-server-20091130.manifest.xml");
+      assertEquals(ubuntuHardy.getOsFamily(), OsFamily.UBUNTU);
+      assertEquals(ubuntuHardy.getUserMetadata(), ImmutableMap.<String, String> of("owner",
+               "099720109477"));
+      assertEquals(ubuntuHardy.getVersion(), "20091130");
+
+      org.jclouds.compute.domain.Image alesticKarmic = parser.apply(Iterables.get(result, 1));
+
+      assertEquals(alesticKarmic.getArchitecture(), org.jclouds.compute.domain.Architecture.X86_32);
+      assertEquals(alesticKarmic.getDescription(),
+               "alestic/ubuntu-9.10-karmic-base-20090623.manifest.xml");
+      assertEquals(alesticKarmic.getId(), "ami-19a34270");
+      assertEquals(alesticKarmic.getLocationId(), "default");
+      assertEquals(alesticKarmic.getName(), "9.10");
+      assertEquals(alesticKarmic.getOsDescription(),
+               "alestic/ubuntu-9.10-karmic-base-20090623.manifest.xml");
+      assertEquals(alesticKarmic.getOsFamily(), OsFamily.UBUNTU);
+      assertEquals(alesticKarmic.getUserMetadata(), ImmutableMap.<String, String> of("owner",
+               "063491364108"));
+      assertEquals(alesticKarmic.getVersion(), "20090623");
+
+      org.jclouds.compute.domain.Image ubuntuKarmic = parser.apply(Iterables.get(result, 2));
+
+      assertEquals(ubuntuKarmic.getArchitecture(), org.jclouds.compute.domain.Architecture.X86_32);
+      assertEquals(ubuntuKarmic.getDescription(),
+               "ubuntu-images-us/ubuntu-karmic-9.10-i386-server-20100121.manifest.xml");
+      assertEquals(ubuntuKarmic.getId(), "ami-bb709dd2");
+      assertEquals(ubuntuKarmic.getLocationId(), "default");
+      assertEquals(ubuntuKarmic.getName(), "9.10");
+      assertEquals(ubuntuKarmic.getOsDescription(),
+               "ubuntu-images-us/ubuntu-karmic-9.10-i386-server-20100121.manifest.xml");
+      assertEquals(ubuntuKarmic.getOsFamily(), OsFamily.UBUNTU);
+      assertEquals(ubuntuKarmic.getUserMetadata(), ImmutableMap.<String, String> of("owner",
+               "099720109477"));
+      assertEquals(ubuntuKarmic.getVersion(), "20100121");
+
+      // should skip testing image
+      assert parser.apply(Iterables.get(result, 3)) == null;
+
+      org.jclouds.compute.domain.Image alesticHardy = parser.apply(Iterables.get(result, 4));
+
+      assertEquals(alesticHardy.getArchitecture(), org.jclouds.compute.domain.Architecture.X86_32);
+      assertEquals(alesticHardy.getDescription(),
+               "alestic/ubuntu-8.04-hardy-base-20080905.manifest.xml");
+      assertEquals(alesticHardy.getId(), "ami-c0fa1ea9");
+      assertEquals(alesticHardy.getLocationId(), "default");
+      assertEquals(alesticHardy.getName(), "8.04");
+      assertEquals(alesticHardy.getOsDescription(),
+               "alestic/ubuntu-8.04-hardy-base-20080905.manifest.xml");
+      assertEquals(alesticHardy.getOsFamily(), OsFamily.UBUNTU);
+      assertEquals(alesticHardy.getUserMetadata(), ImmutableMap.<String, String> of("owner",
+               "063491364108"));
+      assertEquals(alesticHardy.getVersion(), "20080905");
+      
+      // should skip kernel
+      assert parser.apply(Iterables.get(result, 5)) == null;
+   }
+
    public void testParseVostokImage() {
       InputStream is = getClass().getResourceAsStream("/ec2/vostok.xml");
 
@@ -78,7 +153,7 @@ public class ImageParserTest extends BaseHandlerTest {
 
    private void addDefaultRegionToHandler(ParseSax.HandlerWithResult<?> handler) {
       GeneratedHttpRequest<?> request = createMock(GeneratedHttpRequest.class);
-      expect(request.getArgs()).andReturn(new Object[] { Region.DEFAULT });
+      expect(request.getArgs()).andReturn(new Object[] { Region.DEFAULT }).atLeastOnce();
       replay(request);
       handler.setContext(request);
    }

aws/core/src/test/resources/ec2/alestic_canonical.xml

@@ -0,0 +1,82 @@
+<?xml version="1.0"?>
+<DescribeImagesResponse xmlns="http://ec2.amazonaws.com/doc/2009-11-30/">
+    <requestId>6104eee1-affd-49d7-92a0-516cab8a8ba6</requestId>
+    <imagesSet>
+        <item>
+            <imageId>ami-7e28ca17</imageId>
+            <imageLocation>ubuntu-images-us/ubuntu-hardy-8.04-i386-server-20091130.manifest.xml</imageLocation>
+            <imageState>available</imageState>
+            <imageOwnerId>099720109477</imageOwnerId>
+            <isPublic>true</isPublic>
+            <architecture>i386</architecture>
+            <imageType>machine</imageType>
+            <kernelId>aki-92ba58fb</kernelId>
+            <ramdiskId>ari-94ba58fd</ramdiskId>
+            <rootDeviceType>instance-store</rootDeviceType>
+            <blockDeviceMapping />
+        </item>
+        <item>
+            <imageId>ami-19a34270</imageId>
+            <imageLocation>alestic/ubuntu-9.10-karmic-base-20090623.manifest.xml</imageLocation>
+            <imageState>available</imageState>
+            <imageOwnerId>063491364108</imageOwnerId>
+            <isPublic>true</isPublic>
+            <architecture>i386</architecture>
+            <imageType>machine</imageType>
+            <kernelId>aki-a71cf9ce</kernelId>
+            <ramdiskId>ari-a51cf9cc</ramdiskId>
+            <rootDeviceType>instance-store</rootDeviceType>
+            <blockDeviceMapping />
+        </item>
+        <item>
+            <imageId>ami-bb709dd2</imageId>
+            <imageLocation>ubuntu-images-us/ubuntu-karmic-9.10-i386-server-20100121.manifest.xml</imageLocation>
+            <imageState>available</imageState>
+            <imageOwnerId>099720109477</imageOwnerId>
+            <isPublic>true</isPublic>
+            <architecture>i386</architecture>
+            <imageType>machine</imageType>
+            <kernelId>aki-5f15f636</kernelId>
+            <ramdiskId>ari-d5709dbc</ramdiskId>
+            <rootDeviceType>instance-store</rootDeviceType>
+            <blockDeviceMapping />
+        </item>
+        <item>
+            <imageId>ami-190fe070</imageId>
+            <imageLocation>ubuntu-images-testing-us/ubuntu-lucid-daily-amd64-desktop-20100317.manifest.xml</imageLocation>
+            <imageState>available</imageState>
+            <imageOwnerId>099720109477</imageOwnerId>
+            <isPublic>true</isPublic>
+            <architecture>x86_64</architecture>
+            <imageType>machine</imageType>
+            <kernelId>aki-11c72878</kernelId>
+            <name>ubuntu-images-testing/ubuntu-lucid-daily-amd64-desktop-20100317</name>
+            <rootDeviceType>instance-store</rootDeviceType>
+            <blockDeviceMapping />
+        </item>
+        <item>
+            <imageId>ami-c0fa1ea9</imageId>
+            <imageLocation>alestic/ubuntu-8.04-hardy-base-20080905.manifest.xml</imageLocation>
+            <imageState>available</imageState>
+            <imageOwnerId>063491364108</imageOwnerId>
+            <isPublic>true</isPublic>
+            <architecture>i386</architecture>
+            <imageType>machine</imageType>
+            <kernelId>aki-a71cf9ce</kernelId>
+            <ramdiskId>ari-a51cf9cc</ramdiskId>
+            <rootDeviceType>instance-store</rootDeviceType>
+            <blockDeviceMapping />
+        </item>
+        <item>
+            <imageId>aki-fd15f694</imageId>
+            <imageLocation>ubuntu-kernels-us/ubuntu-karmic-amd64-linux-image-2.6.31-302-ec2-v-2.6.31-302.7-kernel.img.manifest.xml</imageLocation>
+            <imageState>available</imageState>
+            <imageOwnerId>099720109477</imageOwnerId>
+            <isPublic>true</isPublic>
+            <architecture>x86_64</architecture>
+            <imageType>kernel</imageType>
+            <rootDeviceType>instance-store</rootDeviceType>
+            <blockDeviceMapping />
+        </item>
+    </imagesSet>
+</DescribeImagesResponse>