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 473495368711eb09f85cebecdb093bf67b1f9f79 Author: Alex Heneveld <[email protected]> AuthorDate: Wed May 26 20:19:07 2021 +0100 preserve original ID and OSGi URL for bundles so update logic is correct --- .../camp/brooklyn/catalog/CatalogScanOsgiTest.java | 2 +- .../mgmt/ha/BrooklynBomOsgiArchiveInstaller.java | 15 ++++++++-- .../apache/brooklyn/core/mgmt/ha/OsgiManager.java | 8 ++++-- .../brooklyn/core/objs/AbstractBrooklynObject.java | 4 +++ .../brooklyn/core/typereg/BasicManagedBundle.java | 33 ++++++++++++++++++---- 5 files changed, 50 insertions(+), 12 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java index 8dc74c5..9798651 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/catalog/CatalogScanOsgiTest.java @@ -220,7 +220,7 @@ public class CatalogScanOsgiTest extends AbstractYamlTest { Streams.closeQuietly(out); OsgiBundleInstallationResult result = ((ManagementContextInternal) mgmt()).getOsgiManager().get().install(InputStreamSource.of("test:" + f, f)).getWithError(); - LOG.info("Installed "+result.getVersionedName()+": "+result.getCode()+" - "+result.getMessage()); + LOG.info("Installed "+result.getVersionedName()+": "+result.getCode()+" - "+result.getMessage() + " ("+result.getMetadata().getChecksum()+")"); if (expectedResultCode!=null) Asserts.assertEquals(result.getCode(), expectedResultCode); } catch (Exception e) { throw Exceptions.propagate(e); 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 bbdd21f..253c46d 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 @@ -498,9 +498,13 @@ 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()+")"); + // Check if exactly this bundle is already installed - if (result.bundle != null && !VersionComparator.isSnapshot(inferredMetadata.getSuppliedVersionString()) && - checksumsMatch(result.getMetadata(), inferredMetadata)) { + 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"); @@ -540,6 +544,11 @@ public class BrooklynBomOsgiArchiveInstaller { + "will not re-install without use of 'force'"); } } + + // if proceeding with install, use the new metadata but the old id and osgi url + // (the osgi url must match because we use "getBundle(...)" to update it) + result.metadata = BasicManagedBundle.copyFirstWithCoordsOfSecond(inferredMetadata, result.metadata); + } else { // No such Brooklyn-managed bundle. @@ -635,7 +644,7 @@ public class BrooklynBomOsgiArchiveInstaller { mgmt().getRebindManager().getChangeListener().onManaged(result.getMetadata()); } } else { - oldZipFile = osgiManager.managedBundlesRecord.updateManagedBundleFile(result, zipFile); + oldZipFile = osgiManager.managedBundlesRecord.updateManagedBundleFileAndMetadata(result, zipFile); result.code = OsgiBundleInstallationResult.ResultCode.UPDATED_EXISTING_BUNDLE; result.message = "Updated Brooklyn catalog bundle "+result.getMetadata().getVersionedName()+" as existing ID "+result.getMetadata().getId()+" ["+result.bundle.getBundleId()+"]"; 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 0d9772d..44065ab 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 @@ -141,8 +141,7 @@ public class OsgiManager { } synchronized void addManagedBundle(OsgiBundleInstallationResult result, File f) { - updateManagedBundleFile(result, f); - managedBundlesByUid.put(result.getMetadata().getId(), result.getMetadata()); + updateManagedBundleFileAndMetadata(result, f); managedBundlesUidByVersionedName.put(VersionedName.toOsgiVersionedName(result.getMetadata().getVersionedName()), result.getMetadata().getId()); if (Strings.isNonBlank(result.getMetadata().getUrl())) { @@ -174,7 +173,7 @@ public class OsgiManager { } /** Updates the bundle file associated with the given record, creating and returning a backup if there was already such a file */ - synchronized File updateManagedBundleFile(OsgiBundleInstallationResult result, File fNew) { + synchronized File updateManagedBundleFileAndMetadata(OsgiBundleInstallationResult result, File fNew) { File fCached = fileFor(result.getMetadata()); File fBak = new File(fCached.getAbsolutePath()+".bak"); if (fBak.equals(fNew)) { @@ -192,6 +191,9 @@ public class OsgiManager { } catch (IOException e) { throw Exceptions.propagate(e); } + + managedBundlesByUid.put(result.getMetadata().getId(), result.getMetadata()); + return fBak; } diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java index 3d7b552..a725633 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractBrooklynObject.java @@ -73,6 +73,10 @@ public abstract class AbstractBrooklynObject implements BrooklynObjectInternal { this(Maps.newLinkedHashMap()); } + protected AbstractBrooklynObject(String id) { + this.id = id; + } + public AbstractBrooklynObject(Map<?, ?> properties) { _legacyConstruction = !InternalFactory.FactoryConstructionTracker.isConstructing(); 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 e722d83..367d106 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 @@ -42,6 +42,7 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage private String symbolicName; private String version; private String checksum; + private String osgiUniqueUrl; private String format; private String url; private Credentials credentials; @@ -60,6 +61,10 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage } public BasicManagedBundle(String name, String version, String url, String format, Credentials credentials, @Nullable String checksum) { + init(name, version, url, format, credentials, checksum); + } + + private void init(String name, String version, String url, String format, Credentials credentials, @Nullable String checksum) { if (name == null && version == null) { Preconditions.checkNotNull(url, "Either a URL or both name and version are required"); } else { @@ -68,10 +73,24 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage } this.symbolicName = name; this.version = version; - this.format = format; this.url = url; - this.checksum = checksum; + this.format = format; this.credentials = credentials; + this.checksum = checksum; + } + + private BasicManagedBundle(String id, String name, String version, String url, String format, Credentials credentials, @Nullable String checksum) { + super(id); + init(name, version, url, format, credentials, checksum); + } + + /** used when updating a persisted bundle, we want to use the coords (ID and OSGI unique URL) of the second with the checksum of the former; + * the other fields should be the same between the two but if in doubt use the first argument + */ + 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()); + result.osgiUniqueUrl = oldOneForCoordinates.getOsgiUniqueUrl(); + return result; } @Override @@ -133,14 +152,18 @@ public class BasicManagedBundle extends AbstractBrooklynObject implements Manage /** * Gets the (internal) value to be used as the location in bundleContext.install(location). - * It thus allows us to tell if a cached OSGi bundle is the same as a bundle we are about to - * install (e.g. one we get from persisted state), or have retrieved from the initial catalog. + * 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. * * Care should be taken to set the checksum <em>before</em> using the OSGi unique url. */ @Override public String getOsgiUniqueUrl() { - return "brooklyn:" + (checksum != null ? checksum : getId()); + if (osgiUniqueUrl==null) { + osgiUniqueUrl = "brooklyn:" + (checksum != null ? checksum : getId()); + } + return osgiUniqueUrl; } @Override
