Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id E30FE200CD9 for ; Wed, 19 Jul 2017 18:25:37 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id E1987169539; Wed, 19 Jul 2017 16:25:37 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id DA42016952F for ; Wed, 19 Jul 2017 18:25:36 +0200 (CEST) Received: (qmail 35824 invoked by uid 500); 19 Jul 2017 16:25:36 -0000 Mailing-List: contact commits-help@brooklyn.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@brooklyn.apache.org Delivered-To: mailing list commits@brooklyn.apache.org Received: (qmail 35378 invoked by uid 99); 19 Jul 2017 16:25:35 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 19 Jul 2017 16:25:35 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id D8D81F5EBD; Wed, 19 Jul 2017 16:25:31 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: geomacy@apache.org To: commits@brooklyn.apache.org Date: Wed, 19 Jul 2017 16:25:39 -0000 Message-Id: In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [09/39] brooklyn-server git commit: Delete wrapper bundles who have no types installed. archived-at: Wed, 19 Jul 2017 16:25:38 -0000 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 Authored: Wed Jun 28 10:43:17 2017 +0100 Committer: Alex Heneveld 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 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 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 osgi = ((ManagementContextInternal)mgmt).getOsgiManager(); + if (osgi.isAbsent()) return; + for (ManagedBundle b: osgi.get().getManagedBundles().values()) { + if (isNoBundleOrSimpleWrappingBundle(b)) { + Iterable 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> 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 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 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 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 getTypesFromBundle(final VersionedName vn) { + @Beta + public Iterable getTypesFromBundle(final VersionedName vn) { final String bundleId = vn.toString(); return mgmt.getTypeRegistry().getMatching(new Predicate() { @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; }