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

Proposed by Martin Packman on 2016-07-11
Status: Merged
Approved by: Martin Packman on 2016-07-11
Approved revision: 27
Merged at revision: 27
Proposed branch: lp:~gz/workspace-runner/ssh_server_alive
Merge into: lp:workspace-runner
Prerequisite: lp:~gz/workspace-runner/sorted_walk_results
Diff against target: 78 lines (+8/-5)
2 files modified
workspace_runner/__init__.py (+1/-1)
workspace_runner/tests/__init__.py (+7/-4)
To merge this branch: bzr merge lp:~gz/workspace-runner/ssh_server_alive
Reviewer Review Type Date Requested Status
Aaron Bentley (community) 2016-07-11 Approve on 2016-07-11
Review via email: mp+299722@code.launchpad.net

Commit message

Add ServerAliveInterval=120 option when using ssh

Description of the change

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.

To post a comment you must log in.
Aaron Bentley (abentley) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
7878
79 It includes the host and private key (if any) to use.79 It includes the host and private key (if any) to use.
80 """80 """
81 ssh_options = ('StrictHostKeyChecking=no', 'ServerAliveInterval=120')
8182
82 def __init__(self, host, private_key):83 def __init__(self, host, private_key):
83 self.host = host84 self.host = host
84 self.private_key = private_key85 self.private_key = private_key
85 self.known_hosts = None86 self.known_hosts = None
86 self.ssh_options = ['StrictHostKeyChecking=no']
8787
88 def get_ssh_options(self):88 def get_ssh_options(self):
89 """Return the standard options for this SSH connection."""89 """Return the standard options for this SSH connection."""
9090
=== 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 @@
217 self.assertEqual(217 self.assertEqual(
218 connection.get_ssh_options(), [218 connection.get_ssh_options(), [
219 '-oStrictHostKeyChecking=no',219 '-oStrictHostKeyChecking=no',
220 '-oServerAliveInterval=120',
220 '-oUserKnownHostsFile=/dev/null',221 '-oUserKnownHostsFile=/dev/null',
221 '-i', 'private-key'])222 '-i', 'private-key'])
222223
@@ -226,6 +227,7 @@
226 self.assertEqual(227 self.assertEqual(
227 connection.get_ssh_options(), [228 connection.get_ssh_options(), [
228 '-oStrictHostKeyChecking=no',229 '-oStrictHostKeyChecking=no',
230 '-oServerAliveInterval=120',
229 '-oUserKnownHostsFile=foo/bar/baz'])231 '-oUserKnownHostsFile=foo/bar/baz'])
230232
231 def test_call_with_input_error(self):233 def test_call_with_input_error(self):
@@ -241,6 +243,7 @@
241 connection.scp_dir_from_remote('source', 'dest')243 connection.scp_dir_from_remote('source', 'dest')
242 pc_mock.assert_called_once_with(244 pc_mock.assert_called_once_with(
243 ['scp', '-r', '-oStrictHostKeyChecking=no',245 ['scp', '-r', '-oStrictHostKeyChecking=no',
246 '-oServerAliveInterval=120',
244 '-oUserKnownHostsFile=/dev/null', '-i', 'key',247 '-oUserKnownHostsFile=/dev/null', '-i', 'key',
245 'host@example.com:source/', 'dest'])248 'host@example.com:source/', 'dest'])
246249
@@ -299,13 +302,13 @@
299302
300 def expect_run(self, primitives, args, out_file):303 def expect_run(self, primitives, args, out_file):
301 def effect(args, stdout):304 def effect(args, stdout):
302 stdout.write(''.join(args[7:]))305 stdout.write(''.join(args[8:]))
303306
304 with patch('subprocess.check_call', side_effect=effect,307 with patch('subprocess.check_call', side_effect=effect,
305 autospec=True) as cc_mock:308 autospec=True) as cc_mock:
306 primitives.run(args, out_file)309 primitives.run(args, out_file)
307 call_args = [310 call_args = [
308 'ssh', '-oStrictHostKeyChecking=no',311 'ssh', '-oStrictHostKeyChecking=no', '-oServerAliveInterval=120',
309 '-oUserKnownHostsFile=/dev/null',312 '-oUserKnownHostsFile=/dev/null',
310 primitives.ssh_connection.host, 'cd', quote(primitives.workspace),313 primitives.ssh_connection.host, 'cd', quote(primitives.workspace),
311 ';'] + args,314 ';'] + args,
@@ -321,7 +324,7 @@
321 dest = '{}:{}/'.format(primitives.ssh_connection.host,324 dest = '{}:{}/'.format(primitives.ssh_connection.host,
322 os.path.join(primitives.workspace, dest))325 os.path.join(primitives.workspace, dest))
323 po_mock.assert_called_once_with(326 po_mock.assert_called_once_with(
324 ['ssh', '-oStrictHostKeyChecking=no',327 ['ssh', '-oStrictHostKeyChecking=no', '-oServerAliveInterval=120',
325 '-oUserKnownHostsFile=/dev/null',328 '-oUserKnownHostsFile=/dev/null',
326 'asdf', 'python - /tmp/workspace/install-dir'],329 'asdf', 'python - /tmp/workspace/install-dir'],
327 stdin=subprocess.PIPE, stdout=subprocess.PIPE)330 stdin=subprocess.PIPE, stdout=subprocess.PIPE)
@@ -333,7 +336,7 @@
333 os.mkdir(sys.argv[1])336 os.mkdir(sys.argv[1])
334 """))337 """))
335 cc_mock.assert_called_once_with([338 cc_mock.assert_called_once_with([
336 'scp', '-oStrictHostKeyChecking=no',339 'scp', '-oStrictHostKeyChecking=no', '-oServerAliveInterval=120',
337 '-oUserKnownHostsFile=/dev/null',340 '-oUserKnownHostsFile=/dev/null',
338 ] + sources + [dest])341 ] + sources + [dest])
339342

Subscribers

People subscribed via source and target branches