Changes based on review comments for machine metrics

- Move metrics enricher creation to init method of MachineEntity
- Enqueue parallel task to stop process and feeds
- Add helper method to create Task from Callable
- Updated tests to check children of parallel stopping task


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

Branch: refs/heads/master
Commit: 2d99fd4add5fd3f8eb1dedecfe5d314193203ec2
Parents: aff1144
Author: Andrew Donald Kennedy <[email protected]>
Authored: Mon Jun 20 07:20:33 2016 -0700
Committer: Andrew Donald Kennedy <[email protected]>
Committed: Wed Jun 22 13:34:10 2016 -0700

----------------------------------------------------------------------
 .../brooklyn/core/entity/AbstractEntity.java    | 33 +++++++++--------
 .../brooklyn/core/entity/EntityInternal.java    |  6 +--
 .../brooklyn/util/core/task/DynamicTasks.java   |  4 +-
 .../apache/brooklyn/util/core/task/Tasks.java   | 10 ++++-
 .../entity/machine/MachineEntityImpl.java       | 23 +++++++-----
 .../software/base/SoftwareProcessImpl.java      |  1 -
 .../MachineLifecycleEffectorTasks.java          |  7 ++--
 .../base/SoftwareProcessEntityTest.java         | 39 +++++++++++---------
 8 files changed, 70 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2d99fd4a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java 
b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java
index 9c6cfcf..42919a6 100644
--- a/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java
+++ b/core/src/main/java/org/apache/brooklyn/core/entity/AbstractEntity.java
@@ -27,6 +27,22 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.commons.lang3.builder.EqualsBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.google.common.annotations.Beta;
+import com.google.common.base.Function;
+import com.google.common.base.Objects;
+import com.google.common.base.Objects.ToStringHelper;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+
 import org.apache.brooklyn.api.effector.Effector;
 import org.apache.brooklyn.api.entity.Application;
 import org.apache.brooklyn.api.entity.Entity;
@@ -100,21 +116,6 @@ import org.apache.brooklyn.util.core.task.DeferredSupplier;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.javalang.Equals;
 import org.apache.brooklyn.util.text.Strings;
-import org.apache.commons.lang3.builder.EqualsBuilder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import com.google.common.annotations.Beta;
-import com.google.common.base.Function;
-import com.google.common.base.Objects;
-import com.google.common.base.Objects.ToStringHelper;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 
 /**
  * Default {@link Entity} implementation, which should be extended whenever 
implementing an entity.
@@ -144,7 +145,7 @@ import com.google.common.collect.Sets;
  * The legacy (pre 0.5) mechanism for creating entities is for others to call 
the constructor directly.
  * This is now deprecated.
  */
