brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rich...@apache.org
Subject [2/5] git commit: Mark getting machine details as “inessential”
Date Tue, 08 Jul 2014 17:00:38 GMT
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 <aled.sage@gmail.com>
Authored: Thu Jul 3 19:53:14 2014 +0100
Committer: Aled Sage <aled.sage@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;
+        }
+    }
 }


Mime
View raw message