Repository: incubator-brooklyn Updated Branches: refs/heads/master 1a2819384 -> f7b90474a
Partial fix for BROOKLYN-149 (rebind when catalog deleted) - If entities etc still on classpath, then will continue with rebind. But if entities cannot be created, will still fail terminally. Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/52478ec9 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/52478ec9 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/52478ec9 Branch: refs/heads/master Commit: 52478ec9f7c056dc869f3f6b3fe3d499858a280c Parents: 1a28193 Author: Aled Sage <[email protected]> Authored: Thu Jun 4 22:42:55 2015 +0100 Committer: Aled Sage <[email protected]> Committed: Mon Jun 8 11:50:06 2015 +0100 ---------------------------------------------------------------------- .../brooklyn/entity/rebind/RebindIteration.java | 66 ++++--- .../BrooklynMementoPersisterToObjectStore.java | 20 +- .../camp/brooklyn/AbstractYamlRebindTest.java | 188 +++++++++++++++++++ .../brooklyn/catalog/CatalogYamlRebindTest.java | 88 +++++++++ 4 files changed, 330 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/52478ec9/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java index ad4915a..d6b3365 100644 --- a/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java +++ b/core/src/main/java/brooklyn/entity/rebind/RebindIteration.java @@ -902,45 +902,53 @@ public abstract class RebindIteration { protected <T extends BrooklynObject> Class<? extends T> load(Class<T> bType, Memento memento) { return load(bType, memento.getType(), memento.getCatalogItemId(), memento.getId()); } + @SuppressWarnings("unchecked") protected <T extends BrooklynObject> Class<? extends T> load(Class<T> bType, String jType, String catalogItemId, String contextSuchAsId) { checkNotNull(jType, "Type of %s (%s) must not be null", contextSuchAsId, bType.getSimpleName()); + if (catalogItemId != null) { - BrooklynClassLoadingContext loader = getLoadingContextFromCatalogItemId(catalogItemId, classLoader, rebindContext); - return loader.loadClass(jType, bType); - } else { - // we have previously used reflections; not sure if that's needed? - try { - return (Class<T>)reflections.loadClass(jType); - } catch (Exception e) { - LOG.warn("Unable to load "+jType+" using reflections; will try standard context"); - } - - if (BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_INFER_CATALOG_ITEM_ON_REBIND)) { - //Try loading from whichever catalog bundle succeeds. - BrooklynCatalog catalog = managementContext.getCatalog(); - for (CatalogItem<?, ?> item : catalog.getCatalogItems()) { - BrooklynClassLoadingContext catalogLoader = CatalogUtils.newClassLoadingContext(managementContext, item); - Maybe<Class<?>> catalogClass = catalogLoader.tryLoadClass(jType); - if (catalogClass.isPresent()) { - return (Class<? extends T>) catalogClass.get(); - } + CatalogItem<?, ?> catalogItem = rebindContext.lookup().lookupCatalogItem(catalogItemId); + if (catalogItem == null && BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_INFER_CATALOG_ITEM_ON_REBIND)) { + // See https://issues.apache.org/jira/browse/BROOKLYN-149 + // This is a dangling reference to the catalog item (which will have been logged by lookupCatalogItem). + // Try loading as any version. + if (CatalogUtils.looksLikeVersionedId(catalogItemId)) { + String symbolicName = CatalogUtils.getIdFromVersionedId(catalogItemId); + catalogItem = rebindContext.lookup().lookupCatalogItem(symbolicName); } - throw new IllegalStateException("No catalogItemId specified and can't load class from either classpath of catalog items"); + } + if (catalogItem != null) { + BrooklynClassLoadingContext loader = CatalogUtils.newClassLoadingContext(managementContext, catalogItem); + return loader.loadClass(jType, bType); } else { - throw new IllegalStateException("No catalogItemId specified and can't load class from classpath"); + LOG.warn("Unable to load catalog item "+catalogItemId+" for "+contextSuchAsId + +" ("+bType.getSimpleName()+"); will try default class loader"); } - } - } + + try { + return (Class<T>)reflections.loadClass(jType); + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + LOG.warn("Unable to load "+jType+" using reflections; will try standard context"); + } - protected BrooklynClassLoadingContext getLoadingContextFromCatalogItemId(String catalogItemId, ClassLoader classLoader, RebindContext rebindContext) { - Preconditions.checkNotNull(catalogItemId, "catalogItemId required (should not be null)"); - CatalogItem<?, ?> catalogItem = rebindContext.lookup().lookupCatalogItem(catalogItemId); - if (catalogItem != null) { - return CatalogUtils.newClassLoadingContext(managementContext, catalogItem); + if (catalogItemId != null) { + throw new IllegalStateException("Unable to load catalog item "+catalogItemId+" for "+contextSuchAsId+", or load class from classpath"); + } else if (BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_INFER_CATALOG_ITEM_ON_REBIND)) { + //Try loading from whichever catalog bundle succeeds. + BrooklynCatalog catalog = managementContext.getCatalog(); + for (CatalogItem<?, ?> item : catalog.getCatalogItems()) { + BrooklynClassLoadingContext catalogLoader = CatalogUtils.newClassLoadingContext(managementContext, item); + Maybe<Class<?>> catalogClass = catalogLoader.tryLoadClass(jType); + if (catalogClass.isPresent()) { + return (Class<? extends T>) catalogClass.get(); + } + } + throw new IllegalStateException("No catalogItemId specified for "+contextSuchAsId+" and can't load class from either classpath or catalog items"); } else { - throw new IllegalStateException("Failed to load catalog item " + catalogItemId + " required for rebinding."); + throw new IllegalStateException("No catalogItemId specified for "+contextSuchAsId+" and can't load class from classpath"); } } http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/52478ec9/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java index 23e40e9..87ef4fa 100644 --- a/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java +++ b/core/src/main/java/brooklyn/entity/rebind/persister/BrooklynMementoPersisterToObjectStore.java @@ -53,6 +53,8 @@ import brooklyn.entity.rebind.dto.BrooklynMementoImpl; import brooklyn.entity.rebind.dto.BrooklynMementoManifestImpl; import brooklyn.entity.rebind.persister.PersistenceObjectStore.StoreObjectAccessor; import brooklyn.entity.rebind.persister.PersistenceObjectStore.StoreObjectAccessorWithLock; +import brooklyn.internal.BrooklynFeatureEnablement; +import brooklyn.management.classloading.BrooklynClassLoadingContext; import brooklyn.management.classloading.ClassLoaderFromBrooklynClassLoadingContext; import brooklyn.mementos.BrooklynMemento; import brooklyn.mementos.BrooklynMementoManifest; @@ -174,11 +176,23 @@ public class BrooklynMementoPersisterToObjectStore implements BrooklynMementoPer if (item==null || item.getCatalogItemId()==null) { return null; } - CatalogItem<?, ?> catalogItem = CatalogUtils.getCatalogItemOptionalVersion(lookupContext.lookupManagementContext(), item.getCatalogItemId()); + // See RebindIteration.BrooklynObjectInstantiator.load(), for handling where catalog item is missing; + // similar logic here. + String catalogItemId = item.getCatalogItemId(); + CatalogItem<?, ?> catalogItem = CatalogUtils.getCatalogItemOptionalVersion(lookupContext.lookupManagementContext(), catalogItemId); + if (catalogItem == null && BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_INFER_CATALOG_ITEM_ON_REBIND)) { + if (CatalogUtils.looksLikeVersionedId(catalogItemId)) { + String symbolicName = CatalogUtils.getIdFromVersionedId(catalogItemId); + catalogItem = CatalogUtils.getCatalogItemOptionalVersion(lookupContext.lookupManagementContext(), symbolicName); + } + } if (catalogItem == null) { - throw new IllegalStateException("Catalog item " + item.getCatalogItemId() + " not found. Can't deserialize object " + objectId + " of type " + type); + // TODO do we need to only log once, rather than risk log.warn too often? I think this only happens on rebind, so ok. + LOG.warn("Unable to load catalog item "+catalogItemId+" for custom class loader of "+type+" "+objectId+"; will use default class loader"); + return null; + } else { + return ClassLoaderFromBrooklynClassLoadingContext.of(CatalogUtils.newClassLoadingContext(lookupContext.lookupManagementContext(), catalogItem)); } - return ClassLoaderFromBrooklynClassLoadingContext.of(CatalogUtils.newClassLoadingContext(lookupContext.lookupManagementContext(), catalogItem)); } @Override public void enableWriteAccess() { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/52478ec9/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlRebindTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlRebindTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlRebindTest.java new file mode 100644 index 0000000..6814c92 --- /dev/null +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/AbstractYamlRebindTest.java @@ -0,0 +1,188 @@ +package io.brooklyn.camp.brooklyn; + +import io.brooklyn.camp.spi.Assembly; +import io.brooklyn.camp.spi.AssemblyTemplate; + +import java.io.Reader; +import java.io.StringReader; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; + +import brooklyn.catalog.internal.CatalogUtils; +import brooklyn.entity.Entity; +import brooklyn.entity.basic.BrooklynTaskTags; +import brooklyn.entity.basic.Entities; +import brooklyn.entity.basic.StartableApplication; +import brooklyn.entity.rebind.RebindOptions; +import brooklyn.entity.rebind.RebindTestFixture; +import brooklyn.management.ManagementContext; +import brooklyn.management.Task; +import brooklyn.management.internal.LocalManagementContext; +import brooklyn.util.ResourceUtils; +import brooklyn.util.config.ConfigBag; + +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; + +public class AbstractYamlRebindTest extends RebindTestFixture<StartableApplication> { + + private static final Logger LOG = LoggerFactory.getLogger(AbstractYamlTest.class); + protected static final String TEST_VERSION = "0.1.2"; + + protected BrooklynCampPlatform platform; + protected BrooklynCampPlatformLauncherNoServer launcher; + private boolean forceUpdate; + + @BeforeMethod(alwaysRun = true) + @Override + public void setUp() throws Exception { + super.setUp(); + launcher = new BrooklynCampPlatformLauncherNoServer() { + @Override + protected LocalManagementContext newMgmtContext() { + return (LocalManagementContext) mgmt(); + } + }; + launcher.launch(); + platform = launcher.getCampPlatform(); + } + + @AfterMethod(alwaysRun = true) + @Override + public void tearDown() throws Exception { + try { + super.tearDown(); + } finally { + if (launcher != null) launcher.stopServers(); + } + } + + protected StartableApplication rebind(RebindOptions options) throws Exception { + StartableApplication result = super.rebind(options); + if (launcher != null) { + launcher.stopServers(); + launcher = new BrooklynCampPlatformLauncherNoServer() { + @Override + protected LocalManagementContext newMgmtContext() { + return (LocalManagementContext) mgmt(); + } + }; + launcher.launch(); + platform = launcher.getCampPlatform(); + } + return result; + } + + @Override + protected StartableApplication createApp() { + return null; + } + + protected ManagementContext mgmt() { + return (newManagementContext != null) ? newManagementContext : origManagementContext; + } + + /////////////////////////////////////////////////// + // TODO code below is duplicate of AbstractYamlTest + /////////////////////////////////////////////////// + + protected void waitForApplicationTasks(Entity app) { + Set<Task<?>> tasks = BrooklynTaskTags.getTasksInEntityContext(origManagementContext.getExecutionManager(), app); + getLogger().info("Waiting on " + tasks.size() + " task(s)"); + for (Task<?> t : tasks) { + t.blockUntilEnded(); + } + } + + protected Reader loadYaml(String yamlFileName, String ...extraLines) throws Exception { + String input = new ResourceUtils(this).getResourceAsString(yamlFileName).trim(); + StringBuilder builder = new StringBuilder(input); + for (String l: extraLines) + builder.append("\n").append(l); + return new StringReader(builder.toString()); + } + + protected Entity createAndStartApplication(String... multiLineYaml) throws Exception { + return createAndStartApplication(joinLines(multiLineYaml)); + } + + protected Entity createAndStartApplication(String input) throws Exception { + return createAndStartApplication(new StringReader(input)); + } + + protected Entity createAndStartApplication(Reader input) throws Exception { + AssemblyTemplate at = platform.pdp().registerDeploymentPlan(input); + Assembly assembly; + try { + assembly = at.getInstantiator().newInstance().instantiate(at, platform); + } catch (Exception e) { + getLogger().warn("Unable to instantiate " + at + " (rethrowing): " + e); + throw e; + } + getLogger().info("Test - created " + assembly); + final Entity app = origManagementContext.getEntityManager().getEntity(assembly.getId()); + getLogger().info("App - " + app); + + // wait for app to have started + Set<Task<?>> tasks = origManagementContext.getExecutionManager().getTasksWithAllTags(ImmutableList.of( + BrooklynTaskTags.EFFECTOR_TAG, + BrooklynTaskTags.tagForContextEntity(app), + BrooklynTaskTags.tagForEffectorCall(app, "start", ConfigBag.newInstance(ImmutableMap.of("locations", ImmutableMap.of()))))); + Iterables.getOnlyElement(tasks).get(); + + return app; + } + + protected Entity createStartWaitAndLogApplication(Reader input) throws Exception { + Entity app = createAndStartApplication(input); + waitForApplicationTasks(app); + + getLogger().info("App started:"); + Entities.dumpInfo(app); + + return app; + } + + protected void addCatalogItems(Iterable<String> catalogYaml) { + addCatalogItems(joinLines(catalogYaml)); + } + + protected void addCatalogItems(String... catalogYaml) { + addCatalogItems(joinLines(catalogYaml)); + } + + protected void addCatalogItems(String catalogYaml) { + mgmt().getCatalog().addItems(catalogYaml, forceUpdate); + } + + protected void deleteCatalogEntity(String catalogItem) { + mgmt().getCatalog().deleteCatalogItem(catalogItem, TEST_VERSION); + } + + protected Logger getLogger() { + return LOG; + } + + private String joinLines(Iterable<String> catalogYaml) { + return Joiner.on("\n").join(catalogYaml); + } + + private String joinLines(String[] catalogYaml) { + return Joiner.on("\n").join(catalogYaml); + } + + protected String ver(String id) { + return CatalogUtils.getVersionedId(id, TEST_VERSION); + } + + public void forceCatalogUpdate() { + forceUpdate = true; + } + +} http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/52478ec9/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java ---------------------------------------------------------------------- diff --git a/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java new file mode 100644 index 0000000..ca8dd75 --- /dev/null +++ b/usage/camp/src/test/java/io/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java @@ -0,0 +1,88 @@ +package io.brooklyn.camp.brooklyn.catalog; + +import static org.testng.Assert.assertFalse; +import io.brooklyn.camp.brooklyn.AbstractYamlRebindTest; + +import org.testng.annotations.Test; + +import brooklyn.catalog.internal.CatalogUtils; +import brooklyn.entity.basic.BasicEntity; +import brooklyn.entity.basic.Entities; +import brooklyn.entity.basic.StartableApplication; +import brooklyn.policy.Enricher; +import brooklyn.policy.Policy; +import brooklyn.test.policy.TestEnricher; +import brooklyn.test.policy.TestPolicy; + +import com.google.common.base.Predicates; +import com.google.common.collect.Iterables; + +public class CatalogYamlRebindTest extends AbstractYamlRebindTest { + + // TODO Other tests (relating to https://issues.apache.org/jira/browse/BROOKLYN-149) include: + // - entities cannot be instantiated because class no longer on classpath (e.g. was OSGi) + // - config/attribute cannot be instantiated (e.g. because class no longer on classpath) + // - entity file corrupt + + // See https://issues.apache.org/jira/browse/BROOKLYN-149. + // Deletes the catalog item before rebind, but the referenced types are still on the + // default classpath. + @Test + public void testRebindWithCatalogDeletedAndAppExisting() throws Exception { + runRebindWithCatalogAndApp(true); + } + + @Test + public void testRebindWithCatalogAndApp() throws Exception { + runRebindWithCatalogAndApp(false); + } + + @SuppressWarnings("unused") + protected void runRebindWithCatalogAndApp(boolean deleteCatalogBeforeRebind) throws Exception { + String symbolicName = "my.catalog.app.id.load"; + String version = "0.1.2"; + + // Create the catalog item + addCatalogItems( + "brooklyn.catalog:", + " id: " + symbolicName, + " version: " + version, + " item:", + " type: "+ BasicEntity.class.getName(), + " brooklyn.enrichers:", + " - type: "+TestEnricher.class.getName(), + " brooklyn.policies:", + " - type: "+TestPolicy.class.getName()); + + // Create an app, using that catalog item + String yaml = "name: simple-app-yaml\n" + + "location: localhost\n" + + "services: \n" + + "- type: "+CatalogUtils.getVersionedId(symbolicName, version); + origApp = (StartableApplication) createAndStartApplication(yaml); + BasicEntity origEntity = (BasicEntity) Iterables.getOnlyElement(origApp.getChildren()); + TestPolicy origPolicy = (TestPolicy) Iterables.getOnlyElement(origEntity.getPolicies()); + TestEnricher origEnricher = (TestEnricher) Iterables.tryFind(origEntity.getEnrichers(), Predicates.instanceOf(TestEnricher.class)).get(); + + // Depending on test-mode, delete the catalog item, and then rebind + if (deleteCatalogBeforeRebind) { + mgmt().getCatalog().deleteCatalogItem(symbolicName, version); + } + + rebind(); + + // Ensure app is still there + BasicEntity newEntity = (BasicEntity) Iterables.getOnlyElement(newApp.getChildren()); + Policy newPolicy = Iterables.getOnlyElement(newEntity.getPolicies()); + Enricher newEnricher = Iterables.tryFind(newEntity.getEnrichers(), Predicates.instanceOf(TestEnricher.class)).get(); + + // Ensure app is still usable - e.g. "stop" effector functions as expected + newApp.stop(); + assertFalse(Entities.isManaged(newApp)); + assertFalse(Entities.isManaged(newEntity)); + + if (!deleteCatalogBeforeRebind) { + mgmt().getCatalog().deleteCatalogItem(symbolicName, version); + } + } +}