-public abstract class AbstractEntity extends AbstractBrooklynObject implements 
EntityLocal, EntityInternal {
+public abstract class AbstractEntity extends AbstractBrooklynObject implements 
EntityInternal {
     
     private static final Logger LOG = 
LoggerFactory.getLogger(AbstractEntity.class);
     

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2d99fd4a/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java 
b/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java
index 933390f..1f627ac 100644
--- a/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java
+++ b/core/src/main/java/org/apache/brooklyn/core/entity/EntityInternal.java
@@ -23,6 +23,8 @@ import java.util.Map;
 
 import javax.annotation.Nullable;
 
+import com.google.common.annotations.Beta;
+
 import org.apache.brooklyn.api.effector.Effector;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntityLocal;
@@ -34,18 +36,14 @@ import org.apache.brooklyn.api.mgmt.SubscriptionContext;
 import org.apache.brooklyn.api.mgmt.rebind.RebindSupport;
 import org.apache.brooklyn.api.mgmt.rebind.Rebindable;
 import org.apache.brooklyn.api.mgmt.rebind.mementos.EntityMemento;
-import org.apache.brooklyn.api.objs.BrooklynObject;
 import org.apache.brooklyn.api.sensor.AttributeSensor;
 import org.apache.brooklyn.api.sensor.Feed;
 import org.apache.brooklyn.config.ConfigKey;
 import org.apache.brooklyn.core.entity.internal.EntityConfigMap;
 import org.apache.brooklyn.core.mgmt.internal.EntityManagementSupport;
 import org.apache.brooklyn.core.objs.BrooklynObjectInternal;
-import 
org.apache.brooklyn.core.objs.BrooklynObjectInternal.SubscriptionSupportInternal;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 
-import com.google.common.annotations.Beta;
-
 /** 
  * Extended Entity interface with additional functionality that is 
purely-internal (i.e. intended 
  * for the brooklyn framework only).

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2d99fd4a/core/src/main/java/org/apache/brooklyn/util/core/task/DynamicTasks.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/brooklyn/util/core/task/DynamicTasks.java 
b/core/src/main/java/org/apache/brooklyn/util/core/task/DynamicTasks.java
index de384fc..c966cf0 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/DynamicTasks.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/DynamicTasks.java
@@ -265,12 +265,12 @@ public class DynamicTasks {
 
     /** @see #queue(org.apache.brooklyn.api.mgmt.TaskAdaptable)  */
     public static <T> Task<T> queue(String name, Callable<T> job) {
-        return 
DynamicTasks.queue(Tasks.<T>builder().displayName(name).body(job).build());
+        return DynamicTasks.queue(Tasks.create(name, job));
     }
 
     /** @see #queue(org.apache.brooklyn.api.mgmt.TaskAdaptable)  */
     public static <T> Task<T> queue(String name, Runnable job) {
-        return 
DynamicTasks.queue(Tasks.<T>builder().displayName(name).body(job).build());
+        return DynamicTasks.queue(Tasks.<T>create(name, job));
     }
 
     /** queues the task if needed, i.e. if it is not yet submitted (so it will 
run), 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2d99fd4a/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java 
b/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java
index 3dae7ab..661f1c2 100644
--- a/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java
+++ b/core/src/main/java/org/apache/brooklyn/util/core/task/Tasks.java
@@ -39,6 +39,7 @@ import org.apache.brooklyn.util.repeat.Repeater;
 import org.apache.brooklyn.util.time.CountdownTimer;
 import org.apache.brooklyn.util.time.Duration;
 import org.apache.brooklyn.util.time.Time;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -78,7 +79,14 @@ public class Tasks {
         if (current instanceof TaskInternal)
             ((TaskInternal<?>)current).resetBlockingTask(); 
     }
-    
+
+    public static <T> Task<T> create(String name, Callable<T> job) {
+        return Tasks.<T>builder().displayName(name).body(job).build();
+    }
+    public static <T> Task<T> create(String name, Runnable job) {
+        return Tasks.<T>builder().displayName(name).body(job).build();
+    }
+
     /** convenience for setting "blocking details" on any task where the 
current thread is running,
      * while the passed code is executed; often used from groovy as
      * <pre>{@code withBlockingDetails("sleeping 5s") { Thread.sleep(5000); } 
}</pre>

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2d99fd4a/software/base/src/main/java/org/apache/brooklyn/entity/machine/MachineEntityImpl.java
----------------------------------------------------------------------
diff --git 
a/software/base/src/main/java/org/apache/brooklyn/entity/machine/MachineEntityImpl.java
 
b/software/base/src/main/java/org/apache/brooklyn/entity/machine/MachineEntityImpl.java
index dda1c71..5fdb28f 100644
--- 
a/software/base/src/main/java/org/apache/brooklyn/entity/machine/MachineEntityImpl.java
+++ 
b/software/base/src/main/java/org/apache/brooklyn/entity/machine/MachineEntityImpl.java
@@ -23,12 +23,12 @@ import java.util.concurrent.TimeoutException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.brooklyn.api.sensor.Feed;
 import org.apache.brooklyn.core.effector.ssh.SshEffectorTasks;
 import org.apache.brooklyn.core.location.Machines;
 import 
org.apache.brooklyn.entity.software.base.AbstractSoftwareProcessSshDriver;
 import org.apache.brooklyn.entity.software.base.EmptySoftwareProcessDriver;
 import org.apache.brooklyn.entity.software.base.EmptySoftwareProcessImpl;
-import org.apache.brooklyn.feed.ssh.SshFeed;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.core.task.system.ProcessTaskWrapper;
@@ -40,28 +40,33 @@ public class MachineEntityImpl extends 
EmptySoftwareProcessImpl implements Machi
 
     private static final Logger LOG = 
LoggerFactory.getLogger(MachineEntityImpl.class);
 
-    private transient SshFeed machineMetricsFeed;
+    private transient Feed machineMetrics;
 
     @Override
-    public void init() {
-        LOG.info("Starting server pool machine with id {}", getId());
-        super.init();
+    protected void initEnrichers() {
+        LOG.info("Adding machine metrics enrichers");
+        AddMachineMetrics.addMachineMetricsEnrichers(this);
+
+        super.initEnrichers();
     }
 
     @Override
     protected void connectSensors() {
         super.connectSensors();
+
         Maybe<SshMachineLocation> location = 
Machines.findUniqueMachineLocation(getLocations(), SshMachineLocation.class);
         if (location.isPresent() && location.get().getOsDetails().isLinux()) {
-            machineMetricsFeed = 
AddMachineMetrics.createMachineMetricsFeed(this);
-            AddMachineMetrics.addMachineMetricsEnrichers(this);
+            LOG.info("Adding machine metrics feed");
+            machineMetrics = AddMachineMetrics.createMachineMetricsFeed(this);
         } else {
             LOG.warn("Not adding machine metrics feed as no suitable location 
available on entity");
         }
     }
 
-    public void disconnectSensors() {
-        if (machineMetricsFeed != null) machineMetricsFeed.stop();
+    @Override
+    protected void disconnectSensors() {
+        if (machineMetrics != null) machineMetrics.stop();
+
         super.disconnectSensors();
     }
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2d99fd4a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessImpl.java
----------------------------------------------------------------------
diff --git 
a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessImpl.java
 
b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessImpl.java
index 7397e0d..bb5f17a 100644
--- 
a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessImpl.java
+++ 
b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/SoftwareProcessImpl.java
@@ -58,7 +58,6 @@ import 
org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic;
 import 
org.apache.brooklyn.core.entity.lifecycle.ServiceStateLogic.ServiceNotUpLogic;
 import org.apache.brooklyn.core.location.LocationConfigKeys;
 import org.apache.brooklyn.core.location.cloud.CloudLocationConfig;
-import org.apache.brooklyn.entity.machine.MachineAttributes;
 import org.apache.brooklyn.feed.function.FunctionFeed;
 import org.apache.brooklyn.feed.function.FunctionPollConfig;
 import org.apache.brooklyn.location.ssh.SshMachineLocation;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2d99fd4a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
----------------------------------------------------------------------
diff --git 
a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
 
b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
index 8b1de62..dfee728 100644
--- 
a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
+++ 
b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/lifecycle/MachineLifecycleEffectorTasks.java
@@ -712,9 +712,10 @@ public abstract class MachineLifecycleEffectorTasks {
         Maybe<MachineLocation> machine = 
Machines.findUniqueMachineLocation(entity().getLocations());
         Task<List<?>> stoppingProcess = null;
         if (canStop(stopProcessMode, entity())) {
-            stoppingProcess = Tasks.parallel(
-                        DynamicTasks.queue("stopping (process)", new 
StopProcessesAtMachineTask()),
-                        DynamicTasks.queue("stopping (feeds)", new 
StopFeedsAtMachineTask()));
+            stoppingProcess = Tasks.parallel("stopping",
+                    Tasks.create("stopping (process)", new 
StopProcessesAtMachineTask()),
+                    Tasks.create("stopping (feeds)", new 
StopFeedsAtMachineTask()));
+            DynamicTasks.queue(stoppingProcess);
         }
 
         Task<StopMachineDetails<Integer>> stoppingMachine = null;

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2d99fd4a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessEntityTest.java
----------------------------------------------------------------------
diff --git 
a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessEntityTest.java
 
b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessEntityTest.java
index 8e56b18..18f7ff2 100644
--- 
a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessEntityTest.java
+++ 
b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/SoftwareProcessEntityTest.java
@@ -30,6 +30,21 @@ import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testng.Assert;
+import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+
+import org.jclouds.util.Throwables2;
+
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.entity.EntityLocal;
 import org.apache.brooklyn.api.entity.EntitySpec;
@@ -59,14 +74,17 @@ import 
org.apache.brooklyn.core.sensor.PortAttributeSensorAndConfigKey;
 import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.TestApplication;
 import 
org.apache.brooklyn.entity.software.base.SoftwareProcess.RestartSoftwareParameters;
-import 
org.apache.brooklyn.entity.software.base.SoftwareProcess.StopSoftwareParameters;
 import 
org.apache.brooklyn.entity.software.base.SoftwareProcess.RestartSoftwareParameters.RestartMachineMode;
+import 
org.apache.brooklyn.entity.software.base.SoftwareProcess.StopSoftwareParameters;
 import 
org.apache.brooklyn.entity.software.base.SoftwareProcess.StopSoftwareParameters.StopMode;
 import 
org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasksTest;
+import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation;
+import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.test.Asserts;
 import org.apache.brooklyn.util.collections.MutableMap;
 import org.apache.brooklyn.util.core.config.ConfigBag;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
+import org.apache.brooklyn.util.core.task.TaskPredicates;
 import org.apache.brooklyn.util.core.task.Tasks;
 import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.exceptions.PropagatedRuntimeException;
@@ -74,21 +92,6 @@ import org.apache.brooklyn.util.net.UserAndHostAndPort;
 import org.apache.brooklyn.util.os.Os;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
-import org.jclouds.util.Throwables2;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.testng.Assert;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.DataProvider;
-import org.testng.annotations.Test;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
-
-import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation;
-import org.apache.brooklyn.location.ssh.SshMachineLocation;
 
 
 public class SoftwareProcessEntityTest extends BrooklynAppUnitTestSupport {
@@ -429,7 +432,9 @@ public class SoftwareProcessEntityTest extends 
BrooklynAppUnitTestSupport {
         Iterator<Task<?>> failures;
         failures = Tasks.failed(Tasks.descendants(t, true)).iterator();
         Assert.assertTrue(failures.hasNext(), "Expected error in descendants");
-        failures = Tasks.failed(Tasks.children(t)).iterator();
+        Optional<Task<?>> stopping = Iterables.tryFind(Tasks.children(t), 
TaskPredicates.displayNameEqualTo("stopping"));
+        Assert.assertTrue(stopping.isPresent(), "Could not find stopping 
task");
+        failures = Tasks.failed(Tasks.children(stopping.get())).iterator();
         Assert.assertTrue(failures.hasNext(), "Expected error in child");
         Throwable e = Tasks.getError(failures.next());
         if (e == null || !e.toString().contains("Simulating stop error")) 

Reply via email to