From 16e557bbc567bc0b890ea6315bdf9df8445fc65c Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Fri, 24 Oct 2025 13:02:54 +0200 Subject: [PATCH 1/4] Update ops to 2.10 --- common/common/abstract_charm.py | 81 ++++++++++--------- common/common/relations/cos.py | 12 --- common/common/relations/database_provides.py | 81 +++++++++++-------- common/common/relations/database_requires.py | 51 +++++++++--- common/common/relations/tls.py | 26 +++--- common/common/workload.py | 36 ++++----- common/pyproject.toml | 2 +- kubernetes/poetry.lock | 13 +-- kubernetes/pyproject.toml | 3 +- kubernetes/src/charm.py | 55 +++++++------ machines/poetry.lock | 13 +-- machines/pyproject.toml | 3 +- machines/src/charm.py | 50 ++++++------ machines/src/machine_workload.py | 12 +-- .../relations/database_providers_wrapper.py | 21 +++-- .../deprecated_shared_db_database_provides.py | 74 ++++++++--------- machines/src/relations/hacluster.py | 14 +++- 17 files changed, 287 insertions(+), 260 deletions(-) diff --git a/common/common/abstract_charm.py b/common/common/abstract_charm.py index 917e7403..9268be6b 100644 --- a/common/common/abstract_charm.py +++ b/common/common/abstract_charm.py @@ -120,31 +120,37 @@ def _logrotate(self) -> logrotate.LogRotate: def _cos_relation_type(self) -> type[cos.COSRelation]: """COSRelation type""" + @property @abc.abstractmethod - def _read_write_endpoints(self, *, event) -> str: + def _read_write_endpoints(self) -> str: """MySQL Router read-write endpoint""" + @property @abc.abstractmethod - def _read_only_endpoints(self, *, event) -> str: + def _read_only_endpoints(self) -> str: """MySQL Router read-only endpoint""" + @property @abc.abstractmethod - def is_externally_accessible(self, *, event) -> bool | None: + def is_externally_accessible(self) -> bool | None: """Whether endpoints should be externally accessible. Only defined in vm charm to return True/False. In k8s charm, returns None. """ + @property @abc.abstractmethod - def tls_sans_ip(self, *, event) -> list[str] | None: + def tls_sans_ip(self) -> list[str] | None: """TLS IP subject alternative names""" + @property @abc.abstractmethod - def tls_sans_dns(self, *, event) -> list[str] | None: + def tls_sans_dns(self) -> list[str] | None: """TLS DNS subject alternative names""" + @property @abc.abstractmethod - def _status(self, *, event) -> ops.StatusBase | None: + def _status(self) -> ops.StatusBase | None: """Status of the charm.""" @property @@ -172,16 +178,14 @@ def tracing_endpoint(self) -> str | None: """Otlp http endpoint for charm instrumentation.""" return self._cos_relation.tracing_endpoint - def _cos_exporter_config(self, event) -> cos.ExporterConfig | None: + @property + def _cos_exporter_config(self) -> cos.ExporterConfig | None: """Returns the exporter config for MySQLRouter exporter if cos relation exists""" - cos_relation_exists = ( - self._cos_relation.relation_exists - and not self._cos_relation.is_relation_breaking(event) - ) - if cos_relation_exists: + if self._cos_relation.relation_exists: return self._cos_relation.exporter_user_config + return None - def get_workload(self, *, event, refresh: charm_refresh.Common = None): + def get_workload(self, *, refresh: charm_refresh.Common = None): """MySQL Router workload Pass `refresh` if `self.refresh` is not set @@ -189,7 +193,7 @@ def get_workload(self, *, event, refresh: charm_refresh.Common = None): if refresh is None: refresh = self.refresh if refresh.workload_allowed_to_start and ( - connection_info := self._database_requires.get_connection_info(event=event) + connection_info := self._database_requires.connection_info ): return self._running_workload_type( container_=self._container, @@ -221,24 +225,24 @@ def _prioritize_statuses(statuses: list[ops.StatusBase]) -> ops.StatusBase: return status return ops.ActiveStatus() - def _determine_app_status(self, *, event) -> ops.StatusBase: + def _determine_app_status(self) -> ops.StatusBase: """Report app status.""" if self.refresh.app_status_higher_priority: return self.refresh.app_status_higher_priority statuses = [] - if status := self._status(event=event): + if status := self._status: statuses.append(status) for endpoint in (self._database_requires, self._database_provides): - if status := endpoint.get_status(event): + if status := endpoint.status: statuses.append(status) return self._prioritize_statuses(statuses) - def _determine_unit_status(self, *, event) -> ops.StatusBase: + def _determine_unit_status(self) -> ops.StatusBase: """Report unit status.""" if self.refresh.unit_status_higher_priority: return self.refresh.unit_status_higher_priority statuses = [] - workload_ = self.get_workload(event=event) + workload_ = self.get_workload() if status := workload_.status: statuses.append(status) # only in machine charms @@ -252,17 +256,17 @@ def _determine_unit_status(self, *, event) -> ops.StatusBase: return refresh_lower_priority return self._prioritize_statuses(statuses) - def set_status(self, *, event, app=True, unit=True) -> None: + def set_status(self, *, app=True, unit=True) -> None: """Set charm status.""" if app and self._unit_lifecycle.authorized_leader: - self.app.status = self._determine_app_status(event=event) + self.app.status = self._determine_app_status() logger.debug(f"Set app status to {self.app.status}") if unit: - self.unit.status = self._determine_unit_status(event=event) + self.unit.status = self._determine_unit_status() logger.debug(f"Set unit status to {self.unit.status}") @abc.abstractmethod - def wait_until_mysql_router_ready(self, *, event) -> None: + def wait_until_mysql_router_ready(self) -> None: """Wait until a connection to MySQL Router is possible. Retry every 5 seconds for up to 30 seconds. @@ -276,36 +280,35 @@ def _reconcile_service(self) -> None: """ @abc.abstractmethod - def _reconcile_ports(self, *, event) -> None: + def _reconcile_ports(self) -> None: """Reconcile exposed ports. Only applies to Machine charm """ @abc.abstractmethod - def _update_endpoints(self, *, event) -> None: + def _update_endpoints(self) -> None: """Update the endpoints in the provider relation if necessary.""" # ======================= # Handlers # ======================= - def reconcile(self, event=None) -> None: # noqa: C901 + def reconcile(self, _=None) -> None: # noqa: C901 """Handle most events.""" if not self._reconcile_allowed: logger.debug("Reconcile not allowed") return - workload_ = self.get_workload(event=event) + workload_ = self.get_workload() logger.debug( "State of reconcile " f"{self._unit_lifecycle.authorized_leader=}, " f"{isinstance(workload_, workload.RunningWorkload)=}, " f"{workload_.container_ready=}, " f"{self.refresh.workload_allowed_to_start=}, " - f"{self._database_requires.is_relation_breaking(event)=}, " + f"{self._database_requires.is_relation_breaking=}, " f"{self._database_requires.does_relation_exist()=}, " - f"{self.refresh.in_progress=}, " - f"{self._cos_relation.is_relation_breaking(event)=}" + f"{self.refresh.in_progress=}" ) if isinstance(self.refresh, charm_refresh.Machines): workload_.install( @@ -321,7 +324,7 @@ def reconcile(self, event=None) -> None: # noqa: C901 try: if self._unit_lifecycle.authorized_leader: - if self._database_requires.is_relation_breaking(event): + if self._database_requires.is_relation_breaking: if self.refresh.in_progress: logger.warning( "Modifying relations during a refresh is not supported. The charm may be in a broken, unrecoverable state. Re-deploy the charm" @@ -334,19 +337,17 @@ def reconcile(self, event=None) -> None: # noqa: C901 ): self._reconcile_service() self._database_provides.reconcile_users( - event=event, - router_read_write_endpoints=self._read_write_endpoints(event=event), - router_read_only_endpoints=self._read_only_endpoints(event=event), + router_read_write_endpoints=self._read_write_endpoints, + router_read_only_endpoints=self._read_only_endpoints, shell=workload_.shell, ) - self._update_endpoints(event=event) + self._update_endpoints() if workload_.container_ready: workload_.reconcile( - event=event, tls=self._tls_certificate_saved, unit_name=self.unit.name, - exporter_config=self._cos_exporter_config(event), + exporter_config=self._cos_exporter_config, key=self._tls_key, certificate=self._tls_certificate, certificate_authority=self._tls_certificate_authority, @@ -354,7 +355,7 @@ def reconcile(self, event=None) -> None: # noqa: C901 if not self.refresh.in_progress and isinstance( workload_, workload.RunningWorkload ): - self._reconcile_ports(event=event) + self._reconcile_ports() logger.debug(f"{workload_.status=}") if not workload_.status: @@ -371,9 +372,9 @@ def reconcile(self, event=None) -> None: # noqa: C901 else: # Waiting for database requires relation; refresh can continue self.refresh.next_unit_allowed_to_refresh = True - self.set_status(event=event) + self.set_status() except server_exceptions.Error as e: # If not for `unit=False`, another `server_exceptions.Error` could be thrown here - self.set_status(event=event, unit=False) + self.set_status(unit=False) self.unit.status = e.status logger.debug(f"Set unit status to {self.unit.status}") diff --git a/common/common/relations/cos.py b/common/common/relations/cos.py index 6e127687..25eb1c49 100644 --- a/common/common/relations/cos.py +++ b/common/common/relations/cos.py @@ -7,8 +7,6 @@ import dataclasses import typing -import ops - from .. import container, utils from . import secrets @@ -91,13 +89,3 @@ def get_monitoring_password(self) -> str: def reset_monitoring_password(self) -> None: """Reset the monitoring password from unit peer data.""" self._secrets.set_value(secrets.UNIT_SCOPE, self._MONITORING_PASSWORD_KEY, None) - - def is_relation_breaking(self, event) -> bool: - """Whether relation will be broken after the current event is handled.""" - if not self.relation_exists: - return False - - return ( - isinstance(event, ops.RelationBrokenEvent) - and event.relation.id == self._charm.model.relations[self._METRICS_RELATION_NAME][0].id - ) diff --git a/common/common/relations/database_provides.py b/common/common/relations/database_provides.py index d51cdc03..14fa82b3 100644 --- a/common/common/relations/database_provides.py +++ b/common/common/relations/database_provides.py @@ -6,6 +6,7 @@ import logging import typing +import charm_ as charm import ops from .. import mysql_shell, status_exception @@ -72,11 +73,11 @@ class _RelationThatRequestedUser(_Relation): """Related application charm that has requested a database & user""" def __init__( - self, *, relation: ops.Relation, interface: data_interfaces.DatabaseProvides, event + self, *, relation: ops.Relation, interface: data_interfaces.DatabaseProvides ) -> None: super().__init__(relation=relation, interface=interface) self._interface = interface - if isinstance(event, ops.RelationBrokenEvent) and event.relation.id == self._id: + if not relation.active: raise _RelationBreaking self._database: str = self._databag["database"] if self._databag.get("extra-user-roles"): @@ -121,10 +122,7 @@ def create_database_and_user( shell.delete_user(username, must_exist=False) logger.debug("Deleted user if exists before creating user") - password = shell.create_application_database( - database=self._database, - username=username, - ) + password = shell.create_application_database(database=self._database, username=username) self._set_databag( username=username, password=password, @@ -151,10 +149,7 @@ def __init__( raise _UserNotShared def update_endpoints( - self, - *, - router_read_write_endpoints: str, - router_read_only_endpoints: str, + self, *, router_read_write_endpoints: str, router_read_only_endpoints: str ) -> None: """Update the endpoints in the databag.""" logger.debug( @@ -187,13 +182,39 @@ class RelationEndpoint: _NAME = "database" def __init__(self, charm_: "abstract_charm.MySQLRouterCharm") -> None: - self._interface = data_interfaces.DatabaseProvides(charm_, relation_name=self._NAME) - charm_.framework.observe(self._interface.on.database_requested, charm_.reconcile) + self._charm = charm_ + self._interface = data_interfaces.DatabaseProvides(self._charm, relation_name=self._NAME) + self._charm.framework.observe(self._interface.on.database_requested, self._charm.reconcile) + + @property + def _relations(self) -> list[ops.Relation]: + """`self._interface.relations` & breaking relations + + Needed because of breaking change in ops 2.10 + https://github.com/canonical/operator/pull/1091 + + Breaking relations included to clean up users during relation-broken event + """ + # Recommended approach from Charm Tech team: + # https://github.com/canonical/operator/issues/1279#issuecomment-2921130420 + + # Use Juju event (`charm.event`) instead of ops event so that breaking relation is included + # in ops deferred or custom events, as recommended by Charm Tech team + if isinstance( + charm.event, charm.RelationBrokenEvent + ) and charm.event.endpoint == charm.Endpoint(self._NAME): + return [ + *self._interface.relations, + self._charm.model.get_relation( + relation_name=self._NAME, relation_id=charm.event.relation.id + ), + ] + return self._interface.relations @property def _shared_users(self) -> list[_RelationWithSharedUser]: shared_users = [] - for relation in self._interface.relations: + for relation in self._relations: try: shared_users.append( _RelationWithSharedUser(relation=relation, interface=self._interface) @@ -202,18 +223,17 @@ def _shared_users(self) -> list[_RelationWithSharedUser]: pass return shared_users - def external_connectivity(self, event) -> bool: + @property + def external_connectivity(self) -> bool: """Whether any of the relations are marked as external. Only used on machines charm """ requested_users = [] - for relation in self._interface.relations: + for relation in self._relations: try: requested_users.append( - _RelationThatRequestedUser( - relation=relation, interface=self._interface, event=event - ) + _RelationThatRequestedUser(relation=relation, interface=self._interface) ) except ( _RelationBreaking, @@ -224,10 +244,7 @@ def external_connectivity(self, event) -> bool: return any(relation.external_connectivity for relation in requested_users) def update_endpoints( - self, - *, - router_read_write_endpoints: str, - router_read_only_endpoints: str, + self, *, router_read_write_endpoints: str, router_read_only_endpoints: str ) -> None: """Update endpoints in the databags.""" for relation in self._shared_users: @@ -239,7 +256,6 @@ def update_endpoints( def reconcile_users( self, *, - event, router_read_write_endpoints: str, router_read_only_endpoints: str, shell: mysql_shell.Shell, @@ -251,15 +267,13 @@ def reconcile_users( relation is broken. """ logger.debug( - f"Reconciling users {event=}, {router_read_write_endpoints=}, {router_read_only_endpoints=}" + f"Reconciling users {router_read_write_endpoints=}, {router_read_only_endpoints=}" ) requested_users = [] - for relation in self._interface.relations: + for relation in self._relations: try: requested_users.append( - _RelationThatRequestedUser( - relation=relation, interface=self._interface, event=event - ) + _RelationThatRequestedUser(relation=relation, interface=self._interface) ) except ( _RelationBreaking, @@ -279,7 +293,7 @@ def reconcile_users( if relation not in requested_users: relation.delete_user(shell=shell) logger.debug( - f"Reconciled users {event=}, {router_read_write_endpoints=}, {router_read_only_endpoints=}" + f"Reconciled users {router_read_write_endpoints=}, {router_read_only_endpoints=}" ) def delete_all_databags(self) -> None: @@ -296,7 +310,8 @@ def delete_all_databags(self) -> None: relation.delete_databag() logger.debug("Deleted all application databags") - def get_status(self, event) -> ops.StatusBase | None: + @property + def status(self) -> ops.StatusBase | None: """Report non-active status.""" requested_users = [] exception_reporting_priority = ( @@ -304,12 +319,10 @@ def get_status(self, event) -> ops.StatusBase | None: remote_databag.IncompleteDatabag, ) exceptions: list[status_exception.StatusException] = [] - for relation in self._interface.relations: + for relation in self._relations: try: requested_users.append( - _RelationThatRequestedUser( - relation=relation, interface=self._interface, event=event - ) + _RelationThatRequestedUser(relation=relation, interface=self._interface) ) except _RelationBreaking: pass diff --git a/common/common/relations/database_requires.py b/common/common/relations/database_requires.py index 33a314fd..df7222f8 100644 --- a/common/common/relations/database_requires.py +++ b/common/common/relations/database_requires.py @@ -69,14 +69,35 @@ class CompleteConnectionInformation(ConnectionInformation): (Different from user that MySQL Router runs with after bootstrap.) """ - def __init__(self, *, interface: data_interfaces.DatabaseRequires, event) -> None: - relations = interface.relations + def __init__( + self, *, interface: data_interfaces.DatabaseRequires, charm_: ops.CharmBase + ) -> None: endpoint_name = interface.relation_name + + # Needed because of breaking change in ops 2.10 + # https://github.com/canonical/operator/pull/1091 + # Breaking relations included to clean up users during relation-broken event + # Recommended approach from Charm Tech team: + # https://github.com/canonical/operator/issues/1279#issuecomment-2921130420 + # Use Juju event (`charm.event`) instead of ops event so that breaking relation is included + # in ops deferred or custom events, as recommended by Charm Tech team + if isinstance( + charm.event, charm.RelationBrokenEvent + ) and charm.event.endpoint == charm.Endpoint(endpoint_name): + relations = [ + *interface.relations, + charm_.model.get_relation( + relation_name=endpoint_name, relation_id=charm.event.relation.id + ), + ] + else: + relations = interface.relations + if not relations: raise _MissingRelation(endpoint_name=endpoint_name) assert len(relations) == 1 relation = relations[0] - if isinstance(event, ops.RelationBrokenEvent) and event.relation.id == relation.id: + if not relation.active: # Relation will be broken after the current event is handled raise _RelationBreaking(endpoint_name=endpoint_name) # MySQL charm databag @@ -103,37 +124,41 @@ class RelationEndpoint: _NAME = "backend-database" def __init__(self, charm_: "abstract_charm.MySQLRouterCharm") -> None: + self._charm = charm_ self._interface = data_interfaces.DatabaseRequires( - charm_, + self._charm, relation_name=self._NAME, # Database name disregarded by MySQL charm if "mysqlrouter" extra user role requested database_name="mysql_innodb_cluster_metadata", extra_user_roles="mysqlrouter", ) - charm_.framework.observe(self._interface.on.database_created, charm_.reconcile) - charm_.framework.observe(self._interface.on.endpoints_changed, charm_.reconcile) + self._charm.framework.observe(self._interface.on.database_created, self._charm.reconcile) + self._charm.framework.observe(self._interface.on.endpoints_changed, self._charm.reconcile) - def get_connection_info(self, *, event) -> CompleteConnectionInformation | None: + @property + def connection_info(self) -> CompleteConnectionInformation | None: """Information for connection to MySQL cluster""" try: - return CompleteConnectionInformation(interface=self._interface, event=event) + return CompleteConnectionInformation(interface=self._interface, charm_=self._charm) except (_MissingRelation, remote_databag.IncompleteDatabag): - return + return None - def is_relation_breaking(self, event) -> bool: + @property + def is_relation_breaking(self) -> bool: """Whether relation will be broken after the current event is handled""" try: - CompleteConnectionInformation(interface=self._interface, event=event) + CompleteConnectionInformation(interface=self._interface, charm_=self._charm) except _RelationBreaking: return True except (_MissingRelation, remote_databag.IncompleteDatabag): pass return False - def get_status(self, event) -> ops.StatusBase | None: + @property + def status(self) -> ops.StatusBase | None: """Report non-active status.""" try: - CompleteConnectionInformation(interface=self._interface, event=event) + CompleteConnectionInformation(interface=self._interface, charm_=self._charm) except (_MissingRelation, remote_databag.IncompleteDatabag) as exception: return exception.status diff --git a/common/common/relations/tls.py b/common/common/relations/tls.py index 99e35c6e..a04bc072 100644 --- a/common/common/relations/tls.py +++ b/common/common/relations/tls.py @@ -108,9 +108,9 @@ def save_certificate(self, event: tls_certificates.CertificateAvailableEvent) -> self._secrets.get_value(secrets.UNIT_SCOPE, _TLS_REQUESTED_CSR), ) logger.debug(f"Saved TLS certificate {event=}") - self._charm.reconcile(event=None) + self._charm.reconcile() - def _generate_csr(self, *, event, key: bytes) -> bytes: + def _generate_csr(self, *, key: bytes) -> bytes: """Generate certificate signing request (CSR).""" return tls_certificates.generate_csr( private_key=key, @@ -118,23 +118,23 @@ def _generate_csr(self, *, event, key: bytes) -> bytes: # (https://github.com/pyca/cryptography/issues/10553) subject=socket.getfqdn()[:64], organization=self._charm.app.name, - sans_ip=self._charm.tls_sans_ip(event=event), - sans_dns=self._charm.tls_sans_dns(event=event), + sans_ip=self._charm.tls_sans_ip, + sans_dns=self._charm.tls_sans_dns, ) - def request_certificate_creation(self, *, event): + def request_certificate_creation(self): """Request new TLS certificate from related provider charm.""" logger.debug("Requesting TLS certificate creation") - csr = self._generate_csr(event=event, key=self.key.encode("utf-8")) + csr = self._generate_csr(key=self.key.encode("utf-8")) self._interface.request_certificate_creation(certificate_signing_request=csr) self._secrets.set_value(secrets.UNIT_SCOPE, _TLS_REQUESTED_CSR, csr.decode("utf-8")) logger.debug("Requested TLS certificate creation") - def request_certificate_renewal(self, *, event): + def request_certificate_renewal(self): """Request TLS certificate renewal from related provider charm.""" logger.debug("Requesting TLS certificate renewal") old_csr = self._secrets.get_value(secrets.UNIT_SCOPE, _TLS_ACTIVE_CSR).encode("utf-8") - new_csr = self._generate_csr(event=event, key=self.key.encode("utf-8")) + new_csr = self._generate_csr(key=self.key.encode("utf-8")) self._interface.request_certificate_renewal( old_certificate_signing_request=old_csr, new_certificate_signing_request=new_csr ) @@ -244,7 +244,7 @@ def _on_set_tls_private_key(self, event: ops.ActionEvent) -> None: logger.debug("No TLS certificate relation active. Skipped certificate request") else: try: - self._relation.request_certificate_creation(event=event) + self._relation.request_certificate_creation() except Exception as e: event.fail(f"Failed to request certificate: {e}") logger.exception( @@ -253,16 +253,16 @@ def _on_set_tls_private_key(self, event: ops.ActionEvent) -> None: raise logger.debug("Handled set TLS private key action") - def _on_tls_relation_created(self, event) -> None: + def _on_tls_relation_created(self, _) -> None: """Request certificate when TLS relation created.""" - self._relation.request_certificate_creation(event=event) + self._relation.request_certificate_creation() def _on_tls_relation_broken(self, _) -> None: """Delete TLS certificate.""" logger.debug("Deleting TLS certificate") for field in _TLS_FIELDS: self._secrets.set_value(secrets.UNIT_SCOPE, field, None) - self._charm.reconcile(event=None) + self._charm.reconcile() logger.debug("Deleted TLS certificate") def _on_certificate_available(self, event: tls_certificates.CertificateAvailableEvent) -> None: @@ -275,4 +275,4 @@ def _on_certificate_expiring(self, event: tls_certificates.CertificateExpiringEv logger.warning("Unknown certificate expiring") return - self._relation.request_certificate_renewal(event=event) + self._relation.request_certificate_renewal() diff --git a/common/common/workload.py b/common/common/workload.py index 20af46c1..5e6b2c94 100644 --- a/common/common/workload.py +++ b/common/common/workload.py @@ -88,7 +88,6 @@ def install( def refresh( self, *, - event, unit: ops.Unit, model_uuid: str, snap_revision: str, @@ -176,7 +175,6 @@ def _disable_router(self) -> None: def reconcile( self, *, - event, tls: bool, unit_name: str, exporter_config: "cos.ExporterConfig", @@ -250,7 +248,7 @@ def _cleanup_after_refresh_or_potential_container_restart(self) -> None: logger.debug("Cleaned up after refresh or container restart") def _get_bootstrap_command( - self, *, event, connection_info: "database_requires.ConnectionInformation" + self, *, connection_info: "database_requires.ConnectionInformation" ) -> list[str]: return [ "--bootstrap", @@ -281,17 +279,17 @@ def _get_bootstrap_command( "destination_status.error_quarantine_interval=5", ] - def _bootstrap_router(self, *, event, tls: bool) -> None: + def _bootstrap_router(self, *, tls: bool) -> None: """Bootstrap MySQL Router.""" logger.debug( f"Bootstrapping router {tls=}, {self._connection_info.host=}, {self._connection_info.port=}" ) # Redact password from log logged_command = self._get_bootstrap_command( - event=event, connection_info=self._connection_info.redacted + connection_info=self._connection_info.redacted ) - command = self._get_bootstrap_command(event=event, connection_info=self._connection_info) + command = self._get_bootstrap_command(connection_info=self._connection_info) try: self._container.run_mysql_router(command) except container.CalledProcessError as e: @@ -341,31 +339,31 @@ def _router_username(self) -> str: """ return self._parse_username_from_config(self._container.router_config_file.read_text()) - def _restart(self, *, event, tls: bool) -> None: + def _restart(self, *, tls: bool) -> None: """Restart MySQL Router to enable or disable TLS.""" logger.debug("Restarting MySQL Router") assert self._container.mysql_router_service_enabled is True self._container.update_mysql_router_service(enabled=True, tls=tls) logger.debug("Restarted MySQL Router") - self._charm.wait_until_mysql_router_ready(event=event) + self._charm.wait_until_mysql_router_ready() # wait_until_mysql_router_ready will set WaitingStatus—override it with current charm # status - self._charm.set_status(event=None) + self._charm.set_status() - def _enable_router(self, *, event, tls: bool, unit_name: str) -> None: + def _enable_router(self, *, tls: bool, unit_name: str) -> None: """Enable router after setting up all the necessary prerequisites.""" logger.info("Enabling MySQL Router service") self._cleanup_after_refresh_or_potential_container_restart() # create an empty credentials file, if the file does not exist self._container.create_router_rest_api_credentials_file() - self._bootstrap_router(event=event, tls=tls) + self._bootstrap_router(tls=tls) self.shell.add_attributes_to_mysql_router_user( username=self._router_username, router_id=self._router_id, unit_name=unit_name ) self._container.update_mysql_router_service(enabled=True, tls=tls) self._logrotate.enable() logger.info("Enabled MySQL Router service") - self._charm.wait_until_mysql_router_ready(event=event) + self._charm.wait_until_mysql_router_ready() def _enable_exporter(self, *, tls: bool, exporter_config: "cos.ExporterConfig") -> None: """Enable the mysqlrouter exporter.""" @@ -384,7 +382,6 @@ def _enable_exporter(self, *, tls: bool, exporter_config: "cos.ExporterConfig") def reconcile( self, *, - event, tls: bool, unit_name: str, exporter_config: "cos.ExporterConfig", @@ -401,9 +398,8 @@ def reconcile( # If the host or port changes, MySQL Router will receive topology change # notifications from MySQL. # Therefore, if the host or port changes, we do not need to restart MySQL Router. - is_charm_exposed = self._charm.is_externally_accessible(event=event) socket_file_exists = self._container.path("/run/mysqlrouter/mysql.sock").exists() - require_rebootstrap = is_charm_exposed == socket_file_exists + require_rebootstrap = self._charm.is_externally_accessible == socket_file_exists if require_rebootstrap: self._disable_router() @@ -414,14 +410,14 @@ def reconcile( key=key, certificate=certificate, certificate_authority=certificate_authority ) if custom_certificate != certificate and self._container.mysql_router_service_enabled: - self._restart(event=event, tls=tls) + self._restart(tls=tls) else: self._disable_tls() if custom_certificate and self._container.mysql_router_service_enabled: - self._restart(event=event, tls=tls) + self._restart(tls=tls) if not self._container.mysql_router_service_enabled: - self._enable_router(event=event, tls=tls, unit_name=unit_name) + self._enable_router(tls=tls, unit_name=unit_name) if (not self._container.mysql_router_exporter_service_enabled and exporter_config) or ( self._container.mysql_router_exporter_service_enabled @@ -448,7 +444,6 @@ def status(self) -> ops.StatusBase | None: def refresh( self, *, - event, unit: ops.Unit, model_uuid: str, snap_revision: str, @@ -465,7 +460,6 @@ def refresh( self._disable_router() try: super().refresh( - event=event, unit=unit, model_uuid=model_uuid, snap_revision=snap_revision, @@ -481,7 +475,7 @@ def refresh( finally: if enabled: logger.debug(message) - self._enable_router(event=event, tls=tls, unit_name=unit.name) + self._enable_router(tls=tls, unit_name=unit.name) if exporter_enabled: self._enable_exporter(tls=tls, exporter_config=exporter_config) diff --git a/common/pyproject.toml b/common/pyproject.toml index 513ba280..6f0dcf23 100644 --- a/common/pyproject.toml +++ b/common/pyproject.toml @@ -3,7 +3,7 @@ name = "common" version = "0.1.0" requires-python = ">=3.10" dependencies = [ - "ops>=2.9.0", + "ops>=2.10.0", "charm-refresh>=3.1.0.2", "charm-api>=0.1.2", "requests>=2.32.5", diff --git a/kubernetes/poetry.lock b/kubernetes/poetry.lock index 5e3f3727..8b32780e 100644 --- a/kubernetes/poetry.lock +++ b/kubernetes/poetry.lock @@ -568,7 +568,7 @@ develop = true charm-api = ">=0.1.2" charm-refresh = ">=3.1.0.2" jinja2 = ">=3.1.6" -ops = ">=2.9.0" +ops = ">=2.10.0" requests = ">=2.32.5" tenacity = ">=9.1.2" @@ -1536,20 +1536,23 @@ files = [ [[package]] name = "ops" -version = "2.9.0" +version = "2.10.0" description = "The Python library behind great charms" optional = false python-versions = ">=3.8" groups = ["main", "charm-libs", "unit"] files = [ - {file = "ops-2.9.0-py3-none-any.whl", hash = "sha256:1d443e4d45e0c2443b8334d37a177287f22a12ee0cb02a30cf7c3159316cb643"}, - {file = "ops-2.9.0.tar.gz", hash = "sha256:d3c541659eded56f42f9c18270408cc6313895968f1360b3f1de75c99cc99ada"}, + {file = "ops-2.10.0-py3-none-any.whl", hash = "sha256:25ecac7054e53b500d1ea6cccf47de4ed69d39de4eaf9a2bc05d49a12f8651bf"}, + {file = "ops-2.10.0.tar.gz", hash = "sha256:7b0bf23c7ae20891d3fd7943830d75f1b38e7cafb216d5b727771d9c2f69d654"}, ] [package.dependencies] PyYAML = "==6.*" websocket-client = "==1.*" +[package.extras] +docs = ["furo", "lxd-sphinx-extensions", "sphinx (==6.2.1)", "sphinx-copybutton", "sphinx-design", "sphinx-tabs"] + [[package]] name = "ops-scenario" version = "6.0.3" @@ -2885,4 +2888,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.1" python-versions = "^3.10" -content-hash = "25139a94df7a8d35d027f943f8b1c1b864dcfdfd474944a1988bf1536939e089" +content-hash = "0f3ad7920de7992343cc56fc6fc95eeaac590fb1743e5311835ccc5951fb8cf6" diff --git a/kubernetes/pyproject.toml b/kubernetes/pyproject.toml index f6bf04ca..0b41578a 100644 --- a/kubernetes/pyproject.toml +++ b/kubernetes/pyproject.toml @@ -8,8 +8,7 @@ requires-poetry = ">=2.0.0" [tool.poetry.dependencies] python = "^3.10" common = {path = "../common", develop = true} -# breaking change in ops 2.10.0: https://github.com/canonical/operator/pull/1091#issuecomment-1888644075 -ops = "^2.9.0, <2.10" +ops = ">=2.10.0,<2.11" lightkube = "^0.17.2" tenacity = "^9.1.2" charm-refresh = "^3.1.0.2" diff --git a/kubernetes/src/charm.py b/kubernetes/src/charm.py index 6c2e37c6..d44a46f8 100755 --- a/kubernetes/src/charm.py +++ b/kubernetes/src/charm.py @@ -138,16 +138,20 @@ def _logrotate(self) -> kubernetes_logrotate.LogRotate: def _cos_relation_type(self) -> type[common.relations.cos.COSRelation]: return relations.kubernetes_cos.COSRelation - def _status(self, *, event) -> ops.StatusBase | None: + @property + def _status(self) -> ops.StatusBase | None: if self.config.get("expose-external", "false") not in [ "false", "nodeport", "loadbalancer", ]: return ops.BlockedStatus("Invalid expose-external config value") - if self._peer_data.get_value( - common.relations.secrets.APP_SCOPE, self._K8S_SERVICE_INITIALIZED_KEY - ) and not self._check_service_connectivity(event=event): + if ( + self._peer_data.get_value( + common.relations.secrets.APP_SCOPE, self._K8S_SERVICE_INITIALIZED_KEY + ) + and not self._check_service_connectivity() + ): if self._peer_data.get_value( common.relations.secrets.APP_SCOPE, self._K8S_SERVICE_CREATING_KEY ): @@ -155,8 +159,10 @@ def _status(self, *, event) -> ops.StatusBase | None: else: return ops.BlockedStatus("K8s service not connectable") - def is_externally_accessible(self, *, event) -> bool | None: + @property + def is_externally_accessible(self) -> bool | None: """No-op since this charm is exposed with the expose-external config.""" + return None def _get_service(self) -> lightkube.resources.core_v1.Service | None: """Get the managed k8s service.""" @@ -262,21 +268,18 @@ def _reconcile_service(self) -> None: logger.info(f"Request to create desired service {desired_service_type=} dispatched") - def _check_service_connectivity(self, *, event) -> bool: + def _check_service_connectivity(self) -> bool: """Check if the service is available (connectable with a socket).""" if not self._get_service() or not isinstance( - self.get_workload(event=None), common.workload.RunningWorkload + self.get_workload(), common.workload.RunningWorkload ): logger.debug("No service or unauthenticated workload") return False - for endpoints in ( - self._read_write_endpoints(event=event), - self._read_only_endpoints(event=event), - ): + for endpoints in (self._read_write_endpoints, self._read_only_endpoints): if endpoints == "": logger.debug( - f"Empty endpoints {self._read_write_endpoints(event=event)=} {self._read_only_endpoints(event=event)=}" + f"Empty endpoints {self._read_write_endpoints=} {self._read_only_endpoints=}" ) return False @@ -299,17 +302,17 @@ def _check_service_connectivity(self, *, event) -> bool: return True - def _reconcile_ports(self, *, event) -> None: + def _reconcile_ports(self) -> None: """Needed for VM, so no-op""" - def _update_endpoints(self, *, event) -> None: - if self._check_service_connectivity(event=event): + def _update_endpoints(self) -> None: + if self._check_service_connectivity(): self._database_provides.update_endpoints( - router_read_write_endpoints=self._read_write_endpoints(event=event), - router_read_only_endpoints=self._read_only_endpoints(event=event), + router_read_write_endpoints=self._read_write_endpoints, + router_read_only_endpoints=self._read_only_endpoints, ) - def wait_until_mysql_router_ready(self, *, event=None) -> None: + def wait_until_mysql_router_ready(self) -> None: logger.debug("Waiting until MySQL Router is ready") self.unit.status = ops.MaintenanceStatus("MySQL Router starting") try: @@ -406,13 +409,16 @@ def _get_hosts_ports(self, port_type: str) -> str: # noqa: C901 return "" - def _read_write_endpoints(self, *, event) -> str: + @property + def _read_write_endpoints(self) -> str: return self._get_hosts_ports("rw") - def _read_only_endpoints(self, *, event) -> str: + @property + def _read_only_endpoints(self) -> str: return self._get_hosts_ports("ro") - def tls_sans_ip(self, *, event) -> list[str] | None: + @property + def tls_sans_ip(self) -> list[str] | None: _, extra_ips = self.get_all_k8s_node_hostnames_and_ips() return [ str(self.model.get_binding("juju-info").network.bind_address), @@ -420,7 +426,8 @@ def tls_sans_ip(self, *, event) -> list[str] | None: *extra_ips, ] - def tls_sans_dns(self, *, event) -> list[str] | None: + @property + def tls_sans_dns(self) -> list[str] | None: service_name = self.service_name unit_name = self.unit.name.replace("/", "-") extra_hosts, _ = self.get_all_k8s_node_hostnames_and_ips() @@ -440,9 +447,7 @@ def tls_sans_dns(self, *, event) -> list[str] | None: *extra_hosts, ] - def get_all_k8s_node_hostnames_and_ips( - self, - ) -> tuple[list[str], list[str]]: + def get_all_k8s_node_hostnames_and_ips(self) -> tuple[list[str], list[str]]: """Return all node hostnames and IPs registered in k8s.""" node = self._get_node(self.unit.name) hostnames = [] diff --git a/machines/poetry.lock b/machines/poetry.lock index 5a6a904f..d532410c 100644 --- a/machines/poetry.lock +++ b/machines/poetry.lock @@ -568,7 +568,7 @@ develop = true charm-api = ">=0.1.2" charm-refresh = ">=3.1.0.2" jinja2 = ">=3.1.6" -ops = ">=2.9.0" +ops = ">=2.10.0" requests = ">=2.32.5" tenacity = ">=9.1.2" @@ -1536,20 +1536,23 @@ files = [ [[package]] name = "ops" -version = "2.9.0" +version = "2.10.0" description = "The Python library behind great charms" optional = false python-versions = ">=3.8" groups = ["main", "charm-libs", "integration", "unit"] files = [ - {file = "ops-2.9.0-py3-none-any.whl", hash = "sha256:1d443e4d45e0c2443b8334d37a177287f22a12ee0cb02a30cf7c3159316cb643"}, - {file = "ops-2.9.0.tar.gz", hash = "sha256:d3c541659eded56f42f9c18270408cc6313895968f1360b3f1de75c99cc99ada"}, + {file = "ops-2.10.0-py3-none-any.whl", hash = "sha256:25ecac7054e53b500d1ea6cccf47de4ed69d39de4eaf9a2bc05d49a12f8651bf"}, + {file = "ops-2.10.0.tar.gz", hash = "sha256:7b0bf23c7ae20891d3fd7943830d75f1b38e7cafb216d5b727771d9c2f69d654"}, ] [package.dependencies] PyYAML = "==6.*" websocket-client = "==1.*" +[package.extras] +docs = ["furo", "lxd-sphinx-extensions", "sphinx (==6.2.1)", "sphinx-copybutton", "sphinx-design", "sphinx-tabs"] + [[package]] name = "ops-scenario" version = "6.0.3" @@ -2868,4 +2871,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.1" python-versions = "^3.10" -content-hash = "dd49f88f78b722bea2beec95b4a8c188308ae218ea23c5d716b7c703d103eff4" +content-hash = "603a188ac1f406d4aba5c1f8af26fc69d531a25e5a436a428fae3c0b6bfbddd9" diff --git a/machines/pyproject.toml b/machines/pyproject.toml index 9367c4b2..d985044a 100644 --- a/machines/pyproject.toml +++ b/machines/pyproject.toml @@ -8,8 +8,7 @@ requires-poetry = ">=2.0.0" [tool.poetry.dependencies] python = "^3.10" common = {path = "../common", develop = true} -# breaking change in ops 2.10.0: https://github.com/canonical/operator/pull/1091#issuecomment-1888644075 -ops = "^2.9.0, <2.10" +ops = ">=2.10.0,<2.11" tenacity = "^9.1.2" charm-refresh = "^3.1.0.2" diff --git a/machines/src/charm.py b/machines/src/charm.py index 90383d90..9d0ae3e0 100755 --- a/machines/src/charm.py +++ b/machines/src/charm.py @@ -49,15 +49,13 @@ class _MachinesRouterRefresh( def refresh_snap( self, *, snap_name: str, snap_revision: str, refresh: charm_refresh.Machines ) -> None: - # TODO: issue on relation-broken event since event not passed? mitigated by regular event handler? - self._charm.get_workload(event=None, refresh=refresh).refresh( - event=None, + self._charm.get_workload(refresh=refresh).refresh( unit=self._charm.unit, model_uuid=self._charm.model.uuid, snap_revision=snap_revision, refresh=refresh, tls=self._charm._tls_certificate_saved, - exporter_config=self._charm._cos_exporter_config(event=None), + exporter_config=self._charm._cos_exporter_config, ) # `reconcile()` will run on every event, which will set # `refresh.next_unit_allowed_to_refresh = True` @@ -125,8 +123,9 @@ def _subordinate_relation_endpoint_names(self) -> collections.abc.Collection[str def _container(self) -> snap.Snap: return snap.Snap(unit_name=self.unit.name) - def _status(self, *, event) -> ops.StatusBase | None: - pass + @property + def _status(self) -> ops.StatusBase | None: + return None @property def _logrotate(self) -> machine_logrotate.LogRotate: @@ -136,20 +135,22 @@ def _logrotate(self) -> machine_logrotate.LogRotate: def _cos_relation_type(self) -> type[common.relations.cos.COSRelation]: return relations.machines_cos.COSRelation - def tls_sans_ip(self, *, event) -> list[str] | None: + @property + def tls_sans_ip(self) -> list[str] | None: sans_ip = ["127.0.0.1"] # needed for the HTTP server when related with COS - if self.is_externally_accessible(event=event): + if self.is_externally_accessible: sans_ip.append(self.host_address) return sans_ip - def tls_sans_dns(self, *, event) -> list[str] | None: + @property + def tls_sans_dns(self) -> list[str] | None: return None @property def host_address(self) -> str: """The host address for the machine.""" if ( - not self.is_externally_accessible(event=None) + not self.is_externally_accessible or not self.config.get("vip") or (self._ha_cluster and not self._ha_cluster.is_clustered()) ): @@ -157,43 +158,46 @@ def host_address(self) -> str: return self.config["vip"] - def _read_write_endpoints(self, *, event) -> str: - if self.is_externally_accessible(event=event): + @property + def _read_write_endpoints(self) -> str: + if self.is_externally_accessible: return f"{self.host_address}:{self._READ_WRITE_PORT}" else: return f"file://{self._container.path('/run/mysqlrouter/mysql.sock')}" - def _read_only_endpoints(self, *, event) -> str: - if self.is_externally_accessible(event=event): + @property + def _read_only_endpoints(self) -> str: + if self.is_externally_accessible: return f"{self.host_address}:{self._READ_ONLY_PORT}" else: return f"file://{self._container.path('/run/mysqlrouter/mysqlro.sock')}" - def is_externally_accessible(self, *, event) -> bool | None: - return self._database_provides.external_connectivity(event) + @property + def is_externally_accessible(self) -> bool | None: + return self._database_provides.external_connectivity def _reconcile_service(self) -> None: """Only applies to Kubernetes charm, so no-op.""" pass - def _reconcile_ports(self, *, event) -> None: - if self.is_externally_accessible(event=event): + def _reconcile_ports(self) -> None: + if self.is_externally_accessible: ports = [self._READ_WRITE_PORT, self._READ_ONLY_PORT] else: ports = [] self.unit.set_ports(*ports) - def _update_endpoints(self, *, event) -> None: + def _update_endpoints(self) -> None: self._database_provides.update_endpoints( - router_read_write_endpoints=self._read_write_endpoints(event=event), - router_read_only_endpoints=self._read_only_endpoints(event=event), + router_read_write_endpoints=self._read_write_endpoints, + router_read_only_endpoints=self._read_only_endpoints, ) def _wait_until_service_reconciled(self) -> None: """Only applies to Kubernetes charm, so no-op.""" pass - def wait_until_mysql_router_ready(self, *, event) -> None: + def wait_until_mysql_router_ready(self) -> None: logger.debug("Waiting until MySQL Router is ready") self.unit.status = ops.MaintenanceStatus("MySQL Router starting") try: @@ -203,7 +207,7 @@ def wait_until_mysql_router_ready(self, *, event) -> None: wait=tenacity.wait_fixed(5), ): with attempt: - if self.is_externally_accessible(event=event): + if self.is_externally_accessible: for port in ( self._READ_WRITE_PORT, self._READ_ONLY_PORT, diff --git a/machines/src/machine_workload.py b/machines/src/machine_workload.py index 1b3fffc9..f0b6a1eb 100644 --- a/machines/src/machine_workload.py +++ b/machines/src/machine_workload.py @@ -21,10 +21,10 @@ class RunningMachineWorkload(common.workload.RunningWorkload): """Workload with connection to MySQL cluster and with Unix sockets enabled""" def _get_bootstrap_command( - self, *, event, connection_info: "common.relations.database_requires.ConnectionInformation" + self, *, connection_info: "common.relations.database_requires.ConnectionInformation" ) -> list[str]: - command = super()._get_bootstrap_command(event=event, connection_info=connection_info) - if self._charm.is_externally_accessible(event=event): + command = super()._get_bootstrap_command(connection_info=connection_info) + if self._charm.is_externally_accessible: command.extend([ "--conf-bind-address", "0.0.0.0", @@ -65,7 +65,7 @@ def _update_configured_socket_file_locations(self) -> None: self._container.router_config_file.write_text(output.getvalue()) logger.debug("Updated configured socket file locations") - def _bootstrap_router(self, *, event, tls: bool) -> None: - super()._bootstrap_router(event=event, tls=tls) - if not self._charm.is_externally_accessible(event=event): + def _bootstrap_router(self, *, tls: bool) -> None: + super()._bootstrap_router(tls=tls) + if not self._charm.is_externally_accessible: self._update_configured_socket_file_locations() diff --git a/machines/src/relations/database_providers_wrapper.py b/machines/src/relations/database_providers_wrapper.py index 32be3d28..734ffacf 100644 --- a/machines/src/relations/database_providers_wrapper.py +++ b/machines/src/relations/database_providers_wrapper.py @@ -37,15 +37,13 @@ def __init__( charm_ ) - def external_connectivity(self, event) -> bool: + @property + def external_connectivity(self) -> bool: """Whether any of the relations are marked as external.""" - return self._database_provides.external_connectivity(event) + return self._database_provides.external_connectivity def update_endpoints( - self, - *, - router_read_write_endpoints: str, - router_read_only_endpoints: str, + self, *, router_read_write_endpoints: str, router_read_only_endpoints: str ) -> None: """Update the endpoints in the provides relationship databags.""" self._database_provides.update_endpoints( @@ -56,7 +54,6 @@ def update_endpoints( def reconcile_users( self, *, - event, router_read_write_endpoints: str, router_read_only_endpoints: str, shell: common.mysql_shell.Shell, @@ -68,12 +65,11 @@ def reconcile_users( relation is broken. """ self._database_provides.reconcile_users( - event=event, router_read_write_endpoints=router_read_write_endpoints, router_read_only_endpoints=router_read_only_endpoints, shell=shell, ) - self._deprecated_shared_db.reconcile_users(event=event, shell=shell) + self._deprecated_shared_db.reconcile_users(shell=shell) def delete_all_databags(self) -> None: """Remove connection information from all databags. @@ -86,10 +82,11 @@ def delete_all_databags(self) -> None: self._database_provides.delete_all_databags() self._deprecated_shared_db.delete_all_databags() - def get_status(self, event) -> ops.StatusBase | None: + @property + def status(self) -> ops.StatusBase | None: """Report non-active status.""" - database_provides_status = self._database_provides.get_status(event) - deprecated_shared_db_status = self._deprecated_shared_db.get_status(event) + database_provides_status = self._database_provides.status + deprecated_shared_db_status = self._deprecated_shared_db.status if ( isinstance(deprecated_shared_db_status, ops.ActiveStatus) and isinstance(database_provides_status, ops.BlockedStatus) diff --git a/machines/src/relations/deprecated_shared_db_database_provides.py b/machines/src/relations/deprecated_shared_db_database_provides.py index 42c62ee0..76f1f6c0 100644 --- a/machines/src/relations/deprecated_shared_db_database_provides.py +++ b/machines/src/relations/deprecated_shared_db_database_provides.py @@ -9,6 +9,7 @@ import logging import typing +import charm_ as charm import common.mysql_shell import common.relations.remote_databag as remote_databag import common.status_exception @@ -56,10 +57,7 @@ class _Relation: """Relation to one application charm""" def __init__( - self, - *, - relation: ops.Relation, - peer_relation_app_databag: ops.RelationDataContent, + self, *, relation: ops.Relation, peer_relation_app_databag: ops.RelationDataContent ) -> None: self._id = relation.id self._peer_app_databag = peer_relation_app_databag @@ -88,11 +86,7 @@ def __init__( assert len(relation.units) == 1 self._remote_unit_name = relation.units.copy().pop().name - def set_databag( - self, - *, - password: str, - ) -> None: + def set_databag(self, *, password: str) -> None: """Share connection information with application charm.""" logger.debug(f"Setting unit databag {self._id=} {self._remote_unit_name=}") self._local_unit_databag["allowed_units"] = self._remote_unit_name @@ -121,9 +115,8 @@ def __init__( relation: ops.Relation, peer_relation_app_databag: ops.RelationDataContent, unit: ops.Unit, - event, ) -> None: - if isinstance(event, ops.RelationBrokenEvent) and event.relation.id == relation.id: + if not relation.active: raise _RelationBreaking super().__init__( relation=relation, unit=unit, peer_relation_app_databag=peer_relation_app_databag @@ -133,11 +126,7 @@ def __init__( self._peer_databag_username_key, self._remote_unit_databag["username"] ) - def create_database_and_user( - self, - *, - shell: common.mysql_shell.Shell, - ) -> None: + def create_database_and_user(self, *, shell: common.mysql_shell.Shell) -> None: """Create database & user and update databag.""" # Delete user if exists # (If the user was previously created by this charm—but the hook failed—the user will @@ -148,8 +137,7 @@ def create_database_and_user( logger.debug("Deleted user if exists before creating user") password = shell.create_application_database( - database=self._database, - username=self._username, + database=self._database, username=self._username ) self._peer_app_databag[self.peer_databag_password_key] = password self.set_databag(password=password) @@ -163,10 +151,7 @@ class _RelationWithSharedUser(_Relation): """Related application charm that has been provided with a database & user""" def __init__( - self, - *, - relation: ops.Relation, - peer_relation_app_databag: ops.RelationDataContent, + self, *, relation: ops.Relation, peer_relation_app_databag: ops.RelationDataContent ) -> None: super().__init__(relation=relation, peer_relation_app_databag=peer_relation_app_databag) for key in (self._peer_databag_username_key, self.peer_databag_password_key): @@ -200,7 +185,24 @@ class RelationEndpoint(ops.Object): def __init__(self, charm_: "common.abstract_charm.MySQLRouterCharm") -> None: super().__init__(charm_, self._NAME) + self._relations = charm_.model.relations[self._NAME] + # Needed because of breaking change in ops 2.10 + # https://github.com/canonical/operator/pull/1091 + # Breaking relations included to clean up users during relation-broken event + # Recommended approach from Charm Tech team: + # https://github.com/canonical/operator/issues/1279#issuecomment-2921130420 + # Use Juju event (`charm.event`) instead of ops event so that breaking relation is included + # in ops deferred or custom events, as recommended by Charm Tech team + if isinstance( + charm.event, charm.RelationBrokenEvent + ) and charm.event.endpoint == charm.Endpoint(self._NAME): + self._relations.append( + charm_.model.get_relation( + relation_name=self._NAME, relation_id=charm.event.relation.id + ) + ) + if self._relations: logger.warning( "'mysql-shared' relation interface is DEPRECATED and will be removed in a future release. Use 'mysql_client' interface instead." @@ -255,27 +257,21 @@ def _shared_users(self) -> list[_RelationWithSharedUser]: try: shared_users.append( _RelationWithSharedUser( - relation=relation, - peer_relation_app_databag=self._peer_app_databag, + relation=relation, peer_relation_app_databag=self._peer_app_databag ) ) except _UserNotShared: pass return shared_users - def reconcile_users( - self, - *, - event, - shell: common.mysql_shell.Shell, - ) -> None: + def reconcile_users(self, *, shell: common.mysql_shell.Shell) -> None: """Create requested users and delete inactive users. When the relation to the MySQL charm is broken, the MySQL charm will delete all users created by this charm. Therefore, this charm does not need to delete users when that relation is broken. """ - logger.debug(f"Reconciling users {event=}") + logger.debug("Reconciling users") requested_users = [] for relation in self._relations: try: @@ -284,24 +280,18 @@ def reconcile_users( relation=relation, unit=self._charm.unit, peer_relation_app_databag=self._peer_app_databag, - event=event, ) ) - except ( - _RelationBreaking, - remote_databag.IncompleteDatabag, - ): + except (_RelationBreaking, remote_databag.IncompleteDatabag): pass logger.debug(f"State of reconcile users {requested_users=}, {self._shared_users=}") for relation in requested_users: if relation not in self._shared_users: - relation.create_database_and_user( - shell=shell, - ) + relation.create_database_and_user(shell=shell) for relation in self._shared_users: if relation not in requested_users: relation.delete_user(shell=shell) - logger.debug(f"Reconciled users {event=}") + logger.debug("Reconciled users") def delete_all_databags(self) -> None: """Remove connection information from all databags. @@ -317,7 +307,8 @@ def delete_all_databags(self) -> None: relation.delete_databag() logger.debug("Deleted all application databags") - def get_status(self, event) -> ops.StatusBase | None: + @property + def status(self) -> ops.StatusBase | None: """Report non-active status.""" requested_users = [] exception_reporting_priority = (remote_databag.IncompleteDatabag,) @@ -329,7 +320,6 @@ def get_status(self, event) -> ops.StatusBase | None: relation=relation, unit=self._charm.unit, peer_relation_app_databag=self._peer_app_databag, - event=event, ) ) except _RelationBreaking: diff --git a/machines/src/relations/hacluster.py b/machines/src/relations/hacluster.py index f1f6e7b4..e82121c8 100644 --- a/machines/src/relations/hacluster.py +++ b/machines/src/relations/hacluster.py @@ -4,11 +4,15 @@ import json import logging +import typing from hashlib import shake_128 from ipaddress import IPv4Address, ip_address import ops +if typing.TYPE_CHECKING: + import common.abstract_charm + HACLUSTER_RELATION_NAME = "ha" logger = logging.getLogger(__name__) @@ -17,7 +21,7 @@ class HACluster(ops.Object): """Defines hacluster functionality.""" - def __init__(self, charm: ops.CharmBase): + def __init__(self, charm: "common.abstract_charm.MySQLRouterCharm"): super().__init__(charm, HACLUSTER_RELATION_NAME) self.charm = charm @@ -41,18 +45,20 @@ def is_clustered(self) -> bool: return True return False - def get_unit_juju_status(self) -> ops.StatusBase: + def get_unit_juju_status(self) -> ops.StatusBase | None: """Returns the status of the hacluster if relation exists.""" - if self.relation and not self.charm.is_externally_accessible(event=None): + if self.relation and not self.charm.is_externally_accessible: return ops.BlockedStatus("ha integration used without data-integrator") vip = self.charm.config.get("vip") if self.relation and not vip: return ops.BlockedStatus("ha integration used without vip configuration") - if vip and not self.charm.is_externally_accessible(event=None): + if vip and not self.charm.is_externally_accessible: return ops.BlockedStatus("vip configuration without data-integrator") + return None + def set_vip(self, vip: str | None) -> None: """Adds the requested virtual IP to the integration.""" if not self.relation: From 6ab80d1bf77207e1fdbb9f52fc54a0c00c71992e Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Mon, 27 Oct 2025 14:22:05 +0100 Subject: [PATCH 2/4] Fix unit tests --- kubernetes/tests/unit/conftest.py | 14 ++++++++++++++ machines/tests/unit/conftest.py | 15 ++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/kubernetes/tests/unit/conftest.py b/kubernetes/tests/unit/conftest.py index d89e36a9..cef08d80 100644 --- a/kubernetes/tests/unit/conftest.py +++ b/kubernetes/tests/unit/conftest.py @@ -1,5 +1,6 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. +import os import pytest from charms.tempo_coordinator_k8s.v0.charm_tracing import charm_tracing_disabled @@ -66,6 +67,19 @@ def patch(monkeypatch): lambda *args, **kwargs: True, ) + # Can be removed when ops-scenario is updated to 8.3.0/ops[testing] 3.3.0 + # (https://github.com/canonical/operator/pull/1996) + original_getitem = os.environ.__getitem__ + + def getitem(self, key): + if key == "JUJU_HOOK_NAME": + if dispatch_path := self.get("JUJU_DISPATCH_PATH"): + _, hook_name = dispatch_path.split("/") + return hook_name + return original_getitem(key) + + monkeypatch.setattr("os._Environ.__getitem__", getitem) + @pytest.fixture(autouse=True) def kubernetes_patch(monkeypatch): diff --git a/machines/tests/unit/conftest.py b/machines/tests/unit/conftest.py index ae8745a0..c4af9344 100644 --- a/machines/tests/unit/conftest.py +++ b/machines/tests/unit/conftest.py @@ -1,6 +1,6 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. - +import os import pathlib import platform @@ -80,6 +80,19 @@ def patch(monkeypatch): lambda *args, **kwargs: True, ) + # Can be removed when ops-scenario is updated to 8.3.0/ops[testing] 3.3.0 + # (https://github.com/canonical/operator/pull/1996) + original_getitem = os.environ.__getitem__ + + def getitem(self, key): + if key == "JUJU_HOOK_NAME": + if dispatch_path := self.get("JUJU_DISPATCH_PATH"): + _, hook_name = dispatch_path.split("/") + return hook_name + return original_getitem(key) + + monkeypatch.setattr("os._Environ.__getitem__", getitem) + # flake8: noqa: C901 @pytest.fixture(autouse=True) From e68bba4b29f76e516a1138a6ae4c755a2d3d2691 Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Mon, 27 Oct 2025 14:43:46 +0100 Subject: [PATCH 3/4] Fix unit tests --- kubernetes/tests/unit/conftest.py | 2 +- machines/tests/unit/conftest.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/kubernetes/tests/unit/conftest.py b/kubernetes/tests/unit/conftest.py index cef08d80..18e05884 100644 --- a/kubernetes/tests/unit/conftest.py +++ b/kubernetes/tests/unit/conftest.py @@ -75,7 +75,7 @@ def getitem(self, key): if key == "JUJU_HOOK_NAME": if dispatch_path := self.get("JUJU_DISPATCH_PATH"): _, hook_name = dispatch_path.split("/") - return hook_name + return hook_name.replace("_", "-") return original_getitem(key) monkeypatch.setattr("os._Environ.__getitem__", getitem) diff --git a/machines/tests/unit/conftest.py b/machines/tests/unit/conftest.py index c4af9344..f5d1d624 100644 --- a/machines/tests/unit/conftest.py +++ b/machines/tests/unit/conftest.py @@ -88,7 +88,7 @@ def getitem(self, key): if key == "JUJU_HOOK_NAME": if dispatch_path := self.get("JUJU_DISPATCH_PATH"): _, hook_name = dispatch_path.split("/") - return hook_name + return hook_name.replace("_", "-") return original_getitem(key) monkeypatch.setattr("os._Environ.__getitem__", getitem) From 63eae15ee50621fed792c70dc47e1ee597b08231 Mon Sep 17 00:00:00 2001 From: Carl Csaposs Date: Mon, 27 Oct 2025 15:09:13 +0100 Subject: [PATCH 4/4] Fix unit tests --- kubernetes/tests/unit/conftest.py | 2 ++ machines/tests/unit/conftest.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/kubernetes/tests/unit/conftest.py b/kubernetes/tests/unit/conftest.py index 18e05884..91f41801 100644 --- a/kubernetes/tests/unit/conftest.py +++ b/kubernetes/tests/unit/conftest.py @@ -80,6 +80,8 @@ def getitem(self, key): monkeypatch.setattr("os._Environ.__getitem__", getitem) + monkeypatch.setattr("charm_._main.Relation._other_app", lambda: os.environ["JUJU_REMOTE_APP"]) + @pytest.fixture(autouse=True) def kubernetes_patch(monkeypatch): diff --git a/machines/tests/unit/conftest.py b/machines/tests/unit/conftest.py index f5d1d624..271c7b2d 100644 --- a/machines/tests/unit/conftest.py +++ b/machines/tests/unit/conftest.py @@ -93,6 +93,8 @@ def getitem(self, key): monkeypatch.setattr("os._Environ.__getitem__", getitem) + monkeypatch.setattr("charm_._main.Relation._other_app", lambda: os.environ["JUJU_REMOTE_APP"]) + # flake8: noqa: C901 @pytest.fixture(autouse=True)