You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2021/09/14 21:56:32 UTC

[brooklyn-server] 13/27: when rebinding, install persisted bundles before default.catalog.bom; but start after

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 2d20fffa2407e4da48fe6edeebc3335e140edccb
Author: Alex Heneveld <al...@cloudsoftcorp.com>
AuthorDate: Tue Sep 14 12:18:13 2021 +0100

    when rebinding, install persisted bundles before default.catalog.bom; but start after
    
    this means the IDs in persistence will be re-used, but start order unchanged.
    previously it did default.catalog.bom before installing and starting persisted bundles,
    with the result that IDs changed on each rebind for things which came from the default.catalog.bom.
    behavior indeterminate wrt snapshot bundles (might switch to preferring those in default.catalog.bom).
---
 .../catalog/internal/CatalogInitialization.java    | 86 ++++++++++++++--------
 .../BrooklynMementoPersisterToObjectStore.java     | 12 +--
 .../brooklyn/core/mgmt/rebind/RebindIteration.java | 11 ++-
 .../mgmt/rebind/dto/BasicManagedBundleMemento.java |  7 ++
 .../BrooklynLauncherRebindCatalogOsgiTest.java     |  4 +-
 5 files changed, 79 insertions(+), 41 deletions(-)

diff --git a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
index 4928e8f..b65e1c7 100644
--- a/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
+++ b/core/src/main/java/org/apache/brooklyn/core/catalog/internal/CatalogInitialization.java
@@ -68,10 +68,10 @@ import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.javalang.JavaClassNames;
 import org.apache.brooklyn.util.os.Os;
 import org.apache.brooklyn.util.osgi.VersionedName;
-import org.apache.brooklyn.util.stream.InputStreamSource;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.apache.commons.lang3.tuple.Pair;
+import org.osgi.framework.Bundle;
 import org.osgi.framework.BundleContext;
 import org.osgi.framework.BundleException;
 import org.slf4j.Logger;
@@ -253,7 +253,11 @@ public class CatalogInitialization implements ManagementContextInjectable {
             } else if (hasRunInitialCatalogInitialization()) {
                 throw new IllegalStateException("Catalog initialization already run for initial catalog by mechanism other than populating persisted state; mode="+mode);      
             }
-            
+
+            // Always install the bundles from persisted state; installed (but not started) prior to catalog,
+            // so that OSGi unique IDs might be picked up when initial catalog is populated
+            Map<InstallableManagedBundle, OsgiBundleInstallationResult> persistenceInstalls = installPersistedBundlesDontStart(persistedState.getBundles(), exceptionHandler, rebindLogger);
+
             populateInitialCatalogImpl(true);
 
             final Maybe<OsgiManager> maybesOsgiManager = managementContext.getOsgiManager();
@@ -267,9 +271,20 @@ public class CatalogInitialization implements ManagementContextInjectable {
                         catalogUpgradeScanner.scan(osgiManager, bundleContext, rebindLogger);
                 CatalogUpgrades.storeInManagementContext(catalogUpgrades, managementContext);
             }
+
             PersistedCatalogState filteredPersistedState = filterBundlesAndCatalogInPersistedState(persistedState, rebindLogger);
-            addPersistedCatalogImpl(filteredPersistedState, exceptionHandler, rebindLogger);
-            
+
+            // previously we effectively installed here, after populating; but now we do it before and then uninstall if needed, to preserve IDs
+//            Map<InstallableManagedBundle, OsgiBundleInstallationResult> persistenceInstalls = installPersistedBundlesDontStart(filteredPersistedState.getBundles(), exceptionHandler, rebindLogger);
+
+            try {
+                startPersistedBundles(filteredPersistedState, persistenceInstalls, exceptionHandler, rebindLogger);
+                BrooklynCatalog catalog = managementContext.getCatalog();
+                catalog.addCatalogLegacyItemsOnRebind(filteredPersistedState.getLegacyCatalogItems());
+            } finally {
+                hasRunPersistenceInitialization = true;
+            }
+
             if (mode == ManagementNodeState.MASTER) {
                 // TODO ideally this would remain false until it has *persisted* the changed catalog;
                 // if there is a subsequent startup failure the forced additions will not be persisted,
@@ -407,20 +422,6 @@ public class CatalogInitialization implements ManagementContextInjectable {
         }
     }
 
-    private void addPersistedCatalogImpl(PersistedCatalogState persistedState, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
-        assert Thread.holdsLock(populatingCatalogMutex);
-
-        try {
-            // Always installing the bundles from persisted state
-            installPersistedBundles(persistedState.getBundles(), exceptionHandler, rebindLogger);
-            
-            BrooklynCatalog catalog = managementContext.getCatalog();
-            catalog.addCatalogLegacyItemsOnRebind(persistedState.getLegacyCatalogItems());
-        } finally {
-            hasRunPersistenceInitialization = true;
-        }
-    }
-    
     private void onFinalCatalog() {
         assert Thread.holdsLock(populatingCatalogMutex);
         
@@ -504,12 +505,12 @@ public class CatalogInitialization implements ManagementContextInjectable {
         return false;
     }
 
-    private void installPersistedBundles(Map<VersionedName, InstallableManagedBundle> bundles, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
+    private Map<InstallableManagedBundle, OsgiBundleInstallationResult> installPersistedBundlesDontStart(Map<VersionedName, InstallableManagedBundle> bundles, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
         Map<InstallableManagedBundle, OsgiBundleInstallationResult> installs = MutableMap.of();
 
         // Install the bundles
         Map<VersionedName, InstallableManagedBundle> remaining = MutableMap.copyOf(bundles);
-        Set<Pair<Entry<VersionedName, InstallableManagedBundle>,Exception>> errors = MutableSet.of();
+        Set<Pair<Entry<VersionedName, InstallableManagedBundle>, Exception>> errors = MutableSet.of();
         while (!remaining.isEmpty()) {
             int installed = 0;
             for (Entry<VersionedName, InstallableManagedBundle> entry : MutableSet.copyOf(remaining.entrySet())) {
@@ -519,35 +520,58 @@ public class CatalogInitialization implements ManagementContextInjectable {
                     remaining.remove(entry.getKey());
                     installed++;
                 } catch (Exception e) {
-                    rebindLogger.debug("Unable to install bundle "+entry.getKey()+", but may re-try in case it has a dependency on another bundle ("+e+")");
+                    rebindLogger.debug("Unable to install bundle " + entry.getKey() + ", but may re-try in case it has a dependency on another bundle (" + e + ")");
                     errors.add(Pair.of(entry, e));
                 }
             }
-            if (installed==0) {
+            if (installed == 0) {
                 // keep trying until either nothing is installed or nothing is left to install
                 break;
             }
         }
         rebindLogger.debug("RebindManager installed bundles {}, {} errors", installs.keySet(), errors.size());
         errors.forEach(err -> exceptionHandler.onCreateFailed(BrooklynObjectType.MANAGED_BUNDLE,
-                err.getLeft().getKey().toString(), err.getLeft().getValue().getManagedBundle().getSymbolicName(), err.getRight()) );
+                err.getLeft().getKey().toString(), err.getLeft().getValue().getManagedBundle().getSymbolicName(), err.getRight()));
 
-        // Start the bundles (now that we've installed them all)
+        return installs;
+    }
 
-        Set<RegisteredType> installedTypes = MutableSet.of();
+    private void startPersistedBundles(PersistedCatalogState filteredPersistedState, Map<InstallableManagedBundle, OsgiBundleInstallationResult> installs, RebindExceptionHandler exceptionHandler, RebindLogger rebindLogger) {
+        // Start the bundles (now that we've installed them all)
 
         // start order is:  OSGi and not catalog; then OSGi and catalog; then not catalog nor OSGi; then catalog and not OSGi
         // (we need OSGi and not catalog to start first; the others are less important)
-        Set<OsgiBundleInstallationResult> bundlesInOrder = MutableSet.copyOf(installs.values());
-        MutableSet.copyOf(bundlesInOrder).stream().filter(b -> b.getBundle()!=null && b.getBundle().getResource("/catalog.bom")!=null).forEach(b -> {
-            bundlesInOrder.remove(b); bundlesInOrder.add(b); // then move catalog.bom items to the end
+        Set<OsgiBundleInstallationResult> bundlesInOrder = MutableSet.of();
+        Set<OsgiBundleInstallationResult> bundlesToRemove = MutableSet.of();
+        installs.values().stream().forEach(candidate -> {
+            if (filteredPersistedState.getBundles().containsKey(candidate.getVersionedName())) {
+                bundlesInOrder.add(candidate);
+            } else {
+                log.debug("Skipping start of persisted bundle "+candidate+" due to catalog upgrade metadata instructions");
+                bundlesToRemove.add(candidate);
+            }
         });
-        MutableSet.copyOf(bundlesInOrder).stream().filter(b -> b.getBundle()!=null && b.getBundle().getResource("/META-INF/MANIFEST.MF")==null).forEach(b -> {
-            bundlesInOrder.remove(b); bundlesInOrder.add(b); // move non-osgi items to the end
+        bundlesToRemove.forEach(b -> {
+            ManagedBundle mb = getManagementContext().getOsgiManager().get().getManagedBundle(b.getVersionedName());
+            if (b.getBundle().getState() >= Bundle.INSTALLED && b.getBundle().getState() < Bundle.STARTING) {
+                // we installed it, catalog did not start it, so let's uninstall it
+                OsgiBundleInstallationResult result = getManagementContext().getOsgiManager().get().uninstallUploadedBundle(b.getMetadata());
+                log.debug("Result of uninstalling "+b+" due to due to catalog upgrade metadata instructions: "+result);
+            }
+        });
+
+        MutableSet.copyOf(bundlesInOrder).stream().filter(b -> b.getBundle() != null && b.getBundle().getResource("/catalog.bom") != null).forEach(b -> {
+            bundlesInOrder.remove(b);
+            bundlesInOrder.add(b); // then move catalog.bom items to the end
+        });
+        MutableSet.copyOf(bundlesInOrder).stream().filter(b -> b.getBundle() != null && b.getBundle().getResource("/META-INF/MANIFEST.MF") == null).forEach(b -> {
+            bundlesInOrder.remove(b);
+            bundlesInOrder.add(b); // move non-osgi items to the end
         });
         if (!bundlesInOrder.isEmpty()) {
-            log.debug("Rebind bundle start order is: "+bundlesInOrder);
+            log.debug("Rebind bundle start order is: " + bundlesInOrder);
         }
+        Set<RegisteredType> installedTypes = MutableSet.of();
 
         for (OsgiBundleInstallationResult br : bundlesInOrder) {
             try {
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/BrooklynMementoPersisterToObjectStore.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/BrooklynMementoPersisterToObjectStore.java
index fb579df..5f16faf 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/BrooklynMementoPersisterToObjectStore.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/persist/BrooklynMementoPersisterToObjectStore.java
@@ -619,7 +619,7 @@ public class BrooklynMementoPersisterToObjectStore implements BrooklynMementoPer
             futures.add(asyncUpdatePlaneId(newMemento.getPlaneId(), exceptionHandler));
             for (BrooklynObjectType type: BrooklynPersistenceUtils.STANDARD_BROOKLYN_OBJECT_TYPE_PERSISTENCE_ORDER) {
                 for (Map.Entry<String, String> entry : newMemento.getObjectsOfType(type).entrySet()) {
-                    addPersistContentIfManagedBundle(type, entry.getKey(), futures, exceptionHandler);
+                    addPersistContentIfManagedBundle(type, entry.getKey(), entry.getValue(), futures, exceptionHandler);
                     futures.add(asyncPersist(type.getSubPathName(), type, entry.getKey(), entry.getValue(), exceptionHandler));
                 }
             }
@@ -705,7 +705,7 @@ public class BrooklynMementoPersisterToObjectStore implements BrooklynMementoPer
             for (BrooklynObjectType type: BrooklynPersistenceUtils.STANDARD_BROOKLYN_OBJECT_TYPE_PERSISTENCE_ORDER) {
                 for (Memento item : delta.getObjectsOfType(type)) {
                     if (!deletedIds.contains(item.getId())) {
-                        addPersistContentIfManagedBundle(type, item.getId(), futures, exceptionHandler);
+                        addPersistContentIfManagedBundle(type, item.getId(), ""+item.getCatalogItemId()+"/"+item.getDisplayName(), futures, exceptionHandler);
                         futures.add(asyncPersist(type.getSubPathName(), item, exceptionHandler));
                     }
                 }
@@ -735,15 +735,17 @@ public class BrooklynMementoPersisterToObjectStore implements BrooklynMementoPer
         return lastErrors;
     }
 
-    private void addPersistContentIfManagedBundle(final BrooklynObjectType type, final String id, List<ListenableFuture<?>> futures, final PersistenceExceptionHandler exceptionHandler) {
+    private void addPersistContentIfManagedBundle(final BrooklynObjectType type, final String id, final String summaryOrContents, List<ListenableFuture<?>> futures, final PersistenceExceptionHandler exceptionHandler) {
         if (type==BrooklynObjectType.MANAGED_BUNDLE) {
             if (mgmt==null) {
                 throw new IllegalStateException("Cannot persist bundles without a management context");
             }
             final ManagedBundle mb = ((ManagementContextInternal)mgmt).getOsgiManager().get().getManagedBundles().get(id);
-            LOG.debug("Persisting managed bundle "+id+": "+mb);
+            LOG.debug("Persisting managed bundle "+id+": "+mb+" - "+summaryOrContents);
             if (mb==null) {
-                LOG.warn("Cannot find managed bundle for added bundle "+id+"; ignoring");
+                // previously happened on rebind because new osgi unique id was made; now it should use the same so we shouldn't see this,
+                // but if we do the log will contain a summary or contents for comparison which will help
+                LOG.warn("Cannot find managed bundle for added bundle "+id+"; ignoring (probably uninstalled or reinstalled with another OSGi ID; see debug log for contents)");
                 return;
             }
             
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 ede24a9..cfbd400 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
@@ -862,8 +862,11 @@ public abstract class RebindIteration {
 
         if (!isEmpty) {
             BrooklynLogging.log(LOG, shouldLogRebinding() ? LoggingLevel.INFO : LoggingLevel.DEBUG,
-                    "Rebind complete " + "(" + mode + (readOnlyRebindCount.get() >= 0 ? ", iteration " + readOnlyRebindCount : "") + ")" +
-                            " in {}: {} app{}, {} entit{}, {} location{}, {} polic{}, {} enricher{}, {} feed{}, {} catalog item{}, {} catalog bundle{}",
+                    "Rebind complete" +
+                    (!exceptionHandler.getExceptions().isEmpty() ? ", with errors" :
+                        !exceptionHandler.getWarnings().isEmpty() ? ", with warnings" : "") +
+                            " (" + mode + (readOnlyRebindCount.get() >= 0 ? ", iteration " + readOnlyRebindCount : "") + ")" +
+                            " in {}: {} app{}, {} entit{}, {} location{}, {} polic{}, {} enricher{}, {} feed{}, {} catalog item{}, {} catalog bundle{}{}{}",
                     Time.makeTimeStringRounded(timer), applications.size(), Strings.s(applications),
                     rebindContext.getEntities().size(), Strings.ies(rebindContext.getEntities()),
                     rebindContext.getLocations().size(), Strings.s(rebindContext.getLocations()),
@@ -871,7 +874,9 @@ public abstract class RebindIteration {
                     rebindContext.getEnrichers().size(), Strings.s(rebindContext.getEnrichers()),
                     rebindContext.getFeeds().size(), Strings.s(rebindContext.getFeeds()),
                     rebindContext.getCatalogItems().size(), Strings.s(rebindContext.getCatalogItems()),
-                    rebindContext.getBundles().size(), Strings.s(rebindContext.getBundles())
+                    rebindContext.getBundles().size(), Strings.s(rebindContext.getBundles()),
+                    (!exceptionHandler.getExceptions().isEmpty() ? "; errors="+exceptionHandler.getExceptions() : ""),
+                    (!exceptionHandler.getWarnings().isEmpty() ? "; warnings="+exceptionHandler.getWarnings() : "")
             );
         }
 
diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicManagedBundleMemento.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicManagedBundleMemento.java
index 850da94..6d74587 100644
--- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicManagedBundleMemento.java
+++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/dto/BasicManagedBundleMemento.java
@@ -161,4 +161,11 @@ public class BasicManagedBundleMemento extends AbstractMemento implements Manage
                 .add("url", getUrl())
                 .add("checksum", getChecksum());
     }
+
+    @Override
+    public String toString() {
+        // include more details on toString here, so we can see what/where it is being persisted
+        return toVerboseString();
+    }
+
 }
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
index a90812b..5cc4f66 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
@@ -631,8 +631,8 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo
         assertEquals(getPersistenceListing(BrooklynObjectType.MANAGED_BUNDLE), ImmutableSet.of(bundlePersistenceId2));
 
         if (isT1KeptRunningWhenT2Starts()) {
-            // would like if we could make them the same but currently code should _always_ change
-            Assert.assertNotEquals(bundlePersistenceId1, bundlePersistenceId2);
+            // now we keep these the same!
+            Assert.assertEquals(bundlePersistenceId1, bundlePersistenceId2);
         }
     }