[DPE-4261] Switch to split snap (#265)

* Fixes from k8s

* Switch snap

* Bump libs

* Variable snap locations

* Wrong dir reference

* Recreate folders

* Add psql

src/charm.py

@@ -50,7 +50,7 @@
     PGB_CONF_DIR,
     PGB_LOG_DIR,
     PGBOUNCER_EXECUTABLE,
-    POSTGRESQL_SNAP_NAME,
+    PGBOUNCER_SNAP_NAME,
     SECRET_DELETED_LABEL,
     SECRET_INTERNAL_LABEL,
     SECRET_KEY_OVERRIDES,
@@ -131,7 +131,7 @@ def __init__(self, *args):
             metrics_endpoints=[
                 {"path": "/metrics", "port": self.config["metrics_port"]},
             ],
-            log_slots=[f"{POSTGRESQL_SNAP_NAME}:logs"],
+            log_slots=[f"{PGBOUNCER_SNAP_NAME}:logs"],
             refresh_events=[self.on.config_changed],
         )
 
@@ -209,7 +209,7 @@ def _on_install(self, _) -> None:
         """
         self.unit.status = MaintenanceStatus("Installing and configuring PgBouncer")
 
-        # Install the charmed PostgreSQL snap.
+        # Install the charmed PgBouncer snap.
         try:
             self._install_snap_packages(packages=SNAP_PACKAGES)
         except snap.SnapError:
@@ -219,9 +219,8 @@ def _on_install(self, _) -> None:
         # Try to disable pgbackrest service
         try:
             cache = snap.SnapCache()
-            selected_snap = cache["charmed-postgresql"]
+            selected_snap = cache[PGBOUNCER_SNAP_NAME]
             selected_snap.alias("psql")
-            selected_snap.stop(services=["pgbackrest-service"], disable=True)
         except snap.SnapError as e:
             error_message = "Failed to stop and disable pgbackrest snap service"
             logger.exception(error_message, exc_info=e)
@@ -439,7 +438,9 @@ 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])
+            sorted_rhosts = [r_host.split(":")[0] for r_host in read_only_endpoints]
+            sorted_rhosts.sort()
+            r_hosts = ",".join(sorted_rhosts)
             if r_hosts:
                 for r_host in read_only_endpoints:
                     r_port = r_host.split(":")[1]
@@ -644,7 +645,7 @@ def generate_relation_databases(self) -> Dict[str, Dict[str, Union[str, bool]]]:
             if "admin" in roles or "superuser" in roles or "createdb" in roles:
                 add_wildcard = True
         if add_wildcard:
-            databases["*"] = {"name": "*", "auth_dbname": database}
+            databases["*"] = {"name": "*", "auth_dbname": database, "legacy": False}
         self.set_relation_databases(databases)
         return databases
 
@@ -660,7 +661,9 @@ def _get_relation_config(self) -> [Dict[str, Dict[str, Union[str, bool]]]]:
         host, port = postgres_endpoint.split(":")
 
         read_only_endpoints = self.backend.get_read_only_endpoints()
-        r_hosts = ",".join([r_host.split(":")[0] for r_host in read_only_endpoints])
+        sorted_rhosts = [r_host.split(":")[0] for r_host in read_only_endpoints]
+        sorted_rhosts.sort()
+        r_hosts = ",".join(sorted_rhosts)
         if r_hosts:
             for r_host in read_only_endpoints:
                 r_port = r_host.split(":")[1]

src/constants.py

@@ -10,22 +10,22 @@
 AUTH_FILE_NAME = "userlist.txt"
 
 # Snap constants.
-PGBOUNCER_EXECUTABLE = "charmed-postgresql.pgbouncer"
-POSTGRESQL_SNAP_NAME = "charmed-postgresql"
+PGBOUNCER_SNAP_NAME = "charmed-pgbouncer"
+PGBOUNCER_EXECUTABLE = f"{PGBOUNCER_SNAP_NAME}.pgbouncer"
 SNAP_PACKAGES = [
     (
-        POSTGRESQL_SNAP_NAME,
-        {"revision": {"aarch64": "112", "x86_64": "113"}, "channel": "14/stable"},
+        PGBOUNCER_SNAP_NAME,
+        {"revision": {"aarch64": "4", "x86_64": "3"}, "channel": "1/stable"},
     )
 ]
 
-SNAP_COMMON_PATH = "/var/snap/charmed-postgresql/common"
-SNAP_CURRENT_PATH = "/var/snap/charmed-postgresql/current"
+SNAP_COMMON_PATH = f"/var/snap/{PGBOUNCER_SNAP_NAME}/common"
+SNAP_CURRENT_PATH = f"/var/snap/{PGBOUNCER_SNAP_NAME}/current"
 
 PGB_CONF_DIR = f"{SNAP_CURRENT_PATH}/etc/pgbouncer"
 PGB_LOG_DIR = f"{SNAP_COMMON_PATH}/var/log/pgbouncer"
 
-SNAP_TMP_DIR = "/tmp/snap-private-tmp/snap.charmed-postgresql/tmp"
+SNAP_TMP_DIR = f"/tmp/snap-private-tmp/snap.{PGBOUNCER_SNAP_NAME}/tmp"
 
 # PGB config
 DATABASES = "databases"
@@ -61,7 +61,7 @@
     "ca": "cauth",
 }
 
-SOCKET_LOCATION = "/tmp/snap-private-tmp/snap.charmed-postgresql/tmp/pgbouncer/instance_0"
+SOCKET_LOCATION = f"/tmp/snap-private-tmp/snap.{PGBOUNCER_SNAP_NAME}/tmp/pgbouncer/instance_0"
 
 TRACING_RELATION_NAME = "tracing"
 TRACING_PROTOCOL = "otlp_http"

src/relations/pgbouncer_provider.py

@@ -126,7 +126,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None:
         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:
+        if "admin" in roles or "superuser" in roles or "createdb" in roles:
             dbs["*"] = {"name": "*", "auth_dbname": database}
         self.charm.set_relation_databases(dbs)
 

src/upgrade.py

@@ -90,6 +90,7 @@ def _on_upgrade_granted(self, event: UpgradeGrantedEvent) -> None:
         if self.charm.unit.is_leader():
             self.charm.generate_relation_databases()
 
+        self.charm.create_instance_directories()
         self.charm.render_pgb_config()
         self.charm.render_utility_files()
         self.charm.reload_pgbouncer()

templates/pgbouncer.service.j2

@@ -5,9 +5,9 @@ After=network.target
 [Service]
 Type=simple
 ExecStartPre=-/usr/bin/install -o snap_daemon -g snap_daemon -m 700 -d \
-        /var/snap/charmed-postgresql/common/var/log/pgbouncer/{{ app_name }}/instance_%i/ \
+        /var/snap/charmed-pgbouncer/common/var/log/pgbouncer/{{ app_name }}/instance_%i/ \
         {{ snap_tmp_dir }}/{{ app_name }}/instance_%i/
-ExecStart=/snap/bin/charmed-postgresql.pgbouncer-server {{ conf_dir }}/{{ app_name }}/instance_%i/pgbouncer.ini
+ExecStart=/snap/bin/charmed-pgbouncer.pgbouncer-server {{ conf_dir }}/{{ app_name }}/instance_%i/pgbouncer.ini
 KillSignal=SIGINT
 ExecReload=kill -HUP $MAINPID
 Restart=always

templates/prometheus-exporter.service.j2

@@ -4,7 +4,7 @@ After=network.target {{ pgb_service }}@.target
 
 [Service]
 Type=simple
-ExecStart=/snap/bin/charmed-postgresql.prometheus-pgbouncer-exporter --web.listen-address=:{{ metrics_port }} --pgBouncer.connectionString="postgres://{{ stats_user }}:{{ stats_password }}@localhost:{{ listen_port }}/pgbouncer?sslmode=disable"
+ExecStart=/snap/bin/charmed-pgbouncer.prometheus-pgbouncer-exporter --web.listen-address=:{{ metrics_port }} --pgBouncer.connectionString="postgres://{{ stats_user }}:{{ stats_password }}@localhost:{{ listen_port }}/pgbouncer?sslmode=disable"
 Restart=always
 RestartSec=5s
 

tests/unit/relations/test_pgbouncer_provider.py

@@ -47,6 +47,7 @@ def setUp(self):
         new_callable=PropertyMock,
         return_value=sentinel.client_rels,
     )
+    @patch("charm.PgBouncerCharm.render_pgb_config")
     @patch("relations.backend_database.BackendDatabaseRequires.check_backend", return_value=True)
     @patch(
         "relations.backend_database.BackendDatabaseRequires.postgres", new_callable=PropertyMock
@@ -83,6 +84,7 @@ def test_on_database_requested(
         _pg_databag,
         _pg,
         _check_backend,
+        _render_pgb_config,
         _,
     ):
         self.harness.set_leader()
@@ -130,6 +132,7 @@ def test_on_database_requested(
             "1": {"name": "test-db", "legacy": False},
             "*": {"name": "*", "auth_dbname": "test-db"},
         })
+        _render_pgb_config.assert_called_once_with(reload_pgbouncer=True)
 
     @patch("relations.backend_database.BackendDatabaseRequires.check_backend", return_value=True)
     @patch(

tests/unit/test_charm.py

@@ -80,7 +80,7 @@ def test_on_install(
         _install,
         _snap_cache,
     ):
-        pg_snap = _snap_cache.return_value["charmed-postgresql"]
+        pg_snap = _snap_cache.return_value["charmed-pgbouncer"]
         self.charm.on.install.emit()
 
         _install.assert_called_once_with(packages=SNAP_PACKAGES)
@@ -222,29 +222,29 @@ def test_install_snap_packages(self, _snap_cache):
 
         # Test for problem with snap update.
         with self.assertRaises(snap.SnapError):
-            self.charm._install_snap_packages([("postgresql", {"channel": "14/edge"})])
-        _snap_cache.return_value.__getitem__.assert_called_once_with("postgresql")
+            self.charm._install_snap_packages([("pgbouncer", {"channel": "1/edge"})])
+        _snap_cache.return_value.__getitem__.assert_called_once_with("pgbouncer")
         _snap_cache.assert_called_once_with()
-        _snap_package.ensure.assert_called_once_with(snap.SnapState.Latest, channel="14/edge")
+        _snap_package.ensure.assert_called_once_with(snap.SnapState.Latest, channel="1/edge")
 
         # Test with a not found package.
         _snap_cache.reset_mock()
         _snap_package.reset_mock()
         _snap_package.ensure.side_effect = snap.SnapNotFoundError
         with self.assertRaises(snap.SnapNotFoundError):
-            self.charm._install_snap_packages([("postgresql", {"channel": "14/edge"})])
-        _snap_cache.return_value.__getitem__.assert_called_once_with("postgresql")
+            self.charm._install_snap_packages([("pgbouncer", {"channel": "1/edge"})])
+        _snap_cache.return_value.__getitem__.assert_called_once_with("pgbouncer")
         _snap_cache.assert_called_once_with()
-        _snap_package.ensure.assert_called_once_with(snap.SnapState.Latest, channel="14/edge")
+        _snap_package.ensure.assert_called_once_with(snap.SnapState.Latest, channel="1/edge")
 
         # Then test a valid one.
         _snap_cache.reset_mock()
         _snap_package.reset_mock()
         _snap_package.ensure.side_effect = None
-        self.charm._install_snap_packages([("postgresql", {"channel": "14/edge"})])
+        self.charm._install_snap_packages([("pgbouncer", {"channel": "1/edge"})])
         _snap_cache.assert_called_once_with()
-        _snap_cache.return_value.__getitem__.assert_called_once_with("postgresql")
-        _snap_package.ensure.assert_called_once_with(snap.SnapState.Latest, channel="14/edge")
+        _snap_cache.return_value.__getitem__.assert_called_once_with("pgbouncer")
+        _snap_package.ensure.assert_called_once_with(snap.SnapState.Latest, channel="1/edge")
         _snap_package.hold.assert_not_called()
 
         # Test revision
@@ -504,6 +504,46 @@ def test_on_update_status(self, _update_leader, _update_status, _collect_readonl
         _update_status.assert_called_once_with()
         _collect_readonly_dbs.assert_called_once_with()
 
+    @patch(
+        "charm.BackendDatabaseRequires.auth_user",
+        new_callable=PropertyMock,
+        return_value="auth_user",
+    )
+    @patch(
+        "charm.BackendDatabaseRequires.postgres_databag",
+        new_callable=PropertyMock,
+        return_value={},
+    )
+    @patch(
+        "charm.BackendDatabaseRequires.relation", new_callable=PropertyMock, return_value=Mock()
+    )
+    @patch_network_get(private_address="1.1.1.1")
+    def test_get_readonly_dbs(self, _backend_rel, _postgres_databag, _):
+        with self.harness.hooks_disabled():
+            self.harness.update_relation_data(
+                self.rel_id, self.charm.app.name, {"readonly_dbs": '["includedb"]'}
+            )
+
+        # Returns empty if no wildcard
+        assert self.charm._get_readonly_dbs({}) == {}
+
+        # Returns empty if no readonly backends
+        assert self.charm._get_readonly_dbs({"*": {"name": "*", "auth_dbname": "authdb"}}) == {}
+
+        _postgres_databag.return_value = {
+            "endpoints": "HOST:PORT",
+            "read-only-endpoints": "HOST2:PORT,HOST3:PORT",
+        }
+        assert self.charm._get_readonly_dbs({"*": {"name": "*", "auth_dbname": "authdb"}}) == {
+            "includedb_readonly": {
+                "auth_dbname": "authdb",
+                "auth_user": "auth_user",
+                "dbname": "includedb",
+                "host": "HOST2,HOST3",
+                "port": "PORT",
+            }
+        }
+
     @patch("charm.BackendDatabaseRequires.postgres")
     @patch(
         "charm.PgBouncerCharm.get_relation_databases", return_value={"1": {"name": "excludeddb"}}

tests/unit/test_upgrade.py

@@ -42,6 +42,7 @@ def test_log_rollback_instructions(self, _logger: Mock):
 
     @patch("charm.PgBouncerCharm.get_secret")
     @patch("charm.BackendDatabaseRequires.postgres", return_value=True, new_callable=PropertyMock)
+    @patch("charm.PgBouncerCharm.create_instance_directories")
     @patch("charm.PgBouncerCharm.update_status")
     @patch("charm.PgbouncerUpgrade.on_upgrade_changed")
     @patch("charm.PgbouncerUpgrade.set_unit_completed")
@@ -66,6 +67,7 @@ def test_on_upgrade_granted(
         _set_unit_completed: Mock,
         _on_upgrade_changed: Mock,
         _update_status: Mock,
+        _create_instance_directories: Mock,
         _,
         __,
     ):
@@ -83,6 +85,7 @@ def test_on_upgrade_granted(
         _cluster_checks.assert_called_once_with()
         _set_unit_completed.assert_called_once_with()
         _update_status.assert_called_once_with()
+        _create_instance_directories.asssert_called_once_with()
         assert not _generate_relation_databases.called
 
         # Test extra call as leader
@@ -97,6 +100,7 @@ def test_on_upgrade_granted(
     @patch("charm.PgBouncerCharm.get_secret")
     @patch("upgrade.wait_fixed", return_value=tenacity.wait_fixed(0))
     @patch("charm.BackendDatabaseRequires.postgres", return_value=True, new_callable=PropertyMock)
+    @patch("charm.PgBouncerCharm.create_instance_directories")
     @patch("charm.PgbouncerUpgrade.on_upgrade_changed")
     @patch("charm.PgbouncerUpgrade.set_unit_completed")
     @patch("charm.PgbouncerUpgrade._cluster_checks")
@@ -120,6 +124,7 @@ def test_on_upgrade_granted_error(
         _,
         __,
         ___,
+        ____,
     ):
         _cluster_checks.side_effect = ClusterNotReadyError("test", "test")