Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command to restart ssh connections #491

Closed
paladox opened this issue Nov 2, 2016 · 26 comments
Closed

Command to restart ssh connections #491

paladox opened this issue Nov 2, 2016 · 26 comments

Comments

@paladox
Copy link

paladox commented Nov 2, 2016

Hi is there a way I can restart a ssh connection after it is disconnected please.

I am trying to do https://gerrit.wikimedia.org/r/#/c/318976/

It allows us to use an irc command to restart the bot. I managed to create another ssh connection but it just duplicates things so i have two ssh connections. Then if we do the command again it will do it a third and so on. We use this for gerrit which gets stopped when it gets updated so we need to restart the ssh connection. But it would automate things doing it through irc.

Could I have help please?

@mscdex
Copy link
Owner

mscdex commented Nov 2, 2016

Are you referring to the connection you're making in subscribeToGerritStream()? Either way you technically should be able to just call .connect() again on the old ssh connection object, but you could just as easily re-create a new connection object and call .connect() again, since you'd have to re-.exec() your command anyway.

Also, on an unrelated note, while looking at the file you linked to I noticed that you're trying to JSON.parse(data) inside a stream's 'data' event handler. This is generally not a safe thing to do since you may/may not receive a complete JSON message in a single 'data' event. Typically when it comes to parsing many JSON messages, users will newline delimit their JSON messages so it's easy to buffer up to a delimiter and then parse the buffered data. If adding a delimiter is not possible (since it is coming from an external command), you might look into a streaming JSON parser (there are at least a few on npm).

@paladox
Copy link
Author

paladox commented Nov 2, 2016

Oh yep this subscribeToGerritStream.

How can I create a new connection and close the other one please?

@mscdex
Copy link
Owner

mscdex commented Nov 2, 2016

If the ssh connection is actually being disconnected by the server, a simple reconnect solution might be something like:

function subscribeToGerritStream(host, port, username, keypath, listener) {
  var conn = ssh2.Client();
  logging.info('Connecting to gerrit..');
  conn.connect({
    // ...
  });
  conn.on('ready', function() {
    // ...
  }).on('close', function() {
    console.log('Client disconnected');
    subscribeToGerritStream(host, port, username, keypath, listener);
  });
}

If the ssh connection is not disconnected by the server and you need to either re-.exec() the command to get your event stream flowing again or you need to explicitly reconnect for whatever reason, then that would be a little more involved because you'd need to store your ssh connection objects somewhere so that you can perform the appropriate action.

@paladox
Copy link
Author

paladox commented Nov 3, 2016

So will doing this work even when it retry's the ssh server might be down, but will keep retrying?

@mscdex
Copy link
Owner

mscdex commented Nov 3, 2016

Yes, the 'close' event will always be called when a socket is closed, whether a connection was ended gracefully or closed due to error. Speaking of which, you'll want to add an 'error' event handler for the connection in case of errors, to prevent the process from dying by default.

@paladox
Copy link
Author

paladox commented Nov 3, 2016

Is it also possible for me to issue an irc command like !grrrit-wm-restart, and it will restart ssh?

If so how please.

@mscdex
Copy link
Owner

mscdex commented Nov 3, 2016

For that case you'd need to store your ssh connection object so that it's accessible from the irc message handler. Move the variable declaration to the irc message handler's parent scope. For example:

var sshConn = null;

function subscribeToGerritStream(host, port, username, keypath, listener) {
  sshConn = new ssh2.Client();

  logging.info('Connecting to gerrit..');

  sshConn.on('ready', function() {
    // ...
  }).on('error', function(err) {
    console.log('Client error: ' + err);
  }).on('close', function() {
    console.log('Client disconnected');
    // Reconnect
    subscribeToGerritStream(host, port, username, keypath, listener);
  }).connect({
    // ...
  });
}

// ...

ircClient.addListener('message', function (from, to, text) {
  // ...
  if (command === '!grrrit-wm-restart') {
    if (sshConn) {
      ircClient.say(to, "forcing reconnection to gerrit ...");
      sshConn.end();
    } else {
      ircClient.say(to, "waiting for initial connection to gerrit ...");
    }
  }
});

@paladox
Copy link
Author

paladox commented Nov 3, 2016

Hi thankyou very much.

But restarting it, causes this

info: Connected to event stream!
/data/project/lolrrit-wm/gerrit/src/relay.js:51
listener(err, JSON.parse(data));
^

TypeError: listener is not a function

@paladox
Copy link
Author

paladox commented Nov 3, 2016

Never mind about the error seems to happen then and again.

@mscdex
Copy link
Owner

mscdex commented Nov 3, 2016

I don't see how that's possible judging by the current code for src/relay.js and the suggested changes. It's passing the exact same values when reconnecting.

@paladox
Copy link
Author

paladox commented Nov 3, 2016

Thankyou very much for helping. Your example worked :).

@mscdex mscdex closed this as completed Nov 3, 2016
@mscdex
Copy link
Owner

mscdex commented Nov 4, 2016

@paladox That commit you just pushed is missing the automatic reconnect (calling the subscribe function again from the ssh connection's 'close' event handler), I don't know if that was intentional or not, but thought I might bring it to your attention just in case.

@paladox
Copy link
Author

paladox commented Nov 4, 2016

@mscdex thanks, yeh I had a change but that change wasn't supposed to be merged. fixed in https://gerrit.wikimedia.org/r/#/c/319780/3

@paladox
Copy link
Author

paladox commented Nov 4, 2016

@mscdex hi, I'm wondering do you know how I can create a whitelist based on a cloak (irc) please. Since we want to only allow certain commands to be run only if you have a cloak which should prevent any spammer from trying to steal another user identity.

https://github.com/martynsmith/node-irc/

how I am currently doing the whitelist is at https://gerrit.wikimedia.org/r/#/c/319780/

but I want to change this so it checks the cloak.

@mscdex
Copy link
Owner

mscdex commented Nov 4, 2016

That's beyond the scope of this module and I haven't used a node.js irc module in years, so I can't really help you with that. Your best bet would be to ask on the issue tracker of the module you're using.

@paladox
Copy link
Author

paladox commented Nov 4, 2016

@mscdex Hi with this https://gerrit.wikimedia.org/r/#/c/319908/3/src/relay.js it seems that when I do grrrit-wm: force-restart

it quits and then rejoins but then I keep getting ssh disconnecting and then it reconnects.

Could I have help to prevent it keep trying to do ssh when it is already connected please?

it keeps doing

info: Connecting to gerrit..
info: Connected; requesting stream-events
info: Connected to event stream!
info: Connecting to gerrit..
info: Connected; requesting stream-events
info: Connected to event stream!
info: Connecting to gerrit..
info: Connected; requesting stream-events
info: Connected to event stream!

even though it is already connected and it does it multiple times when I do grrrit-wm: force-restart.

@mscdex
Copy link
Owner

mscdex commented Nov 4, 2016

I don't see anything wrong with the current code in the repo, other than a duplicate logging.info('Connecting to gerrit..');, 'reconnected' messages being logged/"said" at the wrong times (they're shown immediately instead of when it's actually reconnected), and the potential JSON.parse() issues noted before.

That log seems a bit suspect though because it should be showing at least disconnection messages if it was reconnecting. You may need to change console.log() to logger.info() too.

@paladox
Copy link
Author

paladox commented Nov 4, 2016

Oh, after 10+ mins it causes the bot to disconnect from ssh and irc, and keeps replaying

Client error: Error: Timed out while waiting for handshake
info: Connecting to gerrit..

@paladox
Copy link
Author

paladox commented Nov 4, 2016

it seems that ssh keeps reconnecting, only happends when I do grrrit-wm: restart.

@mscdex
Copy link
Owner

mscdex commented Nov 4, 2016

You could try using sshConn.destroy() (immediately closes the TCP socket) instead of sshConn.end() (tries to perform a graceful disconnection) to be more forceful, that should definitely cancel any pending connection handshakes.

@paladox
Copy link
Author

paladox commented Nov 4, 2016

Oh thankyou.

@paladox
Copy link
Author

paladox commented Nov 4, 2016

Nope that didn't work either, but I have found it keeps calling

it seems that it keeps restarting it's connection when i run grrrit-wm: force-restart

startRelay();
^^ that keeps getting executed in the close part of ssh.

yep it keeps calling it in close }).on('close', function() {

may be a bug?

Or it's the variable

    if (sshConn ) {
        sshConn.destroy();
    }

or it could be sshConn = null not working properly?

@paladox
Copy link
Author

paladox commented Nov 4, 2016

@mscdex how could I fix the problem you said with JSON.parse() please?

@paladox
Copy link
Author

paladox commented Nov 5, 2016

Oh I found the problem, it was setinterval.

@mscdex
Copy link
Owner

mscdex commented Nov 5, 2016

Ah yes, setInterval() will do that :-)

Regarding JSON.parse(), here was my comment from earlier:

Also, on an unrelated note, while looking at the file you linked to I noticed that you're trying to JSON.parse(data) inside a stream's 'data' event handler. This is generally not a safe thing to do since you may/may not receive a complete JSON message in a single 'data' event. Typically when it comes to parsing many JSON messages, users will newline delimit their JSON messages so it's easy to buffer up to a delimiter and then parse the buffered data. If adding a delimiter is not possible (since it is coming from an external command), you might look into a streaming JSON parser (there are at least a few on npm).

@paladox
Copy link
Author

paladox commented Nov 5, 2016

@mscdex thanks but I am unsure how to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants