Skip to content

Commit

Permalink
Ensures that the leader unit will maintain an aggregation of all unit
Browse files Browse the repository at this point in the history
database bind addresses in the applicaion database back for the cluster
relation.
  • Loading branch information
manadart committed Jan 8, 2024
1 parent 789dbe5 commit 95dd0f9
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 30 deletions.
50 changes: 39 additions & 11 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@


class JujuControllerCharm(CharmBase):
DB_BIND_ADDR_KEY = 'db-bind-address'
_stored = StoredState()

def __init__(self, *args):
Expand All @@ -32,7 +33,7 @@ def __init__(self, *args):
self.framework.observe(
self.on.website_relation_joined, self._on_website_relation_joined)

self._stored.set_default(db_bind_address='')
self._stored.set_default(db_bind_address='', last_bind_addresses=[], all_bind_addresses=[])
self._stored.set_default(last_bind_addresses=[])
self.framework.observe(
self.on.dbcluster_relation_changed, self._on_dbcluster_relation_changed)
Expand All @@ -53,16 +54,16 @@ def _on_collect_status(self, event: CollectStatusEvent):
self.api_port()
except AgentConfException as e:
event.add_status(ErrorStatus(
f"cannot read controller API port from agent configuration: {e}"))
f'cannot read controller API port from agent configuration: {e}'))

event.add_status(ActiveStatus())

def _on_config_changed(self, _):
controller_url = self.config['controller-url']
logger.info("got a new controller-url: %r", controller_url)
logger.info('got a new controller-url: %r', controller_url)

