Merge lp:~hazmat/pyjuju/managed-zk-client into lp:pyjuju

[Kapil Thangavelu]

managed zk connections for transient and session failures.

This branch switches agents to use managed session clients
which automatically retry in the face of transient disconnections, and
automatically restablish session on expiration (with watch
and ephemeral node handling).

https://codereview.appspot.com/5989050/

[Kapil Thangavelu (hazmat)]

Reviewers: mp+101132_code.launchpad.net,

Message:
Please take a look.

Description:
managed zk connections for transient and session failures.

This branch switches agents to use managed session clients
which automatically retry in the face of transient disconnections, and
automatically restablish session on expiration (with watch
and ephemeral node handling).

https://code.launchpad.net/~hazmat/juju/managed-zk-client/+merge/101132

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M juju/agents/base.py
   M juju/charm/tests/test_publisher.py
   M juju/lib/testing.py
   M juju/providers/dummy.py
   M juju/providers/local/__init__.py
   M juju/state/tests/common.py
   M juju/state/tests/test_base.py
   M juju/state/tests/test_initialize.py
   M juju/state/tests/test_relation.py
   M juju/state/tests/test_utils.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision:
<email address hidden>

Index: juju/agents/base.py
=== modified file 'juju/agents/base.py'
--- juju/agents/base.py 2011-11-30 09:22:38 +0000
+++ juju/agents/base.py 2012-04-04 04:04:30 +0000
@@ -13,6 +13,7 @@
  from twisted.python.log import PythonLoggingObserver

  from txzookeeper import ZookeeperClient
+from txzookeeper.managed import ManagedClient

  from juju.control.options import setup_twistd_options
  from juju.errors import NoConnection, JujuError
@@ -202,7 +203,7 @@
      def connect(self):
          """Return an authenticated connection to the juju zookeeper."""
          yield self._kill_existing_session()
- self.client = yield ZookeeperClient().connect(
+ self.client = yield ManagedClient().connect(
              self.config["zookeeper_servers"])
          save_client_id(
              self.config["session_file"], self.client.client_id)

Index: juju/lib/testing.py
=== modified file 'juju/lib/testing.py'
--- juju/lib/testing.py 2012-03-29 22:34:58 +0000
+++ juju/lib/testing.py 2012-04-06 14:42:06 +0000
@@ -10,6 +10,7 @@
  from twisted.trial.unittest import TestCase as TrialTestCase

  from txzookeeper import ZookeeperClient
+from txzookeeper.managed import ManagedClient

  from juju.lib.mocker import MockerTestCase
  from juju.tests.common import get_test_zookeeper_address
@@ -154,7 +155,7 @@
          returnValue(True)

      def get_zookeeper_client(self):
- client = ZookeeperClient(
+ client = ManagedClient(
              get_test_zookeeper_address(), session_timeout=1000)
          return client

Index: juju/providers/dummy.py
=== modified file 'juju/providers/dummy.py'
--- juju/providers/dummy.py 2012-03-29 01:37:57 +0000
+++ juju/providers/dummy.py 2012-04-06 16:35:31 +0000
@@ -4,7 +4,8 @@

  from twisted.internet.defer import inlineCallbacks, returnValue, succeed,
fail

-from txzookeeper import ZookeeperClient
+#from txzookeeper import ZookeeperClient
+from txzookeeper....

[William Reade (fwereade)]

LGTM

I retain some misgivings about making this change at this late stage,
but this change and the txzookeeper one both appear to be entirely sane.
I think we're good :).

https://codereview.appspot.com/5989050/

=== modified file 'juju/agents/base.py'
--- juju/agents/base.py	2011-11-30 09:22:38 +0000
+++ juju/agents/base.py	2012-04-06 16:39:29 +0000
@@ -13,6 +13,7 @@
 from twisted.python.log import PythonLoggingObserver
 
 from txzookeeper import ZookeeperClient
+from txzookeeper.managed import ManagedClient
 
 from juju.control.options import setup_twistd_options
 from juju.errors import NoConnection, JujuError
@@ -202,7 +203,7 @@
     def connect(self):
         """Return an authenticated connection to the juju zookeeper."""
         yield self._kill_existing_session()
-        self.client = yield ZookeeperClient().connect(
+        self.client = yield ManagedClient().connect(
             self.config["zookeeper_servers"])
         save_client_id(
             self.config["session_file"], self.client.client_id)

=== modified file 'juju/charm/tests/test_publisher.py'
--- juju/charm/tests/test_publisher.py	2012-02-16 18:27:06 +0000
+++ juju/charm/tests/test_publisher.py	2012-04-06 16:39:29 +0000
@@ -6,7 +6,6 @@
 from twisted.internet.defer import inlineCallbacks, fail
 from twisted.python.failure import Failure
 
-from txzookeeper import ZookeeperClient
 from txzookeeper.tests.utils import deleteTree
 
 from juju.charm.bundle import CharmBundle
@@ -17,7 +16,6 @@
 from juju.providers.dummy import FileStorage
 from juju.state.charm import CharmStateManager
 from juju.state.errors import StateChanged
-from juju.tests.common import get_test_zookeeper_address
 
 from juju.environment.tests.test_config import (
     EnvironmentsConfigTestBase, SAMPLE_ENV)
@@ -54,7 +52,7 @@
         self.charm_storage_key = under.quote(
             "%s:%s" % (self.charm_id, self.charm.get_sha256()))
 
-        self.client = ZookeeperClient(get_test_zookeeper_address())
+        self.client = self.get_zookeeper_client()
         self.storage_dir = self.makeDir()
         self.storage = FileStorage(self.storage_dir)
         self.publisher = CharmPublisher(self.client, self.storage)

=== modified file 'juju/lib/testing.py'
--- juju/lib/testing.py	2012-03-29 22:34:58 +0000
+++ juju/lib/testing.py	2012-04-06 16:39:29 +0000
@@ -10,6 +10,7 @@
 from twisted.trial.unittest import TestCase as TrialTestCase
 
 from txzookeeper import ZookeeperClient
+from txzookeeper.managed import ManagedClient
 
 from juju.lib.mocker import MockerTestCase
 from juju.tests.common import get_test_zookeeper_address
@@ -154,7 +155,7 @@
         returnValue(True)
 
     def get_zookeeper_client(self):
-        client = ZookeeperClient(
+        client = ManagedClient(
             get_test_zookeeper_address(), session_timeout=1000)
         return client
 

=== modified file 'juju/providers/dummy.py'
--- juju/providers/dummy.py	2012-03-29 01:37:57 +0000
+++ juju/providers/dummy.py	2012-04-06 16:39:29 +0000
@@ -4,7 +4,8 @@
 
 from twisted.internet.defer import inlineCallbacks, returnValue, succeed, fail
 
-from txzookeeper import ZookeeperClient
+#from txzookeeper import ZookeeperClient
+from txzookeeper.managed import ManagedClient
 
 from juju.errors import (
     EnvironmentNotFound, MachinesNotFound, ProviderError)
@@ -61,7 +62,7 @@
             attempting to connect to the same provider, if that's feasible.
             Unused for the dummy provider.
         """
-        return ZookeeperClient(
+        return ManagedClient(
             os.environ.get("ZOOKEEPER_ADDRESS", "127.0.0.1:2181"),
             session_timeout=1000).connect()
 

=== modified file 'juju/providers/local/__init__.py'
--- juju/providers/local/__init__.py	2012-03-29 01:37:57 +0000
+++ juju/providers/local/__init__.py	2012-04-06 16:39:29 +0000
@@ -129,6 +129,7 @@
         hierarchy = StateHierarchy(
             client, admin_identity, "local", constraints.data, "local")
         yield hierarchy.initialize()
+        yield client.close()
 
         # Startup the machine agent
         log_file = os.path.join(self._directory, "machine-agent.log")

=== modified file 'juju/state/tests/common.py'
--- juju/state/tests/common.py	2012-03-30 10:13:09 +0000
+++ juju/state/tests/common.py	2012-04-06 16:39:29 +0000
@@ -2,14 +2,12 @@
 
 import zookeeper
 
-from txzookeeper import ZookeeperClient
 from txzookeeper.tests.utils import deleteTree
 
 from juju.charm.directory import CharmDirectory
 from juju.charm.tests.test_directory import sample_directory
 from juju.environment.tests.test_config import EnvironmentsConfigTestBase
 from juju.state.topology import InternalTopology
-from juju.tests.common import get_test_zookeeper_address
 
 
 class StateTestBase(EnvironmentsConfigTestBase):
@@ -33,7 +31,7 @@
         # Close and reopen connection, so that watches set during
         # testing are not affected by the cleaning up.
         self.client.close()
-        client = ZookeeperClient(get_test_zookeeper_address())
+        client = self.get_zookeeper_client()
         yield client.connect()
         deleteTree(handle=client.handle)
         client.close()

=== modified file 'juju/state/tests/test_base.py'
--- juju/state/tests/test_base.py	2011-09-15 18:50:23 +0000
+++ juju/state/tests/test_base.py	2012-04-06 16:39:29 +0000
@@ -1,12 +1,10 @@
 from twisted.internet.defer import inlineCallbacks, Deferred
-from txzookeeper import ZookeeperClient
 
 from juju.state.tests.common import StateTestBase
 
 from juju.state.base import StateBase
 from juju.state.errors import StopWatcher
 from juju.state.topology import InternalTopology
-from juju.tests.common import get_test_zookeeper_address
 
 
 class StateBaseTest(StateTestBase):
@@ -339,7 +337,7 @@
 
         # Use a separate client connection for watching so it can be
         # disconnected.
-        watch_client = ZookeeperClient(get_test_zookeeper_address())
+        watch_client = self.get_zookeeper_client()
         yield watch_client.connect()
         watch_base = StateBase(watch_client)
 
@@ -402,7 +400,7 @@
         """
         # Use a separate client connection for watching so it can be
         # disconnected.
-        watch_client = ZookeeperClient(get_test_zookeeper_address())
+        watch_client = self.get_zookeeper_client()
         yield watch_client.connect()
         watch_base = StateBase(watch_client)
 
@@ -416,7 +414,7 @@
         topology = InternalTopology()
         topology.add_machine("m-0")
         yield self.set_topology(topology)
-        
+
         # Now disconnect the client.
         watch_client.close()
         self.assertFalse(watch_client.connected)

=== modified file 'juju/state/tests/test_initialize.py'
--- juju/state/tests/test_initialize.py	2012-03-29 01:37:57 +0000
+++ juju/state/tests/test_initialize.py	2012-04-06 16:39:29 +0000
@@ -1,7 +1,6 @@
 import zookeeper
 
 from twisted.internet.defer import inlineCallbacks
-from txzookeeper import ZookeeperClient
 from txzookeeper.tests.utils import deleteTree
 
 from juju.environment.tests.test_config import EnvironmentsConfigTestBase
@@ -10,7 +9,6 @@
     GlobalSettingsStateManager, EnvironmentStateManager)
 from juju.state.initialize import StateHierarchy
 from juju.state.machine import MachineStateManager
-from juju.tests.common import get_test_zookeeper_address
 
 
 class LayoutTest(EnvironmentsConfigTestBase):
@@ -20,7 +18,7 @@
         yield super(LayoutTest, self).setUp()
         self.log = self.capture_logging("juju.state.init")
         zookeeper.set_debug_level(0)
-        self.client = ZookeeperClient(get_test_zookeeper_address())
+        self.client = self.get_zookeeper_client()
         self.identity = make_identity("admin:genie")
         constraints_data = {
             "arch": "arm",

=== modified file 'juju/state/tests/test_relation.py'
--- juju/state/tests/test_relation.py	2012-04-06 11:19:31 +0000
+++ juju/state/tests/test_relation.py	2012-04-06 16:39:29 +0000
@@ -8,7 +8,6 @@
 
 from twisted.internet.defer import (
     inlineCallbacks, returnValue, Deferred, fail, succeed)
-from txzookeeper import ZookeeperClient
 
 from juju.charm.directory import CharmDirectory
 from juju.charm.tests import local_charm_id
@@ -854,7 +853,7 @@
 
         # manually construct a unit relation state using a separate
         # connection.
-        client2 = ZookeeperClient(self.client.servers)
+        client2 = self.get_zookeeper_client()
         yield client2.connect()
         service_relation = ServiceRelationState(
             client2,

=== modified file 'juju/state/tests/test_utils.py'
--- juju/state/tests/test_utils.py	2012-02-01 15:39:08 +0000
+++ juju/state/tests/test_utils.py	2012-04-06 16:39:29 +0000
@@ -8,7 +8,6 @@
 from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks
 from twisted.internet.threads import deferToThread
-from txzookeeper import ZookeeperClient
 import yaml
 
 from juju.lib.testing import TestCase
@@ -18,8 +17,6 @@
     PortWatcher, remove_tree, dict_merge, get_open_port,
     YAMLState, YAMLStateNodeMixin, AddedItem, ModifiedItem, DeletedItem)
 
-from juju.tests.common import get_test_zookeeper_address
-
 
 class PortWait(TestCase):
 
@@ -193,7 +190,7 @@
     def setUp(self):
         yield super(RemoveTreeTest, self).setUp()
         zookeeper.set_debug_level(0)
-        self.client = ZookeeperClient(get_test_zookeeper_address())
+        self.client = self.get_zookeeper_client()
         yield self.client.connect()
 
     @inlineCallbacks
@@ -309,7 +306,7 @@
     @inlineCallbacks
     def setUp(self):
         zookeeper.set_debug_level(0)
-        self.client = ZookeeperClient(get_test_zookeeper_address())
+        self.client = self.get_zookeeper_client()
         yield self.client.connect()
         self.path = "/zoo"
 
@@ -653,7 +650,7 @@
     @inlineCallbacks
     def setUp(self):
         zookeeper.set_debug_level(0)
-        self.client = ZookeeperClient(get_test_zookeeper_address())
+        self.client = self.get_zookeeper_client()
         yield self.client.connect()
         self.path = "/path"