Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync configs between units #258

Merged
merged 2 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions lib/charms/tempo_k8s/v1/charm_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,14 @@ def my_tracing_endpoint(self) -> Optional[str]:
from opentelemetry.sdk.resources import Resource
from opentelemetry.sdk.trace import Span, TracerProvider
from opentelemetry.sdk.trace.export import BatchSpanProcessor
from opentelemetry.trace import INVALID_SPAN, Tracer
from opentelemetry.trace import get_current_span as otlp_get_current_span
from opentelemetry.trace import (
INVALID_SPAN,
Tracer,
get_tracer,
get_tracer_provider,
set_span_in_context,
set_tracer_provider,
)
from opentelemetry.trace import get_current_span as otlp_get_current_span
from ops.charm import CharmBase
from ops.framework import Framework

Expand All @@ -147,7 +146,7 @@ def my_tracing_endpoint(self) -> Optional[str]:
# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version

LIBPATCH = 8
LIBPATCH = 10

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

Expand Down Expand Up @@ -295,16 +294,17 @@ def wrap_init(self: CharmBase, framework: Framework, *args, **kwargs):
# self.handle = Handle(None, self.handle_kind, None)

original_event_context = framework._event_context
# default service name isn't just app name because it could conflict with the workload service name
_service_name = service_name or f"{self.app.name}-charm"

_service_name = service_name or self.app.name

