address code review - better report errors, tidy code
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/7986ed16 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/7986ed16 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/7986ed16 Branch: refs/heads/master Commit: 7986ed16133e4c0a5bec860fba0d86014adaaa23 Parents: 4b91ca4 Author: Alex Heneveld <[email protected]> Authored: Tue Aug 29 16:40:10 2017 +0100 Committer: Alex Heneveld <[email protected]> Committed: Tue Aug 29 16:40:10 2017 +0100 ---------------------------------------------------------------------- .../brooklyn/camp/brooklyn/RebindOsgiTest.java | 2 ++ .../CatalogYamlEntityOsgiTypeRegistryTest.java | 7 ++++++- .../catalog/internal/BasicBrooklynCatalog.java | 12 ++++++------ .../core/mgmt/ha/OsgiArchiveInstaller.java | 18 ++++++++++++++++-- 4 files changed, 30 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7986ed16/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 fccfb29..20cb545 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 @@ -516,6 +516,8 @@ public class RebindOsgiTest extends AbstractYamlRebindTest { Assert.assertEquals(newBundles, oldBundles, "Bundles: "+newBundles); rebind(); + newBundles = origManagementContext.getOsgiManager().get().getManagedBundles(); + Assert.assertEquals(newBundles, oldBundles, "Bundles: "+newBundles); } @Test http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7986ed16/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 c666af8..aff1ac0 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,7 +36,12 @@ public class CatalogYamlEntityOsgiTypeRegistryTest extends CatalogYamlEntityTest // use OSGi here @Override protected boolean disableOsgi() { return false; } - 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 } + 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 http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7986ed16/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 564b023..bfca5ca 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 @@ -501,8 +501,8 @@ 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<?, ?>> resultLegacyFormat, Map<RegisteredType, RegisteredType> resultNewFormat, - boolean requireValidation, Map<?, ?> parentMeta, int depth, boolean force) { + 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) @@ -558,14 +558,14 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { * @param sourceYaml - metadata source for reference * @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 resultLegacyFormat - list where items should be added, or add to type registry if null + * @param resultNewFormat - map of new->(old or null) for items added * @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) * @param parentMetadata - inherited metadata * @param depth - depth this is running in - * @param force - whether to force the catalog addition (only applies if result is null) + * @param force - whether to force the catalog addition (does not apply in legacy mode where resultLegacyFormat is non-null) */ @SuppressWarnings("unchecked") private void collectCatalogItemsFromItemMetadataBlock(String sourceYaml, ManagedBundle containingBundle, Map<?,?> itemMetadata, List<CatalogItemDtoAbstract<?, ?>> resultLegacyFormat, Map<RegisteredType, RegisteredType> resultNewFormat, boolean requireValidation, @@ -1073,7 +1073,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { String url = null; File f = ((ManagementContextInternal)mgmt).getOsgiManager().get().getBundleFile(containingBundle); if (f!=null) { - url = "file:"+f.getAbsolutePath(); + url = "file//:"+f.getAbsolutePath(); } if (url==null) { url = containingBundle.getUrl(); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/7986ed16/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 c451363..2b837ba 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 @@ -420,7 +420,14 @@ class OsgiArchiveInstaller { result.bundle.start(); } catch (BundleException e) { log.warn("Error starting bundle "+result.getVersionedName()+", uninstalling, restoring any old bundle, then re-throwing error: "+e); - rollbackBundle(); + try { + rollbackBundle(); + } catch (Throwable t) { + Exceptions.propagateIfFatal(t); + log.warn("Error rolling back "+result.getVersionedName()+" after bundle start problem; server may be in inconsistent state (swallowing this error and propagating installation error): "+Exceptions.collapseText(t), t); + throw Exceptions.propagate(new BundleException("Failure installing and rolling back; server may be in inconsistent state regarding bundle "+result.getVersionedName()+". " + + "Rollback failure ("+Exceptions.collapseText(t)+") detailed in log. Installation error is: "+Exceptions.collapseText(e), e)); + } throw Exceptions.propagate(e); } @@ -445,7 +452,14 @@ class OsgiArchiveInstaller { // 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: "+Exceptions.collapseText(e)); - rollbackBundle(); + try { + rollbackBundle(); + } catch (Throwable t) { + Exceptions.propagateIfFatal(t); + log.warn("Error rolling back "+result.getVersionedName()+" after catalog install problem; server may be in inconsistent state (swallowing this error and propagating installation error): "+Exceptions.collapseText(t), t); + throw Exceptions.propagate(new BundleException("Failure loading catalog items, and also failed rolling back; server may be in inconsistent state regarding bundle "+result.getVersionedName()+". " + + "Rollback failure ("+Exceptions.collapseText(t)+") detailed in log. Installation error is: "+Exceptions.collapseText(e), e)); + } if (itemsFromOldBundle!=null) { // add back all itemsFromOldBundle (when replacing a bundle) for (RegisteredType oldItem: itemsFromOldBundle) {
