You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2017/09/21 13:54:24 UTC
[GitHub] brooklyn-server pull request #842: Improve Brooklyn-management of OSGi-alrea...
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/842
Improve Brooklyn-management of OSGi-already-managed bundles
Previously could claim it was already installed but not brooklyn-manage it.
Now it is much smarter about checking whether what is being supplied now (by reference or by contents) is an exact match and if so, don't uninstall and reinstall it.
Some annoying issues fixing tests due to how karaf/java deals with `target/classes/` bundles in dev, but tests are all working too now.
NB this requires #810 due to conflicts in osgi bundle removal semantics
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server bundles-brooklyn-import-osgi-already-managed
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/842.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #842
----
commit 9b337035992f77c7e156bba4d3b3ce35438ca180
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-07T15:43:45Z
REST API for bundles, types, and subtypes
also tweaks to other classes to make them more usable, eg VersionedName comparables and bundle removal giving an OsgiBundleInstallationResult
test in BundleAndTypeAndSubtypeResourcesTest based on CatalogResourceTest, basically giving parity in terms of functionality and test coverage
commit 1cd6bf455d4cd0004a98881cc2d9a045bf2ffd1e
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-13T11:53:30Z
Merge branch 'master' into bundle-rest-api
BrooklynCatalog api tidy minor conflict
commit 87a28524f5891431aabec1cc04a318f21758e3e5
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-21T12:16:42Z
Merge branch 'bundle-rest-api' into bundles-brooklyn-import-osgi-already-managed
commit 277cb22aa6b8553bb5369dfd4d1d5eba2185a858
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-21T10:35:25Z
fix bundle install issue if already osgi-installed, and tidy
take the URL from the existing osgi-installed bundle.
much better logging.
commit 3816fb7d7b5e167fd74bb156f26f1c8450136c1a
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-21T12:06:21Z
add Streams.compare function
commit 69ea984467e79dc3554d0079a662648c5e9bd922
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-21T12:07:07Z
handle edge cases where importing OSGi-already-managed bundles to Brooklyn mgmt
commit dcde6806264904ea31a71bf8565994d01b0e8d69
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-21T13:08:40Z
improve making bundle from folders
needed for tests now that we do more with bundles supplied in brooklyn.libraries
commit a017a1c58d5dc1df12a8166a4ab8b1dc253f3be8
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-21T13:29:02Z
add support for dev mode bundle urls so tests pass
makes behaviour consistent with dist behaviour regarding karaf urls
----
---
[GitHub] brooklyn-server issue #842: Improve Brooklyn-management of OSGi-already-mana...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/842
all addressed -- thx @geomacy, merging
---
[GitHub] brooklyn-server pull request #842: Improve Brooklyn-management of OSGi-alrea...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/842#discussion_r140833717
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java ---
@@ -129,58 +133,115 @@ private synchronized void init() {
}
private synchronized void makeLocalZipFileFromInputStreamOrUrl() {
+ Maybe<Bundle> existingOsgiInstalledBundle = Maybe.absent();
+ Maybe<ManagedBundle> existingBrooklynInstalledBundle = Maybe.absent();
if (zipIn==null) {
- Maybe<Bundle> installedBundle = Maybe.absent();
if (suppliedKnownBundleMetadata!=null) {
- // if no input stream, look for a URL and/or a matching bundle
+ // if no input stream (zipIn), look for a URL and/or a matching bundle
if (!suppliedKnownBundleMetadata.isNameResolved()) {
- ManagedBundle mbFromUrl = osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl());
- if (mbFromUrl!=null) {
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl()));
+ if (existingBrooklynInstalledBundle.isPresent()) {
// 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.getSuppliedVersionString());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setSymbolicName(existingBrooklynInstalledBundle.get().getSymbolicName());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setVersion(existingBrooklynInstalledBundle.get().getSuppliedVersionString());
}
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
}
- if (suppliedKnownBundleMetadata.getUrl()!=null) {
- if (installedBundle.isAbsent() || force) {
- // reload
- zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(suppliedKnownBundleMetadata.getUrl());
+ if (existingOsgiInstalledBundle.isPresent()) {
+ if (existingBrooklynInstalledBundle.isAbsent()) {
+ // try to find as brooklyn bundle based on knowledge of OSGi bundle
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundle(new VersionedName(existingOsgiInstalledBundle.get())));
}
+ if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // installer did not supply a usable URL, just coords
+ // but bundle is installed at least to OSGi
+ if (existingBrooklynInstalledBundle.isPresent()) {
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to Brooklyn already; no URL or stream supplied, so re-using existing installation");
+ // if bundle is brooklyn-managed simply say "already installed"
+ result.metadata = existingBrooklynInstalledBundle.get();
+ result.setIgnoringAlreadyInstalled();
+ return;
+
+ } else {
+ // if bundle is not brooklyn-managed we want to make it be so
+ // and for that we need to find a URL.
+ // some things declare usable locations, though these might be maven and
+ String candidateUrl = existingOsgiInstalledBundle.get().getLocation();
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to OSGi but not Brooklyn; trying to find a URL to get bundle binary, candidate "+candidateUrl);
+ if (Strings.isBlank(candidateUrl)) {
+ throw new IllegalArgumentException("No input stream available and no URL could be found: no way to promote "+suppliedKnownBundleMetadata+" from "+existingOsgiInstalledBundle.get()+" to Brooklyn management");
+ }
+ try {
+ // do this in special try block, not below, so we can give a better error
+ // (the user won't understand the URL)
+ zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(candidateUrl);
+ isBringingExistingOsgiInstalledBundleUnderBrooklynManagement = true;
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ throw new IllegalArgumentException("Could not find binary for already installed OSGi bundle "+existingOsgiInstalledBundle.get()+" (location "+candidateUrl+") when trying to promote "+suppliedKnownBundleMetadata+" to Brooklyn management", e);
+ }
+ }
+ }
+ } else if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // not installed anywhere and no URL
+ throw new IllegalArgumentException("No input stream available and no URL could be found: no way to install "+suppliedKnownBundleMetadata);
}
- }
-
- 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);
+
+ assert zipIn!=null || suppliedKnownBundleMetadata.getUrl()!=null : "should have found a stream or inferred a URL";
+
+ if (zipIn!=null) {
+ // found input stream for existing osgi bundle
+
+ } else if (existingBrooklynInstalledBundle.isAbsent() || force) {
+ // reload
+ String url = suppliedKnownBundleMetadata.getUrl();
+ if (BrooklynVersion.isDevelopmentEnvironment() && url.startsWith("system:file:")) {
+ // in live dists the url is usually mvn: but in dev/test karaf will prefix it with system;
+ // leave the url alone so we correctly dedupe when considering whether to update, but create a zip file
+ // so that things work consistently in dev/test (in particular ClassLoaderUtilsTest passes).
+ // pretty sure we have to do this, even if not replacing the osgi bundle, because we need to
+ // get a handle on the zip file (although we could skip if not doing persistence - but that feels even worse than this!)
+ try {
+ url = Strings.removeFromStart(url, "system:");
+ File zipTemp = new BundleMaker(ResourceUtils.create()).createJarFromClasspathDir(url);
--- End diff --
I take it that `createJarFromClasspathDir` works ok in this case, where the url will start with `file:`.
---
[GitHub] brooklyn-server pull request #842: Improve Brooklyn-management of OSGi-alrea...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/842#discussion_r141037439
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java ---
@@ -129,58 +133,115 @@ private synchronized void init() {
}
private synchronized void makeLocalZipFileFromInputStreamOrUrl() {
+ Maybe<Bundle> existingOsgiInstalledBundle = Maybe.absent();
+ Maybe<ManagedBundle> existingBrooklynInstalledBundle = Maybe.absent();
if (zipIn==null) {
- Maybe<Bundle> installedBundle = Maybe.absent();
if (suppliedKnownBundleMetadata!=null) {
- // if no input stream, look for a URL and/or a matching bundle
+ // if no input stream (zipIn), look for a URL and/or a matching bundle
if (!suppliedKnownBundleMetadata.isNameResolved()) {
- ManagedBundle mbFromUrl = osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl());
- if (mbFromUrl!=null) {
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl()));
+ if (existingBrooklynInstalledBundle.isPresent()) {
// 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.getSuppliedVersionString());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setSymbolicName(existingBrooklynInstalledBundle.get().getSymbolicName());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setVersion(existingBrooklynInstalledBundle.get().getSuppliedVersionString());
}
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
}
- if (suppliedKnownBundleMetadata.getUrl()!=null) {
- if (installedBundle.isAbsent() || force) {
- // reload
- zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(suppliedKnownBundleMetadata.getUrl());
+ if (existingOsgiInstalledBundle.isPresent()) {
+ if (existingBrooklynInstalledBundle.isAbsent()) {
+ // try to find as brooklyn bundle based on knowledge of OSGi bundle
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundle(new VersionedName(existingOsgiInstalledBundle.get())));
}
+ if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // installer did not supply a usable URL, just coords
+ // but bundle is installed at least to OSGi
+ if (existingBrooklynInstalledBundle.isPresent()) {
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to Brooklyn already; no URL or stream supplied, so re-using existing installation");
+ // if bundle is brooklyn-managed simply say "already installed"
+ result.metadata = existingBrooklynInstalledBundle.get();
+ result.setIgnoringAlreadyInstalled();
+ return;
+
+ } else {
+ // if bundle is not brooklyn-managed we want to make it be so
+ // and for that we need to find a URL.
+ // some things declare usable locations, though these might be maven and
+ String candidateUrl = existingOsgiInstalledBundle.get().getLocation();
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to OSGi but not Brooklyn; trying to find a URL to get bundle binary, candidate "+candidateUrl);
+ if (Strings.isBlank(candidateUrl)) {
+ throw new IllegalArgumentException("No input stream available and no URL could be found: no way to promote "+suppliedKnownBundleMetadata+" from "+existingOsgiInstalledBundle.get()+" to Brooklyn management");
+ }
+ try {
+ // do this in special try block, not below, so we can give a better error
+ // (the user won't understand the URL)
+ zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(candidateUrl);
+ isBringingExistingOsgiInstalledBundleUnderBrooklynManagement = true;
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ throw new IllegalArgumentException("Could not find binary for already installed OSGi bundle "+existingOsgiInstalledBundle.get()+" (location "+candidateUrl+") when trying to promote "+suppliedKnownBundleMetadata+" to Brooklyn management", e);
+ }
+ }
+ }
+ } else if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // not installed anywhere and no URL
+ throw new IllegalArgumentException("No input stream available and no URL could be found: no way to install "+suppliedKnownBundleMetadata);
}
- }
-
- 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);
+
+ assert zipIn!=null || suppliedKnownBundleMetadata.getUrl()!=null : "should have found a stream or inferred a URL";
+
+ if (zipIn!=null) {
+ // found input stream for existing osgi bundle
+
+ } else if (existingBrooklynInstalledBundle.isAbsent() || force) {
+ // reload
+ String url = suppliedKnownBundleMetadata.getUrl();
+ if (BrooklynVersion.isDevelopmentEnvironment() && url.startsWith("system:file:")) {
+ // in live dists the url is usually mvn: but in dev/test karaf will prefix it with system;
+ // leave the url alone so we correctly dedupe when considering whether to update, but create a zip file
+ // so that things work consistently in dev/test (in particular ClassLoaderUtilsTest passes).
+ // pretty sure we have to do this, even if not replacing the osgi bundle, because we need to
+ // get a handle on the zip file (although we could skip if not doing persistence - but that feels even worse than this!)
+ try {
+ url = Strings.removeFromStart(url, "system:");
+ File zipTemp = new BundleMaker(ResourceUtils.create()).createJarFromClasspathDir(url);
--- End diff --
yep, `ClassLoaderUtilsTest` depends on this and it passes. pretty cool.
---
[GitHub] brooklyn-server pull request #842: Improve Brooklyn-management of OSGi-alrea...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/842#discussion_r141034469
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java ---
@@ -129,58 +133,115 @@ private synchronized void init() {
}
private synchronized void makeLocalZipFileFromInputStreamOrUrl() {
+ Maybe<Bundle> existingOsgiInstalledBundle = Maybe.absent();
+ Maybe<ManagedBundle> existingBrooklynInstalledBundle = Maybe.absent();
if (zipIn==null) {
- Maybe<Bundle> installedBundle = Maybe.absent();
if (suppliedKnownBundleMetadata!=null) {
- // if no input stream, look for a URL and/or a matching bundle
+ // if no input stream (zipIn), look for a URL and/or a matching bundle
if (!suppliedKnownBundleMetadata.isNameResolved()) {
- ManagedBundle mbFromUrl = osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl());
- if (mbFromUrl!=null) {
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl()));
+ if (existingBrooklynInstalledBundle.isPresent()) {
// 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.getSuppliedVersionString());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setSymbolicName(existingBrooklynInstalledBundle.get().getSymbolicName());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setVersion(existingBrooklynInstalledBundle.get().getSuppliedVersionString());
}
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
}
- if (suppliedKnownBundleMetadata.getUrl()!=null) {
- if (installedBundle.isAbsent() || force) {
- // reload
- zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(suppliedKnownBundleMetadata.getUrl());
+ if (existingOsgiInstalledBundle.isPresent()) {
+ if (existingBrooklynInstalledBundle.isAbsent()) {
+ // try to find as brooklyn bundle based on knowledge of OSGi bundle
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundle(new VersionedName(existingOsgiInstalledBundle.get())));
}
+ if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // installer did not supply a usable URL, just coords
+ // but bundle is installed at least to OSGi
+ if (existingBrooklynInstalledBundle.isPresent()) {
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to Brooklyn already; no URL or stream supplied, so re-using existing installation");
+ // if bundle is brooklyn-managed simply say "already installed"
+ result.metadata = existingBrooklynInstalledBundle.get();
+ result.setIgnoringAlreadyInstalled();
+ return;
+
+ } else {
+ // if bundle is not brooklyn-managed we want to make it be so
+ // and for that we need to find a URL.
+ // some things declare usable locations, though these might be maven and
+ String candidateUrl = existingOsgiInstalledBundle.get().getLocation();
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to OSGi but not Brooklyn; trying to find a URL to get bundle binary, candidate "+candidateUrl);
+ if (Strings.isBlank(candidateUrl)) {
+ throw new IllegalArgumentException("No input stream available and no URL could be found: no way to promote "+suppliedKnownBundleMetadata+" from "+existingOsgiInstalledBundle.get()+" to Brooklyn management");
+ }
+ try {
+ // do this in special try block, not below, so we can give a better error
+ // (the user won't understand the URL)
+ zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(candidateUrl);
+ isBringingExistingOsgiInstalledBundleUnderBrooklynManagement = true;
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ throw new IllegalArgumentException("Could not find binary for already installed OSGi bundle "+existingOsgiInstalledBundle.get()+" (location "+candidateUrl+") when trying to promote "+suppliedKnownBundleMetadata+" to Brooklyn management", e);
+ }
+ }
+ }
+ } else if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // not installed anywhere and no URL
+ throw new IllegalArgumentException("No input stream available and no URL could be found: no way to install "+suppliedKnownBundleMetadata);
}
- }
-
- 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);
+
+ assert zipIn!=null || suppliedKnownBundleMetadata.getUrl()!=null : "should have found a stream or inferred a URL";
--- End diff --
the assertion was more for a person reading it
---
[GitHub] brooklyn-server pull request #842: Improve Brooklyn-management of OSGi-alrea...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/842#discussion_r140829916
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java ---
@@ -129,58 +133,115 @@ private synchronized void init() {
}
private synchronized void makeLocalZipFileFromInputStreamOrUrl() {
+ Maybe<Bundle> existingOsgiInstalledBundle = Maybe.absent();
+ Maybe<ManagedBundle> existingBrooklynInstalledBundle = Maybe.absent();
if (zipIn==null) {
- Maybe<Bundle> installedBundle = Maybe.absent();
if (suppliedKnownBundleMetadata!=null) {
- // if no input stream, look for a URL and/or a matching bundle
+ // if no input stream (zipIn), look for a URL and/or a matching bundle
if (!suppliedKnownBundleMetadata.isNameResolved()) {
- ManagedBundle mbFromUrl = osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl());
- if (mbFromUrl!=null) {
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl()));
+ if (existingBrooklynInstalledBundle.isPresent()) {
// 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.getSuppliedVersionString());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setSymbolicName(existingBrooklynInstalledBundle.get().getSymbolicName());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setVersion(existingBrooklynInstalledBundle.get().getSuppliedVersionString());
}
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
}
- if (suppliedKnownBundleMetadata.getUrl()!=null) {
- if (installedBundle.isAbsent() || force) {
- // reload
- zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(suppliedKnownBundleMetadata.getUrl());
+ if (existingOsgiInstalledBundle.isPresent()) {
+ if (existingBrooklynInstalledBundle.isAbsent()) {
+ // try to find as brooklyn bundle based on knowledge of OSGi bundle
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundle(new VersionedName(existingOsgiInstalledBundle.get())));
}
+ if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // installer did not supply a usable URL, just coords
+ // but bundle is installed at least to OSGi
+ if (existingBrooklynInstalledBundle.isPresent()) {
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to Brooklyn already; no URL or stream supplied, so re-using existing installation");
+ // if bundle is brooklyn-managed simply say "already installed"
+ result.metadata = existingBrooklynInstalledBundle.get();
+ result.setIgnoringAlreadyInstalled();
+ return;
+
+ } else {
+ // if bundle is not brooklyn-managed we want to make it be so
+ // and for that we need to find a URL.
+ // some things declare usable locations, though these might be maven and
--- End diff --
text ends just with `and` - did you mean to put something more?
---
[GitHub] brooklyn-server pull request #842: Improve Brooklyn-management of OSGi-alrea...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/842
---
[GitHub] brooklyn-server pull request #842: Improve Brooklyn-management of OSGi-alrea...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/842#discussion_r140831967
--- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/ha/OsgiArchiveInstaller.java ---
@@ -129,58 +133,115 @@ private synchronized void init() {
}
private synchronized void makeLocalZipFileFromInputStreamOrUrl() {
+ Maybe<Bundle> existingOsgiInstalledBundle = Maybe.absent();
+ Maybe<ManagedBundle> existingBrooklynInstalledBundle = Maybe.absent();
if (zipIn==null) {
- Maybe<Bundle> installedBundle = Maybe.absent();
if (suppliedKnownBundleMetadata!=null) {
- // if no input stream, look for a URL and/or a matching bundle
+ // if no input stream (zipIn), look for a URL and/or a matching bundle
if (!suppliedKnownBundleMetadata.isNameResolved()) {
- ManagedBundle mbFromUrl = osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl());
- if (mbFromUrl!=null) {
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundleFromUrl(suppliedKnownBundleMetadata.getUrl()));
+ if (existingBrooklynInstalledBundle.isPresent()) {
// 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.getSuppliedVersionString());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setSymbolicName(existingBrooklynInstalledBundle.get().getSymbolicName());
+ ((BasicManagedBundle)suppliedKnownBundleMetadata).setVersion(existingBrooklynInstalledBundle.get().getSuppliedVersionString());
}
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getOsgiUniqueUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getOsgiUniqueUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.getUrl()!=null) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).requiringFromUrl(suppliedKnownBundleMetadata.getUrl()).find();
}
- if (installedBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
- installedBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
+ if (existingOsgiInstalledBundle.isAbsent() && suppliedKnownBundleMetadata.isNameResolved()) {
+ existingOsgiInstalledBundle = Osgis.bundleFinder(osgiManager.framework).symbolicName(suppliedKnownBundleMetadata.getSymbolicName()).version(suppliedKnownBundleMetadata.getSuppliedVersionString()).find();
}
- if (suppliedKnownBundleMetadata.getUrl()!=null) {
- if (installedBundle.isAbsent() || force) {
- // reload
- zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(suppliedKnownBundleMetadata.getUrl());
+ if (existingOsgiInstalledBundle.isPresent()) {
+ if (existingBrooklynInstalledBundle.isAbsent()) {
+ // try to find as brooklyn bundle based on knowledge of OSGi bundle
+ existingBrooklynInstalledBundle = Maybe.ofDisallowingNull(osgiManager.getManagedBundle(new VersionedName(existingOsgiInstalledBundle.get())));
}
+ if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // installer did not supply a usable URL, just coords
+ // but bundle is installed at least to OSGi
+ if (existingBrooklynInstalledBundle.isPresent()) {
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to Brooklyn already; no URL or stream supplied, so re-using existing installation");
+ // if bundle is brooklyn-managed simply say "already installed"
+ result.metadata = existingBrooklynInstalledBundle.get();
+ result.setIgnoringAlreadyInstalled();
+ return;
+
+ } else {
+ // if bundle is not brooklyn-managed we want to make it be so
+ // and for that we need to find a URL.
+ // some things declare usable locations, though these might be maven and
+ String candidateUrl = existingOsgiInstalledBundle.get().getLocation();
+ log.debug("Detected bundle "+suppliedKnownBundleMetadata+" installed to OSGi but not Brooklyn; trying to find a URL to get bundle binary, candidate "+candidateUrl);
+ if (Strings.isBlank(candidateUrl)) {
+ throw new IllegalArgumentException("No input stream available and no URL could be found: no way to promote "+suppliedKnownBundleMetadata+" from "+existingOsgiInstalledBundle.get()+" to Brooklyn management");
+ }
+ try {
+ // do this in special try block, not below, so we can give a better error
+ // (the user won't understand the URL)
+ zipIn = ResourceUtils.create(mgmt()).getResourceFromUrl(candidateUrl);
+ isBringingExistingOsgiInstalledBundleUnderBrooklynManagement = true;
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ throw new IllegalArgumentException("Could not find binary for already installed OSGi bundle "+existingOsgiInstalledBundle.get()+" (location "+candidateUrl+") when trying to promote "+suppliedKnownBundleMetadata+" to Brooklyn management", e);
+ }
+ }
+ }
+ } else if (suppliedKnownBundleMetadata.getUrl()==null) {
+ // not installed anywhere and no URL
+ throw new IllegalArgumentException("No input stream available and no URL could be found: no way to install "+suppliedKnownBundleMetadata);
}
- }
-
- 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);
+
+ assert zipIn!=null || suppliedKnownBundleMetadata.getUrl()!=null : "should have found a stream or inferred a URL";
--- End diff --
Do you really mean to use normal Java `assert` here? If it's worth asserting here is it not worth asserting normally at run time ie. without having to configure JVM `-ea`?
---