unit_name = self.unit.name
resource = Resource.create(
attributes={
"service.name": _service_name,
"compose_service": _service_name,
"charm_type": type(self).__name__,
# juju topology
"juju_unit": self.unit.name,
"juju_unit": unit_name,
"juju_application": self.app.name,
"juju_model": self.model.name,
"juju_model_uuid": self.model.uuid,
Expand Down Expand Up @@ -342,16 +342,18 @@ def wrap_init(self: CharmBase, framework: Framework, *args, **kwargs):
_tracer = get_tracer(_service_name) # type: ignore
_tracer_token = tracer.set(_tracer)

dispatch_path = os.getenv("JUJU_DISPATCH_PATH", "")
dispatch_path = os.getenv("JUJU_DISPATCH_PATH", "") # something like hooks/install
event_name = dispatch_path.split("/")[1] if "/" in dispatch_path else dispatch_path
root_span_name = f"{unit_name}: {event_name} event"
span = _tracer.start_span(root_span_name, attributes={"juju.dispatch_path": dispatch_path})

# all these shenanigans are to work around the fact that the opentelemetry tracing API is built
# on the assumption that spans will be used as contextmanagers.
# Since we don't (as we need to close the span on framework.commit),
# we need to manually set the root span as current.
span = _tracer.start_span("charm exec", attributes={"juju.dispatch_path": dispatch_path})
ctx = set_span_in_context(span)

# log a trace id so we can look it up in tempo.
# log a trace id, so we can pick it up from the logs (and jhack) to look it up in tempo.
root_trace_id = hex(span.get_span_context().trace_id)[2:] # strip 0x prefix
logger.debug(f"Starting root trace with id={root_trace_id!r}.")

Expand Down
13 changes: 3 additions & 10 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ def generate_relation_databases(self) -> Dict[str, Dict[str, Union[str, bool]]]:
self.set_relation_databases(databases)
return databases

def _get_relation_config(self, inject_db=None) -> [Dict[str, Dict[str, Union[str, bool]]]]:
def _get_relation_config(self) -> [Dict[str, Dict[str, Union[str, bool]]]]:
"""Generate pgb config for databases and admin users."""
if not self.backend.relation or not (databases := self.get_relation_databases()):
return {}
Expand Down Expand Up @@ -685,13 +685,6 @@ def _get_relation_config(self, inject_db=None) -> [Dict[str, Dict[str, Union[str
"port": r_port,
"auth_user": self.backend.auth_user,
}
if inject_db:
pgb_dbs[inject_db] = {
"host": host,
"dbname": inject_db,
"port": port,
"auth_user": self.backend.auth_user,
}
if "*" in databases:
pgb_dbs["*"] = {
"host": host,
Expand All @@ -701,7 +694,7 @@ def _get_relation_config(self, inject_db=None) -> [Dict[str, Dict[str, Union[str
}
return pgb_dbs

def render_pgb_config(self, reload_pgbouncer=False, restart=False, inject_db=None):
def render_pgb_config(self, reload_pgbouncer=False, restart=False):
"""Derives config files for the number of required services from given config.

This method takes a primary config and generates one unique config for each intended
Expand Down Expand Up @@ -739,7 +732,7 @@ def render_pgb_config(self, reload_pgbouncer=False, restart=False, inject_db=Non
reserve_pool_size = math.ceil(effective_db_connections / 4)
with open("templates/pgb_config.j2", "r") as file:
template = Template(file.read())
databases = self._get_relation_config(inject_db=inject_db)
databases = self._get_relation_config()
readonly_dbs = self._get_readonly_dbs(databases)
enable_tls = all(self.tls.get_tls_files()) and self._is_exposed
if self._is_exposed:
Expand Down
5 changes: 5 additions & 0 deletions src/relations/peers.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
""" # noqa: W505

import logging
from hashlib import shake_128
from typing import List, Optional, Set

from ops.charm import CharmBase, HookEvent
Expand Down Expand Up @@ -132,7 +133,11 @@ def _on_changed(self, event: HookEvent):
if self.charm.backend.postgres:
self.charm.render_prometheus_service()

pgb_dbs_hash = shake_128(self.app_databag.get("pgb_dbs_config", "{}").encode()).hexdigest(
16
)
self.charm.render_pgb_config(reload_pgbouncer=True)
self.unit_databag["pgb_dbs"] = pgb_dbs_hash

def _on_departed(self, _):
self.update_leader()
Expand Down
42 changes: 21 additions & 21 deletions src/relations/pgbouncer_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
""" # noqa: W505

import logging
from hashlib import shake_128

from charms.data_platform_libs.v0.data_interfaces import (
DatabaseProvides,
Expand All @@ -48,7 +49,7 @@
PostgreSQLDeleteUserError,
PostgreSQLGetPostgreSQLVersionError,
)
from ops.charm import CharmBase, RelationBrokenEvent, RelationChangedEvent, RelationDepartedEvent
from ops.charm import CharmBase, RelationBrokenEvent, RelationDepartedEvent
from ops.framework import Object
from ops.model import (
Application,
Expand Down Expand Up @@ -91,25 +92,13 @@ def __init__(self, charm: CharmBase, relation_name: str = CLIENT_RELATION_NAME)
self.framework.observe(
charm.on[self.relation_name].relation_broken, self._on_relation_broken
)
self.framework.observe(
charm.on[self.relation_name].relation_changed, self._on_relation_changed
)

def _depart_flag(self, relation):
return f"{self.relation_name}_{relation.id}_departing"

def _unit_departing(self, relation):
return self.charm.peers.unit_databag.get(self._depart_flag(relation), None) == "true"

def _on_relation_changed(self, event: RelationChangedEvent) -> None:
"""Injects the database name for follower units."""
if not self.charm.unit.is_leader():
if "database" in event.relation.data[event.app]:
database = event.relation.data[event.app]["database"]
dbs = self.charm.generate_relation_databases()
if database not in [db["name"] for db in dbs]:
self.charm.render_pgb_config(reload_pgbouncer=True, inject_db=database)

def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
"""Handle the client relation-requested event.

Expand All @@ -125,7 +114,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
for key in self.charm.peers.relation.data.keys():
if key != self.charm.app:
if self.charm.peers.relation.data[key].get("auth_file_set", "false") != "true":
logger.info("Backend credentials not yet set on all units")
logger.debug("Backend credentials not yet set on all units")
event.defer()
return

Expand All @@ -134,6 +123,24 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
extra_user_roles = event.extra_user_roles or ""
rel_id = event.relation.id

dbs = self.charm.generate_relation_databases()
dbs[str(event.relation.id)] = {"name": database, "legacy": False}
roles = extra_user_roles.lower().split(",")
if "admin" in roles or "superuser" in roles:
dbs["*"] = {"name": "*", "auth_dbname": database}
self.charm.set_relation_databases(dbs)

pgb_dbs_hash = shake_128(
self.charm.peers.app_databag["pgb_dbs_config"].encode()
).hexdigest(16)
for key in self.charm.peers.relation.data.keys():
# We skip the leader so we don't have to wait on the defer
if key != self.charm.app and key != self.charm.unit:
if self.charm.peers.relation.data[key].get("pgb_dbs", "") != pgb_dbs_hash:
logger.debug("Not all units have synced configuration")
event.defer()
return

# Creates the user and the database for this specific relation.
user = f"relation_id_{rel_id}"
logger.debug("generating relation user")
Expand All @@ -160,13 +167,6 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
)
return

dbs = self.charm.generate_relation_databases()
dbs[str(event.relation.id)] = {"name": database, "legacy": False}
roles = extra_user_roles.lower().split(",")
if "admin" in roles or "superuser" in roles:
dbs["*"] = {"name": "*", "auth_dbname": database}
self.charm.set_relation_databases(dbs)

self.charm.render_pgb_config(reload_pgbouncer=True)

# Share the credentials and updated connection info with the client application.
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/relations/test_pgbouncer_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ def test_on_database_requested(
self.harness.update_relation_data(
self.peers_rel_id, self.unit, {"auth_file_set": "true"}
)
self.harness.update_relation_data(
self.peers_rel_id, self.app, {"pgb_dbs_config": "{}"}
)
self.client_relation._on_database_requested(event)

# Verify we've called everything we should
Expand Down