brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From geom...@apache.org
Subject [09/39] brooklyn-server git commit: Delete wrapper bundles who have no types installed.
Date Wed, 19 Jul 2017 16:25:39 GMT
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 <alex.heneveld@cloudsoftcorp.com>
Authored: Wed Jun 28 10:43:17 2017 +0100
Committer: Alex Heneveld <alex.heneveld@cloudsoftcorp.com>
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;
         }


Mime
View raw message