Merge pull request #297 from richardcloudsoft/ec2-register-ami-cache-issue

EC2 register AMI cache issue

providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/suppliers/AWSEC2ImageSupplier.java

@@ -34,6 +34,7 @@ import javax.inject.Inject;
 import javax.inject.Named;
 import javax.inject.Singleton;
 
+import com.google.common.collect.ForwardingSet;
 import org.jclouds.Constants;
 import org.jclouds.aws.ec2.compute.config.ClusterCompute;
 import org.jclouds.compute.domain.Image;
@@ -127,7 +128,11 @@ public class AWSEC2ImageSupplier implements Supplier<Set<? extends Image>> {
       logger.debug("<< images(%d)", imageMap.size());
       
       // TODO Used to be mutable; was this assumed anywhere?
-      return ImmutableSet.copyOf(imageMap.values());
+      return new ForwardingSet<Image>() {
+         protected Set<Image> delegate() {
+            return ImmutableSet.copyOf(cache.get().asMap().values());
+         }
+      };
    }
    
    private Future<Iterable<Image>> images(Iterable<String> regions, String query, String tag) {

providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/services/AMIClientLiveTest.java

@@ -29,14 +29,26 @@ import java.util.Iterator;
 import java.util.Properties;
 import java.util.Set;
 
+import com.google.common.base.Predicate;
+import com.google.common.base.Predicates;
 import org.jclouds.aws.domain.Region;
 import org.jclouds.compute.BaseVersionedServiceLiveTest;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.ComputeServiceContext;
 import org.jclouds.compute.ComputeServiceContextFactory;
+import org.jclouds.compute.RunNodesException;
+import org.jclouds.compute.domain.NodeMetadata;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.predicates.ImagePredicates;
 import org.jclouds.ec2.EC2AsyncClient;
 import org.jclouds.ec2.EC2Client;
+import org.jclouds.ec2.domain.BlockDevice;
 import org.jclouds.ec2.domain.Image;
+import org.jclouds.ec2.domain.Reservation;
 import org.jclouds.ec2.domain.RootDeviceType;
 import org.jclouds.ec2.domain.Image.ImageType;
+import org.jclouds.ec2.domain.RunningInstance;
+import org.jclouds.ec2.domain.Snapshot;
 import org.jclouds.ec2.services.AMIClient;
 import org.jclouds.logging.log4j.config.Log4JLoggingModule;
 import org.jclouds.rest.RestContext;
@@ -51,6 +63,8 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 import com.google.inject.Module;
 
+import javax.annotation.Nullable;
+
 /**
  * Tests behavior of {@code AMIClient}
  * 
@@ -70,13 +84,16 @@ public class AMIClientLiveTest extends BaseVersionedServiceLiveTest {
    private RestContext<EC2Client, EC2AsyncClient> context;
 
    private Set<String> imagesToDeregister = Sets.newHashSet();
+   private Set<String> snapshotsToDelete = Sets.newHashSet();
 
+   private ComputeServiceContext jcloudsContext;
    @BeforeGroups(groups = { "live" })
    public void setupClient() {
       setupCredentials();
       Properties overrides = setupProperties();
-      context = new ComputeServiceContextFactory().createContext(provider,
+      jcloudsContext = new ComputeServiceContextFactory().createContext(provider,
-               ImmutableSet.<Module> of(new Log4JLoggingModule()), overrides).getProviderSpecificContext();
+         ImmutableSet.<Module>of(new Log4JLoggingModule()), overrides);
+      context = jcloudsContext.getProviderSpecificContext();
       client = context.getApi().getAMIServices();
    }
 
@@ -149,6 +166,50 @@ public class AMIClientLiveTest extends BaseVersionedServiceLiveTest {
       assertEquals(imageRegisteredFromManifestWithOptions.getDescription(), "adrian");
    }
 
+   @Test
+   public void testNewlyRegisteredImageCanBeListed() throws Exception {
+      ComputeService computeService = jcloudsContext.getComputeService();
+      Snapshot snapshot = createSnapshot(computeService);
+
+      // List of images before...
+      int sizeBefore = computeService.listImages().size();
+
+      // Register a new image...
+      final String imageRegisteredId = client.registerUnixImageBackedByEbsInRegion(null, "jcloudstest1", snapshot.getId());
+      imagesToDeregister.add(imageRegisteredId);
+      final Image imageRegistered = Iterables.getOnlyElement(client.describeImagesInRegion(null, imageIds(imageRegisteredId)));
+
+      // This is the suggested method to ensure the new image ID is inserted into the cache
+      // (suggested by adriancole_ on #jclouds)
+      computeService.templateBuilder().imageId(imageRegistered.getRegion() + "/" + imageRegisteredId).build();
+
+      // List of images after - should be one larger than before
+      Set<? extends org.jclouds.compute.domain.Image> after = computeService.listImages();
+      assertEquals(after.size(), sizeBefore + 1);
+
+      // Detailed check: filter for the AMI ID
+      Iterable<? extends org.jclouds.compute.domain.Image> filtered = Iterables.filter(after,
+         ImagePredicates.idEquals(imageRegistered.getRegion() + "/" + imageRegisteredId));
+      assertEquals(Iterables.size(filtered), 1);
+   }
+
+   // Fires up an instance, finds its root volume ID, takes a snapshot, then terminates the instance.
+   private Snapshot createSnapshot(ComputeService computeService) throws RunNodesException {
+      Template options = computeService.templateBuilder().smallest().build();
+      Set<? extends NodeMetadata> nodes = computeService.createNodesInGroup("jcloudstest", 1, options);
+      try {
+         String instanceId = Iterables.getOnlyElement(nodes).getProviderId();
+         Reservation<? extends RunningInstance> reservation = Iterables.getOnlyElement(context.getApi().getInstanceServices().describeInstancesInRegion(null, instanceId));
+         RunningInstance instance = Iterables.getOnlyElement(reservation);
+         BlockDevice device = instance.getEbsBlockDevices().get("/dev/sda1");
+         Snapshot snapshot = context.getApi().getElasticBlockStoreServices().createSnapshotInRegion(null, device.getVolumeId());
+         snapshotsToDelete.add(snapshot.getId());
+         return snapshot;
+      } finally {
+         computeService.destroyNodesMatching(Predicates.in(nodes));
+      }
+   }
+
    @Test(enabled = false)
    // awaiting EBS functionality to be added to jclouds
    public void testRegisterImageBackedByEBS() {
@@ -219,8 +280,10 @@ public class AMIClientLiveTest extends BaseVersionedServiceLiveTest {
    }
 
    @AfterTest
-   public void deregisterImages() {
+   public void cleanUp() {
       for (String imageId : imagesToDeregister)
          client.deregisterImageInRegion(null, imageId);
+      for (String snapshotId : snapshotsToDelete)
+         context.getApi().getElasticBlockStoreServices().deleteSnapshotInRegion(null, snapshotId);
    }
 }