Skip to content

Commit

Permalink
Standardize abstract_charm.py file with the K8s charm (#182)
Browse files Browse the repository at this point in the history
## Issue
The `abstract_charm.py` has diverged with the K8s charm following in
support of HACluster charm integration + `expose-external` config in the
K8s charm.

## Solution
Standardize the files

Counterpart PR in K8s charm:
canonical/mysql-router-k8s-operator#328
  • Loading branch information
shayancanonical authored Dec 9, 2024
1 parent d49eda8 commit d0af047
Show file tree
Hide file tree
Showing 10 changed files with 141 additions and 85 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ jobs:

build:
name: Build charm
uses: canonical/data-platform-workflows/.github/workflows/[email protected].0
uses: canonical/data-platform-workflows/.github/workflows/[email protected].1
with:
# Use of cache blocked by https://github.com/canonical/charmcraft/issues/1456
# Details: https://github.com/canonical/charmcraftcache/issues/3
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ jobs:

build:
name: Build charm
uses: canonical/data-platform-workflows/.github/workflows/[email protected].0
uses: canonical/data-platform-workflows/.github/workflows/[email protected].1

release:
name: Release charm
needs:
- ci-tests
- build
uses: canonical/data-platform-workflows/.github/workflows/[email protected].0
uses: canonical/data-platform-workflows/.github/workflows/[email protected].1
with:
channel: dpe/edge
artifact-prefix: ${{ needs.build.outputs.artifact-prefix }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/sync_docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ on:
jobs:
sync-docs:
name: Sync docs from Discourse
uses: canonical/data-platform-workflows/.github/workflows/[email protected].0
uses: canonical/data-platform-workflows/.github/workflows/[email protected].1
with:
reviewers: a-velasco
permissions:
Expand Down
21 changes: 20 additions & 1 deletion lib/charms/data_platform_libs/v0/data_interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ def _on_topic_requested(self, event: TopicRequestedEvent):

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 39
LIBPATCH = 40

PYDEPS = ["ops>=2.0.0"]

Expand Down Expand Up @@ -391,6 +391,10 @@ class IllegalOperationError(DataInterfacesError):
"""To be used when an operation is not allowed to be performed."""


class PrematureDataAccessError(DataInterfacesError):
"""To be raised when the Relation Data may be accessed (written) before protocol init complete."""


##############################################################################
# Global helpers / utilities
##############################################################################
Expand Down Expand Up @@ -1453,6 +1457,8 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None:
class ProviderData(Data):
"""Base provides-side of the data products relation."""

RESOURCE_FIELD = "database"

def __init__(
self,
model: Model,
Expand Down Expand Up @@ -1618,6 +1624,15 @@ def _fetch_my_specific_relation_data(
def _update_relation_data(self, relation: Relation, data: Dict[str, str]) -> None:
"""Set values for fields not caring whether it's a secret or not."""
req_secret_fields = []

keys = set(data.keys())
if self.fetch_relation_field(relation.id, self.RESOURCE_FIELD) is None and (
keys - {"endpoints", "read-only-endpoints", "replset"}
):
raise PrematureDataAccessError(
"Premature access to relation data, update is forbidden before the connection is initialized."
)

if relation.app:
req_secret_fields = get_encoded_list(relation, relation.app, REQ_SECRET_FIELDS)

Expand Down Expand Up @@ -3290,6 +3305,8 @@ class KafkaRequiresEvents(CharmEvents):
class KafkaProviderData(ProviderData):
"""Provider-side of the Kafka relation."""

RESOURCE_FIELD = "topic"

def __init__(self, model: Model, relation_name: str) -> None:
super().__init__(model, relation_name)

Expand Down Expand Up @@ -3539,6 +3556,8 @@ class OpenSearchRequiresEvents(CharmEvents):
class OpenSearchProvidesData(ProviderData):
"""Provider-side of the OpenSearch relation."""

RESOURCE_FIELD = "index"

def __init__(self, model: Model, relation_name: str) -> None:
super().__init__(model, relation_name)

Expand Down
5 changes: 4 additions & 1 deletion lib/charms/tempo_coordinator_k8s/v0/charm_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ def my_tracing_endpoint(self) -> Optional[str]:
- every event as a span (including custom events)
- every charm method call (except dunders) as a span
We recommend that you scale up your tracing provider and relate it to an ingress so that your tracing requests
go through the ingress and get load balanced across all units. Otherwise, if the provider's leader goes down, your tracing goes down.
## TLS support
If your charm integrates with a TLS provider which is also trusted by the tracing provider (the Tempo charm),
Expand Down Expand Up @@ -269,7 +272,7 @@ def _remove_stale_otel_sdk_packages():
# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version

LIBPATCH = 2
LIBPATCH = 3

PYDEPS = ["opentelemetry-exporter-otlp-proto-http==1.21.0"]

Expand Down
9 changes: 6 additions & 3 deletions lib/charms/tempo_coordinator_k8s/v0/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def __init__(self, *args):
`TracingEndpointRequirer.request_protocols(*protocol:str, relation:Optional[Relation])` method.
Using this method also allows you to use per-relation protocols.
Units of provider charms obtain the tempo endpoint to which they will push their traces by calling
Units of requirer charms obtain the tempo endpoint to which they will push their traces by calling
`TracingEndpointRequirer.get_endpoint(protocol: str)`, where `protocol` is, for example:
- `otlp_grpc`
- `otlp_http`
Expand All @@ -44,7 +44,10 @@ def __init__(self, *args):
If the `protocol` is not in the list of protocols that the charm requested at endpoint set-up time,
the library will raise an error.
## Requirer Library Usage
We recommend that you scale up your tracing provider and relate it to an ingress so that your tracing requests
go through the ingress and get load balanced across all units. Otherwise, if the provider's leader goes down, your tracing goes down.
## Provider Library Usage
The `TracingEndpointProvider` object may be used by charms to manage relations with their
trace sources. For this purposes a Tempo-like charm needs to do two things
Expand Down Expand Up @@ -107,7 +110,7 @@ def __init__(self, *args):

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 2
LIBPATCH = 3

PYDEPS = ["pydantic"]

Expand Down
53 changes: 32 additions & 21 deletions src/abstract_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,23 +100,29 @@ def _logrotate(self) -> logrotate.LogRotate:

@property
@abc.abstractmethod
def _read_write_endpoint(self) -> str:
def _read_write_endpoints(self) -> str:
"""MySQL Router read-write endpoint"""

@property
@abc.abstractmethod
def _read_only_endpoint(self) -> str:
def _read_only_endpoints(self) -> str:
"""MySQL Router read-only endpoint"""

@property
@abc.abstractmethod
def _exposed_read_write_endpoint(self) -> str:
"""The exposed read-write endpoint"""
def _exposed_read_write_endpoints(self) -> typing.Optional[str]:
"""The exposed read-write endpoint.
Only defined in vm charm.
"""

@property
@abc.abstractmethod
def _exposed_read_only_endpoint(self) -> str:
"""The exposed read-only endpoint"""
def _exposed_read_only_endpoints(self) -> typing.Optional[str]:
"""The exposed read-only endpoint.
Only defined in vm charm.
"""

@abc.abstractmethod
def is_externally_accessible(self, *, event) -> typing.Optional[bool]:
Expand All @@ -125,6 +131,11 @@ def is_externally_accessible(self, *, event) -> typing.Optional[bool]:
Only defined in vm charm to return True/False. In k8s charm, returns None.
"""

@property
@abc.abstractmethod
def _status(self) -> ops.StatusBase:
"""Status of the charm."""

@property
def _tls_certificate_saved(self) -> bool:
"""Whether a TLS certificate is available to use"""
Expand Down Expand Up @@ -202,6 +213,8 @@ def _determine_app_status(self, *, event) -> ops.StatusBase:
# (Relations should not be modified during upgrade.)
return upgrade_status
statuses = []
if self._status:
statuses.append(self._status)
for endpoint in (self._database_requires, self._database_provides):
if status := endpoint.get_status(event):
statuses.append(status)
Expand Down Expand Up @@ -236,8 +249,8 @@ def wait_until_mysql_router_ready(self, *, event) -> None:
"""

@abc.abstractmethod
def _reconcile_node_port(self, *, event) -> None:
"""Reconcile node port.
def _reconcile_service(self) -> None:
"""Reconcile service.
Only applies to Kubernetes charm
"""
Expand All @@ -249,6 +262,10 @@ def _reconcile_ports(self, *, event) -> None:
Only applies to Machine charm
"""

@abc.abstractmethod
def _update_endpoints(self) -> None:
"""Update the endpoints in the provider relation if necessary."""

# =======================
# Handlers
# =======================
Expand Down Expand Up @@ -332,23 +349,17 @@ def reconcile(self, event=None) -> None: # noqa: C901
and isinstance(workload_, workload.AuthenticatedWorkload)
and workload_.container_ready
):
self._reconcile_node_port(event=event)
self._reconcile_service()
self._database_provides.reconcile_users(
event=event,
router_read_write_endpoint=self._read_write_endpoint,
router_read_only_endpoint=self._read_only_endpoint,
exposed_read_write_endpoint=self._exposed_read_write_endpoint,
exposed_read_only_endpoint=self._exposed_read_only_endpoint,
router_read_write_endpoints=self._read_write_endpoints,
router_read_only_endpoints=self._read_only_endpoints,
exposed_read_write_endpoints=self._exposed_read_write_endpoints,
exposed_read_only_endpoints=self._exposed_read_only_endpoints,
shell=workload_.shell,
)
# _ha_cluster only assigned a value in machine charms
if self._ha_cluster:
self._database_provides.update_endpoints(
router_read_write_endpoint=self._read_write_endpoint,
router_read_only_endpoint=self._read_only_endpoint,
exposed_read_write_endpoint=self._exposed_read_write_endpoint,
exposed_read_only_endpoint=self._exposed_read_only_endpoint,
)
self._update_endpoints()

if workload_.container_ready:
workload_.reconcile(
event=event,
Expand Down
26 changes: 21 additions & 5 deletions src/machine_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ def _upgrade(self) -> typing.Optional[machine_upgrade.Upgrade]:
except upgrade.PeerRelationNotReady:
pass

@property
def _status(self) -> ops.StatusBase:
pass

@property
def _logrotate(self) -> machine_logrotate.LogRotate:
return machine_logrotate.LogRotate(container_=self._container)
Expand All @@ -95,25 +99,25 @@ def host_address(self) -> str:
return str(self.model.get_binding("juju-info").network.bind_address)

@property
def _read_write_endpoint(self) -> str:
def _read_write_endpoints(self) -> str:
return f'file://{self._container.path("/run/mysqlrouter/mysql.sock")}'

@property
def _read_only_endpoint(self) -> str:
def _read_only_endpoints(self) -> str:
return f'file://{self._container.path("/run/mysqlrouter/mysqlro.sock")}'

@property
def _exposed_read_write_endpoint(self) -> str:
def _exposed_read_write_endpoints(self) -> typing.Optional[str]:
return f"{self.host_address}:{self._READ_WRITE_PORT}"

@property
def _exposed_read_only_endpoint(self) -> str:
def _exposed_read_only_endpoints(self) -> typing.Optional[str]:
return f"{self.host_address}:{self._READ_ONLY_PORT}"

def is_externally_accessible(self, *, event) -> typing.Optional[bool]:
return self._database_provides.external_connectivity(event)

def _reconcile_node_port(self, *, event) -> None:
def _reconcile_service(self) -> None:
"""Only applies to Kubernetes charm, so no-op."""
pass

Expand All @@ -124,6 +128,18 @@ def _reconcile_ports(self, *, event) -> None:
ports = []
self.unit.set_ports(*ports)

def _update_endpoints(self) -> None:
self._database_provides.update_endpoints(
router_read_write_endpoints=self._read_write_endpoints,
router_read_only_endpoints=self._read_only_endpoints,
exposed_read_write_endpoints=self._exposed_read_write_endpoints,
exposed_read_only_endpoints=self._exposed_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:
logger.debug("Waiting until MySQL Router is ready")
self.unit.status = ops.MaintenanceStatus("MySQL Router starting")
Expand Down
32 changes: 16 additions & 16 deletions src/relations/database_providers_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,27 +45,27 @@ def external_connectivity(self, event) -> bool:
def update_endpoints(
self,
*,
router_read_write_endpoint: str,
router_read_only_endpoint: str,
exposed_read_write_endpoint: str,
exposed_read_only_endpoint: str,
router_read_write_endpoints: str,
router_read_only_endpoints: str,
exposed_read_write_endpoints: str,
exposed_read_only_endpoints: str,
) -> None:
"""Update the endpoints in the provides relationship databags."""
self._database_provides.update_endpoints(
router_read_write_endpoint=router_read_write_endpoint,
router_read_only_endpoint=router_read_only_endpoint,
exposed_read_write_endpoint=exposed_read_write_endpoint,
exposed_read_only_endpoint=exposed_read_only_endpoint,
router_read_write_endpoints=router_read_write_endpoints,
router_read_only_endpoints=router_read_only_endpoints,
exposed_read_write_endpoints=exposed_read_write_endpoints,
exposed_read_only_endpoints=exposed_read_only_endpoints,
)

def reconcile_users(
self,
*,
event,
router_read_write_endpoint: str,
router_read_only_endpoint: str,
exposed_read_write_endpoint: str,
exposed_read_only_endpoint: str,
router_read_write_endpoints: str,
router_read_only_endpoints: str,
exposed_read_write_endpoints: str,
exposed_read_only_endpoints: str,
shell: mysql_shell.Shell,
) -> None:
"""Create requested users and delete inactive users.
Expand All @@ -76,10 +76,10 @@ def reconcile_users(
"""
self._database_provides.reconcile_users(
event=event,
router_read_write_endpoint=router_read_write_endpoint,
router_read_only_endpoint=router_read_only_endpoint,
exposed_read_write_endpoint=exposed_read_write_endpoint,
exposed_read_only_endpoint=exposed_read_only_endpoint,
router_read_write_endpoints=router_read_write_endpoints,
router_read_only_endpoints=router_read_only_endpoints,
exposed_read_write_endpoints=exposed_read_write_endpoints,
exposed_read_only_endpoints=exposed_read_only_endpoints,
shell=shell,
)
self._deprecated_shared_db.reconcile_users(event=event, shell=shell)
Expand Down
Loading

0 comments on commit d0af047

Please sign in to comment.