handle case when image location is null (ex. not scoped to a location)

work on templates where image and hardware have no location

better warning messages on templates we cannot use

lookup images less often in vcloud

apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java

@@ -94,6 +94,13 @@ public class EC2TemplateBuilderImplTest extends TemplateBuilderImplTest {
             optionsProvider, templateBuilderProvider, Suppliers.<LoadingCache<RegionAndName, ? extends Image>>ofInstance(imageMap));
    }
    
+
+   @Override
+   @Test
+   public void testHardwareWithImageIdPredicateOnlyAcceptsImageWhenLocationNull() {
+      // not possible to have null region in ec2
+   }
+   
    @SuppressWarnings("unchecked")
    @Test
    public void testParseOnDemand() {

apis/vcloud/src/main/java/org/jclouds/vcloud/VCloudPropertiesBuilder.java

@@ -49,7 +49,8 @@ public class VCloudPropertiesBuilder extends PropertiesBuilder {
       properties.setProperty(PROPERTY_VCLOUD_DEFAULT_FENCEMODE, FenceMode.BRIDGED.toString());
       // TODO integrate this with the {@link ComputeTimeouts} instead of having a single timeout for
       // everything.
-      properties.setProperty(PROPERTY_VCLOUD_TIMEOUT_TASK_COMPLETED, 600l * 1000l + "");
+      properties.setProperty(PROPERTY_VCLOUD_TIMEOUT_TASK_COMPLETED, 1200l * 1000l + "");
+      properties.setProperty(PROPERTY_SESSION_INTERVAL, 300 + "");
       return properties;
    }
 

apis/vcloud/src/main/java/org/jclouds/vcloud/compute/functions/ValidateVAppTemplateAndReturnEnvelopeOrThrowIllegalArgumentException.java

@@ -0,0 +1,78 @@
+/**
+ * Licensed to jclouds, Inc. (jclouds) under one or more
+ * contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  jclouds licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.jclouds.vcloud.compute.functions;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.net.URI;
+import java.util.concurrent.ExecutionException;
+
+import javax.annotation.Resource;
+import javax.inject.Inject;
+import javax.inject.Singleton;
+
+import org.jclouds.logging.Logger;
+import org.jclouds.ovf.Envelope;
+import org.jclouds.vcloud.domain.VAppTemplate;
+
+import com.google.common.base.Function;
+import com.google.common.cache.LoadingCache;
+
+/**
+ * @author Adrian Cole
+ */
+@Singleton
+public class ValidateVAppTemplateAndReturnEnvelopeOrThrowIllegalArgumentException implements
+         Function<VAppTemplate, Envelope> {
+
+   @Resource
+   protected Logger logger = Logger.NULL;
+
+   private final LoadingCache<URI, Envelope> envelopes;
+
+   @Inject
+   protected ValidateVAppTemplateAndReturnEnvelopeOrThrowIllegalArgumentException(LoadingCache<URI, Envelope> envelopes) {
+      this.envelopes = checkNotNull(envelopes, "envelopes");
+   }
+
+   @Override
+   public Envelope apply(VAppTemplate from) {
+      checkArgument(from.getChildren().size() == 1, "multiple vms are not supported: %s", from);
+
+      checkArgument(from.getNetworkSection().getNetworks().size() == 1,
+               "multiple network connections are not supported: %s", from);
+
+      checkArgument(from.isOvfDescriptorUploaded(), "ovf descriptor is not uploaded: %s", from);
+      Envelope ovf = getOVFForVAppTemplateAndValidate(from);
+      return ovf;
+   }
+
+   private Envelope getOVFForVAppTemplateAndValidate(VAppTemplate from) throws IllegalArgumentException {
+      Envelope ovf;
+      try {
+         ovf = envelopes.get(from.getHref());
+         checkArgument(ovf.getVirtualSystem().getVirtualHardwareSections().size() > 0,
+                  "no hardware sections exist in ovf %s", ovf);
+      } catch (ExecutionException e) {
+         throw new IllegalArgumentException("no ovf envelope found for: " + from, e);
+      }
+      return ovf;
+   }
+}

apis/vcloud/src/main/java/org/jclouds/vcloud/compute/strategy/VCloudComputeServiceAdapter.java

@@ -92,7 +92,24 @@ public class VCloudComputeServiceAdapter implements ComputeServiceAdapter<VApp,
 
    @Override
    public Iterable<VAppTemplate> listHardwareProfiles() {
-      return templates.get();
+      return supportedTemplates();
+   }
+
+   private Iterable<VAppTemplate> supportedTemplates() {
+      return Iterables.filter(templates.get(), new Predicate<VAppTemplate>() {
+
+         @Override
+         public boolean apply(VAppTemplate from) {
+            try {
+               templateToEnvelope.apply(from);
+            } catch (IllegalArgumentException e){
+               logger.warn("Unsupported: "+ e.getMessage());
+               return false;
+            }
+            return true;
+         }
+
+      });
    }
 
    @Override

compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java

@@ -25,10 +25,11 @@ import static com.google.common.collect.Iterables.find;
 import static com.google.common.collect.Iterables.size;
 import static com.google.common.collect.Iterables.transform;
 import static com.google.common.collect.Lists.newArrayList;
+import static java.lang.String.format;
 import static org.jclouds.compute.util.ComputeServiceUtils.getCores;
 import static org.jclouds.compute.util.ComputeServiceUtils.getCoresAndSpeed;
 import static org.jclouds.compute.util.ComputeServiceUtils.getSpace;
-import static java.lang.String.format;
+
 import java.util.List;
 import java.util.NoSuchElementException;
 import java.util.Set;
@@ -59,6 +60,7 @@ import com.google.common.base.Predicates;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Ordering;
 import com.google.common.primitives.Doubles;
@@ -67,7 +69,6 @@ import com.google.common.primitives.Doubles;
  * 
  * @author Adrian Cole
  */
-@SuppressWarnings("unchecked")
 public class TemplateBuilderImpl implements TemplateBuilder {
 
    @Resource
@@ -138,7 +139,7 @@ public class TemplateBuilderImpl implements TemplateBuilder {
     * 
     * If the input location is a parent of the specified location, then we are ok.
     */
-   private final Predicate<ComputeMetadata> locationPredicate = new Predicate<ComputeMetadata>() {
+   final Predicate<ComputeMetadata> locationPredicate = new Predicate<ComputeMetadata>() {
       @Override
       public boolean apply(ComputeMetadata input) {
          boolean returnVal = true;
@@ -314,7 +315,8 @@ public class TemplateBuilderImpl implements TemplateBuilder {
             if (input.getName() == null)
                returnVal = false;
             else
-               returnVal = input.getName().contains(imageName) || input.getName().matches(imageName);
+               returnVal = input.getName().equals(imageName) || input.getName().contains(imageName)
+                        || input.getName().matches(imageName);
          }
          return returnVal;
       }
@@ -324,6 +326,7 @@ public class TemplateBuilderImpl implements TemplateBuilder {
          return "imageName(" + imageName + ")";
       }
    };
+   
    private final Predicate<Image> imageDescriptionPredicate = new Predicate<Image>() {
       @Override
       public boolean apply(Image input) {
@@ -350,10 +353,6 @@ public class TemplateBuilderImpl implements TemplateBuilder {
          boolean returnVal = true;
          if (hardwareId != null) {
             returnVal = hardwareId.equals(input.getId());
-            // match our input params so that the later predicates pass.
-            if (returnVal) {
-               fromHardware(input);
-            }
          }
          return returnVal;
       }
@@ -388,8 +387,35 @@ public class TemplateBuilderImpl implements TemplateBuilder {
          return "minRam(" + minRam + ")";
       }
    };
-   private final Predicate<Hardware> hardwarePredicate = and(hardwareIdPredicate, locationPredicate,
+
-         hardwareCoresPredicate, hardwareRamPredicate);
+   private Predicate<Hardware> buildHardwarePredicate() {
+      List<Predicate<Hardware>> predicates = newArrayList();
+      if (hardwareId != null) {
+         predicates.add(hardwareIdPredicate);
+      } else {
+         if (location != null)
+            predicates.add(new Predicate<Hardware>() {
+
+               @Override
+               public boolean apply(Hardware input) {
+                  return locationPredicate.apply(input);
+               }
+
+               @Override
+               public String toString() {
+                  return locationPredicate.toString();
+               }
+            });
+         predicates.add(hardwareCoresPredicate);
+         predicates.add(hardwareRamPredicate);
+      }
+
+      // looks verbose, but explicit <Hardware> type needed for this to compile
+      // properly
+      Predicate<Hardware> hardwarePredicate = predicates.size() == 1 ? Iterables.<Predicate<Hardware>> get(predicates, 0)
+            : Predicates.<Hardware> and(predicates);
+      return hardwarePredicate;
+   }
    
    static final Ordering<Hardware> DEFAULT_SIZE_ORDERING = new Ordering<Hardware>() {
       public int compare(Hardware left, Hardware right) {
@@ -407,6 +433,7 @@ public class TemplateBuilderImpl implements TemplateBuilder {
          return ComparisonChain.start()
                .compare(left.getName(), right.getName(), Ordering.<String> natural().nullsLast())
                .compare(left.getVersion(), right.getVersion(), Ordering.<String> natural().nullsLast())
+               .compare(left.getDescription(), right.getDescription(), Ordering.<String> natural().nullsLast())
                .compare(left.getOperatingSystem().getName(), right.getOperatingSystem().getName(),//
                      Ordering.<String> natural().nullsLast())
                .compare(left.getOperatingSystem().getVersion(), right.getOperatingSystem().getVersion(),//
@@ -543,6 +570,15 @@ public class TemplateBuilderImpl implements TemplateBuilder {
       
    };
    
+
+   private static final Function<Hardware, String> hardwareToId = new Function<Hardware, String>() {
+
+      @Override
+      public String apply(Hardware arg0) {
+         return arg0.getId();
+      }
+      
+   };
    /**
     * {@inheritDoc}
     */
@@ -554,12 +590,14 @@ public class TemplateBuilderImpl implements TemplateBuilder {
             defaultTemplate.options(options);
          return defaultTemplate.build();
       }
-      if (location == null)
+
-         location = defaultLocation.get();
       if (options == null)
          options = optionsProvider.get();
       logger.debug(">> searching params(%s)", this);
       Set<? extends Image> images = getImages();
+      if (location == null)
+         location = defaultLocation.get();
+      
       Predicate<Image> imagePredicate = buildImagePredicate();
       Iterable<? extends Image> supportedImages = filter(images, buildImagePredicate());
       if (size(supportedImages) == 0) {
@@ -570,10 +608,10 @@ public class TemplateBuilderImpl implements TemplateBuilder {
                      images);
          }
       }
-      Hardware hardware = resolveSize(hardwareSorter(), supportedImages);
+
+      Hardware hardware = resolveHardware(hardwareSorter(), supportedImages);
       Image image = resolveImage(hardware, supportedImages);
       logger.debug("<<   matched image(%s)", image.getId());
-
       return new TemplateImpl(image, hardware, location, options);
    }
 
@@ -584,12 +622,11 @@ public class TemplateBuilderImpl implements TemplateBuilder {
       throw exception;
    }
 
-   protected Hardware resolveSize(Ordering<Hardware> hardwareOrdering, final Iterable<? extends Image> images) {
+   protected Hardware resolveHardware(Ordering<Hardware> hardwareOrdering, final Iterable<? extends Image> images) {
       Set<? extends Hardware> hardwarel = hardwares.get();
-      Hardware hardware;
+      Iterable<? extends Hardware> hardwaresThatAreCompatibleWithOurImages = ImmutableSet.of();
       try {
-         Iterable<? extends Hardware> hardwaresThatAreCompatibleWithOurImages = filter(hardwarel,
+         hardwaresThatAreCompatibleWithOurImages = filter(hardwarel, new Predicate<Hardware>() {
-               new Predicate<Hardware>() {
             @Override
             public boolean apply(final Hardware hardware) {
                return Iterables.any(images, new Predicate<Image>() {
@@ -608,12 +645,27 @@ public class TemplateBuilderImpl implements TemplateBuilder {
 
             }
          });
+      } catch (NoSuchElementException exception) {
+
+      }
+      if (size(hardwaresThatAreCompatibleWithOurImages) == 0) {
+         String message = format("no hardware profiles support images matching params: %s", toString());
+         NoSuchElementException exception = new NoSuchElementException(message);
+         if (logger.isTraceEnabled())
+            logger.warn(exception, "hardware profiles %s\nimage ids %s", transform(hardwarel, hardwareToId), transform(
+                     images, imageToId));
+         throw exception;
+      }
+      Predicate<Hardware> hardwarePredicate = buildHardwarePredicate();
+      Hardware hardware;
+      try {
          hardware = hardwareOrdering.max(filter(hardwaresThatAreCompatibleWithOurImages, hardwarePredicate));
       } catch (NoSuchElementException exception) {
-         String message = format("no hardware profiles support images matching params: %s", toString());
+         String message = format("no hardware profiles match params: %s", hardwarePredicate);
          exception = new NoSuchElementException(message);
          if (logger.isTraceEnabled())
-            logger.warn(exception, "hardware profiles %s\nimage ids %s", hardwarel, transform(images, imageToId));
+            logger.warn(exception, "hardware profiles %s", transform(hardwaresThatAreCompatibleWithOurImages,
+                     hardwareToId));
          throw exception;
       }
       logger.debug("<<   matched hardware(%s)", hardware.getId());
@@ -649,6 +701,7 @@ public class TemplateBuilderImpl implements TemplateBuilder {
             return "hardware(" + hardware + ").supportsImage()";
          }
       };
+
       try {
          Iterable<? extends Image> matchingImages = filter(supportedImages, imagePredicate);
          if (logger.isTraceEnabled())

compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java

@@ -18,10 +18,10 @@
  */
 package org.jclouds.compute.domain.internal;
 
+import static org.easymock.EasyMock.createMock;
 import static org.easymock.EasyMock.expect;
-import static org.easymock.classextension.EasyMock.createMock;
+import static org.easymock.EasyMock.replay;
-import static org.easymock.classextension.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
-import static org.easymock.classextension.EasyMock.verify;
 import static org.testng.Assert.assertEquals;
 
 import java.util.NoSuchElementException;
@@ -49,9 +49,50 @@ import com.google.common.collect.ImmutableSet;
  * 
  * @author Adrian Cole
  */
-@Test(groups = "unit")
+@Test(groups = "unit", singleThreaded = true)
 public class TemplateBuilderImplTest {
    
+   @SuppressWarnings("unchecked")
+   public void testLocationPredicateWhenComputeMetadataIsNotLocationBound() {
+      Location defaultLocation = createMock(Location.class);
+      Image image = createMock(Image.class);
+      OperatingSystem os = createMock(OperatingSystem.class);
+
+      Hardware hardware = new HardwareBuilder().id("hardwareId").build();
+
+      Supplier<Set<? extends Location>> locations = Suppliers.<Set<? extends Location>> ofInstance(ImmutableSet
+               .<Location> of(defaultLocation));
+      Supplier<Set<? extends Image>> images = Suppliers.<Set<? extends Image>> ofInstance(ImmutableSet.<Image> of(
+               image));
+      Supplier<Set<? extends Hardware>> hardwares = Suppliers.<Set<? extends Hardware>> ofInstance(ImmutableSet
+               .<Hardware> of(hardware));
+      Provider<TemplateOptions> optionsProvider = createMock(Provider.class);
+      Provider<TemplateBuilder> templateBuilderProvider = createMock(Provider.class);
+      TemplateBuilder defaultTemplate = createMock(TemplateBuilder.class);
+
+      expect(image.getLocation()).andReturn(defaultLocation).anyTimes();
+      expect(image.getProviderId()).andReturn("imageId").anyTimes();
+      expect(defaultLocation.getId()).andReturn("location").anyTimes();
+
+      replay(image);
+      replay(os);
+      replay(defaultTemplate);
+      replay(defaultLocation);
+      replay(optionsProvider);
+      replay(templateBuilderProvider);
+
+      TemplateBuilderImpl template = createTemplateBuilder(null, locations, images, hardwares, defaultLocation,
+               optionsProvider, templateBuilderProvider);
+      assert template.locationPredicate.apply(hardware);
+
+      verify(image);
+      verify(os);
+      verify(defaultTemplate);
+      verify(defaultLocation);
+      verify(optionsProvider);
+      verify(templateBuilderProvider);
+   }
+   
    @SuppressWarnings("unchecked")
    @Test
    public void testResolveImages() {
@@ -75,6 +116,8 @@ public class TemplateBuilderImplTest {
 
       expect(image.getName()).andReturn("imageName");
       expect(image2.getName()).andReturn("imageName");
+      expect(image.getDescription()).andReturn("imageDescription");
+      expect(image2.getDescription()).andReturn("imageDescription");
       expect(image.getVersion()).andReturn("imageVersion");
       expect(image2.getVersion()).andReturn("imageVersion");
       expect(image.getOperatingSystem()).andReturn(os).atLeastOnce();
@@ -179,7 +222,7 @@ public class TemplateBuilderImplTest {
 
    @SuppressWarnings("unchecked")
    @Test
-   public void testSizeWithImageIdPredicateOnlyAcceptsImage() {
+   public void testHardwareWithImageIdPredicateOnlyAcceptsImage() {
       Location defaultLocation = createMock(Location.class);
       Image image = createMock(Image.class);
       OperatingSystem os = createMock(OperatingSystem.class);
@@ -239,7 +282,56 @@ public class TemplateBuilderImplTest {
 
    @SuppressWarnings("unchecked")
    @Test
-   public void testSizeWithImageIdPredicateOnlyDoesntImage() {
+   public void testHardwareWithImageIdPredicateOnlyAcceptsImageWhenLocationNull() {
+      Location defaultLocation = createMock(Location.class);
+      Image image = createMock(Image.class);
+      OperatingSystem os = createMock(OperatingSystem.class);
+
+      Hardware hardware = new HardwareBuilder().id("hardwareId").supportsImage(ImagePredicates.idEquals("myregion/imageId"))
+               .build();
+
+      Supplier<Set<? extends Location>> locations = Suppliers.<Set<? extends Location>> ofInstance(ImmutableSet
+               .<Location> of(defaultLocation));
+      Supplier<Set<? extends Image>> images = Suppliers.<Set<? extends Image>> ofInstance(ImmutableSet
+               .<Image> of(image));
+      Supplier<Set<? extends Hardware>> hardwares = Suppliers.<Set<? extends Hardware>> ofInstance(ImmutableSet
+               .<Hardware> of(hardware));
+      Provider<TemplateOptions> optionsProvider = createMock(Provider.class);
+      Provider<TemplateBuilder> templateBuilderProvider = createMock(Provider.class);
+      TemplateBuilder defaultTemplate = createMock(TemplateBuilder.class);
+
+      expect(optionsProvider.get()).andReturn(new TemplateOptions());
+      expect(image.getId()).andReturn("myregion/imageId").atLeastOnce();
+      expect(image.getLocation()).andReturn(null).atLeastOnce();
+      expect(image.getName()).andReturn(null).atLeastOnce();
+      expect(image.getDescription()).andReturn(null).atLeastOnce();
+      expect(image.getVersion()).andReturn(null).atLeastOnce();
+      expect(image.getOperatingSystem()).andReturn(os).atLeastOnce();
+      expect(image.getProviderId()).andReturn("imageId").anyTimes();
+      
+      expect(defaultLocation.getId()).andReturn("myregion").anyTimes();
+
+      expect(os.getName()).andReturn(null).atLeastOnce();
+      expect(os.getVersion()).andReturn(null).atLeastOnce();
+      expect(os.getFamily()).andReturn(null).atLeastOnce();
+      expect(os.getDescription()).andReturn(null).atLeastOnce();
+      expect(os.getArch()).andReturn(null).atLeastOnce();
+      expect(os.is64Bit()).andReturn(false).atLeastOnce();
+
+      replay(image, os, defaultTemplate, defaultLocation, optionsProvider, templateBuilderProvider);
+
+      TemplateBuilderImpl template = createTemplateBuilder(null, locations, images, hardwares, defaultLocation,
+               optionsProvider, templateBuilderProvider);
+
+      template.imageId("myregion/imageId").build();
+
+      verify(image, os, defaultTemplate, defaultLocation, optionsProvider, templateBuilderProvider);
+
+   }
+
+   @SuppressWarnings("unchecked")
+   @Test
+   public void testHardwareWithImageIdPredicateOnlyDoesntImage() {
       Location defaultLocation = createMock(Location.class);
       Image image = createMock(Image.class);
       OperatingSystem os = createMock(OperatingSystem.class);
@@ -567,9 +659,8 @@ public class TemplateBuilderImplTest {
       Provider<TemplateBuilder> templateBuilderProvider = createMock(Provider.class);
       TemplateOptions defaultOptions = createMock(TemplateOptions.class);
 
-      expect(defaultLocation.getId()).andReturn("us-east-1");
-
       expect(optionsProvider.get()).andReturn(defaultOptions);
+      expect(defaultLocation.getId()).andReturn("us-east-1");
 
       replay(defaultOptions);
       replay(defaultLocation);
@@ -580,11 +671,11 @@ public class TemplateBuilderImplTest {
                optionsProvider, templateBuilderProvider);
 
       try {
-         template.imageDescriptionMatches("region/ami").build();
+         template.imageDescriptionMatches("description").build();
          assert false;
       } catch (NoSuchElementException e) {
          // make sure big data is not in the exception message
-         assertEquals(e.getMessage(), "no image matched predicate: And(locationEqualsOrChildOf(us-east-1),imageDescription(region/ami))");
+         assertEquals(e.getMessage(), "no image matched predicate: And(locationEqualsOrChildOf(us-east-1),imageDescription(description))");
       }
 
       verify(defaultOptions);