Merge pull request #984 from cderici/controller-name-lazy-jujudata

#984

#### Description

This moves the computation of the `controller_name` from the `connector` to the `Controller` class, to be lazily evaluated from `jujudata` (and cached). We can move it out of the connector safely because:

1 - `controller_name` is not used within the connector.
2 - the only location that the `controller_name` is returned to, ignores it.

This has the nice consequence of freeing pylibjuju from relying on things like the `controllers.yaml` file (in turn the juju client to be installed) to connect to a controller.

Fixes #983


#### QA Steps

Added a test for lazy computation of `controller_name`. 

```sh
 $ tox -e integration -- tests/integration/test_controller.py::test_connection_lazy_jujudata
```

But the scenario from the #983 can be tried easily as follows:

```python
# make a new controller and connect it in a normal way:
from juju import controller
c = controller.Controller()
await c.connect()

# get the connection
conn = c.connection()
```

Now go move the `.local/share/juju/controllers.yaml` file temporarily.

```python
# make a new controller
c2 = controller.Controller()

# connect it using the information within the connection we got above
await c2.connect(endpoint=conn.endpoints[0][0], cacert=conn.cacert, username=conn.usertag, password=conn.password)
```

Should succeed without any errors.

Don't forget to move back your `controllers.yaml` back so nothing else on your machine freaks out.

All CI tests need to pass.

#### Notes & Discussion

juju-4891

Author: jujubot

juju/client/connector.py

@@ -9,7 +9,7 @@
 from juju.client.gocookies import GoCookieJar, go_to_py_cookie
 from juju.client.jujudata import FileJujuData, API_ENDPOINTS_KEY
 from juju.client.proxy.factory import proxy_from_config
-from juju.errors import JujuConnectionError, JujuError, PylibjujuProgrammingError
+from juju.errors import JujuConnectionError, JujuError
 from juju.client import client
 from juju.version import SUPPORTED_MAJOR_VERSION, TARGET_JUJU_VERSION
 
@@ -38,7 +38,6 @@ def __init__(
         self.bakery_client = bakery_client
         self._connection = None
         self._log_connection = None
-        self.controller_name = None
         self.controller_uuid = None
         self.model_name = None
         self.jujudata = jujudata or FileJujuData()
@@ -83,11 +82,6 @@ async def connect(self, **kwargs):
                 await self._connection.close()
             self._connection = await Connection.connect(**kwargs)
 
-        if not self.controller_name:
-            if 'endpoint' not in kwargs:
-                raise PylibjujuProgrammingError("Please report this error to the maintainers.")
-            self.controller_name = self.jujudata.controller_name_by_endpoint(kwargs['endpoint'])
-
         # Check if we support the target controller
         juju_server_version = self._connection.info['server-version']
         if not juju_server_version.startswith(TARGET_JUJU_VERSION):
@@ -132,7 +126,6 @@ async def connect_controller(self, controller_name=None, specified_facades=None)
             specified_facades=specified_facades,
             proxy=proxy,
         )
-        self.controller_name = controller_name
         self.controller_uuid = controller["uuid"]
 
     async def connect_model(self, _model_name=None, **kwargs):
@@ -188,9 +181,8 @@ async def connect_model(self, _model_name=None, **kwargs):
         await self.connect(**kwargs)
         # TODO this might be a good spot to trigger refreshing the
         # local cache (the connection to the model might help)
-        self.controller_name = controller_name
         self.model_name = controller_name + ':' + _model_name
-        return self.controller_name, model_uuid
+        return model_uuid
 
     def bakery_client_for_controller(self, controller_name):
         '''Make a copy of the bakery client with a the appropriate controller's

juju/controller.py

@@ -49,6 +49,7 @@ def __init__(
             bakery_client=bakery_client,
             jujudata=jujudata,
         )
+        self._controller_name = None
 
     async def __aenter__(self):
         await self.connect()
@@ -174,7 +175,13 @@ def connection(self):
 
     @property
     def controller_name(self):
-        return self._connector.controller_name
+        if not self._controller_name:
+            try:
+                self._controller_name = self._connector.jujudata.controller_name_by_endpoint(
+                    self._connector.connection().endpoint)
+            except FileNotFoundError:
+                raise errors.PylibjujuError("Unable to determine controller name. controllers.yaml not found.")
+        return self._controller_name
 
     @property
     def controller_uuid(self):

juju/model.py

@@ -663,7 +663,7 @@ async def connect(self, *args, **kwargs):
                 model_name = args[0]
             else:
                 model_name = kwargs.pop('model_name', None)
-            _, model_uuid = await self._connector.connect_model(model_name, **kwargs)
+            model_uuid = await self._connector.connect_model(model_name, **kwargs)
         else:
             # Then we're using the endpoint to pick the model
             if 'model_name' in kwargs:

tests/integration/test_controller.py

@@ -5,7 +5,7 @@
 import uuid
 import hvac
 
-from juju import access
+from juju import access, controller
 from juju.client.connection import Connection
 from juju.client import client
 from juju.errors import JujuAPIError, JujuError
@@ -298,6 +298,22 @@ async def test_grant_revoke_controller_access(event_loop):
                 raise
 
 
+@base.bootstrapped
+async def test_connection_lazy_jujudata(event_loop):
+    async with base.CleanController() as cont1:
+        conn = cont1.connection()
+        new_controller = controller.Controller()
+        await new_controller.connect(endpoint=conn.endpoints[0][0],
+                                     cacert=conn.cacert,
+                                     username=conn.usertag,
+                                     password=conn.password,
+                                     )
+        assert new_controller._controller_name is None
+        new_controller.controller_name
+        assert new_controller._controller_name is not None
+        await new_controller.disconnect()
+
+
 @base.bootstrapped
 async def test_grant_revoke_model_access(event_loop):
     async with base.CleanController() as controller: