You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2014/11/03 16:52:03 UTC

[19/29] git commit: code review for brooklyn upgrade - tidy, fix tests, better state detection

code review for brooklyn upgrade - tidy, fix tests, better state detection


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

Branch: refs/heads/master
Commit: d3f6e34257091abca114eb59bcb148f1a7308ff8
Parents: 14f17bc
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Tue Oct 28 00:42:23 2014 -0700
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Fri Oct 31 09:39:50 2014 -0500

----------------------------------------------------------------------
 .../brooklyn/entity/proxying/EntitySpec.java    |  12 +-
 .../java/brooklyn/entity/basic/Entities.java    |  12 +
 .../brooklyn/entity/basic/EntityPredicates.java |   8 +-
 .../entity/proxying/EntityProxyImpl.java        |   6 +
 .../event/feed/AttributePollHandler.java        |   2 +-
 .../util/task/DynamicSequentialTask.java        |   2 +
 .../brooklynnode/BrooklynClusterImpl.java       |   9 +-
 .../entity/brooklynnode/BrooklynNodeImpl.java   |  10 +-
 .../brooklynnode/BrooklynNodeSshDriver.java     |   4 +-
 .../BrooklynClusterUpgradeEffectorBody.java     |  64 ++---
 .../BrooklynNodeUpgradeEffectorBody.java        |   2 +-
 .../effector/SelectMasterEffectorBody.java      |  28 +-
 .../entity/brooklynnode/brooklyn-cluster.yaml   |   1 +
 .../entity/brooklynnode/BrooklynNodeTest.java   |   6 +-
 .../brooklynnode/CallbackEntityHttpClient.java  |  95 +++++++
 .../entity/brooklynnode/MockBrooklynNode.java   |  68 +++++
 .../brooklynnode/SelectMasterEffectorTest.java  | 257 ++++++++++++++++++
 .../effector/CallbackEntityHttpClient.java      |  95 -------
 .../effector/SelectMasterEffectorTest.java      | 267 -------------------
 .../brooklynnode/effector/TestHttpEntity.java   |  66 -----
 .../rest/resources/CatalogResource.java         |   2 +-
 .../brooklyn/util/text/StringPredicates.java    |  23 +-
 .../util/text/StringPredicatesTest.java         |   6 +-
 23 files changed, 527 insertions(+), 518 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/api/src/main/java/brooklyn/entity/proxying/EntitySpec.java
----------------------------------------------------------------------
diff --git a/api/src/main/java/brooklyn/entity/proxying/EntitySpec.java b/api/src/main/java/brooklyn/entity/proxying/EntitySpec.java
index 7d65121..1b22387 100644
--- a/api/src/main/java/brooklyn/entity/proxying/EntitySpec.java
+++ b/api/src/main/java/brooklyn/entity/proxying/EntitySpec.java
@@ -42,7 +42,6 @@ import brooklyn.policy.EnricherSpec;
 import brooklyn.policy.Policy;
 import brooklyn.policy.PolicySpec;
 
-import com.google.common.annotations.Beta;
 import com.google.common.base.Supplier;
 import com.google.common.base.Throwables;
 import com.google.common.collect.Iterables;
