brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From aleds...@apache.org
Subject [3/9] git commit: Fix NodeJsWebAppSimpleIntegrationTest
Date Fri, 31 Oct 2014 14:10:08 GMT
Fix NodeJsWebAppSimpleIntegrationTest

- Ensure check port availability early, so get nice error message
  for it.
  TODO other entities should probably follow same pattern?!
- copyInstallResources: don’t use env for create-install-dir,
  in case sub-class is setting lots of things or that would
  cause a config value of type attributeWhenReady to be resolved
- Code tidy: Networking.checkPortsValid


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

Branch: refs/heads/master
Commit: cddf4bc24f2fe415d9d7b7cd320c0568f6f8033b
Parents: e4cb735
Author: Aled Sage <aled.sage@gmail.com>
Authored: Fri Oct 31 10:17:41 2014 +0000
Committer: Aled Sage <aled.sage@gmail.com>
Committed: Fri Oct 31 14:08:02 2014 +0000

----------------------------------------------------------------------
 .../basic/AbstractSoftwareProcessSshDriver.java    |  5 ++++-
 .../webapp/nodejs/NodeJsWebAppSshDriver.java       | 13 +++++++++++--
 .../nodejs/NodeJsWebAppSimpleIntegrationTest.java  |  5 +++--
 .../main/java/brooklyn/util/net/Networking.java    | 17 ++++++++---------
 4 files changed, 26 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cddf4bc2/software/base/src/main/java/brooklyn/entity/basic/AbstractSoftwareProcessSshDriver.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/brooklyn/entity/basic/AbstractSoftwareProcessSshDriver.java
b/software/base/src/main/java/brooklyn/entity/basic/AbstractSoftwareProcessSshDriver.java
index c8005f1..e6d02fa 100644
--- a/software/base/src/main/java/brooklyn/entity/basic/AbstractSoftwareProcessSshDriver.java
+++ b/software/base/src/main/java/brooklyn/entity/basic/AbstractSoftwareProcessSshDriver.java
@@ -313,7 +313,10 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
     public void copyInstallResources() {
         getLocation().acquireMutex("installing "+elvis(entity,this),  "installation lock
at host for files and templates");
         try {
-            execute("mkdir -p " + getInstallDir(), "create install directory");
+            // Override environment variables for this simple command. Otherwise sub-classes
might
+            // lookup port numbers and fail with ugly error if port is not set; better to
wait
+            // until in Entity's code (e.g. customize) where such checks are done explicitly.
+            execute(ImmutableMap.of("env", ImmutableMap.of()), ImmutableList.of("mkdir -p
" + getInstallDir()), "create-install-dir");
 
             Map<String, String> installFiles = entity.getConfig(SoftwareProcess.INSTALL_FILES);
             if (installFiles != null && installFiles.size() > 0) {

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cddf4bc2/software/webapp/src/main/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSshDriver.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/main/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSshDriver.java
b/software/webapp/src/main/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSshDriver.java
index f720c60..3f60ec5 100644
--- a/software/webapp/src/main/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSshDriver.java
+++ b/software/webapp/src/main/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSshDriver.java
@@ -34,12 +34,12 @@ import brooklyn.location.basic.SshMachineLocation;
 import brooklyn.util.collections.MutableList;
 import brooklyn.util.collections.MutableMap;
 import brooklyn.util.file.ArchiveUtils;
+import brooklyn.util.net.Networking;
 import brooklyn.util.os.Os;
 import brooklyn.util.ssh.BashCommands;
 import brooklyn.util.text.Strings;
 
 import com.google.common.base.Joiner;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 
@@ -73,7 +73,7 @@ public class NodeJsWebAppSshDriver extends AbstractSoftwareProcessSshDriver
impl
     }
 
     protected Map<String, Integer> getPortMap() {
-        return ImmutableMap.of("http", getHttpPort());
+        return MutableMap.of("http", getHttpPort());
     }
 
     @Override
@@ -84,6 +84,15 @@ public class NodeJsWebAppSshDriver extends AbstractSoftwareProcessSshDriver
impl
                 .build();
     }
 
+    // TODO Suggest that other entities follow this pattern as well: check for port availability
early
+    // to report failures early, and in case getShellEnvironment() tries to convert any null
port numbers
+    // to int.
+    @Override
+    public void preInstall() {
+        super.preInstall();
+        Networking.checkPortsValid(getPortMap());
+    }
+    
     @Override
     public void install() {
         LOG.info("Installing Node.JS {}", getEntity().getConfig(SoftwareProcess.SUGGESTED_VERSION));

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cddf4bc2/software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSimpleIntegrationTest.java
----------------------------------------------------------------------
diff --git a/software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSimpleIntegrationTest.java
b/software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSimpleIntegrationTest.java
index 54e6c7f..010bea5 100644
--- a/software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSimpleIntegrationTest.java
+++ b/software/webapp/src/test/java/brooklyn/entity/webapp/nodejs/NodeJsWebAppSimpleIntegrationTest.java
@@ -75,12 +75,13 @@ public class NodeJsWebAppSimpleIntegrationTest {
             app = TestApplication.Factory.newManagedInstanceForTests();
             nodejs = app.createAndManageChild(EntitySpec.create(NodeJsWebAppService.class).configure("httpPort",
httpPort));
             try {
-                nodejs.start(ImmutableList.of(app.getManagementContext().getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class))));
+                LocalhostMachineProvisioningLocation loc = app.getManagementContext().getLocationManager().createLocation(LocationSpec.create(LocalhostMachineProvisioningLocation.class));
+                nodejs.start(ImmutableList.of(loc));
                 fail("Should have thrown start-exception");
             } catch (Exception e) {
                 // LocalhostMachineProvisioningLocation does NetworkUtils.isPortAvailable,
so get -1
                 IllegalArgumentException iae = Throwables2.getFirstThrowableOfType(e, IllegalArgumentException.class);
-                if (iae == null || iae.getMessage() == null || !iae.getMessage().equals("port
for httpPort is null")) throw e;
+                if (iae == null || iae.getMessage() == null || !iae.getMessage().equals("port
for http is null")) throw e;
             } finally {
                 nodejs.stop();
             }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/cddf4bc2/utils/common/src/main/java/brooklyn/util/net/Networking.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/net/Networking.java b/utils/common/src/main/java/brooklyn/util/net/Networking.java
index 2294cce..d338080 100644
--- a/utils/common/src/main/java/brooklyn/util/net/Networking.java
+++ b/utils/common/src/main/java/brooklyn/util/net/Networking.java
@@ -162,16 +162,15 @@ public class Networking {
         return port;
     }
 
-    public static void checkPortsValid(@SuppressWarnings("rawtypes") Map ports) {
-        for (Object ppo : ports.entrySet()) {
-            Map.Entry<?,?> pp = (Map.Entry<?,?>)ppo;
-            Object val = pp.getValue();
-            if(val == null){
-                throw new IllegalArgumentException("port for "+pp.getKey()+" is null");
-            }else if (!(val instanceof Integer)) {
-                throw new IllegalArgumentException("port "+val+" for "+pp.getKey()+" is not
an integer ("+val.getClass()+")");
+    public static void checkPortsValid(Map<?, ?> ports) {
+        for (Map.Entry<?,?> entry : ports.entrySet()) {
+            Object val = entry.getValue();
+            if (val == null){
+                throw new IllegalArgumentException("port for "+entry.getKey()+" is null");
+            } else if (!(val instanceof Integer)) {
+                throw new IllegalArgumentException("port "+val+" for "+entry.getKey()+" is
not an integer ("+val.getClass()+")");
             }
-            checkPortValid((Integer)val, ""+pp.getKey());
+            checkPortValid((Integer)val, ""+entry.getKey());
         }
     }
 


Mime
View raw message