maps for upgrade types and bundles, and related test fixes incl tidy HA 
transition clearing

fix recent rebind header tests which i forgot to test in previous, some subtle 
HA behaviours,
but now all fixed and documented.

need to clear type registry as part of other clears on HA promotion fail


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/8473fb4b
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/8473fb4b
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/8473fb4b

Branch: refs/heads/master
Commit: 8473fb4be2c21d275bab8b2c92880abcf83eb68a
Parents: 00df1d5
Author: Alex Heneveld <[email protected]>
Authored: Tue Oct 31 09:34:25 2017 +0000
Committer: Alex Heneveld <[email protected]>
Committed: Tue Oct 31 09:35:16 2017 +0000

----------------------------------------------------------------------
 .../mgmt/ha/HighAvailabilityManagerImpl.java    | 14 ++-
 .../mgmt/rebind/RebindExceptionHandlerImpl.java |  2 +-
 .../core/typereg/BasicBrooklynTypeRegistry.java |  7 +-
 .../core/typereg/BundleUpgradeParser.java       | 72 +++++++++++++--
 .../core/typereg/BundleUpgradeParserTest.java   |  5 +-
 .../BrooklynLauncherRebindCatalogOsgiTest.java  | 97 +++++++++-----------
 6 files changed, 127 insertions(+), 70 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8473fb4b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
index 1d4d50c..7220091 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/ha/HighAvailabilityManagerImpl.java
@@ -66,6 +66,7 @@ import 
org.apache.brooklyn.core.mgmt.persist.PersistenceActivityMetrics;
 import org.apache.brooklyn.core.mgmt.rebind.RebindManagerImpl;
 import org.apache.brooklyn.core.mgmt.usage.ManagementNodeStateListener;
 import org.apache.brooklyn.core.server.BrooklynServerConfig;
+import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry;
 import org.apache.brooklyn.util.collections.MutableList;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.task.ScheduledTask;
