From bc11ab250efcf316c975edac85f4ffa1f6cb0c07 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Wed, 3 May 2017 15:21:11 +0100 Subject: [PATCH 01/13] basic support for uninstalling bundle, and sketch of reinstall doesn't yet remove catalog items, and no effort yet to test reinstall. (need to record the defining bundle on registered types.) --- .../brooklyn/api/typereg/ManagedBundle.java | 3 + .../BrooklynComponentTemplateResolver.java | 2 + ...atalogOsgiVersionMoreEntityRebindTest.java | 69 ++++++++++++++ .../CatalogOsgiVersionMoreEntityTest.java | 1 + .../brooklyn/core/mgmt/ha/OsgiManager.java | 91 +++++++++++++++++-- .../core/typereg/BasicManagedBundle.java | 11 ++- 6 files changed, 170 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/org/apache/brooklyn/api/typereg/ManagedBundle.java b/api/src/main/java/org/apache/brooklyn/api/typereg/ManagedBundle.java index 4f975b7fd6..66ca59237e 100644 --- a/api/src/main/java/org/apache/brooklyn/api/typereg/ManagedBundle.java +++ b/api/src/main/java/org/apache/brooklyn/api/typereg/ManagedBundle.java @@ -20,6 +20,7 @@ import org.apache.brooklyn.api.mgmt.rebind.Rebindable; import org.apache.brooklyn.api.objs.BrooklynObject; +import org.apache.brooklyn.util.osgi.VersionedName; /** Describes an OSGi bundle which Brooklyn manages, including persisting */ public interface ManagedBundle extends BrooklynObject, Rebindable, OsgiBundleWithUrl { @@ -29,4 +30,6 @@ public interface ManagedBundle extends BrooklynObject, Rebindable, OsgiBundleWit * This typically includes the unique {@link #getId()} of this item. */ String getOsgiUniqueUrl(); + VersionedName getVersionedName(); + } diff --git a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java index 34f67c38ad..7e9b56279b 100644 --- a/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java +++ b/camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/creation/BrooklynComponentTemplateResolver.java @@ -174,6 +174,8 @@ public EntitySpec resolveSpec(Set encounteredRegis // TODO propagate exception so we can provide better error messages msgDetails = "The reference " + type + " looks like a URL (running the CAMP Brooklyn assembly-template instantiator) but couldn't load it (missing or invalid syntax?). " + "It's also neither a catalog item nor a java type."; + } else if ("brooklyn".equals(proto)){ + msgDetails = "The reference " + type + " is not a registered catalog item nor a java type."; } else { msgDetails = "The reference " + type + " looks like a URL (running the CAMP Brooklyn assembly-template instantiator) but the protocol " + proto + " isn't white listed (" + BrooklynCampConstants.YAML_URL_PROTOCOL_WHITELIST + "). " + diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java index a3ca51bd55..16814c0c56 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java @@ -30,6 +30,7 @@ import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.camp.brooklyn.AbstractYamlRebindTest; +import org.apache.brooklyn.core.effector.Effectors; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.entity.StartableApplication; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; @@ -39,10 +40,13 @@ import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.test.support.TestResourceUnavailableException; +import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ClassLoaderUtils; import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.javalang.Reflections; import org.apache.brooklyn.util.osgi.OsgiTestResources; +import org.apache.brooklyn.util.text.Strings; +import org.osgi.framework.Bundle; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.Assert; @@ -198,4 +202,69 @@ public void testEffectorInBundleReferencedByStockCatalogItem() throws Exception Effector newEffector = newEntity.getEntityType().getEffectorByName("myEffector").get(); newEntity.invoke(newEffector, ImmutableMap.of()).get(); } + + @Test + public void testClassAccessAfterUninstall() throws Exception { + TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), BROOKLYN_TEST_OSGI_MORE_ENTITIES_0_1_0_PATH); + + // install dependency + ((ManagementContextInternal)mgmt()).getOsgiManager().get().installUploadedBundle(new BasicManagedBundle(), + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_OSGI_ENTITIES_URL), true); + + // now the v2 bundle + BasicManagedBundle mb = new BasicManagedBundle(); + Bundle b2a = ((ManagementContextInternal)mgmt()).getOsgiManager().get().installUploadedBundle(mb, + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V2_URL), true); + + Assert.assertEquals(mb.getVersionedName().toString(), BROOKLYN_TEST_MORE_ENTITIES_SYMBOLIC_NAME_FULL+":"+"0.2.0"); + + String yaml = Strings.lines("name: simple-app-yaml", + "services:", + "- type: " + BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY); + Entity app = createAndStartApplication(yaml); + Entity more = Iterables.getOnlyElement( app.getChildren() ); + + Assert.assertEquals( + more.invoke(Effectors.effector(String.class, "sayHI").buildAbstract(), MutableMap.of("name", "Bob")).get(), + "HI BOB FROM V2"); + + ((ManagementContextInternal)mgmt()).getOsgiManager().get().uninstallUploadedBundle(mb); + Assert.assertEquals(b2a.getState(), Bundle.UNINSTALLED); + + // can still call things + Assert.assertEquals( + more.invoke(Effectors.effector(String.class, "sayHI").buildAbstract(), MutableMap.of("name", "Claudia")).get(), + "HI CLAUDIA FROM V2"); + + // but still uninstalled, and attempt to create makes error + Assert.assertEquals(b2a.getState(), Bundle.UNINSTALLED); + try { + Entity app2 = createAndStartApplication(yaml); + Asserts.shouldHaveFailedPreviously("Expected deployment to fail after uninstall; instead got "+app2); + } catch (Exception e) { + // org.apache.brooklyn.util.exceptions.CompoundRuntimeException: Unable to instantiate item; 2 errors including: + // Transformer for brooklyn-camp gave an error creating this plan: Transformer for catalog gave an error creating this plan: + // Unable to instantiate org.apache.brooklyn.test.osgi.entities.more.MoreEntity; + // 2 errors including: Error in definition of org.apache.brooklyn.test.osgi.entities.more.MoreEntity:0.12.0-SNAPSHOT: + // Unable to create spec for type brooklyn:org.apache.brooklyn.test.osgi.entities.more.MoreEntity. + // The reference brooklyn:org.apache.brooklyn.test.osgi.entities.more.MoreEntity looks like a URL + // (running the CAMP Brooklyn assembly-template instantiator) but + // the protocol brooklyn isn't white listed ([classpath, http, https]). + // It's also neither a catalog item nor a java type. + // TODO different error after catalog item uninstalled + Asserts.expectedFailureContainsIgnoreCase(e, BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY, "not", "registered"); + } + + try { + StartableApplication app2 = rebind(); + Asserts.shouldHaveFailedPreviously("Expected deployment to fail rebind; instead got "+app2); + } catch (Exception e) { + Asserts.expectedFailure(e); + // TODO should fail to rebind this app + // (currently fails to load the catalog item, since it wasn't removed) + // Asserts.expectedFailureContainsIgnoreCase(e, BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY, "simple-app-yaml"); + } + } + + } diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java index 118c51830a..5c28c8baa2 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java @@ -73,6 +73,7 @@ public void testBrooklynManagedBundleInstall() throws Exception { Bundle b = ((ManagementContextInternal)mgmt()).getOsgiManager().get().installUploadedBundle(mb, new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V1_URL), true); Assert.assertEquals(mb.getSymbolicName(), b.getSymbolicName()); + Assert.assertEquals(mb.getVersion(), "0.1.0"); // bundle installed Map bundles = ((ManagementContextInternal)mgmt()).getOsgiManager().get().getManagedBundles(); 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 6f6d230cc6..a56bb52eee 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 @@ -58,6 +58,7 @@ import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.os.Os.DeletionResult; +import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.repeat.Repeater; import org.apache.brooklyn.util.stream.Streams; import org.apache.brooklyn.util.text.Strings; @@ -100,6 +101,7 @@ public class OsgiManager { private Set bundlesAtStartup; protected File osgiCacheDir; protected Map managedBundles = MutableMap.of(); + protected Map managedBundleIds = MutableMap.of(); protected AtomicInteger numberOfReusableFrameworksCreated = new AtomicInteger(); @@ -209,9 +211,17 @@ public Boolean call() { } public Map getManagedBundles() { - return ImmutableMap.copyOf(managedBundles); + synchronized (managedBundles) { + return ImmutableMap.copyOf(managedBundles); + } } - + + public String getManagedBundleId(VersionedName vn) { + synchronized (managedBundles) { + return managedBundleIds.get(vn); + } + } + public Bundle installUploadedBundle(ManagedBundle bundleMetadata, InputStream zipIn, boolean loadCatalogBom) { try { Bundle alreadyBundle = checkBundleInstalledThrowIfInconsistent(bundleMetadata, false); @@ -225,8 +235,30 @@ public Bundle installUploadedBundle(ManagedBundle bundleMetadata, InputStream zi zipIn.close(); fos.close(); - Bundle bundleInstalled = framework.getBundleContext().installBundle(bundleMetadata.getOsgiUniqueUrl(), - new FileInputStream(zipF)); + ManagedBundle existingBundleToUpdate = null; + synchronized (managedBundles) { + String id = managedBundleIds.get(bundleMetadata.getVersionedName()); + if (id!=null) { + existingBundleToUpdate = managedBundles.get(id); + } + } + + Bundle bundleInstalled; + if (existingBundleToUpdate==null) { + // install new + bundleInstalled = framework.getBundleContext().installBundle(bundleMetadata.getOsgiUniqueUrl(), + new FileInputStream(zipF)); + } else { + // update + bundleInstalled = framework.getBundleContext().getBundle(existingBundleToUpdate.getOsgiUniqueUrl()); + if (bundleInstalled==null) { + throw new IllegalStateException("Detected bundle "+existingBundleToUpdate+" should be installed but framework cannot find it"); + } + try (InputStream fin = new FileInputStream(zipF)) { + bundleInstalled.update(fin); + } + bundleMetadata = existingBundleToUpdate; + } checkCorrectlyInstalled(bundleMetadata, bundleInstalled); if (!bundleMetadata.isNameResolved()) { ((BasicManagedBundle)bundleMetadata).setSymbolicName(bundleInstalled.getSymbolicName()); @@ -236,6 +268,7 @@ public Bundle installUploadedBundle(ManagedBundle bundleMetadata, InputStream zi synchronized (managedBundles) { managedBundles.put(bundleMetadata.getId(), bundleMetadata); + managedBundleIds.put(bundleMetadata.getVersionedName(), bundleMetadata.getId()); } mgmt.getRebindManager().getChangeListener().onChanged(bundleMetadata); @@ -243,6 +276,10 @@ public Bundle installUploadedBundle(ManagedBundle bundleMetadata, InputStream zi // but may break some things running from the IDE bundleInstalled.start(); + if (existingBundleToUpdate!=null) { + // TODO remove old catalog items (see below) + // (ideally the removal and addition would be atomic) + } if (loadCatalogBom) { loadCatalogBom(bundleInstalled); } @@ -254,8 +291,50 @@ public Bundle installUploadedBundle(ManagedBundle bundleMetadata, InputStream zi } } - // TODO uninstall bundle, and call change listener onRemoved ? - // TODO on snapshot install, uninstall old equivalent snapshots (items in use might stay in use though?) + /** + * Removes this bundle from Brooklyn management, + * removes all catalog items it defined, + * and then uninstalls the bundle from OSGi. + *

+ * No checking is done whether anything is using the bundle; + * behaviour of such things is not guaranteed. They will work for many things + * but attempts to load new classes may fail. + *

