Merge lp:~hazmat/pyjuju/increase-session-timeout into lp:pyjuju

[Kapil Thangavelu]

Increase zk session and ping times.

During scale testing work, one of the take aways was the default zookeeper
session time was too low, which caused issues for any transient communication
problems between the agents the zk. The managed client work landed subsequently
does recovery for this and for session expiration, but its better to avoid
the issue entirely by increasing the session time and increasing the heartbeat
time periodicity.

https://codereview.appspot.com/7231079/

[Kapil Thangavelu (hazmat)]

Reviewers: mp+146169_code.launchpad.net,

Message:
Please take a look.

Description:
Increase zk session and ping times.

During scale testing work, one of the take aways was the default
zookeeper
session time was too low, which caused issues for any transient
communication
problems between the agents the zk. The managed client work landed
subsequently
does recovery for this and for session expiration, but its better to
avoid
the issue entirely by increasing the session time and increasing the
heartbeat
time periodicity.

https://code.launchpad.net/~hazmat/juju/increase-session-timeout/+merge/146169

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7231079/

Affected files:
   A [revision details]
   M juju/agents/base.py
   A juju/lib/port.py
   A juju/lib/tests/test_port.py
   M juju/lib/tests/test_zk.py
   M juju/lib/zk.py
   M juju/providers/common/cloudinit.py
   M juju/providers/common/connect.py
   M juju/providers/common/tests/data/cloud_init_bootstrap
   M juju/providers/common/tests/data/cloud_init_bootstrap_testing
   M juju/providers/common/tests/data/cloud_init_bootstrap_zookeepers
   M juju/providers/local/__init__.py
   M juju/providers/local/tests/test_files.py
   M juju/state/sshclient.py
   M juju/state/tests/test_utils.py
   M juju/state/utils.py
   M test

[Benjamin Saller (bcsaller)]

LGTM, some minors below but they are optional.

https://codereview.appspot.com/7231079/diff/1/juju/lib/port.py
File juju/lib/port.py (right):

https://codereview.appspot.com/7231079/diff/1/juju/lib/port.py#newcode5
juju/lib/port.py:5: """Get an open port on the machine.
There is a race between close and return, but the kernel shouldn't
re-allocate the same port number again for some reasonable window so
this should be ok.

https://codereview.appspot.com/7231079/diff/1/juju/lib/zk.py
File juju/lib/zk.py (right):

https://codereview.appspot.com/7231079/diff/1/juju/lib/zk.py#newcode57
juju/lib/zk.py:57: use_deferred=True, min_session_timeout=30000,
Should you use the constant above for min and define one for max and use
it as well?

https://codereview.appspot.com/7231079/diff/1/juju/providers/common/cloudinit.py
File juju/providers/common/cloudinit.py (right):

https://codereview.appspot.com/7231079/diff/1/juju/providers/common/cloudinit.py#newcode46
juju/providers/common/cloudinit.py:46: 'echo "maxSessionTimeout=%d" >>
/etc/zookeeper/conf/zoo.cfg' % (
It might make sense that these are defined in lib.zk rather than
calculated values here.

https://codereview.appspot.com/7231079/diff/1/juju/providers/common/connect.py
File juju/providers/common/connect.py (right):

https://codereview.appspot.com/7231079/diff/1/juju/providers/common/connect.py#newcode67
juju/providers/common/connect.py:67: client = yield
SSHClient(session_timeout=20000).connect(
Should this be the default in SSHClient.__init__?

https://codereview.appspot.com/7231079/

[Kapil Thangavelu (hazmat)]

thanks for the review.

https://codereview.appspot.com/7231079/diff/1/juju/lib/port.py
File juju/lib/port.py (right):

https://codereview.appspot.com/7231079/diff/1/juju/lib/port.py#newcode5
juju/lib/port.py:5: """Get an open port on the machine.
On 2013/02/01 17:12:20, bcsaller wrote:
> There is a race between close and return, but the kernel shouldn't
re-allocate
> the same port number again for some reasonable window so this should
be ok.

this is a refactor from elsewhere, the so reuse i believe helps with a
concurrent handout, in either case better than a hardcode.

