You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by sw...@apache.org on 2016/04/21 23:00:19 UTC

[2/2] git commit: updated refs/heads/master to d518b61

Merge pull request #1459 from GabrielBrascher/CLOUDSTACK-8611

This closes #561

CLOUDSTACK-8611:Handle SSH if server "forget" to send exit statusContinuing the work started by @likitha, I did not cherry-picked the
commit (b9181c689e0e7b5f1e28c81d73710196dfabd0ba) from PR <https://github.com/apache/cloudstack/pull/561> due to the fact that the path of that SshHelper class was different of the current SshHelper; that is because the fact that by cherry-picking it would seem that I had changed all the class as the code is from another file.

I made some changes from the cherry-picked commit adding @wilderrodrigues suggestions (create simple methods to have reusable code, make unit tests and create the `WAITING_OPEN_SSH_SESSION` variable to manipulate with the delay of 1000 milliseconds).

Also, I tried to simplify the logic by assuming that ....

    if ((conditions & ChannelCondition.EXIT_STATUS) != 0) {
            if ((conditions & (ChannelCondition.STDOUT_DATA | ChannelCondition.STDERR_DATA)) == 0) {
                break;
            }
    }

... is the same as `((conditions & ChannelCondition.EXIT_STATUS) != 0) && ((conditions & (ChannelCondition.STDOUT_DATA | ChannelCondition.STDERR_DATA)) == 0)`. This expression has the following results according to each possible condition.

|Condition|Value|result
|-----------------|-------|------|
TIMEOUT  | 0000001|false
CLOSED  | 0000010 |false
STDERR_DATA | 0000100 | false
STDERR_DATA | 0001000 | false
EOF         | 0010000 | false
EXIT_STATUS | 0100000 | **true**
EXIT_SIGNAL | 1000000 | false

After testing all the possibilities we can note that the condition of `(conditions & ChannelCondition.EXIT_STATUS) != 0` is sufficient; thus, the simplified "if" conditional can be:

`if ((conditions & ChannelCondition.EXIT_STATUS) != 0) {
    break;
}`

This proposed work can be explained by quoting @likitha:
>CheckS2SVpnConnectionsCommand execution involves executing a script (checkbatchs2svpn.sh) in the virtual router. Once CS has opened a session to a virtual router and executed a script in the router, it waits indefinitely till the session either times out or the exit status of the remote process is available. But it is possible that an EOF is reached by the process in the router and the router never set the exit status.

>References -
>1. Some servers never send the exit status, or occasionally "forget" to do so (http://grepcode.com/file/repo1.maven.org/maven2/org.jvnet.hudson/trilead-ssh2/build212-hudson-1/com/trilead/ssh2/ChannelCondition.java).
>2. Get the exit code/status from the remote command - if available. Be careful - not all server implementations return this value - (http://grepcode.com/file/repo1.maven.org/maven2/org.jvnet.hudson/trilead-ssh2/build212-hudson-1/com/trilead/ssh2/Session.java#Session.waitForCondition%28int%2Clong%29).

* pr/1459:
  Handle SSH if server "forget" to send exit status

Signed-off-by: Will Stevens <wi...@gmail.com>


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

Branch: refs/heads/master
Commit: d518b619dda69dde4ecc1640e6c007182c9a9b75
Parents: 5498170 abae908
Author: Will Stevens <wi...@gmail.com>
Authored: Thu Apr 21 16:58:23 2016 -0400
Committer: Will Stevens <wi...@gmail.com>
Committed: Thu Apr 21 16:59:16 2016 -0400

----------------------------------------------------------------------
 .../java/com/cloud/utils/ssh/SshHelper.java     | 108 ++++++++++---
 .../java/com/cloud/utils/ssh/SshHelperTest.java | 151 +++++++++++++++++++
 2 files changed, 241 insertions(+), 18 deletions(-)
----------------------------------------------------------------------