brooklyn-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From s...@apache.org
Subject [1/2] brooklyn-server git commit: Ensure detection of failed pre.install.command and related commands.
Date Fri, 17 Jun 2016 11:04:28 GMT
Repository: brooklyn-server
Updated Branches:
  refs/heads/master b6a65650e -> ee3ca1e71


Ensure detection of failed pre.install.command and related commands.

Currently 'pre.install.command' and related steps do not fail if the command(s) returns a
non-zero exit.
This can mean your install will fail but Brooklyn won't detect it (perhaps until some subsequent
stage, or not at all).

This change adds detection of the return code and failure if it is non-zero. The 'contract'
for the methods is added as a comment.  The AbstractSoftwareProcessSshDriver is changed to
detect the failure and throw an exception. The AbstractSoftwareProcessWinRmDriver already
does the right thing and doesn't need changed.

While this could break some existing blueprints, I think in such cases it's more likely that
it is highlighting a problem that has been missed, rather than causing a problem because someone
is explicitly relying on that behaviour.


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

Branch: refs/heads/master
Commit: d76315ff46c79bd9353961451d133790e4c65cf3
Parents: 36be4a3
Author: Geoff Macartney <geoff.macartney@cloudsoftcorp.com>
Authored: Thu Jun 16 09:57:28 2016 +0100
Committer: Geoff Macartney <geoff.macartney@cloudsoftcorp.com>
Committed: Fri Jun 17 12:03:08 2016 +0100

----------------------------------------------------------------------
 .../base/AbstractSoftwareProcessDriver.java     | 41 +++++++++++++++-
 .../base/AbstractSoftwareProcessSshDriver.java  | 50 ++++++++++++--------
 .../base/SoftwareProcessEntityTest.java         | 44 +++++++++++++++++
 3 files changed, 115 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d76315ff/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
index 72a80ec..f6449b0 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessDriver.java
@@ -219,16 +219,55 @@ public abstract class AbstractSoftwareProcessDriver implements SoftwareProcessDr
      */
     public void preInstall() {}
 
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void runPreInstallCommand();
+
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void setup();
+
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void install();
+
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void runPostInstallCommand();
+
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void runPreCustomizeCommand();
+
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void customize();
+
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void runPostCustomizeCommand();
+
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void runPreLaunchCommand();
+
+    /**
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void launch();
-    /** Only run if launch is run (if start is not skipped). */
+
+    /**
+     * Only run if launch is run (if start is not skipped).
+     * Implementations should fail if the return code is non-zero, by throwing some appropriate
exception.
+     */
     public abstract void runPostLaunchCommand();
 
     @Override

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d76315ff/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
----------------------------------------------------------------------
diff --git a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
index 553c2b9..1702c34 100644
--- a/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
+++ b/software/base/src/main/java/org/apache/brooklyn/entity/software/base/AbstractSoftwareProcessSshDriver.java
@@ -27,6 +27,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.brooklyn.config.ConfigKey;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -226,19 +227,31 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
         return SshEffectorTasks.getSshFlags(getEntity(), getMachine());
     }
 
+    /**
+     * @deprecated since 0.10.0 This method will become private in a future release.
+     */
+    @Deprecated
     public int execute(String command, String summaryForLogging) {
         return execute(ImmutableList.of(command), summaryForLogging);
     }
 
+    /**
+     * @deprecated since 0.10.0 This method will become private in a future release.
+     */
+    @Deprecated
     public int execute(List<String> script, String summaryForLogging) {
         return execute(Maps.newLinkedHashMap(), script, summaryForLogging);
     }
 
+    /**
+     * @deprecated since 0.10.0 This method will become private in a future release.
+     */
     @SuppressWarnings({ "rawtypes", "unchecked" })
     @Override
+    @Deprecated
     public int execute(Map flags2, List<String> script, String summaryForLogging) {
         // TODO replace with SshEffectorTasks.ssh ?; remove the use of flags
-
+        // TODO log the stdin/stdout/stderr upon error
         Map flags = Maps.newLinkedHashMap();
         if (!flags2.containsKey(IGNORE_ENTITY_SSH_FLAGS)) {
             flags.putAll(getSshFlags());
@@ -285,46 +298,45 @@ public abstract class AbstractSoftwareProcessSshDriver extends AbstractSoftwareP
         }
     }
 
+    private void executeSuccessfully(ConfigKey<String> configKey, String label) {
+        if(Strings.isNonBlank(getEntity().getConfig(configKey))) {
+            log.debug("Executing {} on entity {}", label, entity.getDisplayName());
+            int result = execute(ImmutableList.of(getEntity().getConfig(configKey)), label);
+            if (0 != result) {
+                log.debug("Executing {} failed with return code {}", label, result);
+                throw new IllegalStateException("commands for " + configKey.getName() + "
failed with return code " + result);
+            }
+        }
+    }
+
     @Override
     public void runPreInstallCommand() {
-        if(Strings.isNonBlank(getEntity().getConfig(BrooklynConfigKeys.PRE_INSTALL_COMMAND)))
{
-            execute(ImmutableList.of(getEntity().getConfig(BrooklynConfigKeys.PRE_INSTALL_COMMAND)),
"running pre-install commands");
-        }
+        executeSuccessfully(BrooklynConfigKeys.PRE_INSTALL_COMMAND, "running pre-install
commands");
     }
 
     @Override
     public void runPostInstallCommand() {
-        if (Strings.isNonBlank(entity.getConfig(BrooklynConfigKeys.POST_INSTALL_COMMAND)))
{
-            execute(ImmutableList.of(entity.getConfig(BrooklynConfigKeys.POST_INSTALL_COMMAND)),
"running post-install commands");
-        }
+        executeSuccessfully(BrooklynConfigKeys.POST_INSTALL_COMMAND, "running post-install
commands");
     }
 
     @Override
     public void runPreCustomizeCommand() {
-        if(Strings.isNonBlank(getEntity().getConfig(BrooklynConfigKeys.PRE_CUSTOMIZE_COMMAND)))
{
-            execute(ImmutableList.of(getEntity().getConfig(BrooklynConfigKeys.PRE_CUSTOMIZE_COMMAND)),
"running pre-customize commands");
-        }
+        executeSuccessfully(BrooklynConfigKeys.PRE_CUSTOMIZE_COMMAND, "running pre-customize
commands");
     }
 
     @Override
     public void runPostCustomizeCommand() {
-        if (Strings.isNonBlank(entity.getConfig(BrooklynConfigKeys.POST_CUSTOMIZE_COMMAND)))
{
-            execute(ImmutableList.of(entity.getConfig(BrooklynConfigKeys.POST_CUSTOMIZE_COMMAND)),
"running post-customize commands");
-        }
+        executeSuccessfully(BrooklynConfigKeys.POST_CUSTOMIZE_COMMAND, "running post-customize
commands");
     }
 
     @Override
     public void runPreLaunchCommand() {
-        if (Strings.isNonBlank(entity.getConfig(BrooklynConfigKeys.PRE_LAUNCH_COMMAND)))
{
-            execute(ImmutableList.of(entity.getConfig(BrooklynConfigKeys.PRE_LAUNCH_COMMAND)),
"running pre-launch commands");
-        }
+        executeSuccessfully(BrooklynConfigKeys.PRE_LAUNCH_COMMAND, "running pre-launch commands");
     }
 
     @Override
     public void runPostLaunchCommand() {
-        if (Strings.isNonBlank(entity.getConfig(BrooklynConfigKeys.POST_LAUNCH_COMMAND)))
{
-            execute(ImmutableList.of(entity.getConfig(BrooklynConfigKeys.POST_LAUNCH_COMMAND)),
"running post-launch commands");
-        }
+        executeSuccessfully(BrooklynConfigKeys.POST_LAUNCH_COMMAND, "running post-launch
commands");
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/d76315ff/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 6fd330a..8e56b18 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
@@ -79,6 +79,7 @@ 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;
@@ -583,6 +584,39 @@ public class SoftwareProcessEntityTest extends BrooklynAppUnitTestSupport
{
         Assert.assertTrue(entity.getRequiredOpenPorts().contains(9999));
     }
 
+    @DataProvider
+    public Object[][] runCommandsProvider() {
+        return new Object[][] {
+            {BrooklynConfigKeys.PRE_INSTALL_COMMAND},
+            {BrooklynConfigKeys.POST_INSTALL_COMMAND},
+            {BrooklynConfigKeys.PRE_CUSTOMIZE_COMMAND},
+            {BrooklynConfigKeys.POST_CUSTOMIZE_COMMAND},
+            {BrooklynConfigKeys.PRE_LAUNCH_COMMAND},
+            {BrooklynConfigKeys.POST_LAUNCH_COMMAND}
+        };
+    }
+
+    @Test(dataProvider = "runCommandsProvider")
+    public void testCommandFailureCausesEntityFailure(ConfigKey<String> configKey)
{
+        String failingCommand = "unsuccessfulCommand";
+        final ConfigurableFailingCommandMachineLocation location = mgmt.getLocationManager().createLocation(
+            LocationSpec.create(ConfigurableFailingCommandMachineLocation.class)
+                .configure("address", "localhost")
+                .configure(ConfigurableFailingCommandMachineLocation.FAILURE_COMMAND, failingCommand)
+        );
+        VanillaSoftwareProcess entity = app.createAndManageChild(EntitySpec.create(VanillaSoftwareProcess.class)
+            .configure(configKey, failingCommand)
+        );
+
+        try {
+            app.start(ImmutableList.<Location>of(location));
+            Asserts.shouldHaveFailedPreviously();
+        } catch (Exception e) {
+            Asserts.expectedFailureContains(e, configKey.getName());
+        }
+        EntityAsserts.assertAttributeEqualsEventually(entity, Attributes.SERVICE_STATE_ACTUAL,
Lifecycle.ON_FIRE);
+    }
+
     @ImplementedBy(MyServiceImpl.class)
     public interface MyService extends SoftwareProcess {
         PortAttributeSensorAndConfigKey HTTP_PORT = Attributes.HTTP_PORT;
@@ -805,4 +839,14 @@ public class SoftwareProcessEntityTest extends BrooklynAppUnitTestSupport
{
 
     }
 
+    public static class ConfigurableFailingCommandMachineLocation extends SshMachineLocation
{
+
+        public static ConfigKey<String> FAILURE_COMMAND =
+            ConfigKeys.newConfigKey("failingCommand", "A command that will return non zero
result", "fail");
+
+        @Override
+        public int execScript(Map<String,?> props, String summaryForLogging, List<String>
commands, Map<String,?> env) {
+            return commands.contains(getConfig(FAILURE_COMMAND)) ? 1 : 0;
+        }
+    }
 }


Mime
View raw message