https://codereview.appspot.com/7231079/diff/1/juju/lib/zk.py
File juju/lib/zk.py (right):

https://codereview.appspot.com/7231079/diff/1/juju/lib/zk.py#newcode57
juju/lib/zk.py:57: use_deferred=True, min_session_timeout=30000,
On 2013/02/01 17:12:20, bcsaller wrote:
> Should you use the constant above for min and define one for max and
use it as
> well?

makes sense, done.

https://codereview.appspot.com/7231079/diff/1/juju/providers/common/cloudinit.py
File juju/providers/common/cloudinit.py (right):

https://codereview.appspot.com/7231079/diff/1/juju/providers/common/cloudinit.py#newcode46
juju/providers/common/cloudinit.py:46: 'echo "maxSessionTimeout=%d" >>
/etc/zookeeper/conf/zoo.cfg' % (
On 2013/02/01 17:12:20, bcsaller wrote:
> It might make sense that these are defined in lib.zk rather than
calculated
> values here.

done.

https://codereview.appspot.com/7231079/diff/1/juju/providers/common/connect.py
File juju/providers/common/connect.py (right):

https://codereview.appspot.com/7231079/diff/1/juju/providers/common/connect.py#newcode67
juju/providers/common/connect.py:67: client = yield
SSHClient(session_timeout=20000).connect(
On 2013/02/01 17:12:20, bcsaller wrote:
> Should this be the default in SSHClient.__init__?

sshclient doesn't define that, changed to use the lib.zk
CLIENT_SESSION_TIME

https://codereview.appspot.com/7231079/

[lp:~hazmat/pyjuju/increase-session-timeout]

455. By Kapil Thangavelu on 2013-02-01

address review comments

[Kapil Thangavelu (hazmat)]

Please take a look.

https://codereview.appspot.com/7231079/

[Benjamin Saller (bcsaller)]

Thanks for the changes, LGTM

https://codereview.appspot.com/7231079/

[Kapil Thangavelu (hazmat)]

*** Submitted:

Increase zk session and ping times.

During scale testing work, one of the take aways was the default
zookeeper
session time was too low, which caused issues for any transient
communication
problems between the agents the zk. The managed client work landed
subsequently
does recovery for this and for session expiration, but its better to
avoid
the issue entirely by increasing the session time and increasing the
heartbeat
time periodicity.

R=bcsaller
CC=
https://codereview.appspot.com/7231079

https://codereview.appspot.com/7231079/

=== modified file 'juju/agents/base.py'
--- juju/agents/base.py	2012-04-26 18:32:14 +0000
+++ juju/agents/base.py	2013-02-01 17:52:19 +0000
@@ -18,6 +18,7 @@
 from juju.control.options import setup_twistd_options
 from juju.errors import NoConnection, JujuError
 from juju.lib.zklog import ZookeeperHandler
+from juju.lib.zk import CLIENT_SESSION_TIMEOUT
 from juju.state.environment import GlobalSettingsStateManager
 
 
@@ -204,7 +205,8 @@
     def connect(self):
         """Return an authenticated connection to the juju zookeeper."""
         yield self._kill_existing_session()
-        self.client = yield ManagedClient().connect(
+        self.client = yield ManagedClient(
+            session_timeout=CLIENT_SESSION_TIMEOUT).connect(
             self.config["zookeeper_servers"])
         save_client_id(
             self.config["session_file"], self.client.client_id)

=== added file 'juju/lib/port.py'
--- juju/lib/port.py	1970-01-01 00:00:00 +0000
+++ juju/lib/port.py	2013-02-01 17:52:19 +0000
@@ -0,0 +1,13 @@
+import socket
+
+
+def get_open_port(host=""):
+    """Get an open port on the machine.
+    """
+    temp_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    temp_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+    temp_sock.bind((host, 0))
+    port = temp_sock.getsockname()[1]
+    temp_sock.close()
+    del temp_sock
+    return port

=== added file 'juju/lib/tests/test_port.py'
--- juju/lib/tests/test_port.py	1970-01-01 00:00:00 +0000
+++ juju/lib/tests/test_port.py	2013-02-01 17:52:19 +0000
@@ -0,0 +1,32 @@
+import socket
+from unittest import TestCase
+from juju.lib.port import get_open_port
+
+
+class OpenPortTest(TestCase):
+
+    def test_get_open_port(self):
+        port = get_open_port()
+        self.assertTrue(isinstance(port, int))
+
+        sock = socket.socket(
+            socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP)
+
+        # would raise an error if we got it wrong.
+        sock.bind(("127.0.0.1", port))
+        sock.listen(1)
+        sock.close()
+        del sock
+
+    def test_get_open_port_with_host(self):
+        port = get_open_port("localhost")
+        self.assertTrue(isinstance(port, int))
+
+        sock = socket.socket(
+            socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP)
+
+        # would raise an error if we got it wrong.
+        sock.bind(("127.0.0.1", port))
+        sock.listen(1)
+        sock.close()
+        del sock

=== modified file 'juju/lib/tests/test_zk.py'
--- juju/lib/tests/test_zk.py	2011-09-16 00:36:31 +0000
+++ juju/lib/tests/test_zk.py	2013-02-01 17:52:19 +0000
@@ -7,6 +7,7 @@
 
 from juju.lib.testing import TestCase
 from juju.lib.zk import Zookeeper, check_zookeeper
+from juju.lib.port import get_open_port
 
 
 sample_package_environment_conf = """\
@@ -46,12 +47,11 @@
         self.makeFile("", path=os.path.join(
             software_dir, "build", "zookeeper-3.4.0.jar"))
 
-        for p in [
-            "jline-0.9.94.jar",
-            "netty-3.2.2.Final.jar",
-            "log4j-1.2.15.jar",
-            "slf4j-log4j12-1.6.1.jar",
-            "slf4j-api-1.6.1.jar"]:
+        for p in ["jline-0.9.94.jar",
+                  "netty-3.2.2.Final.jar",
+                  "log4j-1.2.15.jar",
+                  "slf4j-log4j12-1.6.1.jar",
+                  "slf4j-api-1.6.1.jar"]:
             self.makeFile("", path=os.path.join(lib_dir, p))
 
         instance = Zookeeper(data_dir, 12345, zk_location=software_dir)
@@ -96,16 +96,22 @@
     def test_managed_zookeeper(self):
         zookeeper.set_debug_level(0)
 
+        port = get_open_port()
         # Start zookeeper
         data_dir = self.makeDir()
-        instance = Zookeeper(data_dir, 12345)
+        instance = Zookeeper(
+            data_dir,
+            port,
+            min_session_timeout=80000,
+            max_session_timeout=100000)
         yield instance.start()
         self.assertTrue(instance.running)
 
         # Connect a client
-        client = ZookeeperClient("127.0.1.1:12345")
+        client = ZookeeperClient("127.0.1.1:%d" % port)
         yield client.connect()
         stat = yield client.exists("/")
+        self.assertEqual(client.session_timeout, 80000)
         yield client.close()
         self.assertTrue(stat)
 

=== modified file 'juju/lib/zk.py'
--- juju/lib/zk.py	2012-06-26 16:36:32 +0000
+++ juju/lib/zk.py	2013-02-01 17:52:19 +0000
@@ -11,6 +11,9 @@
 
 from twisted.internet.threads import deferToThread
 
+CLIENT_SESSION_TIMEOUT = 30000
+TICK_TIME = int(CLIENT_SESSION_TIMEOUT / 2)
+MAX_SESSION_TIMEOUT = CLIENT_SESSION_TIMEOUT * 2
 
 zookeeper_script_template = """\
 #!/bin/bash
@@ -39,6 +42,8 @@
 dataDir=%s
 clientPort=%s
 maxClientCnxns=500
+minSessionTimeout=%d
+maxSessionTimeout=%d
 """
 
 
@@ -51,7 +56,10 @@
 
     def __init__(self, run_dir, port=None, host=None,
                  zk_location="system", user=None, group=None,
-                 use_deferred=True, fsync=True):
+                 use_deferred=True,
+                 min_session_timeout=CLIENT_SESSION_TIMEOUT,
+                 max_session_timeout=MAX_SESSION_TIMEOUT,
+                 fsync=True):
         """
         :param run_dir: Directory to store all zookeeper instance related data.
         :param port: The port zookeeper should run on.
@@ -72,6 +80,8 @@
         self._user = user
         self._group = group
         self._zk_location = zk_location
+        self._min_session_time = min_session_timeout
+        self._max_session_time = max_session_timeout
         self._use_deferred = use_deferred
         self.fsync = fsync
 
@@ -198,9 +208,12 @@
             config.write(log4j_properties)
 
         with open(zk_vars["config_path"], "w") as config:
-            conf = zookeeper_conf_template % (zk_vars["data_dir"], self._port)
-            if self.fsync:
-                conf += "\nfsync=no\n"
+            conf = zookeeper_conf_template % (
+                zk_vars["data_dir"], self._port,
+                self._min_session_time,
+                self._max_session_time)
+            if not self.fsync:
+                conf += "\nforceSync=no\n"
             config.write(conf)
             if self._host:
                 config.write("\nclientPortAddress=%s" % self._host)

=== modified file 'juju/providers/common/cloudinit.py'
--- juju/providers/common/cloudinit.py	2013-01-22 15:29:50 +0000
+++ juju/providers/common/cloudinit.py	2013-02-01 17:52:19 +0000
@@ -5,6 +5,8 @@
 from juju.errors import CloudInitError
 from juju.lib.upstart import UpstartService
 from juju.lib import serializer
+from juju.lib.zk import (
+    CLIENT_SESSION_TIMEOUT, TICK_TIME, MAX_SESSION_TIMEOUT)
 from juju.providers.common.utils import format_cloud_init
 from juju.state.auth import make_identity
 
@@ -38,6 +40,12 @@
 
 def _zookeeper_scripts(instance_id, secret, constraints, provider_type):
     return [
+        'sed -i -e s/tickTime=2000/tickTime=%d/g /etc/zookeeper/conf/zoo.cfg' % (
+            TICK_TIME),
+        'echo "minSessionTimeout=%d" >> /etc/zookeeper/conf/zoo.cfg' % (
+            CLIENT_SESSION_TIMEOUT),
+        'echo "maxSessionTimeout=%d" >> /etc/zookeeper/conf/zoo.cfg' % (
+            MAX_SESSION_TIMEOUT),
         "juju-admin initialize"
         " --instance-id=%s"
         " --admin-identity=%s"
@@ -66,20 +74,20 @@
     service.set_description(
         "Juju unit agent for %s" % unit_name)
     service.set_environ(
-            {"JUJU_MACHINE_ID": str(machine_id),
-             "JUJU_UNIT_NAME": unit_name,
-             "JUJU_HOME": juju_home,
-             "JUJU_ENV_UUID": env_id,
-             "JUJU_ZOOKEEPER": zookeeper_hosts})
+        {"JUJU_MACHINE_ID": str(machine_id),
+         "JUJU_UNIT_NAME": unit_name,
+         "JUJU_HOME": juju_home,
+         "JUJU_ENV_UUID": env_id,
+         "JUJU_ZOOKEEPER": zookeeper_hosts})
     service.set_output_path(
         "/var/log/juju/unit-%s-output.log" % unit_path_name)
     service.set_command(" ".join(
-                ["/usr/bin/python",
-                "-m", "juju.agents.unit",
-                "--nodaemon",
-                "--logfile", "/var/log/juju/unit-%s.log" % unit_path_name,
-                "--session-file",
-                "/var/run/juju/unit-%s-agent.zksession" % unit_path_name]))
+        ["/usr/bin/python",
+         "-m", "juju.agents.unit",
+         "--nodaemon",
+         "--logfile", "/var/log/juju/unit-%s.log" % unit_path_name,
+         "--session-file",
+         "/var/run/juju/unit-%s-agent.zksession" % unit_path_name]))
     return service.get_cloud_init_commands()
 
 

