This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit fc75d84f7fd79b4da19b4cc3a84a85ee0dae7df2 Author: Alex Heneveld <[email protected]> AuthorDate: Wed May 26 20:47:10 2021 +0100 cleaner code for looking up candidate bundles to replace/update tolerating the osgi location being different, in caes needed after an osgi restart. and slightly more efficient, better names and documentation. --- .../mgmt/ha/BrooklynBomOsgiArchiveInstaller.java | 59 +++++++++++----------- .../brooklyn/core/typereg/BasicManagedBundle.java | 10 +++- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java index 253c46d..3f66043 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/BrooklynBomOsgiArchiveInstaller.java @@ -35,6 +35,7 @@ import java.util.function.Supplier; import java.util.jar.Attributes; import java.util.jar.Manifest; import java.util.regex.Pattern; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import javax.annotation.Nullable; @@ -496,31 +497,42 @@ public class BrooklynBomOsgiArchiveInstaller { } } - result.bundle = osgiManager.getFramework().getBundleContext().getBundle(result.getMetadata().getOsgiUniqueUrl()); - log.debug("Request to install "+inferredMetadata.getVersionedName()+" (checksum "+inferredMetadata.getChecksum()+", OSGi URL "+inferredMetadata.getOsgiUniqueUrl()+") in the presence of "+result.getMetadata().getVersionedName()+" (checksum "+result.getMetadata().getChecksum()+", OSGi URL "+result.getMetadata().getOsgiUniqueUrl()+")"); + result.bundle = osgiManager.getFramework().getBundleContext().getBundle(result.getMetadata().getOsgiUniqueUrl()); + // Check if exactly this bundle is already installed if (result.bundle != null && checksumsMatch(result.getMetadata(), inferredMetadata)) { - - isEquivalentBundleAlreadyOsgiInstalled(osgiManager, inferredMetadata, zipFile); - // e.g. repeatedly installing the same bundle log.trace("Bundle "+inferredMetadata+" matches already installed managed bundle "+result.getMetadata() +"; install is no-op"); result.setIgnoringAlreadyInstalled(); return ReferenceWithError.newInstanceWithoutError(result); - } else if (isEquivalentBundleAlreadyOsgiInstalled(osgiManager, inferredMetadata, zipFile)) { + + } + + List<Bundle> matchingVsnBundles = findBundlesBySymbolicNameAndVersion(osgiManager, inferredMetadata); + + List<Bundle> sameContentBundles = matchingVsnBundles.stream().filter(b -> isBundleSameOsgiUrlOrSameContents(b, inferredMetadata, zipFile)).collect(Collectors.toList()); + if (!sameContentBundles.isEmpty()) { // e.g. happens if pre-installed bundle is brought under management, and then add it again via a mvn-style url. - // We wouldn't know the checksum from the pre-installed bundle. + // We wouldn't know the checksum from the pre-installed bundle, the osgi locations might be different, + // but the contents are the same log.trace("Bundle "+inferredMetadata+" matches metadata of managed bundle "+result.getMetadata() +" (but not OSGi bundle location "+result.getMetadata().getOsgiUniqueUrl()+"), " - + "and matches already installed OSGi bundle; ; install is no-op"); + + "and identified as equivalent to installed OSGi bundle; ; install is no-op"); result.setIgnoringAlreadyInstalled(); + result.bundle = sameContentBundles.iterator().next(); return ReferenceWithError.newInstanceWithoutError(result); } - - if (canUpdate()) { + + if (canUpdate()) { + if (result.bundle == null && !matchingVsnBundles.isEmpty()) { + // if we are updating a snapshot bundle or forcing, and somehow we did not manage to preserve the original OSGi location + log.info("Updating existing brooklyn-managed bundle "+result+" with "+inferredMetadata+" with different OSGi location and different contents"); + result.bundle = matchingVsnBundles.iterator().next(); + } + if (result.getBundle() == null) { log.warn("Brooklyn thought it was already managing bundle "+result.getMetadata().getVersionedName() +" but it's not installed to framework at location "+result.getMetadata().getOsgiUniqueUrl()+"; reinstalling it"); @@ -530,13 +542,12 @@ public class BrooklynBomOsgiArchiveInstaller { updating = true; } } else { - List<Bundle> existingBundles = findBundlesByVersion(osgiManager, inferredMetadata); - if (existingBundles.size() > 0 && (result.getMetadata().getChecksum()==null || inferredMetadata.getChecksum()==null)) { + if (matchingVsnBundles.size() > 0 && (result.getMetadata().getChecksum()==null || inferredMetadata.getChecksum()==null)) { // e.g. Checksum would be missing if we brought under management a pre-installed bundle with an unusable url. log.info("Missing bundle checksum data for "+result+"; assuming bundle matches existing brooklyn-managed bundle (not re-installing)"); result.setIgnoringAlreadyInstalled(); return ReferenceWithError.newInstanceWithoutError(result); - } else if (result.bundle != null || existingBundles.size() > 0) { + } else if (result.bundle != null || matchingVsnBundles.size() > 0) { throw new IllegalArgumentException("Bundle "+result.getMetadata().getVersionedName()+" already installed; " + "cannot install a different bundle with the same non-snapshot version"); } else { @@ -569,8 +580,8 @@ public class BrooklynBomOsgiArchiveInstaller { result.metadata = inferredMetadata; // search for already-installed bundles. - List<Bundle> existingBundles = findBundlesByVersion(osgiManager, inferredMetadata); - Maybe<Bundle> existingEquivalentBundle = tryFindEquivalentBundle(existingBundles, inferredMetadata, zipFile); + List<Bundle> existingBundles = findBundlesBySymbolicNameAndVersion(osgiManager, inferredMetadata); + Maybe<Bundle> existingEquivalentBundle = tryFindSameOsgiUrlOrSameContentsBundle(existingBundles, inferredMetadata, zipFile); if (existingEquivalentBundle.isPresent()) { // Identical bundle (by osgi location or binary content) already installed; just bring that under management. @@ -884,7 +895,7 @@ public class BrooklynBomOsgiArchiveInstaller { return blacklistBundlePersistencePredicate.test(managedBundle); } - private static List<Bundle> findBundlesByVersion(OsgiManager osgiManager, ManagedBundle desired) { + private static List<Bundle> findBundlesBySymbolicNameAndVersion(OsgiManager osgiManager, ManagedBundle desired) { return Osgis.bundleFinder(osgiManager.getFramework()) .symbolicName(desired.getSymbolicName()) .version(desired.getOsgiVersionString()) @@ -895,19 +906,9 @@ public class BrooklynBomOsgiArchiveInstaller { return actual.getChecksum() != null && Objects.equal(actual.getChecksum(), desired.getChecksum()); } - private static boolean isEquivalentBundleAlreadyOsgiInstalled(OsgiManager osgiManager, ManagedBundle desired, File zipFile) { - for (Bundle bundle : findBundlesByVersion(osgiManager, desired)) { - if (isEquivalentBundle(bundle, desired, zipFile)) { - return true; - } - } - - return false; - } - - private static Maybe<Bundle> tryFindEquivalentBundle(Iterable<? extends Bundle> bundles, ManagedBundle desired, File zipFile) { + private static Maybe<Bundle> tryFindSameOsgiUrlOrSameContentsBundle(Iterable<? extends Bundle> bundles, ManagedBundle desired, File zipFile) { for (Bundle bundle : bundles) { - if (isEquivalentBundle(bundle, desired, zipFile)) { + if (isBundleSameOsgiUrlOrSameContents(bundle, desired, zipFile)) { return Maybe.of(bundle); } } @@ -915,7 +916,7 @@ public class BrooklynBomOsgiArchiveInstaller { return Maybe.absent(); } - private static boolean isEquivalentBundle(Bundle bundle, ManagedBundle desired, File zipFile) { + private static boolean isBundleSameOsgiUrlOrSameContents(Bundle bundle, ManagedBundle desired, File zipFile) { // Would be nice to also use `desired.getChecksum()`, but not clear if we can get // MD5 checksum from an installed OSGi bundle. 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 367d106..92571d4 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 @@ -89,6 +89,8 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage */ public static BasicManagedBundle copyFirstWithCoordsOfSecond(ManagedBundle update, ManagedBundle oldOneForCoordinates) { BasicManagedBundle result = new BasicManagedBundle(oldOneForCoordinates.getId(), update.getSymbolicName(), update.getSuppliedVersionString(), update.getUrl(), update.getFormat(), update.getUrlCredential(), update.getChecksum()); + // we have secondary logic which should accept a change in the OSGi unique URL, + // but more efficient if we use the original URL result.osgiUniqueUrl = oldOneForCoordinates.getOsgiUniqueUrl(); return result; } @@ -151,10 +153,14 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage } /** - * Gets the (internal) value to be used as the location in bundleContext.install(location). + * Gets the (internal) value to be used as the location in bundleContext.install(location) / bundleContext.getBundle(location). * It thus allows us to tell if a cached OSGi bundle should be considered as replacement * for the one we are about to install (e.g. one we get from persisted state), - * or have retrieved from the initial catalog. + * or have retrieved from the initial catalog with a unique URL for a bundle symbolic-name + version. + * This is typically not a real URL, as the same bundle may come from various locations. + * Typically we use a checksum or internal ID. + * In the case of same-version (eg snapshot or forced) bundle updates we remember and re-use the _original_ checksum. + * However code should be tolerant if this is not unique and use other more-expensive mechanisms for secondary de-duplication. * * Care should be taken to set the checksum <em>before</em> using the OSGi unique url. */