@@ -102,7 +101,7 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E
     }
     
     /**
-     * Wraps an entity spec so its configuration can be overridden without modifying the 
+     * Copies entity spec so its configuration can be overridden without modifying the 
      * original entity spec.
      */
     public static <T extends Entity> EntitySpec<T> create(EntitySpec<T> spec) {
@@ -230,11 +229,10 @@ public class EntitySpec<T extends Entity> extends AbstractBrooklynObjectSpec<T,E
     public Map<ConfigKey<?>, Object> getConfig() {
         return Collections.unmodifiableMap(config);
     }
-    
-    /** @return Live instance of the config map, for instance in case mass clearances are desired */
-    @Beta
-    public Map<ConfigKey<?>, Object> getConfigLive() {
-        return config;
+
+    /** Clears the config map, removing any config previously set. */
+    public void clearConfig() {
+        config.clear();
     }
         
     public List<PolicySpec<?>> getPolicySpecs() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/core/src/main/java/brooklyn/entity/basic/Entities.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/Entities.java b/core/src/main/java/brooklyn/entity/basic/Entities.java
index 77a676d..7d17b7a 100644
--- a/core/src/main/java/brooklyn/entity/basic/Entities.java
+++ b/core/src/main/java/brooklyn/entity/basic/Entities.java
@@ -22,6 +22,7 @@ import java.io.Closeable;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.io.Writer;
+import java.lang.reflect.Proxy;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -51,6 +52,7 @@ import brooklyn.entity.Group;
 import brooklyn.entity.drivers.EntityDriver;
 import brooklyn.entity.drivers.downloads.DownloadResolver;
 import brooklyn.entity.effector.Effectors;
+import brooklyn.entity.proxying.EntityProxyImpl;
 import brooklyn.entity.trait.Startable;
 import brooklyn.entity.trait.StartableMethods;
 import brooklyn.event.AttributeSensor;
@@ -91,6 +93,7 @@ import brooklyn.util.task.system.SystemTasks;
 import brooklyn.util.time.Duration;
 
 import com.google.common.annotations.Beta;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
@@ -768,6 +771,15 @@ public class Entities {
         return ((EntityInternal)e).getManagementSupport().isReadOnly();
     }
 
+    /** Unwraps a proxy to retrieve the real item, if available.
+     * <p>
+     * Only intended for use in tests. For normal operations, callers should ensure the method is
+     * available on an interface and accessed via the proxy. */
+    @Beta @VisibleForTesting
+    public static AbstractEntity deproxy(Entity e) {
+        return (AbstractEntity) ((EntityProxyImpl)Proxy.getInvocationHandler(e)).getDelegate();
+    }
+    
     /**
      * Brings this entity under management only if its ancestor is managed.
      * <p>

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/core/src/main/java/brooklyn/entity/basic/EntityPredicates.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/EntityPredicates.java b/core/src/main/java/brooklyn/entity/basic/EntityPredicates.java
index b81b835..12b656d 100644
--- a/core/src/main/java/brooklyn/entity/basic/EntityPredicates.java
+++ b/core/src/main/java/brooklyn/entity/basic/EntityPredicates.java
@@ -343,7 +343,7 @@ public class EntityPredicates {
      * Create a predicate that matches any entity who has an exact match for the given location
      * (i.e. {@code entity.getLocations().contains(location)}).
      */
-    public static <T> Predicate<Entity> locationsInclude(Location location) {
+    public static <T> Predicate<Entity> locationsIncludes(Location location) {
         return locationsSatisfy(CollectionFunctionals.contains(location));
         
     }
@@ -367,13 +367,13 @@ public class EntityPredicates {
         }
     }
 
-    /** @deprecated since 0.7.0 use {@link #locationsInclude(Location)} */
+    /** @deprecated since 0.7.0 use {@link #locationsIncludes(Location)} */
     @Deprecated 
     public static <T> Predicate<Entity> withLocation(final Location location) {
-        return locationsInclude(location);
+        return locationsIncludes(location);
     }
     
-    /** @deprecated since 0.7.0 use {@link #locationsInclude(Location)}, introduced to allow deserialization of anonymous inner class */
+    /** @deprecated since 0.7.0 use {@link #locationsIncludes(Location)}, introduced to allow deserialization of anonymous inner class */
     @SuppressWarnings("unused") @Deprecated 
     private static <T> Predicate<Entity> withLocationOld(final Location location) {
         return new SerializablePredicate<Entity>() {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/core/src/main/java/brooklyn/entity/proxying/EntityProxyImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/proxying/EntityProxyImpl.java b/core/src/main/java/brooklyn/entity/proxying/EntityProxyImpl.java
index 19188ab..4163204 100644
--- a/core/src/main/java/brooklyn/entity/proxying/EntityProxyImpl.java
+++ b/core/src/main/java/brooklyn/entity/proxying/EntityProxyImpl.java
@@ -48,6 +48,7 @@ import brooklyn.util.config.ConfigBag;
 import brooklyn.util.task.DynamicTasks;
 import brooklyn.util.task.TaskTags;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
 import com.google.common.collect.Sets;
 
@@ -252,6 +253,11 @@ public class EntityProxyImpl implements java.lang.reflect.InvocationHandler {
         }
     }
     
+    @VisibleForTesting
+    public Entity getDelegate() {
+        return delegate;
+    }
+    
     @Override
     public boolean equals(Object obj) {
         return delegate.equals(obj);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/core/src/main/java/brooklyn/event/feed/AttributePollHandler.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/event/feed/AttributePollHandler.java b/core/src/main/java/brooklyn/event/feed/AttributePollHandler.java
index 7252d20..19538b3 100644
--- a/core/src/main/java/brooklyn/event/feed/AttributePollHandler.java
+++ b/core/src/main/java/brooklyn/event/feed/AttributePollHandler.java
@@ -196,7 +196,7 @@ public class AttributePollHandler<V> implements PollHandler<V> {
 
     @SuppressWarnings("unchecked")
     protected void setSensor(Object v) {
-        if (!Entities.isManaged(entity)) {
+        if (Entities.isNoLongerManaged(entity)) {
             if (Tasks.isInterrupted()) return;
             log.warn(""+entity+" is not managed; feed "+this+" setting "+sensor+" to "+v+" at this time is not supported ("+Tasks.current()+")");
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/core/src/main/java/brooklyn/util/task/DynamicSequentialTask.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/util/task/DynamicSequentialTask.java b/core/src/main/java/brooklyn/util/task/DynamicSequentialTask.java
index 266921b..34d8912 100644
--- a/core/src/main/java/brooklyn/util/task/DynamicSequentialTask.java
+++ b/core/src/main/java/brooklyn/util/task/DynamicSequentialTask.java
@@ -138,6 +138,8 @@ public class DynamicSequentialTask<T> extends BasicTask<T> implements HasTaskChi
         synchronized (jobTransitionLock) {
             if (primaryFinished)
                 throw new IllegalStateException("Cannot add a task to "+this+" which is already finished (trying to add "+t+")");
+            if (secondaryQueueAborted)
+                throw new IllegalStateException("Cannot add a task to "+this+" whose queue has been aborted (trying to add "+t+")");
             secondaryJobsAll.add(t);
             secondaryJobsRemaining.add(t);
             BrooklynTaskTags.addTagsDynamically(t, ManagementContextInternal.SUB_TASK_TAG);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynClusterImpl.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynClusterImpl.java b/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynClusterImpl.java
index 48c9472..61b42d5 100644
--- a/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynClusterImpl.java
+++ b/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynClusterImpl.java
@@ -50,9 +50,6 @@ public class BrooklynClusterImpl extends DynamicClusterImpl implements BrooklynC
 
     // TODO should we set a default MEMBER_SPEC ?  difficult though because we'd need to set a password
 
-    @SuppressWarnings("unused")
-    private FunctionFeed scanMaster;
-
     @Override
     public void init() {
         super.init();
@@ -60,12 +57,12 @@ public class BrooklynClusterImpl extends DynamicClusterImpl implements BrooklynC
         getMutableEntityType().addEffector(BrooklynClusterUpgradeEffectorBody.UPGRADE_CLUSTER);
 
         ServiceProblemsLogic.updateProblemsIndicator(this, MASTER_NODE, MSG_NO_MASTER);
-        scanMaster = FunctionFeed.builder()
+        addFeed(FunctionFeed.builder()
                 .entity(this)
                 .poll(new FunctionPollConfig<Object, BrooklynNode>(MASTER_NODE)
                         .period(Duration.ONE_SECOND)
                         .callable(new MasterChildFinder()))
-                .build();
+                .build());
         
         addEnricher( Enrichers.builder().transforming(MASTER_NODE)
             .uniqueTag("master-node-web-uri")
@@ -81,7 +78,7 @@ public class BrooklynClusterImpl extends DynamicClusterImpl implements BrooklynC
         }
     }
 
-    private BrooklynNode findMasterChild() {
+    BrooklynNode findMasterChild() {
         Collection<Entity> masters = FluentIterable.from(getMembers())
                 .filter(EntityPredicates.attributeEqualTo(BrooklynNode.MANAGEMENT_NODE_STATE, ManagementNodeState.MASTER))
                 .toList();

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java b/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java
index fac22ee..395949d 100644
--- a/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java
+++ b/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeImpl.java
@@ -108,6 +108,11 @@ public class BrooklynNodeImpl extends SoftwareProcessImpl implements BrooklynNod
     }
 
     @Override
+    protected void preStart() {
+        ServiceNotUpLogic.clearNotUpIndicator(this, SHUTDOWN.getName());
+    }
+    
+    @Override
     protected void preStop() {
         super.preStop();
 
@@ -233,11 +238,6 @@ public class BrooklynNodeImpl extends SoftwareProcessImpl implements BrooklynNod
 
     }
 
-    @Override
-    protected void preStart() {
-        ServiceNotUpLogic.clearNotUpIndicator(this, SHUTDOWN.getName());
-    }
-    
     public static class StopNodeButLeaveAppsEffectorBody extends EffectorBody<Void> implements StopNodeButLeaveAppsEffector {
         public static final Effector<Void> STOP_NODE_BUT_LEAVE_APPS = Effectors.effector(BrooklynNode.STOP_NODE_BUT_LEAVE_APPS).impl(new StopNodeButLeaveAppsEffectorBody()).build();
 

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeSshDriver.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeSshDriver.java b/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeSshDriver.java
index d5d9c01..1fbf694 100644
--- a/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeSshDriver.java
+++ b/software/base/src/main/java/brooklyn/entity/brooklynnode/BrooklynNodeSshDriver.java
@@ -224,7 +224,7 @@ public class BrooklynNodeSshDriver extends JavaSoftwareProcessSshDriver implemen
         }
         
         String cmd = entity.getConfig(BrooklynNode.EXTRA_CUSTOMIZATION_SCRIPT);
-        if (!Strings.isBlank(cmd)) {
+        if (Strings.isNonBlank(cmd)) {
             DynamicTasks.queueIfPossible( SshEffectorTasks.ssh(cmd).summary("Bespoke BrooklynNode customization script")
                 .requiringExitCodeZero() )
                 .orSubmitAndBlock(getEntity());
@@ -285,7 +285,7 @@ public class BrooklynNodeSshDriver extends JavaSoftwareProcessSshDriver implemen
         if (getEntity().getConfig(BrooklynNode.NO_SHUTDOWN_ON_EXIT)) {
             cmd += " --noShutdownOnExit ";
         }
-        if (!Strings.isBlank(getEntity().getConfig(BrooklynNode.EXTRA_LAUNCH_PARAMETERS))) {
+        if (Strings.isNonBlank(getEntity().getConfig(BrooklynNode.EXTRA_LAUNCH_PARAMETERS))) {
             cmd += " "+getEntity().getConfig(BrooklynNode.EXTRA_LAUNCH_PARAMETERS);
         }
         cmd += format(" >> %s/console 2>&1 </dev/null &", getRunDir());

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynClusterUpgradeEffectorBody.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynClusterUpgradeEffectorBody.java b/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynClusterUpgradeEffectorBody.java
index 48553b3..9fac5a4 100644
--- a/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynClusterUpgradeEffectorBody.java
+++ b/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynClusterUpgradeEffectorBody.java
@@ -21,7 +21,6 @@ package brooklyn.entity.brooklynnode.effector;
 import java.io.File;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.concurrent.Callable;
 import java.util.concurrent.atomic.AtomicBoolean;
 
@@ -49,7 +48,6 @@ import brooklyn.management.ha.HighAvailabilityMode;
 import brooklyn.management.ha.ManagementNodeState;
 import brooklyn.util.collections.MutableMap;
 import brooklyn.util.config.ConfigBag;
-import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.net.Urls;
 import brooklyn.util.task.DynamicTasks;
 import brooklyn.util.task.Tasks;
@@ -67,7 +65,7 @@ public class BrooklynClusterUpgradeEffectorBody extends EffectorBody<Void> imple
     public static final Effector<Void> UPGRADE_CLUSTER = Effectors.effector(UpgradeClusterEffector.UPGRADE_CLUSTER)
         .impl(new BrooklynClusterUpgradeEffectorBody()).build();
 
-    private AtomicBoolean upgradeInProgress = new AtomicBoolean();
+    private final AtomicBoolean upgradeInProgress = new AtomicBoolean();
 
     @Override
     public Void call(ConfigBag parameters) {
@@ -75,37 +73,36 @@ public class BrooklynClusterUpgradeEffectorBody extends EffectorBody<Void> imple
             throw new IllegalStateException("An upgrade is already in progress.");
         }
 
-        EntitySpec<?> memberSpec = entity().getConfig(BrooklynCluster.MEMBER_SPEC);
-        Preconditions.checkNotNull(memberSpec, BrooklynCluster.MEMBER_SPEC.getName() + " is required for " + UpgradeClusterEffector.class.getName());
+        EntitySpec<?> origMemberSpec = entity().getConfig(BrooklynCluster.MEMBER_SPEC);
+        Preconditions.checkNotNull(origMemberSpec, BrooklynCluster.MEMBER_SPEC.getName() + " is required for " + UpgradeClusterEffector.class.getName());
 
-        ConfigBag specCfg = ConfigBag.newInstance( memberSpec.getConfig() );
-        ConfigBag origSpecCfg = ConfigBag.newInstanceCopying(specCfg);
-        log.debug("Upgrading "+entity()+", changing "+BrooklynCluster.MEMBER_SPEC+" from "+memberSpec+" / "+origSpecCfg);
-        
+        log.debug("Upgrading "+entity()+", changing "+BrooklynCluster.MEMBER_SPEC+" from "+origMemberSpec+" / "+origMemberSpec.getConfig());
+
+        boolean success = false;
         try {
             String newDownloadUrl = parameters.get(DOWNLOAD_URL);
-            specCfg.putIfNotNull(DOWNLOAD_URL, newDownloadUrl);
-            specCfg.put(BrooklynNode.DISTRO_UPLOAD_URL, inferUploadUrl(newDownloadUrl));
             
-            specCfg.putAll(ConfigBag.newInstance(parameters.get(EXTRA_CONFIG)).getAllConfigAsConfigKeyMap());
+            EntitySpec<?> newMemberSpec = EntitySpec.create(origMemberSpec);
             
-            memberSpec.clearConfig();
-            memberSpec.configure(specCfg.getAllConfigAsConfigKeyMap());
-            // not necessary, but good practice
-            entity().setConfig(BrooklynCluster.MEMBER_SPEC, memberSpec);
-            log.debug("Upgrading "+entity()+", new "+BrooklynCluster.MEMBER_SPEC+": "+memberSpec+" / "+specCfg);
+            ConfigBag newConfig = ConfigBag.newInstance();
+            newConfig.putIfNotNull(DOWNLOAD_URL, newDownloadUrl);
+            newConfig.put(BrooklynNode.DISTRO_UPLOAD_URL, inferUploadUrl(newDownloadUrl));
+            newConfig.putAll(ConfigBag.newInstance(parameters.get(EXTRA_CONFIG)).getAllConfigAsConfigKeyMap());
+            newMemberSpec.configure(newConfig.getAllConfigAsConfigKeyMap());
             
-            upgrade(parameters);
-        } catch (Exception e) {
-            log.debug("Upgrading "+entity()+" failed, will rethrow after restoring "+BrooklynCluster.MEMBER_SPEC+" to: "+origSpecCfg);
-            memberSpec.clearConfig();
-            memberSpec.configure(origSpecCfg.getAllConfigAsConfigKeyMap());
-            // not necessary, but good practice
-            entity().setConfig(BrooklynCluster.MEMBER_SPEC, memberSpec);
+            entity().setConfig(BrooklynCluster.MEMBER_SPEC, newMemberSpec);
             
-            throw Exceptions.propagate(e);
+            log.debug("Upgrading "+entity()+", new "+BrooklynCluster.MEMBER_SPEC+": "+newMemberSpec+" / "+newMemberSpec.getConfig()+" (adding: "+newConfig+")");
             
+            upgrade(parameters);
+
+            success = true;
         } finally {
+            if (!success) {
+                log.debug("Upgrading "+entity()+" failed, will rethrow after restoring "+BrooklynCluster.MEMBER_SPEC+" to: "+origMemberSpec);
+                entity().setConfig(BrooklynCluster.MEMBER_SPEC, origMemberSpec);
+            }
+            
             upgradeInProgress.set(false);
         }
         return null;
@@ -135,11 +132,15 @@ public class BrooklynClusterUpgradeEffectorBody extends EffectorBody<Void> imple
                 new IllegalStateException("Persistence does not appear to be enabled at this cluster. "
                 + "Cluster upgrade will not succeed unless a custom launch script enables it.")) );
         }
+       
+        //TODO we'd like to disable these nodes as standby targets, ie in some 'hot standby but not available for failover' mode
+        //currently if failover happens to a new node, assumptions below may fail and the cluster may require manual repair
 
         //1. Initially create a single node to check if it will launch successfully
         TaskAdaptable<Collection<Entity>> initialNodeTask = DynamicTasks.queue(newCreateNodesTask(1, "Creating first upgraded version node"));
 
         //2. If everything is OK with the first node launch the rest as well
+        @SuppressWarnings("unused")
         TaskAdaptable<Collection<Entity>> remainingNodesTask = DynamicTasks.queue(newCreateNodesTask(initialClusterSize - 1, "Creating remaining upgraded version nodes ("+(initialClusterSize - 1)+")"));
 
         //3. Once we have all nodes running without errors switch master
@@ -151,13 +152,9 @@ public class BrooklynClusterUpgradeEffectorBody extends EffectorBody<Void> imple
         //   For members that were created meanwhile - they will be using the new version already. If the new version
         //   isn't good then they will fail to start as well, forcing the policies to retry (and succeed once the
         //   URL is reverted).
-        //TODO can get into problem state if more old nodes are created; better might be to set the
-        //version on this cluster before the above select-master call, and then delete any which are running the old
-        //version (would require tracking the version number at the entity)
-        HashSet<Entity> oldMembers = new HashSet<Entity>(initialMembers);
-        oldMembers.removeAll(remainingNodesTask.asTask().getUnchecked());
-        oldMembers.removeAll(initialNodeTask.asTask().getUnchecked());
-        DynamicTasks.queue(Effectors.invocation(BrooklynNode.STOP_NODE_BUT_LEAVE_APPS, Collections.emptyMap(), oldMembers)).asTask().getUnchecked();
+        
+        //any other nodes created via other means should also be using the new spec, so initialMembers will be all the old version nodes
+        DynamicTasks.queue(Effectors.invocation(BrooklynNode.STOP_NODE_BUT_LEAVE_APPS, Collections.emptyMap(), initialMembers)).asTask().getUnchecked();
     }
 
     private TaskAdaptable<Collection<Entity>> newCreateNodesTask(int size, String name) {
@@ -187,6 +184,7 @@ public class BrooklynClusterUpgradeEffectorBody extends EffectorBody<Void> imple
                 Predicates.not(Predicates.equalTo(Lifecycle.STARTING)), Duration.minutes(30)));
 
         //3. Set HOT_STANDBY in case it is not enabled on the command line ...
+        // TODO support via EntitySpec
         DynamicTasks.queue(Effectors.invocation(
                 BrooklynNode.SET_HIGH_AVAILABILITY_MODE,
                 MutableMap.of(SetHighAvailabilityModeEffector.MODE, HighAvailabilityMode.HOT_STANDBY), 
@@ -196,6 +194,8 @@ public class BrooklynClusterUpgradeEffectorBody extends EffectorBody<Void> imple
         DynamicTasks.queue(EntityTasks.requiringAttributeEventually(newNodes, BrooklynNode.MANAGEMENT_NODE_STATE, 
                 Predicates.equalTo(ManagementNodeState.HOT_STANDBY), Duration.FIVE_MINUTES));
 
+        // TODO also check that the nodes created all report the original master, in case persistence changes it
+        
         //5. Just in case check if all of the nodes are SERVICE_UP (which would rule out ON_FIRE as well)
         Collection<Entity> failedNodes = Collections2.filter(newNodes, EntityPredicates.attributeEqualTo(BrooklynNode.SERVICE_UP, Boolean.FALSE));
         if (!failedNodes.isEmpty()) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynNodeUpgradeEffectorBody.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynNodeUpgradeEffectorBody.java b/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynNodeUpgradeEffectorBody.java
index 00ab7bc..22afa15 100644
--- a/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynNodeUpgradeEffectorBody.java
+++ b/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/BrooklynNodeUpgradeEffectorBody.java
@@ -170,7 +170,7 @@ public class BrooklynNodeUpgradeEffectorBody extends EffectorBody<Void> {
     @Beta
     static boolean isPersistenceModeEnabled(Entity entity) {
         // TODO when there are PERSIST* options in BrooklynNode, look at them here!
-        // or, better, have a sensor for persistence
+        // or, even better, make a REST call to check persistence
         String params = null;
         if (entity instanceof BrooklynCluster) {
             EntitySpec<?> spec = entity.getConfig(BrooklynCluster.MEMBER_SPEC);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorBody.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorBody.java b/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorBody.java
index 40d65a7..30f5f2d 100644
--- a/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorBody.java
+++ b/software/base/src/main/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorBody.java
@@ -45,6 +45,7 @@ import brooklyn.util.task.DynamicTasks;
 import brooklyn.util.time.Duration;
 
 import com.google.api.client.util.Preconditions;
+import com.google.common.base.Objects;
 import com.google.common.collect.Iterables;
 
 public class SelectMasterEffectorBody extends EffectorBody<Void> implements SelectMasterEffector {
@@ -84,20 +85,21 @@ public class SelectMasterEffectorBody extends EffectorBody<Void> implements Sele
         final Entity newMaster = getMember(newMasterId);
 
         //1. Increase the priority of the node we wish to become master
-        setNodePriority(newMaster, HA_MASTER_PRIORITY);
+        toggleNodePriority(newMaster, HA_MASTER_PRIORITY);
 
-        //2. Denote the existing master so a new election takes place
+        //2. Demote the existing master so a new election takes place
         try {
-            //If no master was yet selected, at least wait to see
-            //if the new master will be what we expect.
+            // this allows the finally block to run even on failure
+            DynamicTasks.swallowChildrenFailures();
+            
             if (oldMaster != null) {
-                setNodeState(oldMaster, HighAvailabilityMode.HOT_STANDBY);
+                demoteOldMaster(oldMaster, HighAvailabilityMode.HOT_STANDBY);
             }
 
             waitMasterHandover(oldMaster, newMaster);
-        } finally {
+        } finally { 
             //3. Revert the priority of the node once it has become master
-            setNodePriority(newMaster, HA_STANDBY_PRIORITY);
+            toggleNodePriority(newMaster, HA_STANDBY_PRIORITY);
         }
 
         checkMasterSelected(newMaster);
@@ -105,13 +107,13 @@ public class SelectMasterEffectorBody extends EffectorBody<Void> implements Sele
 
     private void waitMasterHandover(final Entity oldMaster, final Entity newMaster) {
         boolean masterChanged = Repeater.create()
-            .backoff(Duration.millis(500), 1.2, Duration.FIVE_SECONDS)
+            .backoff(Duration.millis(50), 1.5, Duration.FIVE_SECONDS)
             .limitTimeTo(Duration.ONE_MINUTE)
             .until(new Callable<Boolean>() {
                 @Override
                 public Boolean call() throws Exception {
                     Entity master = getMasterNode();
-                    return master != oldMaster && master != null;
+                    return !Objects.equal(master, oldMaster) && master != null;
                 }
             })
             .run();
@@ -120,7 +122,7 @@ public class SelectMasterEffectorBody extends EffectorBody<Void> implements Sele
         }
     }
 
-    private void setNodeState(Entity oldMaster, HighAvailabilityMode mode) {
+    private void demoteOldMaster(Entity oldMaster, HighAvailabilityMode mode) {
         ManagementNodeState oldState = DynamicTasks.queue(
                 Effectors.invocation(
                         oldMaster,
@@ -134,17 +136,17 @@ public class SelectMasterEffectorBody extends EffectorBody<Void> implements Sele
         }
     }
 
-    private void setNodePriority(Entity newMaster, int newPriority) {
+    private void toggleNodePriority(Entity node, int newPriority) {
         Integer oldPriority = DynamicTasks.queue(
                 Effectors.invocation(
-                    newMaster,
+                    node,
                     BrooklynNode.SET_HIGH_AVAILABILITY_PRIORITY,
                     MutableMap.of(SetHighAvailabilityPriorityEffector.PRIORITY, newPriority))
             ).asTask().getUnchecked();
 
         Integer expectedPriority = (newPriority == HA_MASTER_PRIORITY ? HA_STANDBY_PRIORITY : HA_MASTER_PRIORITY);
         if (oldPriority != expectedPriority) {
-            LOG.warn("The previous HA priority on node " + newMaster.getId() + " was " + oldPriority +
+            LOG.warn("The previous HA priority on node " + node.getId() + " was " + oldPriority +
                     ", while the expected value is " + expectedPriority + " (while setting priority " +
                     newPriority + ").");
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/main/resources/brooklyn/entity/brooklynnode/brooklyn-cluster.yaml
----------------------------------------------------------------------
diff --git a/software/base/src/main/resources/brooklyn/entity/brooklynnode/brooklyn-cluster.yaml b/software/base/src/main/resources/brooklyn/entity/brooklynnode/brooklyn-cluster.yaml
index f25a879..cb32596 100644
--- a/software/base/src/main/resources/brooklyn/entity/brooklynnode/brooklyn-cluster.yaml
+++ b/software/base/src/main/resources/brooklyn/entity/brooklynnode/brooklyn-cluster.yaml
@@ -27,6 +27,7 @@ services:
 #      type: classpath://brooklyn/entity/brooklynnode/brooklyn-node-persisting-to-tmp.yaml
       type: brooklyn.entity.brooklynnode.BrooklynNode
       brooklyn.config:
+        # persistence location must be the same for all nodes; if anywhere other than localhost configure a shared obj store or nfs mount
         brooklynnode.launch.parameters.extra: --persist auto --persistenceDir /tmp/brooklyn-persistence-example/
 
 # location: localhost

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/test/java/brooklyn/entity/brooklynnode/BrooklynNodeTest.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/brooklyn/entity/brooklynnode/BrooklynNodeTest.java b/software/base/src/test/java/brooklyn/entity/brooklynnode/BrooklynNodeTest.java
index 21e3f34..d1a9390 100644
--- a/software/base/src/test/java/brooklyn/entity/brooklynnode/BrooklynNodeTest.java
+++ b/software/base/src/test/java/brooklyn/entity/brooklynnode/BrooklynNodeTest.java
@@ -58,14 +58,14 @@ public class BrooklynNodeTest {
     
     @Test
     public void testGeneratesCorrectSnapshotDownload() throws Exception {
-        String version = "0.7.0-SNAPSHOT"; // BROOKLYN_VERSION
+        String version = "0.0.1-SNAPSHOT";
         String expectedUrl = "https://repository.apache.org/service/local/artifact/maven/redirect?r=snapshots&g=org.apache.brooklyn&v="+version+"&a=brooklyn-dist&c=dist&e=tar.gz";
         runTestGeneratesCorrectDownloadUrl(version, expectedUrl);
     }
     
     @Test
     public void testGeneratesCorrectReleaseDownload() throws Exception {
-        String version = "0.7.0";
+        String version = "0.0.1";
         String expectedUrl = "http://search.maven.org/remotecontent?filepath=org/apache/brooklyn/brooklyn-dist/"+version+"/brooklyn-dist-"+version+"-dist.tar.gz";
         runTestGeneratesCorrectDownloadUrl(version, expectedUrl);
     }
@@ -74,7 +74,7 @@ public class BrooklynNodeTest {
         // TODO Using BrooklynNodeImpl directly, because want to instantiate a BroolynNodeSshDriver.
         //      Really want to make that easier to test, without going through "wrong" code path for creating entity.
         BrooklynNodeImpl entity = new BrooklynNodeImpl();
-        entity.configure(MutableMap.of("version", version));
+        entity.configure(BrooklynNode.SUGGESTED_VERSION, version);
         entity.setParent(app);
         Entities.manage(entity);
         ConfigToAttributes.apply(entity);

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/test/java/brooklyn/entity/brooklynnode/CallbackEntityHttpClient.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/brooklyn/entity/brooklynnode/CallbackEntityHttpClient.java b/software/base/src/test/java/brooklyn/entity/brooklynnode/CallbackEntityHttpClient.java
new file mode 100644
index 0000000..737e4f1
--- /dev/null
+++ b/software/base/src/test/java/brooklyn/entity/brooklynnode/CallbackEntityHttpClient.java
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package brooklyn.entity.brooklynnode;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.http.HttpStatus;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+
+import brooklyn.entity.Entity;
+import brooklyn.entity.brooklynnode.EntityHttpClient;
+import brooklyn.util.http.HttpTool.HttpClientBuilder;
+import brooklyn.util.http.HttpToolResponse;
+
+import com.google.common.base.Function;
+
+public class CallbackEntityHttpClient implements EntityHttpClient {
+    public static class Request {
+        private Entity entity;
+        private String method;
+        private String path;
+        private Map<String, String> params;
+        public Request(Entity entity, String method, String path, Map<String, String> params) {
+            this.entity = entity;
+            this.method = method;
+            this.path = path;
+            this.params = params;
+        }
+        public Entity getEntity() {
+            return entity;
+        }
+        public String getMethod() {
+            return method;
+        }
+        public String getPath() {
+            return path;
+        }
+        public Map<String, String> getParams() {
+            return params;
+        }
+    }
+    private Function<Request, String> callback;
+    private Entity entity;
+
+    public CallbackEntityHttpClient(Entity entity, Function<Request, String> callback) {
+        this.entity = entity;
+        this.callback = callback;
+    }
+
+    @Override
+    public HttpClientBuilder getHttpClientForBrooklynNode() {
+        throw new IllegalStateException("Method call not expected");
+    }
+
+    @Override
+    public HttpToolResponse get(String path) {
+        String result = callback.apply(new Request(entity, HttpGet.METHOD_NAME, path, Collections.<String, String>emptyMap()));
+        return new HttpToolResponse(HttpStatus.SC_OK, null, result.getBytes(), 0, 0, 0);
+    }
+
+    @Override
+    public HttpToolResponse post(String path, Map<String, String> headers, byte[] body) {
+        throw new IllegalStateException("Method call not expected");
+    }
+
+    @Override
+    public HttpToolResponse post(String path, Map<String, String> headers, Map<String, String> formParams) {
+        String result = callback.apply(new Request(entity, HttpPost.METHOD_NAME, path, formParams));
+        return new HttpToolResponse(HttpStatus.SC_OK, Collections.<String, List<String>>emptyMap(), result.getBytes(), 0, 0, 0);
+    }
+    
+    @Override
+    public HttpToolResponse delete(String path, Map<String, String> headers) {
+        throw new IllegalStateException("Method call not expected");
+    }
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/test/java/brooklyn/entity/brooklynnode/MockBrooklynNode.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/brooklyn/entity/brooklynnode/MockBrooklynNode.java b/software/base/src/test/java/brooklyn/entity/brooklynnode/MockBrooklynNode.java
new file mode 100644
index 0000000..c75131e
--- /dev/null
+++ b/software/base/src/test/java/brooklyn/entity/brooklynnode/MockBrooklynNode.java
@@ -0,0 +1,68 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package brooklyn.entity.brooklynnode;
+
+import java.util.Collection;
+
+import brooklyn.config.ConfigKey;
+import brooklyn.entity.basic.AbstractEntity;
+import brooklyn.entity.basic.ConfigKeys;
+import brooklyn.entity.brooklynnode.BrooklynNode;
+import brooklyn.entity.brooklynnode.EntityHttpClient;
+import brooklyn.entity.brooklynnode.CallbackEntityHttpClient.Request;
+import brooklyn.entity.brooklynnode.effector.SetHighAvailabilityModeEffectorBody;
+import brooklyn.entity.brooklynnode.effector.SetHighAvailabilityPriorityEffectorBody;
+import brooklyn.event.AttributeSensor;
+import brooklyn.event.basic.BasicAttributeSensor;
+import brooklyn.location.Location;
+
+import com.google.common.base.Function;
+import com.google.common.reflect.TypeToken;
+
+public class MockBrooklynNode extends AbstractEntity implements BrooklynNode {
+    @SuppressWarnings("serial")
+    public static final ConfigKey<Function<Request, String>> HTTP_CLIENT_CALLBACK = ConfigKeys.newConfigKey(new TypeToken<Function<Request, String>>(){}, "httpClientCallback");
+    public static final AttributeSensor<Integer> HA_PRIORITY = new BasicAttributeSensor<Integer>(Integer.class, "priority");
+    
+    @Override
+    public void init() {
+        super.init();
+        getMutableEntityType().addEffector(SetHighAvailabilityPriorityEffectorBody.SET_HIGH_AVAILABILITY_PRIORITY);
+        getMutableEntityType().addEffector(SetHighAvailabilityModeEffectorBody.SET_HIGH_AVAILABILITY_MODE);
+        setAttribute(HA_PRIORITY, 0);
+    }
+
+    @Override
+    public EntityHttpClient http() {
+        return new CallbackEntityHttpClient(this, getConfig(HTTP_CLIENT_CALLBACK));
+    }
+
+    @Override
+    public void start(Collection<? extends Location> locations) {
+    }
+
+    @Override
+    public void stop() {
+    }
+
+    @Override
+    public void restart() {
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/test/java/brooklyn/entity/brooklynnode/SelectMasterEffectorTest.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/brooklyn/entity/brooklynnode/SelectMasterEffectorTest.java b/software/base/src/test/java/brooklyn/entity/brooklynnode/SelectMasterEffectorTest.java
new file mode 100644
index 0000000..84b763d
--- /dev/null
+++ b/software/base/src/test/java/brooklyn/entity/brooklynnode/SelectMasterEffectorTest.java
@@ -0,0 +1,257 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package brooklyn.entity.brooklynnode;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.Callable;
+
+import org.apache.http.client.methods.HttpPost;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.Test;
+
+import brooklyn.entity.BrooklynAppUnitTestSupport;
+import brooklyn.entity.Entity;
+import brooklyn.entity.Group;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityLocal;
+import brooklyn.entity.brooklynnode.BrooklynCluster.SelectMasterEffector;
+import brooklyn.entity.brooklynnode.CallbackEntityHttpClient.Request;
+import brooklyn.entity.effector.Effectors;
+import brooklyn.entity.group.DynamicCluster;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.event.feed.AttributePollHandler;
+import brooklyn.event.feed.DelegatingPollHandler;
+import brooklyn.event.feed.Poller;
+import brooklyn.management.ha.ManagementNodeState;
+import brooklyn.test.EntityTestUtils;
+import brooklyn.util.collections.MutableList;
+import brooklyn.util.time.Duration;
+
+import com.google.common.base.Function;
+import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableMap;
+
+public class SelectMasterEffectorTest extends BrooklynAppUnitTestSupport {
+    private static final Logger LOG = LoggerFactory.getLogger(BrooklynClusterImpl.class);
+
+    protected BrooklynCluster cluster;
+    protected HttpCallback http; 
+    protected Poller<Void> poller;
+
+    @BeforeMethod(alwaysRun=true)
+    public void setUp() throws Exception {
+        super.setUp();
+
+        // because the effector calls wait for a state change, use a separate thread to drive that 
+        poller = new Poller<Void>((EntityLocal)app, false);
+        poller.scheduleAtFixedRate(
+            new Callable<Void>() {
+                @Override
+                public Void call() throws Exception {
+                    masterFailoverIfNeeded();
+                    return null;
+                }
+            },
+            new DelegatingPollHandler<Void>(Collections.<AttributePollHandler<? super Void>>emptyList()),
+            Duration.millis(20));
+        poller.start();
+    }
+
+    @Override
+    protected void setUpApp() {
+        super.setUpApp();
+        http = new HttpCallback();
+        cluster = app.createAndManageChild(EntitySpec.create(BrooklynCluster.class)
+            .location(app.newLocalhostProvisioningLocation())
+            .configure(BrooklynCluster.MEMBER_SPEC, EntitySpec.create(BrooklynNode.class)
+                .impl(MockBrooklynNode.class)
+                .configure(MockBrooklynNode.HTTP_CLIENT_CALLBACK, http)));
+    }
+    
+    @AfterMethod(alwaysRun=true)
+    public void tearDown() throws Exception {
+        poller.stop();
+        super.tearDown();
+    }
+
+    @Test
+    public void testInvalidNewMasterIdFails() {
+        try {
+            selectMaster(cluster, "1234");
+            fail("Non-existend entity ID provided.");
+        } catch (Exception e) {
+            assertTrue(e.toString().contains("1234 is not an ID of brooklyn node in this cluster"));
+        }
+    }
+
+    @Test(groups="Integration") // because slow, due to sensor feeds
+    public void testSelectMasterAfterChange() {
+        List<Entity> nodes = makeTwoNodes();
+        EntityTestUtils.assertAttributeEqualsEventually(cluster, BrooklynCluster.MASTER_NODE, (BrooklynNode)nodes.get(0));
+
+        selectMaster(cluster, nodes.get(1).getId());
+        checkMaster(cluster, nodes.get(1));
+    }
+
+    @Test
+    public void testFindMaster() {
+        List<Entity> nodes = makeTwoNodes();
+        Assert.assertEquals(((BrooklynClusterImpl)Entities.deproxy(cluster)).findMasterChild(), nodes.get(0));
+    }
+    
+    @Test(groups="Integration") // because slow, due to sensor feeds
+    public void testSelectMasterFailsAtChangeState() {
+        http.setFailAtStateChange(true);
+
+        List<Entity> nodes = makeTwoNodes();
+        
+        EntityTestUtils.assertAttributeEqualsEventually(cluster, BrooklynCluster.MASTER_NODE, (BrooklynNode)nodes.get(0));
+
+        try {
+            selectMaster(cluster, nodes.get(1).getId());
+            fail("selectMaster should have failed");
+        } catch (Exception e) {
+            // expected
+        }
+        checkMaster(cluster, nodes.get(0));
+    }
+
+    private List<Entity> makeTwoNodes() {
+        List<Entity> nodes = MutableList.copyOf(cluster.resizeByDelta(2));
+        setManagementState(nodes.get(0), ManagementNodeState.MASTER);
+        setManagementState(nodes.get(1), ManagementNodeState.HOT_STANDBY);
+        return nodes;
+    }
+
+    private void checkMaster(Group cluster, Entity node) {
+        assertEquals(node.getAttribute(BrooklynNode.MANAGEMENT_NODE_STATE), ManagementNodeState.MASTER);
+        assertEquals(cluster.getAttribute(BrooklynCluster.MASTER_NODE), node);
+        for (Entity member : cluster.getMembers()) {
+            if (member != node) {
+                assertEquals(member.getAttribute(BrooklynNode.MANAGEMENT_NODE_STATE), ManagementNodeState.HOT_STANDBY);
+            }
+            assertEquals((int)member.getAttribute(MockBrooklynNode.HA_PRIORITY), 0);
+        }
+    }
+
+    private static class HttpCallback implements Function<CallbackEntityHttpClient.Request, String> {
+        private enum State {
+            INITIAL,
+            PROMOTED
+        }
+        private State state = State.INITIAL;
+        private boolean failAtStateChange;
+
+        @Override
+        public String apply(Request input) {
+            if ("/v1/server/ha/state".equals(input.getPath())) {
+                if (failAtStateChange) {
+                    throw new RuntimeException("Testing failure at changing node state");
+                }
+
+                checkRequest(input, HttpPost.METHOD_NAME, "/v1/server/ha/state", "mode", "HOT_STANDBY");
+                Entity entity = input.getEntity();
+                EntityTestUtils.assertAttributeEquals(entity, BrooklynNode.MANAGEMENT_NODE_STATE, ManagementNodeState.MASTER);
+                EntityTestUtils.assertAttributeEquals(entity, MockBrooklynNode.HA_PRIORITY, 0);
+
+                setManagementState(entity, ManagementNodeState.HOT_STANDBY);
+
+                return "MASTER";
+            } else {
+                switch(state) {
+                case INITIAL:
+                    checkRequest(input, HttpPost.METHOD_NAME, "/v1/server/ha/priority", "priority", "1");
+                    state = State.PROMOTED;
+                    setPriority(input.getEntity(), Integer.parseInt(input.getParams().get("priority")));
+                    return "0";
+                case PROMOTED:
+                    checkRequest(input, HttpPost.METHOD_NAME, "/v1/server/ha/priority", "priority", "0");
+                    state = State.INITIAL;
+                    setPriority(input.getEntity(), Integer.parseInt(input.getParams().get("priority")));
+                    return "1";
+                default: throw new IllegalStateException("Illegal call at state " + state + ". Request = " + input.getMethod() + " " + input.getPath());
+                }
+            }
+        }
+
+        public void checkRequest(Request input, String methodName, String path, String key, String value) {
+            if (!input.getMethod().equals(methodName) || !input.getPath().equals(path)) {
+                throw new IllegalStateException("Request doesn't match expected state. Expected = " + input.getMethod() + " " + input.getPath() + ". " +
+                        "Actual = " + methodName + " " + path);
+            }
+
+            String inputValue = input.getParams().get(key);
+            if(!Objects.equal(value, inputValue)) {
+                throw new IllegalStateException("Request doesn't match expected parameter " + methodName + " " + path + ". Parameter " + key + 
+                    " expected = " + value + ", actual = " + inputValue);
+            }
+        }
+
+        public void setFailAtStateChange(boolean failAtStateChange) {
+            this.failAtStateChange = failAtStateChange;
+        }
+
+    }
+
+    private void masterFailoverIfNeeded() {
+        if (!Entities.isManaged(cluster)) return;
+        if (cluster.getAttribute(BrooklynCluster.MASTER_NODE) == null) {
+            Collection<Entity> members = cluster.getMembers();
+            if (members.size() > 0) {
+                for (Entity member : members) {
+                    if (member.getAttribute(MockBrooklynNode.HA_PRIORITY) == 1) {
+                        masterFailover(member);
+                        return;
+                    }
+                }
+                masterFailover(members.iterator().next());
+            }
+        }
+    }
+
+    private void masterFailover(Entity member) {
+        LOG.debug("Master failover to " + member);
+        setManagementState(member, ManagementNodeState.MASTER);
+        EntityTestUtils.assertAttributeEqualsEventually(cluster, BrooklynCluster.MASTER_NODE, (BrooklynNode)member);
+        return;
+    }
+
+    public static void setManagementState(Entity entity, ManagementNodeState state) {
+        ((EntityLocal)entity).setAttribute(BrooklynNode.MANAGEMENT_NODE_STATE, state);
+    }
+
+    public static void setPriority(Entity entity, int priority) {
+        ((EntityLocal)entity).setAttribute(MockBrooklynNode.HA_PRIORITY, priority);
+    }
+
+    private void selectMaster(DynamicCluster cluster, String id) {
+        app.getExecutionContext().submit(Effectors.invocation(cluster, BrooklynCluster.SELECT_MASTER, ImmutableMap.of(SelectMasterEffector.NEW_MASTER_ID.getName(), id))).asTask().getUnchecked();
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/CallbackEntityHttpClient.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/CallbackEntityHttpClient.java b/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/CallbackEntityHttpClient.java
deleted file mode 100644
index 78eef1e..0000000
--- a/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/CallbackEntityHttpClient.java
+++ /dev/null
@@ -1,95 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package brooklyn.entity.brooklynnode.effector;
-
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-
-import org.apache.http.HttpStatus;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.client.methods.HttpPost;
-
-import brooklyn.entity.Entity;
-import brooklyn.entity.brooklynnode.EntityHttpClient;
-import brooklyn.util.http.HttpTool.HttpClientBuilder;
-import brooklyn.util.http.HttpToolResponse;
-
-import com.google.common.base.Function;
-
-public class CallbackEntityHttpClient implements EntityHttpClient {
-    public static class Request {
-        private Entity entity;
-        private String method;
-        private String path;
-        private Map<String, String> params;
-        public Request(Entity entity, String method, String path, Map<String, String> params) {
-            this.entity = entity;
-            this.method = method;
-            this.path = path;
-            this.params = params;
-        }
-        public Entity getEntity() {
-            return entity;
-        }
-        public String getMethod() {
-            return method;
-        }
-        public String getPath() {
-            return path;
-        }
-        public Map<String, String> getParams() {
-            return params;
-        }
-    }
-    private Function<Request, String> callback;
-    private Entity entity;
-
-    public CallbackEntityHttpClient(Entity entity, Function<Request, String> callback) {
-        this.entity = entity;
-        this.callback = callback;
-    }
-
-    @Override
-    public HttpClientBuilder getHttpClientForBrooklynNode() {
-        throw new IllegalStateException("Method call not expected");
-    }
-
-    @Override
-    public HttpToolResponse get(String path) {
-        String result = callback.apply(new Request(entity, HttpGet.METHOD_NAME, path, Collections.<String, String>emptyMap()));
-        return new HttpToolResponse(HttpStatus.SC_OK, null, result.getBytes(), 0, 0, 0);
-    }
-
-    @Override
-    public HttpToolResponse post(String path, Map<String, String> headers, byte[] body) {
-        throw new IllegalStateException("Method call not expected");
-    }
-
-    @Override
-    public HttpToolResponse post(String path, Map<String, String> headers, Map<String, String> formParams) {
-        String result = callback.apply(new Request(entity, HttpPost.METHOD_NAME, path, formParams));
-        return new HttpToolResponse(HttpStatus.SC_OK, Collections.<String, List<String>>emptyMap(), result.getBytes(), 0, 0, 0);
-    }
-    
-    @Override
-    public HttpToolResponse delete(String path, Map<String, String> headers) {
-        throw new IllegalStateException("Method call not expected");
-    }
-}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorTest.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorTest.java b/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorTest.java
deleted file mode 100644
index 6036507..0000000
--- a/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/SelectMasterEffectorTest.java
+++ /dev/null
@@ -1,267 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package brooklyn.entity.brooklynnode.effector;
-
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertTrue;
-import static org.testng.Assert.fail;
-
-import java.util.Collection;
-import java.util.Collections;
-import java.util.concurrent.Callable;
-
-import org.apache.http.client.methods.HttpPost;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
-
-import brooklyn.entity.Entity;
-import brooklyn.entity.Group;
-import brooklyn.entity.basic.ApplicationBuilder;
-import brooklyn.entity.basic.BasicApplication;
-import brooklyn.entity.basic.EntityLocal;
-import brooklyn.entity.brooklynnode.BrooklynCluster;
-import brooklyn.entity.brooklynnode.BrooklynCluster.SelectMasterEffector;
-import brooklyn.entity.brooklynnode.BrooklynClusterImpl;
-import brooklyn.entity.brooklynnode.BrooklynNode;
-import brooklyn.entity.brooklynnode.effector.CallbackEntityHttpClient.Request;
-import brooklyn.entity.effector.Effectors;
-import brooklyn.entity.group.DynamicCluster;
-import brooklyn.entity.proxying.EntitySpec;
-import brooklyn.event.feed.AttributePollHandler;
-import brooklyn.event.feed.DelegatingPollHandler;
-import brooklyn.event.feed.Poller;
-import brooklyn.event.feed.function.FunctionFeed;
-import brooklyn.management.ManagementContext;
-import brooklyn.management.ha.ManagementNodeState;
-import brooklyn.test.EntityTestUtils;
-import brooklyn.test.entity.LocalManagementContextForTests;
-import brooklyn.util.task.BasicExecutionContext;
-import brooklyn.util.task.BasicExecutionManager;
-import brooklyn.util.time.Duration;
-
-import com.google.common.base.Function;
-import com.google.common.base.Objects;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Iterables;
-
-public class SelectMasterEffectorTest {
-    private static final Logger LOG = LoggerFactory.getLogger(BrooklynClusterImpl.class);
-
-    protected ManagementContext mgmt;
-    protected BasicApplication app;
-    protected BasicExecutionContext ec;
-    protected BrooklynCluster cluster;
-    protected FunctionFeed scanMaster;
-    protected Poller<Void> poller; 
-
-    @BeforeMethod
-    public void setUp() {
-        mgmt = new LocalManagementContextForTests();
-        EntitySpec<BasicApplication> appSpec = EntitySpec.create(BasicApplication.class)
-                .child(EntitySpec.create(BrooklynCluster.class));
-        app = ApplicationBuilder.newManagedApp(appSpec, mgmt);
-        cluster = (BrooklynCluster)Iterables.getOnlyElement(app.getChildren());
-
-        BasicExecutionManager em = new BasicExecutionManager("mycontext");
-        ec = new BasicExecutionContext(em);
-
-        poller = new Poller<Void>((EntityLocal)app, false);
-        poller.scheduleAtFixedRate(
-            new Callable<Void>() {
-                @Override
-                public Void call() throws Exception {
-                    masterFailoverIfNeeded();
-                    return null;
-                }
-            },
-            new DelegatingPollHandler<Void>(Collections.<AttributePollHandler<? super Void>>emptyList()),
-            Duration.millis(200));
-        poller.start();
-    }
-
-    @AfterMethod
-    public void tearDown() {
-        poller.stop();
-    }
-
-    @Test
-    public void testInvalidNewMasterIdFails() {
-        try {
-            BrooklynCluster cluster = app.addChild(EntitySpec.create(BrooklynCluster.class));
-            selectMaster(cluster, "1234");
-            fail("Non-existend entity ID provided.");
-        } catch (Exception e) {
-            assertTrue(e.toString().contains("1234 is not an ID of brooklyn node in this cluster"));
-        }
-    }
-
-    @Test
-    public void testSelectMaster() {
-        HttpCallback cb = new HttpCallback();
-        BrooklynNode node1 = cluster.addMemberChild(EntitySpec.create(BrooklynNode.class)
-                .impl(TestHttpEntity.class)
-                .configure(TestHttpEntity.HTTP_CLIENT_CALLBACK, cb));
-        BrooklynNode node2 = cluster.addMemberChild(EntitySpec.create(BrooklynNode.class)
-                .impl(TestHttpEntity.class)
-                .configure(TestHttpEntity.HTTP_CLIENT_CALLBACK, cb));
-
-        cluster.addMemberChild(node1);
-        cluster.addMemberChild(node2);
-
-        setManagementState(node1, ManagementNodeState.MASTER);
-        EntityTestUtils.assertAttributeEqualsEventually(cluster, BrooklynCluster.MASTER_NODE, node1);
-
-        selectMaster(cluster, node2.getId());
-        checkMaster(cluster, node2);
-    }
-
-    @Test(groups="WIP")
-    //after throwing an exception in HttpCallback tasks are no longer executed, why?
-    public void testSelectMasterFailsAtChangeState() {
-        HttpCallback cb = new HttpCallback();
-        cb.setFailAtStateChange(true);
-
-        BrooklynNode node1 = cluster.addMemberChild(EntitySpec.create(BrooklynNode.class)
-                .impl(TestHttpEntity.class)
-                .configure(TestHttpEntity.HTTP_CLIENT_CALLBACK, cb));
-        BrooklynNode node2 = cluster.addMemberChild(EntitySpec.create(BrooklynNode.class)
-                .impl(TestHttpEntity.class)
-                .configure(TestHttpEntity.HTTP_CLIENT_CALLBACK, cb));
-
-        cluster.addMemberChild(node1);
-        cluster.addMemberChild(node2);
-
-        setManagementState(node1, ManagementNodeState.MASTER);
-        EntityTestUtils.assertAttributeEqualsEventually(cluster, BrooklynCluster.MASTER_NODE, node1);
-
-        selectMaster(cluster, node2.getId());
-        checkMaster(cluster, node1);
-    }
-
-    private void checkMaster(Group cluster, Entity node) {
-        assertEquals(node.getAttribute(BrooklynNode.MANAGEMENT_NODE_STATE), ManagementNodeState.MASTER);
-        assertEquals(cluster.getAttribute(BrooklynCluster.MASTER_NODE), node);
-        for (Entity member : cluster.getMembers()) {
-            if (member != node) {
-                assertEquals(member.getAttribute(BrooklynNode.MANAGEMENT_NODE_STATE), ManagementNodeState.HOT_STANDBY);
-            }
-            assertEquals((int)member.getAttribute(TestHttpEntity.HA_PRIORITY), 0);
-        }
-    }
-
-    private static class HttpCallback implements Function<CallbackEntityHttpClient.Request, String> {
-        private enum State {
-            INITIAL,
-            PROMOTED
-        }
-        private State state = State.INITIAL;
-        private boolean failAtStateChange;
-
-        @Override
-        public String apply(Request input) {
-            if ("/v1/server/ha/state".equals(input.getPath())) {
-                if (failAtStateChange) {
-                    throw new RuntimeException("Testing failure at chaning node state");
-                }
-
-                checkRequest(input, HttpPost.METHOD_NAME, "/v1/server/ha/state", "mode", "HOT_STANDBY");
-                Entity entity = input.getEntity();
-                EntityTestUtils.assertAttributeEquals(entity, BrooklynNode.MANAGEMENT_NODE_STATE, ManagementNodeState.MASTER);
-                EntityTestUtils.assertAttributeEquals(entity, TestHttpEntity.HA_PRIORITY, 0);
-
-                setManagementState(entity, ManagementNodeState.HOT_STANDBY);
-
-                return "MASTER";
-            } else {
-                switch(state) {
-                case INITIAL:
-                    checkRequest(input, HttpPost.METHOD_NAME, "/v1/server/ha/priority", "priority", "1");
-                    state = State.PROMOTED;
-                    setPriority(input.getEntity(), Integer.parseInt(input.getParams().get("priority")));
-                    return "0";
-                case PROMOTED:
-                    checkRequest(input, HttpPost.METHOD_NAME, "/v1/server/ha/priority", "priority", "0");
-                    state = State.INITIAL;
-                    setPriority(input.getEntity(), Integer.parseInt(input.getParams().get("priority")));
-                    return "1";
-                default: throw new IllegalStateException("Illegal call at state " + state + ". Request = " + input.getMethod() + " " + input.getPath());
-                }
-            }
-        }
-
-        public void checkRequest(Request input, String methodName, String path, String... keyValue) {
-            if (!input.getMethod().equals(methodName) || !input.getPath().equals(path)) {
-                throw new IllegalStateException("Request doesn't match expected state. Expected = " + input.getMethod() + " " + input.getPath() + ". " +
-                        "Actual = " + methodName + " " + path);
-            }
-            for(int i = 0; i < keyValue.length / 2; i++) {
-                String key = keyValue[i];
-                String value = keyValue[i+1];
-                String inputValue = input.getParams().get(key);
-                if(!Objects.equal(value, inputValue)) {
-                    throw new IllegalStateException("Request doesn't match expected parameter " + methodName + " " + path + ". Parameter " + key + 
-                            " expected = " + value + ", actual = " + inputValue);
-                }
-            }
-        }
-
-        public void setFailAtStateChange(boolean failAtStateChange) {
-            this.failAtStateChange = failAtStateChange;
-        }
-
-    }
-
-    private void masterFailoverIfNeeded() {
-        if (cluster.getAttribute(BrooklynCluster.MASTER_NODE) == null) {
-            Collection<Entity> members = cluster.getMembers();
-            if (members.size() > 0) {
-                for (Entity member : members) {
-                    if (member.getAttribute(TestHttpEntity.HA_PRIORITY) == 1) {
-                        masterFailover(member);
-                        return;
-                    }
-                }
-                masterFailover(members.iterator().next());
-            }
-        }
-    }
-
-    private void masterFailover(Entity member) {
-        LOG.debug("Master failover to " + member);
-        setManagementState(member, ManagementNodeState.MASTER);
-        EntityTestUtils.assertAttributeEqualsEventually(cluster, BrooklynCluster.MASTER_NODE, (BrooklynNode)member);
-        return;
-    }
-
-    public static void setManagementState(Entity entity, ManagementNodeState state) {
-        ((EntityLocal)entity).setAttribute(BrooklynNode.MANAGEMENT_NODE_STATE, state);
-    }
-
-    public static void setPriority(Entity entity, int priority) {
-        ((EntityLocal)entity).setAttribute(TestHttpEntity.HA_PRIORITY, priority);
-    }
-
-    private void selectMaster(DynamicCluster cluster, String id) {
-        ec.submit(Effectors.invocation(cluster, BrooklynCluster.SELECT_MASTER, ImmutableMap.of(SelectMasterEffector.NEW_MASTER_ID.getName(), id))).asTask().getUnchecked();
-    }
-
-}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/TestHttpEntity.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/TestHttpEntity.java b/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/TestHttpEntity.java
deleted file mode 100644
index 335f7f7..0000000
--- a/software/base/src/test/java/brooklyn/entity/brooklynnode/effector/TestHttpEntity.java
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package brooklyn.entity.brooklynnode.effector;
-
-import java.util.Collection;
-
-import brooklyn.config.ConfigKey;
-import brooklyn.entity.basic.AbstractEntity;
-import brooklyn.entity.basic.ConfigKeys;
-import brooklyn.entity.brooklynnode.BrooklynNode;
-import brooklyn.entity.brooklynnode.EntityHttpClient;
-import brooklyn.entity.brooklynnode.effector.CallbackEntityHttpClient.Request;
-import brooklyn.event.AttributeSensor;
-import brooklyn.event.basic.BasicAttributeSensor;
-import brooklyn.location.Location;
-
-import com.google.common.base.Function;
-import com.google.common.reflect.TypeToken;
-
-public class TestHttpEntity extends AbstractEntity implements BrooklynNode {
-    @SuppressWarnings("serial")
-    public static final ConfigKey<Function<Request, String>> HTTP_CLIENT_CALLBACK = ConfigKeys.newConfigKey(new TypeToken<Function<Request, String>>(){}, "httpClientCallback");
-    public static final AttributeSensor<Integer> HA_PRIORITY = new BasicAttributeSensor<Integer>(Integer.class, "priority");
-    
-    @Override
-    public void init() {
-        super.init();
-        getMutableEntityType().addEffector(SetHighAvailabilityPriorityEffectorBody.SET_HIGH_AVAILABILITY_PRIORITY);
-        getMutableEntityType().addEffector(SetHighAvailabilityModeEffectorBody.SET_HIGH_AVAILABILITY_MODE);
-        setAttribute(HA_PRIORITY, 0);
-    }
-
-    @Override
-    public EntityHttpClient http() {
-        return new CallbackEntityHttpClient(this, getConfig(HTTP_CLIENT_CALLBACK));
-    }
-
-    @Override
-    public void start(Collection<? extends Location> locations) {
-    }
-
-    @Override
-    public void stop() {
-    }
-
-    @Override
-    public void restart() {
-    }
-
-}

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java
----------------------------------------------------------------------
diff --git a/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java b/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java
index 53ef4cf..0ba8b8c 100644
--- a/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java
+++ b/usage/rest-server/src/main/java/brooklyn/rest/resources/CatalogResource.java
@@ -185,7 +185,7 @@ public class CatalogResource extends AbstractBrooklynRestResource implements Cat
         if (Strings.isNonEmpty(regex))
             filters.add(CatalogPredicates.xml(StringPredicates.containsRegex(regex)));
         if (Strings.isNonEmpty(fragment))
-            filters.add(CatalogPredicates.xml(StringPredicates.containsLiteralCaseInsensitive(fragment)));
+            filters.add(CatalogPredicates.xml(StringPredicates.containsLiteralIgnoreCase(fragment)));
 
         return FluentIterable.from(brooklyn().getCatalog().getCatalogItems())
                 .filter(Predicates.and(filters))

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/utils/common/src/main/java/brooklyn/util/text/StringPredicates.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/text/StringPredicates.java b/utils/common/src/main/java/brooklyn/util/text/StringPredicates.java
index 1f9c574..15a306a 100644
--- a/utils/common/src/main/java/brooklyn/util/text/StringPredicates.java
+++ b/utils/common/src/main/java/brooklyn/util/text/StringPredicates.java
@@ -20,6 +20,7 @@ package brooklyn.util.text;
 
 import java.io.Serializable;
 import java.util.Arrays;
+import java.util.List;
 import java.util.Set;
 
 import javax.annotation.Nullable;
@@ -30,6 +31,7 @@ import com.google.common.base.Function;
 import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
 
 public class StringPredicates {
 
@@ -67,14 +69,14 @@ public class StringPredicates {
 
     // -----------------
     
-    public static <T extends CharSequence> Predicate<T> containsLiteralCaseInsensitive(final String fragment) {
-        return new ContainsLiteralCaseInsensitive<T>(fragment);
+    public static <T extends CharSequence> Predicate<T> containsLiteralIgnoreCase(final String fragment) {
+        return new ContainsLiteralIgnoreCase<T>(fragment);
     }
 
-    private static final class ContainsLiteralCaseInsensitive<T extends CharSequence> implements Predicate<T> {
+    private static final class ContainsLiteralIgnoreCase<T extends CharSequence> implements Predicate<T> {
         private final String fragment;
 
-        private ContainsLiteralCaseInsensitive(String fragment) {
+        private ContainsLiteralIgnoreCase(String fragment) {
             this.fragment = fragment;
         }
 
@@ -144,14 +146,11 @@ public class StringPredicates {
     // -----------------
     
     public static <T extends CharSequence> Predicate<T> containsAllLiterals(final String... fragments) {
-        return Predicates.and(Iterables.transform(Arrays.asList(fragments), new ConvertStringToContainsLiteralPredicate()));
-    }
-
-    private static final class ConvertStringToContainsLiteralPredicate implements Function<String, Predicate<CharSequence>> {
-        @Override
-        public Predicate<CharSequence> apply(String input) {
-            return containsLiteral(input);
+        List<Predicate<CharSequence>> fragmentPredicates = Lists.newArrayList();
+        for (String fragment : fragments) {
+            fragmentPredicates.add(containsLiteral(fragment));
         }
+        return Predicates.and(fragmentPredicates);
     }
 
     /** @deprecated since 0.7.0 kept only to allow conversion of anonymous inner classes */
@@ -209,7 +208,7 @@ public class StringPredicates {
     /** true if the object *is* a {@link CharSequence} starting with the given prefix */
     public static Predicate<Object> isStringStartingWith(final String prefix) {
         return Predicates.<Object>and(Predicates.instanceOf(CharSequence.class),
-            Predicates.compose(new StartsWith<String>(prefix), StringFunctions.toStringFunction()));
+            Predicates.compose(startsWith(prefix), StringFunctions.toStringFunction()));
     }
 
     /** @deprecated since 0.7.0 kept only to allow conversion of anonymous inner classes */

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/d3f6e342/utils/common/src/test/java/brooklyn/util/text/StringPredicatesTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/brooklyn/util/text/StringPredicatesTest.java b/utils/common/src/test/java/brooklyn/util/text/StringPredicatesTest.java
index 26738db..a27900c 100644
--- a/utils/common/src/test/java/brooklyn/util/text/StringPredicatesTest.java
+++ b/utils/common/src/test/java/brooklyn/util/text/StringPredicatesTest.java
@@ -39,9 +39,9 @@ public class StringPredicatesTest {
         Assert.assertFalse(StringPredicates.containsLiteral("xx").apply("text test"));
         Assert.assertFalse(StringPredicates.containsLiteral("xx").apply("texXxt tessst"));
         
-        Assert.assertTrue(StringPredicates.containsLiteralCaseInsensitive("xx").apply("texxxt tessst"));
-        Assert.assertFalse(StringPredicates.containsLiteralCaseInsensitive("xx").apply("text test"));
-        Assert.assertTrue(StringPredicates.containsLiteralCaseInsensitive("xx").apply("texXxt tessst"));
+        Assert.assertTrue(StringPredicates.containsLiteralIgnoreCase("xx").apply("texxxt tessst"));
+        Assert.assertFalse(StringPredicates.containsLiteralIgnoreCase("xx").apply("text test"));
+        Assert.assertTrue(StringPredicates.containsLiteralIgnoreCase("xx").apply("texXxt tessst"));
         
         Assert.assertTrue(StringPredicates.containsAllLiterals("xx", "ss").apply("texxxt tessst"));
         Assert.assertFalse(StringPredicates.containsAllLiterals("xx", "tt").apply("texxxt tessst"));