New N2VC interface + updated libjuju

This commit introduces the Python3 N2VC module, which acts as a standard
interface to the VCA.

The goal of this is to provide a common way for modules to interface
with the VCA.

- Updated libjuju from 0.6.1 to 0.7.3

Signed-off-by: Adam Israel <adam.israel@canonical.com>
Change-Id: Ide70fb5ae5797eb6486de24653dc09a23f9c009e
diff --git a/modules/libjuju/juju/client/_client.py b/modules/libjuju/juju/client/_client.py
index 2ef0ffd..d959a56 100644
--- a/modules/libjuju/juju/client/_client.py
+++ b/modules/libjuju/juju/client/_client.py
@@ -1,10 +1,8 @@
 # DO NOT CHANGE THIS FILE! This file is auto-generated by facade.py.
 # Changes will be overwritten/lost when the file is regenerated.
 
-from juju.client._definitions import *  # noqa
-
 from juju.client import _client1, _client2, _client3, _client4, _client5
-
+from juju.client._definitions import *  # noqa
 
 CLIENTS = {
     "1": _client1,
@@ -43,7 +41,13 @@
         @param connection: initialized Connection object.
 
         """
-        version = connection.facades[cls.__name__[:-6]]
+        facade_name = cls.__name__
+        if not facade_name.endswith('Facade'):
+           raise TypeError('Unexpected class name: {}'.format(facade_name))
+        facade_name = facade_name[:-len('Facade')]
+        version = connection.facades.get(facade_name)
+        if version is None:
+            raise Exception('No facade {} in facades {}'.format(facade_name, connection.facades))
 
         c = lookup_facade(cls.__name__, version)
         c = c()
diff --git a/modules/libjuju/juju/client/_client1.py b/modules/libjuju/juju/client/_client1.py
index 3774056..e161973 100644
--- a/modules/libjuju/juju/client/_client1.py
+++ b/modules/libjuju/juju/client/_client1.py
@@ -1,8 +1,8 @@
 # DO NOT CHANGE THIS FILE! This file is auto-generated by facade.py.
 # Changes will be overwritten/lost when the file is regenerated.
 
-from juju.client.facade import Type, ReturnMapping
 from juju.client._definitions import *
+from juju.client.facade import ReturnMapping, Type
 
 
 class AgentToolsFacade(Type):
@@ -6641,5 +6641,3 @@
         _params['include-disabled'] = include_disabled
         reply = await self.rpc(msg)
         return reply
-
-
diff --git a/modules/libjuju/juju/client/_client2.py b/modules/libjuju/juju/client/_client2.py
index 283e803..6f92a86 100644
--- a/modules/libjuju/juju/client/_client2.py
+++ b/modules/libjuju/juju/client/_client2.py
@@ -1,8 +1,8 @@
 # DO NOT CHANGE THIS FILE! This file is auto-generated by facade.py.
 # Changes will be overwritten/lost when the file is regenerated.
 
-from juju.client.facade import Type, ReturnMapping
 from juju.client._definitions import *
+from juju.client.facade import ReturnMapping, Type
 
 
 class ActionFacade(Type):
@@ -4223,5 +4223,3 @@
 
         reply = await self.rpc(msg)
         return reply
-
-
diff --git a/modules/libjuju/juju/client/_client3.py b/modules/libjuju/juju/client/_client3.py
index 3f9ef55..b5f4b9d 100644
--- a/modules/libjuju/juju/client/_client3.py
+++ b/modules/libjuju/juju/client/_client3.py
@@ -1,8 +1,8 @@
 # DO NOT CHANGE THIS FILE! This file is auto-generated by facade.py.
 # Changes will be overwritten/lost when the file is regenerated.
 
-from juju.client.facade import Type, ReturnMapping
 from juju.client._definitions import *
+from juju.client.facade import ReturnMapping, Type
 
 
 class ApplicationFacade(Type):
@@ -5216,5 +5216,3 @@
         _params['entities'] = entities
         reply = await self.rpc(msg)
         return reply
-
-
diff --git a/modules/libjuju/juju/client/_client4.py b/modules/libjuju/juju/client/_client4.py
index 68ee3f9..9c47561 100644
--- a/modules/libjuju/juju/client/_client4.py
+++ b/modules/libjuju/juju/client/_client4.py
@@ -1,8 +1,8 @@
 # DO NOT CHANGE THIS FILE! This file is auto-generated by facade.py.
 # Changes will be overwritten/lost when the file is regenerated.
 
-from juju.client.facade import Type, ReturnMapping
 from juju.client._definitions import *
+from juju.client.facade import ReturnMapping, Type
 
 
 class ApplicationFacade(Type):
@@ -2667,5 +2667,3 @@
         _params['entities'] = entities
         reply = await self.rpc(msg)
         return reply
-
-
diff --git a/modules/libjuju/juju/client/_client5.py b/modules/libjuju/juju/client/_client5.py
index 22805ed..f0f1282 100644
--- a/modules/libjuju/juju/client/_client5.py
+++ b/modules/libjuju/juju/client/_client5.py
@@ -1,8 +1,8 @@
 # DO NOT CHANGE THIS FILE! This file is auto-generated by facade.py.
 # Changes will be overwritten/lost when the file is regenerated.
 
-from juju.client.facade import Type, ReturnMapping
 from juju.client._definitions import *
+from juju.client.facade import ReturnMapping, Type
 
 
 class ApplicationFacade(Type):
@@ -2650,5 +2650,3 @@
         _params['entities'] = entities
         reply = await self.rpc(msg)
         return reply
-
-
diff --git a/modules/libjuju/juju/client/_definitions.py b/modules/libjuju/juju/client/_definitions.py
index 198784d..fde035f 100644
--- a/modules/libjuju/juju/client/_definitions.py
+++ b/modules/libjuju/juju/client/_definitions.py
@@ -1,7 +1,7 @@
 # DO NOT CHANGE THIS FILE! This file is auto-generated by facade.py.
 # Changes will be overwritten/lost when the file is regenerated.
 
-from juju.client.facade import Type, ReturnMapping
+from juju.client.facade import ReturnMapping, Type
 
 
 class APIHostPortsResult(Type):
@@ -8018,5 +8018,3 @@
         results : typing.Sequence<+T_co>[~ZoneResult]<~ZoneResult>
         '''
         self.results = [ZoneResult.from_json(o) for o in results or []]
-
-
diff --git a/modules/libjuju/juju/client/client.py b/modules/libjuju/juju/client/client.py
index 2f3e49d..2721d07 100644
--- a/modules/libjuju/juju/client/client.py
+++ b/modules/libjuju/juju/client/client.py
@@ -1,8 +1,7 @@
 '''Replace auto-generated classes with our own, where necessary.
 '''
 
-from . import _client, _definitions, overrides
-
+from . import _client, _definitions, overrides  # isort:skip
 
 for o in overrides.__all__:
     if "Facade" not in o:
@@ -31,4 +30,4 @@
             if not a.startswith('_'):
                 setattr(c_type, a, getattr(o_type, a))
 
-from ._client import *  # noqa
+from ._client import *  # noqa, isort:skip
diff --git a/modules/libjuju/juju/client/connection.py b/modules/libjuju/juju/client/connection.py
index c09468c..bdd1c3f 100644
--- a/modules/libjuju/juju/client/connection.py
+++ b/modules/libjuju/juju/client/connection.py
@@ -1,28 +1,21 @@
+import asyncio
 import base64
-import io
 import json
 import logging
-import os
-import random
-import shlex
 import ssl
-import string
-import subprocess
+import urllib.request
 import weakref
-import websockets
 from concurrent.futures import CancelledError
 from http.client import HTTPSConnection
-from pathlib import Path
 
-import asyncio
-import yaml
-
-from juju import tag, utils
+import macaroonbakery.httpbakery as httpbakery
+import macaroonbakery.bakery as bakery
+import websockets
+from juju import errors, tag, utils
 from juju.client import client
-from juju.errors import JujuError, JujuAPIError, JujuConnectionError
 from juju.utils import IdQueue
 
-log = logging.getLogger("websocket")
+log = logging.getLogger('juju.client.connection')
 
 
 class Monitor:
@@ -30,7 +23,7 @@
     Monitor helper class for our Connection class.
 
     Contains a reference to an instantiated Connection, along with a
-    reference to the Connection.receiver Future. Upon inspecttion of
+    reference to the Connection.receiver Future. Upon inspection of
     these objects, this class determines whether the connection is in
     an 'error', 'connected' or 'disconnected' state.
 
@@ -48,10 +41,6 @@
         self.connection = weakref.ref(connection)
         self.reconnecting = asyncio.Lock(loop=connection.loop)
         self.close_called = asyncio.Event(loop=connection.loop)
-        self.receiver_stopped = asyncio.Event(loop=connection.loop)
-        self.pinger_stopped = asyncio.Event(loop=connection.loop)
-        self.receiver_stopped.set()
-        self.pinger_stopped.set()
 
     @property
     def status(self):
@@ -81,7 +70,8 @@
             return self.DISCONNECTING
 
         # connection closed uncleanly (we didn't call connection.close)
-        if self.receiver_stopped.is_set() or not connection.ws.open:
+        stopped = connection._receiver_task.stopped.is_set()
+        if stopped or not connection.ws.open:
             return self.ERROR
 
         # everything is fine!
@@ -96,47 +86,91 @@
         client = await Connection.connect(
             api_endpoint, model_uuid, username, password, cacert)
 
-        # Connect using a controller/model name
-        client = await Connection.connect_model('local.local:default')
-
-        # Connect to the currently active model
-        client = await Connection.connect_current()
-
     Note: Any connection method or constructor can accept an optional `loop`
     argument to override the default event loop from `asyncio.get_event_loop`.
     """
 
-    DEFAULT_FRAME_SIZE = 'default_frame_size'
     MAX_FRAME_SIZE = 2**22
     "Maximum size for a single frame.  Defaults to 4MB."
 
-    def __init__(
-            self, endpoint, uuid, username, password, cacert=None,
-            macaroons=None, loop=None, max_frame_size=DEFAULT_FRAME_SIZE):
-        self.endpoint = endpoint
-        self._endpoint = endpoint
+    @classmethod
+    async def connect(
+            cls,
+            endpoint=None,
+            uuid=None,
+            username=None,
+            password=None,
+            cacert=None,
+            bakery_client=None,
+            loop=None,
+            max_frame_size=None,
+    ):
+        """Connect to the websocket.
+
+        If uuid is None, the connection will be to the controller. Otherwise it
+        will be to the model.
+        :param str endpoint The hostname:port of the controller to connect to.
+        :param str uuid The model UUID to connect to (None for a
+            controller-only connection).
+        :param str username The username for controller-local users (or None
+            to use macaroon-based login.)
+        :param str password The password for controller-local users.
+        :param str cacert The CA certificate of the controller (PEM formatted).
+        :param httpbakery.Client bakery_client The macaroon bakery client to
+            to use when performing macaroon-based login. Macaroon tokens
+            acquired when logging will be saved to bakery_client.cookies.
+            If this is None, a default bakery_client will be used.
+        :param loop asyncio.BaseEventLoop The event loop to use for async
+            operations.
+        :param max_frame_size The maximum websocket frame size to allow.
+        """
+        self = cls()
+        if endpoint is None:
+            raise ValueError('no endpoint provided')
         self.uuid = uuid
-        if macaroons:
-            self.macaroons = macaroons
-            self.username = ''
-            self.password = ''
-        else:
-            self.macaroons = []
-            self.username = username
-            self.password = password
-        self.cacert = cacert
-        self._cacert = cacert
+        if bakery_client is None:
+            bakery_client = httpbakery.Client()
+        self.bakery_client = bakery_client
+        if username and '@' in username and not username.endswith('@local'):
+            # We're trying to log in as an external user - we need to use
+            # macaroon authentication with no username or password.
+            if password is not None:
+                raise errors.JujuAuthError('cannot log in as external '
+                                           'user with a password')
+            username = None
+        self.usertag = tag.user(username)
+        self.password = password
         self.loop = loop or asyncio.get_event_loop()
 
         self.__request_id__ = 0
+
+        # The following instance variables are initialized by the
+        # _connect_with_redirect method, but create them here
+        # as a reminder that they will exist.
         self.addr = None
         self.ws = None
+        self.endpoint = None
+        self.cacert = None
+        self.info = None
+
+        # Create that _Task objects but don't start the tasks yet.
+        self._pinger_task = _Task(self._pinger, self.loop)
+        self._receiver_task = _Task(self._receiver, self.loop)
+
         self.facades = {}
         self.messages = IdQueue(loop=self.loop)
         self.monitor = Monitor(connection=self)
-        if max_frame_size is self.DEFAULT_FRAME_SIZE:
+        if max_frame_size is None:
             max_frame_size = self.MAX_FRAME_SIZE
         self.max_frame_size = max_frame_size
+        await self._connect_with_redirect([(endpoint, cacert)])
+        return self
+
+    @property
+    def username(self):
+        if not self.usertag:
+            return None
+        return self.usertag[len('user-'):]
 
     @property
     def is_open(self):
@@ -146,39 +180,34 @@
         return ssl.create_default_context(
             purpose=ssl.Purpose.CLIENT_AUTH, cadata=cert)
 
-    async def open(self):
+    async def _open(self, endpoint, cacert):
         if self.uuid:
-            url = "wss://{}/model/{}/api".format(self.endpoint, self.uuid)
+            url = "wss://{}/model/{}/api".format(endpoint, self.uuid)
         else:
-            url = "wss://{}/api".format(self.endpoint)
+            url = "wss://{}/api".format(endpoint)
 
-        kw = dict()
-        kw['ssl'] = self._get_ssl(self.cacert)
-        kw['loop'] = self.loop
-        kw['max_size'] = self.max_frame_size
-        self.addr = url
-        self.ws = await websockets.connect(url, **kw)
-        self.loop.create_task(self.receiver())
-        self.monitor.receiver_stopped.clear()
-        log.info("Driver connected to juju %s", url)
-        self.monitor.close_called.clear()
-        return self
+        return (await websockets.connect(
+            url,
+            ssl=self._get_ssl(cacert),
+            loop=self.loop,
+            max_size=self.max_frame_size,
+        ), url, endpoint, cacert)
 
     async def close(self):
         if not self.ws:
             return
         self.monitor.close_called.set()
-        await self.monitor.pinger_stopped.wait()
-        await self.monitor.receiver_stopped.wait()
+        await self._pinger_task.stopped.wait()
+        await self._receiver_task.stopped.wait()
         await self.ws.close()
         self.ws = None
 
-    async def recv(self, request_id):
+    async def _recv(self, request_id):
         if not self.is_open:
             raise websockets.exceptions.ConnectionClosed(0, 'websocket closed')
         return await self.messages.get(request_id)
 
-    async def receiver(self):
+    async def _receiver(self):
         try:
             while self.is_open:
                 result = await utils.run_with_interrupt(
@@ -205,10 +234,8 @@
             # make pending listeners aware of the error
             await self.messages.put_all(e)
             raise
-        finally:
-            self.monitor.receiver_stopped.set()
 
-    async def pinger(self):
+    async def _pinger(self):
         '''
         A Controller can time us out if we are silent for too long. This
         is especially true in JaaS, which has a fairly strict timeout.
@@ -232,11 +259,21 @@
                     loop=self.loop)
                 if self.monitor.close_called.is_set():
                     break
-        finally:
-            self.monitor.pinger_stopped.set()
-            return
+        except websockets.exceptions.ConnectionClosed:
+            # The connection has closed - we can't do anything
+            # more until the connection is restarted.
+            log.debug('ping failed because of closed connection')
+            pass
 
     async def rpc(self, msg, encoder=None):
+        '''Make an RPC to the API. The message is encoded as JSON
+        using the given encoder if any.
+        :param msg: Parameters for the call (will be encoded as JSON).
+        :param encoder: Encoder to be used when encoding the message.
+        :return: The result of the call.
+        :raises JujuAPIError: When there's an error returned.
+        :raises JujuError:
+        '''
         self.__request_id__ += 1
         msg['request-id'] = self.__request_id__
         if'params' not in msg:
@@ -244,7 +281,12 @@
         if "version" not in msg:
             msg['version'] = self.facades[msg['type']]
         outgoing = json.dumps(msg, indent=2, cls=encoder)
+        log.debug('connection {} -> {}'.format(id(self), outgoing))
         for attempt in range(3):
+            if self.monitor.status == Monitor.DISCONNECTED:
+                # closed cleanly; shouldn't try to reconnect
+                raise websockets.exceptions.ConnectionClosed(
+                    0, 'websocket closed')
             try:
                 await self.ws.send(outgoing)
                 break
@@ -257,14 +299,19 @@
                 # be cancelled when the pinger is cancelled by the reconnect,
                 # and we don't want the reconnect to be aborted halfway through
                 await asyncio.wait([self.reconnect()], loop=self.loop)
-        result = await self.recv(msg['request-id'])
+                if self.monitor.status != Monitor.CONNECTED:
+                    # reconnect failed; abort and shutdown
+                    log.error('RPC: Automatic reconnect failed')
+                    raise
+        result = await self._recv(msg['request-id'])
+        log.debug('connection {} <- {}'.format(id(self), result))
 
         if not result:
             return result
 
         if 'error' in result:
             # API Error Response
-            raise JujuAPIError(result)
+            raise errors.JujuAPIError(result)
 
         if 'response' not in result:
             # This may never happen
@@ -272,30 +319,34 @@
 
         if 'results' in result['response']:
             # Check for errors in a result list.
-            errors = []
+            # TODO This loses the results that might have succeeded.
+            # Perhaps JujuError should return all the results including
+            # errors, or perhaps a keyword parameter to the rpc method
+            # could be added to trigger this behaviour.
+            err_results = []
             for res in result['response']['results']:
                 if res.get('error', {}).get('message'):
-                    errors.append(res['error']['message'])
-            if errors:
-                raise JujuError(errors)
+                    err_results.append(res['error']['message'])
+            if err_results:
+                raise errors.JujuError(err_results)
 
         elif result['response'].get('error', {}).get('message'):
-            raise JujuError(result['response']['error']['message'])
+            raise errors.JujuError(result['response']['error']['message'])
 
         return result
 
-    def http_headers(self):
+    def _http_headers(self):
         """Return dictionary of http headers necessary for making an http
         connection to the endpoint of this Connection.
 
         :return: Dictionary of headers
 
         """
-        if not self.username:
+        if not self.usertag:
             return {}
 
         creds = u'{}:{}'.format(
-            tag.user(self.username),
+            self.usertag,
             self.password or ''
         )
         token = base64.b64encode(creds.encode())
@@ -328,70 +379,46 @@
             "/model/{}".format(self.uuid)
             if self.uuid else ""
         )
-        return conn, self.http_headers(), path
+        return conn, self._http_headers(), path
 
     async def clone(self):
         """Return a new Connection, connected to the same websocket endpoint
         as this one.
 
         """
-        return await Connection.connect(
-            self.endpoint,
-            self.uuid,
-            self.username,
-            self.password,
-            self.cacert,
-            self.macaroons,
-            self.loop,
-            self.max_frame_size,
-        )
+        return await Connection.connect(**self.connect_params())
+
+    def connect_params(self):
+        """Return a tuple of parameters suitable for passing to
+        Connection.connect that can be used to make a new connection
+        to the same controller (and model if specified. The first
+        element in the returned tuple holds the endpoint argument;
+        the other holds a dict of the keyword args.
+        """
+        return {
+            'endpoint': self.endpoint,
+            'uuid': self.uuid,
+            'username': self.username,
+            'password': self.password,
+            'cacert': self.cacert,
+            'bakery_client': self.bakery_client,
+            'loop': self.loop,
+            'max_frame_size': self.max_frame_size,
+        }
 
     async def controller(self):
         """Return a Connection to the controller at self.endpoint
-
         """
         return await Connection.connect(
             self.endpoint,
-            None,
-            self.username,
-            self.password,
-            self.cacert,
-            self.macaroons,
-            self.loop,
+            username=self.username,
+            password=self.password,
+            cacert=self.cacert,
+            bakery_client=self.bakery_client,
+            loop=self.loop,
+            max_frame_size=self.max_frame_size,
         )
 
-    async def _try_endpoint(self, endpoint, cacert):
-        success = False
-        result = None
-        new_endpoints = []
-
-        self.endpoint = endpoint
-        self.cacert = cacert
-        await self.open()
-        try:
-            result = await self.login()
-            if 'discharge-required-error' in result['response']:
-                log.info('Macaroon discharge required, disconnecting')
-            else:
-                # successful login!
-                log.info('Authenticated')
-                success = True
-        except JujuAPIError as e:
-            if e.error_code != 'redirection required':
-                raise
-            log.info('Controller requested redirect')
-            redirect_info = await self.redirect_info()
-            redir_cacert = redirect_info['ca-cert']
-            new_endpoints = [
-                ("{value}:{port}".format(**s), redir_cacert)
-                for servers in redirect_info['servers']
-                for s in servers if s["scope"] == 'public'
-            ]
-        finally:
-            if not success:
-                await self.close()
-        return success, result, new_endpoints
-
     async def reconnect(self):
         """ Force a reconnection.
         """
@@ -400,256 +427,149 @@
             return
         async with monitor.reconnecting:
             await self.close()
-            await self._connect()
+            await self._connect_with_login([(self.endpoint, self.cacert)])
 
-    async def _connect(self):
-        endpoints = [(self._endpoint, self._cacert)]
-        while endpoints:
-            _endpoint, _cacert = endpoints.pop(0)
-            success, result, new_endpoints = await self._try_endpoint(
-                _endpoint, _cacert)
-            if success:
+    async def _connect(self, endpoints):
+        if len(endpoints) == 0:
+            raise errors.JujuConnectionError('no endpoints to connect to')
+
+        async def _try_endpoint(endpoint, cacert, delay):
+            if delay:
+                await asyncio.sleep(delay)
+            return await self._open(endpoint, cacert)
+
+        # Try all endpoints in parallel, with slight increasing delay (+100ms
+        # for each subsequent endpoint); the delay allows us to prefer the
+        # earlier endpoints over the latter. Use first successful connection.
+        tasks = [self.loop.create_task(_try_endpoint(endpoint, cacert,
+                                                     0.1 * i))
+                 for i, (endpoint, cacert) in enumerate(endpoints)]
+        for task in asyncio.as_completed(tasks, loop=self.loop):
+            try:
+                result = await task
                 break
-            endpoints.extend(new_endpoints)
+            except ConnectionError:
+                continue  # ignore; try another endpoint
         else:
-            # ran out of endpoints without a successful login
-            raise JujuConnectionError("Couldn't authenticate to {}".format(
-                self._endpoint))
+            raise errors.JujuConnectionError(
+                'Unable to connect to any endpoint: {}'.format(', '.join([
+                    endpoint for endpoint, cacert in endpoints])))
+        for task in tasks:
+            task.cancel()
+        self.ws = result[0]
+        self.addr = result[1]
+        self.endpoint = result[2]
+        self.cacert = result[3]
+        self._receiver_task.start()
+        log.info("Driver connected to juju %s", self.addr)
+        self.monitor.close_called.clear()
 
-        response = result['response']
-        self.info = response.copy()
-        self.build_facades(response.get('facades', {}))
-        self.loop.create_task(self.pinger())
-        self.monitor.pinger_stopped.clear()
-
-    @classmethod
-    async def connect(
-            cls, endpoint, uuid, username, password, cacert=None,
-            macaroons=None, loop=None, max_frame_size=None):
+    async def _connect_with_login(self, endpoints):
         """Connect to the websocket.
 
         If uuid is None, the connection will be to the controller. Otherwise it
         will be to the model.
-
+        :return: The response field of login response JSON object.
         """
-        client = cls(endpoint, uuid, username, password, cacert, macaroons,
-                     loop, max_frame_size)
-        await client._connect()
-        return client
+        success = False
+        try:
+            await self._connect(endpoints)
+            # It's possible that we may get several discharge-required errors,
+            # corresponding to different levels of authentication, so retry
+            # a few times.
+            for i in range(0, 2):
+                result = (await self.login())['response']
+                macaroonJSON = result.get('discharge-required')
+                if macaroonJSON is None:
+                    self.info = result
+                    success = True
+                    return result
+                macaroon = bakery.Macaroon.from_dict(macaroonJSON)
+                self.bakery_client.handle_error(
+                    httpbakery.Error(
+                        code=httpbakery.ERR_DISCHARGE_REQUIRED,
+                        message=result.get('discharge-required-error'),
+                        version=macaroon.version,
+                        info=httpbakery.ErrorInfo(
+                            macaroon=macaroon,
+                            macaroon_path=result.get('macaroon-path'),
+                        ),
+                    ),
+                    # note: remove the port number.
+                    'https://' + self.endpoint + '/',
+                )
+            raise errors.JujuAuthError('failed to authenticate '
+                                       'after several attempts')
+        finally:
+            if not success:
+                await self.close()
 
-    @classmethod
-    async def connect_current(cls, loop=None, max_frame_size=None):
-        """Connect to the currently active model.
+    async def _connect_with_redirect(self, endpoints):
+        try:
+            login_result = await self._connect_with_login(endpoints)
+        except errors.JujuRedirectException as e:
+            login_result = await self._connect_with_login(e.endpoints)
+        self._build_facades(login_result.get('facades', {}))
+        self._pinger_task.start()
 
-        """
-        jujudata = JujuData()
-
-        controller_name = jujudata.current_controller()
-        if not controller_name:
-            raise JujuConnectionError('No current controller')
-
-        model_name = jujudata.current_model()
-
-        return await cls.connect_model(
-            '{}:{}'.format(controller_name, model_name), loop, max_frame_size)
-
-    @classmethod
-    async def connect_current_controller(cls, loop=None, max_frame_size=None):
-        """Connect to the currently active controller.
-
-        """
-        jujudata = JujuData()
-        controller_name = jujudata.current_controller()
-        if not controller_name:
-            raise JujuConnectionError('No current controller')
-
-        return await cls.connect_controller(controller_name, loop,
-                                            max_frame_size)
-
-    @classmethod
-    async def connect_controller(cls, controller_name, loop=None,
-                                 max_frame_size=None):
-        """Connect to a controller by name.
-
-        """
-        jujudata = JujuData()
-        controller = jujudata.controllers()[controller_name]
-        endpoint = controller['api-endpoints'][0]
-        cacert = controller.get('ca-cert')
-        accounts = jujudata.accounts()[controller_name]
-        username = accounts['user']
-        password = accounts.get('password')
-        macaroons = get_macaroons(controller_name) if not password else None
-
-        return await cls.connect(
-            endpoint, None, username, password, cacert, macaroons, loop,
-            max_frame_size)
-
-    @classmethod
-    async def connect_model(cls, model, loop=None, max_frame_size=None):
-        """Connect to a model by name.
-
-        :param str model: [<controller>:]<model>
-
-        """
-        jujudata = JujuData()
-
-        if ':' in model:
-            # explicit controller given
-            controller_name, model_name = model.split(':')
-        else:
-            # use the current controller if one isn't explicitly given
-            controller_name = jujudata.current_controller()
-            model_name = model
-
-        accounts = jujudata.accounts()[controller_name]
-        username = accounts['user']
-        # model name must include a user prefix, so add it if it doesn't
-        if '/' not in model_name:
-            model_name = '{}/{}'.format(username, model_name)
-
-        controller = jujudata.controllers()[controller_name]
-        endpoint = controller['api-endpoints'][0]
-        cacert = controller.get('ca-cert')
-        password = accounts.get('password')
-        models = jujudata.models()[controller_name]
-        model_uuid = models['models'][model_name]['uuid']
-        macaroons = get_macaroons(controller_name) if not password else None
-
-        return await cls.connect(
-            endpoint, model_uuid, username, password, cacert, macaroons, loop,
-            max_frame_size)
-
-    def build_facades(self, facades):
+    def _build_facades(self, facades):
         self.facades.clear()
         for facade in facades:
             self.facades[facade['name']] = facade['versions'][-1]
 
     async def login(self):
-        username = self.username
-        if username and not username.startswith('user-'):
-            username = 'user-{}'.format(username)
+        params = {}
+        if self.password:
+            params['auth-tag'] = self.usertag
+            params['credentials'] = self.password
+        else:
+            macaroons = _macaroons_for_domain(self.bakery_client.cookies,
+                                              self.endpoint)
+            params['macaroons'] = [[bakery.macaroon_to_dict(m) for m in ms]
+                                   for ms in macaroons]
 
-        result = await self.rpc({
-            "type": "Admin",
-            "request": "Login",
-            "version": 3,
-            "params": {
-                "auth-tag": username,
-                "credentials": self.password,
-                "nonce": "".join(random.sample(string.printable, 12)),
-                "macaroons": self.macaroons
-            }})
-        return result
-
-    async def redirect_info(self):
         try:
-            result = await self.rpc({
+            return await self.rpc({
+                "type": "Admin",
+                "request": "Login",
+                "version": 3,
+                "params": params,
+            })
+        except errors.JujuAPIError as e:
+            if e.error_code != 'redirection required':
+                raise
+            log.info('Controller requested redirect')
+            # Fetch additional redirection information now so that
+            # we can safely close the connection after login
+            # fails.
+            redirect_info = (await self.rpc({
                 "type": "Admin",
                 "request": "RedirectInfo",
                 "version": 3,
-            })
-        except JujuAPIError as e:
-            if e.message == 'not redirected':
-                return None
-            raise
-        return result['response']
+            }))['response']
+            raise errors.JujuRedirectException(redirect_info) from e
 
 
-class JujuData:
-    def __init__(self):
-        self.path = os.environ.get('JUJU_DATA') or '~/.local/share/juju'
-        self.path = os.path.abspath(os.path.expanduser(self.path))
+class _Task:
+    def __init__(self, task, loop):
+        self.stopped = asyncio.Event(loop=loop)
+        self.stopped.set()
+        self.task = task
+        self.loop = loop
 
-    def current_controller(self):
-        cmd = shlex.split('juju list-controllers --format yaml')
-        output = subprocess.check_output(cmd)
-        output = yaml.safe_load(output)
-        return output.get('current-controller', '')
-
-    def current_model(self, controller_name=None):
-        if not controller_name:
-            controller_name = self.current_controller()
-        models = self.models()[controller_name]
-        if 'current-model' not in models:
-            raise JujuError('No current model')
-        return models['current-model']
-
-    def controllers(self):
-        return self._load_yaml('controllers.yaml', 'controllers')
-
-    def models(self):
-        return self._load_yaml('models.yaml', 'controllers')
-
-    def accounts(self):
-        return self._load_yaml('accounts.yaml', 'controllers')
-
-    def credentials(self):
-        return self._load_yaml('credentials.yaml', 'credentials')
-
-    def load_credential(self, cloud, name=None):
-        """Load a local credential.
-
-        :param str cloud: Name of cloud to load credentials from.
-        :param str name: Name of credential. If None, the default credential
-            will be used, if available.
-        :returns: A CloudCredential instance, or None.
-        """
-        try:
-            cloud = tag.untag('cloud-', cloud)
-            creds_data = self.credentials()[cloud]
-            if not name:
-                default_credential = creds_data.pop('default-credential', None)
-                default_region = creds_data.pop('default-region', None)  # noqa
-                if default_credential:
-                    name = creds_data['default-credential']
-                elif len(creds_data) == 1:
-                    name = list(creds_data)[0]
-                else:
-                    return None, None
-            cred_data = creds_data[name]
-            auth_type = cred_data.pop('auth-type')
-            return name, client.CloudCredential(
-                auth_type=auth_type,
-                attrs=cred_data,
-            )
-        except (KeyError, FileNotFoundError):
-            return None, None
-
-    def _load_yaml(self, filename, key):
-        filepath = os.path.join(self.path, filename)
-        with io.open(filepath, 'rt') as f:
-            return yaml.safe_load(f)[key]
-
-
-def get_macaroons(controller_name=None):
-    """Decode and return macaroons from default ~/.go-cookies
-
-    """
-    cookie_files = []
-    if controller_name:
-        cookie_files.append('~/.local/share/juju/cookies/{}.json'.format(
-            controller_name))
-    cookie_files.append('~/.go-cookies')
-    for cookie_file in cookie_files:
-        cookie_file = Path(cookie_file).expanduser()
-        if cookie_file.exists():
+    def start(self):
+        async def run():
             try:
-                cookies = json.loads(cookie_file.read_text())
-                break
-            except (OSError, ValueError):
-                log.warn("Couldn't load macaroons from %s", cookie_file)
-                return []
-    else:
-        log.warn("Couldn't load macaroons from %s", ' or '.join(cookie_files))
-        return []
+                return await(self.task())
+            finally:
+                self.stopped.set()
+        self.stopped.clear()
+        self.loop.create_task(run())
 
-    base64_macaroons = [
-        c['Value'] for c in cookies
-        if c['Name'].startswith('macaroon-') and c['Value']
-    ]
 
-    return [
-        json.loads(base64.b64decode(value).decode('utf-8'))
-        for value in base64_macaroons
-    ]
+def _macaroons_for_domain(cookies, domain):
+    '''Return any macaroons from the given cookie jar that
+    apply to the given domain name.'''
+    req = urllib.request.Request('https://' + domain + '/')
+    cookies.add_cookie_header(req)
+    return httpbakery.extract_macaroons(req)
diff --git a/modules/libjuju/juju/client/connector.py b/modules/libjuju/juju/client/connector.py
new file mode 100644
index 0000000..64fbe44
--- /dev/null
+++ b/modules/libjuju/juju/client/connector.py
@@ -0,0 +1,147 @@
+import asyncio
+import logging
+import copy
+
+import macaroonbakery.httpbakery as httpbakery
+from juju.client.connection import Connection
+from juju.client.jujudata import FileJujuData
+from juju.errors import JujuConnectionError, JujuError
+
+log = logging.getLogger('connector')
+
+
+class NoConnectionException(Exception):
+    '''Raised by Connector when the connection method is called
+    and there is no current connection.'''
+    pass
+
+
+class Connector:
+    '''This class abstracts out a reconnectable client that can connect
+    to controllers and models found in the Juju data files.
+    '''
+    def __init__(
+        self,
+        loop=None,
+        max_frame_size=None,
+        bakery_client=None,
+        jujudata=None,
+    ):
+        '''Initialize a connector that will use the given parameters
+        by default when making a new connection'''
+        self.max_frame_size = max_frame_size
+        self.loop = loop or asyncio.get_event_loop()
+        self.bakery_client = bakery_client
+        self._connection = None
+        self.controller_name = None
+        self.model_name = None
+        self.jujudata = jujudata or FileJujuData()
+
+    def is_connected(self):
+        '''Report whether there is a currently connected controller or not'''
+        return self._connection is not None
+
+    def connection(self):
+        '''Return the current connection; raises an exception if there
+        is no current connection.'''
+        if not self.is_connected():
+            raise NoConnectionException('not connected')
+        return self._connection
+
+    async def connect(self, **kwargs):
+        """Connect to an arbitrary Juju model.
+
+        kwargs are passed through to Connection.connect()
+        """
+        kwargs.setdefault('loop', self.loop)
+        kwargs.setdefault('max_frame_size', self.max_frame_size)
+        kwargs.setdefault('bakery_client', self.bakery_client)
+        self._connection = await Connection.connect(**kwargs)
+
+    async def disconnect(self):
+        """Shut down the watcher task and close websockets.
+        """
+        if self._connection:
+            log.debug('Closing model connection')
+            await self._connection.close()
+            self._connection = None
+
+    async def connect_controller(self, controller_name=None):
+        """Connect to a controller by name. If the name is empty, it
+        connect to the current controller.
+        """
+        if not controller_name:
+            controller_name = self.jujudata.current_controller()
+        if not controller_name:
+            raise JujuConnectionError('No current controller')
+
+        controller = self.jujudata.controllers()[controller_name]
+        # TODO change Connection so we can pass all the endpoints
+        # instead of just the first.
+        endpoint = controller['api-endpoints'][0]
+        accounts = self.jujudata.accounts().get(controller_name, {})
+
+        await self.connect(
+            endpoint=endpoint,
+            uuid=None,
+            username=accounts.get('user'),
+            password=accounts.get('password'),
+            cacert=controller.get('ca-cert'),
+            bakery_client=self.bakery_client_for_controller(controller_name),
+        )
+        self.controller_name = controller_name
+
+    async def connect_model(self, model_name=None):
+        """Connect to a model by name. If either controller or model
+        parts of the name are empty, the current controller and/or model
+        will be used.
+
+        :param str model: <controller>:<model>
+        """
+
+        try:
+            controller_name, model_name = self.jujudata.parse_model(model_name)
+            controller = self.jujudata.controllers().get(controller_name)
+        except JujuError as e:
+            raise JujuConnectionError(e.message) from e
+        if controller is None:
+            raise JujuConnectionError('Controller {} not found'.format(
+                controller_name))
+        # TODO change Connection so we can pass all the endpoints
+        # instead of just the first one.
+        endpoint = controller['api-endpoints'][0]
+        account = self.jujudata.accounts().get(controller_name, {})
+        models = self.jujudata.models().get(controller_name, {}).get('models',
+                                                                     {})
+        if model_name not in models:
+            raise JujuConnectionError('Model not found: {}'.format(model_name))
+
+        # TODO if there's no record for the required model name, connect
+        # to the controller to find out the model's uuid, then connect
+        # to that. This will let connect_model work with models that
+        # haven't necessarily synced with the local juju data,
+        # and also remove the need for base.CleanModel to
+        # subclass JujuData.
+        await self.connect(
+            endpoint=endpoint,
+            uuid=models[model_name]['uuid'],
+            username=account.get('user'),
+            password=account.get('password'),
+            cacert=controller.get('ca-cert'),
+            bakery_client=self.bakery_client_for_controller(controller_name),
+        )
+        self.controller_name = controller_name
+        self.model_name = controller_name + ':' + model_name
+
+    def bakery_client_for_controller(self, controller_name):
+        '''Make a copy of the bakery client with a the appropriate controller's
+        cookiejar in it.
+        '''
+        bakery_client = self.bakery_client
+        if bakery_client:
+            bakery_client = copy.copy(bakery_client)
+        else:
+            bakery_client = httpbakery.Client()
+        bakery_client.cookies = self.jujudata.cookies_for_controller(
+            controller_name)
+        return bakery_client
diff --git a/modules/libjuju/juju/client/facade.py b/modules/libjuju/juju/client/facade.py
index c015c5f..1c7baa0 100644
--- a/modules/libjuju/juju/client/facade.py
+++ b/modules/libjuju/juju/client/facade.py
@@ -1,16 +1,16 @@
 import argparse
 import builtins
-from collections import defaultdict
 import functools
-from glob import glob
 import json
 import keyword
-from pathlib import Path
 import pprint
 import re
 import textwrap
-from typing import Sequence, Mapping, TypeVar, Any, Union
 import typing
+from collections import defaultdict
+from glob import glob
+from pathlib import Path
+from typing import Any, Mapping, Sequence, TypeVar, Union
 
 from . import codegen
 
diff --git a/modules/libjuju/juju/client/gocookies.py b/modules/libjuju/juju/client/gocookies.py
new file mode 100644
index 0000000..a8a0df8
--- /dev/null
+++ b/modules/libjuju/juju/client/gocookies.py
@@ -0,0 +1,102 @@
+import datetime
+import http.cookiejar as cookiejar
+import json
+import time
+
+import pyrfc3339
+
+
+class GoCookieJar(cookiejar.FileCookieJar):
+    '''A CookieJar implementation that reads and writes cookies
+    to the cookiejar format as understood by the Go package
+    github.com/juju/persistent-cookiejar.'''
+    def _really_load(self, f, filename, ignore_discard, ignore_expires):
+        '''Implement the _really_load method called by FileCookieJar
+        to implement the actual cookie loading'''
+        data = json.load(f) or []
+        now = time.time()
+        for cookie in map(_new_py_cookie, data):
+            if not ignore_expires and cookie.is_expired(now):
+                continue
+            self.set_cookie(cookie)
+
+    def save(self, filename=None, ignore_discard=False, ignore_expires=False):
+        '''Implement the FileCookieJar abstract method.'''
+        if filename is None:
+            if self.filename is not None:
+                filename = self.filename
+            else:
+                raise ValueError(cookiejar.MISSING_FILENAME_TEXT)
+
+        # TODO: obtain file lock, read contents of file, and merge with
+        # current content.
+        go_cookies = []
+        now = time.time()
+        for cookie in self:
+            if not ignore_discard and cookie.discard:
+                continue
+            if not ignore_expires and cookie.is_expired(now):
+                continue
+            go_cookies.append(_new_go_cookie(cookie))
+        with open(filename, "w") as f:
+            f.write(json.dumps(go_cookies))
+
+
+def _new_py_cookie(go_cookie):
+    '''Convert a Go-style JSON-unmarshaled cookie into a Python cookie'''
+    expires = None
+    if go_cookie.get('Expires') is not None:
+        t = pyrfc3339.parse(go_cookie['Expires'])
+        expires = t.timestamp()
+    return cookiejar.Cookie(
+        version=0,
+        name=go_cookie['Name'],
+        value=go_cookie['Value'],
+        port=None,
+        port_specified=False,
+        # Unfortunately Python cookies don't record the original
+        # host that the cookie came from, so we'll just use Domain
+        # for that purpose, and record that the domain was specified,
+        # even though it probably was not. This means that
+        # we won't correctly record the CanonicalHost entry
+        # when writing the cookie file after reading it.
+        domain=go_cookie['Domain'],
+        domain_specified=not go_cookie['HostOnly'],
+        domain_initial_dot=False,
+        path=go_cookie['Path'],
+        path_specified=True,
+        secure=go_cookie['Secure'],
+        expires=expires,
+        discard=False,
+        comment=None,
+        comment_url=None,
+        rest=None,
+        rfc2109=False,
+    )
+
+
+def _new_go_cookie(py_cookie):
+    '''Convert a python cookie to the JSON-marshalable Go-style cookie form.'''
+    # TODO (perhaps):
+    #   HttpOnly
+    #   Creation
+    #   LastAccess
+    #   Updated
+    # not done properly: CanonicalHost.
+    go_cookie = {
+        'Name': py_cookie.name,
+        'Value': py_cookie.value,
+        'Domain': py_cookie.domain,
+        'HostOnly': not py_cookie.domain_specified,
+        'Persistent': not py_cookie.discard,
+        'Secure': py_cookie.secure,
+        'CanonicalHost': py_cookie.domain,
+    }
+    if py_cookie.path_specified:
+        go_cookie['Path'] = py_cookie.path
+    if py_cookie.expires is not None:
+        unix_time = datetime.datetime.fromtimestamp(py_cookie.expires)
+        # Note: fromtimestamp bizarrely produces a time without
+        # a time zone, so we need to use accept_naive.
+        go_cookie['Expires'] = pyrfc3339.generate(unix_time, accept_naive=True)
+    return go_cookie
diff --git a/modules/libjuju/juju/client/jujudata.py b/modules/libjuju/juju/client/jujudata.py
new file mode 100644
index 0000000..8b844c2
--- /dev/null
+++ b/modules/libjuju/juju/client/jujudata.py
@@ -0,0 +1,219 @@
+import abc
+import io
+import os
+import pathlib
+
+import juju.client.client as jujuclient
+import yaml
+from juju import tag
+from juju.client.gocookies import GoCookieJar
+from juju.errors import JujuError
+
+
+class NoModelException(Exception):
+    pass
+
+
+class JujuData:
+    __metaclass__ = abc.ABCMeta
+
+    @abc.abstractmethod
+    def current_controller(self):
+        '''Return the current controller name'''
+        raise NotImplementedError()
+
+    @abc.abstractmethod
+    def controllers(self):
+        '''Return all the currently known controllers as a dict
+        mapping controller name to a dict containing the
+        following string keys:
+        uuid: The UUID of the controller
+        api-endpoints: A list of host:port addresses for the controller.
+        ca-cert: the PEM-encoded CA cert of the controller (optional)
+
+        This is compatible with the "controllers" entry in the YAML-unmarshaled data
+        stored in ~/.local/share/juju/controllers.yaml.
+        '''
+        raise NotImplementedError()
+
+    @abc.abstractmethod
+    def models(self):
+        '''Return all the currently known models as a dict
+        containing a key for each known controller,
+        each holding a dict value containing an optional "current-model"
+        key (the name of the current model for that controller,
+        if there is one), and a dict mapping fully-qualified
+        model names to a dict containing a "uuid" key with the
+        key for that model.
+        This is compatible with the YAML-unmarshaled data
+        stored in ~/.local/share/juju/models.yaml.
+        '''
+        raise NotImplementedError()
+
+    @abc.abstractmethod
+    def accounts(self):
+        '''Return the currently known accounts, as a dict
+        containing a key for each known controller, with
+        each value holding a dict with the following keys:
+
+        user: The username to use when logging into the controller (str)
+        password: The password to use when logging into the controller (str, optional)
+        '''
+        raise NotImplementedError()
+
+    @abc.abstractmethod
+    def cookies_for_controller(self, controller_name):
+        '''Return the cookie jar to use when connecting to the
+        controller with the given name.
+        :return http.cookiejar.CookieJar
+        '''
+        raise NotImplementedError()
+
+    @abc.abstractmethod
+    def current_model(self, controller_name=None, model_only=False):
+        '''Return the current model, qualified by its controller name.
+        If controller_name is specified, the current model for
+        that controller will be returned.
+        If model_only is true, only the model name, not qualified by
+        its controller name, will be returned.
+        '''
+        raise NotImplementedError()
+
+    def parse_model(self, model):
+        """Split the given model_name into controller and model parts.
+        If the controller part is empty, the current controller will be used.
+        If the model part is empty, the current model will be used for
+        the controller.
+        The returned model name will always be qualified with a username.
+        :param model str: The model name to parse.
+        :return (str, str): The controller and model names.
+        """
+        # TODO if model is empty, use $JUJU_MODEL environment variable.
+        if model and ':' in model:
+            # explicit controller given
+            controller_name, model_name = model.split(':')
+        else:
+            # use the current controller if one isn't explicitly given
+            controller_name = self.current_controller()
+            model_name = model
+        if not controller_name:
+            controller_name = self.current_controller()
+        if not model_name:
+            model_name = self.current_model(controller_name, model_only=True)
+            if not model_name:
+                raise NoModelException('no current model')
+
+        if '/' not in model_name:
+            # model name doesn't include a user prefix, so add one
+            # by using the current user for the controller.
+            accounts = self.accounts().get(controller_name)
+            if accounts is None:
+                raise JujuError('No account found for controller {} '.format(controller_name))
+            username = accounts.get('user')
+            if username is None:
+                raise JujuError('No username found for controller {}'.format(controller_name))
+            model_name = username + "/" + model_name
+
+        return controller_name, model_name
+
+
+class FileJujuData(JujuData):
+    '''Provide access to the Juju client configuration files.
+    Any configuration file is read once and then cached.'''
+    def __init__(self):
+        self.path = os.environ.get('JUJU_DATA') or '~/.local/share/juju'
+        self.path = os.path.abspath(os.path.expanduser(self.path))
+        # _loaded keeps track of the loaded YAML from
+        # the Juju data files so we don't need to load the same
+        # file many times.
+        self._loaded = {}
+
+    def refresh(self):
+        '''Forget the cache of configuration file data'''
+        self._loaded = {}
+
+    def current_controller(self):
+        '''Return the current controller name'''
+        return self._load_yaml('controllers.yaml', 'current-controller')
+
+    def current_model(self, controller_name=None, model_only=False):
+        '''Return the current model, qualified by its controller name.
+        If controller_name is specified, the current model for
+        that controller will be returned.
+
+        If model_only is true, only the model name, not qualified by
+        its controller name, will be returned.
+        '''
+        # TODO respect JUJU_MODEL environment variable.
+        if not controller_name:
+            controller_name = self.current_controller()
+        if not controller_name:
+            raise JujuError('No current controller')
+        models = self.models()[controller_name]
+        if 'current-model' not in models:
+            return None
+        if model_only:
+            return models['current-model']
+        return controller_name + ':' + models['current-model']
+
+    def load_credential(self, cloud, name=None):
+        """Load a local credential.
+
+        :param str cloud: Name of cloud to load credentials from.
+        :param str name: Name of credential. If None, the default credential
+            will be used, if available.
+        :return: A CloudCredential instance, or None.
+        """
+        try:
+            cloud = tag.untag('cloud-', cloud)
+            creds_data = self.credentials()[cloud]
+            if not name:
+                default_credential = creds_data.pop('default-credential', None)
+                default_region = creds_data.pop('default-region', None)  # noqa
+                if default_credential:
+                    name = creds_data['default-credential']
+                elif len(creds_data) == 1:
+                    name = list(creds_data)[0]
+                else:
+                    return None, None
+            cred_data = creds_data[name]
+            auth_type = cred_data.pop('auth-type')
+            return name, jujuclient.CloudCredential(
+                auth_type=auth_type,
+                attrs=cred_data,
+            )
+        except (KeyError, FileNotFoundError):
+            return None, None
+
+    def controllers(self):
+        return self._load_yaml('controllers.yaml', 'controllers')
+
+    def models(self):
+        return self._load_yaml('models.yaml', 'controllers')
+
+    def accounts(self):
+        return self._load_yaml('accounts.yaml', 'controllers')
+
+    def credentials(self):
+        return self._load_yaml('credentials.yaml', 'credentials')
+
+    def _load_yaml(self, filename, key):
+        if filename in self._loaded:
+            # Data already exists in the cache.
+            return self._loaded[filename].get(key)
+        # TODO use the file lock like Juju does.
+        filepath = os.path.join(self.path, filename)
+        with io.open(filepath, 'rt') as f:
+            data = yaml.safe_load(f)
+            self._loaded[filename] = data
+            return data.get(key)
+
+    def cookies_for_controller(self, controller_name):
+        f = pathlib.Path(self.path) / 'cookies' / (controller_name + '.json')
+        if not f.exists():
+            f = pathlib.Path('~/.go-cookies').expanduser()
+            # TODO if neither cookie file exists, where should
+            # we create the cookies?
+        jar = GoCookieJar(str(f))
+        jar.load()
+        return jar
diff --git a/modules/libjuju/juju/client/overrides.py b/modules/libjuju/juju/client/overrides.py
index 5e98e56..8b29de7 100644
--- a/modules/libjuju/juju/client/overrides.py
+++ b/modules/libjuju/juju/client/overrides.py
@@ -1,10 +1,8 @@
-from collections import namedtuple
 import re
+from collections import namedtuple
 
+from . import _client, _definitions
 from .facade import ReturnMapping, Type, TypeEncoder
-from .import _client
-from .import _definitions
-
 
 __all__ = [
     'Delta',