restore old registered types and bundle if new install fails does everything except revert to the old OSGi bundle from persistence store
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/8127dbba Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/8127dbba Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/8127dbba Branch: refs/heads/master Commit: 8127dbba4e0bdc8b20f7c9cc7a871e5cc8e19f8e Parents: 3566ac0 Author: Alex Heneveld <[email protected]> Authored: Wed Aug 16 11:27:01 2017 +0100 Committer: Alex Heneveld <[email protected]> Committed: Wed Aug 16 14:59:18 2017 +0100 ---------------------------------------------------------------------- .../brooklyn/api/catalog/BrooklynCatalog.java | 10 +-- .../camp/brooklyn/AbstractYamlTest.java | 44 ++++++++++-- .../brooklyn/camp/brooklyn/RebindOsgiTest.java | 19 +++++- .../camp/brooklyn/ReferencedYamlOsgiTest.java | 2 +- .../catalog/CatalogMakeOsgiBundleTest.java | 3 +- .../CatalogYamlEntityOsgiTypeRegistryTest.java | 31 ++++++++- .../brooklyn/catalog/CatalogYamlEntityTest.java | 47 ++++++++++--- .../catalog/internal/BasicBrooklynCatalog.java | 63 +++++++++++------ .../catalog/internal/CatalogBundleLoader.java | 39 ++++++++--- .../core/catalog/internal/CatalogUtils.java | 26 ++++++- .../core/mgmt/ha/OsgiArchiveInstaller.java | 71 +++++++++++++++++--- .../brooklyn/core/mgmt/ha/OsgiManager.java | 42 +++++------- .../rest/resources/CatalogResource.java | 2 +- .../java/org/apache/brooklyn/test/Asserts.java | 6 +- 14 files changed, 309 insertions(+), 96 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java b/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java index 65c2b88..aad8c1d 100644 --- a/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java +++ b/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java @@ -99,12 +99,12 @@ public interface BrooklynCatalog { /** Adds the given registered types defined in a bundle's catalog BOM; * no validation performed, so caller should do that subsequently after * loading all potential inter-dependent types. - * <p> - * Nothing is returned but caller can retrieve the results by searching the - * type registry for types with the same containing bundle. + * Optionally updates a supplied (empty) map containing newly added types as keys + * and any previously existing type they replace as values, for reference or for use rolling back + * (this is populated with work done so far if the method throws an error). */ - @Beta // method may move elsewhere - public void addTypesFromBundleBom(String yaml, ManagedBundle bundle, boolean forceUpdate); + @Beta // method may move elsewhere, or return type may change + public void addTypesFromBundleBom(String yaml, ManagedBundle bundle, boolean forceUpdate, Map<RegisteredType, RegisteredType> result); /** As {@link #validateType(RegisteredType)} but taking a set of types, returning a map whose keys are * those types where validation failed, mapped to the collection of errors validating that type. http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java index 121413c..6c269d2 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/AbstractYamlTest.java @@ -58,6 +58,7 @@ import org.apache.brooklyn.util.exceptions.ReferenceWithError; import org.apache.brooklyn.util.net.Urls; import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.stream.Streams; +import org.osgi.framework.Constants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.annotations.AfterMethod; @@ -232,7 +233,16 @@ public abstract class AbstractYamlTest { mgmt().getCatalog().addItems(catalogYaml, forceUpdate); } - public static void addCatalogItemsAsOsgi(ManagementContext mgmt, String catalogYaml, VersionedName bundleName, boolean force) { + /* + * Have two variants of this as some tests use bundles which can't be started in their environment + * but we can load the specific YAML provided (eg they reference libraries who aren't loadable). + * + * TODO we should refactor so those tests where dependent bundles can't be started either + * have the dependent bundles refactored with java split out from unloadable BOM + * or have the full camp parser available so the BOMs can be loaded. + * (in other words ideally we'd always use the "usual way" method below this instead of this one.) + */ + public static void addCatalogItemsAsOsgiWithoutStartingBundles(ManagementContext mgmt, String catalogYaml, VersionedName bundleName, boolean force) { try { BundleMaker bundleMaker = new BundleMaker(mgmt); File bf = bundleMaker.createTempZip("test", MutableMap.of( @@ -241,16 +251,40 @@ public abstract class AbstractYamlTest { new BasicManagedBundle(bundleName.getSymbolicName(), bundleName.getVersionString(), null), new FileInputStream(bf), false); - // bundle not started (no need), and BOM not installed nor validated above; + + // bundle not started (no need, and can break), and BOM not installed nor validated above; // do BOM install and validation below manually to test the type registry approach - mgmt.getCatalog().addTypesFromBundleBom(catalogYaml, b.get().getMetadata(), force); + // but skipping the rollback / uninstall + mgmt.getCatalog().addTypesFromBundleBom(catalogYaml, b.get().getMetadata(), force, null); Map<RegisteredType, Collection<Throwable>> validation = mgmt.getCatalog().validateTypes( mgmt.getTypeRegistry().getMatching(RegisteredTypePredicates.containingBundle(b.get().getVersionedName())) ); if (!validation.isEmpty()) { - // TODO rollback - throw Exceptions.propagate("Brooklyn failed to load types: "+validation.keySet(), + throw Exceptions.propagate("Brooklyn failed to load types (in tests, skipping rollback): "+validation.keySet(), Iterables.concat(validation.values())); } + + + } catch (Exception e) { + throw Exceptions.propagate(e); + } + } + + public static void addCatalogItemsAsOsgiInUsualWay(ManagementContext mgmt, String catalogYaml, VersionedName bundleName, boolean force) { + try { + BundleMaker bundleMaker = new BundleMaker(mgmt); + File bf = bundleMaker.createTempZip("test", MutableMap.of( + new ZipEntry(BasicBrooklynCatalog.CATALOG_BOM), new ByteArrayInputStream(catalogYaml.getBytes()))); + if (bundleName!=null) { + bf = bundleMaker.copyAddingManifest(bf, MutableMap.of( + "Manifest-Version", "2.0", + Constants.BUNDLE_SYMBOLICNAME, bundleName.getSymbolicName(), + Constants.BUNDLE_VERSION, bundleName.getOsgiVersion().toString())); + } + ReferenceWithError<OsgiBundleInstallationResult> b = ((ManagementContextInternal)mgmt).getOsgiManager().get().install( + new FileInputStream(bf) ); + + b.checkNoError(); + } catch (Exception e) { throw Exceptions.propagate(e); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java index 730616b..fccfb29 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/RebindOsgiTest.java @@ -44,6 +44,7 @@ import org.apache.brooklyn.core.mgmt.osgi.OsgiStandaloneTest; import org.apache.brooklyn.core.mgmt.osgi.OsgiVersionMoreEntityTest; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.test.entity.TestEntity; +import org.apache.brooklyn.entity.stock.BasicEntity; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.test.support.TestResourceUnavailableException; import org.apache.brooklyn.util.core.ResourceUtils; @@ -497,7 +498,7 @@ public class RebindOsgiTest extends AbstractYamlRebindTest { @Test public void testRebindAfterFailedInstall() throws Exception { String appSymbolicName = "my.catalog.app.id.load"; - String appVersion = "0.1.0"; + String appVersion = "0.1.0-SNAPSHOT"; Map<String, ManagedBundle> oldBundles = origManagementContext.getOsgiManager().get().getManagedBundles(); try { addCatalogItems( @@ -517,6 +518,22 @@ public class RebindOsgiTest extends AbstractYamlRebindTest { rebind(); } + @Test + public void testRebindAfterFailedInstallReplacing() throws Exception { + String appSymbolicName = "my.catalog.app.id.load"; + String appVersion = "0.1.0-SNAPSHOT"; + addCatalogItems( + "brooklyn.catalog:", + " id: " + appSymbolicName, + " version: " + appVersion, + " itemType: entity", + " item:", + " type: "+BasicEntity.class.getName()); + // test below will follow a different path if the bundle is already installed; + // it needs to restore the old bundle ZIP input stream from persisted state + testRebindAfterFailedInstall(); + } + private Bundle getBundle(ManagementContext mgmt, final String symbolicName) throws Exception { OsgiManager osgiManager = ((ManagementContextInternal)mgmt).getOsgiManager().get(); Framework framework = osgiManager.getFramework(); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ReferencedYamlOsgiTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ReferencedYamlOsgiTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ReferencedYamlOsgiTest.java index e46a34f..0f982b8 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ReferencedYamlOsgiTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ReferencedYamlOsgiTest.java @@ -31,7 +31,7 @@ public class ReferencedYamlOsgiTest extends ReferencedYamlTest { @Override protected void addCatalogItems(String catalogYaml) { - addCatalogItemsAsOsgi(mgmt(), catalogYaml, VersionedName.fromString("sample-bundle:0-SNAPSHOT"), isForceUpdate()); + addCatalogItemsAsOsgiWithoutStartingBundles(mgmt(), catalogYaml, VersionedName.fromString("sample-bundle:0-SNAPSHOT"), isForceUpdate()); } // these are not broken with OSGi http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java index f5ac4bd..ec96235 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java @@ -57,7 +57,6 @@ import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; -import com.google.common.base.Predicates; import com.google.common.collect.Iterables; public class CatalogMakeOsgiBundleTest extends AbstractYamlTest { @@ -138,7 +137,7 @@ public class CatalogMakeOsgiBundleTest extends AbstractYamlTest { String customName = "catalog-bundle-1-manual-"+Identifiers.makeRandomId(4); jf = bm.copyAddingManifest(jf, MutableMap.of( - "Manifest-Version", "1.0", + "Manifest-Version", "2.0", "Bundle-SymbolicName", customName, "Bundle-Version", "0.0.0.SNAPSHOT")); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java index 35c074d..c666af8 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityOsgiTypeRegistryTest.java @@ -36,15 +36,42 @@ public class CatalogYamlEntityOsgiTypeRegistryTest extends CatalogYamlEntityTest // use OSGi here @Override protected boolean disableOsgi() { return false; } - // use type registry appraoch + enum CatalogItemsInstallationMode { ADD_YAML_ITEMS_UNBUNDLED, BUNDLE_BUT_NOT_STARTED, USUAL_OSGI_WAY_AS_BUNDLE_WITH_DEFAULT_NAME, USUAL_OSGI_WAY_AS_ZIP_NO_MANIFEST_NAME_MAYBE_IN_BOM } + CatalogItemsInstallationMode itemsInstallMode = null; + + // use type registry approach @Override protected void addCatalogItems(String catalogYaml) { - addCatalogItemsAsOsgi(mgmt(), catalogYaml, new VersionedName(bundleName(), bundleVersion()), isForceUpdate()); + switch (itemsInstallMode!=null ? itemsInstallMode : + // this is the default because some "bundles" aren't resolvable or library BOMs loadable in test context + CatalogItemsInstallationMode.BUNDLE_BUT_NOT_STARTED) { + case ADD_YAML_ITEMS_UNBUNDLED: super.addCatalogItems(catalogYaml); break; + case BUNDLE_BUT_NOT_STARTED: + addCatalogItemsAsOsgiWithoutStartingBundles(mgmt(), catalogYaml, new VersionedName(bundleName(), bundleVersion()), isForceUpdate()); + break; + case USUAL_OSGI_WAY_AS_BUNDLE_WITH_DEFAULT_NAME: + addCatalogItemsAsOsgiInUsualWay(mgmt(), catalogYaml, new VersionedName(bundleName(), bundleVersion()), isForceUpdate()); + break; + case USUAL_OSGI_WAY_AS_ZIP_NO_MANIFEST_NAME_MAYBE_IN_BOM: + addCatalogItemsAsOsgiInUsualWay(mgmt(), catalogYaml, null, isForceUpdate()); + break; + } } protected String bundleName() { return "sample-bundle"; } protected String bundleVersion() { return BasicBrooklynCatalog.NO_VERSION; } + @Override + protected void doTestReplacementFailureLeavesPreviousIntact(boolean bundleHasId) throws Exception { + try { + itemsInstallMode = bundleHasId ? CatalogItemsInstallationMode.USUAL_OSGI_WAY_AS_ZIP_NO_MANIFEST_NAME_MAYBE_IN_BOM : + CatalogItemsInstallationMode.ADD_YAML_ITEMS_UNBUNDLED; + super.doTestReplacementFailureLeavesPreviousIntact(bundleHasId); + } finally { + itemsInstallMode = null; + } + } + @Test // basic test that this approach to adding types works public void testAddTypes() throws Exception { String symbolicName = "my.catalog.app.id.load"; http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java index b449f69..f34e05d 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlEntityTest.java @@ -23,6 +23,8 @@ import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; +import java.util.List; + import org.apache.brooklyn.api.catalog.BrooklynCatalog; import org.apache.brooklyn.api.entity.Entity; import org.apache.brooklyn.api.entity.EntitySpec; @@ -41,6 +43,7 @@ import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.entity.stock.BasicEntity; import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.collections.MutableList; import org.testng.Assert; import org.testng.annotations.Test; @@ -672,13 +675,31 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { } @Test - public void testReplacementFailureLeavesPreviousIntact() throws Exception { + public void testReplacementFailureLeavesPreviousNamedBundleIntact() throws Exception { + doTestReplacementFailureLeavesPreviousIntact(true); + } + + @Test + public void testReplacementFailureLeavesPreviousItemFromAnonymousBundleIntact() throws Exception { + // for anonymous bundles we have to look at what items from other bundles might have been replaced + doTestReplacementFailureLeavesPreviousIntact(false); + } + + protected void doTestReplacementFailureLeavesPreviousIntact(boolean includeBundleName) throws Exception { String symbolicName = "my.catalog.app.id.load"; - addCatalogItems( + List<String> lines = MutableList.of( "brooklyn.catalog:", - " id: " + symbolicName, - " version: " + TEST_VERSION_SNAPSHOT, - " item: " + BasicEntity.class.getName()); + " bundle: testing-replacement", + " version: 0.1-SNAPSHOT", + " items:", + " - ", + " id: " + symbolicName, + " version: " + TEST_VERSION_SNAPSHOT, + " item: " + BasicEntity.class.getName()); + if (!includeBundleName) { + lines.remove(1); lines.remove(1); + } + addCatalogItems(lines); RegisteredType item = mgmt().getTypeRegistry().get(symbolicName, TEST_VERSION_SNAPSHOT); Assert.assertNotNull(item); @@ -686,11 +707,19 @@ public class CatalogYamlEntityTest extends AbstractYamlTest { assertEquals(item.getSymbolicName(), symbolicName); try { - addCatalogItems( + lines = MutableList.of( "brooklyn.catalog:", - " id: " + symbolicName, - " version: " + TEST_VERSION_SNAPSHOT, - " item: " + "DeliberatelyMissing"); + " bundle: testing-replacement", + " version: 0.1-SNAPSHOT", + " items:", + " - ", + " id: " + symbolicName, + " version: " + TEST_VERSION_SNAPSHOT, + " item: " + "DeliberatelyMissing"); + if (!includeBundleName) { + lines.remove(1); lines.remove(1); + } + addCatalogItems(lines); Asserts.shouldHaveFailedPreviously(); } catch (Exception e) { Asserts.expectedFailureContains(e, "DeliberatelyMissing", symbolicName); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java index 83d5028..deaae6f 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/BasicBrooklynCatalog.java @@ -500,7 +500,9 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { /** See comments on {@link #collectCatalogItemsFromItemMetadataBlock(String, ManagedBundle, Map, List, boolean, Map, int, boolean)}; * this is a shell around that that parses the `brooklyn.catalog` header on the BOM YAML file */ - private void collectCatalogItemsFromCatalogBomRoot(String yaml, ManagedBundle containingBundle, List<CatalogItemDtoAbstract<?, ?>> result, boolean requireValidation, Map<?, ?> parentMeta, int depth, boolean force) { + private void collectCatalogItemsFromCatalogBomRoot(String yaml, ManagedBundle containingBundle, + List<CatalogItemDtoAbstract<?, ?>> resultLegacyFormat, Map<RegisteredType, RegisteredType> resultNewFormat, + boolean requireValidation, Map<?, ?> parentMeta, int depth, boolean force) { Map<?,?> itemDef = Yamls.getAs(Yamls.parseAll(yaml), Map.class); Map<?,?> catalogMetadata = getFirstAsMap(itemDef, "brooklyn.catalog").orNull(); if (catalogMetadata==null) @@ -508,7 +510,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { catalogMetadata = MutableMap.copyOf(catalogMetadata); collectCatalogItemsFromItemMetadataBlock(Yamls.getTextOfYamlAtPath(yaml, "brooklyn.catalog").getMatchedYamlTextOrWarn(), - containingBundle, catalogMetadata, result, requireValidation, parentMeta, 0, force); + containingBundle, catalogMetadata, resultLegacyFormat, resultNewFormat, requireValidation, parentMeta, 0, force); itemDef.remove("brooklyn.catalog"); catalogMetadata.remove("item"); @@ -527,7 +529,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { if (rootItemYaml.startsWith(match)) rootItemYaml = Strings.removeFromStart(rootItemYaml, match); else rootItemYaml = Strings.replaceAllNonRegex(rootItemYaml, "\n"+match, ""); } - collectCatalogItemsFromItemMetadataBlock("item:\n"+makeAsIndentedObject(rootItemYaml), containingBundle, rootItem, result, requireValidation, catalogMetadata, 1, force); + collectCatalogItemsFromItemMetadataBlock("item:\n"+makeAsIndentedObject(rootItemYaml), containingBundle, rootItem, resultLegacyFormat, resultNewFormat, requireValidation, catalogMetadata, 1, force); } } @@ -557,6 +559,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { * @param containingBundle - bundle where this is being loaded, or null * @param itemMetadata - map of this item metadata reap * @param result - list where items should be added, or add to type registry if null + * @param resultNewFormat * @param requireValidation - whether to require items to be validated; if false items might not be valid, * and/or their catalog item types might not be set correctly yet; caller should normally validate later * (useful if we have to load a bunch of things before they can all be validated) @@ -565,7 +568,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { * @param force - whether to force the catalog addition (only applies if result is null) */ @SuppressWarnings("unchecked") - private void collectCatalogItemsFromItemMetadataBlock(String sourceYaml, ManagedBundle containingBundle, Map<?,?> itemMetadata, List<CatalogItemDtoAbstract<?, ?>> result, boolean requireValidation, + private void collectCatalogItemsFromItemMetadataBlock(String sourceYaml, ManagedBundle containingBundle, Map<?,?> itemMetadata, List<CatalogItemDtoAbstract<?, ?>> resultLegacyFormat, Map<RegisteredType, RegisteredType> resultNewFormat, boolean requireValidation, Map<?,?> parentMetadata, int depth, boolean force) { if (sourceYaml==null) sourceYaml = new Yaml().dump(itemMetadata); @@ -650,12 +653,15 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { throw new IllegalStateException("Cannot scan for Java catalog items when libraries declared on an ancestor; scanJavaAnnotations should be specified alongside brooklyn.libraries (or ideally those libraries should specify to scan)"); } if (scanResult!=null && !scanResult.isEmpty()) { - if (result!=null) { - result.addAll( scanResult ); + if (resultLegacyFormat!=null) { + resultLegacyFormat.addAll( scanResult ); } else { - // not returning a result; we need to add here + // not returning a result; we need to add here, as type for (CatalogItem item: scanResult) { + RegisteredType replacedInstance = mgmt.getTypeRegistry().get(item.getSymbolicName(), item.getVersion()); mgmt.getCatalog().addItem(item); + RegisteredType newInstance = mgmt.getTypeRegistry().get(item.getSymbolicName(), item.getVersion()); + updateResultNewFormat(resultNewFormat, replacedInstance, newInstance); } } } @@ -681,18 +687,19 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { int count = 0; for (Object ii: checkType(items, "items", List.class)) { if (ii instanceof String) { - collectUrlReferencedCatalogItems((String) ii, containingBundle, result, requireValidation, catalogMetadata, depth+1, force); + collectUrlReferencedCatalogItems((String) ii, containingBundle, resultLegacyFormat, resultNewFormat, requireValidation, catalogMetadata, depth+1, force); } else { Map<?,?> i = checkType(ii, "entry in items list", Map.class); collectCatalogItemsFromItemMetadataBlock(Yamls.getTextOfYamlAtPath(sourceYaml, "items", count).getMatchedYamlTextOrWarn(), - containingBundle, i, result, requireValidation, catalogMetadata, depth+1, force); + containingBundle, i, resultLegacyFormat, resultNewFormat, requireValidation, catalogMetadata, depth+1, force); } count++; } } if (url != null) { - collectUrlReferencedCatalogItems(checkType(url, "include in catalog meta", String.class), containingBundle, result, requireValidation, catalogMetadata, depth+1, force); + collectUrlReferencedCatalogItems(checkType(url, "include in catalog meta", String.class), containingBundle, + resultLegacyFormat, resultNewFormat, requireValidation, catalogMetadata, depth+1, force); } if (item==null) return; @@ -716,7 +723,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { log.warn("Name property will be ignored due to the existence of displayName and at least one of id, symbolicName"); } - PlanInterpreterGuessingType planInterpreter = new PlanInterpreterGuessingType(null, item, sourceYaml, itemType, libraryBundles, result).reconstruct(); + PlanInterpreterGuessingType planInterpreter = new PlanInterpreterGuessingType(null, item, sourceYaml, itemType, libraryBundles, resultLegacyFormat).reconstruct(); Exception resolutionError = null; if (!planInterpreter.isResolved()) { // don't throw yet, we may be able to add it in an unresolved state @@ -858,13 +865,13 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { final Boolean catalogDeprecated = Boolean.valueOf(deprecated); // run again now that we know the ID to catch recursive definitions and possibly other mistakes (itemType inconsistency?) - planInterpreter = new PlanInterpreterGuessingType(id, item, sourceYaml, itemType, libraryBundles, result).reconstruct(); + planInterpreter = new PlanInterpreterGuessingType(id, item, sourceYaml, itemType, libraryBundles, resultLegacyFormat).reconstruct(); if (resolutionError==null && !planInterpreter.isResolved()) { resolutionError = new IllegalStateException("Plan resolution for "+id+" breaks after id and itemType are set; is there a recursive reference or other type inconsistency?\n"+sourceYaml); } String sourcePlanYaml = planInterpreter.getPlanYaml(); - if (result==null) { + if (resultLegacyFormat==null) { // horrible API but basically if `result` is null then add to local unpersisted registry instead, // without forcing resolution and ignoring errors; this lets us deal with forward references, but // we'll have to do a validation step subsequently. (already we let bundles deal with persistence, @@ -922,8 +929,10 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { // record original source in case it was changed RegisteredTypes.notePlanEquivalentToThis(type, new BasicTypeImplementationPlan(format, sourceYaml)); + RegisteredType replacedInstance = mgmt.getTypeRegistry().get(type.getSymbolicName(), type.getVersion()); ((BasicBrooklynTypeRegistry) mgmt.getTypeRegistry()).addToLocalUnpersistedTypeRegistry(type, force); - + updateResultNewFormat(resultNewFormat, replacedInstance, type); + } else { if (resolutionError!=null) { // if there was an error, throw it here @@ -940,7 +949,18 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { .build(); dto.setManagementContext((ManagementContextInternal) mgmt); - result.add(dto); + resultLegacyFormat.add(dto); + } + } + + private void updateResultNewFormat(Map<RegisteredType, RegisteredType> resultNewFormat, RegisteredType replacedInstance, + RegisteredType newInstance) { + if (resultNewFormat!=null) { + if (resultNewFormat.containsKey(newInstance)) { + log.debug("Multiple definitions for "+newInstance+" in BOM; only recording one"); + } else { + resultNewFormat.put(newInstance, replacedInstance); + } } } @@ -978,7 +998,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { return wrapped!=null && wrapped.equalsIgnoreCase("true"); } - private void collectUrlReferencedCatalogItems(String url, ManagedBundle containingBundle, List<CatalogItemDtoAbstract<?, ?>> result, boolean requireValidation, Map<Object, Object> parentMeta, int depth, boolean force) { + private void collectUrlReferencedCatalogItems(String url, ManagedBundle containingBundle, List<CatalogItemDtoAbstract<?, ?>> resultLegacyFormat, Map<RegisteredType, RegisteredType> resultNewFormat, boolean requireValidation, Map<Object, Object> parentMeta, int depth, boolean force) { @SuppressWarnings("unchecked") List<?> parentLibrariesRaw = MutableList.copyOf(getFirstAs(parentMeta, Iterable.class, "brooklyn.libraries", "libraries").orNull()); Collection<CatalogBundle> parentLibraries = CatalogItemDtoAbstract.parseLibraries(parentLibrariesRaw); @@ -990,7 +1010,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { Exceptions.propagateIfFatal(e); throw new IllegalStateException("Remote catalog url " + url + " can't be fetched.", e); } - collectCatalogItemsFromCatalogBomRoot(yaml, containingBundle, result, requireValidation, parentMeta, depth, force); + collectCatalogItemsFromCatalogBomRoot(yaml, containingBundle, resultLegacyFormat, resultNewFormat, requireValidation, parentMeta, depth, force); } @SuppressWarnings("unchecked") @@ -1390,7 +1410,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { bf.delete(); } if (result.getCode().isError()) { - // TODO rollback installation, restore others? + // rollback done by install call above throw new IllegalStateException(result.getMessage()); } uninstallEmptyWrapperBundles(); @@ -1428,7 +1448,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { log.debug("Adding catalog item to "+mgmt+": "+yaml); checkNotNull(yaml, "yaml"); List<CatalogItemDtoAbstract<?, ?>> result = MutableList.of(); - collectCatalogItemsFromCatalogBomRoot(yaml, bundle, result, true, ImmutableMap.of(), 0, forceUpdate); + collectCatalogItemsFromCatalogBomRoot(yaml, bundle, result, null, true, ImmutableMap.of(), 0, forceUpdate); // do this at the end for atomic updates; if there are intra-yaml references, we handle them specially for (CatalogItemDtoAbstract<?, ?> item: result) { @@ -1441,10 +1461,11 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { } @Override @Beta - public void addTypesFromBundleBom(String yaml, ManagedBundle bundle, boolean forceUpdate) { + public void addTypesFromBundleBom(String yaml, ManagedBundle bundle, boolean forceUpdate, Map<RegisteredType, RegisteredType> result) { log.debug("Adding catalog item to "+mgmt+": "+yaml); checkNotNull(yaml, "yaml"); - collectCatalogItemsFromCatalogBomRoot(yaml, bundle, null, false, MutableMap.of(), 0, forceUpdate); + if (result==null) result = MutableMap.of(); + collectCatalogItemsFromCatalogBomRoot(yaml, bundle, null, result, false, MutableMap.of(), 0, forceUpdate); } @Override @Beta http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java index 339280e..df2f212 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java @@ -27,6 +27,7 @@ import java.net.URL; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.mgmt.ManagementContext; @@ -35,6 +36,7 @@ import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.stream.Streams; @@ -58,22 +60,34 @@ public class CatalogBundleLoader { this.managementContext = managementContext; } + /** @deprecated since 0.12.0 use {@link #scanForCatalog(Bundle, boolean, boolean, Map)} */ public void scanForCatalog(Bundle bundle, boolean force, boolean validate) { - scanForCatalogInternal(bundle, force, validate, false); + scanForCatalog(bundle, force, validate, null); + } + + public void scanForCatalog(Bundle bundle, boolean force, boolean validate, Map<RegisteredType, RegisteredType> result) { + scanForCatalogInternal(bundle, force, validate, false, result); } /** @deprecated since 0.12.0 */ @Deprecated // scans a bundle which is installed but Brooklyn isn't managing (will probably remove) public Iterable<? extends CatalogItem<?, ?>> scanForCatalogLegacy(Bundle bundle, boolean force) { LOG.warn("Bundle "+bundle+" being loaded with deprecated legacy loader"); - return scanForCatalogInternal(bundle, force, true, true); + return scanForCatalogInternal(bundle, force, true, true, null).legacyResult; } - private Iterable<? extends CatalogItem<?, ?>> scanForCatalogInternal(Bundle bundle, boolean force, boolean validate, boolean legacy) { + private static class TemporaryInternalScanResult { + Iterable<? extends CatalogItem<?, ?>> legacyResult; + Map<RegisteredType, RegisteredType> mapOfNewToReplaced; + + } + private TemporaryInternalScanResult scanForCatalogInternal(Bundle bundle, boolean force, boolean validate, boolean legacy, Map<RegisteredType, RegisteredType> resultNewFormat) { ManagedBundle mb = ((ManagementContextInternal)managementContext).getOsgiManager().get().getManagedBundle( new VersionedName(bundle)); - Iterable<? extends CatalogItem<?, ?>> catalogItems = MutableList.of(); + TemporaryInternalScanResult result = new TemporaryInternalScanResult(); + result.legacyResult = MutableList.of(); + result.mapOfNewToReplaced = resultNewFormat; final URL bom = bundle.getResource(CatalogBundleLoader.CATALOG_BOM_URL); if (null != bom) { @@ -84,15 +98,20 @@ public class CatalogBundleLoader { legacy = true; } if (legacy) { - catalogItems = this.managementContext.getCatalog().addItems(bomText, mb, force); - for (CatalogItem<?, ?> item : catalogItems) { + result.legacyResult = this.managementContext.getCatalog().addItems(bomText, mb, force); + for (CatalogItem<?, ?> item : result.legacyResult) { LOG.debug("Added to catalog: {}, {}", item.getSymbolicName(), item.getVersion()); } } else { - this.managementContext.getCatalog().addTypesFromBundleBom(bomText, mb, force); + this.managementContext.getCatalog().addTypesFromBundleBom(bomText, mb, force, result.mapOfNewToReplaced); if (validate) { - Map<RegisteredType, Collection<Throwable>> validationErrors = this.managementContext.getCatalog().validateTypes( - this.managementContext.getTypeRegistry().getMatching(RegisteredTypePredicates.containingBundle(mb.getVersionedName())) ); + Set<RegisteredType> matches = MutableSet.copyOf(this.managementContext.getTypeRegistry().getMatching(RegisteredTypePredicates.containingBundle(mb.getVersionedName()))); + if (!matches.equals(result.mapOfNewToReplaced.keySet())) { + // sanity check + LOG.warn("Discrepancy in list of Brooklyn items found for "+mb.getVersionedName()+": "+ + "installer said "+result.mapOfNewToReplaced+" but registry looking found "+matches); + } + Map<RegisteredType, Collection<Throwable>> validationErrors = this.managementContext.getCatalog().validateTypes( matches ); if (!validationErrors.isEmpty()) { throw Exceptions.propagate("Failed to install "+mb.getVersionedName()+", types "+validationErrors.keySet()+" gave errors", Iterables.concat(validationErrors.values())); @@ -107,7 +126,7 @@ public class CatalogBundleLoader { LOG.debug("No BOM found in {} {} {}", CatalogUtils.bundleIds(bundle)); } - return catalogItems; + return result; } /** http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java index f75a406..06286c4 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java @@ -20,6 +20,7 @@ package org.apache.brooklyn.core.catalog.internal; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.NoSuchElementException; import javax.annotation.Nullable; @@ -52,7 +53,9 @@ import org.apache.brooklyn.core.typereg.RegisteredTypeNaming; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; +import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.text.Strings; @@ -65,6 +68,7 @@ import com.google.common.annotations.Beta; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; +import com.google.common.collect.Iterables; public class CatalogUtils { private static final Logger log = LoggerFactory.getLogger(CatalogUtils.class); @@ -185,10 +189,28 @@ public class CatalogUtils { } results.add(result); } + Map<String, Throwable> errors = MutableMap.of(); for (OsgiBundleInstallationResult r: results) { if (r.getDeferredStart()!=null) { - r.getDeferredStart().run(); - // TODO on failure? + try { + r.getDeferredStart().run(); + } catch (Throwable t) { + Exceptions.propagateIfFatal(t); + // above will done rollback for the failed item, but we need consistent behaviour for all libraries; + // for simplicity we simply have each bundle either fully installed or fully rolled back + // (alternative would be to roll back everything) + errors.put(r.getVersionedName().toString(), t); + } + } + } + if (!errors.isEmpty()) { + logDebugOrTraceIfRebinding(log, "Tried registering {} libraries, {} succeeded, but failed {} (throwing)", + new Object[] { libraries.size(), libraries.size() - errors.size(), errors.keySet() }); + if (errors.size()==1) { + throw Exceptions.propagateAnnotated("Error starting referenced library in Brooklyn bundle "+Iterables.getOnlyElement(errors.keySet()), + Iterables.getOnlyElement(errors.values())); + } else { + throw Exceptions.create("Error starting referenced libraries in Brooklyn bundles "+errors.keySet(), errors.values()); } } if (log.isDebugEnabled()) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java index 231bb07..0b018b2 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java @@ -36,6 +36,7 @@ import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult.ResultCode; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; +import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry; import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; import org.apache.brooklyn.util.collections.MutableList; @@ -59,6 +60,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.base.Objects; +import com.google.common.base.Preconditions; import com.google.common.collect.Iterables; // package-private so we can move this one if/when we move OsgiManager @@ -352,6 +354,7 @@ class OsgiArchiveInstaller { } osgiManager.checkCorrectlyInstalled(result.getMetadata(), result.bundle); + File oldZipIfSet = ((BasicManagedBundle)result.getMetadata()).getTempLocalFileWhenJustUploaded(); ((BasicManagedBundle)result.getMetadata()).setTempLocalFileWhenJustUploaded(zipFile); zipFile = null; // don't close/delete it here, we'll use it for uploading, then it will delete it @@ -379,26 +382,76 @@ class OsgiArchiveInstaller { // eg rebind code, brooklyn.libraries list -- deferred start allows caller to // determine whether not to start or to start all after things are installed Runnable startRunnable = new Runnable() { + private void rollbackBundle() { + if (updating) { + if (oldZipIfSet!=null) { + ((BasicManagedBundle)result.getMetadata()).setTempLocalFileWhenJustUploaded(oldZipIfSet); + } else { + // TODO look in persistence + } + log.debug("Rolling back bundle "+result.getVersionedName()+" to state from "+oldZipIfSet); + try { + result.bundle.update(new FileInputStream(Preconditions.checkNotNull(oldZipIfSet, "Couldn't find contents of old version of bundle"))); + } catch (Exception e) { + log.error("Error rolling back following failed install of updated "+result.getVersionedName()+"; " + + "installation will likely be corrupted and correct version should be manually installed.", e); + } + + } else { + log.debug("Uninstalling bundle "+result.getVersionedName()+" (rolling back, but no previous version)"); + osgiManager.uninstallUploadedBundle(result.getMetadata()); + } + } public void run() { if (start) { try { log.debug("Starting bundle "+result.getVersionedName()); result.bundle.start(); } catch (BundleException e) { + log.warn("Error starting bundle "+result.getVersionedName()+", uninstalling, restoring any old bundle, then re-throwing error: "+e); + rollbackBundle(); + throw Exceptions.propagate(e); } } if (loadCatalogBom) { - if (updating) { - osgiManager.uninstallCatalogItemsFromBundle( result.getVersionedName() ); - // (ideally removal and addition would be atomic) - } - osgiManager.loadCatalogBom(result.bundle, force, validateTypes); - Iterable<RegisteredType> items = mgmt().getTypeRegistry().getMatching(RegisteredTypePredicates.containingBundle(result.getMetadata())); - log.debug("Adding items from bundle "+result.getVersionedName()+": "+items); - for (RegisteredType ci: items) { - result.catalogItemsInstalled.add(ci.getId()); + Iterable<RegisteredType> itemsFromOldBundle = null; + Map<RegisteredType, RegisteredType> itemsReplacedHere = null; + try { + if (updating) { + itemsFromOldBundle = osgiManager.uninstallCatalogItemsFromBundle( result.getVersionedName() ); + // (ideally removal and addition would be atomic) + } + itemsReplacedHere = MutableMap.of(); + osgiManager.loadCatalogBom(result.bundle, force, validateTypes, itemsReplacedHere); + Iterable<RegisteredType> items = mgmt().getTypeRegistry().getMatching(RegisteredTypePredicates.containingBundle(result.getMetadata())); + log.debug("Adding items from bundle "+result.getVersionedName()+": "+items); + for (RegisteredType ci: items) { + result.catalogItemsInstalled.add(ci.getId()); + } + } catch (Exception e) { + // unable to install new items; rollback bundles + // and reload replaced items + log.warn("Error adding Brooklyn items from bundle "+result.getVersionedName()+", uninstalling, restoring any old bundle and items, then re-throwing error: "+e); + rollbackBundle(); + if (itemsFromOldBundle!=null) { + // add back all itemsFromOldBundle (when replacing a bundle) + for (RegisteredType oldItem: itemsFromOldBundle) { + ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(oldItem, true); + } + } + if (itemsReplacedHere!=null) { + // and restore any items from other bundles (eg wrappers) that were replaced + MutableList<RegisteredType> replaced = MutableList.copyOf(itemsReplacedHere.values()); + // in reverse order so if other bundle adds multiple we end up with the real original + Collections.reverse(replaced); + for (RegisteredType oldItem: replaced) { + ((BasicBrooklynTypeRegistry)mgmt().getTypeRegistry()).addToLocalUnpersistedTypeRegistry(oldItem, true); + } + } + + throw Exceptions.propagate(e); } } } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java index b65894c..18a2b33 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiManager.java @@ -34,7 +34,6 @@ import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nullable; -import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.catalog.CatalogItem.CatalogBundle; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.typereg.ManagedBundle; @@ -337,12 +336,13 @@ public class OsgiManager { } @Beta - public void uninstallCatalogItemsFromBundle(VersionedName bundle) { + public Iterable<RegisteredType> uninstallCatalogItemsFromBundle(VersionedName bundle) { List<RegisteredType> thingsFromHere = ImmutableList.copyOf(getTypesFromBundle( bundle )); log.debug("Uninstalling items from bundle "+bundle+": "+thingsFromHere); for (RegisteredType t: thingsFromHere) { mgmt.getCatalog().deleteCatalogItem(t.getSymbolicName(), t.getVersion()); } + return thingsFromHere; } @Beta @@ -369,35 +369,27 @@ public class OsgiManager { } } - // since 0.12.0 no longer returns items; it installs non-persisted RegisteredTypes to the type registry instead + /** installs RegisteredTypes in the BOM of this bundle into the type registry, + * non-persisted but done on rebind for each persisted bundle + * + * @param bundle + * @param force + * @param validate + * @param results optional parameter collecting all results, with new type as key, and any type it replaces as value + * + * @since 0.12.0 + */ + // returns map of new items pointing at any replaced item (for reference / rollback) @Beta - public void loadCatalogBom(Bundle bundle, boolean force, boolean validate) { - loadCatalogBomInternal(mgmt, bundle, force, validate); - } - - private static Iterable<? extends CatalogItem<?, ?>> loadCatalogBomInternal(ManagementContext mgmt, Bundle bundle, boolean force, boolean validate) { - Iterable<? extends CatalogItem<?, ?>> catalogItems = MutableList.of(); - + public void loadCatalogBom(Bundle bundle, boolean force, boolean validate, Map<RegisteredType,RegisteredType> result) { try { - CatalogBundleLoader cl = new CatalogBundleLoader(mgmt); - cl.scanForCatalog(bundle, force, validate); - catalogItems = null; + new CatalogBundleLoader(mgmt).scanForCatalog(bundle, force, validate, result); } catch (RuntimeException ex) { - // TODO uninstall? - - // TODO confirm -- as of May 2017 we no longer uninstall the bundle if install of catalog items fails; - // caller needs to upgrade, or uninstall then reinstall - // (this uninstall wouldn't have unmanaged it in brooklyn in any case) -// try { -// bundle.uninstall(); -// } catch (BundleException e) { -// log.error("Cannot uninstall bundle " + bundle.getSymbolicName() + ":" + bundle.getVersion()+" (after error installing catalog items)", e); -// } + // as of May 2017 we no longer uninstall the bundle here if install of catalog items fails; + // the OsgiManager routines which call this method will do this throw new IllegalArgumentException("Error installing catalog items", ex); } - - return catalogItems; } void checkCorrectlyInstalled(OsgiBundleWithUrl bundle, Bundle b) { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java ---------------------------------------------------------------------- diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index 230c986..d327e40 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -197,7 +197,7 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat } if (result.hasError()) { - // TODO rollback installation? + // (rollback already done as part of install, if necessary) if (log.isTraceEnabled()) { log.trace("Unable to create from archive, returning 400: "+result.getError().getMessage(), result.getError()); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8127dbba/utils/common/src/main/java/org/apache/brooklyn/test/Asserts.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/test/Asserts.java b/utils/common/src/main/java/org/apache/brooklyn/test/Asserts.java index b5cdcbc..0a105d9 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/test/Asserts.java +++ b/utils/common/src/main/java/org/apache/brooklyn/test/Asserts.java @@ -1283,7 +1283,7 @@ public class Asserts { public static void expectedFailureContains(Throwable e, String phrase1ToContain, String ...optionalOtherPhrasesToContain) { if (e instanceof ShouldHaveFailedPreviouslyAssertionError) throw (Error)e; try { - assertStringContains(e.toString(), phrase1ToContain, optionalOtherPhrasesToContain); + assertStringContains(Exceptions.collapseText(e), phrase1ToContain, optionalOtherPhrasesToContain); } catch (AssertionError ee) { rethrowPreferredException(e, ee); } @@ -1293,7 +1293,7 @@ public class Asserts { public static void expectedFailureContainsIgnoreCase(Throwable e, String phrase1ToContain, String ...optionalOtherPhrasesToContain) { if (e instanceof ShouldHaveFailedPreviouslyAssertionError) throw (Error)e; try { - assertStringContainsIgnoreCase(e.toString(), phrase1ToContain, optionalOtherPhrasesToContain); + assertStringContainsIgnoreCase(Exceptions.collapseText(e), phrase1ToContain, optionalOtherPhrasesToContain); } catch (AssertionError ee) { rethrowPreferredException(e, ee); } @@ -1305,7 +1305,7 @@ public class Asserts { public static void expectedFailureDoesNotContain(Throwable e, String phrase1ToNotContain, String ...optionalOtherPhrasesToNotContain) { if (e instanceof ShouldHaveFailedPreviouslyAssertionError) throw (Error)e; try { - assertStringDoesNotContain(e.toString(), phrase1ToNotContain, optionalOtherPhrasesToNotContain); + assertStringDoesNotContain(Exceptions.collapseText(e), phrase1ToNotContain, optionalOtherPhrasesToNotContain); } catch (AssertionError ee) { rethrowPreferredException(e, ee); }