+ * Callers should typically fail if anything from this bundle is in use. + */ + public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { + synchronized (managedBundles) { + ManagedBundle metadata = managedBundles.remove(bundleMetadata.getId()); + if (metadata==null) { + throw new IllegalStateException("No such bundle registered: "+bundleMetadata); + } + managedBundleIds.remove(bundleMetadata.getVersionedName()); + } + mgmt.getRebindManager().getChangeListener().onUnmanaged(bundleMetadata); + + // TODO uninstall things that come from this bundle +// mgmt.getTypeRegistry().getMatching(new Predicate() { +// @Override +// public boolean apply(RegisteredType input) { +// XXX; +// input.getLibraries(); +// return false; +// } +// }); + + Bundle bundle = framework.getBundleContext().getBundle(bundleMetadata.getOsgiUniqueUrl()); + if (bundle==null) { + throw new IllegalStateException("No such bundle installed: "+bundleMetadata); + } + try { + bundle.stop(); + bundle.uninstall(); + } catch (BundleException e) { + throw Exceptions.propagate(e); + } + } + + // TODO DO on snapshot install, uninstall old equivalent snapshots (items in use might stay in use though?) public synchronized Bundle registerBundle(CatalogBundle bundleMetadata) { try { diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java index 1ee8aa51b6..37e706fc5c 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java @@ -28,6 +28,8 @@ import org.apache.brooklyn.core.mgmt.rebind.BasicManagedBundleRebindSupport; import org.apache.brooklyn.core.objs.AbstractBrooklynObject; import org.apache.brooklyn.core.objs.BrooklynObjectInternal; +import org.apache.brooklyn.util.osgi.VersionedName; +import org.osgi.framework.Version; import com.google.common.annotations.Beta; import com.google.common.base.MoreObjects; @@ -51,7 +53,8 @@ public BasicManagedBundle(String name, String version, String url) { Preconditions.checkNotNull(name, "Either a URL or both name and version are required"); Preconditions.checkNotNull(version, "Either a URL or both name and version are required"); } - + Version.parseVersion(version); + this.symbolicName = name; this.version = version; this.url = url; @@ -80,6 +83,12 @@ public void setVersion(String version) { this.version = version; } + @Override + public VersionedName getVersionedName() { + if (symbolicName==null) return null; + return new VersionedName(symbolicName, Version.parseVersion(version)); + } + @Override public String getUrl() { return url; From 8a4886fd72709ae13fb6b4ed312df2826d8123ba Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Wed, 3 May 2017 15:44:38 +0100 Subject: [PATCH 02/13] record the containing bundle for any catalog item / registered type --- .../brooklyn/api/catalog/BrooklynCatalog.java | 15 ++++++++++++--- .../brooklyn/api/catalog/CatalogItem.java | 2 ++ .../brooklyn/api/typereg/RegisteredType.java | 1 + .../CatalogOsgiVersionMoreEntityTest.java | 1 + .../catalog/internal/BasicBrooklynCatalog.java | 15 ++++++++++++++- .../catalog/internal/CatalogBundleLoader.java | 7 ++++++- .../core/catalog/internal/CatalogItemDo.java | 5 +++++ .../internal/CatalogItemDtoAbstract.java | 17 +++++++++++++++-- .../brooklyn/core/mgmt/ha/OsgiManager.java | 6 ++++++ .../core/typereg/BasicRegisteredType.java | 6 ++++++ .../brooklyn/core/typereg/RegisteredTypes.java | 10 ++++++++++ 11 files changed, 78 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java b/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java index 3bda040991..75fe634376 100644 --- a/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java +++ b/api/src/main/java/org/apache/brooklyn/api/catalog/BrooklynCatalog.java @@ -21,7 +21,11 @@ import java.util.Collection; import java.util.NoSuchElementException; +import javax.annotation.Nullable; + import org.apache.brooklyn.api.internal.AbstractBrooklynObjectSpec; +import org.apache.brooklyn.api.typereg.ManagedBundle; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; @@ -111,12 +115,17 @@ public interface BrooklynCatalog { CatalogItem addItem(String yaml, boolean forceUpdate); /** - * Adds items (represented in yaml) to the catalog. - * Fails if the same version exists in catalog. + * As {@link #addItemsFromBundle(String, ManagedBundle)} with a null bundle. + */ + Iterable> addItems(String yaml); + + /** + * Adds items (represented in yaml) to the catalog coming from the indicated managed bundle. + * Fails if the same version exists in catalog (unless snapshot). * * @throws IllegalArgumentException if the yaml was invalid */ - Iterable> addItems(String yaml); + Iterable> addItems(String yaml, @Nullable ManagedBundle definingBundle); /** * Adds items (represented in yaml) to the catalog. diff --git a/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java b/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java index 6f27af7ad3..342cd2a8cf 100644 --- a/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java +++ b/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java @@ -123,6 +123,8 @@ public static interface CatalogItemLibraries { @Nullable public String getIconUrl(); public String getSymbolicName(); + + public String getContainingBundle(); public String getVersion(); diff --git a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java index 1eb7391f55..fad3d3565d 100644 --- a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java +++ b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java @@ -37,6 +37,7 @@ public interface RegisteredType extends Identifiable { String getSymbolicName(); String getVersion(); + String getContainingBundle(); Collection getLibraries(); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java index 5c28c8baa2..cb38df595e 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java @@ -83,6 +83,7 @@ public void testBrooklynManagedBundleInstall() throws Exception { // types installed RegisteredType t = mgmt().getTypeRegistry().get(BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY); Assert.assertNotNull(t); + Assert.assertEquals(t.getContainingBundle(), b.getSymbolicName()+":"+b.getVersion()); // can deploy createAndStartApplication("services: [ { type: "+BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY+" } ]"); 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 2634ecf689..9769f7e2aa 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 @@ -39,6 +39,7 @@ import org.apache.brooklyn.api.location.LocationSpec; 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.core.catalog.CatalogPredicates; import org.apache.brooklyn.core.catalog.internal.CatalogClasspathDo.CatalogScanningModes; import org.apache.brooklyn.core.location.BasicLocationRegistry; @@ -980,7 +981,12 @@ public CatalogItem addItem(String yaml) { @Override public List> addItems(String yaml) { - return addItems(yaml, false); + return addItems(yaml, null); + } + + @Override + public List> addItems(String yaml, ManagedBundle bundle) { + return addItems(yaml, bundle, false); } @Override @@ -990,12 +996,19 @@ public CatalogItem addItem(String yaml, boolean forceUpdate) { @Override public List> addItems(String yaml, boolean forceUpdate) { + return addItems(yaml, null, forceUpdate); + } + + private List> addItems(String yaml, ManagedBundle bundle, boolean forceUpdate) { log.debug("Adding manual catalog item to "+mgmt+": "+yaml); checkNotNull(yaml, "yaml"); List> result = collectCatalogItems(yaml); // do this at the end for atomic updates; if there are intra-yaml references, we handle them specially for (CatalogItemDtoAbstract item: result) { + if (bundle!=null && bundle.getVersionedName()!=null) { + item.setContainingBundle(bundle.getVersionedName()); + } addItemDto(item, forceUpdate); } return result; diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java index b1f8fd3b0f..4bf88240c5 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogBundleLoader.java @@ -29,8 +29,11 @@ import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.mgmt.ManagementContext; +import org.apache.brooklyn.api.typereg.ManagedBundle; +import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.stream.Streams; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.yaml.Yamls; @@ -69,6 +72,8 @@ public CatalogBundleLoader(Predicate applicationsPermitted, ManagementCo * @throws RuntimeException if the catalog items failed to be added to the catalog */ public Iterable> scanForCatalog(Bundle bundle) { + ManagedBundle mb = ((ManagementContextInternal)managementContext).getOsgiManager().get().getManagedBundle( + new VersionedName(bundle)); Iterable> catalogItems = MutableList.of(); @@ -77,7 +82,7 @@ public CatalogBundleLoader(Predicate applicationsPermitted, ManagementCo LOG.debug("Found catalog BOM in {} {} {}", CatalogUtils.bundleIds(bundle)); String bomText = readBom(bom); String bomWithLibraryPath = addLibraryDetails(bundle, bomText); - catalogItems = this.managementContext.getCatalog().addItems(bomWithLibraryPath); + catalogItems = this.managementContext.getCatalog().addItems(bomWithLibraryPath, mb); for (CatalogItem item : catalogItems) { LOG.debug("Added to catalog: {}, {}", item.getSymbolicName(), item.getVersion()); } diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java index 90697fe6e4..cf3c2d7fbd 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java @@ -197,6 +197,11 @@ public String getIconUrl() { public String getSymbolicName() { return itemDto.getSymbolicName(); } + + @Override + public String getContainingBundle() { + return itemDto.getContainingBundle(); + } @Override public String getVersion() { diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java index 5aec768626..e072b807b0 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java @@ -20,7 +20,6 @@ import java.util.Collection; import java.util.Collections; -import java.util.List; import java.util.Map; import java.util.Set; @@ -37,6 +36,8 @@ import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.SetFromFlag; +import org.apache.brooklyn.util.osgi.VersionedName; +import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,6 +53,7 @@ public abstract class CatalogItemDtoAbstract extends AbstractBrooklynO private @SetFromFlag String symbolicName; private @SetFromFlag String version = BasicBrooklynCatalog.NO_VERSION; + private @SetFromFlag String containingBundle; private @SetFromFlag String displayName; private @SetFromFlag String description; @@ -120,6 +122,11 @@ public String getName() { public String getRegisteredTypeName() { return getSymbolicName(); } + + @Override + public String getContainingBundle() { + return containingBundle; + } @Override public String getDisplayName() { @@ -191,7 +198,7 @@ public String getPlanYaml() { @Override public int hashCode() { - return Objects.hashCode(symbolicName, planYaml, javaType, nullIfEmpty(libraries), version, getCatalogItemId(), + return Objects.hashCode(symbolicName, containingBundle, planYaml, javaType, nullIfEmpty(libraries), version, getCatalogItemId(), getCatalogItemIdSearchPath()); } @@ -202,6 +209,7 @@ public boolean equals(Object obj) { if (getClass() != obj.getClass()) return false; CatalogItemDtoAbstract other = (CatalogItemDtoAbstract) obj; if (!Objects.equal(symbolicName, other.symbolicName)) return false; + if (!Objects.equal(containingBundle, other.containingBundle)) return false; if (!Objects.equal(planYaml, other.planYaml)) return false; if (!Objects.equal(javaType, other.javaType)) return false; if (!Objects.equal(nullIfEmpty(libraries), nullIfEmpty(other.libraries))) return false; @@ -360,6 +368,10 @@ protected void setVersion(String version) { this.version = version; } + public void setContainingBundle(VersionedName versionedName) { + this.containingBundle = Strings.toString(versionedName); + } + protected void setDescription(String description) { this.description = description; } @@ -439,4 +451,5 @@ private static String stringValOrNull(Map map, String key) { Object val = map.get(key); return val != null ? String.valueOf(val) : null; } + } 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 a56bb52eee..cdb40f43f4 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 @@ -221,6 +221,12 @@ public String getManagedBundleId(VersionedName vn) { return managedBundleIds.get(vn); } } + + public ManagedBundle getManagedBundle(VersionedName vn) { + synchronized (managedBundles) { + return managedBundles.get(managedBundleIds.get(vn)); + } + } public Bundle installUploadedBundle(ManagedBundle bundleMetadata, InputStream zipIn, boolean loadCatalogBom) { try { diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java index e0e2305ff4..f345ec9625 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java @@ -39,6 +39,7 @@ public class BasicRegisteredType implements RegisteredType { final RegisteredTypeKind kind; final String symbolicName; final String version; + String containingBundle; final List bundles = MutableList.of(); String displayName; @@ -83,6 +84,11 @@ public String getVersion() { return version; } + @Override + public String getContainingBundle() { + return containingBundle; + } + @Override public Collection getLibraries() { return ImmutableSet.copyOf(bundles); diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java index b3886bd650..72feeddedd 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java @@ -33,6 +33,7 @@ import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind; +import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.api.typereg.RegisteredType.TypeImplementationPlan; import org.apache.brooklyn.api.typereg.RegisteredTypeLoadingContext; @@ -47,6 +48,7 @@ import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.guava.Maybe.Absent; import org.apache.brooklyn.util.text.NaturalOrderComparator; +import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.text.VersionComparator; import org.apache.brooklyn.util.yaml.Yamls; import org.slf4j.Logger; @@ -100,6 +102,7 @@ public static RegisteredType of(CatalogItem item) { } BasicRegisteredType type = (BasicRegisteredType) spec(item.getSymbolicName(), item.getVersion(), impl, item.getCatalogItemJavaType()); + type.containingBundle = item.getContainingBundle(); type.displayName = item.getDisplayName(); type.description = item.getDescription(); type.iconUrl = item.getIconUrl(); @@ -166,6 +169,13 @@ public static Class loadActualJavaType(String javaTypeName, ManagementContext ((BasicRegisteredType)type).getCache().put(ACTUAL_JAVA_TYPE, clazz); } + @Beta + public static RegisteredType setContainingBundle(RegisteredType type, @Nullable ManagedBundle bundle) { + ((BasicRegisteredType)type).containingBundle = + bundle==null ? null : Strings.toString(bundle.getVersionedName()); + return type; + } + @Beta public static RegisteredType addSuperType(RegisteredType type, @Nullable Class superType) { if (superType!=null) { From f76118cec3d7f8a1a2788d16dc7268843cb073b7 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Wed, 3 May 2017 17:06:53 +0100 Subject: [PATCH 03/13] uninstall removes catalog items --- .../brooklyn/api/typereg/RegisteredType.java | 1 + ...atalogOsgiVersionMoreEntityRebindTest.java | 18 ++---------- .../brooklyn/core/mgmt/ha/OsgiManager.java | 28 +++++++++++++------ 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java index fad3d3565d..8225187173 100644 --- a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java +++ b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java @@ -37,6 +37,7 @@ public interface RegisteredType extends Identifiable { String getSymbolicName(); String getVersion(); + /** Bundle in symbolicname:id format where this type is defined */ String getContainingBundle(); Collection getLibraries(); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java index 16814c0c56..e47f719188 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java @@ -242,27 +242,15 @@ public void testClassAccessAfterUninstall() throws Exception { Entity app2 = createAndStartApplication(yaml); Asserts.shouldHaveFailedPreviously("Expected deployment to fail after uninstall; instead got "+app2); } catch (Exception e) { - // org.apache.brooklyn.util.exceptions.CompoundRuntimeException: Unable to instantiate item; 2 errors including: - // Transformer for brooklyn-camp gave an error creating this plan: Transformer for catalog gave an error creating this plan: - // Unable to instantiate org.apache.brooklyn.test.osgi.entities.more.MoreEntity; - // 2 errors including: Error in definition of org.apache.brooklyn.test.osgi.entities.more.MoreEntity:0.12.0-SNAPSHOT: - // Unable to create spec for type brooklyn:org.apache.brooklyn.test.osgi.entities.more.MoreEntity. - // The reference brooklyn:org.apache.brooklyn.test.osgi.entities.more.MoreEntity looks like a URL - // (running the CAMP Brooklyn assembly-template instantiator) but - // the protocol brooklyn isn't white listed ([classpath, http, https]). - // It's also neither a catalog item nor a java type. - // TODO different error after catalog item uninstalled - Asserts.expectedFailureContainsIgnoreCase(e, BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY, "not", "registered"); + Asserts.expectedFailureContainsIgnoreCase(e, "unable to match", BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY); } try { StartableApplication app2 = rebind(); Asserts.shouldHaveFailedPreviously("Expected deployment to fail rebind; instead got "+app2); } catch (Exception e) { - Asserts.expectedFailure(e); - // TODO should fail to rebind this app - // (currently fails to load the catalog item, since it wasn't removed) - // Asserts.expectedFailureContainsIgnoreCase(e, BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY, "simple-app-yaml"); + // should fail to rebind this entity + Asserts.expectedFailureContainsIgnoreCase(e, more.getId(), "unable to load", BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY); } } 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 cdb40f43f4..17d5645719 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 @@ -38,6 +38,7 @@ import org.apache.brooklyn.api.mgmt.ManagementContext; 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.config.ConfigKey; import org.apache.brooklyn.core.BrooklynFeatureEnablement; import org.apache.brooklyn.core.BrooklynVersion; @@ -74,6 +75,7 @@ import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.base.Predicates; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; @@ -297,6 +299,8 @@ public Bundle installUploadedBundle(ManagedBundle bundleMetadata, InputStream zi } } + + /** * Removes this bundle from Brooklyn management, * removes all catalog items it defined, @@ -318,15 +322,11 @@ public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { } mgmt.getRebindManager().getChangeListener().onUnmanaged(bundleMetadata); - // TODO uninstall things that come from this bundle -// mgmt.getTypeRegistry().getMatching(new Predicate() { -// @Override -// public boolean apply(RegisteredType input) { -// XXX; -// input.getLibraries(); -// return false; -// } -// }); + // uninstall things that come from this bundle + List thingsFromHere = ImmutableList.copyOf(getTypesFromBundle( bundleMetadata.getVersionedName() )); + for (RegisteredType t: thingsFromHere) { + mgmt.getCatalog().deleteCatalogItem(t.getSymbolicName(), t.getVersion()); + } Bundle bundle = framework.getBundleContext().getBundle(bundleMetadata.getOsgiUniqueUrl()); if (bundle==null) { @@ -339,6 +339,16 @@ public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { throw Exceptions.propagate(e); } } + + protected Iterable getTypesFromBundle(final VersionedName vn) { + final String bundleId = vn.toString(); + return mgmt.getTypeRegistry().getMatching(new Predicate() { + @Override + public boolean apply(RegisteredType input) { + return bundleId.equals(input.getContainingBundle()); + } + }); + } // TODO DO on snapshot install, uninstall old equivalent snapshots (items in use might stay in use though?) From 145245479ea31c6aeffffb86533cb988b39f1820 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 4 May 2017 17:35:32 +0100 Subject: [PATCH 04/13] refactor osgi installation to facilitate upgrades and richer return semantics rebind can break however -- we need to phase the bundle installs and the bundle starts --- .../catalog/CatalogMakeOsgiBundleTest.java | 9 +- ...atalogOsgiVersionMoreEntityRebindTest.java | 80 +++- .../CatalogOsgiVersionMoreEntityTest.java | 15 +- .../apache/brooklyn/core/BrooklynVersion.java | 3 +- .../internal/BasicBrooklynCatalog.java | 10 +- .../core/catalog/internal/CatalogUtils.java | 3 +- .../core/mgmt/ha/OsgiArchiveInstaller.java | 359 ++++++++++++++++++ .../mgmt/ha/OsgiBundleInstallationResult.java | 71 ++++ .../brooklyn/core/mgmt/ha/OsgiManager.java | 129 ++----- .../core/mgmt/rebind/RebindContextImpl.java | 4 +- .../core/typereg/BasicManagedBundle.java | 5 + .../apache/brooklyn/rest/domain/ApiError.java | 21 +- .../rest/resources/CatalogResource.java | 137 ++----- .../brooklyn/util/maven/MavenArtifact.java | 3 +- .../brooklyn/util/text/VersionComparator.java | 9 +- .../util/text/VersionComparatorTest.java | 7 + 16 files changed, 636 insertions(+), 229 deletions(-) create mode 100644 core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java create mode 100644 core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiBundleInstallationResult.java diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java index 6eca121460..044a2eafa3 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java @@ -37,10 +37,11 @@ import org.apache.brooklyn.core.catalog.internal.CatalogBomScanner; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityAsserts; +import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult; import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext; +import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.sensor.Sensors; import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; -import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; @@ -172,10 +173,8 @@ public void testCatalogBomFromBundleWithManualManifest() throws Exception { private void installBundle(File jf) { try (FileInputStream fin = new FileInputStream(jf)) { - BasicManagedBundle bundleMetadata = new BasicManagedBundle(); - Bundle bundle = - ((LocalManagementContext)mgmt()).getOsgiManager().get().installUploadedBundle(bundleMetadata, fin, true); - bundlesToRemove.add(bundle); + OsgiBundleInstallationResult br = ((ManagementContextInternal)mgmt()).getOsgiManager().get().install(fin).get(); + bundlesToRemove.add(br.getBundle()); } catch (Exception e) { throw Exceptions.propagate(e); } diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java index e47f719188..a258fcd313 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java @@ -33,10 +33,10 @@ import org.apache.brooklyn.core.effector.Effectors; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.entity.StartableApplication; +import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.mgmt.osgi.OsgiVersionMoreEntityTest; import org.apache.brooklyn.core.test.entity.TestEntity; -import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.test.support.TestResourceUnavailableException; @@ -70,8 +70,8 @@ protected boolean useOsgi() { @Test public void testRebindAppIncludingBundle() throws Exception { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_COM_EXAMPLE_PATH); - ((ManagementContextInternal)mgmt()).getOsgiManager().get().installUploadedBundle(new BasicManagedBundle(), - new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V1_URL), true); + ((ManagementContextInternal)mgmt()).getOsgiManager().get().install( + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V1_URL) ); createAndStartApplication("services: [ { type: "+BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY+" } ]"); @@ -208,15 +208,14 @@ public void testClassAccessAfterUninstall() throws Exception { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), BROOKLYN_TEST_OSGI_MORE_ENTITIES_0_1_0_PATH); // install dependency - ((ManagementContextInternal)mgmt()).getOsgiManager().get().installUploadedBundle(new BasicManagedBundle(), - new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_OSGI_ENTITIES_URL), true); + ((ManagementContextInternal)mgmt()).getOsgiManager().get().install( + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_OSGI_ENTITIES_URL) ); // now the v2 bundle - BasicManagedBundle mb = new BasicManagedBundle(); - Bundle b2a = ((ManagementContextInternal)mgmt()).getOsgiManager().get().installUploadedBundle(mb, - new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V2_URL), true); + OsgiBundleInstallationResult b = ((ManagementContextInternal)mgmt()).getOsgiManager().get().install( + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V2_URL) ).get(); - Assert.assertEquals(mb.getVersionedName().toString(), BROOKLYN_TEST_MORE_ENTITIES_SYMBOLIC_NAME_FULL+":"+"0.2.0"); + Assert.assertEquals(b.getVersionedName().toString(), BROOKLYN_TEST_MORE_ENTITIES_SYMBOLIC_NAME_FULL+":"+"0.2.0"); String yaml = Strings.lines("name: simple-app-yaml", "services:", @@ -228,8 +227,8 @@ public void testClassAccessAfterUninstall() throws Exception { more.invoke(Effectors.effector(String.class, "sayHI").buildAbstract(), MutableMap.of("name", "Bob")).get(), "HI BOB FROM V2"); - ((ManagementContextInternal)mgmt()).getOsgiManager().get().uninstallUploadedBundle(mb); - Assert.assertEquals(b2a.getState(), Bundle.UNINSTALLED); + ((ManagementContextInternal)mgmt()).getOsgiManager().get().uninstallUploadedBundle(b.getMetadata()); + Assert.assertEquals(b.getBundle().getState(), Bundle.UNINSTALLED); // can still call things Assert.assertEquals( @@ -237,7 +236,7 @@ public void testClassAccessAfterUninstall() throws Exception { "HI CLAUDIA FROM V2"); // but still uninstalled, and attempt to create makes error - Assert.assertEquals(b2a.getState(), Bundle.UNINSTALLED); + Assert.assertEquals(b.getBundle().getState(), Bundle.UNINSTALLED); try { Entity app2 = createAndStartApplication(yaml); Asserts.shouldHaveFailedPreviously("Expected deployment to fail after uninstall; instead got "+app2); @@ -254,5 +253,62 @@ public void testClassAccessAfterUninstall() throws Exception { } } + @Test + public void testClassAccessAfterUpgrade() throws Exception { + TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), BROOKLYN_TEST_OSGI_MORE_ENTITIES_0_1_0_PATH); + + // install dependency + ((ManagementContextInternal)mgmt()).getOsgiManager().get().install( + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_OSGI_ENTITIES_URL) ).checkNoError(); + + // now the v2 bundle + OsgiBundleInstallationResult b2a = ((ManagementContextInternal)mgmt()).getOsgiManager().get().install( + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V2_URL) ).get(); + + Assert.assertEquals(b2a.getVersionedName().toString(), BROOKLYN_TEST_MORE_ENTITIES_SYMBOLIC_NAME_FULL+":"+"0.2.0"); + Assert.assertEquals(b2a.getCode(), OsgiBundleInstallationResult.ResultCode.INSTALLED_NEW_BUNDLE); + + String yaml = Strings.lines("name: simple-app-yaml", + "services:", + "- type: " + BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY); + Entity app = createAndStartApplication(yaml); + Entity more = Iterables.getOnlyElement( app.getChildren() ); + + Assert.assertEquals( + more.invoke(Effectors.effector(String.class, "sayHI").buildAbstract(), MutableMap.of("name", "Bob")).get(), + "HI BOB FROM V2"); + + // unforced upgrade should report already installed + Assert.assertEquals( ((ManagementContextInternal)mgmt()).getOsgiManager().get().install( + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V2_EVIL_TWIN_URL) ).get().getCode(), + OsgiBundleInstallationResult.ResultCode.IGNORING_BUNDLE_AREADY_INSTALLED); + + // force upgrade + OsgiBundleInstallationResult b2b = ((ManagementContextInternal)mgmt()).getOsgiManager().get().install(b2a.getMetadata(), + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V2_EVIL_TWIN_URL), true, true).get(); + Assert.assertEquals(b2a.getBundle(), b2b.getBundle()); + Assert.assertEquals(b2b.getCode(), OsgiBundleInstallationResult.ResultCode.UPDATED_EXISTING_BUNDLE); + + // calls to things previously instantiated get the old behaviour + Assert.assertEquals( + more.invoke(Effectors.effector(String.class, "sayHI").buildAbstract(), MutableMap.of("name", "Claudia")).get(), + "HI CLAUDIA FROM V2"); + + // but new deployment gets the new behaviour + StartableApplication app2 = (StartableApplication) createAndStartApplication(yaml); + Entity more2 = Iterables.getOnlyElement( app2.getChildren() ); + Assert.assertEquals( + more2.invoke(Effectors.effector(String.class, "sayHI").buildAbstract(), MutableMap.of("name", "Daphne")).get(), + "HO DAPHNE FROM V2 EVIL TWIN"); + app2.stop(); + + // and after rebind on the old we get new behaviour + StartableApplication app1 = rebind(); + Entity more1 = Iterables.getOnlyElement( app1.getChildren() ); + Assert.assertEquals( + more1.invoke(Effectors.effector(String.class, "sayHI").buildAbstract(), MutableMap.of("name", "Eric")).get(), + "HO ERIC FROM V2 EVIL TWIN"); + } + } diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java index cb38df595e..aebfed9e8b 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java @@ -33,10 +33,10 @@ import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest; import org.apache.brooklyn.camp.brooklyn.spi.creation.BrooklynEntityMatcher; +import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.mgmt.osgi.OsgiVersionMoreEntityTest; import org.apache.brooklyn.core.objs.BrooklynTypes; -import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.test.Asserts; @@ -44,7 +44,6 @@ import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.osgi.OsgiTestResources; import org.apache.brooklyn.util.text.Strings; -import org.osgi.framework.Bundle; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.Assert; @@ -69,21 +68,19 @@ private static String getLocalResource(String filename) { @Test public void testBrooklynManagedBundleInstall() throws Exception { - BasicManagedBundle mb = new BasicManagedBundle(); - Bundle b = ((ManagementContextInternal)mgmt()).getOsgiManager().get().installUploadedBundle(mb, - new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V1_URL), true); - Assert.assertEquals(mb.getSymbolicName(), b.getSymbolicName()); - Assert.assertEquals(mb.getVersion(), "0.1.0"); + OsgiBundleInstallationResult br = ((ManagementContextInternal)mgmt()).getOsgiManager().get().install( + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V1_URL) ).get(); + Assert.assertEquals(br.getVersionedName().toString(), BROOKLYN_TEST_MORE_ENTITIES_SYMBOLIC_NAME_FULL+":"+"0.1.0"); // bundle installed Map bundles = ((ManagementContextInternal)mgmt()).getOsgiManager().get().getManagedBundles(); Asserts.assertSize(bundles.keySet(), 1); - Assert.assertEquals(mb.getId(), Iterables.getOnlyElement( bundles.keySet() )); + Assert.assertEquals(br.getMetadata().getId(), Iterables.getOnlyElement( bundles.keySet() )); // types installed RegisteredType t = mgmt().getTypeRegistry().get(BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY); Assert.assertNotNull(t); - Assert.assertEquals(t.getContainingBundle(), b.getSymbolicName()+":"+b.getVersion()); + Assert.assertEquals(t.getContainingBundle(), br.getVersionedName().toString()); // can deploy createAndStartApplication("services: [ { type: "+BROOKLYN_TEST_MORE_ENTITIES_MORE_ENTITY+" } ]"); diff --git a/core/src/main/java/org/apache/brooklyn/core/BrooklynVersion.java b/core/src/main/java/org/apache/brooklyn/core/BrooklynVersion.java index fc9556dc57..161eff0903 100644 --- a/core/src/main/java/org/apache/brooklyn/core/BrooklynVersion.java +++ b/core/src/main/java/org/apache/brooklyn/core/BrooklynVersion.java @@ -46,6 +46,7 @@ import org.apache.brooklyn.util.osgi.OsgiUtil; import org.apache.brooklyn.util.stream.Streams; import org.apache.brooklyn.util.text.Strings; +import org.apache.brooklyn.util.text.VersionComparator; import org.osgi.framework.Bundle; import org.osgi.framework.Constants; import org.osgi.framework.FrameworkUtil; @@ -155,7 +156,7 @@ public String getVersion() { @Override public boolean isSnapshot() { - return (getVersion().indexOf("-SNAPSHOT") >= 0); + return VersionComparator.isSnapshot(getVersion()); } private void readPropertiesFromMavenResource(ClassLoader resourceLoader) { 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 9769f7e2aa..c91b7cdec1 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 @@ -425,17 +425,25 @@ public static Map getCatalogMetadata(String yaml) { return getFirstAsMap(itemDef, "brooklyn.catalog").orNull(); } + /** @deprecated since 0.12.0 - use {@link #getVersionedName(Map, boolean)} */ + @Deprecated public static VersionedName getVersionedName(Map catalogMetadata) { + return getVersionedName(catalogMetadata, true); + } + + public static VersionedName getVersionedName(Map catalogMetadata, boolean required) { String version = getFirstAs(catalogMetadata, String.class, "version").orNull(); String bundle = getFirstAs(catalogMetadata, String.class, "bundle").orNull(); if (Strings.isBlank(bundle) && Strings.isBlank(version)) { + if (!required) return null; throw new IllegalStateException("Catalog BOM must define bundle and version"); } if (Strings.isBlank(bundle)) { + if (!required) return null; throw new IllegalStateException("Catalog BOM must define bundle"); } if (Strings.isBlank(version)) { - throw new IllegalStateException("Catalog BOM must define version"); + throw new IllegalStateException("Catalog BOM must define version if bundle is defined"); } return new VersionedName(bundle, Version.valueOf(version)); } diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java index 776894f5fa..394814de1d 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java @@ -43,6 +43,7 @@ import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.mgmt.rebind.RebindManagerImpl.RebindTracker; import org.apache.brooklyn.core.objs.BrooklynObjectInternal; +import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; import org.apache.brooklyn.core.typereg.RegisteredTypes; @@ -170,7 +171,7 @@ public static void installLibraries(ManagementContext managementContext, @Nullab new Object[] {managementContext, Joiner.on(", ").join(libraries)}); Stopwatch timer = Stopwatch.createStarted(); for (CatalogBundle bundleUrl : libraries) { - osgi.get().registerBundle(bundleUrl); + osgi.get().install(BasicManagedBundle.of(bundleUrl), null, true, false).get(); } if (log.isDebugEnabled()) logDebugOrTraceIfRebinding(log, 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 new file mode 100644 index 0000000000..0e069bdda1 --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java @@ -0,0 +1,359 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.mgmt.ha; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.jar.Attributes; +import java.util.jar.Manifest; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +import org.apache.brooklyn.api.typereg.ManagedBundle; +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.core.ResourceUtils; +import org.apache.brooklyn.util.core.osgi.BundleMaker; +import org.apache.brooklyn.util.core.osgi.Osgis; +import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.exceptions.ReferenceWithError; +import org.apache.brooklyn.util.guava.Maybe; +import org.apache.brooklyn.util.os.Os; +import org.apache.brooklyn.util.osgi.VersionedName; +import org.apache.brooklyn.util.stream.Streams; +import org.apache.brooklyn.util.text.Strings; +import org.apache.brooklyn.util.text.VersionComparator; +import org.osgi.framework.Bundle; +import org.osgi.framework.Constants; + +import com.google.common.base.Objects; + +// package-private so we can move this one if/when we move OsgiManager +class OsgiArchiveInstaller { + + final private OsgiManager osgiManager; + private ManagedBundle suppliedKnownBundleMetadata; + private InputStream zipIn; + + private boolean loadCatalogBom; + private boolean force; + + private File zipFile; + private Manifest discoveredManifest; + private VersionedName discoveredBomVersionedName; + OsgiBundleInstallationResult result; + + private ManagedBundle inferredMetadata; + + OsgiArchiveInstaller(OsgiManager osgiManager, ManagedBundle knownBundleMetadata, InputStream zipIn) { + this.osgiManager = osgiManager; + this.suppliedKnownBundleMetadata = knownBundleMetadata; + this.zipIn = zipIn; + } + + public void setLoadCatalogBom(boolean loadCatalogBom) { + this.loadCatalogBom = loadCatalogBom; + } + + public void setForce(boolean force) { + this.force = force; + } + + private ManagementContextInternal mgmt() { + return (ManagementContextInternal) osgiManager.mgmt; + } + + private synchronized void init() { + if (result!=null) { + if (zipFile!=null || zipIn==null) return; + throw new IllegalStateException("This installer instance has already been used and the input stream discarded"); + } + result = new OsgiBundleInstallationResult(); + inferredMetadata = suppliedKnownBundleMetadata==null ? new BasicManagedBundle() : suppliedKnownBundleMetadata; + } + + private synchronized void makeLocalZipFileFromInputStreamOrUrl() { + if (zipIn==null) { + Maybe installedBundle = Maybe.absent(); + if (suppliedKnownBundleMetadata!=null) { + // if no input stream, look for a URL and/or a matching bundle + if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) { + installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find(); + } + if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) { + installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find(); + } + if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) { + installedBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getVersion()).find(); + } + if (suppliedKnownBundleMetadata.getUrl()!=null) { + if (installedBundle.isAbsent() || force) { + // reload + zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(suppliedKnownBundleMetadata.getUrl()); + } + } + } + + if (installedBundle.isPresent()) { + result.bundle = installedBundle.get(); + + if (zipIn==null) { + // no way to install (no url), or no need to install (not forced); just ignore it + result.metadata = osgiManager.getManagedBundle(new VersionedName(installedBundle.get())); + result.setIgnoringAlreadyInstalled(); + return; + } + } else { + result.metadata = suppliedKnownBundleMetadata; + throw new IllegalArgumentException("No input stream available and no URL could be found; nothing to install"); + } + } + + zipFile = Os.newTempFile("brooklyn-bundle-transient-"+suppliedKnownBundleMetadata, "zip"); + try { + FileOutputStream fos = new FileOutputStream(zipFile); + Streams.copy(zipIn, fos); + zipIn.close(); + fos.close(); + } catch (Exception e) { + throw Exceptions.propagate(e); + } finally { + zipIn = null; + } + } + + private void discoverManifestFromCatalogBom(boolean isCatalogBomRequired) { + discoveredManifest = new BundleMaker(mgmt()).getManifest(zipFile); + ZipFile zf = null; + try { + try { + zf = new ZipFile(zipFile); + } catch (IOException e) { + throw new IllegalArgumentException("Invalid ZIP/JAR archive: "+e); + } + ZipEntry bom = zf.getEntry("catalog.bom"); + if (bom==null) { + bom = zf.getEntry("/catalog.bom"); + } + if (bom==null) { + if (isCatalogBomRequired) { + throw new IllegalArgumentException("Archive must contain a catalog.bom file in the root"); + } else { + return; + } + } + String bomS; + try { + bomS = Streams.readFullyString(zf.getInputStream(bom)); + } catch (IOException e) { + throw new IllegalArgumentException("Error reading catalog.bom from ZIP/JAR archive: "+e); + } + discoveredBomVersionedName = BasicBrooklynCatalog.getVersionedName( BasicBrooklynCatalog.getCatalogMetadata(bomS), false ); + } finally { + Streams.closeQuietly(zf); + } + } + + private void updateManifestFromAllSourceInformation() { + if (discoveredBomVersionedName!=null) { + matchSetOrFail("catalog.bom in archive", discoveredBomVersionedName.getSymbolicName(), discoveredBomVersionedName.getVersion().toString()); + } + + boolean manifestNeedsUpdating = false; + if (discoveredManifest==null) { + discoveredManifest = new Manifest(); + manifestNeedsUpdating = true; + } + if (!matchSetOrFail("MANIFEST.MF in archive", discoveredManifest.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME), + discoveredManifest.getMainAttributes().getValue(Constants.BUNDLE_VERSION) )) { + manifestNeedsUpdating = true; + discoveredManifest.getMainAttributes().putValue(Constants.BUNDLE_SYMBOLICNAME, inferredMetadata.getSymbolicName()); + discoveredManifest.getMainAttributes().putValue(Constants.BUNDLE_VERSION, inferredMetadata.getVersion()); + } + if (Strings.isBlank(inferredMetadata.getSymbolicName())) { + throw new IllegalArgumentException("Missing bundle symbolic name in BOM or MANIFEST"); + } + if (Strings.isBlank(inferredMetadata.getVersion())) { + throw new IllegalArgumentException("Missing bundle version in BOM or MANIFEST"); + } + if (discoveredManifest.getMainAttributes().getValue(Attributes.Name.MANIFEST_VERSION)==null) { + discoveredManifest.getMainAttributes().putValue(Attributes.Name.MANIFEST_VERSION.toString(), "1.0"); + manifestNeedsUpdating = true; + } + if (manifestNeedsUpdating) { + File zf2 = new BundleMaker(mgmt()).copyAddingManifest(zipFile, discoveredManifest); + zipFile.delete(); + zipFile = zf2; + } + } + + private synchronized void close() { + if (zipFile!=null) { + zipFile.delete(); + zipFile = null; + } + } + + /** + * Installs a bundle, taking from ZIP input stream if supplied, falling back to URL in the {@link ManagedBundle} metadata supplied. + * It will take metadata from any of: a MANIFEST.MF in the ZIP; a catalog.bom in the ZIP; the {@link ManagedBundle} metadata supplied. + * If metadata is supplied in multiple such places, it must match. + * Appropriate metadata will be added to the ZIP and installation attempted. + *

+ * If a matching bundle is already installed, the installation will stop with a {@link ResultCode#IGNORING_BUNDLE_AREADY_INSTALLED} + * unless the bundle is a snapshot or "force" is specified. + * In the latter two cases, if there is an installed matching bundle, that bundle will be updated with the input stream here, + * with any catalog items from the old bundle removed and those from this bundle installed. + *

+ * Default behaviour is {@link #setLoadCatalogBom(boolean)} true and {@link #setForce(boolean)} false. + *

+ * The return value is extensive but should be self-evident, and will include a list of any registered types (catalog items) installed. + */ + public ReferenceWithError install() { + boolean startedInstallation = false; + + try { + init(); + makeLocalZipFileFromInputStreamOrUrl(); + if (result.code!=null) return ReferenceWithError.newInstanceWithoutError(result); + discoverManifestFromCatalogBom(false); + if (result.code!=null) return ReferenceWithError.newInstanceWithoutError(result); + updateManifestFromAllSourceInformation(); + if (result.code!=null) return ReferenceWithError.newInstanceWithoutError(result); + assert inferredMetadata.isNameResolved() : "Should have resolved "+inferredMetadata; + + Boolean updating = null; + result.metadata = osgiManager.getManagedBundle(inferredMetadata.getVersionedName()); + if (result.getMetadata()!=null) { + // already have a managed bundle + if (canUpdate()) { + result.bundle = osgiManager.framework.getBundleContext().getBundle(result.getMetadata().getOsgiUniqueUrl()); + if (result.getBundle()==null) { + throw new IllegalStateException("Detected already managing bundle "+result.getMetadata().getVersionedName()+" but framework cannot find it"); + } + updating = true; + } else { + result.setIgnoringAlreadyInstalled(); + return ReferenceWithError.newInstanceWithoutError(result); + } + } else { + result.metadata = inferredMetadata; + // no such managed bundle + Maybe b = Osgis.bundleFinder(osgiManager.framework).symbolicName(result.getMetadata().getSymbolicName()).version(result.getMetadata().getVersion()).find(); + if (b.isPresent()) { + // if it's non-brooklyn installed then fail + // (e.g. someone trying to install brooklyn or guice through this mechanism!) + result.bundle = b.get(); + result.code = OsgiBundleInstallationResult.ResultCode.ERROR_INSTALLING_BUNDLE; + throw new IllegalStateException("Bundle "+result.getMetadata().getVersionedName()+" already installed in framework but not managed by Brooklyn; cannot install or update through Brooklyn"); + } + // normal install + updating = false; + } + + startedInstallation = true; + try (InputStream fin = new FileInputStream(zipFile)) { + if (!updating) { + // install new + assert result.getBundle()==null; + result.bundle = osgiManager.framework.getBundleContext().installBundle(result.getMetadata().getOsgiUniqueUrl(), fin); + } else { + result.bundle.update(fin); + } + } + + osgiManager.checkCorrectlyInstalled(result.getMetadata(), result.bundle); + ((BasicManagedBundle)result.getMetadata()).setTempLocalFileWhenJustUploaded(zipFile); + zipFile = null; // don't close/delete it here, we'll use it for uploading, then it will delete it + + if (!updating) { + synchronized (osgiManager.managedBundles) { + osgiManager.managedBundles.put(result.getMetadata().getId(), result.getMetadata()); + osgiManager.managedBundleIds.put(result.getMetadata().getVersionedName(), result.getMetadata().getId()); + } + result.code = OsgiBundleInstallationResult.ResultCode.INSTALLED_NEW_BUNDLE; + result.message = "Installed "+result.getMetadata().getVersionedName()+" with ID "+result.getMetadata().getId(); + 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(); + mgmt().getRebindManager().getChangeListener().onChanged(result.getMetadata()); + } + // 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; + // that seems fine and probably better than allowing bundles to start and catalog items to be installed + // when brooklyn isn't aware it is supposed to be managing it + + // starting here flags wiring issues earlier + // but may break some things running from the IDE + result.bundle.start(); + + if (updating!=null) { + osgiManager.uninstallCatalogItemsFromBundle( result.getVersionedName() ); + // (ideally removal and addition would be atomic) + } + if (loadCatalogBom) { + osgiManager.loadCatalogBom(result.bundle); + } + + return ReferenceWithError.newInstanceWithoutError(result); + + } catch (Exception e) { + Exceptions.propagateIfFatal(e); + result.code = startedInstallation ? OsgiBundleInstallationResult.ResultCode.ERROR_INSTALLING_BUNDLE : OsgiBundleInstallationResult.ResultCode.ERROR_PREPARING_BUNDLE; + result.message = "Bundle "+inferredMetadata+" failed "+ + (startedInstallation ? "installation" : "preparation") + ": " + Exceptions.collapseText(e); + return ReferenceWithError.newInstanceThrowingError(result, new IllegalStateException(result.message, e)); + } finally { + close(); + } + } + + private boolean canUpdate() { + return force || VersionComparator.isSnapshot(inferredMetadata.getVersion()); + } + + /** true if the supplied name and version are complete; updates if the known data is incomplete; + * throws if there is a mismatch; false if the supplied data is incomplete */ + private boolean matchSetOrFail(String source, String name, String version) { + boolean suppliedIsComplete = true; + if (Strings.isBlank(name)) { + suppliedIsComplete = false; + } else if (Strings.isBlank(inferredMetadata.getSymbolicName())) { + ((BasicManagedBundle)inferredMetadata).setSymbolicName(name); + } else if (!Objects.equal(inferredMetadata.getSymbolicName(), name)){ + throw new IllegalArgumentException("Symbolic name mismatch '"+name+"' from "+source+" (expected '"+inferredMetadata.getSymbolicName()+"')"); + } + + if (Strings.isBlank(version)) { + suppliedIsComplete = false; + } else if (Strings.isBlank(inferredMetadata.getVersion())) { + ((BasicManagedBundle)inferredMetadata).setVersion(version); + } else if (!Objects.equal(inferredMetadata.getVersion(), version)){ + throw new IllegalArgumentException("Bundle version mismatch '"+version+"' from "+source+" (expected '"+inferredMetadata.getVersion()+"')"); + } + + return suppliedIsComplete; + } +} diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiBundleInstallationResult.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiBundleInstallationResult.java new file mode 100644 index 0000000000..50ca081a97 --- /dev/null +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiBundleInstallationResult.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.brooklyn.core.mgmt.ha; + +import java.util.List; + +import org.apache.brooklyn.api.typereg.ManagedBundle; +import org.apache.brooklyn.util.collections.MutableList; +import org.apache.brooklyn.util.osgi.VersionedName; +import org.osgi.framework.Bundle; + +import com.google.common.annotations.Beta; +import com.google.common.collect.ImmutableList; + +@Beta +public class OsgiBundleInstallationResult { + String message; + ManagedBundle metadata; + Bundle bundle; + ResultCode code; + + public enum ResultCode { + INSTALLED_NEW_BUNDLE, + UPDATED_EXISTING_BUNDLE, + IGNORING_BUNDLE_AREADY_INSTALLED, + ERROR_PREPARING_BUNDLE, + ERROR_INSTALLING_BUNDLE + } + final List catalogItemsInstalled = MutableList.of(); + + public String getMessage() { + return message; + } + public Bundle getBundle() { + return bundle; + } + public ManagedBundle getMetadata() { + return metadata; + } + public ResultCode getCode() { + return code; + } + public List getCatalogItemsInstalled() { + return ImmutableList.copyOf(catalogItemsInstalled); + } + public VersionedName getVersionedName() { + if (getMetadata()==null) return null; + return getMetadata().getVersionedName(); + } + + void setIgnoringAlreadyInstalled() { + code = OsgiBundleInstallationResult.ResultCode.IGNORING_BUNDLE_AREADY_INSTALLED; + message = "Bundle "+getMetadata().getVersionedName()+" already installed as "+getMetadata().getId(); + } +} \ No newline at end of file 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 17d5645719..1685541b98 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 @@ -19,8 +19,6 @@ package org.apache.brooklyn.core.mgmt.ha; import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.InputStream; import java.net.URL; import java.util.Arrays; @@ -33,6 +31,8 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import javax.annotation.Nullable; + import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.catalog.CatalogItem.CatalogBundle; import org.apache.brooklyn.api.mgmt.ManagementContext; @@ -47,7 +47,6 @@ import org.apache.brooklyn.core.mgmt.persist.OsgiClassPrefixer; import org.apache.brooklyn.core.server.BrooklynServerConfig; import org.apache.brooklyn.core.server.BrooklynServerPaths; -import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; @@ -55,13 +54,13 @@ import org.apache.brooklyn.util.core.osgi.Osgis.BundleFinder; import org.apache.brooklyn.util.core.osgi.SystemFrameworkLoader; import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.exceptions.ReferenceWithError; import org.apache.brooklyn.util.exceptions.UserFacingException; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.os.Os.DeletionResult; import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.repeat.Repeater; -import org.apache.brooklyn.util.stream.Streams; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Duration; import org.osgi.framework.Bundle; @@ -96,18 +95,18 @@ public class OsgiManager { /* see `Osgis` class for info on starting framework etc */ - protected final ManagementContext mgmt; - protected final OsgiClassPrefixer osgiClassPrefixer; - protected Framework framework; - protected boolean reuseFramework; + final ManagementContext mgmt; + final OsgiClassPrefixer osgiClassPrefixer; + Framework framework; + + private boolean reuseFramework; private Set bundlesAtStartup; - protected File osgiCacheDir; - protected Map managedBundles = MutableMap.of(); - protected Map managedBundleIds = MutableMap.of(); - protected AtomicInteger numberOfReusableFrameworksCreated = new AtomicInteger(); - + private File osgiCacheDir; + Map managedBundles = MutableMap.of(); + Map managedBundleIds = MutableMap.of(); - protected static final List OSGI_FRAMEWORK_CONTAINERS_FOR_REUSE = MutableList.of(); + private static AtomicInteger numberOfReusableFrameworksCreated = new AtomicInteger(); + private static final List OSGI_FRAMEWORK_CONTAINERS_FOR_REUSE = MutableList.of(); public OsgiManager(ManagementContext mgmt) { this.mgmt = mgmt; @@ -230,76 +229,21 @@ public ManagedBundle getManagedBundle(VersionedName vn) { } } - public Bundle installUploadedBundle(ManagedBundle bundleMetadata, InputStream zipIn, boolean loadCatalogBom) { - try { - Bundle alreadyBundle = checkBundleInstalledThrowIfInconsistent(bundleMetadata, false); - if (alreadyBundle!=null) { - return alreadyBundle; - } - - File zipF = Os.newTempFile("brooklyn-bundle-transient-"+bundleMetadata, "zip"); - FileOutputStream fos = new FileOutputStream(zipF); - Streams.copy(zipIn, fos); - zipIn.close(); - fos.close(); - - ManagedBundle existingBundleToUpdate = null; - synchronized (managedBundles) { - String id = managedBundleIds.get(bundleMetadata.getVersionedName()); - if (id!=null) { - existingBundleToUpdate = managedBundles.get(id); - } - } - - Bundle bundleInstalled; - if (existingBundleToUpdate==null) { - // install new - bundleInstalled = framework.getBundleContext().installBundle(bundleMetadata.getOsgiUniqueUrl(), - new FileInputStream(zipF)); - } else { - // update - bundleInstalled = framework.getBundleContext().getBundle(existingBundleToUpdate.getOsgiUniqueUrl()); - if (bundleInstalled==null) { - throw new IllegalStateException("Detected bundle "+existingBundleToUpdate+" should be installed but framework cannot find it"); - } - try (InputStream fin = new FileInputStream(zipF)) { - bundleInstalled.update(fin); - } - bundleMetadata = existingBundleToUpdate; - } - checkCorrectlyInstalled(bundleMetadata, bundleInstalled); - if (!bundleMetadata.isNameResolved()) { - ((BasicManagedBundle)bundleMetadata).setSymbolicName(bundleInstalled.getSymbolicName()); - ((BasicManagedBundle)bundleMetadata).setVersion(bundleInstalled.getVersion().toString()); - } - ((BasicManagedBundle)bundleMetadata).setTempLocalFileWhenJustUploaded(zipF); - - synchronized (managedBundles) { - managedBundles.put(bundleMetadata.getId(), bundleMetadata); - managedBundleIds.put(bundleMetadata.getVersionedName(), bundleMetadata.getId()); - } - mgmt.getRebindManager().getChangeListener().onChanged(bundleMetadata); - - // starting here flags wiring issues earlier - // but may break some things running from the IDE - bundleInstalled.start(); - - if (existingBundleToUpdate!=null) { - // TODO remove old catalog items (see below) - // (ideally the removal and addition would be atomic) - } - if (loadCatalogBom) { - loadCatalogBom(bundleInstalled); - } - - return bundleInstalled; - } catch (Exception e) { - Exceptions.propagateIfFatal(e); - throw new IllegalStateException("Bundle "+bundleMetadata+" failed to install: " + Exceptions.collapseText(e), e); - } + /** See {@link OsgiArchiveInstaller#install()}, using default values */ + public ReferenceWithError install(InputStream zipIn) { + return install(null, zipIn, true, false); } - + /** See {@link OsgiArchiveInstaller#install()} */ + public ReferenceWithError install(@Nullable ManagedBundle knownBundleMetadata, @Nullable InputStream zipIn, + boolean loadCatalogBom, boolean forceUpdateOfNonSnapshots) { + + OsgiArchiveInstaller installer = new OsgiArchiveInstaller(this, knownBundleMetadata, zipIn); + installer.setLoadCatalogBom(loadCatalogBom); + installer.setForce(forceUpdateOfNonSnapshots); + + return installer.install(); + } /** * Removes this bundle from Brooklyn management, @@ -321,12 +265,8 @@ public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { managedBundleIds.remove(bundleMetadata.getVersionedName()); } mgmt.getRebindManager().getChangeListener().onUnmanaged(bundleMetadata); - - // uninstall things that come from this bundle - List thingsFromHere = ImmutableList.copyOf(getTypesFromBundle( bundleMetadata.getVersionedName() )); - for (RegisteredType t: thingsFromHere) { - mgmt.getCatalog().deleteCatalogItem(t.getSymbolicName(), t.getVersion()); - } + + uninstallCatalogItemsFromBundle( bundleMetadata.getVersionedName() ); Bundle bundle = framework.getBundleContext().getBundle(bundleMetadata.getOsgiUniqueUrl()); if (bundle==null) { @@ -340,6 +280,13 @@ public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { } } + void uninstallCatalogItemsFromBundle(VersionedName bundle) { + List thingsFromHere = ImmutableList.copyOf(getTypesFromBundle( bundle )); + for (RegisteredType t: thingsFromHere) { + mgmt.getCatalog().deleteCatalogItem(t.getSymbolicName(), t.getVersion()); + } + } + protected Iterable getTypesFromBundle(final VersionedName vn) { final String bundleId = vn.toString(); return mgmt.getTypeRegistry().getMatching(new Predicate() { @@ -350,8 +297,8 @@ public boolean apply(RegisteredType input) { }); } - // TODO DO on snapshot install, uninstall old equivalent snapshots (items in use might stay in use though?) - + /** @deprecated since 0.12.0 use {@link #install(ManagedBundle, InputStream, boolean, boolean)} */ + @Deprecated public synchronized Bundle registerBundle(CatalogBundle bundleMetadata) { try { Bundle alreadyBundle = checkBundleInstalledThrowIfInconsistent(bundleMetadata, true); @@ -402,7 +349,7 @@ public List> loadCatalogBom(Bundle bundle) { return catalogItems; } - private void checkCorrectlyInstalled(OsgiBundleWithUrl bundle, Bundle b) { + void checkCorrectlyInstalled(OsgiBundleWithUrl bundle, Bundle b) { String nv = b.getSymbolicName()+":"+b.getVersion().toString(); if (!isBundleNameEqualOrAbsent(bundle, b)) { diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindContextImpl.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindContextImpl.java index 1e9cbbacc2..13c85f6aa7 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindContextImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindContextImpl.java @@ -36,7 +36,7 @@ import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.api.sensor.Feed; import org.apache.brooklyn.api.typereg.ManagedBundle; -import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext; +import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.util.collections.MutableMap; import com.google.common.collect.Maps; @@ -91,7 +91,7 @@ public void registerCatalogItem(String id, CatalogItem catalogItem) { // we don't track register/unregister of bundles; it isn't needed as it happens so early public void installBundle(ManagedBundle bundle, InputStream zipInput) { - ((LocalManagementContext)mgmt).getOsgiManager().get().installUploadedBundle(bundle, zipInput, true); + ((ManagementContextInternal)mgmt).getOsgiManager().get().install(bundle, zipInput, true, false).checkNoError(); } public void unregisterPolicy(Policy policy) { diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java index 37e706fc5c..4799c06b7a 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java @@ -21,6 +21,7 @@ import java.io.File; import java.util.Map; +import org.apache.brooklyn.api.catalog.CatalogItem.CatalogBundle; import org.apache.brooklyn.api.mgmt.rebind.RebindSupport; import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.api.typereg.OsgiBundleWithUrl; @@ -181,4 +182,8 @@ protected BrooklynObjectInternal configure(Map flags) { throw new UnsupportedOperationException(); } + public static ManagedBundle of(CatalogBundle bundleUrl) { + return new BasicManagedBundle(bundleUrl.getSymbolicName(), bundleUrl.getVersion(), bundleUrl.getUrl()); + } + } diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java index bec524bb1c..a2a183414d 100644 --- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java +++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/domain/ApiError.java @@ -72,6 +72,7 @@ public static Builder builderFromThrowable(Throwable t) { public static class Builder { private String message; private String details; + private Object data; private Integer errorCode; public Builder message(String message) { @@ -80,7 +81,12 @@ public Builder message(String message) { } public Builder details(String details) { - this.details = checkNotNull(details, "details"); + this.details = details; + return this; + } + + public Builder data(Object data) { + this.data = data; return this; } @@ -111,7 +117,7 @@ public Builder prefixMessage(String prefix, String separatorIfMessageNotBlank) { } public ApiError build() { - return new ApiError(message, details, errorCode); + return new ApiError(message, details, data, errorCode); } /** @deprecated since 0.7.0; use {@link #copy(ApiError)} */ @@ -136,18 +142,23 @@ public String getMessage() { @JsonInclude(JsonInclude.Include.NON_EMPTY) private final String details; + + @JsonInclude(JsonInclude.Include.NON_NULL) + private final Object data; @JsonInclude(JsonInclude.Include.NON_NULL) private final Integer error; public ApiError(String message) { this(message, null); } - public ApiError(String message, String details) { this(message, details, null); } + public ApiError(String message, String details) { this(message, details, null, null); } public ApiError( @JsonProperty("message") String message, @JsonProperty("details") String details, + @JsonProperty("data") Object data, @JsonProperty("error") Integer error) { this.message = checkNotNull(message, "message"); this.details = details != null ? details : ""; + this.data = data; this.error = error; } @@ -159,6 +170,10 @@ public String getDetails() { return details; } + public Object getData() { + return data; + } + public Integer getError() { return error; } diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index d0f7270e0d..7359e4d397 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -19,9 +19,6 @@ package org.apache.brooklyn.rest.resources; import java.io.ByteArrayInputStream; -import java.io.File; -import java.io.FileInputStream; -import java.io.IOException; import java.io.InputStreamReader; import java.net.URI; import java.util.ArrayList; @@ -29,8 +26,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.jar.Attributes; -import java.util.jar.Manifest; import javax.annotation.Nullable; import javax.ws.rs.core.MediaType; @@ -44,8 +39,10 @@ import org.apache.brooklyn.api.entity.EntitySpec; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationSpec; +import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.api.policy.Policy; import org.apache.brooklyn.api.policy.PolicySpec; +import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.core.catalog.CatalogPredicates; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; @@ -54,8 +51,8 @@ import org.apache.brooklyn.core.catalog.internal.CatalogUtils; import org.apache.brooklyn.core.mgmt.entitlement.Entitlements; import org.apache.brooklyn.core.mgmt.entitlement.Entitlements.StringAndArgument; +import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; -import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; import org.apache.brooklyn.rest.api.CatalogApi; import org.apache.brooklyn.rest.domain.ApiError; @@ -65,24 +62,16 @@ import org.apache.brooklyn.rest.domain.CatalogPolicySummary; import org.apache.brooklyn.rest.filter.HaHotStateRequired; import org.apache.brooklyn.rest.transform.CatalogTransformer; -import org.apache.brooklyn.rest.util.DefaultExceptionMapper; import org.apache.brooklyn.rest.util.WebResourceUtils; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.ResourceUtils; -import org.apache.brooklyn.util.core.osgi.BundleMaker; import org.apache.brooklyn.util.exceptions.Exceptions; -import org.apache.brooklyn.util.os.Os; -import org.apache.brooklyn.util.osgi.VersionedName; -import org.apache.brooklyn.util.stream.Streams; +import org.apache.brooklyn.util.exceptions.ReferenceWithError; import org.apache.brooklyn.util.text.StringPredicates; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.yaml.Yamls; -import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; -import org.apache.commons.compress.archivers.zip.ZipFile; -import org.osgi.framework.Bundle; -import org.osgi.framework.Constants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -161,6 +150,29 @@ public Response createFromYaml(String yaml) { } } + public static class BundleInstallationRestResult { + String message; + ManagedBundle metadata; + + enum ResultCode { + INSTALLED_NEW_BUNDLE, + UPDATED_EXISTING_BUNDLE, + IGNORING_BUNDLE_AREADY_INSTALLED, + ERROR_PREPARING_BUNDLE, + ERROR_INSTALLING_BUNDLE + } + Map catalogItemsInstalled; + + public String getMessage() { + return message; + } + + public static BundleInstallationRestResult of(OsgiBundleInstallationResult result, ManagementContext mgmt) { + // TODO + return null; + } + } + @Override @Beta public Response createFromArchive(byte[] zipInput) { @@ -169,92 +181,15 @@ public Response createFromArchive(byte[] zipInput) { Entitlements.getEntitlementContext().user()); } - BundleMaker bm = new BundleMaker(mgmtInternal()); - File f=null, f2=null; - try { - f = Os.newTempFile("brooklyn-posted-archive", "zip"); - try { - Files.write(zipInput, f); - } catch (IOException e) { - Exceptions.propagate(e); - } - - ZipFile zf; - try { - zf = new ZipFile(f); - } catch (IOException e) { - throw new IllegalArgumentException("Invalid ZIP/JAR archive: "+e); - } - ZipArchiveEntry bom = zf.getEntry("catalog.bom"); - if (bom==null) { - bom = zf.getEntry("/catalog.bom"); - } - if (bom==null) { - throw new IllegalArgumentException("Archive must contain a catalog.bom file in the root"); - } - String bomS; - try { - bomS = Streams.readFullyString(zf.getInputStream(bom)); - } catch (IOException e) { - throw new IllegalArgumentException("Error reading catalog.bom from ZIP/JAR archive: "+e); - } - - try { - zf.close(); - } catch (IOException e) { - log.debug("Swallowed exception closing zipfile. Full error logged at trace: {}", e.getMessage()); - log.trace("Exception closing zipfile", e); - } - - VersionedName vn = BasicBrooklynCatalog.getVersionedName( BasicBrooklynCatalog.getCatalogMetadata(bomS) ); - - Manifest mf = bm.getManifest(f); - if (mf==null) { - mf = new Manifest(); - } - String bundleNameInMF = mf.getMainAttributes().getValue(Constants.BUNDLE_SYMBOLICNAME); - if (Strings.isNonBlank(bundleNameInMF)) { - if (!bundleNameInMF.equals(vn.getSymbolicName())) { - throw new IllegalArgumentException("JAR MANIFEST symbolic-name '"+bundleNameInMF+"' does not match '"+vn.getSymbolicName()+"' defined in BOM"); - } - } else { - bundleNameInMF = vn.getSymbolicName(); - mf.getMainAttributes().putValue(Constants.BUNDLE_SYMBOLICNAME, bundleNameInMF); - } - - String bundleVersionInMF = mf.getMainAttributes().getValue(Constants.BUNDLE_VERSION); - if (Strings.isNonBlank(bundleVersionInMF)) { - if (!bundleVersionInMF.equals(vn.getVersion().toString())) { - throw new IllegalArgumentException("JAR MANIFEST version '"+bundleVersionInMF+"' does not match '"+vn.getVersion()+"' defined in BOM"); - } - } else { - bundleVersionInMF = vn.getVersion().toString(); - mf.getMainAttributes().putValue(Constants.BUNDLE_VERSION, bundleVersionInMF); - } - if (mf.getMainAttributes().getValue(Attributes.Name.MANIFEST_VERSION)==null) { - mf.getMainAttributes().putValue(Attributes.Name.MANIFEST_VERSION.toString(), "1.0"); - } - - f2 = bm.copyAddingManifest(f, mf); - - BasicManagedBundle bundleMetadata = new BasicManagedBundle(bundleNameInMF, bundleVersionInMF, null); - Bundle bundle; - try (FileInputStream f2in = new FileInputStream(f2)) { - bundle = ((ManagementContextInternal)mgmt()).getOsgiManager().get().installUploadedBundle(bundleMetadata, f2in, false); - } catch (Exception e) { - throw Exceptions.propagate(e); - } - - Iterable> catalogItems = MutableList.copyOf( - ((ManagementContextInternal)mgmt()).getOsgiManager().get().loadCatalogBom(bundle) ); - - return buildCreateResponse(catalogItems); - } catch (RuntimeException ex) { - throw WebResourceUtils.badRequest(ex); - } finally { - if (f!=null) f.delete(); - if (f2!=null) f2.delete(); + ReferenceWithError result = ((ManagementContextInternal)mgmt()).getOsgiManager().get() + .install(new ByteArrayInputStream(zipInput)); + + if (result.hasError()) { + return ApiError.builder().errorCode(Status.BAD_REQUEST).message(result.getWithoutError().getMessage()) + .data(result).build().asJsonResponse(); } + + return Response.status(Status.CREATED).entity( BundleInstallationRestResult.of(result.get(), mgmt()) ).build(); } private Response buildCreateResponse(Iterable> catalogItems) { @@ -575,4 +510,4 @@ private static List castList(List list, Class elementType) return result; } } - \ No newline at end of file + diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/maven/MavenArtifact.java b/utils/common/src/main/java/org/apache/brooklyn/util/maven/MavenArtifact.java index b9de2f139b..248b72e25f 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/maven/MavenArtifact.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/maven/MavenArtifact.java @@ -26,6 +26,7 @@ import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.javalang.JavaClassNames; import org.apache.brooklyn.util.text.Strings; +import org.apache.brooklyn.util.text.VersionComparator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -100,7 +101,7 @@ public String getPackaging() { } public boolean isSnapshot() { - return getVersion().toUpperCase().contains("SNAPSHOT"); + return VersionComparator.isSnapshot(getVersion()); } /** @see #customFileNameAfterArtifactMarker */ diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java b/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java index 94553b0dd5..071f591363 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/text/VersionComparator.java @@ -52,14 +52,19 @@ public static VersionComparator getInstance() { return INSTANCE; } + public static boolean isSnapshot(String version) { + if (version==null) return false; + return version.toUpperCase().contains(SNAPSHOT); + } + @Override public int compare(String v1, String v2) { if (v1==null && v2==null) return 0; if (v1==null) return -1; if (v2==null) return 1; - boolean isV1Snapshot = v1.toUpperCase().contains(SNAPSHOT); - boolean isV2Snapshot = v2.toUpperCase().contains(SNAPSHOT); + boolean isV1Snapshot = isSnapshot(v1); + boolean isV2Snapshot = isSnapshot(v2); if (isV1Snapshot == isV2Snapshot) { // if snapshot status is the same, look at dot-split parts first return compareDotSplitParts(splitOnDot(v1), splitOnDot(v2)); diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/text/VersionComparatorTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/text/VersionComparatorTest.java index 3a4a71c35c..6c65a8a92e 100644 --- a/utils/common/src/test/java/org/apache/brooklyn/util/text/VersionComparatorTest.java +++ b/utils/common/src/test/java/org/apache/brooklyn/util/text/VersionComparatorTest.java @@ -92,6 +92,13 @@ public void testComparison() { assertVersionOrder("0.10.0-SNAPSHOT", "0.10.0.SNAPSHOT", "0.10.0-GA", "0.10.0.GA", "0.10.0"); } + @Test + public void testIsSnapshot() { + Assert.assertTrue(VersionComparator.isSnapshot("0.10.0-SNAPSHOT")); + Assert.assertTrue(VersionComparator.isSnapshot("0.10.0.snapshot")); + Assert.assertFalse(VersionComparator.isSnapshot("0.10.0")); + } + private static void assertVersionOrder(String v1, String v2, String ...otherVersions) { List versions = MutableList.of().append(v1, v2, otherVersions); From 062fdc4d0d000ffc00941d31d15bbe4534a5d707 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 4 May 2017 17:51:49 +0100 Subject: [PATCH 05/13] install and start in separate phases when rebinding or loading libs also recognise bundles from their originally installed URL, and misc other related cleanups; upgrade tests and others now passing! note: transformers are only allowed to delete bundles, not change --- ...atalogOsgiVersionMoreEntityRebindTest.java | 2 +- .../catalog/CatalogYamlRebindTest.java | 4 + .../internal/BasicBrooklynCatalog.java | 2 +- .../core/catalog/internal/CatalogUtils.java | 17 ++- .../core/mgmt/ha/OsgiArchiveInstaller.java | 112 ++++++++++++++---- .../mgmt/ha/OsgiBundleInstallationResult.java | 9 ++ .../brooklyn/core/mgmt/ha/OsgiManager.java | 91 ++++++++++---- .../core/mgmt/rebind/RebindContextImpl.java | 13 +- .../core/mgmt/rebind/RebindIteration.java | 15 ++- .../transformer/CompoundTransformer.java | 17 ++- .../core/typereg/BasicManagedBundle.java | 1 - .../brooklyn/core/BrooklynVersionTest.java | 4 +- .../mgmt/osgi/OsgiVersionMoreEntityTest.java | 4 +- ...eStoreObjectAccessorWriterTestFixture.java | 2 +- .../persist/XmlMementoSerializerTest.java | 13 +- .../transformer/CompoundTransformerTest.java | 1 - .../brooklyn/util/core/osgi/OsgiTestBase.java | 18 +++ .../rest/resources/CatalogResource.java | 37 ++++-- .../rest/resources/CatalogResourceTest.java | 28 +++-- 19 files changed, 299 insertions(+), 91 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java index a258fcd313..d71142cb50 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityRebindTest.java @@ -285,7 +285,7 @@ public void testClassAccessAfterUpgrade() throws Exception { // force upgrade OsgiBundleInstallationResult b2b = ((ManagementContextInternal)mgmt()).getOsgiManager().get().install(b2a.getMetadata(), - new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V2_EVIL_TWIN_URL), true, true).get(); + new ResourceUtils(getClass()).getResourceFromUrl(BROOKLYN_TEST_MORE_ENTITIES_V2_EVIL_TWIN_URL), true, true, true).get(); Assert.assertEquals(b2a.getBundle(), b2b.getBundle()); Assert.assertEquals(b2b.getCode(), OsgiBundleInstallationResult.ResultCode.UPDATED_EXISTING_BUNDLE); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java index c4af503476..127a110fdd 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogYamlRebindTest.java @@ -175,6 +175,10 @@ private void applyCompoundStateTransformer(RebindOptions options, final Compound for (BrooklynObjectType type : BrooklynObjectType.values()) { final List contents = objectStore.listContentsWithSubPath(type.getSubPathName()); for (String path : contents) { + if (path.endsWith(".jar")) { + // don't apply transformers to JARs + continue; + } StoreObjectAccessor accessor = objectStore.newAccessor(path); String memento = checkNotNull(accessor.get(), path); String replacement = transformed.getObjectsOfType(type).get(idFromPath(type, path)); 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 c91b7cdec1..30bf841839 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 @@ -582,7 +582,7 @@ private void collectCatalogItems(String sourceYaml, Map itemMetadata, List< PlanInterpreterGuessingType planInterpreter = new PlanInterpreterGuessingType(null, item, sourceYaml, itemType, libraryBundles, result).reconstruct(); if (!planInterpreter.isResolved()) { - throw Exceptions.create("Could not resolve item" + throw Exceptions.create("Could not resolve definition of item" + (Strings.isNonBlank(id) ? " '"+id+"'" : Strings.isNonBlank(symbolicName) ? " '"+symbolicName+"'" : Strings.isNonBlank(name) ? " '"+name+"'" : "") // better not to show yaml, takes up lots of space, and with multiple plan transformers there might be multiple errors; // some of the errors themselves may reproduce it diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java index 394814de1d..47dab6d5f7 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogUtils.java @@ -39,6 +39,7 @@ import org.apache.brooklyn.core.mgmt.classloading.BrooklynClassLoadingContextSequential; import org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext; import org.apache.brooklyn.core.mgmt.classloading.OsgiBrooklynClassLoadingContext; +import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult; import org.apache.brooklyn.core.mgmt.ha.OsgiManager; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.mgmt.rebind.RebindManagerImpl.RebindTracker; @@ -47,6 +48,7 @@ import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; import org.apache.brooklyn.core.typereg.RegisteredTypes; +import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Time; @@ -170,13 +172,24 @@ public static void installLibraries(ManagementContext managementContext, @Nullab "Loading bundles in {}: {}", new Object[] {managementContext, Joiner.on(", ").join(libraries)}); Stopwatch timer = Stopwatch.createStarted(); + List results = MutableList.of(); for (CatalogBundle bundleUrl : libraries) { - osgi.get().install(BasicManagedBundle.of(bundleUrl), null, true, false).get(); + OsgiBundleInstallationResult result = osgi.get().installDeferredStart(BasicManagedBundle.of(bundleUrl), null).get(); + if (log.isDebugEnabled()) { + logDebugOrTraceIfRebinding(log, "Installation of library "+bundleUrl+": "+result); + } + results.add(result); } - if (log.isDebugEnabled()) + for (OsgiBundleInstallationResult r: results) { + if (r.getDeferredStart()!=null) { + r.getDeferredStart().run(); + } + } + if (log.isDebugEnabled()) { logDebugOrTraceIfRebinding(log, "Registered {} bundles in {}", new Object[]{libraries.size(), Time.makeTimeStringRounded(timer)}); + } } } 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 0e069bdda1..4044f231de 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 @@ -28,6 +28,7 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipFile; +import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.core.catalog.internal.BasicBrooklynCatalog; import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult.ResultCode; @@ -45,19 +46,26 @@ import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.text.VersionComparator; import org.osgi.framework.Bundle; +import org.osgi.framework.BundleException; import org.osgi.framework.Constants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.Objects; // package-private so we can move this one if/when we move OsgiManager class OsgiArchiveInstaller { + private static final Logger log = LoggerFactory.getLogger(OsgiArchiveInstaller.class); + final private OsgiManager osgiManager; private ManagedBundle suppliedKnownBundleMetadata; private InputStream zipIn; - private boolean loadCatalogBom; - private boolean force; + private boolean start = true; + private boolean loadCatalogBom = true; + private boolean force = false; + private boolean deferredStart = false; private File zipFile; private Manifest discoveredManifest; @@ -65,13 +73,19 @@ class OsgiArchiveInstaller { OsgiBundleInstallationResult result; private ManagedBundle inferredMetadata; + private final boolean inputStreamSupplied; OsgiArchiveInstaller(OsgiManager osgiManager, ManagedBundle knownBundleMetadata, InputStream zipIn) { this.osgiManager = osgiManager; this.suppliedKnownBundleMetadata = knownBundleMetadata; this.zipIn = zipIn; + inputStreamSupplied = zipIn!=null; } + public void setStart(boolean start) { + this.start = start; + } + public void setLoadCatalogBom(boolean loadCatalogBom) { this.loadCatalogBom = loadCatalogBom; } @@ -80,6 +94,10 @@ public void setForce(boolean force) { this.force = force; } + public void setDeferredStart(boolean deferredStart) { + this.deferredStart = deferredStart; + } + private ManagementContextInternal mgmt() { return (ManagementContextInternal) osgiManager.mgmt; } @@ -98,6 +116,15 @@ private synchronized void makeLocalZipFileFromInputStreamOrUrl() { Maybe installedBundle = Maybe.absent(); if (suppliedKnownBundleMetadata!=null) { // if no input stream, look for a URL and/or a matching bundle + if (!suppliedKnownBundleMetadata.isNameResolved()) { + ManagedBundle mbFromUrl = osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl()); + if (mbFromUrl!=null) { + // user supplied just a URL (eg brooklyn.libraries), but we recognise it, + // so don't try to reload it, just record the info we know about it to retrieve the bundle + ((BasicManagedBundle)suppliedKnownBundleMetadata).setSymbolicName(mbFromUrl.getSymbolicName()); + ((BasicManagedBundle)suppliedKnownBundleMetadata).setVersion(mbFromUrl.getVersion()); + } + } if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) { installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find(); } @@ -115,19 +142,22 @@ private synchronized void makeLocalZipFileFromInputStreamOrUrl() { } } - if (installedBundle.isPresent()) { - result.bundle = installedBundle.get(); - - if (zipIn==null) { + if (zipIn==null) { + if (installedBundle.isPresent()) { // no way to install (no url), or no need to install (not forced); just ignore it result.metadata = osgiManager.getManagedBundle(new VersionedName(installedBundle.get())); + if (result.metadata==null) { + // low-level installed bundle + result.metadata = new BasicManagedBundle(installedBundle.get().getSymbolicName(), installedBundle.get().getVersion().toString(), + suppliedKnownBundleMetadata!=null ? suppliedKnownBundleMetadata.getUrl() : null); + } result.setIgnoringAlreadyInstalled(); return; } - } else { result.metadata = suppliedKnownBundleMetadata; throw new IllegalArgumentException("No input stream available and no URL could be found; nothing to install"); } + result.bundle = installedBundle.orNull(); } zipFile = Os.newTempFile("brooklyn-bundle-transient-"+suppliedKnownBundleMetadata, "zip"); @@ -243,10 +273,23 @@ public ReferenceWithError install() { if (result.code!=null) return ReferenceWithError.newInstanceWithoutError(result); assert inferredMetadata.isNameResolved() : "Should have resolved "+inferredMetadata; - Boolean updating = null; + final boolean updating; result.metadata = osgiManager.getManagedBundle(inferredMetadata.getVersionedName()); if (result.getMetadata()!=null) { - // already have a managed bundle + // already have a managed bundle - check if this is using a new/different URL + if (suppliedKnownBundleMetadata!=null && suppliedKnownBundleMetadata.getUrl()!=null) { + String knownIdForThisUrl = osgiManager.managedBundlesByUrl.get(suppliedKnownBundleMetadata.getUrl()); + if (knownIdForThisUrl==null) { + // it's a new URL, but a bundle we already know about + log.warn("Request to install from "+suppliedKnownBundleMetadata.getUrl()+" which is not recognized but "+ + "appears to match "+result.getMetadata()+"; now associating with the latter"); + osgiManager.managedBundlesByUrl.put(suppliedKnownBundleMetadata.getUrl(), result.getMetadata().getId()); + } else if (!knownIdForThisUrl.equals(result.getMetadata().getId())) { + log.warn("Request to install from "+suppliedKnownBundleMetadata.getUrl()+" which is associated to "+knownIdForThisUrl+" but "+ + "appears to match "+result.getMetadata()+"; now associating with the latter"); + osgiManager.managedBundlesByUrl.put(suppliedKnownBundleMetadata.getUrl(), result.getMetadata().getId()); + } + } if (canUpdate()) { result.bundle = osgiManager.framework.getBundleContext().getBundle(result.getMetadata().getOsgiUniqueUrl()); if (result.getBundle()==null) { @@ -290,7 +333,10 @@ public ReferenceWithError install() { if (!updating) { synchronized (osgiManager.managedBundles) { osgiManager.managedBundles.put(result.getMetadata().getId(), result.getMetadata()); - osgiManager.managedBundleIds.put(result.getMetadata().getVersionedName(), result.getMetadata().getId()); + osgiManager.managedBundlesByNam.put(result.getMetadata().getVersionedName(), result.getMetadata().getId()); + if (Strings.isNonBlank(result.getMetadata().getUrl())) { + osgiManager.managedBundlesByUrl.put(result.getMetadata().getUrl(), result.getMetadata().getId()); + } } result.code = OsgiBundleInstallationResult.ResultCode.INSTALLED_NEW_BUNDLE; result.message = "Installed "+result.getMetadata().getVersionedName()+" with ID "+result.getMetadata().getId(); @@ -305,16 +351,37 @@ public ReferenceWithError install() { // that seems fine and probably better than allowing bundles to start and catalog items to be installed // when brooklyn isn't aware it is supposed to be managing it - // starting here flags wiring issues earlier + // starting here flags wiring issues earlier // but may break some things running from the IDE - result.bundle.start(); - - if (updating!=null) { - osgiManager.uninstallCatalogItemsFromBundle( result.getVersionedName() ); - // (ideally removal and addition would be atomic) - } - if (loadCatalogBom) { - osgiManager.loadCatalogBom(result.bundle); + // eg if it doesn't have OSGi deps, or if it doesn't have camp parser, + // or if caller is installing multiple things that depend on each other + // eg rebind code, brooklyn.libraries list -- deferred start allows caller to + // determine whether not to start or to start all after things are installed + Runnable startRunnable = new Runnable() { + public void run() { + if (start) { + try { + result.bundle.start(); + } catch (BundleException e) { + throw Exceptions.propagate(e); + } + } + + if (loadCatalogBom) { + if (updating) { + osgiManager.uninstallCatalogItemsFromBundle( result.getVersionedName() ); + // (ideally removal and addition would be atomic) + } + for (CatalogItem ci: osgiManager.loadCatalogBom(result.bundle)) { + result.catalogItemsInstalled.add(ci.getId()); + } + } + } + }; + if (deferredStart) { + result.deferredStart = startRunnable; + } else { + startRunnable.run(); } return ReferenceWithError.newInstanceWithoutError(result); @@ -331,7 +398,9 @@ public ReferenceWithError install() { } private boolean canUpdate() { - return force || VersionComparator.isSnapshot(inferredMetadata.getVersion()); + // only update if forced, or it's a snapshot for which a byte stream is supplied + // (IE don't update a snapshot verison every time its URL is referenced in a 'libraries' section) + return force || (VersionComparator.isSnapshot(inferredMetadata.getVersion()) && inputStreamSupplied); } /** true if the supplied name and version are complete; updates if the known data is incomplete; @@ -355,5 +424,6 @@ private boolean matchSetOrFail(String source, String name, String version) { } return suppliedIsComplete; - } + } + } diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiBundleInstallationResult.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiBundleInstallationResult.java index 50ca081a97..c3a725a6bb 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiBundleInstallationResult.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiBundleInstallationResult.java @@ -34,6 +34,7 @@ public class OsgiBundleInstallationResult { ManagedBundle metadata; Bundle bundle; ResultCode code; + Runnable deferredStart; public enum ResultCode { INSTALLED_NEW_BUNDLE, @@ -63,9 +64,17 @@ public VersionedName getVersionedName() { if (getMetadata()==null) return null; return getMetadata().getVersionedName(); } + public Runnable getDeferredStart() { + return deferredStart; + } void setIgnoringAlreadyInstalled() { code = OsgiBundleInstallationResult.ResultCode.IGNORING_BUNDLE_AREADY_INSTALLED; message = "Bundle "+getMetadata().getVersionedName()+" already installed as "+getMetadata().getId(); } + + @Override + public String toString() { + return OsgiBundleInstallationResult.class.getSimpleName()+"["+code+", "+metadata+", "+message+"]"; + } } \ No newline at end of file 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 1685541b98..9936ba956e 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 @@ -103,7 +103,8 @@ public class OsgiManager { private Set bundlesAtStartup; private File osgiCacheDir; Map managedBundles = MutableMap.of(); - Map managedBundleIds = MutableMap.of(); + Map managedBundlesByNam = MutableMap.of(); + Map managedBundlesByUrl = MutableMap.of(); private static AtomicInteger numberOfReusableFrameworksCreated = new AtomicInteger(); private static final List OSGI_FRAMEWORK_CONTAINERS_FOR_REUSE = MutableList.of(); @@ -219,26 +220,45 @@ public Map getManagedBundles() { public String getManagedBundleId(VersionedName vn) { synchronized (managedBundles) { - return managedBundleIds.get(vn); + return managedBundlesByNam.get(vn); } } public ManagedBundle getManagedBundle(VersionedName vn) { synchronized (managedBundles) { - return managedBundles.get(managedBundleIds.get(vn)); + return managedBundles.get(managedBundlesByNam.get(vn)); } } + /** For bundles which are installed by a URL, see whether a bundle has been installed from that URL */ + public ManagedBundle getManagedBundleFromUrl(String url) { + synchronized (managedBundles) { + String id = managedBundlesByUrl.get(url); + if (id==null) return null; + return managedBundles.get(id); + } + } + /** See {@link OsgiArchiveInstaller#install()}, using default values */ public ReferenceWithError install(InputStream zipIn) { - return install(null, zipIn, true, false); + return new OsgiArchiveInstaller(this, null, zipIn).install(); + } + + /** See {@link OsgiArchiveInstaller#install()}, but deferring the start and catalog load */ + public ReferenceWithError installDeferredStart(@Nullable ManagedBundle knownBundleMetadata, @Nullable InputStream zipIn) { + OsgiArchiveInstaller installer = new OsgiArchiveInstaller(this, knownBundleMetadata, zipIn); + installer.setDeferredStart(true); + + return installer.install(); } - /** See {@link OsgiArchiveInstaller#install()} */ + /** See {@link OsgiArchiveInstaller#install()} - this exposes custom options */ + @Beta public ReferenceWithError install(@Nullable ManagedBundle knownBundleMetadata, @Nullable InputStream zipIn, - boolean loadCatalogBom, boolean forceUpdateOfNonSnapshots) { + boolean start, boolean loadCatalogBom, boolean forceUpdateOfNonSnapshots) { OsgiArchiveInstaller installer = new OsgiArchiveInstaller(this, knownBundleMetadata, zipIn); + installer.setStart(start); installer.setLoadCatalogBom(loadCatalogBom); installer.setForce(forceUpdateOfNonSnapshots); @@ -262,7 +282,8 @@ public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { if (metadata==null) { throw new IllegalStateException("No such bundle registered: "+bundleMetadata); } - managedBundleIds.remove(bundleMetadata.getVersionedName()); + managedBundlesByNam.remove(bundleMetadata.getVersionedName()); + managedBundlesByUrl.remove(bundleMetadata.getUrl()); } mgmt.getRebindManager().getChangeListener().onUnmanaged(bundleMetadata); @@ -280,7 +301,8 @@ public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { } } - void uninstallCatalogItemsFromBundle(VersionedName bundle) { + @Beta + public void uninstallCatalogItemsFromBundle(VersionedName bundle) { List thingsFromHere = ImmutableList.copyOf(getTypesFromBundle( bundle )); for (RegisteredType t: thingsFromHere) { mgmt.getCatalog().deleteCatalogItem(t.getSymbolicName(), t.getVersion()); @@ -338,11 +360,14 @@ public List> loadCatalogBom(Bundle bundle) { catalogItems = new CatalogBundleLoader(applicationsPermitted, mgmt).scanForCatalog(bundle); } catch (RuntimeException ex) { - try { - bundle.uninstall(); - } catch (BundleException e) { - log.error("Cannot uninstall bundle " + bundle.getSymbolicName() + ":" + bundle.getVersion(), e); - } + // TODO confirm -- as of May 2017 we no longer uninstall the bundle if install of catalog items fails; + // caller needs to upgrade, or uninstall then reinstall + // (this uninstall wouldn't have unmanaged it in brooklyn in any case) +// try { +// bundle.uninstall(); +// } catch (BundleException e) { +// log.error("Cannot uninstall bundle " + bundle.getSymbolicName() + ":" + bundle.getVersion()+" (after error installing catalog items)", e); +// } throw new IllegalArgumentException("Error installing catalog items", ex); } } @@ -469,21 +494,39 @@ public Maybe> tryResolveClass(String type, Iterable findBundle(OsgiBundleWithUrl catalogBundle) { - //Either fail at install time when the user supplied name:version is different - //from the one reported from the bundle - //or - //Ignore user supplied name:version when URL is supplied to be able to find the - //bundle even if it's with a different version. - // - //For now we just log a warning if there's a version discrepancy at install time, - //so prefer URL if supplied. - BundleFinder bundleFinder = Osgis.bundleFinder(framework); + // Prefer OSGi Location as URL or the managed bundle recorded URL, + // not bothering to check name:version if supplied here (eg to forgive snapshot version discrepancies); + // but fall back to name/version if URL is not known. + // Version checking may be stricter at install time. + Maybe result = null; if (catalogBundle.getUrl() != null) { + BundleFinder bundleFinder = Osgis.bundleFinder(framework); bundleFinder.requiringFromUrl(catalogBundle.getUrl()); - } else { + result = bundleFinder.find(); + if (result.isPresent()) { + return result; + } + + ManagedBundle mb = getManagedBundleFromUrl(catalogBundle.getUrl()); + if (mb!=null) { + bundleFinder.requiringFromUrl(null); + bundleFinder.symbolicName(mb.getSymbolicName()).version(mb.getVersion()); + result = bundleFinder.find(); + if (result.isPresent()) { + return result; + } + } + } + + if (catalogBundle.getSymbolicName()!=null) { + BundleFinder bundleFinder = Osgis.bundleFinder(framework); bundleFinder.symbolicName(catalogBundle.getSymbolicName()).version(catalogBundle.getVersion()); + return bundleFinder.find(); + } + if (result!=null) { + return result; } - return bundleFinder.find(); + return Maybe.absent("Insufficient information in "+catalogBundle+" to find bundle"); } /** diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindContextImpl.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindContextImpl.java index 13c85f6aa7..6bea476c38 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindContextImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindContextImpl.java @@ -36,8 +36,10 @@ import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.api.sensor.Feed; import org.apache.brooklyn.api.typereg.ManagedBundle; +import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.util.collections.MutableMap; +import org.osgi.framework.BundleException; import com.google.common.collect.Maps; @@ -90,9 +92,16 @@ public void registerCatalogItem(String id, CatalogItem catalogItem) { } // we don't track register/unregister of bundles; it isn't needed as it happens so early - public void installBundle(ManagedBundle bundle, InputStream zipInput) { - ((ManagementContextInternal)mgmt).getOsgiManager().get().install(bundle, zipInput, true, false).checkNoError(); + // but we do need to know which ones to start subsequently + public OsgiBundleInstallationResult installBundle(ManagedBundle bundle, InputStream zipInput) { + return ((ManagementContextInternal)mgmt).getOsgiManager().get().installDeferredStart(bundle, zipInput).get(); } + public void startBundle(OsgiBundleInstallationResult br) throws BundleException { + if (br.getDeferredStart()!=null) { + br.getDeferredStart().run(); + } + } + public void unregisterPolicy(Policy policy) { policies.remove(policy.getId()); diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java index 34c795cbe9..e92a774dbe 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java @@ -34,8 +34,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import com.google.common.collect.ImmutableList; - import org.apache.brooklyn.api.catalog.BrooklynCatalog; import org.apache.brooklyn.api.catalog.CatalogItem; import org.apache.brooklyn.api.entity.Application; @@ -82,6 +80,7 @@ import org.apache.brooklyn.core.location.internal.LocationInternal; import org.apache.brooklyn.core.mgmt.classloading.BrooklynClassLoadingContextSequential; import org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContext; +import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult; import org.apache.brooklyn.core.mgmt.internal.BrooklynObjectManagementMode; import org.apache.brooklyn.core.mgmt.internal.BrooklynObjectManagerInternal; import org.apache.brooklyn.core.mgmt.internal.EntityManagerInternal; @@ -115,6 +114,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -325,15 +325,24 @@ protected void installBundlesAndRebuildCatalog() { // Install bundles if (rebindManager.persistBundlesEnabled) { + List installs = MutableList.of(); logRebindingDebug("RebindManager installing bundles: {}", mementoManifest.getBundleIds()); for (ManagedBundleMemento bundleM : mementoManifest.getBundles().values()) { logRebindingDebug("RebindManager installing bundle {}", bundleM.getId()); try (InputStream in = bundleM.getJarContent().openStream()) { - rebindContext.installBundle(instantiator.newManagedBundle(bundleM), in); + installs.add(rebindContext.installBundle(instantiator.newManagedBundle(bundleM), in)); } catch (Exception e) { exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE, bundleM.getId(), bundleM.getSymbolicName(), e); } } + // Start them all after we've installed them + for (OsgiBundleInstallationResult br: installs) { + try { + rebindContext.startBundle(br); + } catch (Exception e) { + exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE, br.getMetadata().getId(), br.getMetadata().getSymbolicName(), e); + } + } } else { logRebindingDebug("Not rebinding bundles; feature disabled: {}", mementoManifest.getBundleIds()); } diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/CompoundTransformer.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/CompoundTransformer.java index 78c8e9192d..5f2330d6e1 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/CompoundTransformer.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/transformer/CompoundTransformer.java @@ -28,7 +28,6 @@ import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoPersister; import org.apache.brooklyn.api.mgmt.rebind.mementos.BrooklynMementoRawData; import org.apache.brooklyn.api.objs.BrooklynObjectType; -import org.apache.brooklyn.core.mgmt.persist.BrooklynMementoPersisterToObjectStore; import org.apache.brooklyn.core.mgmt.rebind.transformer.impl.XsltTransformer; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ResourceUtils; @@ -46,6 +45,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import com.google.common.collect.Sets; +import com.google.common.io.ByteSource; @Beta public class CompoundTransformer { @@ -265,6 +265,7 @@ public BrooklynMementoRawData transform(BrooklynMementoRawData rawData) throws E Map feeds = MutableMap.copyOf(rawData.getFeeds()); Map catalogItems = MutableMap.copyOf(rawData.getCatalogItems()); Map bundles = MutableMap.copyOf(rawData.getBundles()); + Map bundleJars = MutableMap.copyOf(rawData.getBundleJars()); // TODO @neykov asks whether transformers should be run in registration order, // rather than in type order. TBD. (would be an easy change.) @@ -328,7 +329,11 @@ public BrooklynMementoRawData transform(BrooklynMementoRawData rawData) throws E LOG.warn("Unable to delete " + type + " id"+Strings.s(missing.size())+" ("+missing+"), " + "because not found in persisted state (continuing)"); } + // bundles have to be supplied by ID, but if so they can be deleted along with the jars bundles.keySet().removeAll(itemsToDelete); + for (String item: itemsToDelete) { + bundleJars.remove(item+".jar"); + } break; case UNKNOWN: break; // no-op @@ -372,9 +377,12 @@ public BrooklynMementoRawData transform(BrooklynMementoRawData rawData) throws E } break; case MANAGED_BUNDLE: - for (Map.Entry entry : bundles.entrySet()) { - entry.setValue(transformer.transform(entry.getValue())); - } + // transform of bundles and JARs not supported - you can delete, that's all + // TODO we should support a better way of adding/removing bundles, + // e.g. start in management mode where you can edit brooklyn-managed bundles +// for (Map.Entry entry : bundles.entrySet()) { +// entry.setValue(transformer.transform(entry.getValue())); +// } break; case UNKNOWN: break; // no-op @@ -393,6 +401,7 @@ public BrooklynMementoRawData transform(BrooklynMementoRawData rawData) throws E .feeds(feeds) .catalogItems(catalogItems) .bundles(bundles) + .bundleJars(bundleJars) .build(); } diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java index 4799c06b7a..6c0fa3549f 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicManagedBundle.java @@ -185,5 +185,4 @@ protected BrooklynObjectInternal configure(Map flags) { public static ManagedBundle of(CatalogBundle bundleUrl) { return new BasicManagedBundle(bundleUrl.getSymbolicName(), bundleUrl.getVersion(), bundleUrl.getUrl()); } - } diff --git a/core/src/test/java/org/apache/brooklyn/core/BrooklynVersionTest.java b/core/src/test/java/org/apache/brooklyn/core/BrooklynVersionTest.java index ccaca98ccb..049916c156 100644 --- a/core/src/test/java/org/apache/brooklyn/core/BrooklynVersionTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/BrooklynVersionTest.java @@ -29,9 +29,10 @@ import org.apache.brooklyn.core.catalog.internal.CatalogItemBuilder; import org.apache.brooklyn.core.catalog.internal.CatalogItemDtoAbstract; import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext; -import org.apache.brooklyn.util.osgi.OsgiTestResources; import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; import org.apache.brooklyn.test.support.TestResourceUnavailableException; +import org.apache.brooklyn.util.core.osgi.OsgiTestBase; +import org.apache.brooklyn.util.osgi.OsgiTestResources; import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -101,6 +102,7 @@ public void testGetFeatures() throws Exception { String version = "0.1.0"; String type = "brooklyn.osgi.tests.SimpleEntity"; List libraries = Lists.newArrayList("classpath:" + OsgiTestResources.BROOKLYN_TEST_OSGI_ENTITIES_PATH); + OsgiTestBase.preinstallLibrariesLowLevelToPreventCatalogBomParsing(mgmt, libraries.toArray(new String[1])); CatalogEntityItemDto c1 = CatalogItemBuilder.newEntity(symName, version) .javaType(type) diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/osgi/OsgiVersionMoreEntityTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/osgi/OsgiVersionMoreEntityTest.java index f19556eedf..d4cf6f52f4 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/osgi/OsgiVersionMoreEntityTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/osgi/OsgiVersionMoreEntityTest.java @@ -46,6 +46,7 @@ import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.core.osgi.OsgiTestBase; import org.apache.brooklyn.util.core.osgi.Osgis; +import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.os.Os; import org.apache.brooklyn.util.osgi.OsgiTestResources; @@ -128,9 +129,10 @@ protected RegisteredType addCatalogItemWithTypeAsName(String type, String versio protected RegisteredType addCatalogItemWithNameAndType(String symName, String version, String type, String ...libraries) { return addCatalogItemWithNameAndType(mgmt, symName, version, type, libraries); } - + @SuppressWarnings("deprecation") static RegisteredType addCatalogItemWithNameAndType(ManagementContext mgmt, String symName, String version, String type, String ...libraries) { + OsgiTestBase.preinstallLibrariesLowLevelToPreventCatalogBomParsing(mgmt, libraries); CatalogEntityItemDto c1 = newCatalogItemWithNameAndType(symName, version, type, libraries); mgmt.getCatalog().addItem(c1); RegisteredType c2 = mgmt.getTypeRegistry().get(symName, version); diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/PersistenceStoreObjectAccessorWriterTestFixture.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/PersistenceStoreObjectAccessorWriterTestFixture.java index b4ac83fbc2..47e42045e6 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/PersistenceStoreObjectAccessorWriterTestFixture.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/PersistenceStoreObjectAccessorWriterTestFixture.java @@ -104,7 +104,7 @@ public void testLastModifiedTime() throws Exception { Date write1 = accessor.getLastModifiedDate(); Assert.assertNotNull(write1); - Time.sleep(getLastModifiedResolution().times(2)); + Time.sleep(getLastModifiedResolution().multiply(2)); accessor.put("abc"); accessor.waitForCurrentWrites(TIMEOUT); Date write2 = accessor.getLastModifiedDate(); diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java index 9b8ccae4a4..59ea8f2f4a 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/persist/XmlMementoSerializerTest.java @@ -531,9 +531,7 @@ public void testRenamedOsgiClassWithoutBundlePrefixInRename() throws Exception { bundlePrefix + ":" + classname, bundlePrefix + ":" + oldClassname)); } - // TODO This doesn't get the bundleName - should we expect it to? Is this because of - // how we're using Felix? Would it also be true in Karaf? - @Test(groups="Broken") + @Test public void testOsgiBundleNamePrefixIncludedForDownstreamDependency() throws Exception { mgmt = LocalManagementContextForTests.builder(true).enableOsgiReusable().build(); serializer.setLookupContext(newEmptyLookupManagementContext(mgmt, true)); @@ -546,9 +544,14 @@ public void testOsgiBundleNamePrefixIncludedForDownstreamDependency() throws Exc assertSerializeAndDeserialize(obj); // i.e. prepended with bundle name - String expectedForm = "<"+bundleName+":"+classname+">ALWAYS_TRUE"; + String expectedFormInFelix = "<"+classname+">ALWAYS_TRUE"; + String expectedFormInKaraf = "<"+bundleName+":"+classname+">ALWAYS_TRUE"; String serializedForm = serializer.toString(obj); - assertEquals(serializedForm.trim(), expectedForm.trim()); + // TODO we don't currently have a way to test this with karaf or check if we are karaf or felix + // so this test isn't yet of much value, but other tests assert the full form for installed bundles + // so I think we're alright + Assert.assertTrue(serializedForm.trim().equals(expectedFormInFelix) || serializedForm.trim().equals(expectedFormInKaraf), + "Should have matched either the karaf or the felix form, but was "+serializedForm); } @Test diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/transformer/CompoundTransformerTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/transformer/CompoundTransformerTest.java index 9f8e5826cf..05969ec53c 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/transformer/CompoundTransformerTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/rebind/transformer/CompoundTransformerTest.java @@ -38,7 +38,6 @@ import org.apache.brooklyn.api.objs.BrooklynObjectType; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.config.BasicConfigKey; -import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.mgmt.persist.BrooklynMementoPersisterToObjectStore; import org.apache.brooklyn.core.mgmt.persist.FileBasedObjectStore; import org.apache.brooklyn.core.mgmt.persist.PersistMode; diff --git a/core/src/test/java/org/apache/brooklyn/util/core/osgi/OsgiTestBase.java b/core/src/test/java/org/apache/brooklyn/util/core/osgi/OsgiTestBase.java index db66c1341a..0e6d751f87 100644 --- a/core/src/test/java/org/apache/brooklyn/util/core/osgi/OsgiTestBase.java +++ b/core/src/test/java/org/apache/brooklyn/util/core/osgi/OsgiTestBase.java @@ -17,6 +17,10 @@ import java.io.File; import java.io.IOException; + +import org.apache.brooklyn.api.mgmt.ManagementContext; +import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; +import org.apache.brooklyn.util.exceptions.Exceptions; import org.apache.brooklyn.util.os.Os; import org.apache.commons.io.FileUtils; import org.osgi.framework.BundleException; @@ -53,4 +57,18 @@ public static void tearDownOsgiFramework(Framework framework, File storageTempDi } } + public static void preinstallLibrariesLowLevelToPreventCatalogBomParsing(ManagementContext mgmt, String ...libraries) { + // catalog BOM CAMP syntax not available in core; need to pre-install + // to prevent Brooklyn from installing BOMs in those libraries + for (String lib: libraries) { + // install libs manually to prevent catalog BOM loading + // (could do OsgiManager.installDeferredStart also, then just ignore the start) + try { + Osgis.install(((ManagementContextInternal)mgmt).getOsgiManager().get().getFramework(), lib); + } catch (BundleException e) { + throw Exceptions.propagate(e); + } + } + } + } diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index 7359e4d397..c5a2b59bb9 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -62,6 +62,7 @@ import org.apache.brooklyn.rest.domain.CatalogPolicySummary; import org.apache.brooklyn.rest.filter.HaHotStateRequired; import org.apache.brooklyn.rest.transform.CatalogTransformer; +import org.apache.brooklyn.rest.util.BrooklynRestResourceUtils; import org.apache.brooklyn.rest.util.WebResourceUtils; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; @@ -151,25 +152,35 @@ public Response createFromYaml(String yaml) { } public static class BundleInstallationRestResult { + // as Osgi result, but without bundle, and with maps of catalog items installed + String message; ManagedBundle metadata; + OsgiBundleInstallationResult.ResultCode code; - enum ResultCode { - INSTALLED_NEW_BUNDLE, - UPDATED_EXISTING_BUNDLE, - IGNORING_BUNDLE_AREADY_INSTALLED, - ERROR_PREPARING_BUNDLE, - ERROR_INSTALLING_BUNDLE - } - Map catalogItemsInstalled; + Map types; public String getMessage() { return message; } - public static BundleInstallationRestResult of(OsgiBundleInstallationResult result, ManagementContext mgmt) { - // TODO - return null; + @SuppressWarnings("deprecation") + public static BundleInstallationRestResult of(OsgiBundleInstallationResult in, ManagementContext mgmt, BrooklynRestResourceUtils brooklynU, UriInfo ui) { + BundleInstallationRestResult result = new BundleInstallationRestResult(); + result.message = in.getMessage(); + result.metadata = in.getMetadata(); + result.code = in.getCode(); + if (in.getCatalogItemsInstalled()!=null) { + result.types = MutableMap.of(); + for (String id: in.getCatalogItemsInstalled()) { + // TODO prefer to use RegisteredType, but we need transformer for those in REST + //RegisteredType ci = mgmt.getTypeRegistry().get(id); + + CatalogItem ci = CatalogUtils.getCatalogItemOptionalVersion(mgmt, id); + CatalogTransformer.catalogItemSummary(brooklynU, ci, ui.getBaseUriBuilder()); + } + } + return result; } } @@ -186,10 +197,10 @@ public Response createFromArchive(byte[] zipInput) { if (result.hasError()) { return ApiError.builder().errorCode(Status.BAD_REQUEST).message(result.getWithoutError().getMessage()) - .data(result).build().asJsonResponse(); + .data(BundleInstallationRestResult.of(result.getWithoutError(), mgmt(), brooklyn(), ui)).build().asJsonResponse(); } - return Response.status(Status.CREATED).entity( BundleInstallationRestResult.of(result.get(), mgmt()) ).build(); + return Response.status(Status.CREATED).entity( BundleInstallationRestResult.of(result.get(), mgmt(), brooklyn(), ui) ).build(); } private Response buildCreateResponse(Iterable> catalogItems) { diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java index 5cfcf4cc9c..714dad465b 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/resources/CatalogResourceTest.java @@ -59,6 +59,7 @@ import org.apache.brooklyn.rest.domain.CatalogLocationSummary; import org.apache.brooklyn.rest.domain.CatalogPolicySummary; import org.apache.brooklyn.rest.testing.BrooklynRestResourceTest; +import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.test.support.TestResourceUnavailableException; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ResourceUtils; @@ -483,7 +484,7 @@ public void testInvalidArchive() throws Exception { .post(Streams.readFully(new FileInputStream(f))); assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode()); - assertTrue(response.readEntity(String.class).contains("Invalid ZIP/JAR archive")); + Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), "zip file is empty"); } @Test @@ -495,7 +496,7 @@ public void testArchiveWithoutBom() throws Exception { .post(Streams.readFully(new FileInputStream(f))); assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode()); - assertTrue(response.readEntity(String.class).contains("Archive must contain a catalog.bom file in the root")); + Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), "Missing bundle symbolic name in BOM or MANIFEST"); } @Test @@ -514,7 +515,7 @@ public void testArchiveWithoutBundleAndVersion() throws Exception { .post(Streams.readFully(new FileInputStream(f))); assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode()); - assertTrue(response.readEntity(String.class).contains("Catalog BOM must define bundle and version")); + Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), "Missing bundle symbolic name in BOM or MANIFEST"); } @Test @@ -534,7 +535,8 @@ public void testArchiveWithoutBundle() throws Exception { .post(Streams.readFully(new FileInputStream(f))); assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode()); - assertTrue(response.readEntity(String.class).contains("Catalog BOM must define bundle")); + Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), + "Missing bundle symbolic name in BOM or MANIFEST"); } @Test @@ -554,7 +556,7 @@ public void testArchiveWithoutVersion() throws Exception { .post(Streams.readFully(new FileInputStream(f))); assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode()); - assertTrue(response.readEntity(String.class).contains("Catalog BOM must define version")); + Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), "Catalog BOM must define version"); } @Test @@ -562,6 +564,7 @@ public void testJarWithoutMatchingBundle() throws Exception { String name = "My Catalog App"; String bundle = "org.apache.brooklyn.test"; String version = "0.1.0"; + String wrongBundleName = "org.apache.brooklyn.test2"; File f = createJar(ImmutableMap.of( "catalog.bom", Joiner.on("\n").join( "brooklyn.catalog:", @@ -576,7 +579,7 @@ public void testJarWithoutMatchingBundle() throws Exception { "META-INF/MANIFEST.MF", Joiner.on("\n").join( "Manifest-Version: 1.0", "Bundle-Name: " + name, - "Bundle-SymbolicName: org.apache.brooklyn.test2", + "Bundle-SymbolicName: "+wrongBundleName, "Bundle-Version: " + version, "Bundle-ManifestVersion: " + version))); @@ -585,7 +588,9 @@ public void testJarWithoutMatchingBundle() throws Exception { .post(Streams.readFully(new FileInputStream(f))); assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode()); - assertTrue(response.readEntity(String.class).contains("JAR MANIFEST symbolic-name 'org.apache.brooklyn.test2' does not match '"+bundle+"' defined in BOM")); + Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), + "symbolic name mismatch", + wrongBundleName, bundle); } @Test @@ -593,6 +598,7 @@ public void testJarWithoutMatchingVersion() throws Exception { String name = "My Catalog App"; String bundle = "org.apache.brooklyn.test"; String version = "0.1.0"; + String wrongVersion = "0.3.0"; File f = createJar(ImmutableMap.of( "catalog.bom", Joiner.on("\n").join( "brooklyn.catalog:", @@ -608,15 +614,17 @@ public void testJarWithoutMatchingVersion() throws Exception { "Manifest-Version: 1.0", "Bundle-Name: " + name, "Bundle-SymbolicName: " + bundle, - "Bundle-Version: 0.3.0", - "Bundle-ManifestVersion: 0.3.0"))); + "Bundle-Version: " + wrongVersion, + "Bundle-ManifestVersion: " + wrongVersion))); Response response = client().path("/catalog") .header(HttpHeaders.CONTENT_TYPE, "application/x-jar") .post(Streams.readFully(new FileInputStream(f))); assertEquals(response.getStatus(), Response.Status.BAD_REQUEST.getStatusCode()); - assertTrue(response.readEntity(String.class).contains("JAR MANIFEST version '0.3.0' does not match '"+version+"' defined in BOM")); + Asserts.assertStringContainsIgnoreCase(response.readEntity(String.class), + "version mismatch", + wrongVersion, version); } @Test From abdb13e6a13e5f298b798c64856e95c94c1345f4 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 8 May 2017 11:10:33 +0100 Subject: [PATCH 06/13] Fixup of tests which failed following BOM auto-install Tests assume BOMs wouldn't be installed, and either test wasn't expecting to read BOM or BOM had a problem --- .../catalog/CatalogMakeOsgiBundleTest.java | 7 ++++-- .../catalog/CatalogOsgiLibraryTest.java | 9 +++---- .../CatalogOsgiVersionMoreEntityTest.java | 4 +++- .../catalog/CatalogOsgiYamlEntityTest.java | 15 ++++++------ .../brooklyn/test/lite/CampYamlLiteTest.java | 22 ++++++++++++++---- .../src/main/resources/catalog.bom | 7 +++--- .../java/org/apache/brooklyn/util/os/Os.java | 5 +++- ...rooklyn-test-osgi-com-example-entities.jar | Bin 22130 -> 22138 bytes .../osgi/brooklyn-test-osgi-entities.jar | Bin 22067 -> 22072 bytes ...brooklyn-test-osgi-more-entities_0.1.0.jar | Bin 15997 -> 16003 bytes ...brooklyn-test-osgi-more-entities_0.2.0.jar | Bin 16903 -> 16924 bytes ...est-osgi-more-entities_evil-twin_0.2.0.jar | Bin 14091 -> 14099 bytes 12 files changed, 43 insertions(+), 26 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java index 044a2eafa3..397bcf70fd 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogMakeOsgiBundleTest.java @@ -49,6 +49,7 @@ import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.core.osgi.BundleMaker; import org.apache.brooklyn.util.exceptions.Exceptions; +import org.apache.brooklyn.util.osgi.VersionedName; import org.apache.brooklyn.util.text.Identifiers; import org.apache.brooklyn.util.text.Strings; import org.osgi.framework.Bundle; @@ -90,7 +91,8 @@ public void cleanUpButKeepMgmt() throws Exception { Entities.destroy(app); } for (Bundle b: bundlesToRemove) { - b.uninstall(); + ((ManagementContextInternal)mgmt()).getOsgiManager().get().uninstallUploadedBundle( + ((ManagementContextInternal)mgmt()).getOsgiManager().get().getManagedBundle(new VersionedName(b))); } bundlesToRemove.clear(); } @@ -157,7 +159,8 @@ public void testCatalogBomFromBundleWithManualManifest() throws Exception { jf = bm.copyAddingManifest(jf, MutableMap.of( "Manifest-Version", "1.0", - "Bundle-SymbolicName", customName)); + "Bundle-SymbolicName", customName, + "Bundle-Version", "0.0.0.SNAPSHOT")); Assert.assertTrue(bm.hasOsgiManifest(jf)); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiLibraryTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiLibraryTest.java index 111ec4b647..aebd80d86c 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiLibraryTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiLibraryTest.java @@ -178,9 +178,8 @@ public void testLibraryUrlDoesNotExist() throws Exception { " - type: " + BasicApplication.class.getName()); Asserts.shouldHaveFailedPreviously(); } catch (Exception e) { - if (!e.toString().contains("Bundle from " + wrongUrl + " failed to install")) { - throw e; - } + Asserts.expectedFailureContains(e, wrongUrl); + Asserts.expectedFailureContainsIgnoreCase(e, "not found"); } } @@ -199,9 +198,7 @@ public void testLibraryMalformed() throws Exception { " - type: " + BasicApplication.class.getName()); Asserts.shouldHaveFailedPreviously(); } catch (Exception e) { - if (!e.toString().contains("not a jar file")) { - throw e; - } + Asserts.expectedFailureContainsIgnoreCase(e, "opening zip"); } } diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java index aebfed9e8b..8961c6f7d1 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiVersionMoreEntityTest.java @@ -33,6 +33,7 @@ import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.camp.brooklyn.AbstractYamlTest; import org.apache.brooklyn.camp.brooklyn.spi.creation.BrooklynEntityMatcher; +import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.mgmt.ha.OsgiBundleInstallationResult; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.mgmt.osgi.OsgiVersionMoreEntityTest; @@ -180,7 +181,8 @@ public void testMoreEntityV1ThenV2GivesV2() throws Exception { OsgiVersionMoreEntityTest.assertV2MethodCall(moreEntity); } - @Test + @Test(groups="Broken") // won't work until search path is based on bundles instead of registered types + // (though it would work if we set versions properly in the OSGi bundles, but brooklyn types there all declare brooklyn version) public void testMoreEntityBothV1AndV2() throws Exception { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.1.0.jar"); TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), "/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar"); diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java index 7b726bc6e7..fe86ed0648 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogOsgiYamlEntityTest.java @@ -39,6 +39,7 @@ import org.apache.brooklyn.core.test.entity.TestEntityImpl; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.entity.stock.BasicEntity; +import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.test.support.TestResourceUnavailableException; import org.apache.brooklyn.util.core.ResourceUtils; import org.apache.brooklyn.util.osgi.OsgiTestResources; @@ -224,9 +225,9 @@ public void testReferenceNonInstalledBundledByNameFails() { " version: " + nonExistentVersion, " item:", " type: " + SIMPLE_ENTITY_TYPE); - fail(); + Asserts.shouldHaveFailedPreviously(); } catch (IllegalStateException e) { - Assert.assertEquals(e.getMessage(), "Bundle from null failed to install: Bundle CatalogBundleDto{symbolicName=" + nonExistentId + ", version=" + nonExistentVersion + ", url=null} not previously registered, but URL is empty."); + Asserts.expectedFailureContainsIgnoreCase(e, nonExistentId, nonExistentVersion, "no input stream", "no URL"); } } @@ -305,13 +306,11 @@ public void testFullBundleReferenceUrlMetaOverridesLocalNameVersion() { "", " item:", " type: " + SIMPLE_ENTITY_TYPE); - fail(); + Asserts.shouldHaveFailedPreviously(); } catch (IllegalStateException e) { - assertEquals(e.getMessage(), "Bundle from " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL + " failed to install: " + - "Bundle already installed as " + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_NAME + ":" + - OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_VERSION + " but user explicitly requested " + - "CatalogBundleDto{symbolicName=" + nonExistentId + ", version=" + nonExistentVersion + ", url=" + - OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL + "}"); + Asserts.expectedFailureContainsIgnoreCase(e, nonExistentId, nonExistentVersion, + "symbolic name mismatch", OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_NAME, + OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL); } } diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java index 1257b3e140..2b1b4882a5 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/test/lite/CampYamlLiteTest.java @@ -52,6 +52,7 @@ import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; import org.apache.brooklyn.core.test.entity.TestApplication; import org.apache.brooklyn.core.test.entity.TestEntity; +import org.apache.brooklyn.core.typereg.BasicManagedBundle; import org.apache.brooklyn.core.typereg.RegisteredTypeLoadingContexts; import org.apache.brooklyn.core.typereg.RegisteredTypes; import org.apache.brooklyn.test.support.TestResourceUnavailableException; @@ -156,6 +157,7 @@ public void testAddChildrenEffector() throws Exception { @Test public void testYamlServiceForCatalog() { TestResourceUnavailableException.throwIfResourceUnavailable(getClass(), OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_PATH); + installWithoutCatalogBom(mgmt, OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL); CatalogItem realItem = Iterables.getOnlyElement(mgmt.getCatalog().addItems(Streams.readFullyStringAndClose(getClass().getResourceAsStream("test-app-service-blueprint.yaml")))); Iterable> retrievedItems = mgmt.getCatalog() @@ -184,13 +186,13 @@ public void testRegisterCustomEntityWithBundleWhereEntityIsFromCoreAndIconFromBu String symbolicName = "my.catalog.app.id"; String bundleUrl = OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL; - String yaml = getSampleMyCatalogAppYaml(symbolicName, bundleUrl); + String yaml = prepAndGetSampleMyCatalogAppYaml(symbolicName, bundleUrl); mgmt.getCatalog().addItems(yaml); assertMgmtHasSampleMyCatalogApp(symbolicName, bundleUrl); } - + @SuppressWarnings("deprecation") @Test public void testResetXmlWithCustomEntity() throws IOException { @@ -198,10 +200,11 @@ public void testResetXmlWithCustomEntity() throws IOException { String symbolicName = "my.catalog.app.id"; String bundleUrl = OsgiStandaloneTest.BROOKLYN_TEST_OSGI_ENTITIES_URL; - String yaml = getSampleMyCatalogAppYaml(symbolicName, bundleUrl); + String yaml = prepAndGetSampleMyCatalogAppYaml(symbolicName, bundleUrl); LocalManagementContext mgmt2 = LocalManagementContextForTests.newInstanceWithOsgi(); try { + installWithoutCatalogBom(mgmt2, bundleUrl); CampPlatformWithJustBrooklynMgmt platform2 = new CampPlatformWithJustBrooklynMgmt(mgmt2); MockWebPlatform.populate(platform2, TestAppAssemblyInstantiator.class); @@ -215,7 +218,9 @@ public void testResetXmlWithCustomEntity() throws IOException { assertMgmtHasSampleMyCatalogApp(symbolicName, bundleUrl); } - private String getSampleMyCatalogAppYaml(String symbolicName, String bundleUrl) { + private String prepAndGetSampleMyCatalogAppYaml(String symbolicName, String bundleUrl) { + installWithoutCatalogBom(mgmt, bundleUrl); + return Joiner.on("\n").join( "brooklyn.catalog:", " id: " + symbolicName, @@ -227,7 +232,14 @@ private String getSampleMyCatalogAppYaml(String symbolicName, String bundleUrl) " libraries:", " - url: " + bundleUrl, " item:", - " type: io.camp.mock:AppServer"); + " type: " + MockWebPlatform.APPSERVER.getName()); + } + + protected void installWithoutCatalogBom(LocalManagementContext mgmt, String bundleUrl) { + // install bundle for class access but without loading its catalog.bom, + // since we only have mock matchers here + // (if we don't do this, the default routines install it and try to process the catalog.bom, failing) + mgmt.getOsgiManager().get().installDeferredStart(new BasicManagedBundle(null, null, bundleUrl), null).get(); } private void assertMgmtHasSampleMyCatalogApp(String symbolicName, String bundleUrl) { diff --git a/utils/common/dependencies/osgi/more-entities-v2/src/main/resources/catalog.bom b/utils/common/dependencies/osgi/more-entities-v2/src/main/resources/catalog.bom index 3ae38e5ce1..630750898c 100644 --- a/utils/common/dependencies/osgi/more-entities-v2/src/main/resources/catalog.bom +++ b/utils/common/dependencies/osgi/more-entities-v2/src/main/resources/catalog.bom @@ -38,6 +38,7 @@ brooklyn.catalog: - id: org.apache.brooklyn.test.osgi.entities.more.MoreTemplate itemType: template item: - type: org.apache.brooklyn.test.osgi.entities.more.MoreTemplate - name: More Template - description: Cataliog item OSGi test template + services: + - type: org.apache.brooklyn.test.osgi.entities.more.MoreTemplate + name: More Template + description: Catalog item OSGi test template diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java b/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java index bd285589da..f1c35dca73 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/os/Os.java @@ -533,10 +533,13 @@ private static boolean testForMicrosoftWindows() { /** creates a private temp file which will be deleted on exit; * either prefix or ext may be null; - * if ext is non-empty and not > 4 chars and not starting with a ., then a dot will be inserted */ + * if ext is non-empty and not > 4 chars and not starting with a ., then a dot will be inserted; + * if either name part is too long it will be shortened to prevent filesystem errors */ public static File newTempFile(String prefix, String ext) { String sanitizedPrefix = (Strings.isNonEmpty(prefix) ? Strings.makeValidFilename(prefix) + "-" : ""); + if (sanitizedPrefix.length()>101) sanitizedPrefix = sanitizedPrefix.substring(0, 100)+"--"; String extWithPrecedingSeparator = (Strings.isNonEmpty(ext) ? ext.startsWith(".") || ext.length()>4 ? ext : "."+ext : ""); + if (extWithPrecedingSeparator.length()>13) sanitizedPrefix = sanitizedPrefix.substring(0, 12)+"--"; try { File tempFile = File.createTempFile(sanitizedPrefix, extWithPrecedingSeparator, new File(tmp())); tempFile.deleteOnExit(); diff --git a/utils/common/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-com-example-entities.jar b/utils/common/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-com-example-entities.jar index 86dfd8bbc5dadf24e4cd5731afdc130ceab15d4a..7180c4d60169e9aa1f3d4f2be61b8c1416cc1ad6 100644 GIT binary patch delta 2324 zcmZ8i2~<;O7Jk_V5=cnc0=%#}fk2QD2T%w^iXkXl2qq}wfuaa1U~m}+8Z1Uq!Pb#Q zf3euI#1;@VBicxnB8uoxMo_D`00(3jH|joCL^|)ia3b-Y^PhX)m+vnBU0z;lano8H zRT@PgQXt3_f@)Lh4^Y_zRQPdJNn85))_KcEL&EaP*EG<`hwu&4(qFPNNlABh)n zt$ z1AN!t(%f0Y`<55u`F+0EoikjoVLKOY*{yQF3+_oUL_W@+Lzj%Dm%PoU}`KQL#c z?0OGz;G2Ul-TqF#w|4vL{KH35G)WD|&Qlh{wA0&Ca>(rwWR8pZ64nmgw%XqPKA*Mv zKQC>|tgHNIo9$&QQQ?WUqjv30Uw3|;;d_`-5*XI|s&80Zlzih^)!FZh_@qKcIMwnX zC(BYhyYY|8AaK61^K^XR7V$v;52^>}c=Q`C3mQ5V@{5xhsmX5d-vsn8c@bPDFASFc zZ`rW~+uu)G$Y=Z}c($=tfvpMF>zjk8r*_Gnzp00Dz;c!P5gTNa7S&Ayk?3k|MpSD zRrXfxmpO@<{)!tRZ-3d@MOV(vK_l6ER_;?{JwlJ zU|;Ts>rk-IhDzL%>bEU~$7B0w4_AptO1ws$)k8_5Yc1zjnW{bBw*K_zqNLqh_Z8lB z*LgAvg2Kmnxu@?A<+O%exR{pjy!X}7(^pUV3GNndSp9R;{l9ujlFPRjg?MLu7#hC$ z;=3Z1G|I&EYGd!}CwOEJkX?|P8`h#Gr5+>@oC&DRC2%lVL*Qfb9>E{o7hpjYU^1F0 zVj#RoXx`%qaLx-sH<5x`==zA&c+7OC$!^p{H)|4$!xSmGK(F*t_;?I4qb3tDMM1C8 zBi2^wdd_a^O&9_c^bj4F+4Av){+NV-=}c6x>8YDUJAU~%g$uq3D=2QU&? zi|G|Y2e3v)*XaZg^z;g0kY4Ez;bVCKs)&o`0|X)&CQC&-35iWV=xAYT5(a>Xd{rn- z&8Lgs3jf2_2Br6HT9NRu#-8ZP(k{fVnc^GO1gb>>P$}0Bl2#7m z_oGlM@Q$aTpCKTKhw+p!1WEy_FcZ)gZ;lPX{Qb`|xJ0i}5`mE=tDOGQj79!GRM$1o delta 2210 zcmZWp3s{U<82;y?W~P~G($Lhe>4Gjp4ApKXBvVvV(=-#ZTdBw{-3Y0sOsQd!4F|bY zx;UfC~B1&i*in>cEGH?{DJ-ujhp0kfn5We?6= z^XM<=y85o|jgu!o&+g1p>B|KV(UzDh{{fQNC)f3CAf+uf>fAHdTFZ>o3@5_=?J*5k zg=5Pj$^xG|R&gg7nI?C>ENsZSr@N$o?96fgNaB^1PhpwMj-G6)f%%3k(3ypa9 z3O8)c5e+KSTLR|q?S7Z!ceT5-Cv)}nR)sWn^KM38pOEQhz_okAZl0`@5PRg+-3zm- z&0i|ZTC11l&bF}q&(@El9{l-I;vIqy$gt&Hk zZzA0_H(>IcQS-pCZG%;Q)e^$+@Lr3DRvhL0Z8C%5_4WJCZC^4cC*OHrL)Fx=x>9_s z{3AK@_k@qKj1DAwX^+9lu|21|V(xdh-ZKqqY#)6Q-_ZTM!mv`Z?a7@d^Z~hvurLni zakO*S1Daj?j(o2|OGPZpNFozFJlVcy?rm?LV_~t7&EYiHmrOjpChEL9`Fh{Q&Emf7 zitwHCvXLgk+@zyzX_KlwW-IWomsp19F1Iz#BIH{RUl0Z5TidQ~n|nEV?QwF~yy&N| zN*6rbm?*tb5|?0hjb3N%9-Y!lZQE_k&f7S|r3C4pv}bX9eD2_0fB4id3*NfnyVQkp zx^qN{?%&(uiRZ{K8EQe zx5bT@UoJ_md?Oul^>xe@XZxb`Aev&B?Rn+Z;;e|VQ>DWrPWMNeBFA(^0$mdMnW(fA zk01aWf&Yhr&>=ktLJ4NjrV*^6y+q(b+fHzVmPO=2yNt+(_Bhc^Ws5qAC-4wR*G++d zF3A@Mot0#{+UcNhA*F~(TSb7}L1UMqYm9I;YHGMn&D+KNSuLds+Z$b1hX;+@)Q)nB zmD(9&v(%1}g&r9mHJx)_9|||ly9t2n67~Dt6MGcni!y$sJt>Gh_4sB? z`g zu#g|Jq^q2d;auW^tWj={jO0UQ-H!dy(B>KSp+aBGSdHQI~c%1wW`n*VCzja`&!XVOA!z9 zye(&ZC83ppjTYYp<9`n`Xz@mIa@B4y(~R<2fr*evBt&&$&G~n6>##>HAR>sKMgc+2 zq}BtWj|o(W@Imp>x>%wdP@zi#hoON~#4 z#3STM;71V^lFQ@<24xtz2D@HwEpi5C+`Z<;#L4jALrLh7bxgWT-NO@Olx7-^7G)HIx3$ohg0S z>^g+W>rD7sz7-6JJfXyVKNR0$3?;ZG)Ndq?<>lKIVYN%K+C8)~tN^$CRzL|8F^Yf5 z#w0AT&P@VQVs&y!Vibo1N5s=0=_a<)2Q6at_vc@1RSt|LY#8H7P<#>(d83sjR=68`goG$tg7g7PDw1|Mj6zk>)wRwW@J;SG|Nhb#gj5J990F;K*Ul*cXL zB3*0cbOAxd018!f3yLiYMYXQph)*C=5am}LURg*p)~X7x843}pQHQ?85_)^E%Ly5Cw7wX^l) z#%zb=e)og=S@orc)wa~Pn+}zl<>d(;vwA z&ik--#l}Hd#GFZbz?mcM3*)J#J!I?Sd%3K&mSqduBPsGX*7L&JGvDQ!ezD0kw1`KU zwfm-MloPzzSbQ^tcpr@k;n7mpM!6q-lgGM#&^oW(qvQu-gh6fivO3d<_L`&z`H7*4 zmeS)TPpPDlsnHC_oPOZfx`m2-~_?(pR_qIt3;rtBei{VCD zAFfyMZHMsL;7sqDr$gu{!&Z~i7I2&8W?j{x>s=h@w|nytH=z7d;W|sdft-raLg@yR z;#^yLzAXLPTS6>5QrG5`qHmwyZg#RlI^IJL)S8>Wcdy)jE}wpFAi7t+N`GURcd(Fm7 zv7~1Gq%5$2>$zR0X()e-=BLkpPB(nEO|L_|)!)xi$Uk28%15kqXB~M9-|P0dMZ)bl0&mB|7Ax*ilggQYywsxp?J<;t*LM50PTZ92qJ==Oi6+%~%(pe8P&? zR<^SZ-Ts$Bti#iA2(lGel0F4D#B@+#{5y1%w{AZUD(}{>R=L(U*+GDFV9`}H&%q6rOW>ku|^JbND)MX`s zfQ=k*lDi4Yn_O=w^|$RONE*IX}jMQTjAfjzwRJ{b2~G^YsICe=V79ZXKo7 zh~b)YoqK!Yj2TVNmhiGYU(#0{Xt?(@>-F;gOr4?*7+#%jMs=SVU3t+oYBCY%YcF=Z z7~LN`qB*6}J3c;KbuC%1fz|67S^wllbz6PybfmG6ai1|rw$@2&sXZFHGPI^Xv5n&E z<1b%#o%>b9Y|5__ccN*!KNi(#S2IO?9DW61X?TJp51WssCusDrfv;%ENA;4hff^AX zXa1p?vQn5l6juVAcqn8B@lgB?0&pYhfjk}+Lgg6rS_v6!{>YYxrDW-2E@=$prQ7Br zz8Ne6>}cz~LtR}ev%u`cN1Qz7&igPtoPR4t&7 zJsPTcGyra%rtm(Ao+vI5TPe%%lRx#aQO(2G(B_B(1<(YlKnw&ZXPAVgH7qG?Uwo8V zczoRZ43QCK^>8IysvEGnmt)!g@V*uDM35*5g7>*AKyhD!QGqdBF@z}Y1YiqQ{VBv& z8$gWE5bos)QG6c@ySWMI7OpV*zvfzs7eOlId}EC03I@bfrjppA;m^>-X$FarViXcR%Yt2JG14e@q>~zvYk|dY4!Xqp@Q?{HivP+ALVZ?L z_Q#119>3_6h+QoNdk;r7Kk2s6=(P7Ln8Rjo6u;d8gN(othn0Zpqk2o!2gQkiz0XD? zWgWFnM|DkOQ7M~dwVF4+GYE4=LD?T>RX!*_)dl3ctWm2ZY_fd#+6aBg2LoALECgD< zhH%=L`=a<3E;dvg|J2+^&0Ggq`uTsrCnMOv;_(;Ay8E$4OQs=6@Dg8H2%ls8XqasE VS@xb(A&=Sl$7D0qi1AQv{1298ZbJY7 delta 2570 zcmaJ>2~<&cnMaL`mPuw3NCJrbl>R^{T1F9+$`C-sFLqJz%3Lh; zQ;QlP6^ft;D3vOF8Kf2}GKd3E7DZ7$*r@!M7mS3}<;!|G=bf|nS^M5|?`aXB8U?5= zfqob)0TMsY(#{rcp(s2}JL`)}eAlanfuNPmh0a?1-lPXss$*&0t5w`VpU)#AF^?Yk zll1AMJiWWen<^|E+}bzMzk1qZ^L58U5i$2UUf8r??={`dTRw2p;P%;>T5=^|v{kj?7csbJ09CVO`qhAlI2>byTaLF&Q&EWK5~4Fys=wZ+kiYSQp{CpEZJ0%U^Z}6H%}2SNiS@+&;0V+7EqdD$l){ZWMR4 z{zZp2-Ax~}n5$C~BDamT!z4Rhr8ePT%}aJZk)@LwhSx?SVn;_d$LRJ&{zFOM%Yqj2 zuv&F$cQ~c~x5HH4wi2SvUYV>k4ZF+8z{e4W z1>^gZ%jbplqYbydX(+J1!;ZR0n{>EySwK;~r&?}k@STSw zN-d2u)I?z^5qB;~iJ_HcGc!^=XdZUiAaju;nADY9;+`X1_(r{Fe0FBXk?x^CUR7wt zkRFDF>~CuEsWLut5OcuHbFT=S^3dD7`DSe+-;+0@p}$P9D%_Lau}ejn^J}&d`n!s>JT%ewT!g;YF(*L@GkG##s>4JPh#0e z;tTtS)8(C34ob)ly!9z#u-h7>^`NZUxJ+6*LH5y=W z4?k{_Ic@ZP>HU7i*8^=4IRPob{)1<}J1OE6%OAoiHw^4pG%0?PI>?+JNUAK-{bRDH zY|!!aUcqE}j)S2v|48f{YczDa@o34MNYs_Pz6>2r%zX=m&QJbgpA<#@$!R(D z+>1aNzoPz`Me#!Xw=-uIbSx}G!rDi#{_2ph-R)=8ARk4$lCRrXK5g3;Gx4%Kytg@a z&iTXLfiDfn#^_?i_CFXy#NNeNiR~FI4f(XgIv_R$D}j|@w>Xh7^Ug9IcI zO0h#B4xq+M9IT;+gsf-mg+WqZnB_|%A`7Mj6JQk}iB#K6OPtfRTM~zDCns?}by$%& zp}S2bPOh^W1_MtOaL&yHvAu3A#Ok@nV>ClSW1;(F>}D9?D;IJpKL=!ZC-QGvDY-1C zdsH3D-?O!Pz_ny*lE>Gh_wO=lO>9;DYq|N@!v zwN8HNB0bUK!hZ+Lx-|WjsS!-)Km|V(S`{Omhu}+}of`%sxaPByxv^hZY4enzdDnP& zaz-Y^Sq82vwvj=UVH5xtUoz6Ca9=X|Dh4QG)fG3;K{cG37pHW9w!h?L%)%im77l3v zB?^wefgy9GWF6P>(k?{jd;(;CsCN!pUAd1l==+AT_A~2&&2cH~5oI(G@qksc9-V zP7#8_%^*lg29pb{8M+8&{O!^16L9Vd{K8P_QAQwyo% zUmoW28U;bK@J(@)spDj|2DU(izkvDqHd6kC4H5XTh)9jFIicf0SPKWYQA;WfAr!F8 zkQyByX<>>xpSWz;p=)?IGZ-WOo#x_D2^)TZQ z=wX|1#4~H1>~#%Pf>u{gyFn1$BgoaPg2ZT%`nlxzcK4@jUM}z8;wHCXfRCaGS*W P?eMP!9zjt);;;V$M~#ih diff --git a/utils/common/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.1.0.jar b/utils/common/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.1.0.jar index 9994cfb559b14b01c7ab25c5e491fd319979608a..d0504f0be478674cdc66f5cb52b8b2f2b3c77148 100644 GIT binary patch delta 1844 zcmaJ>3pA8j9RFs%@n+4U3}X={CXX4fX~xK-Hu6|8C9kHHEq0Y6q}?zc(H4vGMT+v+ z)+0kPC8uJ&qLYotR%pFKnwmx=8*2B?XOEh8&wl58=l{6B-~ZnGzxQ|F#U{rodAh+k zc}V>BdYyeZFa1fv?|~t8 z@Wh~~-@!Gp&b=UeesO*%^0zG6^Oj-Llv1nHdX8TBg`u1hRl9Elb90-Tcdh&Bfz7zjMkWb{#5~Pw9x2e9L1EQ zF~KNnOCZLNl?KW!aO0nTJ3el)N0$`T65e`dXlG-N&a+lz28m2mWA1ftfCXH6jLk|L|DFc100g>*fV(&XTa= z*{`Qq8cPmSmKq$j9;tvEGPKUkQup1=hz^MbOcNH?u3N*Jb?yZc4H?&$cNXza6php> z^;f7LdOH!O=kV0D{EC~em057-8w#=dU3(W-MAn-!@C#ezlcW%~#VZgSvQvR2IAO)) zy`GeJmJ8zLgp3+>GX0p{wX+_a%FY&QS(TZvAbNnU(yaPSrQ@$CLH_K~yYGle9fmy- z@IZ>0{aHbL?|E`k2J!XrV39lL{vClDNV_=)m8zx3{oMVqqA8%1BnE9FFTp4pZW{*?v`&T{w#`0a6Rb2?_)aIlYVFVe| zdiOD^jnd%d*Zk?n^RXhG4_6o5Sp);?z+$TpuSKmpOhIitY=zpZFax!(;EkxI;hf>h zpmKf!&JT+^kEIu7QD;CoPg(3B(K?sJP`;sVE*5o~^m3LRU((>RLo;&0ppvOmD0`I$ zBqJwvQafuljHF+1JR;cd5Iqru!EHkx8mU;q2nUx*TF*}l#h#j%21GCe1R0x zG|}U0q!F=BFv2CTmRJj*XRjw>kBCQz2z|Yx?xzK{B??{=19eUQZv}=Dgy&)c8R`Fz z$|e#(a9munnem6pvJkjau0h}H=Uak(%Y#;Y*c$|0Nc;iWMb|{{-BCIb^8h(bH$bCC z5cMGgbR9HW0?|Q)z|cm!z+e!uYjnXx7RZkb0=m~7O>30tV1jK@5VUb+MdAsr>!H9I zhBgY!n%?K10Dy%6=pX@Pkx<|n68@v!%0i%%NTLl9OEp4=@7XM$gb<`+$slEiY+)|< zo2@vLEdnK@F%V=cQTl)h$}MFQ(DgSniP&qVXbNwuiAurLmWYu#ww^1aPdZ(vXdg zb__SYt7q+VtdeoH;yD@iQ`_%vZk)oDdsA@#{r%tP$-0?5*PL?dC+nssO8p)0tG1P@ z-kfsYai5%h%7>t-DZv}oW^;x(KKe3UaHp2WY$nNzmQR+%KlNnuSfOUKeQn)QdDH}1zI&ZpLA*SJ=I&- z_j0G&|2$LwmYx&BX)*4-uB*gCn0~4S+&_@>x-2Zkz`1{M@Ee1&`{fAl%*SLGv~%$06R2Q3+_e!Y#m@UnGoUj6c` zAHPbKIG+pGnE9mDXl3%cv#-8+Xf4T{@mzS(g_Ax?Ym1gWGN z&V9P*&`a0pyh{?!*^223%wKk3%7pax6kZ2zz1K#sa+D=@y>a^XYG%aVJ-IBe_OOap zah^VQwC3u+)Wciu-`M@B*0b2G*JEMf?1SsW_WWmhJX`yCzsDrCI);LKuMJ=HO_t+6 zyy5QKFHy-H^_p`whwZ-my2bPK2YcCX-)_V`{&#H8!I|~94(M&%+22+kXMe?)X z3vXIG-`{r0>i9;p)xY}pZQE9!<8WoA$oa4kwNF$0`2uoZ3;qj}nSaHUt7qrGLl4}} z=(cx@{C9VF^wjQW(I&gydlnZ<@%#CkMeVz7d)3O{DMj$2{J{gcau3Z_KK$8M{;oRi z`0HEKa{n?T(kw8sPUc{iX9m$)%xYjdm{}i8w=$c6={?MrVEPBM6PUJTac6#BJ!Nw{ zOC%GR@tEy52bl4Ye+EB@F}X&5JxJ+hJ%wpZV8(H!sSrlI>TL+aR>PAKEY+#`9wJh$ z;|38qqU+Dd49rcF#q_nnw6lH?n4Y0u%q$f#c{88E5f)BRKC{gB^J<%X+oZnUC2-*d zNAbdfP4*L(rAx2bS+rB+*Y|DBk00+1ziX&ieK4+kb=@!4$*XQ{eDV6suY|4ESKY&R zZ{D}}R_|R`-4m8USvnRQvJ$jDFV62^|FD@~+3C2;{7hek8LSenxGW{laqGT{wSfY^+f3R@j2R-ASdf^Uk&3Rw3#=AOh`>1l3~wFnCqFP#mInn0To}lZgmXY@ z3V~|GCoj-poor{$1CB>ub7{sEV9^G1F|f!?pvV)bI^oIpq3Yh5OEYP*Onzu42GlMw z+0uf?1{lX)NtMd8je)-5Vqsw5hFitJu%xjVD3Y95l9-d9u9uXbi|{H311NDcT1YdV zpFGz>MIL0#mMxFka`_n;;&d1o1W=4QJNbcyJXp=Wzg5RtfpXh{7^oBumNaHema~)t zyWQSWnn_9l>Pnf(3sl)APq7pNORWS-ZB{~(;+o84B?6X`w324>Rz;E$m|S7SqW}s@ z+YK2JAAnZXGcqvP!L0=OPg4V~p-zKO2ibohjoY}Ff1Cjf7*19O1{)NOrJ8V!r!;vc z`&mOGBGy`(DP0FFHNn~hZ288$6U)8<^)_HwzCjmmhM0|%JTRM}L^>jP1Q`DRXPWG9 zBL~hp1vb)*`jbD}SZO1i3$C(IbVtCQ3)0;JRG$vYaFe5Lr5S$#dC`+E*$RUl2+nyx d)pLM24Xz6mItNWA%NoQ@PPUU`J8cS*0RW>uz~le` diff --git a/utils/common/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar b/utils/common/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_0.2.0.jar index 3d1ce7b1c8baa9cb9c4204ff1d90caaacc40e443..872b412bc988b86e2bd2aadcdb132a17f2a6a824 100644 GIT binary patch delta 2717 zcmZ8i2|QG58=uX-%rFuSSwfe|Hb(YlWF5;`QrQcY>}wJ-bS+u39SYg<*_tG>raN6m zi+dx{<}$WO&CQn3CFMIaO6kn+oM+zuv%T;0K1*@}v?u{8U~36S@IqKmgJ(^)fSf?R zXN`rLQD6LX7zBdPJ|IOIBBE$6ixkI@8ev60ek)xT{oM+T)JtIbz zchUcxukBQbL1fvi^hAGL>FtT?M%YNdTGFa1vJ^t~g#^4Oo1hQCo4>9X(WA2grQ

h`n=)co6#n|LT*fWEn<{OA!p-e=Z>eGb zoZNk{K?VE4EPAE$OY;n&ZgO?!=()+)#6D_C3~ably06{3`U1Zv)AtgwWKY@2yP0{! zl1qZ6NkehDqWL3wBZo`at+-TE(yT(}f!=ixm{<>jI zb>hzY`KW+T!e(FornM*!uOWY={Vg!x+4=wNg8*qzsJH5O2@VJZ6agII3UC%Z4_9T= zTW~y^PQ!KBR2FfNO}!Cj6qZQ=s@3ncUQudK9&$SaXfAu4stYd)|Sd+ zHe2{Czn!t!LW*aaUP=?M;2){O`k}dcD&R2M8MS! z?vWG_68VKj;e1;QlM$G~@Aihr-ggvxZPBzSJ85-al%e#QK}3uViH8P=H8(nQb@%#U z)HK9|7_lu6MH-n+`z(`R_)A_NU>c5wlSYh|*6pQ7i7Kt+RJJzz+eRR!Q1!v5ceBi5;2mW{?ErhBBy%-ZUA?8kdryiLx z{=)61D`9b327zruu5+m_!b)S6JC?_||C?GxdJz{>MJLRz)xygEx(VbLF|NVk(I$nF z;qmHY3;7i(;g9SS9OmqJqK$YRbYAVZwdO~+qjj{FO$HKXn4z4%H|H!G+(RrS?b2z3hJA=~+r*rXfB-h;j46xX)o zTVoEv!x~iHHkh}GfV=h?3SA9F{1jtS7M6h*w78a*7XGe+kX7Z)?_1|um$*)w9bXxz zy<({CiSBf=Ih+~WRa=<7m~nVGEzs@l(A?D0^4F(NTD+TOt{C5G9q*uaH>q?n&}Iv1 zNY#Xtl+n^c!+LHtQD>dwP$hN6zMPT+-S^h@!osKPr!CVt8|6rjLeIgL0)>gdw`M7D zLI=Nnm;#kLnyeTABRZI!%JA=P=7$!Ght$PDZ2+w9E@@V5er`-6TMHCq)-Vzds3Syy zXkC7`$y8kojLL#A+xEe-*blNMSS)EkP;XOOQV#=@M1T-hX`A zP6|LmAnxE6P}^k6!KJl{gP}5sY^rKSGBP0_GWpfR=$EL6iJ(?1_<}~Zo{D$_>7TU8 zHV&ZvugVgGSu-1Suy1>CLLmBEx~%=pOM?;62E+`6*%4PZz`*$AK*$)#FxU)hF$k3_ zfRH}0@=c2G00uUu2tqi(`M~C#iu=%S=Yt-2z{#@RLp+u(cn8b>(1C|5~tXGW<6%uy2g7vcP`hFZ81=-k$#Qk*i0FQ{1f#87qareI}Yi*sJrCbT1 zh^FiUbRuG)Yv4*3DASe!o)Y<4soW1Ad%zbme`OeJEd~`pDhszgFO<^50sxBV^(9F;hJ3fB-%C2_J0uIL@R0yJo{+i5WV!~|5oJ9GqA=Z2 z#qx4T`nS^r+W1H5#ti?_sO9j{-ji1LsQs9~tP{Vh;4t!c4K@ub`D$P-GJ z9e_`rb}jLbA`l(6x<=m_ey1J^$#V(V5FJ|M01v8h8<1=%x z1+_LZ){=giW}RoJNTEH_xo}T&>T4anNAt~%Z~t`Uo-^!pzQ3QQc9_C*y~0lI^=6y}L7?MUn7(c-o?4`u0so|2x8@Tg|D{(Eg;quAitgv_Z$5zO`O$fys& z2B)|Z*{u7r7HTIl1_l~L>Yrxi&uo~u5Iw82ZM4HHI0$Tgf+Qqf@lWK+^DAMnPWPm` z{S_0#WdD!(A-mEDqIddcRZ~i9DAen>UJ7dKeQQh*{Lp-_NJKqE?vK`T=OhQmov(zA zyG)UWd8bq7nsw#*mo)s7p4pi@vWgRY^d_ESH)$y&r9de=3k*BJGtO7TB z8axGBP0S5eY^x=;w9qp4wXIqRnm589uiWBW2k&pg56pjv8eKuSag8iep1waBcpSfE zW47WoBSt1EJq^D}f6)K@{IxlG{lfVWTLReIEYoo8gU)>$zj{GA+A+yKdMwb z#Wh5hINC>GHn7}nQ6&z4W@ku*J0W{2xF!VLYR{vJnpvU z)Nnqe4=Hv4*ak_*$LqM)u^uYN<-t00gkYSiwb08 z3+?5h4e=R%P5IPHG{_>#sxW$bOCF^6SAVkvzsvjQSp#X2&C{rARV@2^{0HH-x)!%}FYz8>6U(OL)?qYHQD*+s zc#<;W7H{jYDmv0MWwH2~HLOpt*TQP;-Q)Bas#E~Btli{>sqquv#y29yWutbM5&Ln^ z5>zm@`?>8Y+Y_oEnySUqXP4OGE;`NqR#kdA_(9mkgY1~tHPWh+mDh>cFH*(IE;9zz zRC$S3j+>Y5j<=lP^eFJRG^2>dSG!z6oq|c_UC2HrG&aqX(s{Dmx3ze0f|}v$=DppY zEj@V8Nmb^+=IfgzR`Xt^x<#AM-g!7vs*!_7&Bq@$T*2Sy-FMVnWXV z$i4-mk9?IKp4_pSx!1aUX2N0CoXXR_c696l$#99l*7MTBAOfqe@3=wBhtCGEj#gRQ z@=M=te(7zKG%sZB7+U|<(6Vavf5HY9U)jHFfCGSfxh|buf|JqQbya*a6$)TXPrBm5=20}pd2{nZ^Y1FWDN=^N;IaTs5aV?Ux@9#j=L zy#;&tAqeS7(+-1UOKzPUCI7F44HN-F59`Nnekzc{;jb zF6!HhQ8XDlDbxzw))K}d2>j! zT~&O&7XsR|0)X)r;EFT^w3TF3GS@)d}aXXX&}ZzR$=m08Hh5~ z(PJP6^9E-OB$!{EhGHxkY9LK*A98CK20{muYLHf3w_HA?E1bXtmT(;EKW!cW@YqtD bE68UQi2k?aeu7G3EVchHKv6bF9mc`mvZNaY diff --git a/utils/common/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_evil-twin_0.2.0.jar b/utils/common/src/test/resources/brooklyn/osgi/brooklyn-test-osgi-more-entities_evil-twin_0.2.0.jar index f00a115a52680a15003cca56a0b96a62ff06fc7c..c777bfbbbab2f5cd648eabbddd81587e010a7503 100644 GIT binary patch delta 1645 zcmeCqo1DiR;LXg!#Q*|Zl2%OQRpH&7w8Bf3VgBN|ObiS=C)&x?``#u+F#GPtd!@RC|ZKI^eiv-b$tLJ=_oM0H;pwb+qCaJJs z4%bVo=Ae{ir;ir*~q>BJU2z z=`2eG-5fUZ-nj8)?}cCY`%byM?66sM{8-kSpv@^#C(K*_gwA?>=geoGXDbRfbn713 z);o8@SI7KG9U8V@BT|pf&R9Bc^VBao+m|m4So$oWV$}>TelewD*-t6clCD2kSYOJU z;^e-@AbDy+FvHX4y#3OTF267Rvu-x;e}ym#Dc znfN@H&3&rc($bA9=EiT+dcJmUu55vEv-` zA6?dc?AJXE1ix)=P+oE4q0&)%#*8~B7VEp8z57%*dP7A;&6a06TzftzpZ>feL`?Aa zQ?9*y`+h8xFZmlh=hMvkcUJ2@^CJ>4Fy&7E#U#%RqGg!Xz_cr~KA0|NHUZP?m@UEd zD`qD!tFw3rs)Phyl}XnzhWaTNiCUrpd<23Cd8dJ=y3`hYxwqF#zB}B+=Q&yJ%ro`?Z$>5&W_aG8{6^mdHSM={|UP`w#g_b)?fCT*6<`i9bz za}|V=mGvDWg~h3Pkal+Keqfwx{|a831m3fkglS delta 1741 zcmbQ7*PX{3;LXg!#Q*~Ds;5lkRpEVGJ;f`*E30q{69dDBiFR`J!M7Yl_NS};Kg`$O zcW%Z6mKK5I%@Ma=HI(+eUCN$*bI*akMDx2j4*!1d^HtCaci@0|FDt~lG?R@<)t&+X!_xW?Q&ozFWV!Z0&*~%wY%^iPw_7@wieIm5T zP?d4v&SwE{gl5f{Cq3iO?$C)Iq7&WE?C6v_9kPA}(LEa%KtQ3^U66XjaC-0Rfy9Nn13Z?!a1R!WGh(@DG2s(L=B+vcU{eg2A^_8ot$)3w&Wq)PGan+;_9{<+-0+tid+dL%9vj(|p&fOkMSAUbp1M7a_f> zx~#?pvkLdtU7h&m`e6&D;Kisb~|q4+cIY9VzxyM{~vDJ6|?2I`Sr!SE1o>? zi2b`r`d>=%?z2~#mzB7S&(FzY?z!H*(q;Lq`hquM+g#bhlV9~!zS3F3lz#r<3)S3* z@BK5rO!8hjxBAq)mv0t#eBGIICwB7FMMqw`PS;(MaL!ijtl<1*2eyQyx2I?~2*#MF zUa9O(c(JK*i{7--Pp8AC?%7(t*4dSHw!4b?j<;uCy?wLa^JBmE@qUjLn+d&Q@xDWBhbnx>a&V`Y0Y>u28oD=%!kX1cl_soB7^z4?9hgP*bQ z?|k1g@A`J#+rQWm=@yh=!3h{jumI`&qI7)}6_Zt&)tG@QC;Kt$gXsol6EMA<*%C~D zW_AM87A)={dUF#?Boml%k4=sf%=phggCDG9mi&6KfRDm7h?1vDQy`2QRTpMvU_P3B zSVId;f6#~l)Bc*Z%u*4PH=oyJW90-Y@Or$OUAK4gJ-xbmu4V_GhKGOWJv#8SDXL4s zVvG073oG9}y%OU5RcU9&$G5AUkIVnl-Pd8!lN%9pLvf8Gn#vHfv#JF}DWnfZm>3nWBYl=UwqvGmz8t~1&9 z#rt4IVAb!i@;~D51l~#gxO1e5uX&P?+h^tgZ$>5&W_W2Z`HQ{@YH={x)W8%R1i1#% z;%|Wk1SFghxA?Sb*|1n3PLxL%O^%qHg>dxHaZudy`Kc_pCFEhg(I^GudAf%sn6M4Bm0 z6)fCjA_h*SvrVKK*G}GUBBuaKqDaOuIKnvs4FCV%(wO{T!*#NiDHn#qBou{~nm~ol zlh2tcPCjAEqX5hSUU&B?RW<=#>%+&ukO@}|a`8@>V%5npX8d52!MOyeXc-U}peR11 N2Q}HA?XEsZ1_0csoGJhS From bb7c04dd1d49bdabdd76390bff0ee27b7ff1a62a Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 5 Jun 2017 11:05:00 +0100 Subject: [PATCH 07/13] address PR comments - the straightforward ones --- .../core/mgmt/ha/OsgiArchiveInstaller.java | 14 +++++++++----- .../apache/brooklyn/core/mgmt/ha/OsgiManager.java | 8 ++++---- .../brooklyn/rest/resources/CatalogResource.java | 3 ++- 3 files changed, 15 insertions(+), 10 deletions(-) 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 4044f231de..408856462c 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 @@ -58,6 +58,10 @@ class OsgiArchiveInstaller { private static final Logger log = LoggerFactory.getLogger(OsgiArchiveInstaller.class); + // must be 1.0; see bottom of + // http://www.eclipse.org/virgo/documentation/virgo-documentation-3.7.0.M01/docs/virgo-user-guide/html/ch02s02.html + private static final String OSGI_MANIFEST_VERSION_VALUE = "1.0"; + final private OsgiManager osgiManager; private ManagedBundle suppliedKnownBundleMetadata; private InputStream zipIn; @@ -161,11 +165,9 @@ private synchronized void makeLocalZipFileFromInputStreamOrUrl() { } zipFile = Os.newTempFile("brooklyn-bundle-transient-"+suppliedKnownBundleMetadata, "zip"); - try { - FileOutputStream fos = new FileOutputStream(zipFile); + try (FileOutputStream fos = new FileOutputStream(zipFile)) { Streams.copy(zipIn, fos); zipIn.close(); - fos.close(); } catch (Exception e) { throw Exceptions.propagate(e); } finally { @@ -228,7 +230,7 @@ private void updateManifestFromAllSourceInformation() { throw new IllegalArgumentException("Missing bundle version in BOM or MANIFEST"); } if (discoveredManifest.getMainAttributes().getValue(Attributes.Name.MANIFEST_VERSION)==null) { - discoveredManifest.getMainAttributes().putValue(Attributes.Name.MANIFEST_VERSION.toString(), "1.0"); + discoveredManifest.getMainAttributes().putValue(Attributes.Name.MANIFEST_VERSION.toString(), OSGI_MANIFEST_VERSION_VALUE); manifestNeedsUpdating = true; } if (manifestNeedsUpdating) { @@ -333,7 +335,7 @@ public ReferenceWithError install() { if (!updating) { synchronized (osgiManager.managedBundles) { osgiManager.managedBundles.put(result.getMetadata().getId(), result.getMetadata()); - osgiManager.managedBundlesByNam.put(result.getMetadata().getVersionedName(), result.getMetadata().getId()); + osgiManager.managedBundlesByName.put(result.getMetadata().getVersionedName(), result.getMetadata().getId()); if (Strings.isNonBlank(result.getMetadata().getUrl())) { osgiManager.managedBundlesByUrl.put(result.getMetadata().getUrl(), result.getMetadata().getId()); } @@ -346,6 +348,8 @@ public ReferenceWithError install() { result.message = "Updated "+result.getMetadata().getVersionedName()+" as existing ID "+result.getMetadata().getId(); mgmt().getRebindManager().getChangeListener().onChanged(result.getMetadata()); } + log.info(result.message); + // 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; // that seems fine and probably better than allowing bundles to start and catalog items to be installed 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 9936ba956e..6b9c526253 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 @@ -103,7 +103,7 @@ public class OsgiManager { private Set bundlesAtStartup; private File osgiCacheDir; Map managedBundles = MutableMap.of(); - Map managedBundlesByNam = MutableMap.of(); + Map managedBundlesByName = MutableMap.of(); Map managedBundlesByUrl = MutableMap.of(); private static AtomicInteger numberOfReusableFrameworksCreated = new AtomicInteger(); @@ -220,13 +220,13 @@ public Map getManagedBundles() { public String getManagedBundleId(VersionedName vn) { synchronized (managedBundles) { - return managedBundlesByNam.get(vn); + return managedBundlesByName.get(vn); } } public ManagedBundle getManagedBundle(VersionedName vn) { synchronized (managedBundles) { - return managedBundles.get(managedBundlesByNam.get(vn)); + return managedBundles.get(managedBundlesByName.get(vn)); } } @@ -282,7 +282,7 @@ public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { if (metadata==null) { throw new IllegalStateException("No such bundle registered: "+bundleMetadata); } - managedBundlesByNam.remove(bundleMetadata.getVersionedName()); + managedBundlesByName.remove(bundleMetadata.getVersionedName()); managedBundlesByUrl.remove(bundleMetadata.getUrl()); } mgmt.getRebindManager().getChangeListener().onUnmanaged(bundleMetadata); diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index 730482a2a4..57c4ead40c 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -181,7 +181,8 @@ public static BundleInstallationRestResult of(OsgiBundleInstallationResult in, M //RegisteredType ci = mgmt.getTypeRegistry().get(id); CatalogItem ci = CatalogUtils.getCatalogItemOptionalVersion(mgmt, id); - CatalogTransformer.catalogItemSummary(brooklynU, ci, ui.getBaseUriBuilder()); + CatalogItemSummary summary = CatalogTransformer.catalogItemSummary(brooklynU, ci, ui.getBaseUriBuilder()); + result.types.put(id, summary); } } return result; From 3cf028a6aa811696ddb60d3e55a746de4a40ff3d Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 5 Jun 2017 11:23:23 +0100 Subject: [PATCH 08/13] utils for working with VersionedName better --- .../coerce/CommonAdaptorTypeCoercions.java | 2 +- .../brooklyn/util/osgi/VersionedName.java | 7 +++ .../brooklyn/util/osgi/VersionedNameTest.java | 43 +++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 utils/common/src/test/java/org/apache/brooklyn/util/osgi/VersionedNameTest.java diff --git a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java index 03745e614a..2bc2a98abb 100644 --- a/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java +++ b/utils/common/src/main/java/org/apache/brooklyn/util/javalang/coerce/CommonAdaptorTypeCoercions.java @@ -41,8 +41,8 @@ import org.apache.brooklyn.util.net.Cidr; import org.apache.brooklyn.util.net.Networking; import org.apache.brooklyn.util.net.UserAndHostAndPort; -import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.text.StringEscapes.JavaStringEscapes; +import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.time.Duration; import org.apache.brooklyn.util.time.Time; import org.apache.brooklyn.util.yaml.Yamls; 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 2d89be278d..5807904027 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 @@ -76,4 +76,11 @@ public boolean equals(Object other) { return Objects.equal(symbolicName, o.symbolicName) && Objects.equal(version, o.version); } + public static VersionedName fromString(String nameOptionalColonVersion) { + if (nameOptionalColonVersion==null) return null; + int colon = nameOptionalColonVersion.indexOf(':'); + if (colon<0) throw new IllegalArgumentException("Versioned name '"+nameOptionalColonVersion+"' must be of form 'name:version'"); + return new VersionedName(nameOptionalColonVersion.substring(0, colon), Version.parseVersion(nameOptionalColonVersion.substring(colon+1))); + } + } diff --git a/utils/common/src/test/java/org/apache/brooklyn/util/osgi/VersionedNameTest.java b/utils/common/src/test/java/org/apache/brooklyn/util/osgi/VersionedNameTest.java new file mode 100644 index 0000000000..c838258115 --- /dev/null +++ b/utils/common/src/test/java/org/apache/brooklyn/util/osgi/VersionedNameTest.java @@ -0,0 +1,43 @@ +/* + * Copyright 2016 The Apache Software Foundation. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.brooklyn.util.osgi; + +import org.apache.brooklyn.util.javalang.coerce.TypeCoercerExtensible; +import org.osgi.framework.Version; +import org.testng.Assert; +import org.testng.annotations.Test; + +public class VersionedNameTest { + + @Test + public void testVersionedNameFromString() { + VersionedName foo1 = new VersionedName("foo", new Version("1.0")); + Assert.assertEquals(foo1, VersionedName.fromString("foo:1.0")); + Assert.assertEquals(foo1, TypeCoercerExtensible.newDefault().coerce("foo:1.0", VersionedName.class)); + } + + @Test(expectedExceptions=IllegalArgumentException.class) + public void testDoesNotAcceptInvalidVersions() { + Assert.assertEquals(new VersionedName("foo", new Version("1.0.0.alpha")), VersionedName.fromString("foo:1.0-alpha")); + } + + @Test + public void testManuallyCorrectingVersion() { + Assert.assertEquals(new VersionedName("foo", new Version("1.0.0.alpha")), VersionedName.fromString("foo:"+ + OsgiUtils.toOsgiVersion("1.0-alpha"))); + } + +} From f4205fde2c380a3193953c5305e398f974069111 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 5 Jun 2017 11:23:37 +0100 Subject: [PATCH 09/13] record VersionedName instead of string for containing bundle, as per PR --- .../java/org/apache/brooklyn/api/catalog/CatalogItem.java | 3 ++- .../org/apache/brooklyn/api/typereg/RegisteredType.java | 3 ++- .../brooklyn/core/catalog/internal/CatalogItemDo.java | 3 ++- .../core/catalog/internal/CatalogItemDtoAbstract.java | 7 +++---- .../apache/brooklyn/core/typereg/BasicRegisteredType.java | 5 +++-- .../org/apache/brooklyn/core/typereg/RegisteredTypes.java | 4 +--- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java b/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java index f3440d9cd5..a9c93fab47 100644 --- a/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java +++ b/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java @@ -38,6 +38,7 @@ import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.api.sensor.EnricherSpec; import org.apache.brooklyn.api.typereg.OsgiBundleWithUrl; +import org.apache.brooklyn.util.osgi.VersionedName; import com.google.common.annotations.Beta; @@ -129,7 +130,7 @@ public static interface CatalogItemLibraries { public String getSymbolicName(); - public String getContainingBundle(); + public VersionedName getContainingBundle(); public String getVersion(); diff --git a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java index 8225187173..8385355b25 100644 --- a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java +++ b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java @@ -26,6 +26,7 @@ import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.objs.Identifiable; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind; +import org.apache.brooklyn.util.osgi.VersionedName; import com.google.common.annotations.Beta; @@ -38,7 +39,7 @@ public interface RegisteredType extends Identifiable { String getSymbolicName(); String getVersion(); /** Bundle in symbolicname:id format where this type is defined */ - String getContainingBundle(); + VersionedName getContainingBundle(); Collection getLibraries(); diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java index 2c1bec64bc..805513d72c 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java @@ -31,6 +31,7 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.objs.BrooklynObjectInternal; import org.apache.brooklyn.core.relations.EmptyRelationSupport; +import org.apache.brooklyn.util.osgi.VersionedName; import com.google.common.base.Preconditions; @@ -199,7 +200,7 @@ public String getSymbolicName() { } @Override - public String getContainingBundle() { + public VersionedName getContainingBundle() { return itemDto.getContainingBundle(); } diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java index 4644d8d901..2fab74e8f1 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java @@ -37,7 +37,6 @@ import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.SetFromFlag; import org.apache.brooklyn.util.osgi.VersionedName; -import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,7 +52,7 @@ public abstract class CatalogItemDtoAbstract extends AbstractBrooklynO private @SetFromFlag String symbolicName; private @SetFromFlag String version = BasicBrooklynCatalog.NO_VERSION; - private @SetFromFlag String containingBundle; + private @SetFromFlag VersionedName containingBundle; private @SetFromFlag String displayName; private @SetFromFlag String description; @@ -124,7 +123,7 @@ public String getRegisteredTypeName() { } @Override - public String getContainingBundle() { + public VersionedName getContainingBundle() { return containingBundle; } @@ -355,7 +354,7 @@ protected void setVersion(String version) { } public void setContainingBundle(VersionedName versionedName) { - this.containingBundle = Strings.toString(versionedName); + this.containingBundle = versionedName; } protected void setDescription(String description) { diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java index f345ec9625..92516593f2 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java @@ -29,6 +29,7 @@ import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.javalang.JavaClassNames; +import org.apache.brooklyn.util.osgi.VersionedName; import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableSet; @@ -39,7 +40,7 @@ public class BasicRegisteredType implements RegisteredType { final RegisteredTypeKind kind; final String symbolicName; final String version; - String containingBundle; + VersionedName containingBundle; final List bundles = MutableList.of(); String displayName; @@ -85,7 +86,7 @@ public String getVersion() { } @Override - public String getContainingBundle() { + public VersionedName getContainingBundle() { return containingBundle; } diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java index 72feeddedd..acefe6b02e 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java @@ -48,7 +48,6 @@ import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.guava.Maybe.Absent; import org.apache.brooklyn.util.text.NaturalOrderComparator; -import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.text.VersionComparator; import org.apache.brooklyn.util.yaml.Yamls; import org.slf4j.Logger; @@ -171,8 +170,7 @@ public static Class loadActualJavaType(String javaTypeName, ManagementContext @Beta public static RegisteredType setContainingBundle(RegisteredType type, @Nullable ManagedBundle bundle) { - ((BasicRegisteredType)type).containingBundle = - bundle==null ? null : Strings.toString(bundle.getVersionedName()); + ((BasicRegisteredType)type).containingBundle = bundle==null ? null : bundle.getVersionedName(); return type; } From 7260bf9cf3f3ebaaa790693e1b7217a81bef78a7 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 5 Jun 2017 11:24:03 +0100 Subject: [PATCH 10/13] Revert "record VersionedName instead of string for containing bundle, as per PR" This reverts commit f4205fde2c380a3193953c5305e398f974069111. The "containingBundle" might, for a transitional period, need to support a reference to a catalog item instead of a bundle, in which case OSGi conventions might not be valid and we want this to be a string. TBC. Will add comment to that effect in the code shortly. If/when reinstated we should also ensure the deserialization accepts strings (should probably also give a custom serializer so VersionedName is always written as a string to prevent bloat in the serialized format). --- .../java/org/apache/brooklyn/api/catalog/CatalogItem.java | 3 +-- .../org/apache/brooklyn/api/typereg/RegisteredType.java | 3 +-- .../brooklyn/core/catalog/internal/CatalogItemDo.java | 3 +-- .../core/catalog/internal/CatalogItemDtoAbstract.java | 7 ++++--- .../apache/brooklyn/core/typereg/BasicRegisteredType.java | 5 ++--- .../org/apache/brooklyn/core/typereg/RegisteredTypes.java | 4 +++- 6 files changed, 12 insertions(+), 13 deletions(-) diff --git a/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java b/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java index a9c93fab47..f3440d9cd5 100644 --- a/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java +++ b/api/src/main/java/org/apache/brooklyn/api/catalog/CatalogItem.java @@ -38,7 +38,6 @@ import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.api.sensor.EnricherSpec; import org.apache.brooklyn.api.typereg.OsgiBundleWithUrl; -import org.apache.brooklyn.util.osgi.VersionedName; import com.google.common.annotations.Beta; @@ -130,7 +129,7 @@ public static interface CatalogItemLibraries { public String getSymbolicName(); - public VersionedName getContainingBundle(); + public String getContainingBundle(); public String getVersion(); diff --git a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java index 8385355b25..8225187173 100644 --- a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java +++ b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java @@ -26,7 +26,6 @@ import org.apache.brooklyn.api.objs.BrooklynObject; import org.apache.brooklyn.api.objs.Identifiable; import org.apache.brooklyn.api.typereg.BrooklynTypeRegistry.RegisteredTypeKind; -import org.apache.brooklyn.util.osgi.VersionedName; import com.google.common.annotations.Beta; @@ -39,7 +38,7 @@ public interface RegisteredType extends Identifiable { String getSymbolicName(); String getVersion(); /** Bundle in symbolicname:id format where this type is defined */ - VersionedName getContainingBundle(); + String getContainingBundle(); Collection getLibraries(); diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java index 805513d72c..2c1bec64bc 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDo.java @@ -31,7 +31,6 @@ import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.objs.BrooklynObjectInternal; import org.apache.brooklyn.core.relations.EmptyRelationSupport; -import org.apache.brooklyn.util.osgi.VersionedName; import com.google.common.base.Preconditions; @@ -200,7 +199,7 @@ public String getSymbolicName() { } @Override - public VersionedName getContainingBundle() { + public String getContainingBundle() { return itemDto.getContainingBundle(); } diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java index 2fab74e8f1..4644d8d901 100644 --- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java +++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogItemDtoAbstract.java @@ -37,6 +37,7 @@ import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.SetFromFlag; import org.apache.brooklyn.util.osgi.VersionedName; +import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,7 +53,7 @@ public abstract class CatalogItemDtoAbstract extends AbstractBrooklynO private @SetFromFlag String symbolicName; private @SetFromFlag String version = BasicBrooklynCatalog.NO_VERSION; - private @SetFromFlag VersionedName containingBundle; + private @SetFromFlag String containingBundle; private @SetFromFlag String displayName; private @SetFromFlag String description; @@ -123,7 +124,7 @@ public String getRegisteredTypeName() { } @Override - public VersionedName getContainingBundle() { + public String getContainingBundle() { return containingBundle; } @@ -354,7 +355,7 @@ protected void setVersion(String version) { } public void setContainingBundle(VersionedName versionedName) { - this.containingBundle = versionedName; + this.containingBundle = Strings.toString(versionedName); } protected void setDescription(String description) { diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java index 92516593f2..f345ec9625 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicRegisteredType.java @@ -29,7 +29,6 @@ import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.javalang.JavaClassNames; -import org.apache.brooklyn.util.osgi.VersionedName; import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableSet; @@ -40,7 +39,7 @@ public class BasicRegisteredType implements RegisteredType { final RegisteredTypeKind kind; final String symbolicName; final String version; - VersionedName containingBundle; + String containingBundle; final List bundles = MutableList.of(); String displayName; @@ -86,7 +85,7 @@ public String getVersion() { } @Override - public VersionedName getContainingBundle() { + public String getContainingBundle() { return containingBundle; } diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java index acefe6b02e..72feeddedd 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/RegisteredTypes.java @@ -48,6 +48,7 @@ import org.apache.brooklyn.util.guava.Maybe; import org.apache.brooklyn.util.guava.Maybe.Absent; import org.apache.brooklyn.util.text.NaturalOrderComparator; +import org.apache.brooklyn.util.text.Strings; import org.apache.brooklyn.util.text.VersionComparator; import org.apache.brooklyn.util.yaml.Yamls; import org.slf4j.Logger; @@ -170,7 +171,8 @@ public static Class loadActualJavaType(String javaTypeName, ManagementContext @Beta public static RegisteredType setContainingBundle(RegisteredType type, @Nullable ManagedBundle bundle) { - ((BasicRegisteredType)type).containingBundle = bundle==null ? null : bundle.getVersionedName(); + ((BasicRegisteredType)type).containingBundle = + bundle==null ? null : Strings.toString(bundle.getVersionedName()); return type; } From 5fad03d9770e339a94e1f554c60370315c580f0e Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 5 Jun 2017 11:27:30 +0100 Subject: [PATCH 11/13] notes on containing bundle being a versioned name --- .../java/org/apache/brooklyn/api/typereg/RegisteredType.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java index 8225187173..0696dfc57a 100644 --- a/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java +++ b/api/src/main/java/org/apache/brooklyn/api/typereg/RegisteredType.java @@ -38,6 +38,9 @@ public interface RegisteredType extends Identifiable { String getSymbolicName(); String getVersion(); /** Bundle in symbolicname:id format where this type is defined */ + // TODO would prefer this to be VersionedName if/when everything comes from OSGi bundles + // unrevert 7260bf9cf3f3ebaaa790693e1b7217a81bef78a7 to start that, and adjust serialization + // as described in that commit message (supporting String in xstream serialization for VN) String getContainingBundle(); Collection getLibraries(); From e1b0be1847550a40e2fb853885fca35b09a03df2 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 5 Jun 2017 11:52:47 +0100 Subject: [PATCH 12/13] synchronize osgi bundle managers inside dedicated package-private class --- .../core/mgmt/ha/OsgiArchiveInstaller.java | 14 ++-- .../brooklyn/core/mgmt/ha/OsgiManager.java | 71 +++++++++++++------ 2 files changed, 53 insertions(+), 32 deletions(-) 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 408856462c..7d2267aa8e 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 @@ -280,16 +280,16 @@ public ReferenceWithError install() { if (result.getMetadata()!=null) { // already have a managed bundle - check if this is using a new/different URL if (suppliedKnownBundleMetadata!=null && suppliedKnownBundleMetadata.getUrl()!=null) { - String knownIdForThisUrl = osgiManager.managedBundlesByUrl.get(suppliedKnownBundleMetadata.getUrl()); + String knownIdForThisUrl = osgiManager.managedBundlesRecord.getManagedBundleIdFromUrl(suppliedKnownBundleMetadata.getUrl()); if (knownIdForThisUrl==null) { // it's a new URL, but a bundle we already know about log.warn("Request to install from "+suppliedKnownBundleMetadata.getUrl()+" which is not recognized but "+ "appears to match "+result.getMetadata()+"; now associating with the latter"); - osgiManager.managedBundlesByUrl.put(suppliedKnownBundleMetadata.getUrl(), result.getMetadata().getId()); + osgiManager.managedBundlesRecord.setManagedBundleUrl(suppliedKnownBundleMetadata.getUrl(), result.getMetadata().getId()); } else if (!knownIdForThisUrl.equals(result.getMetadata().getId())) { log.warn("Request to install from "+suppliedKnownBundleMetadata.getUrl()+" which is associated to "+knownIdForThisUrl+" but "+ "appears to match "+result.getMetadata()+"; now associating with the latter"); - osgiManager.managedBundlesByUrl.put(suppliedKnownBundleMetadata.getUrl(), result.getMetadata().getId()); + osgiManager.managedBundlesRecord.setManagedBundleUrl(suppliedKnownBundleMetadata.getUrl(), result.getMetadata().getId()); } } if (canUpdate()) { @@ -333,13 +333,7 @@ public ReferenceWithError install() { zipFile = null; // don't close/delete it here, we'll use it for uploading, then it will delete it if (!updating) { - synchronized (osgiManager.managedBundles) { - osgiManager.managedBundles.put(result.getMetadata().getId(), result.getMetadata()); - osgiManager.managedBundlesByName.put(result.getMetadata().getVersionedName(), result.getMetadata().getId()); - if (Strings.isNonBlank(result.getMetadata().getUrl())) { - osgiManager.managedBundlesByUrl.put(result.getMetadata().getUrl(), result.getMetadata().getId()); - } - } + osgiManager.managedBundlesRecord.addManagedBundle(result); result.code = OsgiBundleInstallationResult.ResultCode.INSTALLED_NEW_BUNDLE; result.message = "Installed "+result.getMetadata().getVersionedName()+" with ID "+result.getMetadata().getId(); mgmt().getRebindManager().getChangeListener().onManaged(result.getMetadata()); 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 6b9c526253..69021fe605 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 @@ -102,9 +102,47 @@ public class OsgiManager { private boolean reuseFramework; private Set bundlesAtStartup; private File osgiCacheDir; - Map managedBundles = MutableMap.of(); - Map managedBundlesByName = MutableMap.of(); - Map managedBundlesByUrl = MutableMap.of(); + final ManagedBundlesRecord managedBundlesRecord = new ManagedBundlesRecord(); + + static class ManagedBundlesRecord { + private Map managedBundles = MutableMap.of(); + private Map managedBundlesByName = MutableMap.of(); + private Map managedBundlesByUrl = MutableMap.of(); + + synchronized Map getManagedBundles() { + return ImmutableMap.copyOf(managedBundles); + } + + synchronized String getManagedBundleId(VersionedName vn) { + return managedBundlesByName.get(vn); + } + + synchronized ManagedBundle getManagedBundle(VersionedName vn) { + return managedBundles.get(managedBundlesByName.get(vn)); + } + + synchronized String getManagedBundleIdFromUrl(String url) { + return managedBundlesByUrl.get(url); + } + + synchronized ManagedBundle getManagedBundleFromUrl(String url) { + String id = getManagedBundleIdFromUrl(url); + if (id==null) return null; + return managedBundles.get(id); + } + + synchronized void setManagedBundleUrl(String url, String id) { + managedBundlesByUrl.put(url, id); + } + + synchronized void addManagedBundle(OsgiBundleInstallationResult result) { + managedBundles.put(result.getMetadata().getId(), result.getMetadata()); + managedBundlesByName.put(result.getMetadata().getVersionedName(), result.getMetadata().getId()); + if (Strings.isNonBlank(result.getMetadata().getUrl())) { + managedBundlesByUrl.put(result.getMetadata().getUrl(), result.getMetadata().getId()); + } + } + } private static AtomicInteger numberOfReusableFrameworksCreated = new AtomicInteger(); private static final List OSGI_FRAMEWORK_CONTAINERS_FOR_REUSE = MutableList.of(); @@ -213,30 +251,20 @@ public Boolean call() { } public Map getManagedBundles() { - synchronized (managedBundles) { - return ImmutableMap.copyOf(managedBundles); - } + return managedBundlesRecord.getManagedBundles(); } public String getManagedBundleId(VersionedName vn) { - synchronized (managedBundles) { - return managedBundlesByName.get(vn); - } + return managedBundlesRecord.getManagedBundleId(vn); } public ManagedBundle getManagedBundle(VersionedName vn) { - synchronized (managedBundles) { - return managedBundles.get(managedBundlesByName.get(vn)); - } + return managedBundlesRecord.getManagedBundle(vn); } /** For bundles which are installed by a URL, see whether a bundle has been installed from that URL */ public ManagedBundle getManagedBundleFromUrl(String url) { - synchronized (managedBundles) { - String id = managedBundlesByUrl.get(url); - if (id==null) return null; - return managedBundles.get(id); - } + return managedBundlesRecord.getManagedBundleFromUrl(url); } /** See {@link OsgiArchiveInstaller#install()}, using default values */ @@ -277,13 +305,13 @@ public ReferenceWithError install(@Nullable Manage * Callers should typically fail if anything from this bundle is in use. */ public void uninstallUploadedBundle(ManagedBundle bundleMetadata) { - synchronized (managedBundles) { - ManagedBundle metadata = managedBundles.remove(bundleMetadata.getId()); + synchronized (managedBundlesRecord) { + ManagedBundle metadata = managedBundlesRecord.managedBundles.remove(bundleMetadata.getId()); if (metadata==null) { throw new IllegalStateException("No such bundle registered: "+bundleMetadata); } - managedBundlesByName.remove(bundleMetadata.getVersionedName()); - managedBundlesByUrl.remove(bundleMetadata.getUrl()); + managedBundlesRecord.managedBundlesByName.remove(bundleMetadata.getVersionedName()); + managedBundlesRecord.managedBundlesByUrl.remove(bundleMetadata.getUrl()); } mgmt.getRebindManager().getChangeListener().onUnmanaged(bundleMetadata); @@ -569,5 +597,4 @@ public Iterable getResources(String name, Iterable Date: Mon, 5 Jun 2017 13:58:56 +0100 Subject: [PATCH 13/13] better detail in REST API response when posting ZIP preserves backwards compatibility, adding a detail=true flag which gives richer info. see extended comment in code. also fixes bug where the added/changed items weren't actually listed, and where it tried to return the full BasicManagedBundle when it just wants an ID. --- .../brooklyn/core/mgmt/ha/OsgiManager.java | 7 +++---- .../org/apache/brooklyn/rest/api/CatalogApi.java | 16 ++++++++++++++-- .../brooklyn/rest/resources/CatalogResource.java | 12 ++++++------ 3 files changed, 23 insertions(+), 12 deletions(-) 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 69021fe605..2f61d16de9 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 @@ -372,12 +372,11 @@ public synchronized Bundle registerBundle(CatalogBundle bundleMetadata) { // possibly remove that other capability, so that bundles with BOMs _have_ to be installed via this method. // (load order gets confusing with auto-scanning...) public List> loadCatalogBom(Bundle bundle) { - List> catalogItems = MutableList.of(); - loadCatalogBom(mgmt, bundle, catalogItems); - return catalogItems; + return MutableList.copyOf(loadCatalogBom(mgmt, bundle)); } - private static Iterable> loadCatalogBom(ManagementContext mgmt, Bundle bundle, Iterable> catalogItems) { + private static Iterable> loadCatalogBom(ManagementContext mgmt, Bundle bundle) { + Iterable> catalogItems = MutableList.of(); if (!BrooklynFeatureEnablement.isEnabled(BrooklynFeatureEnablement.FEATURE_LOAD_BUNDLE_CATALOG_BOM)) { // if the above feature is not enabled, let's do it manually (as a contract of this method) try { diff --git a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java index 972c2a60aa..fe2346fa7c 100644 --- a/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java +++ b/rest/rest-api/src/main/java/org/apache/brooklyn/rest/api/CatalogApi.java @@ -88,13 +88,22 @@ public Response createFromYaml( @Valid String yaml); @Beta + /* TODO the polymorphic return type dependent on 'detail' is ugly, + * but we're stuck in this API because backwards compatibility expects the types map + * whereas typical usage wants more feedback. we should introduce a + * /registry and/or /types and/or /bundles endpoint that always provides details + * (and an approach to handling types more aligned with BrooklynTypeRegistry and OSGi bundling). + * Not too concerned here as this method is beta and the above switch will probably naturally happen soon. + * The main client who cares about this is the Go CLI. */ @POST @Consumes({"application/x-zip", "application/x-jar"}) @ApiOperation( value = "Add a catalog items (e.g. new type of entity, policy or location) by uploading a ZIP/JAR archive.", notes = "Accepts either an OSGi bundle JAR, or ZIP which will be turned into bundle JAR. Bother format must " + "contain a catalog.bom at the root of the archive, which must contain the bundle and version key." - + "Return value is map of ID to CatalogItemSummary.", + + "Return value is map of ID to CatalogItemSummary unless detail=true is passed as a parameter in which " + + "case the return value is a BundleInstallationRestResult map containing the types map in types along " + + "with a message, bundle, and code.", response = String.class, hidden = true) @ApiResponses(value = { @@ -106,7 +115,10 @@ public Response createFromArchive( name = "archive", value = "Bundle to install, in ZIP or JAR format, requiring catalog.bom containing bundle name and version", required = true) - byte[] archive); + byte[] archive, + @ApiParam(name="detail", value="Provide a wrapping details map", required=false) + @QueryParam("detail") @DefaultValue("false") + boolean detail); @Beta @POST diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java index 57c4ead40c..612e2a00ea 100644 --- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java +++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/CatalogResource.java @@ -45,7 +45,6 @@ import org.apache.brooklyn.api.policy.PolicySpec; import org.apache.brooklyn.api.sensor.Enricher; import org.apache.brooklyn.api.sensor.EnricherSpec; -import org.apache.brooklyn.api.typereg.ManagedBundle; import org.apache.brooklyn.api.typereg.RegisteredType; import org.apache.brooklyn.core.catalog.CatalogPredicates; import org.apache.brooklyn.core.catalog.internal.CatalogItemComparator; @@ -130,7 +129,7 @@ public Response createFromUpload(byte[] item) { return createFromYaml(new String(item)); } - return createFromArchive(item); + return createFromArchive(item, false); } @Override @@ -159,7 +158,7 @@ public static class BundleInstallationRestResult { // as Osgi result, but without bundle, and with maps of catalog items installed String message; - ManagedBundle metadata; + String bundle; OsgiBundleInstallationResult.ResultCode code; Map types; @@ -172,7 +171,7 @@ public String getMessage() { public static BundleInstallationRestResult of(OsgiBundleInstallationResult in, ManagementContext mgmt, BrooklynRestResourceUtils brooklynU, UriInfo ui) { BundleInstallationRestResult result = new BundleInstallationRestResult(); result.message = in.getMessage(); - result.metadata = in.getMetadata(); + result.bundle = in.getMetadata().getVersionedName().toString(); result.code = in.getCode(); if (in.getCatalogItemsInstalled()!=null) { result.types = MutableMap.of(); @@ -191,7 +190,7 @@ public static BundleInstallationRestResult of(OsgiBundleInstallationResult in, M @Override @Beta - public Response createFromArchive(byte[] zipInput) { + public Response createFromArchive(byte[] zipInput, boolean detail) { if (!Entitlements.isEntitled(mgmt().getEntitlementManager(), Entitlements.ROOT, null)) { throw WebResourceUtils.forbidden("User '%s' is not authorized to add catalog item", Entitlements.getEntitlementContext().user()); @@ -205,7 +204,8 @@ public Response createFromArchive(byte[] zipInput) { .data(BundleInstallationRestResult.of(result.getWithoutError(), mgmt(), brooklyn(), ui)).build().asJsonResponse(); } - return Response.status(Status.CREATED).entity( BundleInstallationRestResult.of(result.get(), mgmt(), brooklyn(), ui) ).build(); + BundleInstallationRestResult resultR = BundleInstallationRestResult.of(result.get(), mgmt(), brooklyn(), ui); + return Response.status(Status.CREATED).entity( detail ? resultR : resultR.types ).build(); } private Response buildCreateResponse(Iterable> catalogItems) {