From d84573000b6ddddcc521fc6497857d4edf62e8b9 Mon Sep 17 00:00:00 2001 From: Dragomir Penev <6687393+dragomirp@users.noreply.github.com> Date: Wed, 5 Jun 2024 12:56:51 +0300 Subject: [PATCH] [DPE-3927] Collect readonly dbs (#228) * Collect read only databases * Fixes * Fail the action if not run against the leader * Only collect if backend is up * Backoff action * Update libs --- .../data_platform_libs/v0/data_interfaces.py | 14 ++++--- lib/charms/operator_libs_linux/v2/snap.py | 14 +++++-- src/charm.py | 42 +++++++++++++++++++ templates/pgb_config.j2 | 3 ++ tests/unit/test_charm.py | 2 + 5 files changed, 66 insertions(+), 9 deletions(-) diff --git a/lib/charms/data_platform_libs/v0/data_interfaces.py b/lib/charms/data_platform_libs/v0/data_interfaces.py index 5cb309b1f..59a97226a 100644 --- a/lib/charms/data_platform_libs/v0/data_interfaces.py +++ b/lib/charms/data_platform_libs/v0/data_interfaces.py @@ -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 = 35 +LIBPATCH = 37 PYDEPS = ["ops>=2.0.0"] @@ -642,8 +642,8 @@ def _move_to_new_label_if_needed(self): return # Create a new secret with the new label - old_meta = self._secret_meta content = self._secret_meta.get_content() + self._secret_uri = None # I wish we could just check if we are the owners of the secret... try: @@ -651,13 +651,17 @@ def _move_to_new_label_if_needed(self): except ModelError as err: if "this unit is not the leader" not in str(err): raise - old_meta.remove_all_revisions() + self.current_label = None def set_content(self, content: Dict[str, str]) -> None: """Setting cached secret content.""" if not self.meta: return + # DPE-4182: do not create new revision if the content stay the same + if content == self.get_content(): + return + if content: self._move_to_new_label_if_needed() self.meta.set_content(content) @@ -1586,7 +1590,7 @@ def _register_secret_to_relation( """ label = self._generate_secret_label(relation_name, relation_id, group) - # Fetchin the Secret's meta information ensuring that it's locally getting registered with + # Fetching the Secret's meta information ensuring that it's locally getting registered with CachedSecret(self._model, self.component, label, secret_id).meta def _register_secrets_to_relation(self, relation: Relation, params_name_list: List[str]): @@ -2309,7 +2313,7 @@ def _secrets(self) -> dict: return self._cached_secrets def _get_secret(self, group) -> Optional[Dict[str, str]]: - """Retrieveing secrets.""" + """Retrieving secrets.""" if not self.app: return if not self._secrets.get(group): diff --git a/lib/charms/operator_libs_linux/v2/snap.py b/lib/charms/operator_libs_linux/v2/snap.py index ef426775d..9d09a78d3 100644 --- a/lib/charms/operator_libs_linux/v2/snap.py +++ b/lib/charms/operator_libs_linux/v2/snap.py @@ -83,7 +83,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 5 +LIBPATCH = 7 # Regex to locate 7-bit C1 ANSI sequences @@ -319,7 +319,10 @@ def get(self, key: Optional[str], *, typed: bool = False) -> Any: Default is to return a string. """ if typed: - config = json.loads(self._snap("get", ["-d", key])) + args = ["-d"] + if key: + args.append(key) + config = json.loads(self._snap("get", args)) if key: return config.get(key) return config @@ -584,13 +587,16 @@ def ensure( "Installing snap %s, revision %s, tracking %s", self._name, revision, channel ) self._install(channel, cohort, revision) - else: + logger.info("The snap installation completed successfully") + elif revision is None or revision != self._revision: # The snap is installed, but we are changing it (e.g., switching channels). logger.info( "Refreshing snap %s, revision %s, tracking %s", self._name, revision, channel ) self._refresh(channel=channel, cohort=cohort, revision=revision, devmode=devmode) - logger.info("The snap installation completed successfully") + logger.info("The snap refresh completed successfully") + else: + logger.info("Refresh of snap %s was unnecessary", self._name) self._update_snap_apps() self._state = state diff --git a/src/charm.py b/src/charm.py index 279f346e2..92689b01e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -15,6 +15,7 @@ from configparser import ConfigParser from typing import Dict, List, Literal, Optional, Union, get_args +import psycopg2 from charms.data_platform_libs.v0.data_interfaces import DataPeerData, DataPeerUnitData from charms.grafana_agent.v0.cos_agent import COSAgentProvider from charms.operator_libs_linux.v1 import systemd @@ -402,6 +403,44 @@ def _on_start(self, _) -> None: def _on_leader_elected(self, _): self.peers.update_leader() + def _get_readonly_dbs(self, databases: Dict) -> Dict[str, str]: + readonly_dbs = {} + if self.backend.relation and "*" in databases: + read_only_endpoints = self.backend.get_read_only_endpoints() + r_hosts = ",".join([r_host.split(":")[0] for r_host in read_only_endpoints]) + if r_hosts: + for r_host in read_only_endpoints: + r_port = r_host.split(":")[1] + break + + backend_databases = json.loads(self.peers.app_databag.get("readonly_dbs", "[]")) + for name in backend_databases: + readonly_dbs[f"{name}_readonly"] = { + "host": r_hosts, + "dbname": name, + "port": r_port, + "auth_dbname": databases["*"]["auth_dbname"], + "auth_user": self.backend.auth_user, + } + return readonly_dbs + + def _collect_readonly_dbs(self) -> None: + if self.unit.is_leader() and self.backend.postgres: + existing_dbs = [db["name"] for db in self.get_relation_databases().values()] + existing_dbs += ["postgres", "pgbouncer"] + try: + with self.backend.postgres._connect_to_database( + PGB + ) as conn, conn.cursor() as cursor: + cursor.execute("SELECT datname FROM pg_database WHERE datistemplate = false;") + results = cursor.fetchall() + conn.close() + except psycopg2.Error: + logger.warning("PostgreSQL connection failed") + readonly_dbs = [db[0] for db in results if db and db[0] not in existing_dbs] + readonly_dbs.sort() + self.peers.app_databag["readonly_dbs"] = json.dumps(readonly_dbs) + def _on_update_status(self, _) -> None: """Update Status hook. @@ -411,6 +450,7 @@ def _on_update_status(self, _) -> None: self.update_status() self.peers.update_leader() + self._collect_readonly_dbs() def update_status(self): """Health check to update pgbouncer status based on charm state.""" @@ -645,6 +685,7 @@ def render_pgb_config(self, reload_pgbouncer=False): with open("templates/pgb_config.j2", "r") as file: template = Template(file.read()) 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: addr = "*" @@ -657,6 +698,7 @@ def render_pgb_config(self, reload_pgbouncer=False): f"{app_conf_dir}/{INSTANCE_DIR}{service_id}/pgbouncer.ini", template.render( databases=databases, + readonly_databases=readonly_dbs, peer_id=service_id, base_socket_dir=f"{app_temp_dir}/{INSTANCE_DIR}", peers=self.service_ids, diff --git a/templates/pgb_config.j2 b/templates/pgb_config.j2 index 1a7178488..ff8c9a3ef 100644 --- a/templates/pgb_config.j2 +++ b/templates/pgb_config.j2 @@ -2,6 +2,9 @@ {% for name, database in databases.items() -%} {{ name }} = host={{ database.host }} {% if database.dbname %}dbname={{ database.dbname }}{% else %}auth_dbname={{ database.auth_dbname }}{% endif %} port={{ database.port }} auth_user={{ database.auth_user }} {% endfor %} +{% for name, database in readonly_databases.items() -%} +{{ name }} = host={{ database.host }} dbname={{ database.dbname }} auth_dbname={{ database.auth_dbname }} port={{ database.port }} auth_user={{ database.auth_user }} +{% endfor %} [peers] {% for peer in peers -%} diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 91696cf3c..65866cd73 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -374,6 +374,7 @@ def test_render_pgb_config( for i in range(self.charm._cores): expected_content = template.render( databases=expected_databases, + readonly_databases={}, peer_id=i, peers=range(self.charm._cores), base_socket_dir="/tmp/pgbouncer/instance_", @@ -420,6 +421,7 @@ def test_render_pgb_config( for i in range(self.charm._cores): expected_content = template.render( databases=expected_databases, + readonly_databases={}, peer_id=i, peers=range(self.charm._cores), base_socket_dir="/tmp/pgbouncer/instance_",