Skip to content

High availability support#402

Merged
jujubot merged 2 commits intojuju:masterfrom
davigar15:ha-support
Apr 21, 2020
Merged

High availability support#402
jujubot merged 2 commits intojuju:masterfrom
davigar15:ha-support

Conversation

@davigar15
Copy link
Copy Markdown

This patch aims to add HA support in libjuju, and fix bug #395.

There are a few things that have to be implemented:

  • Property in the controller to get the api_endpoints
  • When the controller machine that libjuju is connected to goes down, reconnect internally to the other endpoints if available
  • Notify the libjuju client when juju has scaled.

@jujubot
Copy link
Copy Markdown
Contributor

jujubot commented Apr 15, 2020

Can one of the admins verify this patch?

1 similar comment
@jujubot
Copy link
Copy Markdown
Contributor

jujubot commented Apr 15, 2020

Can one of the admins verify this patch?

@davigar15
Copy link
Copy Markdown
Author

This is work in progress.

@davigar15
Copy link
Copy Markdown
Author

Added api_endpoints property to controller

@timClicks timClicks changed the title Ha support High availability support Apr 17, 2020
@timClicks timClicks marked this pull request as draft April 17, 2020 09:32
@SimonRichardson SimonRichardson marked this pull request as ready for review April 17, 2020 09:52
Comment thread juju/client/connector.py
Comment on lines +73 to +76
self._connection.endpoints = [
(e, controller["ca-cert"])
for e in controller["api-endpoints"]
]
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 :-)

Comment thread juju/client/connection.py
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.

@SimonRichardson
Copy link
Copy Markdown
Member

$$merge$$

@SimonRichardson
Copy link
Copy Markdown
Member

!!build!!

@SimonRichardson
Copy link
Copy Markdown
Member

$$merge$$

@jujubot jujubot merged commit a0380b2 into juju:master Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants