diff --git a/refresh_versions.toml b/refresh_versions.toml new file mode 100644 index 0000000..ee23144 --- /dev/null +++ b/refresh_versions.toml @@ -0,0 +1,2 @@ +charm = "16/0.0.0" +workload = "16.0" diff --git a/single_kernel_postgresql/charms/abstract_charm.py b/single_kernel_postgresql/charms/abstract_charm.py index 28ea358..63f8978 100644 --- a/single_kernel_postgresql/charms/abstract_charm.py +++ b/single_kernel_postgresql/charms/abstract_charm.py @@ -100,6 +100,9 @@ def __init__(self, *args): self.patroni_manager, ) + # Resume or prepare the refresh (the charms' post-construction resume block). + self.refresh_manager.on_init() + # Status Handler self.status_handler = StatusHandler( self, @@ -177,10 +180,37 @@ def set_app_status(self) -> None: pass @abstractmethod - def update_config(self) -> bool: + def update_config(self, *, refresh: "charm_refresh.Machines | None" = None) -> bool: """Re-render the Patroni configuration and apply it.""" pass + @abstractmethod + def post_refresh_side_effects(self) -> None: + """Run the post-snap-refresh side effects owned by not-yet-migrated modules. + + The VM charm sets up the exporter and pgBackRest exporter, starts/stops the + pgBackRest service and updates the watcher unit address here. + """ + pass + + @abstractmethod + def has_async_replication_relation(self) -> bool: + """Whether this unit is related to an async replication partner. + + Owned by the async-replication module until that phase migrates; the temp + tablespace migration skips units inside an async cluster. + """ + pass + + @abstractmethod + def update_relation_endpoints(self) -> None: + """Refresh the client and async relation endpoints after a switchover. + + Owned by the client-relation and async-replication modules until those + phases migrate; the VM pre-refresh checks call it after switching primary. + """ + pass + @property @abstractmethod def primary_endpoint(self) -> str | None: diff --git a/single_kernel_postgresql/charms/k8s_charm.py b/single_kernel_postgresql/charms/k8s_charm.py index 18ef241..d1a8a30 100755 --- a/single_kernel_postgresql/charms/k8s_charm.py +++ b/single_kernel_postgresql/charms/k8s_charm.py @@ -89,7 +89,7 @@ def set_unit_status( status: StatusBase, /, *, - refresh: "charm_refresh.Kubernetes | None" = None, + refresh: "charm_refresh.Machines | charm_refresh.Kubernetes | None" = None, ) -> None: """Set the unit status without overriding a higher-priority refresh status.""" self.refresh_manager.set_unit_status(status, refresh=refresh) @@ -101,7 +101,16 @@ def set_default_unit_status(self) -> None: def set_app_status(self) -> None: """Set the application status from the async-replication state.""" - def update_config(self, *, refresh: "charm_refresh.Kubernetes | None" = None) -> bool: + def post_refresh_side_effects(self) -> None: + """Run the post-snap-refresh side effects owned by not-yet-migrated modules.""" + + def has_async_replication_relation(self) -> bool: + """Whether this unit is related to an async replication partner.""" + return False + + def update_config( + self, *, refresh: "charm_refresh.Machines | charm_refresh.Kubernetes | None" = None + ) -> bool: """Re-render the Patroni configuration and apply it.""" return self.config_manager.update_config(self.postgresql) @@ -113,3 +122,6 @@ def primary_endpoint(self) -> str | None: def get_async_primary_cluster_endpoint(self) -> str | None: """Endpoint of the primary cluster of the async replication partner, if any.""" return None + + def update_relation_endpoints(self) -> None: + """Refresh the client and async relation endpoints after a switchover.""" diff --git a/single_kernel_postgresql/charms/vm_charm.py b/single_kernel_postgresql/charms/vm_charm.py index 25803df..c089758 100755 --- a/single_kernel_postgresql/charms/vm_charm.py +++ b/single_kernel_postgresql/charms/vm_charm.py @@ -80,7 +80,7 @@ def set_unit_status( status: StatusBase, /, *, - refresh: "charm_refresh.Machines | None" = None, + refresh: "charm_refresh.Machines | charm_refresh.Kubernetes | None" = None, ) -> None: """Set the unit status without overriding a higher-priority refresh status.""" self.refresh_manager.set_unit_status(status, refresh=refresh) @@ -92,7 +92,16 @@ def set_default_unit_status(self) -> None: def set_app_status(self) -> None: """Set the application status from the async-replication state.""" - def update_config(self, *, refresh: "charm_refresh.Machines | None" = None) -> bool: + def post_refresh_side_effects(self) -> None: + """Set up the exporters, pgBackRest service, and watcher unit address.""" + + def has_async_replication_relation(self) -> bool: + """Whether this unit is related to an async replication partner.""" + return False + + def update_config( + self, *, refresh: "charm_refresh.Machines | charm_refresh.Kubernetes | None" = None + ) -> bool: """Re-render the Patroni configuration and apply it.""" if refresh is None: refresh = self.refresh_manager.refresh @@ -107,3 +116,6 @@ def primary_endpoint(self) -> str | None: def get_async_primary_cluster_endpoint(self) -> str | None: """Endpoint of the primary cluster of the async replication partner, if any.""" return None + + def update_relation_endpoints(self) -> None: + """Refresh the client and async relation endpoints after a switchover.""" diff --git a/single_kernel_postgresql/managers/config.py b/single_kernel_postgresql/managers/config.py index e8bfb50..e40554d 100644 --- a/single_kernel_postgresql/managers/config.py +++ b/single_kernel_postgresql/managers/config.py @@ -479,7 +479,7 @@ def update_config( watcher_raft_address: str | None = None, no_peers: bool = False, *, - refresh: charm_refresh.Machines | None = None, + refresh: charm_refresh.Machines | charm_refresh.Kubernetes | None = None, ) -> bool: """Updates Patroni config file based on the existence of the TLS files. @@ -578,7 +578,7 @@ def update_config( self.state.substrate == Substrates.VM and refresh is not None and cast("VMWorkload", self.workload).get_snap_revision() - != refresh.pinned_snap_revision + != cast("charm_refresh.Machines", refresh).pinned_snap_revision ): logger.debug("Early exit: snap was not refreshed to the right version yet") return True diff --git a/single_kernel_postgresql/managers/refresh.py b/single_kernel_postgresql/managers/refresh.py index 0683b9a..8b9e10f 100755 --- a/single_kernel_postgresql/managers/refresh.py +++ b/single_kernel_postgresql/managers/refresh.py @@ -18,9 +18,18 @@ from typing import TYPE_CHECKING, cast import charm_refresh +import psycopg2 from charm_refresh import CharmVersion, PrecheckFailed -from ops import ActiveStatus, MaintenanceStatus, StatusBase -from tenacity import Retrying, stop_after_attempt, wait_fixed +from cryptography.x509 import load_pem_x509_certificate +from cryptography.x509.oid import NameOID +from ops import ActiveStatus, BlockedStatus, MaintenanceStatus, StatusBase, WaitingStatus +from tenacity import ( + RetryError, + Retrying, + stop_after_attempt, + stop_after_delay, + wait_fixed, +) from single_kernel_postgresql.config.enums import Substrates from single_kernel_postgresql.config.exceptions import SwitchoverFailedError @@ -28,6 +37,7 @@ K8S_CHARM_NAME, K8S_OCI_RESOURCE_NAME, LAST_REFRESH_UNIT_STATUS_FILE, + UNIT_SCOPE, VM_CHARM_NAME, WORKLOAD_NAME, ) @@ -217,6 +227,8 @@ def refresh_snap( revision=snap_revision, refresh=refresh ) + self._charm.refresh_manager.post_snap_refresh(refresh) + class RefreshManager(BaseManager): """PostgreSQL Refresh Manager. @@ -338,6 +350,233 @@ def reconcile_refresh_status(self, _=None) -> None: new_refresh_unit_status = refresh_status.message path.write_text(json.dumps(new_refresh_unit_status)) + # -- Machines substrate: resume the workload after a snap refresh + + def on_init(self) -> None: + """Resume or prepare the refresh after the charm has been initialized.""" + refresh = self.refresh + if refresh is None: + return + if self.state.substrate == Substrates.VM and not refresh.next_unit_allowed_to_refresh: + if refresh.in_progress: + self.post_snap_refresh(refresh) + else: + self.migrate_temp_tablespace_location() + refresh.next_unit_allowed_to_refresh = True + + def post_snap_refresh( + self, refresh: charm_refresh.Machines | charm_refresh.Kubernetes + ) -> None: + """Start PostgreSQL, check if this app and unit are healthy, and allow next unit to refresh. + + Called after snap refresh. + """ + self.check_and_update_internal_cert() + + if not self._charm.patroni_manager.start_patroni(): + self.set_unit_status(BlockedStatus("Failed to start PostgreSQL"), refresh=refresh) + return + + self._charm.post_refresh_side_effects() + + # Wait until the database initialise. + self.set_unit_status(WaitingStatus("waiting for database initialisation"), refresh=refresh) + try: + for attempt in Retrying(stop=stop_after_attempt(30), wait=wait_fixed(10)): + with attempt: + # Check if the member hasn't started or hasn't joined the cluster yet. + if ( + not self._charm.patroni_manager.member_started + or self._charm.unit.name.replace("/", "-") + not in self._charm.patroni_manager.cluster_members + or not self._charm.patroni_manager.is_replication_healthy() + ): + logger.debug( + "Instance not yet back in the cluster." + f" Retry {attempt.retry_state.attempt_number}/6" + ) + raise Exception() + except RetryError: + logger.debug( + "Did not allow next unit to refresh: member not ready or not joined the cluster yet" + ) + else: + try: + self._charm.patroni_manager.set_max_timelines_history() + except Exception: + logger.warning("Unable to patch in max_timelines_history") + peer_relation = self._charm.state.peer_relation + all_units = sorted( + [self._charm.unit, *(peer_relation.units if peer_relation else [])], + key=lambda u: int(u.name.split("/")[1]), + ) + if self._charm.unit == all_units[0]: + for attempt in Retrying( + stop=stop_after_delay(180), wait=wait_fixed(5), reraise=True + ): + with attempt: + if not self.migrate_temp_tablespace_location(required=True): + raise Exception("Temp tablespace migration not yet complete") + refresh.next_unit_allowed_to_refresh = True + self.set_unit_status(ActiveStatus(), refresh=refresh) + + def check_and_update_internal_cert(self) -> None: + """Check if the internal cert CN matches the unit IP and regenerate if needed.""" + try: + if ( + (raw_cert := self._charm.state.get_secret(UNIT_SCOPE, "internal-cert")) + and (cert := load_pem_x509_certificate(raw_cert.encode())) + and ( + cert.subject.get_attributes_for_oid(NameOID.COMMON_NAME)[0].value + != self._charm.state.unit_ip + ) + ): + self._charm.tls_manager.generate_internal_peer_cert() + self._charm.tls_manager.push_tls_files() + self._charm.update_config() + except Exception: + logger.exception("Unable to check or update internal cert") + + def migrate_temp_tablespace_location(self, *, required: bool = False) -> bool: + """One-shot migration of the temp tablespace to the versioned directory. + + During a snap upgrade, the post-refresh hook migrates temp data from the + old non-versioned storage root to the versioned subdirectory. This method + updates the PostgreSQL catalog entry to match. + + During a snap downgrade (rollback), the pre-refresh hook handles both + file migration and catalog migration (DROP/CREATE TABLESPACE) back to + the non-versioned root. This method only handles the forward case. + + DROP TABLESPACE and CREATE TABLESPACE cannot run inside a transaction + block, so this method avoids using the connection as a context manager + (which would create one in psycopg2). Instead it uses plain assignments + and explicit close(), mirroring the pattern in the single_kernel_postgresql + set_up_database helper. + + Args: + required: If True (used during upgrade), return False when the + primary is unavailable so the caller can retry. If False + (default, used during install), return True to skip gracefully + when no cluster exists yet. + """ + if not self._charm.primary_endpoint: + return not required + + if self._charm.has_async_replication_relation(): + return True + + target_host = self._resolve_primary_host() + if target_host is None: + return False + + return self._execute_temp_tablespace_migration(target_host) + + def _resolve_primary_host(self) -> str | None: + """Wait for Patroni to settle and return the primary host. + + After a snap refresh, Patroni may briefly report this unit as the + primary before discovering the real cluster topology. Query the + Patroni API directly (bypassing primary_endpoint, which can return + stale data from the peer databag) and retry until the primary + points to a different host or this unit truly is the primary. + """ + try: + for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3)): + with attempt: + primary = self._charm.patroni_manager.get_primary() + if not primary: + raise Exception("No primary found yet") + target_host = self._charm.patroni_manager.get_member_ip(primary) + if not target_host: + raise Exception("Primary IP not available yet") + if ( + target_host != self._charm.state.unit_ip + or self._charm.patroni_manager.get_primary(unit_name_pattern=True) + == self._charm.unit.name + ): + return target_host + raise Exception("Patroni not settled yet") + except RetryError: + logger.warning("Patroni did not settle within 60s") + return None + return None + + def _execute_temp_tablespace_migration(self, target_host: str) -> bool: + """Execute the temp tablespace DDL migration on the given host.""" + temp_storage_path = str(self._charm.workload.paths.temp.parent) + temp_data_dir = str(self._charm.workload.paths.temp) + connection = None + cursor = None + try: + connection = self._charm.postgresql._connect_to_database(database_host=target_host) + connection.autocommit = True + cursor = connection.cursor() + + cursor.execute( + "SELECT pg_tablespace_location(oid) FROM pg_tablespace WHERE spcname='temp';" + ) + row = cursor.fetchone() + if row is None: + return True + + current_location = row[0] + if current_location == temp_data_dir: + return True + + if current_location != temp_storage_path: + logger.warning( + "Skipping temp tablespace migration: unexpected location %s " + "(expected %s or %s)", + current_location, + temp_storage_path, + temp_data_dir, + ) + return True + + logger.info( + "Migrating temp tablespace location from %s to %s", + temp_storage_path, + temp_data_dir, + ) + cursor.execute("DROP TABLESPACE temp;") + cursor.execute(f"CREATE TABLESPACE temp LOCATION '{temp_data_dir}';") + cursor.execute("GRANT CREATE ON TABLESPACE temp TO public;") + # Flush WAL past the CREATE TABLESPACE record so replicas won't + # need to replay it during a future rollback (the versioned + # directory may not exist after the snap's pre-refresh hook). + cursor.execute("CHECKPOINT;") + except psycopg2.Error: + logger.exception("Failed to migrate temp tablespace location") + try: + check_conn = self._charm.postgresql._connect_to_database(database_host=target_host) + check_conn.autocommit = True + check_cur = check_conn.cursor() + check_cur.execute( + "SELECT count(*) FROM pg_class WHERE reltablespace = " + "(SELECT oid FROM pg_tablespace WHERE spcname = 'temp')" + ) + obj_count = check_cur.fetchone()[0] + check_cur.close() + check_conn.close() + if obj_count > 0: + logger.error( + "Temp tablespace has %d object(s). " + "Please move or drop all objects from the temp tablespace, " + "then run 'juju resolved postgresql/' to retry.", + obj_count, + ) + except Exception: + logger.debug("Could not query temp tablespace for blocking objects") + return False + finally: + if cursor is not None: + cursor.close() + if connection is not None: + connection.close() + + return True + __all__ = [ "PostgreSQLRefreshBase", diff --git a/tests/unit/test_refresh.py b/tests/unit/test_refresh.py index 16dc6b9..3541480 100644 --- a/tests/unit/test_refresh.py +++ b/tests/unit/test_refresh.py @@ -17,6 +17,7 @@ PostgreSQLRefreshK8s, RefreshManager, ) +from tenacity import RetryError CHARM_VERSION = "16/1.0.0" @@ -336,3 +337,194 @@ def test_reconcile_refresh_status_ignores_unrelated_status(refresh_manager, char assert charm.unit.status == BlockedStatus("unrelated") refresh_manager.refresh.unit_status_lower_priority.assert_not_called() + + +@pytest.fixture +def vm_manager(charm, set_default_status): + """A refresh manager on the VM substrate wired to the mock charm.""" + state = MagicMock(name="state") + state.substrate = Substrates.VM + return RefreshManager( + state=state, + workload=MagicMock(name="workload"), + charm=charm, + set_default_status=set_default_status, + ) + + +@pytest.fixture +def refresh_vm(charm): + from single_kernel_postgresql.managers.refresh import PostgreSQLRefreshVM + + return PostgreSQLRefreshVM(workload_name="PostgreSQL", charm_name="postgresql", _charm=charm) + + +def test_refresh_snap_runs_the_post_refresh_flow(refresh_vm, charm): + refresh = MagicMock(name="refresh") + charm.refresh_manager = MagicMock(name="refresh_manager") + + refresh_vm.refresh_snap(snap_name="charmed-postgresql", snap_revision="366", refresh=refresh) + + charm.update_config.assert_called_once_with(refresh=refresh) + charm.workload.install_snap_package.assert_called_once_with(revision="366", refresh=refresh) + charm.refresh_manager.post_snap_refresh.assert_called_once_with(refresh) + + +def test_post_snap_refresh_blocks_when_patroni_fails_to_start(vm_manager, charm): + charm.patroni_manager.start_patroni.return_value = False + vm_manager.refresh.next_unit_allowed_to_refresh = False + + vm_manager.post_snap_refresh(vm_manager.refresh) + + assert charm.unit.status == BlockedStatus("Failed to start PostgreSQL") + charm.post_refresh_side_effects.assert_not_called() + assert vm_manager.refresh.next_unit_allowed_to_refresh is False + + +def test_post_snap_refresh_allows_next_unit_when_healthy(vm_manager, charm): + charm.patroni_manager.start_patroni.return_value = True + charm.patroni_manager.member_started = True + charm.patroni_manager.cluster_members = {"postgresql-2"} + charm.patroni_manager.is_replication_healthy.return_value = True + charm.unit.name = "postgresql/2" + lower_peer, middle_peer = MagicMock(), MagicMock() + lower_peer.name = "postgresql/0" + middle_peer.name = "postgresql/1" + charm.state.peer_relation = MagicMock(units=[lower_peer, middle_peer]) + + vm_manager.post_snap_refresh(vm_manager.refresh) + + charm.post_refresh_side_effects.assert_called_once() + assert vm_manager.refresh.next_unit_allowed_to_refresh is True + assert charm.unit.status == ActiveStatus() + + +def test_post_snap_refresh_retries_exhausted_keeps_unit_blocked_from_refresh(vm_manager, charm): + charm.patroni_manager.start_patroni.return_value = True + charm.patroni_manager.member_started = False + vm_manager.refresh.next_unit_allowed_to_refresh = False + + with patch( + "single_kernel_postgresql.managers.refresh.Retrying", + side_effect=RetryError("last attempt"), + ): + vm_manager.post_snap_refresh(vm_manager.refresh) + + charm.post_refresh_side_effects.assert_called_once() + assert vm_manager.refresh.next_unit_allowed_to_refresh is False + + +def test_on_init_marks_next_unit_allowed_when_not_in_progress(vm_manager): + vm_manager.refresh.next_unit_allowed_to_refresh = False + vm_manager.refresh.in_progress = False + + with patch.object(vm_manager, "migrate_temp_tablespace_location") as migrate: + vm_manager.on_init() + + migrate.assert_called_once() + assert vm_manager.refresh.next_unit_allowed_to_refresh is True + + +def test_on_init_runs_post_refresh_when_in_progress(vm_manager): + vm_manager.refresh.next_unit_allowed_to_refresh = False + vm_manager.refresh.in_progress = True + + with patch.object(vm_manager, "post_snap_refresh") as post_snap_refresh: + vm_manager.on_init() + + post_snap_refresh.assert_called_once_with(vm_manager.refresh) + + +def test_migrate_temp_tablespace_skips_without_primary_endpoint(vm_manager, charm): + charm.primary_endpoint = None + assert vm_manager.migrate_temp_tablespace_location() is True + assert vm_manager.migrate_temp_tablespace_location(required=True) is False + + +def test_migrate_temp_tablespace_skips_for_async_relation(vm_manager, charm): + charm.primary_endpoint = "10.1.0.1" + charm.has_async_replication_relation.return_value = True + + assert vm_manager.migrate_temp_tablespace_location() is True + + +def test_execute_temp_tablespace_migration_noop_when_already_migrated(vm_manager, charm): + temp_data_dir = MagicMock() + temp_data_dir.__str__.return_value = "/var/snap/charmed-postgresql/common/data/temp/16/main" + temp_root = MagicMock() + temp_root.__str__.return_value = "/var/snap/charmed-postgresql/common/data/temp" + charm.workload.paths.temp = temp_data_dir + charm.workload.paths.temp.parent = temp_root + cursor = charm.postgresql._connect_to_database.return_value.cursor.return_value + cursor.fetchone.return_value = ("/var/snap/charmed-postgresql/common/data/temp/16/main",) + + assert vm_manager._execute_temp_tablespace_migration("10.1.0.1") is True + cursor.execute.assert_called_once_with( + "SELECT pg_tablespace_location(oid) FROM pg_tablespace WHERE spcname='temp';" + ) + + +def test_execute_temp_tablespace_migration_performs_the_ddl(vm_manager, charm): + temp_data_dir = MagicMock() + temp_data_dir.__str__.return_value = "/var/snap/charmed-postgresql/common/data/temp/16/main" + temp_root = MagicMock() + temp_root.__str__.return_value = "/var/snap/charmed-postgresql/common/data/temp" + charm.workload.paths.temp = temp_data_dir + charm.workload.paths.temp.parent = temp_root + cursor = charm.postgresql._connect_to_database.return_value.cursor.return_value + cursor.fetchone.return_value = ("/var/snap/charmed-postgresql/common/data/temp",) + + assert vm_manager._execute_temp_tablespace_migration("10.1.0.1") is True + cursor.execute.assert_any_call("DROP TABLESPACE temp;") + cursor.execute.assert_any_call( + "CREATE TABLESPACE temp LOCATION '/var/snap/charmed-postgresql/common/data/temp/16/main';" + ) + cursor.execute.assert_any_call("GRANT CREATE ON TABLESPACE temp TO public;") + cursor.execute.assert_any_call("CHECKPOINT;") + + +def test_execute_temp_tablespace_migration_skips_unexpected_location(vm_manager, charm): + temp_data_dir = MagicMock() + temp_data_dir.__str__.return_value = "/var/snap/charmed-postgresql/common/data/temp/16/main" + temp_root = MagicMock() + temp_root.__str__.return_value = "/var/snap/charmed-postgresql/common/data/temp" + charm.workload.paths.temp = temp_data_dir + charm.workload.paths.temp.parent = temp_root + cursor = charm.postgresql._connect_to_database.return_value.cursor.return_value + cursor.fetchone.return_value = ("/somewhere/else",) + + assert vm_manager._execute_temp_tablespace_migration("10.1.0.1") is True + cursor.execute.assert_called_once() + + +def test_check_and_update_internal_cert_regenerates_on_cn_mismatch(vm_manager, charm): + cert = MagicMock() + cert.subject.get_attributes_for_oid.return_value = [MagicMock(value="10.9.9.9")] + charm.state.get_secret.return_value = "raw-cert" + charm.state.unit_ip = "10.1.0.1" + + with patch( + "single_kernel_postgresql.managers.refresh.load_pem_x509_certificate", + return_value=cert, + ): + vm_manager.check_and_update_internal_cert() + + charm.tls_manager.generate_internal_peer_cert.assert_called_once() + charm.tls_manager.push_tls_files.assert_called_once() + charm.update_config.assert_called_once() + + +def test_check_and_update_internal_cert_keeps_matching_cert(vm_manager, charm): + cert = MagicMock() + cert.subject.get_attributes_for_oid.return_value = [MagicMock(value="10.1.0.1")] + charm.state.get_secret.return_value = "raw-cert" + charm.state.unit_ip = "10.1.0.1" + + with patch( + "single_kernel_postgresql.managers.refresh.load_pem_x509_certificate", + return_value=cert, + ): + vm_manager.check_and_update_internal_cert() + + charm.tls_manager.generate_internal_peer_cert.assert_not_called() + charm.update_config.assert_not_called()