=== modified file 'juju/providers/common/connect.py'
--- juju/providers/common/connect.py	2011-12-08 20:17:19 +0000
+++ juju/providers/common/connect.py	2013-02-01 17:52:19 +0000
@@ -6,6 +6,7 @@
 
 from juju.errors import EnvironmentNotFound, EnvironmentPending, NoConnection
 from juju.lib.twistutils import sleep
+from juju.lib.zk import CLIENT_SESSION_TIMEOUT
 from juju.state.sshclient import SSHClient
 
 from .utils import log
@@ -64,8 +65,9 @@
         chosen = random.choice(assigned)
         log.debug("Connecting to environment using %s...", chosen.dns_name)
         try:
-            client = yield SSHClient().connect(
-                chosen.dns_name + ":2181", timeout=30, share=share)
+            client = yield SSHClient(
+                session_timeout=CLIENT_SESSION_TIMEOUT).connect(
+                    chosen.dns_name + ":2181", timeout=30, share=share)
         except (NoConnection, ConnectionTimeoutException) as e:
             raise EnvironmentPending(
                 "Cannot connect to environment using %s "

=== modified file 'juju/providers/common/tests/data/cloud_init_bootstrap'
--- juju/providers/common/tests/data/cloud_init_bootstrap	2012-08-23 16:14:42 +0000
+++ juju/providers/common/tests/data/cloud_init_bootstrap	2013-02-01 17:52:19 +0000
@@ -6,7 +6,9 @@
 output: {all: '| tee -a /var/log/cloud-init-output.log'}
 packages: [bzr, byobu, tmux, python-setuptools, python-twisted, python-txaws, python-zookeeper,
   default-jre-headless, zookeeper, zookeeperd, juju]
-runcmd: [sudo mkdir -p /var/lib/juju, sudo mkdir -p /var/log/juju, 'juju-admin initialize
+runcmd: [sudo mkdir -p /var/lib/juju, sudo mkdir -p /var/log/juju, sed -i -e s/tickTime=2000/tickTime=15000/g
+    /etc/zookeeper/conf/zoo.cfg, echo "minSessionTimeout=30000" >> /etc/zookeeper/conf/zoo.cfg,
+  echo "maxSessionTimeout=60000" >> /etc/zookeeper/conf/zoo.cfg, 'juju-admin initialize
     --instance-id=token --admin-identity=admin:19vlzY4Vc3q4Ew5OsCwKYqrq1HI= --constraints-data=e2NwdTogJzIwJywgcHJvdmlkZXItdHlwZTogZHVtbXksIHVidW50dS1zZXJpZXM6IGFzdG9uaXNoaW5nfQo=
     --provider-type=dummy', 'cat >> /etc/init/juju-machine-agent.conf <<EOF
 
@@ -33,8 +35,7 @@
 
     EOF
 
-    ', /sbin/start juju-machine-agent, 'cat >> /etc/init/juju-provision-agent.conf
-    <<EOF
+', /sbin/start juju-machine-agent, 'cat >> /etc/init/juju-provision-agent.conf <<EOF
 
     description "Juju provisioning agent"
 
@@ -57,5 +58,5 @@
 
     EOF
 
-    ', /sbin/start juju-provision-agent]
+', /sbin/start juju-provision-agent]
 ssh_authorized_keys: [chubb]

=== modified file 'juju/providers/common/tests/data/cloud_init_bootstrap_testing'
--- juju/providers/common/tests/data/cloud_init_bootstrap_testing	2013-01-22 15:29:50 +0000
+++ juju/providers/common/tests/data/cloud_init_bootstrap_testing	2013-02-01 17:52:19 +0000
@@ -6,7 +6,9 @@
 output: {all: '| tee -a /var/log/cloud-init-output.log'}
 packages: [bzr, byobu, tmux, python-setuptools, python-twisted, python-txaws, python-zookeeper,
   default-jre-headless, zookeeper, zookeeperd, juju]
-runcmd: [sudo mkdir -p /var/lib/juju, sudo mkdir -p /var/log/juju, 'juju-admin initialize
+runcmd: [sudo mkdir -p /var/lib/juju, sudo mkdir -p /var/log/juju, sed -i -e s/tickTime=2000/tickTime=15000/g
+    /etc/zookeeper/conf/zoo.cfg, echo "minSessionTimeout=30000" >> /etc/zookeeper/conf/zoo.cfg,
+  echo "maxSessionTimeout=60000" >> /etc/zookeeper/conf/zoo.cfg, 'juju-admin initialize
     --instance-id=token --admin-identity=admin:19vlzY4Vc3q4Ew5OsCwKYqrq1HI= --constraints-data=e2NwdTogJzIwJywgcHJvdmlkZXItdHlwZTogZHVtbXksIHVidW50dS1zZXJpZXM6IGFzdG9uaXNoaW5nfQo=
     --provider-type=dummy', 'cat >> /etc/init/juju-machine-agent.conf <<EOF
 

=== modified file 'juju/providers/common/tests/data/cloud_init_bootstrap_zookeepers'
--- juju/providers/common/tests/data/cloud_init_bootstrap_zookeepers	2012-08-23 16:14:42 +0000
+++ juju/providers/common/tests/data/cloud_init_bootstrap_zookeepers	2013-02-01 17:52:19 +0000
@@ -6,7 +6,9 @@
 output: {all: '| tee -a /var/log/cloud-init-output.log'}
 packages: [bzr, byobu, tmux, python-setuptools, python-twisted, python-txaws, python-zookeeper,
   default-jre-headless, zookeeper, zookeeperd, juju]
-runcmd: [sudo mkdir -p /var/lib/juju, sudo mkdir -p /var/log/juju, 'juju-admin initialize
+runcmd: [sudo mkdir -p /var/lib/juju, sudo mkdir -p /var/log/juju, sed -i -e s/tickTime=2000/tickTime=15000/g
+    /etc/zookeeper/conf/zoo.cfg, echo "minSessionTimeout=30000" >> /etc/zookeeper/conf/zoo.cfg,
+  echo "maxSessionTimeout=60000" >> /etc/zookeeper/conf/zoo.cfg, 'juju-admin initialize
     --instance-id=token --admin-identity=admin:19vlzY4Vc3q4Ew5OsCwKYqrq1HI= --constraints-data=e2NwdTogJzIwJywgcHJvdmlkZXItdHlwZTogZHVtbXksIHVidW50dS1zZXJpZXM6IGFzdG9uaXNoaW5nfQo=
     --provider-type=dummy', 'cat >> /etc/init/juju-machine-agent.conf <<EOF
 
@@ -33,8 +35,7 @@
 
     EOF
 
-    ', /sbin/start juju-machine-agent, 'cat >> /etc/init/juju-provision-agent.conf
-    <<EOF
+', /sbin/start juju-machine-agent, 'cat >> /etc/init/juju-provision-agent.conf <<EOF
 
     description "Juju provisioning agent"
 
@@ -57,5 +58,5 @@
 
     EOF
 
-    ', /sbin/start juju-provision-agent]
+', /sbin/start juju-provision-agent]
 ssh_authorized_keys: [chubb]

=== modified file 'juju/providers/local/__init__.py'
--- juju/providers/local/__init__.py	2012-09-28 06:32:46 +0000
+++ juju/providers/local/__init__.py	2013-02-01 17:52:19 +0000
@@ -8,6 +8,7 @@
 from juju.errors import ProviderError, EnvironmentNotFound
 from juju.lib.lxc import LXCContainer, get_containers
 from juju.lib.zk import Zookeeper
+from juju.lib.port import get_open_port
 from juju.providers.common.base import MachineProviderBase
 from juju.providers.common.connect import ZookeeperConnect
 from juju.providers.common.utils import get_user_authorized_keys
@@ -20,7 +21,6 @@
 from juju.state.auth import make_identity
 from juju.state.initialize import StateHierarchy
 from juju.state.placement import LOCAL_POLICY
-from juju.state.utils import get_open_port
 
 
 log = logging.getLogger("juju.local-dev")

=== modified file 'juju/providers/local/tests/test_files.py'
--- juju/providers/local/tests/test_files.py	2012-09-28 06:13:47 +0000
+++ juju/providers/local/tests/test_files.py	2013-02-01 17:52:19 +0000
@@ -1,7 +1,6 @@
 import os
 import signal
-from StringIO  import StringIO
-import subprocess
+from StringIO import StringIO
 
 from twisted.internet.defer import inlineCallbacks, succeed
 from twisted.web.client import getPage
@@ -12,9 +11,9 @@
 from juju.lib.testing import TestCase
 from juju.lib.service import TwistedDaemonService
 from juju.lib.mocker import ANY
+from juju.lib.port import get_open_port
 from juju.providers.local.files import (
     LocalStorage, StorageServer, SERVER_URL_KEY)
-from juju.state.utils import get_open_port
 
 
 class WebFileStorageTest(TestCase):

=== modified file 'juju/state/sshclient.py'
--- juju/state/sshclient.py	2011-12-07 21:13:52 +0000
+++ juju/state/sshclient.py	2013-02-01 17:52:19 +0000
@@ -5,11 +5,14 @@
 from twisted.internet.defer import Deferred, inlineCallbacks, returnValue
 from txzookeeper.client import ConnectionTimeoutException
 
+
 from juju.errors import NoConnection, InvalidHost, InvalidUser
+from juju.lib.port import get_open_port
 from juju.state.security import SecurityPolicyConnection
 from juju.state.sshforward import forward_port, ClientTunnelProtocol
 
-from .utils import PortWatcher, get_open_port
+from .utils import PortWatcher
+
 
 SERVER_RE = re.compile("^(\S+):(\d+)$")
 

=== modified file 'juju/state/tests/test_utils.py'
--- juju/state/tests/test_utils.py	2012-09-10 03:20:20 +0000
+++ juju/state/tests/test_utils.py	2013-02-01 17:52:19 +0000
@@ -14,7 +14,7 @@
 from juju.state.base import StateBase
 from juju.state.errors import StateChanged, StateNotFound
 from juju.state.utils import (
-    PortWatcher, remove_tree, dict_merge, get_open_port,
+    PortWatcher, remove_tree, dict_merge,
     YAMLState, YAMLStateNodeMixin, AddedItem, ModifiedItem, DeletedItem)
 
 
@@ -227,35 +227,6 @@
             dict(b=2, c=1))
 
 
-class OpenPortTest(TestCase):
-
-    def test_get_open_port(self):
-        port = get_open_port()
-        self.assertTrue(isinstance(port, int))
-
-        sock = socket.socket(
-            socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP)
-
-        # would raise an error if we got it wrong.
-        sock.bind(("127.0.0.1", port))
-        sock.listen(1)
-        sock.close()
-        del sock
-
-    def test_get_open_port_with_host(self):
-        port = get_open_port("localhost")
-        self.assertTrue(isinstance(port, int))
-
-        sock = socket.socket(
-            socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP)
-
-        # would raise an error if we got it wrong.
-        sock.bind(("127.0.0.1", port))
-        sock.listen(1)
-        sock.close()
-        del sock
-
-
 class ChangeItemsTest(TestCase):
     """Tests the formatting of change items.
 

=== modified file 'juju/state/utils.py'
--- juju/state/utils.py	2012-09-10 03:20:20 +0000
+++ juju/state/utils.py	2013-02-01 17:52:19 +0000
@@ -84,18 +84,6 @@
     yield client.delete(path)
 
 
-def get_open_port(host=""):
-    """Get an open port on the machine.
-    """
-    temp_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
-    temp_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-    temp_sock.bind((host, 0))
-    port = temp_sock.getsockname()[1]
-    temp_sock.close()
-    del temp_sock
-    return port
-
-
 def dict_merge(d1, d2):
     """Return a union of dicts if they have no conflicting values.
 

=== modified file 'test'
--- test	2012-07-19 17:39:35 +0000
+++ test	2013-02-01 17:52:19 +0000
@@ -42,10 +42,9 @@
             print ("JUJU_TEST_TIMEOUT must be a number")
             exit()
 
-
     with zookeeper_test_context(
             os.environ["ZOOKEEPER_PATH"],
-            os.environ.get("ZOOKEEPER_TEST_PORT", 28181)):
+            os.environ.get("ZOOKEEPER_TEST_PORT", 28181)) as zk:
         run()
 
 if __name__ == "__main__":