def _on_dashboard_relation_joined(self, event):
logger.info("got a new dashboard relation: %r", event)
logger.info('got a new dashboard relation: %r', event)
if self.unit.is_leader():
event.relation.data[self.app].update({
'controller-url': self.config['controller-url'],
Expand All @@ -79,7 +80,7 @@ def _on_website_relation_joined(self, event):
try:
api_port = self.api_port()
except AgentConfException as e:
logger.error("cannot read controller API port from agent configuration: {}", e)
logger.error('cannot read controller API port from agent configuration: {}', e)
return

address = None
Expand All @@ -102,7 +103,7 @@ def _on_metrics_endpoint_relation_created(self, event: RelationJoinedEvent):
try:
api_port = self.api_port()
except AgentConfException as e:
logger.error("cannot read controller API port from agent configuration: {}", e)
logger.error('cannot read controller API port from agent configuration: {}', e)
return

metrics_endpoint = MetricsEndpointProvider(
Expand Down Expand Up @@ -132,20 +133,47 @@ def _on_metrics_endpoint_relation_broken(self, event: RelationDepartedEvent):
self.control_socket.remove_metrics_user(username)

def _on_dbcluster_relation_changed(self, event):
ips = self.model.get_binding(event.relation).network.ingress_addresses
"""Ensure that a bind address for Dqlite is set in relation data,
if we can determine a unique one from the relation's bound space.
If we are the leader, aggregate the bind addresses for all the peers,
and ensure the result is set in the application data bag.
"""
self._ensure_db_bind_address(event)

if self.unit.is_leader():
# The event only has *other* units so include this
# unit's bind address if we have managed to set it.
ip = self._stored.db_bind_address
all_bind_addresses = [ip] if ip else []

for unit in event.relation.units:
unit_data = event.relation.data[unit]
if self.DB_BIND_ADDR_KEY in unit_data:
all_bind_addresses.append(unit_data[self.DB_BIND_ADDR_KEY])

all_bind_addresses.sort()
logger.critical(all_bind_addresses)
if self._stored.all_bind_addresses == all_bind_addresses:
return

event.relation.data[self.app]['db-bind-addresses'] = ','.join(all_bind_addresses)
self._stored.all_bind_addresses = all_bind_addresses

def _ensure_db_bind_address(self, event):
ips = [str(ip) for ip in self.model.get_binding(event.relation).network.ingress_addresses]
self._stored.last_bind_addresses = ips

if len(ips) > 1:
logger.error(
'multiple possible DB bind addresses; set a suitable bcluster network binding')
'multiple possible DB bind addresses; set a suitable cluster network binding')
return

ip = str(ips[0])
ip = ips[0]
if self._stored.db_bind_address == ip:
return

event.relation.data[self.unit].update({self.DB_BIND_ADDR_KEY: ip})
self._stored.db_bind_address = ip
event.relation.data[self.unit].update({'db-bind-address': ip})

def api_port(self) -> str:
"""Return the port on which the controller API server is listening."""
Expand All @@ -157,7 +185,7 @@ def api_port(self) -> str:

parsed_url = urllib.parse.urlsplit('//' + api_addresses[0])
if not parsed_url.port:
raise AgentConfException("API address does not include port")
raise AgentConfException('API address does not include port')
return parsed_url.port

def ca_cert(self) -> str:
Expand Down
38 changes: 19 additions & 19 deletions tests/test_charm.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Copyright 2021 Canonical Ltd.
# Licensed under the GPLv3, see LICENSE file for details.

import ipaddress
import os
import unittest
from charm import JujuControllerCharm, AgentConfException
Expand Down Expand Up @@ -113,18 +114,14 @@ def test_metrics_endpoint_relation(self, mock_remove_user, mock_add_user,

@patch("builtins.open", new_callable=mock_open, read_data=agent_conf_apiaddresses_missing)
def test_apiaddresses_missing(self, _):
harness = Harness(JujuControllerCharm)
self.addCleanup(harness.cleanup)
harness.begin()
harness = self.harness

with self.assertRaisesRegex(AgentConfException, "agent.conf key 'apiaddresses' missing"):
harness.charm.api_port()

@patch("builtins.open", new_callable=mock_open, read_data=agent_conf_apiaddresses_not_list)
def test_apiaddresses_not_list(self, _):
harness = Harness(JujuControllerCharm)
self.addCleanup(harness.cleanup)
harness.begin()
harness = self.harness

with self.assertRaisesRegex(
AgentConfException, "agent.conf key 'apiaddresses' is not a list"
Expand All @@ -134,43 +131,46 @@ def test_apiaddresses_not_list(self, _):
@patch("builtins.open", new_callable=mock_open, read_data=agent_conf_apiaddresses_missing)
@patch("controlsocket.Client.add_metrics_user")
def test_apiaddresses_missing_status(self, *_):
harness = Harness(JujuControllerCharm)
self.addCleanup(harness.cleanup)
harness.begin()
harness = self.harness

harness.add_relation('metrics-endpoint', 'prometheus-k8s')
harness.evaluate_status()
self.assertIsInstance(harness.charm.unit.status, ErrorStatus)

@patch("builtins.open", new_callable=mock_open, read_data=agent_conf_ipv4)
def test_apiaddresses_ipv4(self, _):
harness = Harness(JujuControllerCharm)
self.addCleanup(harness.cleanup)
harness.begin()
harness = self.harness

self.assertEqual(harness.charm.api_port(), 17070)

@patch("builtins.open", new_callable=mock_open, read_data=agent_conf_ipv6)
def test_apiaddresses_ipv6(self, _):
harness = Harness(JujuControllerCharm)
self.addCleanup(harness.cleanup)
harness.begin()
harness = self.harness

self.assertEqual(harness.charm.api_port(), 17070)

@patch("builtins.open", new_callable=mock_open, read_data=agent_conf)
@patch("ops.model.Model.get_binding")
def test_dbcluster_relation_changed_single_addr(self, binding, _):
harness = self.harness
binding.return_value = mockBinding(["192.168.1.17"])
binding.return_value = mockBinding(['192.168.1.17'])

# Have another unit enter the relation.
# Its bind address should end up in the application data bindings list.
relation_id = harness.add_relation('dbcluster', 'controller')
harness.add_relation_unit(relation_id, 'juju-controller/1')
self.harness.update_relation_data(
relation_id, 'juju-controller/1', {'db-bind-address': '192.168.1.100'})

harness.set_leader()
harness.charm._on_dbcluster_relation_changed(
harness.charm.model.get_relation('dbcluster').data[harness.charm.unit])

data = harness.get_relation_data(relation_id, 'juju-controller/0')
self.assertEqual(data["db-bind-address"], "192.168.1.17")
unit_data = harness.get_relation_data(relation_id, 'juju-controller/0')
self.assertEqual(unit_data['db-bind-address'], '192.168.1.17')

app_data = harness.get_relation_data(relation_id, 'juju-controller')
self.assertEqual(app_data['db-bind-addresses'], '192.168.1.100,192.168.1.17')

harness.evaluate_status()
self.assertIsInstance(harness.charm.unit.status, ActiveStatus)
Expand All @@ -193,7 +193,7 @@ def test_dbcluster_relation_changed_multi_addr_error(self, binding, _):

class mockNetwork:
def __init__(self, addresses):
self.ingress_addresses = addresses
self.ingress_addresses = [ipaddress.ip_address(addr) for addr in addresses]
self.ingress_address = addresses[0]


Expand Down

0 comments on commit 95dd0f9

Please sign in to comment.