Merge lp:~gz/workspace-runner/ssh_server_alive into lp:workspace-runner

[Martin Packman]

Add ServerAliveInterval=120 option when using ssh

It seems the timeouts on the race job under workspace runner are due to a long period with no output from the remote end and no activity from the runner.

The last line in the gating race output is the api/watcher suite. Next up would be apiserver, which takes >400 seconds to complete in the main race job.

ok github.com/juju/juju/api/watcher 14.754s
ok github.com/juju/juju/apiserver 403.091s

We can try to work around this by getting the client to poke at a regular interval. For now, seeing if 2 minutes is good enough.

[Aaron Bentley (abentley)]

Works for me.

I was a little surprised to see ssh_options becoming a class variable instead of an instance variable, but I guess it doesn't matter much-- class variables can always be overridden.

review: Approve

=== modified file 'workspace_runner/__init__.py'
--- workspace_runner/__init__.py	2015-10-08 23:44:14 +0000
+++ workspace_runner/__init__.py	2016-07-11 18:17:23 +0000
@@ -78,12 +78,12 @@
 
     It includes the host and private key (if any) to use.
     """
+    ssh_options = ('StrictHostKeyChecking=no', 'ServerAliveInterval=120')
 
     def __init__(self,  host, private_key):
         self.host = host
         self.private_key = private_key
         self.known_hosts = None
-        self.ssh_options = ['StrictHostKeyChecking=no']
 
     def get_ssh_options(self):
         """Return the standard options for this SSH connection."""
 
=== modified file 'workspace_runner/tests/__init__.py'
--- workspace_runner/tests/__init__.py	2016-07-11 18:17:23 +0000
+++ workspace_runner/tests/__init__.py	2016-07-11 18:17:23 +0000
@@ -217,6 +217,7 @@
         self.assertEqual(
             connection.get_ssh_options(), [
                 '-oStrictHostKeyChecking=no',
+                '-oServerAliveInterval=120',
                 '-oUserKnownHostsFile=/dev/null',
                 '-i', 'private-key'])
 
@@ -226,6 +227,7 @@
         self.assertEqual(
             connection.get_ssh_options(), [
                 '-oStrictHostKeyChecking=no',
+                '-oServerAliveInterval=120',
                 '-oUserKnownHostsFile=foo/bar/baz'])
 
     def test_call_with_input_error(self):
@@ -241,6 +243,7 @@
             connection.scp_dir_from_remote('source', 'dest')
         pc_mock.assert_called_once_with(
             ['scp', '-r', '-oStrictHostKeyChecking=no',
+             '-oServerAliveInterval=120',
              '-oUserKnownHostsFile=/dev/null', '-i', 'key',
              '[e-mail]:source/', 'dest'])
 
@@ -299,13 +302,13 @@
 
     def expect_run(self, primitives, args, out_file):
         def effect(args, stdout):
-            stdout.write(''.join(args[7:]))
+            stdout.write(''.join(args[8:]))
 
         with patch('subprocess.check_call', side_effect=effect,
                    autospec=True) as cc_mock:
             primitives.run(args, out_file)
         call_args = [
-            'ssh', '-oStrictHostKeyChecking=no',
+            'ssh', '-oStrictHostKeyChecking=no', '-oServerAliveInterval=120',
             '-oUserKnownHostsFile=/dev/null',
             primitives.ssh_connection.host, 'cd', quote(primitives.workspace),
             ';'] + args,
@@ -321,7 +324,7 @@
         dest = '{}:{}/'.format(primitives.ssh_connection.host,
                                os.path.join(primitives.workspace, dest))
         po_mock.assert_called_once_with(
-            ['ssh', '-oStrictHostKeyChecking=no',
+            ['ssh', '-oStrictHostKeyChecking=no', '-oServerAliveInterval=120',
              '-oUserKnownHostsFile=/dev/null',
              'asdf', 'python - /tmp/workspace/install-dir'],
             stdin=subprocess.PIPE, stdout=subprocess.PIPE)
@@ -333,7 +336,7 @@
             os.mkdir(sys.argv[1])
             """))
         cc_mock.assert_called_once_with([
-            'scp', '-oStrictHostKeyChecking=no',
+            'scp', '-oStrictHostKeyChecking=no', '-oServerAliveInterval=120',
             '-oUserKnownHostsFile=/dev/null',
             ] + sources + [dest])