Delete wrapper bundles who have no types installed. There is no point to them (they aren't OSGi). Fixes bug where uploading the same BOM multiple times, could cause bundle leakage.
And tidy logging, more tests. Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/480a0c76 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/480a0c76 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/480a0c76 Branch: refs/heads/master Commit: 480a0c76ae434a7d05455ec0de63e33b5c05aebc Parents: dc43f03 Author: Alex Heneveld <[email protected]> Authored: Wed Jun 28 10:43:17 2017 +0100 Committer: Alex Heneveld <[email protected]> Committed: Wed Jun 28 10:43:17 2017 +0100 ---------------------------------------------------------------------- .../catalog/CatalogOsgiYamlVersioningTest.java | 46 +++++++++++++++++++- .../catalog/internal/BasicBrooklynCatalog.java | 22 ++++++++++ .../core/mgmt/ha/OsgiArchiveInstaller.java | 24 ++++++++-- .../brooklyn/core/mgmt/ha/OsgiManager.java | 6 ++- .../brooklyn/util/osgi/VersionedName.java | 7 ++- 5 files changed, 97 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/480a0c76/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java ---------------------------------------------------------------------- diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java index aab9920..9a047d8 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlVersioningTest.java @@ -18,7 +18,12 @@ */ package org.apache.brooklyn.camp.brooklyn.catalog; +import java.util.Collection; + +import org.apache.brooklyn.api.typereg.ManagedBundle; +import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.test.Asserts; +import org.testng.Assert; import org.testng.annotations.Test; /** As parent tests, but using OSGi, and some of the additions are stricter / different */ @@ -33,7 +38,8 @@ public class CatalogOsgiYamlVersioningTest extends CatalogYamlVersioningTest { @Test public void testAddSameVersionWithoutBundle() { try { - // parent test should fail in OSGi + // parent test should fail in OSGi - anonymous bundle is snapshot so updating is attempted + // but item version is not snapshot and containing bundle is different, so ultimately fails super.testAddSameVersionWithoutBundle(); Asserts.shouldHaveFailedPreviously("Expected to fail because containing bundle will be different when using OSGi"); } catch (Exception e) { @@ -43,6 +49,16 @@ public class CatalogOsgiYamlVersioningTest extends CatalogYamlVersioningTest { } @Test + public void testAddSameVersionWithoutBundleWorksIfItemIsSnapshot() { + String symbolicName = "sampleId"; + String version = "0.1.0-SNAPSHOT"; + addCatalogEntityWithoutBundle(symbolicName, version); + // allowed because item is snapshot + addCatalogEntityWithoutBundle(symbolicName, version); + assertJustOneBundle(); + } + + @Test public void testAddSameVersionWithoutBundleWorksIfForced() { String symbolicName = "sampleId"; String version = "0.1.0"; @@ -50,7 +66,6 @@ public class CatalogOsgiYamlVersioningTest extends CatalogYamlVersioningTest { forceCatalogUpdate(); addCatalogEntityWithoutBundle(symbolicName, version); } - @Override protected void checkAddSameVersionFailsWhenIconIsDifferent(Exception e) { @@ -58,4 +73,31 @@ public class CatalogOsgiYamlVersioningTest extends CatalogYamlVersioningTest { "cannot install a different bundle at a same non-snapshot version"); assertExpectedFailureIncludesSampleId(e); } + + @Test + public void testEmptyCatalogBundleIsRemoved() { + Collection<ManagedBundle> bundles = ((ManagementContextInternal)mgmt()).getOsgiManager().get().getManagedBundles().values(); + Assert.assertTrue(bundles.isEmpty(), "Expected no bundles before starting; but had: "+bundles); + } + + @Override + @Test + public void testAddSameVersionWorksIfSame() { + // in OSGi, assert additionally that we aren't leaking bundles + super.testAddSameVersionWorksIfSame(); + assertJustOneBundle(); + } + + protected void assertJustOneBundle() { + Collection<ManagedBundle> bundles = ((ManagementContextInternal)mgmt()).getOsgiManager().get().getManagedBundles().values(); + Assert.assertTrue(bundles.size()==1, "Expected one bundle after installing the same; but had: "+bundles); + } + + @Override + @Test + public void testAddSameSnapshotVersionSucceedsWhenIconIsDifferent() { + super.testAddSameSnapshotVersionSucceedsWhenIconIsDifferent(); + assertJustOneBundle(); + } + } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/480a0c76/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 5a8c31a..dacdb90 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 @@ -50,6 +50,7 @@ import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.mgmt.classloading.BrooklynClassLoadingContext; import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.api.typereg.OsgiBundleWithUrl; +import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.core.catalog.CatalogPredicates; import org.apache.brooklyn.core.catalog.internal.CatalogClasspathDo.CatalogScanningModes; import org.apache.brooklyn.core.location.BasicLocationRegistry; @@ -1191,6 +1192,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { vn = new VersionedName(vn!=null && Strings.isNonBlank(vn.getSymbolicName()) ? vn.getSymbolicName() : "brooklyn-catalog-bom-"+Identifiers.makeRandomId(8), vn!=null && vn.getVersionString()!=null ? vn.getVersionString() : getFirstAs(cm, String.class, "version").or(NO_VERSION)); } + log.debug("Wrapping supplied BOM as "+vn); Manifest mf = new Manifest(); mf.getMainAttributes().putValue(Constants.BUNDLE_SYMBOLICNAME, vn.getSymbolicName()); mf.getMainAttributes().putValue(Constants.BUNDLE_VERSION, vn.getOsgiVersionString() ); @@ -1208,6 +1210,7 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { throw Exceptions.propagate(e); } bf.delete(); + uninstallEmptyWrapperBundles(); if (result.getCode().isError()) { throw new IllegalStateException(result.getMessage()); } @@ -1477,4 +1480,23 @@ public class BasicBrooklynCatalog implements BrooklynCatalog { cache.put(itemId, spec); } } + + private Object uninstallingEmptyLock = new Object(); + public void uninstallEmptyWrapperBundles() { + log.debug("uninstalling empty wrapper bundles"); + synchronized (uninstallingEmptyLock) { + Maybe<OsgiManager> osgi = ((ManagementContextInternal)mgmt).getOsgiManager(); + if (osgi.isAbsent()) return; + for (ManagedBundle b: osgi.get().getManagedBundles().values()) { + if (isNoBundleOrSimpleWrappingBundle(b)) { + Iterable<RegisteredType> typesInBundle = osgi.get().getTypesFromBundle(b.getVersionedName()); + if (Iterables.isEmpty(typesInBundle)) { + log.debug("uninstalling empty wrapper bundle "+b); + osgi.get().uninstallUploadedBundle(b); + } + } + } + } + } + } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/480a0c76/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 50f8752..1497b42 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 @@ -23,6 +23,7 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.util.List; import java.util.jar.Attributes; import java.util.jar.Manifest; import java.util.zip.ZipEntry; @@ -34,6 +35,7 @@ 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.BasicManagedBundle; +import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.core.osgi.BundleMaker; import org.apache.brooklyn.util.core.osgi.Osgis; @@ -53,6 +55,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.base.Objects; +import com.google.common.collect.Iterables; // package-private so we can move this one if/when we move OsgiManager class OsgiArchiveInstaller { @@ -340,14 +343,14 @@ class OsgiArchiveInstaller { if (!updating) { osgiManager.managedBundlesRecord.addManagedBundle(result); result.code = OsgiBundleInstallationResult.ResultCode.INSTALLED_NEW_BUNDLE; - result.message = "Installed "+result.getMetadata().getVersionedName()+" with ID "+result.getMetadata().getId(); + result.message = "Installed Brooklyn catalog bundle "+result.getMetadata().getVersionedName()+" with ID "+result.getMetadata().getId()+" ["+result.bundle.getBundleId()+"]"; mgmt().getRebindManager().getChangeListener().onManaged(result.getMetadata()); } else { result.code = OsgiBundleInstallationResult.ResultCode.UPDATED_EXISTING_BUNDLE; - result.message = "Updated "+result.getMetadata().getVersionedName()+" as existing ID "+result.getMetadata().getId(); + result.message = "Updated Brooklyn catalog bundle "+result.getMetadata().getVersionedName()+" as existing ID "+result.getMetadata().getId()+" ["+result.bundle.getBundleId()+"]"; mgmt().getRebindManager().getChangeListener().onChanged(result.getMetadata()); } - log.info(result.message); + log.debug(result.message + " (in osgi container)"); // setting the above before the code below means if there is a problem starting or loading catalog items // a user has to remove then add again, or forcibly reinstall; @@ -364,6 +367,7 @@ class OsgiArchiveInstaller { public void run() { if (start) { try { + log.debug("Starting bundle "+result.getVersionedName()); result.bundle.start(); } catch (BundleException e) { throw Exceptions.propagate(e); @@ -375,7 +379,9 @@ class OsgiArchiveInstaller { osgiManager.uninstallCatalogItemsFromBundle( result.getVersionedName() ); // (ideally removal and addition would be atomic) } - for (CatalogItem<?,?> ci: osgiManager.loadCatalogBom(result.bundle, force)) { + List<? extends CatalogItem<?, ?>> items = osgiManager.loadCatalogBom(result.bundle, force); + log.debug("Adding items from bundle "+result.getVersionedName()+": "+items); + for (CatalogItem<?,?> ci: items) { result.catalogItemsInstalled.add(ci.getId()); } } @@ -383,8 +389,18 @@ class OsgiArchiveInstaller { }; if (deferredStart) { result.deferredStart = startRunnable; + log.debug(result.message+" (Brooklyn load deferred)"); } else { startRunnable.run(); + if (!result.catalogItemsInstalled.isEmpty()) { + // show fewer info messages, only for 'interesting' and non-deferred installations + // (rebind is deferred, as are tests, but REST is not) + MutableList<String> firstFive = MutableList.copyOf(Iterables.limit(result.catalogItemsInstalled, 5)); + log.info(result.message+", items: "+firstFive+ + (result.catalogItemsInstalled.size() > 5 ? " (and others, "+result.catalogItemsInstalled.size()+" total)" : "") ); + } else { + log.debug(result.message+" (into Brooklyn), with no catalog items"); + } } return ReferenceWithError.newInstanceWithoutError(result); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/480a0c76/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 77d2a80..a73c016 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 @@ -288,6 +288,8 @@ public class OsgiManager { public ReferenceWithError<OsgiBundleInstallationResult> install(@Nullable ManagedBundle knownBundleMetadata, @Nullable InputStream zipIn, boolean start, boolean loadCatalogBom, boolean forceUpdateOfNonSnapshots) { + log.debug("Installing bundle from stream - known details: "+knownBundleMetadata); + OsgiArchiveInstaller installer = new OsgiArchiveInstaller(this, knownBundleMetadata, zipIn); installer.setStart(start); installer.setLoadCatalogBom(loadCatalogBom); @@ -335,12 +337,14 @@ public class OsgiManager { @Beta public void 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()); } } - protected Iterable<RegisteredType> getTypesFromBundle(final VersionedName vn) { + @Beta + public Iterable<RegisteredType> getTypesFromBundle(final VersionedName vn) { final String bundleId = vn.toString(); return mgmt.getTypeRegistry().getMatching(new Predicate<RegisteredType>() { @Override http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/480a0c76/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java ---------------------------------------------------------------------- diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java b/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java index 98d6759..dc35334 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/osgi/VersionedName.java @@ -126,8 +126,13 @@ public class VersionedName { return Objects.equal(name, o.name) && Objects.equal(v, o.v); } - /** As {@link #equals(Object)} but accepting the argument as equal if versions are identical when injected to OSGi-valid versions */ + /** As {@link #equals(Object)} but accepting the argument as equal + * if versions are identical when injected to OSGi-valid versions, + * and accepting strings as the other */ public boolean equalsOsgi(Object other) { + if (other instanceof String) { + other = VersionedName.fromString((String)other); + } if (!(other instanceof VersionedName)) { return false; }
