You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2017/09/07 15:47:53 UTC
[02/11] brooklyn-server git commit: restore old registered types and
bundle if new install fails
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 <al...@cloudsoftcorp.com>
Authored: Wed Aug 16 11:27:01 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
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);
}