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:48:01 UTC
[10/11] brooklyn-server git commit: address code review - better
report errors, tidy code
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 <al...@cloudsoftcorp.com>
Authored: Tue Aug 29 16:40:10 2017 +0100
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
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) {