You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by ri...@apache.org on 2014/07/08 19:00:38 UTC
[2/5] git commit: Mark getting machine details as “inessential”
Mark getting machine details as “inessential”
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/45a3b5f1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/45a3b5f1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/45a3b5f1
Branch: refs/heads/master
Commit: 45a3b5f1276fc62ee0d2381358a60ec3177c0d94
Parents: d591c81
Author: Aled Sage <al...@gmail.com>
Authored: Thu Jul 3 19:53:14 2014 +0100
Committer: Aled Sage <al...@gmail.com>
Committed: Tue Jul 8 15:00:29 2014 +0100
----------------------------------------------------------------------
.../location/basic/BasicMachineDetails.java | 10 +-
.../location/basic/SshMachineLocationTest.java | 109 ++++++++++++++++++-
.../jclouds/JcloudsSshMachineLocation.java | 14 ++-
.../src/main/java/brooklyn/test/Asserts.java | 45 ++++++++
.../test/java/brooklyn/test/AssertsTest.java | 58 ++++++++++
5 files changed, 222 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/45a3b5f1/core/src/main/java/brooklyn/location/basic/BasicMachineDetails.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/BasicMachineDetails.java b/core/src/main/java/brooklyn/location/basic/BasicMachineDetails.java
index d1361fa..a680d6d 100644
--- a/core/src/main/java/brooklyn/location/basic/BasicMachineDetails.java
+++ b/core/src/main/java/brooklyn/location/basic/BasicMachineDetails.java
@@ -4,7 +4,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
import java.io.BufferedReader;
import java.io.IOException;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -21,19 +20,16 @@ import brooklyn.management.Task;
import brooklyn.util.ResourceUtils;
import brooklyn.util.stream.Streams;
import brooklyn.util.task.DynamicTasks;
+import brooklyn.util.task.TaskTags;
import brooklyn.util.task.ssh.internal.PlainSshExecTaskFactory;
import brooklyn.util.task.system.ProcessTaskWrapper;
-import brooklyn.util.text.Strings;
import com.google.common.base.CharMatcher;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.Objects;
-import com.google.common.base.Optional;
import com.google.common.base.Splitter;
import com.google.common.base.Throwables;
-import com.google.common.collect.Iterables;
-import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.io.CharStreams;
@@ -77,9 +73,9 @@ public class BasicMachineDetails implements MachineDetails {
* #taskForSshMachineLocation(SshMachineLocation)} instead.
*/
static BasicMachineDetails forSshMachineLocation(SshMachineLocation location) {
- return DynamicTasks.queueIfPossible(taskForSshMachineLocation(location))
+ return TaskTags.markInessential(DynamicTasks.queueIfPossible(taskForSshMachineLocation(location))
.orSubmitAsync()
- .asTask()
+ .asTask())
.getUnchecked();
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/45a3b5f1/core/src/test/java/brooklyn/location/basic/SshMachineLocationTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/brooklyn/location/basic/SshMachineLocationTest.java b/core/src/test/java/brooklyn/location/basic/SshMachineLocationTest.java
index f4fa966..9dd08ed 100644
--- a/core/src/test/java/brooklyn/location/basic/SshMachineLocationTest.java
+++ b/core/src/test/java/brooklyn/location/basic/SshMachineLocationTest.java
@@ -2,29 +2,60 @@ package brooklyn.location.basic;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.OutputStream;
import java.net.InetAddress;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Callable;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
+import brooklyn.entity.Effector;
+import brooklyn.entity.basic.ApplicationBuilder;
+import brooklyn.entity.basic.BrooklynConfigKeys;
+import brooklyn.entity.basic.Entities;
+import brooklyn.entity.basic.EntityInternal;
+import brooklyn.entity.basic.EntityLocal;
+import brooklyn.entity.effector.EffectorBody;
+import brooklyn.entity.effector.EffectorTaskTest;
+import brooklyn.entity.effector.Effectors;
+import brooklyn.entity.proxying.EntityInitializer;
+import brooklyn.entity.proxying.EntitySpec;
+import brooklyn.location.LocationSpec;
+import brooklyn.location.MachineDetails;
+import brooklyn.location.MachineLocation;
import brooklyn.location.PortRange;
import brooklyn.location.basic.PortRanges.LinearPortRange;
+import brooklyn.management.ManagementContext;
+import brooklyn.test.Asserts;
+import brooklyn.test.entity.LocalManagementContextForTests;
+import brooklyn.test.entity.TestApplication;
import brooklyn.util.collections.MutableMap;
+import brooklyn.util.config.ConfigBag;
import brooklyn.util.file.ArchiveUtils;
+import brooklyn.util.guava.Maybe;
+import brooklyn.util.internal.ssh.RecordingSshTool;
import brooklyn.util.internal.ssh.SshException;
+import brooklyn.util.internal.ssh.SshTool;
import brooklyn.util.net.Networking;
import brooklyn.util.net.Urls;
import brooklyn.util.os.Os;
import brooklyn.util.stream.Streams;
+import brooklyn.util.task.BasicExecutionContext;
+import brooklyn.util.task.BasicExecutionManager;
+import brooklyn.util.time.Duration;
import com.google.common.base.Charsets;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.Files;
@@ -34,6 +65,7 @@ import com.google.common.io.Files;
public class SshMachineLocationTest {
private SshMachineLocation host;
+ private ManagementContext mgmt;
@BeforeMethod(alwaysRun=true)
public void setUp() throws Exception {
@@ -43,6 +75,72 @@ public class SshMachineLocationTest {
@AfterMethod(alwaysRun=true)
public void tearDown() throws Exception {
if (host != null) Streams.closeQuietly(host);
+ if (mgmt != null) Entities.destroyAll(mgmt);
+ }
+
+ @Test(groups = "Integration")
+ public void testGetMachineDetails() throws Exception {
+ BasicExecutionManager execManager = new BasicExecutionManager("mycontextid");
+ BasicExecutionContext execContext = new BasicExecutionContext(execManager);
+ try {
+ MachineDetails details = execContext.submit(new Callable<MachineDetails>() {
+ public MachineDetails call() {
+ return host.getMachineDetails();
+ }}).get();
+ assertNotNull(details);
+ } finally {
+ execManager.shutdownNow();
+ }
+ }
+
+ // Wow, this is hard to test (until I accepted creating the entity + effector)! Code smell?
+ // Need to call getMachineDetails in a DynamicSequentialTask so that the "innessential" takes effect,
+ // to not fail its caller. But to get one of those outside of an effector is non-obvious.
+ @Test(groups = "Integration")
+ public void testGetMachineIsInessentialOnFailure() throws Exception {
+ ManagementContext mgmt = new LocalManagementContextForTests();
+
+ SshMachineLocation host2 = mgmt.getLocationManager().createLocation(LocationSpec.create(SshMachineLocation.class)
+ .configure("address", Networking.getLocalHost())
+ .configure(SshTool.PROP_TOOL_CLASS, FailingSshTool.class.getName()));
+
+ final Effector<MachineDetails> GET_MACHINE_DETAILS = Effectors.effector(MachineDetails.class, "getMachineDetails")
+ .impl(new EffectorBody<MachineDetails>() {
+ public MachineDetails call(ConfigBag parameters) {
+ Maybe<MachineLocation> machine = Machines.findUniqueMachineLocation(entity().getLocations());
+ try {
+ machine.get().getMachineDetails();
+ throw new IllegalStateException("Expected failure in ssh");
+ } catch (RuntimeException e) {
+ return null;
+ }
+ }})
+ .build();
+
+ EntitySpec<TestApplication> appSpec = EntitySpec.create(TestApplication.class)
+ .configure(BrooklynConfigKeys.SKIP_ON_BOX_BASE_DIR_RESOLUTION, true)
+ .addInitializer(new EntityInitializer() {
+ public void apply(EntityLocal entity) {
+ ((EntityInternal)entity).getMutableEntityType().addEffector(EffectorTaskTest.DOUBLE_1);
+ }});
+
+ TestApplication app = ApplicationBuilder.newManagedApp(appSpec, mgmt);
+
+ app.start(ImmutableList.of(host2));
+
+ MachineDetails details = app.invoke(GET_MACHINE_DETAILS, ImmutableMap.<String, Object>of()).get();
+ assertNull(details);
+ }
+ public static class FailingSshTool extends RecordingSshTool {
+ public FailingSshTool(Map<?, ?> props) {
+ super(props);
+ }
+ @Override public int execScript(Map<String, ?> props, List<String> commands, Map<String, ?> env) {
+ throw new RuntimeException("Simulating failure of ssh: cmds="+commands);
+ }
+ @Override public int execCommands(Map<String, ?> props, List<String> commands, Map<String, ?> env) {
+ throw new RuntimeException("Simulating failure of ssh: cmds="+commands);
+ }
}
// Note: requires `ssh localhost` to be setup such that no password is required
@@ -148,12 +246,17 @@ public class SshMachineLocationTest {
assertTrue(host.isSshable());
}
- // Note: requires `ssh localhost` to be setup such that no password is required
+ // Note: on some (home/airport) networks, `ssh 123.123.123.123` hangs seemingly forever.
+ // Make sure we fail, waiting for longer than the 70 second TCP timeout.
@Test(groups = "Integration")
public void testIsSshableWhenFalse() throws Exception {
byte[] unreachableIp = new byte[] {123,123,123,123};
- SshMachineLocation unreachableHost = new SshMachineLocation(MutableMap.of("address", InetAddress.getByAddress("unreachablename", unreachableIp)));
- assertFalse(unreachableHost.isSshable());
+ final SshMachineLocation unreachableHost = new SshMachineLocation(MutableMap.of("address", InetAddress.getByAddress("unreachablename", unreachableIp)));
+ Asserts.assertReturnsEventually(new Runnable() {
+ public void run() {
+ assertFalse(unreachableHost.isSshable());
+ }},
+ Duration.TWO_MINUTES);
}
@Test
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/45a3b5f1/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsSshMachineLocation.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
index ad05973..1fd8e89 100644
--- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
+++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsSshMachineLocation.java
@@ -37,6 +37,7 @@ import brooklyn.location.basic.BasicMachineDetails;
import brooklyn.location.basic.BasicOsDetails;
import brooklyn.location.basic.HasSubnetHostname;
import brooklyn.location.basic.SshMachineLocation;
+import brooklyn.util.exceptions.Exceptions;
import brooklyn.util.flags.SetFromFlag;
import brooklyn.util.net.Networking;
import brooklyn.util.text.Strings;
@@ -288,10 +289,15 @@ public class JcloudsSshMachineLocation extends SshMachineLocation implements Has
putIfNotNull(builder, "ram", "" + (hardware != null ? hardware.getRam() : null));
putIfNotNull(builder, "cpus", "" + (processors != null ? processors.size() : null));
- OsDetails osDetails = getOsDetails();
- putIfNotNull(builder, "osName", osDetails.getName());
- putIfNotNull(builder, "osArch", osDetails.getArch());
- putIfNotNull(builder, "64bit", osDetails.is64bit() ? "true" : "false");
+ try {
+ OsDetails osDetails = getOsDetails();
+ putIfNotNull(builder, "osName", osDetails.getName());
+ putIfNotNull(builder, "osArch", osDetails.getArch());
+ putIfNotNull(builder, "64bit", osDetails.is64bit() ? "true" : "false");
+ } catch (Exception e) {
+ Exceptions.propagateIfFatal(e);
+ LOG.warn("Unable to get OS Details for "+node+"; continuing", e);
+ }
return builder.build();
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/45a3b5f1/utils/common/src/main/java/brooklyn/test/Asserts.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/test/Asserts.java b/utils/common/src/main/java/brooklyn/test/Asserts.java
index be98183..30bbe06 100644
--- a/utils/common/src/main/java/brooklyn/test/Asserts.java
+++ b/utils/common/src/main/java/brooklyn/test/Asserts.java
@@ -2,6 +2,7 @@ package brooklyn.test;
import groovy.lang.Closure;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Enumeration;
import java.util.Iterator;
@@ -9,12 +10,16 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executors;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import brooklyn.util.collections.MutableSet;
+import brooklyn.util.exceptions.Exceptions;
import brooklyn.util.time.Duration;
import com.google.common.annotations.Beta;
@@ -60,6 +65,16 @@ public class Asserts {
}
/**
+ * Asserts that a condition is false. If it isn't,
+ * an AssertionError, with the given message, is thrown.
+ * @param condition the condition to evaluate
+ * @param message the assertion error message
+ */
+ public static void assertFalse(boolean condition, String message) {
+ if (condition) fail(message);
+ }
+
+ /**
* Fails a test with the given message.
* @param message the assertion error message
*/
@@ -358,6 +373,36 @@ public class Asserts {
if (!failed) fail("Test code should have thrown exception but did not");
}
+ public static void assertReturnsEventually(final Runnable r, Duration timeout) throws InterruptedException, ExecutionException, TimeoutException {
+ final AtomicReference<Throwable> throwable = new AtomicReference<Throwable>();
+ Runnable wrappedR = new Runnable() {
+ @Override public void run() {
+ try {
+ r.run();
+ } catch (Throwable t) {
+ throwable.set(t);
+ throw Exceptions.propagate(t);
+ }
+ }
+ };
+ Thread thread = new Thread(wrappedR, "assertReturnsEventually("+r+")");
+ try {
+ thread.start();
+ thread.join(timeout.toMilliseconds());
+ if (thread.isAlive()) {
+ throw new TimeoutException("Still running: r="+r+"; thread="+Arrays.toString(thread.getStackTrace()));
+ }
+ } catch (InterruptedException e) {
+ throw Exceptions.propagate(e);
+ } finally {
+ thread.interrupt();
+ }
+
+ if (throwable.get() != null) {
+ throw new ExecutionException(throwable.get());
+ }
+ }
+
@SuppressWarnings("rawtypes")
private static boolean groovyTruth(Object o) {
// TODO Doesn't handle matchers (see http://docs.codehaus.org/display/GROOVY/Groovy+Truth)
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/45a3b5f1/utils/common/src/test/java/brooklyn/test/AssertsTest.java
----------------------------------------------------------------------
diff --git a/utils/common/src/test/java/brooklyn/test/AssertsTest.java b/utils/common/src/test/java/brooklyn/test/AssertsTest.java
index 38bf2ba..b43d1b7 100644
--- a/utils/common/src/test/java/brooklyn/test/AssertsTest.java
+++ b/utils/common/src/test/java/brooklyn/test/AssertsTest.java
@@ -1,14 +1,26 @@
package brooklyn.test;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.testng.Assert;
import org.testng.annotations.Test;
import brooklyn.util.collections.MutableMap;
+import brooklyn.util.exceptions.Exceptions;
import brooklyn.util.time.Duration;
import com.google.common.util.concurrent.Callables;
public class AssertsTest {
+ private static final Runnable NOOP_RUNNABLE = new Runnable() {
+ @Override public void run() {
+ }
+ };
+
// TODO this is confusing -- i'd expect it to fail since it always returns false;
// see notes at start of Asserts and in succeedsEventually method
@Test
@@ -16,4 +28,50 @@ public class AssertsTest {
Asserts.succeedsEventually(MutableMap.of("timeout", Duration.millis(50)), Callables.returning(false));
}
+ @Test
+ public void testAssertReturnsEventually() throws Exception {
+ Asserts.assertReturnsEventually(NOOP_RUNNABLE, Duration.THIRTY_SECONDS);
+ }
+
+ @Test
+ public void testAssertReturnsEventuallyTimesOut() throws Exception {
+ final AtomicBoolean interrupted = new AtomicBoolean();
+
+ try {
+ Asserts.assertReturnsEventually(new Runnable() {
+ public void run() {
+ try {
+ Thread.sleep(60*1000);
+ } catch (InterruptedException e) {
+ interrupted.set(true);
+ Thread.currentThread().interrupt();
+ return;
+ }
+ }},
+ Duration.of(10, TimeUnit.MILLISECONDS));
+ Assert.fail("Should have thrown AssertionError on timeout");
+ } catch (TimeoutException e) {
+ // success
+ }
+
+ Asserts.succeedsEventually(new Runnable() {
+ @Override public void run() {
+ Assert.assertTrue(interrupted.get());
+ }});
+ }
+
+ @Test
+ public void testAssertReturnsEventuallyPropagatesException() throws Exception {
+ try {
+ Asserts.assertReturnsEventually(new Runnable() {
+ public void run() {
+ throw new IllegalStateException("Simulating failure");
+ }},
+ Duration.THIRTY_SECONDS);
+ Assert.fail("Should have thrown AssertionError on timeout");
+ } catch (ExecutionException e) {
+ IllegalStateException ise = Exceptions.getFirstThrowableOfType(e, IllegalStateException.class);
+ if (ise == null || !ise.toString().contains("Simulating failure")) throw e;
+ }
+ }
}