From 3589c1475aa919e5a874b7082af748425a6b97b4 Mon Sep 17 00:00:00 2001 From: Ignasi Barrera Date: Tue, 10 Jun 2014 17:07:29 +0200 Subject: [PATCH] JCLOUDS-588: Register discovered images in the image cache Images were cached in memory using a memoized supplier. To allow growing this cache with the discovered images, the ImageCacheSupplier class has been created. It provides an in-memory cache with all discovered images and acts as a view over the image cache that also provides access to them. The in-memory cache for the discovered images expires with the session, just as the image cache does. The default memoized image supplier has been changed to the ImageCacheSupplier, to make sure all providers get injected the right instance, and the old supplier has been qualified with the 'imageCache' name, in case a provider needs the basic image cache. --- .../CloudSigmaTemplateBuilderImpl.java | 4 +- .../internal/EC2TemplateBuilderImpl.java | 3 +- .../ec2/compute/EC2TemplateBuilderTest.java | 3 +- .../internal/EC2TemplateBuilderImplTest.java | 11 ++- .../internal/VCloudTemplateBuilderImpl.java | 4 +- .../BaseComputeServiceContextModule.java | 6 +- .../domain/internal/TemplateBuilderImpl.java | 40 +++++---- .../AdaptingComputeServiceStrategies.java | 9 +- .../compute/suppliers/ImageCacheSupplier.java | 84 +++++++++++++++++++ .../internal/TemplateBuilderImplTest.java | 80 +++++++++++++++++- .../suppliers/ImageCacheSupplierTest.java | 77 +++++++++++++++++ .../compute/AWSEC2TemplateBuilderImpl.java | 3 +- 12 files changed, 294 insertions(+), 30 deletions(-) create mode 100644 compute/src/main/java/org/jclouds/compute/suppliers/ImageCacheSupplier.java create mode 100644 compute/src/test/java/org/jclouds/compute/suppliers/ImageCacheSupplierTest.java diff --git a/apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/CloudSigmaTemplateBuilderImpl.java b/apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/CloudSigmaTemplateBuilderImpl.java index af386645de..2032db9b23 100644 --- a/apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/CloudSigmaTemplateBuilderImpl.java +++ b/apis/cloudsigma/src/main/java/org/jclouds/cloudsigma/compute/CloudSigmaTemplateBuilderImpl.java @@ -24,11 +24,11 @@ import javax.inject.Provider; import org.jclouds.collect.Memoized; import org.jclouds.compute.domain.Hardware; -import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import com.google.common.base.Supplier; @@ -39,7 +39,7 @@ import com.google.common.base.Supplier; public class CloudSigmaTemplateBuilderImpl extends TemplateBuilderImpl { @Inject public CloudSigmaTemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> hardwares, + ImageCacheSupplier images, @Memoized Supplier> hardwares, Supplier defaultLocation2, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy) { super(locations, images, hardwares, defaultLocation2, optionsProvider, defaultTemplateProvider, getImageStrategy); diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java index 63df6af966..3662dec7c5 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImpl.java @@ -33,6 +33,7 @@ import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.util.Throwables2; @@ -53,7 +54,7 @@ public class EC2TemplateBuilderImpl extends TemplateBuilderImpl { @Inject protected EC2TemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> sizes, + ImageCacheSupplier images, @Memoized Supplier> sizes, Supplier defaultLocation, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy, Supplier> imageMap) { diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java index f0c3ea5729..ccc8c0282d 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java @@ -49,6 +49,7 @@ import org.jclouds.compute.domain.Template; import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; @@ -227,7 +228,7 @@ public class EC2TemplateBuilderTest { m1_small().build(), m1_xlarge().build(), m2_xlarge().build(), m2_2xlarge().build(), m2_4xlarge().build(),g2_2xlarge().build(),CC1_4XLARGE)); - return new EC2TemplateBuilderImpl(locations, images, sizes, Suppliers.ofInstance(location), optionsProvider, + return new EC2TemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60), sizes, Suppliers.ofInstance(location), optionsProvider, templateBuilderProvider, getImageStrategy, imageCache) { }; } diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java index 1c754f6593..ca0fa8ee00 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java @@ -38,6 +38,7 @@ import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.domain.internal.TemplateBuilderImplTest; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.ec2.compute.functions.ImagesToRegionAndIdMap; @@ -88,7 +89,7 @@ public class EC2TemplateBuilderImplTest extends TemplateBuilderImplTest { ImagesToRegionAndIdMap.imagesToMap(images.get())))); } - return new EC2TemplateBuilderImpl(locations, images, sizes, Suppliers.ofInstance(defaultLocation), + return new EC2TemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60), sizes, Suppliers.ofInstance(defaultLocation), optionsProvider, templateBuilderProvider, getImageStrategy, Suppliers.>ofInstance(imageMap)); } @@ -223,5 +224,11 @@ public class EC2TemplateBuilderImplTest extends TemplateBuilderImplTest { public void testFindImageWithIdDefaultToGetImageStrategy() { } - + + // The EC2 provider already overrides the getImage method so this test is not useful for EC2 + @Override + public void testFindImageWithIdDefaultToGetImageStrategyAndPopulatesTheCache() { + + } + } diff --git a/apis/vcloud/src/main/java/org/jclouds/vcloud/compute/internal/VCloudTemplateBuilderImpl.java b/apis/vcloud/src/main/java/org/jclouds/vcloud/compute/internal/VCloudTemplateBuilderImpl.java index 73b5f40ebf..1259d508cb 100644 --- a/apis/vcloud/src/main/java/org/jclouds/vcloud/compute/internal/VCloudTemplateBuilderImpl.java +++ b/apis/vcloud/src/main/java/org/jclouds/vcloud/compute/internal/VCloudTemplateBuilderImpl.java @@ -24,11 +24,11 @@ import javax.inject.Provider; import org.jclouds.collect.Memoized; import org.jclouds.compute.domain.Hardware; -import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.domain.internal.TemplateBuilderImpl; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import com.google.common.base.Supplier; @@ -41,7 +41,7 @@ public class VCloudTemplateBuilderImpl extends TemplateBuilderImpl { @Inject protected VCloudTemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> sizes, + ImageCacheSupplier images, @Memoized Supplier> sizes, Supplier defaultLocation, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy) { super(locations, images, sizes, defaultLocation, optionsProvider, defaultTemplateProvider, getImageStrategy); diff --git a/compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java b/compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java index a6da2be889..9de17b9bc8 100644 --- a/compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java +++ b/compute/src/main/java/org/jclouds/compute/config/BaseComputeServiceContextModule.java @@ -53,6 +53,7 @@ import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.compute.strategy.CustomizeNodeAndAddToGoodMapOrPutExceptionIntoBadMap; import org.jclouds.compute.strategy.InitializeRunScriptOnNodeOrPlaceInBadMap; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.config.ValueOfConfigurationKeyOrNull; import org.jclouds.domain.LoginCredentials; import org.jclouds.json.Json; @@ -115,6 +116,9 @@ public abstract class BaseComputeServiceContextModule extends AbstractModule { }, InitializeRunScriptOnNodeOrPlaceInBadMap.class).build(InitializeRunScriptOnNodeOrPlaceInBadMap.Factory.class)); install(new FactoryModuleBuilder().build(BlockUntilInitScriptStatusIsZeroThenReturnOutput.Factory.class)); + + bind(new TypeLiteral>>() { + }).annotatedWith(Memoized.class).to(ImageCacheSupplier.class); } protected void bindCredentialsOverriderFunction() { @@ -233,7 +237,7 @@ public abstract class BaseComputeServiceContextModule extends AbstractModule { @Provides @Singleton - @Memoized + @Named("imageCache") protected Supplier> supplyImageCache(AtomicReference authException, @Named(PROPERTY_SESSION_INTERVAL) long seconds, final Supplier> imageSupplier, Injector injector) { if (shouldEagerlyParseImages(injector)) { diff --git a/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java b/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java index c893779069..adb794a036 100644 --- a/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java +++ b/compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java @@ -54,6 +54,7 @@ import org.jclouds.compute.domain.TemplateBuilderSpec; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.logging.Logger; @@ -81,7 +82,7 @@ public class TemplateBuilderImpl implements TemplateBuilder { @Named(ComputeServiceConstants.COMPUTE_LOGGER) protected Logger logger = Logger.NULL; - protected final Supplier> images; + protected final ImageCacheSupplier images; protected final Supplier> hardwares; protected final Supplier> locations; protected final Supplier defaultLocation; @@ -134,13 +135,13 @@ public class TemplateBuilderImpl implements TemplateBuilder { @Inject protected TemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> hardwares, - Supplier defaultLocation2, @Named("DEFAULT") Provider optionsProvider, + ImageCacheSupplier images, @Memoized Supplier> hardwares, + Supplier defaultLocation, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy) { this.locations = checkNotNull(locations, "locations"); - this.images = checkNotNull(images, "locations"); + this.images = checkNotNull(images, "images"); this.hardwares = checkNotNull(hardwares, "hardwares"); - this.defaultLocation = checkNotNull(defaultLocation2, "defaultLocation2"); + this.defaultLocation = checkNotNull(defaultLocation, "defaultLocation"); this.optionsProvider = checkNotNull(optionsProvider, "optionsProvider"); this.defaultTemplateProvider = checkNotNull(defaultTemplateProvider, "defaultTemplateProvider"); this.getImageStrategy = checkNotNull(getImageStrategy, "getImageStrategy"); @@ -731,21 +732,26 @@ public class TemplateBuilderImpl implements TemplateBuilder { } private Image findImageWithId(Set images) { - // Try find the image in the cache and fallback to the GetImageStrategy + // Try to find the image in the cache and fallback to the GetImageStrategy // see https://issues.apache.org/jira/browse/JCLOUDS-570 Optional image = tryFind(images, idPredicate); - if (!image.isPresent()) { - logger.warn("Image %s not found in the image cache. Trying to get it directly...", imageId); - // Note that this might generate make a call to the provider instead of using a cache, but - // this will be executed rarely, only when an image is not present in the image list but - // it actually exists in the provider. It shouldn't be an expensive call so using a cache just for - // this corner case is overkill. - image = Optional.fromNullable(getImageStrategy.getImage(imageId)); - if (!image.isPresent()) { - throwNoSuchElementExceptionAfterLoggingImageIds(format("%s not found", idPredicate), images); - } + if (image.isPresent()) { + return image.get(); } - return image.get(); + + logger.info("Image %s not found in the image cache. Trying to get it from the provider...", imageId); + // Note that this will generate make a call to the provider instead of using a cache, but + // this will be executed rarely, only when an image is not present in the image list but + // it actually exists in the provider. It shouldn't be an expensive call so using a cache just for + // this corner case is overkill. + Image imageFromProvider = getImageStrategy.getImage(imageId); + if (imageFromProvider == null) { + throwNoSuchElementExceptionAfterLoggingImageIds(format("%s not found", idPredicate), images); + } + // Register the just found image in the image cache, so subsequent uses of the TemplateBuilder and + // the ComptueService find it. + this.images.registerImage(imageFromProvider); + return imageFromProvider; } private Hardware findHardwareWithId(Set hardwaresToSearch) { diff --git a/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java b/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java index 6c37090e74..2669d84cc8 100644 --- a/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java +++ b/compute/src/main/java/org/jclouds/compute/strategy/impl/AdaptingComputeServiceStrategies.java @@ -34,6 +34,7 @@ import javax.inject.Singleton; import org.jclouds.compute.ComputeServiceAdapter; import org.jclouds.compute.ComputeServiceAdapter.NodeAndInitialCredentials; +import org.jclouds.compute.config.ComputeServiceAdapterContextModule.AddDefaultCredentialsToImage; import org.jclouds.compute.domain.ComputeMetadata; import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.NodeMetadata; @@ -78,12 +79,13 @@ public class AdaptingComputeServiceStrategies implements CreateNodeW private final ComputeServiceAdapter client; private final Function nodeMetadataAdapter; private final Function imageAdapter; + private final AddDefaultCredentialsToImage addDefaultCredentialsToImage; @Inject public AdaptingComputeServiceStrategies(Map credentialStore, PrioritizeCredentialsFromTemplate prioritizeCredentialsFromTemplate, ComputeServiceAdapter client, Function nodeMetadataAdapter, - Function imageAdapter) { + Function imageAdapter, AddDefaultCredentialsToImage addDefaultCredentialsToImage) { this.credentialStore = checkNotNull(credentialStore, "credentialStore"); this.prioritizeCredentialsFromTemplate = checkNotNull(prioritizeCredentialsFromTemplate, "prioritizeCredentialsFromTemplate"); @@ -91,6 +93,7 @@ public class AdaptingComputeServiceStrategies implements CreateNodeW this.nodeMetadataAdapter = Functions.compose(addLoginCredentials, checkNotNull(nodeMetadataAdapter, "nodeMetadataAdapter")); this.imageAdapter = checkNotNull(imageAdapter, "imageAdapter"); + this.addDefaultCredentialsToImage = checkNotNull(addDefaultCredentialsToImage, "addDefaultCredentialsToImage"); } private final Function addLoginCredentials = new Function() { @@ -128,7 +131,9 @@ public class AdaptingComputeServiceStrategies implements CreateNodeW I image = client.getImage(checkNotNull(id, "id")); if (image == null) return null; - return imageAdapter.apply(image); + // The image supplier configured in the ComputeServiceAdapterContextModule also adds the default credentials to + // each image in the image list. When getting a single image, the behavior must be the same. + return addDefaultCredentialsToImage.apply(imageAdapter.apply(image)); } @Override diff --git a/compute/src/main/java/org/jclouds/compute/suppliers/ImageCacheSupplier.java b/compute/src/main/java/org/jclouds/compute/suppliers/ImageCacheSupplier.java new file mode 100644 index 0000000000..400525bec0 --- /dev/null +++ b/compute/src/main/java/org/jclouds/compute/suppliers/ImageCacheSupplier.java @@ -0,0 +1,84 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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.compute.suppliers; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.Iterables.concat; +import static org.jclouds.Constants.PROPERTY_SESSION_INTERVAL; + +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import javax.inject.Named; +import javax.inject.Singleton; + +import org.jclouds.compute.domain.Image; + +import com.google.common.base.Supplier; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import com.google.common.collect.ImmutableSet; +import com.google.inject.Inject; + +/** + * Image supplier that allows new images to be registered. + *

+ * This is a wrapper for the memoized image supplier (the actual image cache), to provide a way to register new images as + * needed. Once a new image is created by the {@link org.jclouds.compute.extensions.ImageExtension}, or discovered by + * other means (see https://issues.apache.org/jira/browse/JCLOUDS-570) this supplier will allow the image to be appended + * to the cached list, so it can be properly used normally. + */ +@Singleton +public class ImageCacheSupplier implements Supplier> { + + private final Supplier> imageCache; + + private final Cache uncachedImages; + + @Inject + public ImageCacheSupplier(@Named("imageCache") Supplier> imageCache, + @Named(PROPERTY_SESSION_INTERVAL) long sessionIntervalSeconds) { + this.imageCache = checkNotNull(imageCache, "imageCache"); + // We use a cache to let the entries in the "uncached" set expire as soon as the image cache expires. We want the + // uncached set to be regenerated when the original cache is also regenerated. + this.uncachedImages = CacheBuilder.newBuilder().expireAfterWrite(sessionIntervalSeconds, TimeUnit.SECONDS) + .build(); + } + + @Override + public Set get() { + return ImmutableSet.copyOf(concat(imageCache.get(), uncachedImages.asMap().values())); + } + + /** + * Registers a new image in the image cache. + *

+ * This method should be called to register new images into the image cache, when some image that is known to exist + * in the provider is still not cached. For example, this can happen when an image is created after the image cache + * has been populated for the first time. + *

+ * Note that this method does not check if the image is already cached, to avoid loading all images if the image + * cache is still not populated. + * + * @param image The image to be registered to the cache. + */ + public void registerImage(Image image) { + checkNotNull(image, "image"); + uncachedImages.put(image.getId(), image); + } + +} diff --git a/compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java b/compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java index 3dfa777792..c771631f0d 100644 --- a/compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java +++ b/compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java @@ -22,6 +22,7 @@ import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; import static org.testng.Assert.fail; @@ -46,6 +47,7 @@ import org.jclouds.compute.domain.Volume; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.predicates.ImagePredicates; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; @@ -471,7 +473,7 @@ public class TemplateBuilderImplTest { Supplier> images, Supplier> hardwares, Location defaultLocation, Provider optionsProvider, Provider templateBuilderProvider, GetImageStrategy getImageStrategy) { - TemplateBuilderImpl template = new TemplateBuilderImpl(locations, images, hardwares, Suppliers + TemplateBuilderImpl template = new TemplateBuilderImpl(locations, new ImageCacheSupplier(images, 60), hardwares, Suppliers .ofInstance(defaultLocation), optionsProvider, templateBuilderProvider, getImageStrategy); return template; } @@ -829,6 +831,82 @@ public class TemplateBuilderImplTest { verify(getImageStrategy); } + @Test + public void testFindImageWithIdDefaultToGetImageStrategyAndPopulatesTheCache() { + final Supplier> locations = Suppliers.> ofInstance(ImmutableSet + . of(region)); + final Supplier> images = Suppliers.> ofInstance(ImmutableSet + . of( + new ImageBuilder() + .ids("Ubuntu 11.04 x64") + .name("Ubuntu 11.04 x64") + .description("Ubuntu 11.04 x64") + .location(region) + .status(Status.AVAILABLE) + .operatingSystem( + OperatingSystem.builder().name("Ubuntu 11.04 x64").description("Ubuntu 11.04 x64") + .is64Bit(true).version("11.04").family(OsFamily.UBUNTU).build()).build(), + new ImageBuilder() + .ids("Ubuntu 11.04 64-bit") + .name("Ubuntu 11.04 64-bit") + .description("Ubuntu 11.04 64-bit") + .location(region) + .status(Status.AVAILABLE) + .operatingSystem( + OperatingSystem.builder().name("Ubuntu 11.04 64-bit").description("Ubuntu 11.04 64-bit") + .is64Bit(true).version("11.04").family(OsFamily.UBUNTU).build()).build())); + + final Supplier> hardwares = Suppliers.> ofInstance(ImmutableSet + . of( + new HardwareBuilder() + .ids(String.format("datacenter(%s)platform(%s)cpuCores(%d)memorySizeMB(%d)diskSizeGB(%d)", + "Falkenberg", "Xen", 1, 512, 5)).ram(512) + .processors(ImmutableList.of(new Processor(1, 1.0))) + .volumes(ImmutableList. of(new VolumeImpl((float) 5, true, true))).hypervisor("Xen") + .location(region) + .supportsImage(ImagePredicates.idEquals(image.getId())).build())); + + final Provider optionsProvider = new Provider() { + @Override + public TemplateOptions get() { + return new TemplateOptions(); + } + }; + + final GetImageStrategy getImageStrategy = createMock(GetImageStrategy.class); + + expect(getImageStrategy.getImage(image.getId())).andReturn(image); + replay(getImageStrategy); + + Provider templateBuilderProvider = new Provider() { + @Override + public TemplateBuilder get() { + return createTemplateBuilder(null, locations, images, hardwares, region, optionsProvider, this, getImageStrategy); + } + }; + + TemplateBuilder templateBuilder = templateBuilderProvider.get(); + + try { + // First call searching for the image properties will fail, as the image is not in the cache + templateBuilder.osNameMatches(image.getOperatingSystem().getName()).build(); + fail("Image should not exist in the cache"); + } catch (Exception ex) { + // Expected path + } + + // A second call using the imageId will fallback to the GetImageStrategy and populate the image in the cache. + assertNotNull(templateBuilder.imageId(image.getId()).build()); + + // The third call will succeed, as the previous one should have populated the image in the cache. + templateBuilder.imageId(null); // Clear all criteria + Template template = templateBuilder.osNameMatches(image.getOperatingSystem().getName()).build(); + assertEquals(template.getImage().getId(), image.getId()); + + // Verify this is called only once, as the third call will already find the image in the cache + verify(getImageStrategy); + } + @SuppressWarnings("unchecked") @Test public void testHardwareIdNullsHypervisor() { diff --git a/compute/src/test/java/org/jclouds/compute/suppliers/ImageCacheSupplierTest.java b/compute/src/test/java/org/jclouds/compute/suppliers/ImageCacheSupplierTest.java new file mode 100644 index 0000000000..29a7caf3d0 --- /dev/null +++ b/compute/src/test/java/org/jclouds/compute/suppliers/ImageCacheSupplierTest.java @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF 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.compute.suppliers; + +import static org.testng.Assert.assertEquals; + +import java.util.Set; + +import org.jclouds.compute.domain.Image; +import org.jclouds.compute.domain.ImageBuilder; +import org.jclouds.compute.domain.OperatingSystem; +import org.jclouds.domain.Location; +import org.jclouds.domain.LocationBuilder; +import org.jclouds.domain.LocationScope; +import org.testng.annotations.Test; + +import com.google.common.base.Suppliers; +import com.google.common.collect.ImmutableSet; + +/** + * Unit tests for the {@link ImageCacheSupplier} class. + */ +@Test(groups = "unit", testName = "ImageCacheSupplierTest") +public class ImageCacheSupplierTest { + + private Location location = new LocationBuilder().scope(LocationScope.PROVIDER).id("location") + .description("location").build(); + + private OperatingSystem os = OperatingSystem.builder().name("osName").version("osVersion") + .description("osDescription").arch("X86_32").build(); + + private Image image = new ImageBuilder().id("imageId").providerId("imageId").name("imageName") + .description("imageDescription").version("imageVersion").operatingSystem(os).status(Image.Status.AVAILABLE) + .location(location).build(); + + private Set images = ImmutableSet.of(image); + + @Test(expectedExceptions = NullPointerException.class) + public void testRegisterNullImageIsNotAllowed() { + ImageCacheSupplier imageCache = new ImageCacheSupplier(Suppliers.> ofInstance(images), 60); + imageCache.registerImage(null); + } + + @Test + public void testRegisterImageIgnoresDuplicates() { + ImageCacheSupplier imageCache = new ImageCacheSupplier(Suppliers.> ofInstance(images), 60); + assertEquals(imageCache.get().size(), 1); + + imageCache.registerImage(image); + + assertEquals(imageCache.get().size(), 1); + } + + @Test + public void testRegisterNewImage() { + ImageCacheSupplier imageCache = new ImageCacheSupplier(Suppliers.> ofInstance(images), 60); + assertEquals(imageCache.get().size(), 1); + + imageCache.registerImage(ImageBuilder.fromImage(image).id("newimage").build()); + + assertEquals(imageCache.get().size(), 2); + } +} diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java index c059044388..b93a4b951a 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2TemplateBuilderImpl.java @@ -28,6 +28,7 @@ import org.jclouds.compute.domain.Image; import org.jclouds.compute.domain.TemplateBuilder; import org.jclouds.compute.options.TemplateOptions; import org.jclouds.compute.strategy.GetImageStrategy; +import org.jclouds.compute.suppliers.ImageCacheSupplier; import org.jclouds.domain.Location; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.ec2.compute.internal.EC2TemplateBuilderImpl; @@ -43,7 +44,7 @@ public class AWSEC2TemplateBuilderImpl extends EC2TemplateBuilderImpl { @Inject protected AWSEC2TemplateBuilderImpl(@Memoized Supplier> locations, - @Memoized Supplier> images, @Memoized Supplier> sizes, + ImageCacheSupplier images, @Memoized Supplier> sizes, Supplier defaultLocation, @Named("DEFAULT") Provider optionsProvider, @Named("DEFAULT") Provider defaultTemplateProvider, GetImageStrategy getImageStrategy, Supplier> imageMap) {