Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 16 additions & 3 deletions juju/client/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ async def connect(
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 endpoint: The hostname:port of the controller to connect to (or list of strings).
: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
Expand All @@ -256,6 +256,8 @@ async def connect(
self = cls()
if endpoint is None:
raise ValueError('no endpoint provided')
if not isinstance(endpoint, str) and not isinstance(endpoint, list):
raise TypeError("Endpoint should be either str or list")
self.uuid = uuid
if bakery_client is None:
bakery_client = httpbakery.Client()
Expand All @@ -279,6 +281,7 @@ async def connect(
self.addr = None
self.ws = None
self.endpoint = None
self.endpoints = None
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we can get rid of self.endpoint altogether and then say self.endpoints = [endpoint]. That way we reduce the number of if isinstance(endpoint, str) etc.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do this: self.endpoints = [endpoint]

But removing the self.endpoint is more complicated, because that variable is used to by the class to know to which endpoint in particular it is connected to

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmmmm but if I do [endpoint], and endpoint is a list, then self.endpoints will be [["endpoint1", ...]]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davigar15 then we need to update self.endpoint to always be the one it's connected to. That way we're always in sync.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonRichardson That is updated in line 629, in the _connect function.

Every time libjuju reconnects, it executes the function _connect, and then this is executed:
self.endpoint = result[2]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent.

self.cacert = None
self.info = None

Expand All @@ -297,7 +300,11 @@ async def connect(
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)])
await self._connect_with_redirect(
[(endpoint, cacert)]
if isinstance(endpoint, str)
else [(e, cacert) for e in endpoint]
)
return self

@property
Expand Down Expand Up @@ -570,7 +577,11 @@ async def reconnect(self):
return
async with monitor.reconnecting:
await self.close()
await self._connect_with_login([(self.endpoint, self.cacert)])
await self._connect_with_login(
[(self.endpoint, self.cacert)]
if not self.endpoints else
self.endpoints
)

async def _connect(self, endpoints):
if len(endpoints) == 0:
Expand Down Expand Up @@ -660,6 +671,8 @@ async def _connect_with_login(self, endpoints):
finally:
if not success:
await self.close()
else:
self._pinger_task.start()

async def _connect_with_redirect(self, endpoints):
try:
Expand Down
19 changes: 11 additions & 8 deletions juju/client/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ async def connect(self, **kwargs):
for macaroon in kwargs.pop('macaroons'):
jar.set_cookie(go_to_py_cookie(macaroon))
self._connection = await Connection.connect(**kwargs)
controller = self.jujudata.controllers()[
self.jujudata.current_controller()
]
self._connection.endpoints = [
(e, controller["ca-cert"])
for e in controller["api-endpoints"]
]
Comment on lines +73 to +76
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonRichardson I would like to set the endpoints within the Connection, but the problem is that if I get the controller from the connection, it enters into a loop.

If you come up with a better solution, let me know :-)


async def disconnect(self):
"""Shut down the watcher task and close websockets.
Expand All @@ -86,13 +93,11 @@ async def connect_controller(self, controller_name=None, specified_facades=None)
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]
endpoints = controller['api-endpoints']
accounts = self.jujudata.accounts().get(controller_name, {})

await self.connect(
endpoint=endpoint,
endpoint=endpoints,
uuid=None,
username=accounts.get('user'),
password=accounts.get('password'),
Expand All @@ -119,9 +124,7 @@ async def connect_model(self, model_name=None):
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]
endpoints = controller['api-endpoints']
account = self.jujudata.accounts().get(controller_name, {})
models = self.jujudata.models().get(controller_name, {}).get('models',
{})
Expand All @@ -135,7 +138,7 @@ async def connect_model(self, model_name=None):
# and also remove the need for base.CleanModel to
# subclass JujuData.
await self.connect(
endpoint=endpoint,
endpoint=endpoints,
uuid=models[model_name]['uuid'],
username=account.get('user'),
password=account.get('password'),
Expand Down
8 changes: 8 additions & 0 deletions juju/controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,14 @@ def controller_name(self):
def controller_uuid(self):
return self._connector.controller_uuid

@property
def api_endpoints(self):
controller_name = self._connector.jujudata.current_controller()
if controller_name:
return self._connector.jujudata.controllers().get(controller_name).get(
"api-endpoints"
)

async def disconnect(self):
"""Shut down the watcher task and close websockets.

Expand Down