@@ -939,12 +940,15 @@ public class HighAvailabilityManagerImpl implements 
HighAvailabilityManager {
     /** 
      * Clears all managed items from the management context.
      * 
-     * The same items are destroyed as in the course of a rebind cycle, except 
for clearBrooklynManagedBundles.
-     * That last operation could be expensive (causing bundles to be installed 
again). Therefore we only do it
-     * when we stop being a hotProxy or when we are demoted (e.g. during the 
periodic rebind as hot_stanby
-     * we will not repeatedly clear the brooklyn-managed-bundles).
+     * The same items are destroyed as in the course of a rebind cycle, with 
the addition of bundles/types
+     * which is comparatively more expensive, so we only do it when we stop 
being a hotProxy or when we are demoted 
+     * (e.g. during the periodic rebind as hot_stanby we will not repeatedly 
clear the brooklyn-managed-bundles).
      */
     protected void clearManagedItems(ManagementTransitionMode mode) {
+        // log this because it may be surprising, it is just HA transitions,
+        // not symmetric with usual single-node start
+        LOG.info("Clearing all managed items on transition to "+mode);
+        
         // start with the root applications
         for (Application app: managementContext.getApplications()) {
             if (((EntityInternal)app).getManagementSupport().isDeployed()) {
@@ -967,7 +971,7 @@ public class HighAvailabilityManagerImpl implements 
HighAvailabilityManager {
         }
         
         
((BasicBrooklynCatalog)managementContext.getCatalog()).reset(CatalogDto.newEmptyInstance("<reset-by-ha-status-change>"));
-        
+        
((BasicBrooklynTypeRegistry)managementContext.getTypeRegistry()).clear();
         
managementContext.getCatalogInitialization().clearBrooklynManagedBundles();
     }
     

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8473fb4b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindExceptionHandlerImpl.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindExceptionHandlerImpl.java
 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindExceptionHandlerImpl.java
index 794161b..54564a8 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindExceptionHandlerImpl.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindExceptionHandlerImpl.java
@@ -453,7 +453,7 @@ public class RebindExceptionHandlerImpl implements 
RebindExceptionHandler {
                     warn("Dangling item"+Strings.s(totalDangling)+" 
("+totalDangling+" of "+totalItems+") found on rebind exceeds quorum, assuming 
failed: "+danglingIds);
                     allExceptions.add(new IllegalStateException("Too many 
dangling references: "+totalDangling+" of "+totalItems));
                 } else {
-                    LOG.info("Dangling item"+Strings.s(totalDangling)+" 
("+totalDangling+" of "+totalItems+") found on rebind, assuming deleted: 
"+danglingIds);
+                    LOG.info("Dangling item"+Strings.s(totalDangling)+" 
("+totalDangling+" of "+totalItems+") found on rebind, within quorum, assuming 
deleted: "+danglingIds);
                 }
             }
         }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8473fb4b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
index dd0b4b9..94e9b6b 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BasicBrooklynTypeRegistry.java
@@ -51,7 +51,6 @@ import org.apache.brooklyn.util.concurrent.Locks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.osgi.VersionedName;
-import org.apache.brooklyn.util.osgi.VersionedName.VersionedNameComparator;
 import 
org.apache.brooklyn.util.osgi.VersionedName.VersionedNameStringComparator;
 import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
 import org.apache.brooklyn.util.text.Identifiers;
@@ -425,6 +424,7 @@ public class BasicBrooklynTypeRegistry implements 
BrooklynTypeRegistry {
                             
oldContainingBundlesToRemove.add(existingT.getContainingBundle());
                         }
                         if (canForce) {
+                            // may be forcing because of internal type 
validation, or of course user flag
                             log.debug("Addition of "+type+" to replace 
"+existingT+" allowed because force is on");
                             continue;
                         }
@@ -616,5 +616,10 @@ public class BasicBrooklynTypeRegistry implements 
BrooklynTypeRegistry {
     public void delete(String id) {
         delete(VersionedName.fromString(id));
     }
+
+    /** Deletes all items, for use when resetting management context */
+    public void clear() {
+        Locks.withLock(localRegistryLock.writeLock(), () -> 
localRegisteredTypesAndContainingBundles.clear());
+    }
     
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8473fb4b/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java
index 24d905f..73327e9 100644
--- 
a/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java
+++ 
b/core/src/main/java/org/apache/brooklyn/core/typereg/BundleUpgradeParser.java
@@ -23,7 +23,6 @@ import static 
com.google.common.base.Preconditions.checkNotNull;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Dictionary;
-import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -32,6 +31,7 @@ import java.util.Set;
 import org.apache.brooklyn.api.catalog.CatalogItem;
 import org.apache.brooklyn.api.typereg.RegisteredType;
 import org.apache.brooklyn.util.collections.MutableList;
+import org.apache.brooklyn.util.collections.MutableSet;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.osgi.VersionedName;
 import org.apache.brooklyn.util.text.BrooklynVersionSyntax;
@@ -45,7 +45,10 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.Multimap;
 
 /**
  * Internal class for parsing bundle manifests to extract their upgrade 
instructions.
@@ -119,8 +122,9 @@ public class BundleUpgradeParser {
      * These will be advisory unless the bundle being upgraded is 
force-removed in which case it will be applied automatically
      * wherever the bundle is in use.
      * 
-     * The format is a comma separate list of {@code key=value} pairs, where 
each key and value is a name with a version range
-     * (as per {@link #MANIFEST_HEADER_FORCE_REMOVE_BUNDLES}). The {@code 
=value} can be omitted, and usually is,
+     * The format is a comma separate list of {@code key=value} pairs, where 
each key is a name with a version range
+     * (as per {@link #MANIFEST_HEADER_FORCE_REMOVE_BUNDLES}) specifying what 
should be upgraded, and {@code value} is a name and
+     * version specifying what it should be ugpraded to. The {@code =value} 
can be omitted, and usually is,
      * to mean this bundle at this version. (The {@code =value} is available 
if one bundle is defining upgrades for other bundles.)  
      * 
      * A wildcard can be given as the key, without a version ({@code *}) or 
with ({@code *:[0,1)}) to refer to 
@@ -200,8 +204,8 @@ public class BundleUpgradeParser {
         public static class Builder {
             private Set<VersionRangedName> removedLegacyItems = new 
LinkedHashSet<>();
             private Set<VersionRangedName> removedBundles = new 
LinkedHashSet<>();
-            private Map<VersionedName,VersionRangedName> upgradeBundles = new 
LinkedHashMap<>();
-            private Map<VersionedName,VersionRangedName> upgradeTypes = new 
LinkedHashMap<>();
+            private Multimap<VersionedName,VersionRangedName> 
upgradesProvidedByBundles = LinkedHashMultimap.create();
+            private Multimap<VersionedName,VersionRangedName> 
upgradesProvidedByTypes = LinkedHashMultimap.create();
 
             public Builder removedLegacyItems(Collection<VersionRangedName> 
vals) {
                 removedLegacyItems.addAll(vals);
@@ -211,9 +215,19 @@ public class BundleUpgradeParser {
                 removedBundles.addAll(vals);
                 return this;
             }
+            public Builder 
upgradeBundles(Multimap<VersionedName,VersionRangedName> vals) {
+                upgradesProvidedByBundles.putAll(vals);
+                return this;
+            }
+            public Builder 
upgradeTypes(Multimap<VersionedName,VersionRangedName> vals) {
+                upgradesProvidedByTypes.putAll(vals);
+                return this;
+            }
             public Builder addAll(CatalogUpgrades other) {
                 removedLegacyItems.addAll(other.removedLegacyItems);
                 removedBundles.addAll(other.removedBundles);
+                
upgradesProvidedByBundles.putAll(other.upgradesProvidedByBundles);
+                upgradesProvidedByTypes.putAll(other.upgradesProvidedByTypes);
                 return this;
             }
             public CatalogUpgrades build() {
@@ -227,10 +241,14 @@ public class BundleUpgradeParser {
         
         private final Set<VersionRangedName> removedLegacyItems;
         private final Set<VersionRangedName> removedBundles;
+        private final Multimap<VersionedName,VersionRangedName> 
upgradesProvidedByBundles;
+        private final Multimap<VersionedName,VersionRangedName> 
upgradesProvidedByTypes;
         
         public CatalogUpgrades(Builder builder) {
             this.removedLegacyItems = 
ImmutableSet.copyOf(builder.removedLegacyItems);
             this.removedBundles = ImmutableSet.copyOf(builder.removedBundles);
+            this.upgradesProvidedByBundles = 
ImmutableMultimap.copyOf(builder.upgradesProvidedByBundles);
+            this.upgradesProvidedByTypes = 
ImmutableMultimap.copyOf(builder.upgradesProvidedByTypes);
         }
 
         public boolean isEmpty() {
@@ -245,6 +263,14 @@ public class BundleUpgradeParser {
             return removedBundles;
         }
 
+        public Multimap<VersionedName, VersionRangedName> 
getUpgradesProvidedByBundles() {
+            return upgradesProvidedByBundles;
+        }
+        
+        public Multimap<VersionedName, VersionRangedName> 
getUpgradesProvidedByTypes() {
+            return upgradesProvidedByTypes;
+        }
+        
         public boolean isLegacyItemRemoved(CatalogItem<?, ?> 
legacyCatalogItem) {
             VersionedName name = new 
VersionedName(legacyCatalogItem.getSymbolicName(), 
legacyCatalogItem.getVersion());
             return contains(removedLegacyItems, name);
@@ -254,14 +280,36 @@ public class BundleUpgradeParser {
             return contains(removedBundles, bundle);
         }
         
-        public boolean contains(Iterable<VersionRangedName> names, 
VersionedName name) {
+        public Set<VersionedName> getUpgradesForBundle(VersionedName bundle) {
+            return findUpgradesIn(bundle, upgradesProvidedByBundles);
+        }
+        public Set<VersionedName> getUpgradesForType(VersionedName type) {
+            return findUpgradesIn(type, upgradesProvidedByTypes);
+        }
+        private static Set<VersionedName> findUpgradesIn(VersionedName bundle, 
Multimap<VersionedName,VersionRangedName> upgradesMap) {
+            Set<VersionedName> result = MutableSet.of();
+            for (Map.Entry<VersionedName,VersionRangedName> n: 
upgradesMap.entries()) {
+                if (contains(n.getValue(), bundle)) {
+                    result.add(n.getKey());
+                }
+            }
+            return result;
+        }
+        
+        @Beta
+        public static boolean contains(Iterable<VersionRangedName> names, 
VersionedName name) {
             for (VersionRangedName contender : names) {
-                if (contender.getSymbolicName().equals(name.getSymbolicName()) 
&& contender.getOsgiVersionRange().includes(name.getOsgiVersion())) {
+                if (contains(contender, name)) {
                     return true;
                 }
             }
             return false;
         }
+
+        @Beta
+        public static boolean contains(VersionRangedName range, VersionedName 
name) {
+            return range.getSymbolicName().equals(name.getSymbolicName()) && 
range.getOsgiVersionRange().includes(name.getOsgiVersion());
+        }
     }
     
     /**
@@ -356,12 +404,22 @@ public class BundleUpgradeParser {
         //   section "Bundle Upgrade Metadata"
         
         Dictionary<String, String> headers = bundle.getHeaders();
+        Multimap<VersionedName,VersionRangedName> upgradesForBundles = 
parseUpgradeForBundlesHeader(headers.get(MANIFEST_HEADER_UPGRADE_FOR_BUNDLES), 
bundle);
         return CatalogUpgrades.builder()
                 
.removedLegacyItems(parseForceRemoveLegacyItemsHeader(headers.get(MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS),
 bundle, typeSupplier))
                 
.removedBundles(parseForceRemoveBundlesHeader(headers.get(MANIFEST_HEADER_FORCE_REMOVE_BUNDLES),
 bundle))
+                .upgradeBundles(upgradesForBundles)
+                
.upgradeTypes(parseUpgradeForTypesHeader(headers.get(MANIFEST_HEADER_UPGRADE_FOR_TYPES),
 bundle, upgradesForBundles))
                 .build();
     }
 
+    private static Multimap<VersionedName, VersionRangedName> 
parseUpgradeForBundlesHeader(String input, Bundle bundle) {
+        return LinkedHashMultimap.<VersionedName, VersionRangedName>create();
+    }
+    private static Multimap<VersionedName, VersionRangedName> 
parseUpgradeForTypesHeader(String input, Bundle bundle, Multimap<VersionedName, 
VersionRangedName> upgradesForBundles) {
+        return LinkedHashMultimap.<VersionedName, VersionRangedName>create();
+    }
+
     @VisibleForTesting
     static List<VersionRangedName> parseForceRemoveLegacyItemsHeader(String 
input, Bundle bundle, Supplier<? extends Iterable<? extends RegisteredType>> 
typeSupplier) {
         if (input == null) return ImmutableList.of();

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8473fb4b/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
 
b/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
index 5fc5188..64c8a6e 100644
--- 
a/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
+++ 
b/core/src/test/java/org/apache/brooklyn/core/typereg/BundleUpgradeParserTest.java
@@ -39,7 +39,6 @@ import org.mockito.Mockito;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.Version;
 import org.osgi.framework.VersionRange;
-import org.testng.Assert;
 import org.testng.annotations.Test;
 
 import com.google.common.base.Supplier;
@@ -158,7 +157,9 @@ public class BundleUpgradeParserTest {
     public void testParseBundleManifest() throws Exception {
         Bundle bundle = newMockBundle(ImmutableMap.of(
                 BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_LEGACY_ITEMS, 
"\"foo:[0,1.0.0)\",\"foo:1.0.0.SNAPSHOT\",\"bar:[0,1.0.0)\"",
-                BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_BUNDLES, 
"\"org.example.brooklyn.mybundle:[0,1.0.0)\""));
+                BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_BUNDLES, 
"\"org.example.brooklyn.mybundle:[0,1.0.0)\"",
+                BundleUpgradeParser.MANIFEST_HEADER_UPGRADE_FOR_BUNDLES, 
"\"org.example.brooklyn.mybundle:[0,1.0.0)=org.example.brooklyn.mybundle:1\"",
+                BundleUpgradeParser.MANIFEST_HEADER_UPGRADE_FOR_TYPES, 
"\"foo:[0,1)=foo:1\""));
         checkParse(bundle);
     }
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/8473fb4b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
----------------------------------------------------------------------
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 3f261df..23f564c 100644
--- 
a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
+++ 
b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynLauncherRebindCatalogOsgiTest.java
@@ -30,7 +30,6 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
-import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.api.mgmt.ha.HighAvailabilityMode;
@@ -142,6 +141,7 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest 
extends AbstractBroo
         }
         l.start();
         launcherLast = launcherT1 = l;
+        assertMasterEventually(launcherT1);
         
         if (startupAssertions!=null) startupAssertions.run();
     }
@@ -150,7 +150,7 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest 
extends AbstractBroo
         startT2(l, true);
     }
     
-    protected void startT2(BrooklynLauncher l, boolean assertSuccess) {
+    protected void startT2(BrooklynLauncher l, boolean expectSuccess) {
         if (launcherT2!=null) throw new IllegalStateException("Already started 
T2 launcher");
         
         try {
@@ -165,8 +165,14 @@ public abstract class 
BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo
         }
         l.start();
         launcherLast = launcherT2 = l;
-        if (isT1KeptRunningWhenT2Starts()) {
-            assertHotStandbyNow(launcherT2);
+        if (expectSuccess) {
+            if (isT1KeptRunningWhenT2Starts()) { 
+                assertHotStandbyNow(launcherT2);
+            } else {
+                assertMasterEventually(launcherT2);
+            }
+        } else {
+            assertFailsEventually(launcherLast);
         }
         
         if (startupAssertions!=null) startupAssertions.run();
@@ -174,11 +180,13 @@ public abstract class 
BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo
     protected void promoteT2IfStandby() {
         promoteT2IfStandby(true);
     }
-    protected void promoteT2IfStandby(boolean assertSuccess) {
+    protected void promoteT2IfStandby(boolean expectHealthy) {
         if (isT1KeptRunningWhenT2Starts()) {
             launcherT1.terminate();
-            if (assertSuccess) {
+            if (expectHealthy) {
                 assertMasterEventually(launcherT2);
+            } else {
+                assertFailsEventually(launcherT2);
             }
             
             if (startupAssertions!=null) startupAssertions.run();
@@ -617,37 +625,10 @@ public abstract class 
BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo
             Assert.assertNotEquals(bundlePersistenceId1, bundlePersistenceId2);
         }
     }
-
-    @Test
-    public void testRebindRemovedItemWorksIfJavaClassFound() throws Exception {
-        Set<VersionedName> bundleItemsV1 = 
ImmutableSet.of(VersionedName.fromString("one:1.0.0"));
-        String bundleBomV1 = createCatalogYaml(ImmutableList.of(), 
bundleItemsV1);
-        VersionedName bundleNameV1 = new 
VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "1.0.0");
-        File bundleFileV1 = 
newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, 
bundleBomV1.getBytes(StandardCharsets.UTF_8)), bundleNameV1);
-        File initialBomFileV1 = 
newTmpFile(createCatalogYaml(ImmutableList.of(bundleFileV1.toURI()), 
ImmutableList.of()));
-
-        Set<VersionedName> bundleItemsV2 = 
ImmutableSet.of(VersionedName.fromString("one:2.0.0"));
-        String bundleBomV2 = createCatalogYaml(ImmutableList.of(), 
bundleItemsV2);
-        VersionedName bundleNameV2 = new 
VersionedName("org.example.testRebindGetsInitialOsgiCatalog", "2.0.0");
-        File bundleFileV2 = 
newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, 
bundleBomV2.getBytes(StandardCharsets.UTF_8)), bundleNameV2,
-            
ImmutableMap.of(BundleUpgradeParser.MANIFEST_HEADER_FORCE_REMOVE_BUNDLES, "*"));
-        File initialBomFileV2 = 
newTmpFile(createCatalogYaml(ImmutableList.of(bundleFileV2.toURI()), 
ImmutableList.of()));
-        
-        startT1(newLauncherForTests(initialBomFileV1.getAbsolutePath()));
-        Application app = 
createAndStartApplication(launcherLast.getManagementContext(), 
-            "services: [ { type: 'one:1.0.0' } ]");
-
-        // should start and promote fine, but entity's type shouldn't be 
loadable (and there will be warnings)
-        startT2(newLauncherForTests(initialBomFileV2.getAbsolutePath()));
-        promoteT2IfStandby();
-        
-        Entity entity = Iterables.getOnlyElement( 
Iterables.getOnlyElement(launcherLast.getManagementContext().getApplications()).getChildren()
 );
-        Assert.assertEquals(entity.getCatalogItemId(), 
bundleItemsV1.iterator().next());
-    }
     
     @Test
     public void testRebindRemovedItemButLeavingJavaSucceeds() throws Exception 
{
-        File initialBomFileV2 = 
prepForRebindRemovedItemTestReturningBomV2(true);
+        File initialBomFileV2 = 
prepForRebindRemovedItemTestReturningBomV2(false);
         createAndStartApplication(launcherLast.getManagementContext(), 
             "services: [ { type: 'simple-entity:1.0.0' } ]");
         
@@ -663,6 +644,7 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest 
extends AbstractBroo
         // TODO prep should set upgrade types and this should be 2.0.0
         Assert.assertEquals(entity.getCatalogItemId(), "simple-entity:1.0.0");
     }
+    
     @Test
     public void testRebindRemovedItemIncludingJavaFails() throws Exception {
         File initialBomFileV2 = 
prepForRebindRemovedItemTestReturningBomV2(true);
@@ -673,16 +655,28 @@ public abstract class 
BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo
         
         // TODO when we switch to loading from type registry types instead of 
persisted java type
         // T2 startup should succeed as it knows the type is now a BasicEntity
-        startT2(newLauncherForTests(initialBomFileV2.getAbsolutePath()));
-        assertHotStandbyNow(launcherLast, false);
+
+        // for now however:
+        if (isT1KeptRunningWhenT2Starts()) {
+            // if starting hot-standby, HA manager will clear all state and 
set it to FAILED state;
+            // for master/standalone/single it treats it as a startup 
subsystem error and does not
+            // so for hot-standby we should assert everything is cleared
+            startupAssertions = () -> {
+                
Asserts.assertSize(launcherLast.getManagementContext().getTypeRegistry().getAll(),
 0);
+                
Asserts.assertSize(launcherLast.getManagementContext().getEntityManager().getEntities(),
 0);
+            };
+        }
+        startT2(newLauncherForTests(initialBomFileV2.getAbsolutePath()), 
false);
         Assert.assertFalse( 
((ManagementContextInternal)launcherLast.getManagementContext()).errors().isEmpty()
 );
         
-        promoteT2IfStandby();
-        assertMasterEventually(launcherLast, false);
+        promoteT2IfStandby(false);
         Assert.assertFalse( 
((ManagementContextInternal)launcherLast.getManagementContext()).errors().isEmpty()
 );
 
-        // entity shouldn't be loaded
-        Asserts.assertSize( 
launcherLast.getManagementContext().getApplications(), 0 );
+        if (!isT1KeptRunningWhenT2Starts()) {
+            // if it did start, then entity shouldn't be loaded, though app is
+            Asserts.assertSize( 
launcherLast.getManagementContext().getApplications(), 1 );
+            Asserts.assertSize( 
Iterables.getOnlyElement(launcherLast.getManagementContext().getApplications()).getChildren(),
 0 );
+        }
     }
     
     private File prepForRebindRemovedItemTestReturningBomV2(boolean 
removeSourceJavaBundle) throws Exception {
@@ -718,7 +712,7 @@ public abstract class BrooklynLauncherRebindCatalogOsgiTest 
extends AbstractBroo
             VersionedName bv = new 
VersionedName("org.example.testRebindGetsInitialOsgiCatalog", v);
             VersionedName iv = new VersionedName("simple-entity", v);
             assertManagedBundle(launcherLast, bv, MutableSet.of(iv));
-            if (launcherT2==null) {
+            if (launcherT2==null || !removeSourceJavaBundle) {
                 assertCatalogConsistsOfIds(launcherLast, 
MutableList.copyOf(COM_EXAMPLE_BUNDLE_CATALOG_IDS).append(iv));
             } else {
                 assertCatalogConsistsOfIds(launcherLast, MutableList.of(iv));
@@ -747,34 +741,28 @@ public abstract class 
BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo
     }
 
     protected void assertHotStandbyNow(BrooklynLauncher launcher) {
-        assertHotStandbyNow(launcher, true);
-    }
-    protected void assertHotStandbyNow(BrooklynLauncher launcher, boolean 
assertHealthy) {
         ManagementContext mgmt = launcher.getManagementContext();
         assertTrue(mgmt.isStartupComplete());
         assertTrue(mgmt.isRunning());
         assertEquals(mgmt.getNodeState(), ManagementNodeState.HOT_STANDBY);
-        if (assertHealthy) {
-            Asserts.assertSize(((ManagementContextInternal)mgmt).errors(), 0);
-        }
+        Asserts.assertSize(((ManagementContextInternal)mgmt).errors(), 0);
     }
     
     protected void assertMasterEventually(BrooklynLauncher launcher) {
-        assertMasterEventually(launcher, true);
-    }
-    protected void assertMasterEventually(BrooklynLauncher launcher, boolean 
assertHealthy) {
         ManagementContext mgmt = launcher.getManagementContext();
         Asserts.succeedsEventually(new Runnable() {
             public void run() {
                 assertTrue(mgmt.isStartupComplete());
                 assertTrue(mgmt.isRunning());
                 assertEquals(mgmt.getNodeState(), ManagementNodeState.MASTER);
-                if (assertHealthy) {
-                    
Asserts.assertSize(((ManagementContextInternal)mgmt).errors(), 0);
-                }
+                Asserts.assertSize(((ManagementContextInternal)mgmt).errors(), 
0);
             }});
     }
     
+    protected void assertFailsEventually(BrooklynLauncher launcher) {
+        Asserts.succeedsEventually(() -> 
assertEquals(launcher.getManagementContext().getNodeState(), 
ManagementNodeState.FAILED));
+    }
+    
     protected Bundle installBundle(Framework framework, File bundle) throws 
Exception {
         try (FileInputStream stream = new FileInputStream(bundle)) {
             return 
framework.getBundleContext().installBundle(bundle.toURI().toString(), stream);
@@ -831,13 +819,14 @@ public abstract class 
BrooklynLauncherRebindCatalogOsgiTest extends AbstractBroo
         return newTmpBundle(ImmutableMap.of(BasicBrooklynCatalog.CATALOG_BOM, 
bundleBom.getBytes(StandardCharsets.UTF_8)), bundleName);
     }
 
+    // convenience for testing just a single test (TestNG plugin otherwise 
runs lots of them)
     public static void main(String[] args) throws Exception {
         try {
             BrooklynLauncherRebindCatalogOsgiTest fixture = new 
LauncherRebindSubTests();
             fixture.setUp();
             fixture.testRebindRemovedItemIncludingJavaFails();
             fixture.tearDown();
-        } catch (Exception e) {
+        } catch (Throwable e) {
             e.printStackTrace();
             System.exit(1);
         } 

Reply via email to