From a21cdfb1ac5fd1a870dfc37d0dcd14720217fe73 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Fri, 2 Sep 2022 15:30:59 +0000 Subject: [PATCH 01/13] Correctlt update the read-only-endpoints when a unit is added or removed --- src/charm.py | 5 ++ src/relations/database.py | 119 +++++++++++++++++++++++++++++++++++++- 2 files changed, 122 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index fff634dff9..53d472e486 100755 --- a/src/charm.py +++ b/src/charm.py @@ -176,6 +176,11 @@ def _on_peer_relation_joined(self, event: RelationJoinedEvent) -> None: event.defer() return + # Safeguard againts event deferall + if self._mysql.is_instance_in_cluster(event_unit_label): + logger.debug(f"Unit {event_unit_label} is already part of the cluster, don't try to add it again.") + return + # Add the instance to the cluster. This operation uses locks to ensure that # only one instance is added to the cluster at a time # (so only one instance is involved in a state transfer at a time) diff --git a/src/relations/database.py b/src/relations/database.py index 1d2b7b013b..78976c4081 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -19,11 +19,12 @@ MySQLGrantPrivilegesToUserError, MySQLUpgradeUserForMySQLRouterError, ) -from ops.charm import RelationBrokenEvent + +from ops.charm import RelationDepartedEvent, RelationJoinedEvent, RelationEvent, RelationBrokenEvent from ops.framework import Object from ops.model import BlockedStatus -from constants import DB_RELATION_NAME, PASSWORD_LENGTH +from constants import DB_RELATION_NAME, PASSWORD_LENGTH, PEER from utils import generate_random_password logger = logging.getLogger(__name__) @@ -43,6 +44,116 @@ def __init__(self, charm): self.framework.observe( self.charm.on[DB_RELATION_NAME].relation_broken, self._on_database_broken ) + self.framework.observe( + self.charm.on[PEER].relation_joined, self._on_relation_joined + ) + self.framework.observe( + self.charm.on[PEER].relation_departed, self._on_relation_departed + ) + + def _on_relation_departed(self, event: RelationDepartedEvent): + """Handle the peer relation departed event for the database relation.""" + if not self.charm.unit.is_leader(): + return + # get all relations involving the database relation + relations = list(self.model.relations[DB_RELATION_NAME]) + logger.info(f"Number of relations: {len(relations)}") + if len(relations) == 0: + return + + if not self.charm.cluster_initialized: + logger.debug("Waiting cluster to be initialized") + return + + # get unit name that departed + dep_unit_name = event.departing_unit.name.replace("/", "-") + + # differ if the added unit is still in the cluster + if self.charm._mysql.is_instance_in_cluster(dep_unit_name): + logger.info(f"Departing unit {dep_unit_name} is still in the cluster!") + event.defer() + return + + relation_data = self.database.fetch_relation_data() + # for all relations update the read-only-endpoints + for relation in relations: + # check if the on_database_requested has been executed + if relation.id not in relation_data: + logger.info("On database requested not happened yet! Nothing to do in this case") + continue + # update the endpoints + self._update_endpoints(relation.id, event) + + + def _on_relation_joined(self, event: RelationJoinedEvent): + """Handle the peer relation joined event for the database relation.""" + if not self.charm.unit.is_leader(): + return + # get all relations involving the database relation + relations = list(self.model.relations[DB_RELATION_NAME]) + logger.info(f"Number of relations: {len(relations)}") + if len(relations) == 0: + return + + if not self.charm.cluster_initialized: + logger.debug("Waiting cluster to be initialized") + return + + # get unit name that joined + event_unit_label = event.unit.name.replace("/", "-") + + # differ if the added unit is not in the cluster + if not self.charm._mysql.is_instance_in_cluster(event_unit_label): + logger.info(f"Added unit {event_unit_label} it is not part of the cluster: differ!") + event.defer() + return + relation_data = self.database.fetch_relation_data() + # for all relations update the read-only-endpoints + for relation in relations: + # check if the on_database_requested has been executed + if relation.id not in relation_data: + logger.info("On database requested not happened yet! Nothing to do in this case") + continue + # update the endpoints + self._update_endpoints(relation.id, event) + + + def _update_endpoints(self, relation_id: int, event: RelationEvent): + """Update the read-only-endpoints + + Args: + relation_id (int): The id of the relation + event (RelationEvent): the triggered event + + """ + remote_app = event.app.name + logger.info("Start endpoint update: ") + try: + + primary_endpoint = self.charm._mysql.get_cluster_primary_address() + self.database.set_endpoints(relation_id, primary_endpoint) + # get read only endpoints by removing primary from all members + read_only_endpoints = sorted( + self.charm._mysql.get_cluster_members_addresses() + - { + primary_endpoint, + } + ) + self.database.set_read_only_endpoints(relation_id, ",".join(read_only_endpoints)) + logger.debug(f"Updateed endpoints for {remote_app}") + + except MySQLCreateApplicationDatabaseAndScopedUserError: + logger.error(f"Failed to create scoped user for app {remote_app}") + self.charm.unit.status = BlockedStatus("Failed to create scoped user") + except MySQLGetMySQLVersionError as e: + logger.exception("Failed to get MySQL version", exc_info=e) + self.charm.unit.status = BlockedStatus("Failed to get MySQL version") + except MySQLGetClusterMembersAddressesError as e: + logger.exception("Failed to get cluster members", exc_info=e) + self.charm.unit.status = BlockedStatus("Failed to get cluster members") + except MySQLClientError as e: + logger.exception("Failed to get primary", exc_info=e) + self.charm.unit.status = BlockedStatus("Failed to get primary") def _get_or_set_password(self, relation) -> str: """Retrieve password from cache or generate a new one. @@ -129,6 +240,7 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: if not self.charm.unit.is_leader(): # run once by the leader return +<<<<<<< HEAD if self.charm._peers.data[self.charm.unit].get("unit-status", None) == "removing": # safeguard against relation broken being triggered for @@ -136,6 +248,9 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: # https://github.com/canonical/mysql-operator/issues/32 return +======= + logger.info(f"On database broken!") +>>>>>>> Correctlt update the read-only-endpoints when a unit is added or removed try: relation_id = event.relation.id self.charm._mysql.delete_user_for_relation(relation_id) From b645527563276745ebcd32415438014a164404a1 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Mon, 5 Sep 2022 19:42:22 +0000 Subject: [PATCH 02/13] Add helper functions --- src/relations/database.py | 3 -- tests/integration/helpers.py | 78 +++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/relations/database.py b/src/relations/database.py index 78976c4081..3eb3d15e73 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -240,7 +240,6 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: if not self.charm.unit.is_leader(): # run once by the leader return -<<<<<<< HEAD if self.charm._peers.data[self.charm.unit].get("unit-status", None) == "removing": # safeguard against relation broken being triggered for @@ -248,9 +247,7 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: # https://github.com/canonical/mysql-operator/issues/32 return -======= logger.info(f"On database broken!") ->>>>>>> Correctlt update the read-only-endpoints when a unit is added or removed try: relation_id = event.relation.id self.charm._mysql.delete_user_for_relation(relation_id) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 5b201c8eaf..517d2aa0b6 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -7,8 +7,10 @@ import re import secrets import string +import yaml +import logging import subprocess -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Set from connector import MysqlConnector from juju.unit import Unit @@ -18,6 +20,7 @@ from constants import SERVER_CONFIG_USERNAME +logger = logging.getLogger(__name__) async def run_command_on_unit(unit, command: str) -> Optional[str]: """Run a command in one Juju unit. @@ -92,6 +95,7 @@ async def scale_application( # Scale down units_to_destroy = [unit.name for unit in application.units[count:]] + logger.info(f"Units to be destroyed: {units_to_destroy}") for unit_to_destroy in units_to_destroy: await ops_test.model.destroy_units(unit_to_destroy) @@ -372,3 +376,75 @@ def cluster_name(unit: Unit, model_name: str) -> str: output = json.loads(output.decode("utf-8")) return output[unit.name]["relation-info"][0]["application-data"]["cluster-name"] + +async def get_relation_data( + ops_test: OpsTest, + application_name: str, + relation_name: str, +) -> list: + """Returns a that contains the relation-data + Args: + ops_test: The ops test framework instance + application_name: The name of the application + relation_name: name of the relation to get connection data from + Returns: + a dictionary that contains the relation-data + """ + unit_name = f"{application_name}/0" + raw_data = (await ops_test.juju("show-unit", unit_name))[1] + if not raw_data: + raise ValueError(f"no unit info could be grabbed for {unit_name}") + data = yaml.safe_load(raw_data) + # Filter the data based on the relation name. + relation_data = [v for v in data[unit_name]["relation-info"] if v["endpoint"] == relation_name] + if len(relation_data) == 0: + raise ValueError( + f"no relation data could be grabbed on relation with endpoint {relation_name}" + ) + logger.info(f"Relation data: {relation_data} with type: {type(relation_data)}") + + return relation_data + +def get_read_only_endpoints(relation_data: list) -> Set[str]: + """Returns the read-only-endpoints from the relation data + Args: + relation_data: The dictionary that contains the info + Returns: + a set that contains the read-only-endpoints + """ + related_units = relation_data[0]['related-units'] + roe = set() + for _, r_data in related_units.items(): + assert 'data' in r_data + data = r_data['data']['data'] + + try: + j_data = json.loads(data) + if 'read-only-endpoints' in j_data: + read_only_endpoints = j_data['read-only-endpoints'] + if read_only_endpoints.strip() == '': + continue + for ep in read_only_endpoints.split(','): + roe.add(ep) + except json.JSONDecodeError: + raise ValueError("Relation data are not valid JSON.") + + return roe + +async def get_unit_hostname(ops_test: OpsTest, app_name: str, units: List[str]) -> List[str]: + """Retrieves hostnames of given application units.""" + unit_hostname = {} + status = await ops_test.model.get_status() # noqa: F821 + machine_hostname = {} + + for machine_id, v in status["machines"].items(): + machine_hostname[machine_id] = v["hostname"] + + unit_machine = {} + for unit in units: + unit_machine[unit] = status["applications"][app_name]["units"][f"{unit}"]["machine"] + + for unit, machine in unit_machine.items(): + if machine in machine_hostname: + unit_hostname[unit] = machine_hostname[machine] + return unit_hostname From 96bd2d75b918fd463de5b212116610f6cc5b0d67 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Mon, 5 Sep 2022 19:44:00 +0000 Subject: [PATCH 03/13] Fix relation broken --- src/relations/database.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/relations/database.py b/src/relations/database.py index 3eb3d15e73..a12bc7a411 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -65,6 +65,13 @@ def _on_relation_departed(self, event: RelationDepartedEvent): logger.debug("Waiting cluster to be initialized") return + logger.info(f"self charm: {self.charm.unit.name} and departing unit: {event.departing_unit.name}") + # check if the leader is departing + logger.info(f"is leader leaving: {self.charm.unit.name == event.departing_unit.name}") + if self.charm.unit.name == event.departing_unit.name: + event.defer() + return + # get unit name that departed dep_unit_name = event.departing_unit.name.replace("/", "-") @@ -140,7 +147,7 @@ def _update_endpoints(self, relation_id: int, event: RelationEvent): } ) self.database.set_read_only_endpoints(relation_id, ",".join(read_only_endpoints)) - logger.debug(f"Updateed endpoints for {remote_app}") + logger.debug(f"Updated endpoints for {remote_app}") except MySQLCreateApplicationDatabaseAndScopedUserError: logger.error(f"Failed to create scoped user for app {remote_app}") From c2881718447a4ef16cc8a3f13f5dccc9c39245c9 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Mon, 5 Sep 2022 19:56:31 +0000 Subject: [PATCH 04/13] small refactor --- src/relations/database.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/relations/database.py b/src/relations/database.py index a12bc7a411..7dbf92461a 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -254,6 +254,7 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: # https://github.com/canonical/mysql-operator/issues/32 return + logger.info(f"On database broken!") try: relation_id = event.relation.id From 4d73b916e08f8df2c211e2bf99fbc1906cc47fa0 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Tue, 6 Sep 2022 17:05:20 +0000 Subject: [PATCH 05/13] Fix get_relation_data --- tests/integration/helpers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 517d2aa0b6..703089a66a 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -381,6 +381,7 @@ async def get_relation_data( ops_test: OpsTest, application_name: str, relation_name: str, + unit_id: int = 0 ) -> list: """Returns a that contains the relation-data Args: @@ -390,7 +391,7 @@ async def get_relation_data( Returns: a dictionary that contains the relation-data """ - unit_name = f"{application_name}/0" + unit_name = f"{application_name}/{unit_id}" raw_data = (await ops_test.juju("show-unit", unit_name))[1] if not raw_data: raise ValueError(f"no unit info could be grabbed for {unit_name}") From 02546d537fd76bee8bc9c83fc98f5a1a3b2f8acb Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Tue, 6 Sep 2022 20:01:27 +0000 Subject: [PATCH 06/13] Add update for leader election --- src/relations/database.py | 40 +++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/src/relations/database.py b/src/relations/database.py index 7dbf92461a..398293f10e 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -20,11 +20,12 @@ MySQLUpgradeUserForMySQLRouterError, ) -from ops.charm import RelationDepartedEvent, RelationJoinedEvent, RelationEvent, RelationBrokenEvent +from ops.charm import RelationDepartedEvent, RelationJoinedEvent, RelationBrokenEvent from ops.framework import Object from ops.model import BlockedStatus from constants import DB_RELATION_NAME, PASSWORD_LENGTH, PEER +from tests.integration.helpers import app_name from utils import generate_random_password logger = logging.getLogger(__name__) @@ -50,7 +51,36 @@ def __init__(self, charm): self.framework.observe( self.charm.on[PEER].relation_departed, self._on_relation_departed ) - + + self.framework.observe( + self.charm.on.leader_elected, self._on_leader_elected + ) + + def _on_leader_elected(self, _ ): + logger.info(f"On leader elected!") + + if not self.charm.unit.is_leader(): + return + # get all relations involving the database relation + relations = list(self.model.relations[DB_RELATION_NAME]) + # check if there are relations in place + if len(relations) == 0: + return + + if not self.charm.cluster_initialized: + logger.debug("Waiting cluster to be initialized") + return + + relation_data = self.database.fetch_relation_data() + # for all relations update the read-only-endpoints + for relation in relations: + # check if the on_database_requested has been executed + if relation.id not in relation_data: + logger.info("On database requested not happened yet! Nothing to do in this case") + continue + self._update_endpoints(relation_id=relation.id, app_name=self.charm.unit.name) + + def _on_relation_departed(self, event: RelationDepartedEvent): """Handle the peer relation departed event for the database relation.""" if not self.charm.unit.is_leader(): @@ -125,15 +155,13 @@ def _on_relation_joined(self, event: RelationJoinedEvent): self._update_endpoints(relation.id, event) - def _update_endpoints(self, relation_id: int, event: RelationEvent): + def _update_endpoints(self, relation_id: int, remote_app: str): """Update the read-only-endpoints Args: relation_id (int): The id of the relation - event (RelationEvent): the triggered event - + remote_app (str): The name of the remote application """ - remote_app = event.app.name logger.info("Start endpoint update: ") try: From 10e0df02cb783f3abf90ae376381179a643c05ba Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Wed, 7 Sep 2022 08:19:48 +0000 Subject: [PATCH 07/13] Refactor of some functions --- tests/integration/helpers.py | 73 +++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 703089a66a..a6fa027efd 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -4,14 +4,14 @@ import itertools import json +import logging import re import secrets import string -import yaml -import logging import subprocess from typing import Dict, List, Optional, Set +import yaml from connector import MysqlConnector from juju.unit import Unit from mysql.connector.errors import InterfaceError, OperationalError, ProgrammingError @@ -22,6 +22,7 @@ logger = logging.getLogger(__name__) + async def run_command_on_unit(unit, command: str) -> Optional[str]: """Run a command in one Juju unit. @@ -377,13 +378,14 @@ def cluster_name(unit: Unit, model_name: str) -> str: return output[unit.name]["relation-info"][0]["application-data"]["cluster-name"] + async def get_relation_data( ops_test: OpsTest, application_name: str, relation_name: str, - unit_id: int = 0 ) -> list: - """Returns a that contains the relation-data + """Returns a that contains the relation-data. + Args: ops_test: The ops test framework instance application_name: The name of the application @@ -391,7 +393,13 @@ async def get_relation_data( Returns: a dictionary that contains the relation-data """ - unit_name = f"{application_name}/{unit_id}" + # get available unit id for the desidered application + units_ids = [ + app_unit.name.split("/")[1] + for app_unit in ops_test.model.applications[application_name].units + ] + assert len(units_ids) > 0 + unit_name = f"{application_name}/{units_ids[0]}" raw_data = (await ops_test.juju("show-unit", unit_name))[1] if not raw_data: raise ValueError(f"no unit info could be grabbed for {unit_name}") @@ -403,38 +411,59 @@ async def get_relation_data( f"no relation data could be grabbed on relation with endpoint {relation_name}" ) logger.info(f"Relation data: {relation_data} with type: {type(relation_data)}") - + return relation_data -def get_read_only_endpoints(relation_data: list) -> Set[str]: - """Returns the read-only-endpoints from the relation data + +def get_read_only_endpoint(relation_data: list) -> Set[str]: + """Returns the read-only-endpoints from the relation data. + Args: relation_data: The dictionary that contains the info Returns: a set that contains the read-only-endpoints """ - related_units = relation_data[0]['related-units'] + related_units = relation_data[0]["related-units"] roe = set() for _, r_data in related_units.items(): - assert 'data' in r_data - data = r_data['data']['data'] + assert "data" in r_data + data = r_data["data"]["data"] try: j_data = json.loads(data) - if 'read-only-endpoints' in j_data: - read_only_endpoints = j_data['read-only-endpoints'] - if read_only_endpoints.strip() == '': + if "read-only-endpoints" in j_data: + read_only_endpoints = j_data["read-only-endpoints"] + if read_only_endpoints.strip() == "": continue - for ep in read_only_endpoints.split(','): + for ep in read_only_endpoints.split(","): roe.add(ep) except json.JSONDecodeError: raise ValueError("Relation data are not valid JSON.") - + return roe -async def get_unit_hostname(ops_test: OpsTest, app_name: str, units: List[str]) -> List[str]: + +def get_read_only_endpoint_hostnames(relation_data: list) -> List[str]: + """Returns the read-only-endpoint hostnames from the relation data. + + Args: + relation_data: The dictionary that contains the info + Returns: + a set that contains the read-only-endpoint hostnames + """ + roe = get_read_only_endpoint(relation_data) + roe_hostnames = [] + for r in roe: + if ":" in r: + roe_hostnames.append(r.split(":")[0]) + else: + raise ValueError("Malformed endpoint") + return roe_hostnames + + +async def get_unit_hostname(ops_test: OpsTest, app_name: str) -> List[str]: """Retrieves hostnames of given application units.""" - unit_hostname = {} + units = [app_unit.name for app_unit in ops_test.model.applications[app_name].units] status = await ops_test.model.get_status() # noqa: F821 machine_hostname = {} @@ -442,10 +471,10 @@ async def get_unit_hostname(ops_test: OpsTest, app_name: str, units: List[str]) machine_hostname[machine_id] = v["hostname"] unit_machine = {} - for unit in units: + for unit in units: unit_machine[unit] = status["applications"][app_name]["units"][f"{unit}"]["machine"] - + hostnames = [] for unit, machine in unit_machine.items(): if machine in machine_hostname: - unit_hostname[unit] = machine_hostname[machine] - return unit_hostname + hostnames.append(machine_hostname[machine]) + return hostnames From 61e0dd7ac957a9692a6d22fd23c673903ad6e8cc Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Wed, 7 Sep 2022 08:20:42 +0000 Subject: [PATCH 08/13] Add tests --- tests/integration/test_database.py | 134 ++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_database.py b/tests/integration/test_database.py index 6b35568575..0c1bf51742 100644 --- a/tests/integration/test_database.py +++ b/tests/integration/test_database.py @@ -7,10 +7,22 @@ import pytest import yaml -from helpers import is_relation_broken, is_relation_joined +from helpers import ( + get_read_only_endpoint_hostnames, + get_relation_data, + get_unit_hostname, + is_relation_broken, + is_relation_joined, + scale_application, +) from pytest_operator.plugin import OpsTest -from constants import PASSWORD_LENGTH, ROOT_USERNAME, SERVER_CONFIG_USERNAME +from constants import ( + DB_RELATION_NAME, + PASSWORD_LENGTH, + ROOT_USERNAME, + SERVER_CONFIG_USERNAME, +) from tests.integration.helpers import ( execute_commands_on_unit, fetch_credentials, @@ -241,6 +253,124 @@ async def test_relation_creation(ops_test: OpsTest): @pytest.mark.order(6) @pytest.mark.abort_on_fail @pytest.mark.database_tests +async def test_ready_only_endpoints(ops_test: OpsTest): + """TODO.""" + + relation_data = await get_relation_data( + ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME + ) + assert len(relation_data) == 1 + read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) + # check correct number of read-only-endpoint is correct + assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( + read_only_endpoints + ) + app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) + logger.info(f"Hostnames from status: {app_hostnames}") + logger.info(f"Read-only-endpoints: {read_only_endpoints}") + # check that endpoints are the one of the application + for r_endpoint in read_only_endpoints: + assert r_endpoint in app_hostnames + + logger.info("Scale to 4 unit") + # increase the number of units + async with ops_test.fast_forward(): + await scale_application(ops_test, DATABASE_APP_NAME, 4) + # check update for read-only-endpoints + relation_data = await get_relation_data( + ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME + ) + read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) + logger.info( + f"Read-only-endpoitns: {read_only_endpoints} --- number of units: {len(ops_test.model.applications[DATABASE_APP_NAME].units)}" + ) + assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( + read_only_endpoints + ) + logger.info( + f"Read-only-endpoints after unit addition: {read_only_endpoints} [{len(read_only_endpoints)}]" + ) + app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) + # check that endpoints are the one of the application + for r_endpoint in read_only_endpoints: + assert r_endpoint in app_hostnames + + logger.info("Scale to 2 units") + # increase the number of units + async with ops_test.fast_forward(): + await scale_application(ops_test, DATABASE_APP_NAME, 2) + # check update for read-only-endpoints + relation_data = await get_relation_data( + ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME + ) + read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) + logger.info( + f"Read-only-endpoitns: {read_only_endpoints} --- number of units: {len(ops_test.model.applications[DATABASE_APP_NAME].units)}" + ) + assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( + read_only_endpoints + ) + logger.info( + f"Read-only-endpoints after unit addition: {read_only_endpoints} [{len(read_only_endpoints)}]" + ) + app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) + # check that endpoints are the one of the application + for r_endpoint in read_only_endpoints: + assert r_endpoint in app_hostnames + + logger.info("Scale up to 3 units and delete the leader unit ") + # increase the number of units + async with ops_test.fast_forward(): + await scale_application(ops_test, DATABASE_APP_NAME, 3) + + leader_unit = None + for app_unit in ops_test.model.applications[DATABASE_APP_NAME].units: + is_leader = await app_unit.is_leader_from_status() + if is_leader: + leader_unit = app_unit.name + + units_to_destroy = [leader_unit] + logger.info(f"Units to be destroyed: {units_to_destroy}") + + for unit_to_destroy in units_to_destroy: + await ops_test.model.destroy_units(unit_to_destroy) + + count = 2 + + application = ops_test.model.applications[DATABASE_APP_NAME] + await ops_test.model.block_until(lambda: len(application.units) == count) + + if count > 0: + await ops_test.model.wait_for_idle( + apps=[DATABASE_APP_NAME], + status="active", + raise_on_blocked=True, + timeout=1000, + ) + relation_data = await get_relation_data( + ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME + ) + read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) + logger.info( + f"Read-only-endpoitns: {read_only_endpoints} --- number of units: {len(ops_test.model.applications[DATABASE_APP_NAME].units)}" + ) + assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( + read_only_endpoints + ) + logger.info( + f"Read-only-endpoints after unit addition: {read_only_endpoints} [{len(read_only_endpoints)}]" + ) + app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) + # check that endpoints are the one of the application + for r_endpoint in read_only_endpoints: + assert r_endpoint in app_hostnames + + logger.info(f"Done with read-only-endpoints test!") + + +@pytest.mark.order(7) +@pytest.mark.abort_on_fail +@pytest.mark.database_tests async def test_relation_broken(ops_test: OpsTest): """Remove relation and wait for the expected changes in status.""" await ops_test.model.applications[DATABASE_APP_NAME].remove_relation( From 0cf488b646acba602925a3fc6916cbcc12323975 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Wed, 7 Sep 2022 08:26:10 +0000 Subject: [PATCH 09/13] happy linter --- src/relations/database.py | 49 ++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/src/relations/database.py b/src/relations/database.py index 398293f10e..81788fdd0c 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -19,13 +19,11 @@ MySQLGrantPrivilegesToUserError, MySQLUpgradeUserForMySQLRouterError, ) - -from ops.charm import RelationDepartedEvent, RelationJoinedEvent, RelationBrokenEvent +from ops.charm import RelationBrokenEvent, RelationDepartedEvent, RelationJoinedEvent from ops.framework import Object from ops.model import BlockedStatus from constants import DB_RELATION_NAME, PASSWORD_LENGTH, PEER -from tests.integration.helpers import app_name from utils import generate_random_password logger = logging.getLogger(__name__) @@ -45,20 +43,15 @@ def __init__(self, charm): self.framework.observe( self.charm.on[DB_RELATION_NAME].relation_broken, self._on_database_broken ) - self.framework.observe( - self.charm.on[PEER].relation_joined, self._on_relation_joined - ) - self.framework.observe( - self.charm.on[PEER].relation_departed, self._on_relation_departed - ) + self.framework.observe(self.charm.on[PEER].relation_joined, self._on_relation_joined) + self.framework.observe(self.charm.on[PEER].relation_departed, self._on_relation_departed) - self.framework.observe( - self.charm.on.leader_elected, self._on_leader_elected - ) + self.framework.observe(self.charm.on.leader_elected, self._on_leader_elected) + + def _on_leader_elected(self, _): + """Handle on leader elected event for the database relation.""" + logger.info("On leader elected!") - def _on_leader_elected(self, _ ): - logger.info(f"On leader elected!") - if not self.charm.unit.is_leader(): return # get all relations involving the database relation @@ -70,7 +63,7 @@ def _on_leader_elected(self, _ ): if not self.charm.cluster_initialized: logger.debug("Waiting cluster to be initialized") return - + relation_data = self.database.fetch_relation_data() # for all relations update the read-only-endpoints for relation in relations: @@ -80,7 +73,6 @@ def _on_leader_elected(self, _ ): continue self._update_endpoints(relation_id=relation.id, app_name=self.charm.unit.name) - def _on_relation_departed(self, event: RelationDepartedEvent): """Handle the peer relation departed event for the database relation.""" if not self.charm.unit.is_leader(): @@ -94,8 +86,10 @@ def _on_relation_departed(self, event: RelationDepartedEvent): if not self.charm.cluster_initialized: logger.debug("Waiting cluster to be initialized") return - - logger.info(f"self charm: {self.charm.unit.name} and departing unit: {event.departing_unit.name}") + + logger.info( + f"self charm: {self.charm.unit.name} and departing unit: {event.departing_unit.name}" + ) # check if the leader is departing logger.info(f"is leader leaving: {self.charm.unit.name == event.departing_unit.name}") if self.charm.unit.name == event.departing_unit.name: @@ -104,13 +98,13 @@ def _on_relation_departed(self, event: RelationDepartedEvent): # get unit name that departed dep_unit_name = event.departing_unit.name.replace("/", "-") - + # differ if the added unit is still in the cluster if self.charm._mysql.is_instance_in_cluster(dep_unit_name): logger.info(f"Departing unit {dep_unit_name} is still in the cluster!") event.defer() return - + relation_data = self.database.fetch_relation_data() # for all relations update the read-only-endpoints for relation in relations: @@ -121,7 +115,6 @@ def _on_relation_departed(self, event: RelationDepartedEvent): # update the endpoints self._update_endpoints(relation.id, event) - def _on_relation_joined(self, event: RelationJoinedEvent): """Handle the peer relation joined event for the database relation.""" if not self.charm.unit.is_leader(): @@ -135,9 +128,9 @@ def _on_relation_joined(self, event: RelationJoinedEvent): if not self.charm.cluster_initialized: logger.debug("Waiting cluster to be initialized") return - + # get unit name that joined - event_unit_label = event.unit.name.replace("/", "-") + event_unit_label = event.unit.name.replace("/", "-") # differ if the added unit is not in the cluster if not self.charm._mysql.is_instance_in_cluster(event_unit_label): @@ -154,12 +147,11 @@ def _on_relation_joined(self, event: RelationJoinedEvent): # update the endpoints self._update_endpoints(relation.id, event) - def _update_endpoints(self, relation_id: int, remote_app: str): - """Update the read-only-endpoints + """Update the read-only-endpoints. Args: - relation_id (int): The id of the relation + relation_id (int): The id of the relation remote_app (str): The name of the remote application """ logger.info("Start endpoint update: ") @@ -282,8 +274,7 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: # https://github.com/canonical/mysql-operator/issues/32 return - - logger.info(f"On database broken!") + logger.info("On database broken!") try: relation_id = event.relation.id self.charm._mysql.delete_user_for_relation(relation_id) From 6a6b40bf2206c686c047bb0430e495e69cd7436d Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Thu, 8 Sep 2022 07:23:15 +0000 Subject: [PATCH 10/13] Small refactor --- src/charm.py | 8 ++-- src/relations/database.py | 28 +++++-------- tests/integration/test_database.py | 66 +++++++----------------------- 3 files changed, 30 insertions(+), 72 deletions(-) diff --git a/src/charm.py b/src/charm.py index 53d472e486..cf79f28214 100755 --- a/src/charm.py +++ b/src/charm.py @@ -176,11 +176,13 @@ def _on_peer_relation_joined(self, event: RelationJoinedEvent) -> None: event.defer() return - # Safeguard againts event deferall + # Safeguard against event deferall if self._mysql.is_instance_in_cluster(event_unit_label): - logger.debug(f"Unit {event_unit_label} is already part of the cluster, don't try to add it again.") + logger.debug( + f"Unit {event_unit_label} is already part of the cluster, don't try to add it again." + ) return - + # Add the instance to the cluster. This operation uses locks to ensure that # only one instance is added to the cluster at a time # (so only one instance is involved in a state transfer at a time) diff --git a/src/relations/database.py b/src/relations/database.py index 81788fdd0c..05fa47ae57 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -50,7 +50,6 @@ def __init__(self, charm): def _on_leader_elected(self, _): """Handle on leader elected event for the database relation.""" - logger.info("On leader elected!") if not self.charm.unit.is_leader(): return @@ -69,9 +68,9 @@ def _on_leader_elected(self, _): for relation in relations: # check if the on_database_requested has been executed if relation.id not in relation_data: - logger.info("On database requested not happened yet! Nothing to do in this case") + logger.debug("On database requested not happened yet! Nothing to do in this case") continue - self._update_endpoints(relation_id=relation.id, app_name=self.charm.unit.name) + self._update_endpoints(relation.id, self.charm.unit.name) def _on_relation_departed(self, event: RelationDepartedEvent): """Handle the peer relation departed event for the database relation.""" @@ -79,7 +78,6 @@ def _on_relation_departed(self, event: RelationDepartedEvent): return # get all relations involving the database relation relations = list(self.model.relations[DB_RELATION_NAME]) - logger.info(f"Number of relations: {len(relations)}") if len(relations) == 0: return @@ -87,11 +85,7 @@ def _on_relation_departed(self, event: RelationDepartedEvent): logger.debug("Waiting cluster to be initialized") return - logger.info( - f"self charm: {self.charm.unit.name} and departing unit: {event.departing_unit.name}" - ) # check if the leader is departing - logger.info(f"is leader leaving: {self.charm.unit.name == event.departing_unit.name}") if self.charm.unit.name == event.departing_unit.name: event.defer() return @@ -101,7 +95,7 @@ def _on_relation_departed(self, event: RelationDepartedEvent): # differ if the added unit is still in the cluster if self.charm._mysql.is_instance_in_cluster(dep_unit_name): - logger.info(f"Departing unit {dep_unit_name} is still in the cluster!") + logger.debug(f"Departing unit {dep_unit_name} is still in the cluster!") event.defer() return @@ -110,10 +104,10 @@ def _on_relation_departed(self, event: RelationDepartedEvent): for relation in relations: # check if the on_database_requested has been executed if relation.id not in relation_data: - logger.info("On database requested not happened yet! Nothing to do in this case") + logger.debug("On database requested not happened yet! Nothing to do in this case") continue # update the endpoints - self._update_endpoints(relation.id, event) + self._update_endpoints(relation.id, event.app.name) def _on_relation_joined(self, event: RelationJoinedEvent): """Handle the peer relation joined event for the database relation.""" @@ -121,7 +115,7 @@ def _on_relation_joined(self, event: RelationJoinedEvent): return # get all relations involving the database relation relations = list(self.model.relations[DB_RELATION_NAME]) - logger.info(f"Number of relations: {len(relations)}") + if len(relations) == 0: return @@ -134,7 +128,6 @@ def _on_relation_joined(self, event: RelationJoinedEvent): # differ if the added unit is not in the cluster if not self.charm._mysql.is_instance_in_cluster(event_unit_label): - logger.info(f"Added unit {event_unit_label} it is not part of the cluster: differ!") event.defer() return relation_data = self.database.fetch_relation_data() @@ -142,19 +135,19 @@ def _on_relation_joined(self, event: RelationJoinedEvent): for relation in relations: # check if the on_database_requested has been executed if relation.id not in relation_data: - logger.info("On database requested not happened yet! Nothing to do in this case") + logger.debug("On database requested not happened yet! Nothing to do in this case") continue # update the endpoints - self._update_endpoints(relation.id, event) + self._update_endpoints(relation.id, event.app.name) def _update_endpoints(self, relation_id: int, remote_app: str): - """Update the read-only-endpoints. + """Updates the read-only-endpoints. Args: relation_id (int): The id of the relation remote_app (str): The name of the remote application """ - logger.info("Start endpoint update: ") + try: primary_endpoint = self.charm._mysql.get_cluster_primary_address() @@ -274,7 +267,6 @@ def _on_database_broken(self, event: RelationBrokenEvent) -> None: # https://github.com/canonical/mysql-operator/issues/32 return - logger.info("On database broken!") try: relation_id = event.relation.id self.charm._mysql.delete_user_for_relation(relation_id) diff --git a/tests/integration/test_database.py b/tests/integration/test_database.py index 0c1bf51742..70707e6d36 100644 --- a/tests/integration/test_database.py +++ b/tests/integration/test_database.py @@ -2,8 +2,10 @@ # Copyright 2022 Canonical Ltd. # See LICENSE file for licensing details. import asyncio +from email.mime import application import logging from pathlib import Path +import time import pytest import yaml @@ -13,6 +15,7 @@ get_unit_hostname, is_relation_broken, is_relation_joined, + remove_leader_unit, scale_application, ) from pytest_operator.plugin import OpsTest @@ -254,7 +257,7 @@ async def test_relation_creation(ops_test: OpsTest): @pytest.mark.abort_on_fail @pytest.mark.database_tests async def test_ready_only_endpoints(ops_test: OpsTest): - """TODO.""" + """Check read-only-endpoints are correctly updated.""" relation_data = await get_relation_data( ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME @@ -266,13 +269,10 @@ async def test_ready_only_endpoints(ops_test: OpsTest): read_only_endpoints ) app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) - logger.info(f"Hostnames from status: {app_hostnames}") - logger.info(f"Read-only-endpoints: {read_only_endpoints}") # check that endpoints are the one of the application for r_endpoint in read_only_endpoints: assert r_endpoint in app_hostnames - logger.info("Scale to 4 unit") # increase the number of units async with ops_test.fast_forward(): await scale_application(ops_test, DATABASE_APP_NAME, 4) @@ -281,92 +281,56 @@ async def test_ready_only_endpoints(ops_test: OpsTest): ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME ) read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) - logger.info( - f"Read-only-endpoitns: {read_only_endpoints} --- number of units: {len(ops_test.model.applications[DATABASE_APP_NAME].units)}" - ) + # check that the number of read-only-endpoints is correct assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( read_only_endpoints ) - logger.info( - f"Read-only-endpoints after unit addition: {read_only_endpoints} [{len(read_only_endpoints)}]" - ) app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) # check that endpoints are the one of the application for r_endpoint in read_only_endpoints: assert r_endpoint in app_hostnames - logger.info("Scale to 2 units") - # increase the number of units + # decrease the number of units async with ops_test.fast_forward(): await scale_application(ops_test, DATABASE_APP_NAME, 2) + + # wait for the update of the endpoints + time.sleep(2 * 60) # check update for read-only-endpoints relation_data = await get_relation_data( ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME ) read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) - logger.info( - f"Read-only-endpoitns: {read_only_endpoints} --- number of units: {len(ops_test.model.applications[DATABASE_APP_NAME].units)}" - ) assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( read_only_endpoints ) - logger.info( - f"Read-only-endpoints after unit addition: {read_only_endpoints} [{len(read_only_endpoints)}]" - ) app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) # check that endpoints are the one of the application for r_endpoint in read_only_endpoints: assert r_endpoint in app_hostnames - logger.info("Scale up to 3 units and delete the leader unit ") # increase the number of units async with ops_test.fast_forward(): await scale_application(ops_test, DATABASE_APP_NAME, 3) - leader_unit = None - for app_unit in ops_test.model.applications[DATABASE_APP_NAME].units: - is_leader = await app_unit.is_leader_from_status() - if is_leader: - leader_unit = app_unit.name - - units_to_destroy = [leader_unit] - logger.info(f"Units to be destroyed: {units_to_destroy}") - - for unit_to_destroy in units_to_destroy: - await ops_test.model.destroy_units(unit_to_destroy) - - count = 2 - - application = ops_test.model.applications[DATABASE_APP_NAME] - await ops_test.model.block_until(lambda: len(application.units) == count) - - if count > 0: - await ops_test.model.wait_for_idle( - apps=[DATABASE_APP_NAME], - status="active", - raise_on_blocked=True, - timeout=1000, - ) + # remove the leader unit + await remove_leader_unit(ops_test=ops_test, application_name=DATABASE_APP_NAME) + + # wait for the update of the endpoints + time.sleep(2 * 60) relation_data = await get_relation_data( ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME ) + read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) - logger.info( - f"Read-only-endpoitns: {read_only_endpoints} --- number of units: {len(ops_test.model.applications[DATABASE_APP_NAME].units)}" - ) assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( read_only_endpoints ) - logger.info( - f"Read-only-endpoints after unit addition: {read_only_endpoints} [{len(read_only_endpoints)}]" - ) app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) # check that endpoints are the one of the application for r_endpoint in read_only_endpoints: assert r_endpoint in app_hostnames - logger.info(f"Done with read-only-endpoints test!") - @pytest.mark.order(7) @pytest.mark.abort_on_fail From 8956c7660c18a85ffbc2970e312b494e79640fe7 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Thu, 8 Sep 2022 07:27:43 +0000 Subject: [PATCH 11/13] Add and document some helper functions --- tests/integration/helpers.py | 43 +++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index a6fa027efd..5ccb58c2f4 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -20,8 +20,6 @@ from constants import SERVER_CONFIG_USERNAME -logger = logging.getLogger(__name__) - async def run_command_on_unit(unit, command: str) -> Optional[str]: """Run a command in one Juju unit. @@ -96,7 +94,6 @@ async def scale_application( # Scale down units_to_destroy = [unit.name for unit in application.units[count:]] - logger.info(f"Units to be destroyed: {units_to_destroy}") for unit_to_destroy in units_to_destroy: await ops_test.model.destroy_units(unit_to_destroy) @@ -410,7 +407,6 @@ async def get_relation_data( raise ValueError( f"no relation data could be grabbed on relation with endpoint {relation_name}" ) - logger.info(f"Relation data: {relation_data} with type: {type(relation_data)}") return relation_data @@ -460,9 +456,46 @@ def get_read_only_endpoint_hostnames(relation_data: list) -> List[str]: raise ValueError("Malformed endpoint") return roe_hostnames +async def remove_leader_unit(ops_test: OpsTest, application_name: str): + """Removes the leader unit of a specified application. + + Args: + ops_test: The ops test framework instance + application_name: The name of the application + """ + leader_unit = None + for app_unit in ops_test.model.applications[application_name].units: + is_leader = await app_unit.is_leader_from_status() + if is_leader: + leader_unit = app_unit.name + + units_to_destroy = [leader_unit] + + for unit_to_destroy in units_to_destroy: + await ops_test.model.destroy_units(unit_to_destroy) + + count = len(ops_test.model.applications[application_name].units) + + application = ops_test.model.applications[application_name] + await ops_test.model.block_until(lambda: len(application.units) == count) + + if count > 0: + await ops_test.model.wait_for_idle( + apps=[application_name], + status="active", + raise_on_blocked=True, + timeout=1000, + ) + + async def get_unit_hostname(ops_test: OpsTest, app_name: str) -> List[str]: - """Retrieves hostnames of given application units.""" + """Retrieves hostnames of given application units. + + Args: + ops_test: The ops test framework instance + application_name: The name of the application + """ units = [app_unit.name for app_unit in ops_test.model.applications[app_name].units] status = await ops_test.model.get_status() # noqa: F821 machine_hostname = {} From 9a8579fe8c46bda4c856840807966daac1ef32c5 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Thu, 8 Sep 2022 07:38:31 +0000 Subject: [PATCH 12/13] Happy lint --- src/relations/database.py | 2 -- tests/integration/helpers.py | 9 +++++---- tests/integration/test_database.py | 10 ++++------ 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/relations/database.py b/src/relations/database.py index 05fa47ae57..089534b799 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -50,7 +50,6 @@ def __init__(self, charm): def _on_leader_elected(self, _): """Handle on leader elected event for the database relation.""" - if not self.charm.unit.is_leader(): return # get all relations involving the database relation @@ -147,7 +146,6 @@ def _update_endpoints(self, relation_id: int, remote_app: str): relation_id (int): The id of the relation remote_app (str): The name of the remote application """ - try: primary_endpoint = self.charm._mysql.get_cluster_primary_address() diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 5ccb58c2f4..55640e8607 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -4,7 +4,6 @@ import itertools import json -import logging import re import secrets import string @@ -456,6 +455,7 @@ def get_read_only_endpoint_hostnames(relation_data: list) -> List[str]: raise ValueError("Malformed endpoint") return roe_hostnames + async def remove_leader_unit(ops_test: OpsTest, application_name: str): """Removes the leader unit of a specified application. @@ -486,15 +486,16 @@ async def remove_leader_unit(ops_test: OpsTest, application_name: str): raise_on_blocked=True, timeout=1000, ) - async def get_unit_hostname(ops_test: OpsTest, app_name: str) -> List[str]: """Retrieves hostnames of given application units. - + Args: ops_test: The ops test framework instance - application_name: The name of the application + app_name: The name of the application + Returns: + a list that contains the hostnames of a given application """ units = [app_unit.name for app_unit in ops_test.model.applications[app_name].units] status = await ops_test.model.get_status() # noqa: F821 diff --git a/tests/integration/test_database.py b/tests/integration/test_database.py index 70707e6d36..4d18677e81 100644 --- a/tests/integration/test_database.py +++ b/tests/integration/test_database.py @@ -2,10 +2,9 @@ # Copyright 2022 Canonical Ltd. # See LICENSE file for licensing details. import asyncio -from email.mime import application import logging -from pathlib import Path import time +from pathlib import Path import pytest import yaml @@ -258,7 +257,6 @@ async def test_relation_creation(ops_test: OpsTest): @pytest.mark.database_tests async def test_ready_only_endpoints(ops_test: OpsTest): """Check read-only-endpoints are correctly updated.""" - relation_data = await get_relation_data( ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME ) @@ -293,7 +291,7 @@ async def test_ready_only_endpoints(ops_test: OpsTest): # decrease the number of units async with ops_test.fast_forward(): await scale_application(ops_test, DATABASE_APP_NAME, 2) - + # wait for the update of the endpoints time.sleep(2 * 60) # check update for read-only-endpoints @@ -313,9 +311,9 @@ async def test_ready_only_endpoints(ops_test: OpsTest): async with ops_test.fast_forward(): await scale_application(ops_test, DATABASE_APP_NAME, 3) - # remove the leader unit + # remove the leader unit await remove_leader_unit(ops_test=ops_test, application_name=DATABASE_APP_NAME) - + # wait for the update of the endpoints time.sleep(2 * 60) relation_data = await get_relation_data( From 4e7d33397b50cfdf4f159d4618485d91631e1049 Mon Sep 17 00:00:00 2001 From: Paolo Sottovia Date: Thu, 8 Sep 2022 15:58:25 +0000 Subject: [PATCH 13/13] Address PR comments --- src/relations/database.py | 13 ++---- tests/integration/helpers.py | 64 ++++++++++++++++---------- tests/integration/test_database.py | 73 ++++++++++-------------------- 3 files changed, 68 insertions(+), 82 deletions(-) diff --git a/src/relations/database.py b/src/relations/database.py index 089534b799..1bce458f99 100644 --- a/src/relations/database.py +++ b/src/relations/database.py @@ -69,7 +69,7 @@ def _on_leader_elected(self, _): if relation.id not in relation_data: logger.debug("On database requested not happened yet! Nothing to do in this case") continue - self._update_endpoints(relation.id, self.charm.unit.name) + self._update_endpoints(relation.id, self.charm.app.name) def _on_relation_departed(self, event: RelationDepartedEvent): """Handle the peer relation departed event for the database relation.""" @@ -86,13 +86,12 @@ def _on_relation_departed(self, event: RelationDepartedEvent): # check if the leader is departing if self.charm.unit.name == event.departing_unit.name: - event.defer() return # get unit name that departed dep_unit_name = event.departing_unit.name.replace("/", "-") - # differ if the added unit is still in the cluster + # defer if the added unit is still in the cluster if self.charm._mysql.is_instance_in_cluster(dep_unit_name): logger.debug(f"Departing unit {dep_unit_name} is still in the cluster!") event.defer() @@ -125,7 +124,7 @@ def _on_relation_joined(self, event: RelationJoinedEvent): # get unit name that joined event_unit_label = event.unit.name.replace("/", "-") - # differ if the added unit is not in the cluster + # defer if the added unit is not in the cluster if not self.charm._mysql.is_instance_in_cluster(event_unit_label): event.defer() return @@ -160,12 +159,6 @@ def _update_endpoints(self, relation_id: int, remote_app: str): self.database.set_read_only_endpoints(relation_id, ",".join(read_only_endpoints)) logger.debug(f"Updated endpoints for {remote_app}") - except MySQLCreateApplicationDatabaseAndScopedUserError: - logger.error(f"Failed to create scoped user for app {remote_app}") - self.charm.unit.status = BlockedStatus("Failed to create scoped user") - except MySQLGetMySQLVersionError as e: - logger.exception("Failed to get MySQL version", exc_info=e) - self.charm.unit.status = BlockedStatus("Failed to get MySQL version") except MySQLGetClusterMembersAddressesError as e: logger.exception("Failed to get cluster members", exc_info=e) self.charm.unit.status = BlockedStatus("Failed to get cluster members") diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 55640e8607..76260c1987 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -380,14 +380,14 @@ async def get_relation_data( application_name: str, relation_name: str, ) -> list: - """Returns a that contains the relation-data. + """Returns a list that contains the relation-data. Args: ops_test: The ops test framework instance application_name: The name of the application relation_name: name of the relation to get connection data from Returns: - a dictionary that contains the relation-data + a list that contains the relation-data """ # get available unit id for the desidered application units_ids = [ @@ -410,7 +410,7 @@ async def get_relation_data( return relation_data -def get_read_only_endpoint(relation_data: list) -> Set[str]: +def get_read_only_endpoints(relation_data: list) -> Set[str]: """Returns the read-only-endpoints from the relation data. Args: @@ -419,23 +419,23 @@ def get_read_only_endpoint(relation_data: list) -> Set[str]: a set that contains the read-only-endpoints """ related_units = relation_data[0]["related-units"] - roe = set() - for _, r_data in related_units.items(): - assert "data" in r_data - data = r_data["data"]["data"] + read_only_endpoints = set() + for _, relation_data in related_units.items(): + assert "data" in relation_data + data = relation_data["data"]["data"] try: j_data = json.loads(data) if "read-only-endpoints" in j_data: - read_only_endpoints = j_data["read-only-endpoints"] - if read_only_endpoints.strip() == "": + read_only_endpoint_field = j_data["read-only-endpoints"] + if read_only_endpoint_field.strip() == "": continue - for ep in read_only_endpoints.split(","): - roe.add(ep) + for ep in read_only_endpoint_field.split(","): + read_only_endpoints.add(ep) except json.JSONDecodeError: raise ValueError("Relation data are not valid JSON.") - return roe + return read_only_endpoints def get_read_only_endpoint_hostnames(relation_data: list) -> List[str]: @@ -446,14 +446,14 @@ def get_read_only_endpoint_hostnames(relation_data: list) -> List[str]: Returns: a set that contains the read-only-endpoint hostnames """ - roe = get_read_only_endpoint(relation_data) - roe_hostnames = [] - for r in roe: - if ":" in r: - roe_hostnames.append(r.split(":")[0]) + read_only_endpoints = get_read_only_endpoints(relation_data) + read_only_endpoint_hostnames = [] + for read_only_endpoint in read_only_endpoints: + if ":" in read_only_endpoint: + read_only_endpoint_hostnames.append(read_only_endpoint.split(":")[0]) else: raise ValueError("Malformed endpoint") - return roe_hostnames + return read_only_endpoint_hostnames async def remove_leader_unit(ops_test: OpsTest, application_name: str): @@ -469,10 +469,7 @@ async def remove_leader_unit(ops_test: OpsTest, application_name: str): if is_leader: leader_unit = app_unit.name - units_to_destroy = [leader_unit] - - for unit_to_destroy in units_to_destroy: - await ops_test.model.destroy_units(unit_to_destroy) + await ops_test.model.destroy_units(leader_unit) count = len(ops_test.model.applications[application_name].units) @@ -498,7 +495,7 @@ async def get_unit_hostname(ops_test: OpsTest, app_name: str) -> List[str]: a list that contains the hostnames of a given application """ units = [app_unit.name for app_unit in ops_test.model.applications[app_name].units] - status = await ops_test.model.get_status() # noqa: F821 + status = await ops_test.model.get_status() machine_hostname = {} for machine_id, v in status["machines"].items(): @@ -512,3 +509,24 @@ async def get_unit_hostname(ops_test: OpsTest, app_name: str) -> List[str]: if machine in machine_hostname: hostnames.append(machine_hostname[machine]) return hostnames + + +async def check_read_only_endpoints(ops_test: OpsTest, app_name: str, relation_name: str): + """Checks that read-only-endpoints are correctly set. + + Args: + ops_test: The ops test framework instance + app_name: The name of the application + relation_name: The name of the relation + """ + # check update for read-only-endpoints + relation_data = await get_relation_data( + ops_test=ops_test, application_name=app_name, relation_name=relation_name + ) + read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) + # check that the number of read-only-endpoints is correct + assert len(ops_test.model.applications[app_name].units) - 1 == len(read_only_endpoints) + app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=app_name) + # check that endpoints are the one of the application + for r_endpoint in read_only_endpoints: + assert r_endpoint in app_hostnames diff --git a/tests/integration/test_database.py b/tests/integration/test_database.py index 4d18677e81..ac9f02609d 100644 --- a/tests/integration/test_database.py +++ b/tests/integration/test_database.py @@ -3,21 +3,20 @@ # See LICENSE file for licensing details. import asyncio import logging -import time from pathlib import Path import pytest import yaml from helpers import ( - get_read_only_endpoint_hostnames, + check_read_only_endpoints, get_relation_data, - get_unit_hostname, is_relation_broken, is_relation_joined, remove_leader_unit, scale_application, ) from pytest_operator.plugin import OpsTest +from tenacity import AsyncRetrying, RetryError, stop_after_delay, wait_fixed from constants import ( DB_RELATION_NAME, @@ -261,51 +260,31 @@ async def test_ready_only_endpoints(ops_test: OpsTest): ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME ) assert len(relation_data) == 1 - read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) - # check correct number of read-only-endpoint is correct - assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( - read_only_endpoints + check_read_only_endpoints( + ops_test=ops_test, app_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME ) - app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) - # check that endpoints are the one of the application - for r_endpoint in read_only_endpoints: - assert r_endpoint in app_hostnames # increase the number of units async with ops_test.fast_forward(): await scale_application(ops_test, DATABASE_APP_NAME, 4) - # check update for read-only-endpoints - relation_data = await get_relation_data( - ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME - ) - read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) - # check that the number of read-only-endpoints is correct - assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( - read_only_endpoints + check_read_only_endpoints( + ops_test=ops_test, app_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME ) - app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) - # check that endpoints are the one of the application - for r_endpoint in read_only_endpoints: - assert r_endpoint in app_hostnames # decrease the number of units async with ops_test.fast_forward(): await scale_application(ops_test, DATABASE_APP_NAME, 2) # wait for the update of the endpoints - time.sleep(2 * 60) - # check update for read-only-endpoints - relation_data = await get_relation_data( - ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME - ) - read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) - assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( - read_only_endpoints - ) - app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) - # check that endpoints are the one of the application - for r_endpoint in read_only_endpoints: - assert r_endpoint in app_hostnames + try: + for attempt in AsyncRetrying(stop=stop_after_delay(5), wait=wait_fixed(20)): + with attempt: + # check update for read-only-endpoints + check_read_only_endpoints( + ops_test=ops_test, app_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME + ) + except RetryError: + assert False # increase the number of units async with ops_test.fast_forward(): @@ -315,19 +294,15 @@ async def test_ready_only_endpoints(ops_test: OpsTest): await remove_leader_unit(ops_test=ops_test, application_name=DATABASE_APP_NAME) # wait for the update of the endpoints - time.sleep(2 * 60) - relation_data = await get_relation_data( - ops_test=ops_test, application_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME - ) - - read_only_endpoints = get_read_only_endpoint_hostnames(relation_data) - assert len(ops_test.model.applications[DATABASE_APP_NAME].units) - 1 == len( - read_only_endpoints - ) - app_hostnames = await get_unit_hostname(ops_test=ops_test, app_name=DATABASE_APP_NAME) - # check that endpoints are the one of the application - for r_endpoint in read_only_endpoints: - assert r_endpoint in app_hostnames + try: + for attempt in AsyncRetrying(stop=stop_after_delay(5), wait=wait_fixed(20)): + with attempt: + # check update for read-only-endpoints + check_read_only_endpoints( + ops_test=ops_test, app_name=DATABASE_APP_NAME, relation_name=DB_RELATION_NAME + ) + except RetryError: + assert False @pytest.mark.order(7)