You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2014/11/26 00:32:13 UTC
[GitHub] incubator-brooklyn pull request: Feature/async ssh
GitHub user aledsage opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/358
Feature/async ssh
Supports executing a script asynchronously over ssh. Script redirects stdout and stderr to files, along with pid and exit status. Then caller executes additional ssh commands to poll for the script's completion.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/incubator-brooklyn feature/async-ssh
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/358.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #358
----
commit e8217628e13d5dc8ff01fadf373bb8fd117dc45a
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-25T21:42:33Z
Adds SshjTool exec script async
commit 109af263c5c7e7050c148af3a220f86518dcb3ab
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-25T21:44:06Z
ShellToolAbstractTset.execCommands: remove duplication
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r22168119
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +268,161 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
--- End diff --
Empty TODO
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#issuecomment-64661733
@aledsage You can force push the branch (with no changes) to restart the build, without the close/open messages being required.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/358
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097265
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
--- End diff --
Could have used an `Optional.fromNullable(summary).or(scriptPath)` perhaps. Not sure how much more readable that is?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21098000
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/sshj/SshjTool.java ---
@@ -299,16 +302,134 @@ public int copyFromServer(Map<String,?> props, String pathAndFileOnRemoteServer,
*
* So on balance, the script-based approach seems most reliable, even if there is an overhead
* of separate message(s) for copying the file!
+ *
+ * Another consideration is long-running scripts. On some clouds when executing a script that takes
+ * several minutes, we have seen it fail with -1 (e.g. 1 in 20 times). This suggests the ssh connection
+ * is being dropped. To avoid this problem, we can execute the script asynchronously, writing to files
+ * the stdout/stderr/pid/exitStatus. We then periodically poll to retrieve the contents of these files.
+ * Use {@link #PROP_EXEC_ASYNC} to force this mode of execution.
*/
@Override
public int execScript(final Map<String,?> props, final List<String> commands, final Map<String,?> env) {
- return new ToolAbstractExecScript(props) {
+ Boolean execAsync = getOptionalVal(props, PROP_EXEC_ASYNC);
+ if (Boolean.TRUE.equals(execAsync)) {
+ return execScriptAsyncAndPoll(props, commands, env);
+ } else {
+ return new ToolAbstractExecScript(props) {
+ public int run() {
+ String scriptContents = toScript(props, commands, env);
+ if (LOG.isTraceEnabled()) LOG.trace("Running shell command at {} as script: {}", host, scriptContents);
+ copyToServer(ImmutableMap.of("permissions", "0700"), scriptContents.getBytes(), scriptPath);
+
+ return asInt(acquire(new ShellAction(buildRunScriptCommand(), out, err)), -1);
+ }
+ }.run();
+ }
+ }
+
+ protected int execScriptAsyncAndPoll(final Map<String,?> props, final List<String> commands, final Map<String,?> env) {
--- End diff --
Complex, but looks good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097377
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
+ String touchCmd = String.format("touch %s; touch %s; touch %s; touch %s", stdoutPath, stderrPath, exitStatusPath, pidPath);
+ String cmd = String.format("( %s > %s 2> %s < /dev/null ; echo $? > %s ) & disown", scriptPath, stdoutPath, stderrPath, exitStatusPath);
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(touchCmd) : touchCmd))
--- End diff --
Extra parenthesis here and next line...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r22168192
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
+ String touchCmd = String.format("touch %s; touch %s; touch %s; touch %s", stdoutPath, stderrPath, exitStatusPath, pidPath);
+ String cmd = String.format("( %s > %s 2> %s < /dev/null ; echo $? > %s ) & disown", scriptPath, stdoutPath, stderrPath, exitStatusPath);
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(touchCmd) : touchCmd))
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("echo $! > "+pidPath)
+ .add("RESULT=$?");
+ if (noExtraOutput==null || !noExtraOutput) {
+ cmds.add("echo Executing async "+scriptPath);
+ }
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the exit status of the command, written to stdout.
+ */
+ protected List<String> buildRetrieveStatusCommand() {
+ String cmd =
+ "if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+"\n"+
+ "elif test -s "+pidPath+"; then"+"\n"+
+ " pid=`cat "+pidPath+"`"+"\n"+
+ " if ! ps -p $pid > /dev/null < /dev/null; then"+"\n"+
+ " # no exit status, and not executing; give a few seconds grace in case just about to write exit status"+"\n"+
+ " sleep 3"+"\n"+
+ " if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+""+"\n"+
+ " else"+"\n"+
+ " echo \"No exit status in a.exitstatus, and pid in a.pid ($pid) not executing\""+"\n"+
+ " exit 1"+"\n"+
+ " fi"+"\n"+
+ " fi"+"\n"+
+ "else"+"\n"+
+ " echo \"No exit status in "+exitStatusPath+", and "+pidPath+" is empty\""+"\n"+
+ " exit 1"+"\n"+
+ "fi"+"\n";
+
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("RESULT=$?");
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the stdout and stderr of the async command.
+ * An offset can be given, to only retrieve data starting at a particular character (indexed from 0).
+ */
+ protected List<String> buildRetrieveStdoutAndStderrCommand(int stdoutPosition, int stderrPosition) {
+ // Note that `tail -c +1` means start at the *first* character (i.e. start counting from 1, not 0)
+ String catStdoutCmd = "tail -c +"+(stdoutPosition+1)+" "+stdoutPath;
+ String catStderrCmd = "tail -c +"+(stderrPosition+1)+" "+stderrPath+" 1>&2";
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(catStdoutCmd) : catStdoutCmd))
--- End diff --
Ditto line 344
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r22168813
--- Diff: core/src/main/java/brooklyn/internal/BrooklynFeatureEnablement.java ---
@@ -102,6 +104,7 @@ static void setDefaults() {
setDefault(FEATURE_USE_BROOKLYN_LIVE_OBJECTS_DATAGRID_STORAGE, false);
setDefault(FEATURE_RENAME_THREADS, false);
setDefault(FEATURE_INFER_CATALOG_ITEM_ON_REBIND, true);
+ setDefault(FEATURE_SSH_ASYNC_EXEC, true);
--- End diff --
Should this default to false? My concern is that if it default to true, then existing scripts that already redirect stdin/out/err or fork them process may not play nicely with the changes
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#issuecomment-64566921
Thinking about this more, when we poll for the script's result should that be a long poll (i.e. leave this second ssh connection open until it gets a result; and if that poll connection fails then we can just re-connect). That sounds better than repeated ssh polling.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097323
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
+ String touchCmd = String.format("touch %s; touch %s; touch %s; touch %s", stdoutPath, stderrPath, exitStatusPath, pidPath);
--- End diff --
Can do `touch %s %s %s %s` as a single command.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097618
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
+ String touchCmd = String.format("touch %s; touch %s; touch %s; touch %s", stdoutPath, stderrPath, exitStatusPath, pidPath);
+ String cmd = String.format("( %s > %s 2> %s < /dev/null ; echo $? > %s ) & disown", scriptPath, stdoutPath, stderrPath, exitStatusPath);
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(touchCmd) : touchCmd))
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("echo $! > "+pidPath)
+ .add("RESULT=$?");
+ if (noExtraOutput==null || !noExtraOutput) {
+ cmds.add("echo Executing async "+scriptPath);
+ }
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the exit status of the command, written to stdout.
+ */
+ protected List<String> buildRetrieveStatusCommand() {
+ String cmd =
+ "if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+"\n"+
+ "elif test -s "+pidPath+"; then"+"\n"+
+ " pid=`cat "+pidPath+"`"+"\n"+
+ " if ! ps -p $pid > /dev/null < /dev/null; then"+"\n"+
+ " # no exit status, and not executing; give a few seconds grace in case just about to write exit status"+"\n"+
+ " sleep 3"+"\n"+
+ " if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+""+"\n"+
+ " else"+"\n"+
+ " echo \"No exit status in a.exitstatus, and pid in a.pid ($pid) not executing\""+"\n"+
+ " exit 1"+"\n"+
+ " fi"+"\n"+
+ " fi"+"\n"+
+ "else"+"\n"+
+ " echo \"No exit status in "+exitStatusPath+", and "+pidPath+" is empty\""+"\n"+
+ " exit 1"+"\n"+
+ "fi"+"\n";
+
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
--- End diff --
Extra parenthesis.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097514
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
+ String touchCmd = String.format("touch %s; touch %s; touch %s; touch %s", stdoutPath, stderrPath, exitStatusPath, pidPath);
+ String cmd = String.format("( %s > %s 2> %s < /dev/null ; echo $? > %s ) & disown", scriptPath, stdoutPath, stderrPath, exitStatusPath);
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(touchCmd) : touchCmd))
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("echo $! > "+pidPath)
+ .add("RESULT=$?");
+ if (noExtraOutput==null || !noExtraOutput) {
+ cmds.add("echo Executing async "+scriptPath);
+ }
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the exit status of the command, written to stdout.
+ */
+ protected List<String> buildRetrieveStatusCommand() {
+ String cmd =
+ "if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+"\n"+
+ "elif test -s "+pidPath+"; then"+"\n"+
+ " pid=`cat "+pidPath+"`"+"\n"+
+ " if ! ps -p $pid > /dev/null < /dev/null; then"+"\n"+
+ " # no exit status, and not executing; give a few seconds grace in case just about to write exit status"+"\n"+
+ " sleep 3"+"\n"+
+ " if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+""+"\n"+
+ " else"+"\n"+
+ " echo \"No exit status in a.exitstatus, and pid in a.pid ($pid) not executing\""+"\n"+
--- End diff --
are `a.exitstatus` and `a.pid` meant to be variable interpolations?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by aledsage <gi...@git.apache.org>.
GitHub user aledsage reopened a pull request:
https://github.com/apache/incubator-brooklyn/pull/358
Feature/async ssh
Supports executing a script asynchronously over ssh. Script redirects stdout and stderr to files, along with pid and exit status. Then caller executes additional ssh commands to poll for the script's completion.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/incubator-brooklyn feature/async-ssh
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/358.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #358
----
commit e8217628e13d5dc8ff01fadf373bb8fd117dc45a
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-25T21:42:33Z
Adds SshjTool exec script async
commit 109af263c5c7e7050c148af3a220f86518dcb3ab
Author: Aled Sage <al...@gmail.com>
Date: 2014-11-25T21:44:06Z
ShellToolAbstractTset.execCommands: remove duplication
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097945
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellTool.java ---
@@ -55,9 +56,13 @@
public static final ConfigKey<String> PROP_DIRECT_HEADER = newConfigKey("directHeader", "commands to run at the target before any caller-supplied commands for direct execution", "exec bash -e");
ConfigKey<Boolean> PROP_NO_DELETE_SCRIPT = newConfigKey("noDeleteAfterExec", "Retains the generated script file after executing the commands instead of deleting it", false);
-
+
ConfigKey<String> PROP_SUMMARY = ConfigKeys.newStringConfigKey("summary", "Provides a human-readable summary, used in file generation etc");
+ ConfigKey<Boolean> PROP_EXEC_ASYNC = newConfigKey("execAsync", "Executes the script asynchronously, and then polls for the result (and for stdout/stderr)", false);
+
+ ConfigKey<Duration> PROP_EXEC_ASYNC_TIMEOUT = newConfigKey("execAsyncTimeout", "Timeout when executing a script asynchronously", Duration.PRACTICALLY_FOREVER);
--- End diff --
Perhaps have a shorter timeout for the default, maybe 15 minutes?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#issuecomment-67960103
jenkins confirmed, and review comments incorporated. Note this is "low risk" because the feature is disabled by default. So merging.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#issuecomment-65097094
@aledsage Do you have more changes you want to add to this? Apart from the changes for tidying up how the command strings are created, this looks OK.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097660
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
+ String touchCmd = String.format("touch %s; touch %s; touch %s; touch %s", stdoutPath, stderrPath, exitStatusPath, pidPath);
+ String cmd = String.format("( %s > %s 2> %s < /dev/null ; echo $? > %s ) & disown", scriptPath, stdoutPath, stderrPath, exitStatusPath);
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(touchCmd) : touchCmd))
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("echo $! > "+pidPath)
+ .add("RESULT=$?");
+ if (noExtraOutput==null || !noExtraOutput) {
+ cmds.add("echo Executing async "+scriptPath);
+ }
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the exit status of the command, written to stdout.
+ */
+ protected List<String> buildRetrieveStatusCommand() {
+ String cmd =
+ "if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+"\n"+
+ "elif test -s "+pidPath+"; then"+"\n"+
+ " pid=`cat "+pidPath+"`"+"\n"+
+ " if ! ps -p $pid > /dev/null < /dev/null; then"+"\n"+
+ " # no exit status, and not executing; give a few seconds grace in case just about to write exit status"+"\n"+
+ " sleep 3"+"\n"+
+ " if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+""+"\n"+
+ " else"+"\n"+
+ " echo \"No exit status in a.exitstatus, and pid in a.pid ($pid) not executing\""+"\n"+
+ " exit 1"+"\n"+
+ " fi"+"\n"+
+ " fi"+"\n"+
+ "else"+"\n"+
+ " echo \"No exit status in "+exitStatusPath+", and "+pidPath+" is empty\""+"\n"+
+ " exit 1"+"\n"+
+ "fi"+"\n";
+
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("RESULT=$?");
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the stdout and stderr of the async command.
+ * An offset can be given, to only retrieve data starting at a particular character (indexed from 0).
+ */
+ protected List<String> buildRetrieveStdoutAndStderrCommand(int stdoutPosition, int stderrPosition) {
+ // Note that `tail -c +1` means start at the *first* character (i.e. start counting from 1, not 0)
+ String catStdoutCmd = "tail -c +"+(stdoutPosition+1)+" "+stdoutPath;
+ String catStderrCmd = "tail -c +"+(stderrPosition+1)+" "+stderrPath+" 1>&2";
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(catStdoutCmd) : catStdoutCmd))
--- End diff --
Extra parenthesis.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#issuecomment-64569146
I've fixed the (unrelated) failing test in https://github.com/apache/incubator-brooklyn/pull/361
Closing and re-opening to kick off apache jenkins again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097896
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
+ String touchCmd = String.format("touch %s; touch %s; touch %s; touch %s", stdoutPath, stderrPath, exitStatusPath, pidPath);
+ String cmd = String.format("( %s > %s 2> %s < /dev/null ; echo $? > %s ) & disown", scriptPath, stdoutPath, stderrPath, exitStatusPath);
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(touchCmd) : touchCmd))
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("echo $! > "+pidPath)
+ .add("RESULT=$?");
+ if (noExtraOutput==null || !noExtraOutput) {
+ cmds.add("echo Executing async "+scriptPath);
+ }
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the exit status of the command, written to stdout.
+ */
+ protected List<String> buildRetrieveStatusCommand() {
+ String cmd =
+ "if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+"\n"+
+ "elif test -s "+pidPath+"; then"+"\n"+
+ " pid=`cat "+pidPath+"`"+"\n"+
+ " if ! ps -p $pid > /dev/null < /dev/null; then"+"\n"+
+ " # no exit status, and not executing; give a few seconds grace in case just about to write exit status"+"\n"+
+ " sleep 3"+"\n"+
+ " if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+""+"\n"+
+ " else"+"\n"+
+ " echo \"No exit status in a.exitstatus, and pid in a.pid ($pid) not executing\""+"\n"+
+ " exit 1"+"\n"+
+ " fi"+"\n"+
+ " fi"+"\n"+
+ "else"+"\n"+
+ " echo \"No exit status in "+exitStatusPath+", and "+pidPath+" is empty\""+"\n"+
+ " exit 1"+"\n"+
+ "fi"+"\n";
+
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("RESULT=$?");
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the stdout and stderr of the async command.
+ * An offset can be given, to only retrieve data starting at a particular character (indexed from 0).
+ */
+ protected List<String> buildRetrieveStdoutAndStderrCommand(int stdoutPosition, int stderrPosition) {
+ // Note that `tail -c +1` means start at the *first* character (i.e. start counting from 1, not 0)
+ String catStdoutCmd = "tail -c +"+(stdoutPosition+1)+" "+stdoutPath;
+ String catStderrCmd = "tail -c +"+(stderrPosition+1)+" "+stderrPath+" 1>&2";
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(catStdoutCmd) : catStdoutCmd))
+ .add((runAsRoot ? BashCommands.sudo(catStderrCmd) : catStderrCmd))
+ .add("RESULT=$?");
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ protected List<String> deleteTemporaryFilesCommand() {
+ if (!Boolean.TRUE.equals(noDeleteAfterExec)) {
+ // use "-f" because some systems have "rm" aliased to "rm -i"
+ // use "< /dev/null" to guarantee doesn't hang
+ return ImmutableList.of(
+ "rm -f "+scriptPath+" < /dev/null",
+ "rm -f "+stdoutPath+" < /dev/null",
+ "rm -f "+stderrPath+" < /dev/null",
+ "rm -f "+exitStatusPath+" < /dev/null",
+ "rm -f "+pidPath+" < /dev/null");
--- End diff --
These _could_ be a single `rm -f` command?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/358
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#issuecomment-67844090
A couple of minor comments, but other than that, looks good
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Feature/async ssh
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/358#discussion_r21097589
--- Diff: core/src/main/java/brooklyn/util/internal/ssh/ShellAbstractTool.java ---
@@ -262,7 +264,112 @@ public ToolAbstractExecScript(Map<String,?> props) {
return cmds.build();
}
+ protected String getSummary() {
+ String summary = getOptionalVal(props, PROP_SUMMARY);
+ return (summary != null) ? summary : scriptPath;
+ }
+
public abstract int run();
}
+ protected abstract class ToolAbstractAsyncExecScript extends ToolAbstractExecScript {
+ protected final String stdoutPath;
+ protected final String stderrPath;
+ protected final String exitStatusPath;
+ protected final String pidPath;
+
+ public ToolAbstractAsyncExecScript(Map<String,?> props) {
+ super(props);
+
+ stdoutPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stdout");
+ stderrPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".stderr");
+ exitStatusPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".exitstatus");
+ pidPath = Os.mergePathsUnix(scriptDir, scriptNameWithoutExtension + ".pid");
+ }
+
+ /**
+ * Builds the command to run the given script, asynchronously.
+ * The executed command will return immediately, but the output from the script
+ * will continue to be written
+ * note that some modes require \$RESULT passed in order to access a variable, whereas most just need $ */
+ protected List<String> buildRunScriptCommand() {
+ // TODO
+ String touchCmd = String.format("touch %s; touch %s; touch %s; touch %s", stdoutPath, stderrPath, exitStatusPath, pidPath);
+ String cmd = String.format("( %s > %s 2> %s < /dev/null ; echo $? > %s ) & disown", scriptPath, stdoutPath, stderrPath, exitStatusPath);
+ MutableList.Builder<String> cmds = MutableList.<String>builder()
+ .add((runAsRoot ? BashCommands.sudo(touchCmd) : touchCmd))
+ .add((runAsRoot ? BashCommands.sudo(cmd) : cmd))
+ .add("echo $! > "+pidPath)
+ .add("RESULT=$?");
+ if (noExtraOutput==null || !noExtraOutput) {
+ cmds.add("echo Executing async "+scriptPath);
+ }
+ cmds.add("exit $RESULT");
+ return cmds.build();
+ }
+
+ /**
+ * Builds the command to retrieve the exit status of the command, written to stdout.
+ */
+ protected List<String> buildRetrieveStatusCommand() {
+ String cmd =
+ "if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+"\n"+
+ "elif test -s "+pidPath+"; then"+"\n"+
+ " pid=`cat "+pidPath+"`"+"\n"+
+ " if ! ps -p $pid > /dev/null < /dev/null; then"+"\n"+
+ " # no exit status, and not executing; give a few seconds grace in case just about to write exit status"+"\n"+
+ " sleep 3"+"\n"+
+ " if test -s "+exitStatusPath+"; then"+"\n"+
+ " cat "+exitStatusPath+""+"\n"+
+ " else"+"\n"+
+ " echo \"No exit status in a.exitstatus, and pid in a.pid ($pid) not executing\""+"\n"+
+ " exit 1"+"\n"+
+ " fi"+"\n"+
+ " fi"+"\n"+
+ "else"+"\n"+
+ " echo \"No exit status in "+exitStatusPath+", and "+pidPath+" is empty\""+"\n"+
+ " exit 1"+"\n"+
+ "fi"+"\n";
--- End diff --
Might be clearer to have a `List<String>` created with the commands, and then `Iterables.join("\n")` to make the script.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---