diff --git a/poetry.lock b/poetry.lock index 7fe359e1812..8a8c3c6ab03 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 2.3.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 2.4.1 and should not be changed by hand. [[package]] name = "allure-pytest" @@ -2484,6 +2484,34 @@ files = [ docs = ["myst-parser", "pydata-sphinx-theme", "sphinx"] test = ["argcomplete (>=3.0.3)", "mypy (>=1.17.0,<1.19)", "pre-commit", "pytest (>=7.0,<8.2)", "pytest-mock", "pytest-mypy-testing"] +[[package]] +name = "ty" +version = "0.0.42" +description = "An extremely fast Python type checker, written in Rust." +optional = false +python-versions = ">=3.8" +groups = ["lint"] +files = [ + {file = "ty-0.0.42-py3-none-linux_armv6l.whl", hash = "sha256:c08a0066610c13627b7d7ad758adb96ca99685791e641eb26837e20803851c53"}, + {file = "ty-0.0.42-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:3e944ee4e3d5cdaf70e4ea87f9dd474cc3db612837b50a3ce57afa8da400ecc2"}, + {file = "ty-0.0.42-py3-none-macosx_11_0_arm64.whl", hash = "sha256:603085306e4aac2ce592b39119a4b49ebf8b780cd394e2cfc7dbf3fd8228f954"}, + {file = "ty-0.0.42-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a58f17834d7f078c49326a01111a5aac16c979a774b98cdfd8e2350068316676"}, + {file = "ty-0.0.42-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:e6ed1027f313202c5c74e376007d1eb5d214494299beb0ea047078b8ad307d40"}, + {file = "ty-0.0.42-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:063838d2360c1d2c065b45ca76a56ecd6df07fff6570813e74183236559e16d9"}, + {file = "ty-0.0.42-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:c3f9ed508dfae4cbc943d7766324dd9c57ac8302c8543505fc29cae8ed425fe9"}, + {file = "ty-0.0.42-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:0fe1eb7d98472ac56ac19fec51c6ed8fe56d86ea0d232a11a127e8c62c882a66"}, + {file = "ty-0.0.42-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:de75f9e78bfa81209f2f297528758977cfd4518ba35ef45a0acb516c892a27a5"}, + {file = "ty-0.0.42-py3-none-manylinux_2_31_riscv64.whl", hash = "sha256:c63281f2f1d4df339117fcd4a6dfe17cb999f84eafe707b30e9ebbe26f0bb54a"}, + {file = "ty-0.0.42-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:19e3856477f25255f772851fa7f16f5356c4e1927324d074d49c7bc9a9b211e1"}, + {file = "ty-0.0.42-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:2f0f4acac9028264cee5ea0b88229df0b9b2586fc917dbadb6ee35a0e99e8b06"}, + {file = "ty-0.0.42-py3-none-musllinux_1_2_i686.whl", hash = "sha256:ae244c84e30fdf2bb1a3cbf2b973da8aa535e57c701f12db44b2939604586c04"}, + {file = "ty-0.0.42-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:2430434f4a52bec0da552ff6a061dcc1c5d11973259248679a1146d776c12f37"}, + {file = "ty-0.0.42-py3-none-win32.whl", hash = "sha256:984a55c2fe63b40dac03f5a144b99033c7ed720eb7611787e3f0bd49af8dcf12"}, + {file = "ty-0.0.42-py3-none-win_amd64.whl", hash = "sha256:f7afd81b10b377d9d4ce6aad355a4f47fd37d47f443118c01ca6e79d46fe6608"}, + {file = "ty-0.0.42-py3-none-win_arm64.whl", hash = "sha256:4068c24b0b264fc9f1901e06b97988a041fcaa36c90f18d7747f05124701c7b3"}, + {file = "ty-0.0.42.tar.gz", hash = "sha256:70f5553ac678fc63558d4d77b08a18a68a228f44be2a2fe1afc3f5988db662e7"}, +] + [[package]] name = "typing-extensions" version = "4.15.0" @@ -2649,4 +2677,4 @@ h11 = ">=0.16.0,<1" [metadata] lock-version = "2.1" python-versions = "^3.10" -content-hash = "13008404e9c39a5a91db7a61fae6baec14698861c47760cad2bcd87dfe86b68e" +content-hash = "283d09b8ba4855040753248f32ee448ec7348bb86160f79d7601bd8882959e90" diff --git a/pyproject.toml b/pyproject.toml index 1dc88fb8009..30d459e3922 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,7 @@ optional = true [tool.poetry.group.lint.dependencies] codespell = "^2.4.2" +ty = "^0.0.42" [tool.poetry.group.unit] optional = true @@ -136,3 +137,11 @@ max-complexity = 10 [tool.ruff.lint.pydocstyle] convention = "google" + +[tool.ty.environment] +python = ".tox/lint/" +extra-paths = ["./lib"] + +[tool.ty.src] +include = ["src", "scripts"] +exclude = ["tests"] diff --git a/src/authorisation_rules_observer.py b/src/authorisation_rules_observer.py index 9751efcedd6..20e2514d56b 100644 --- a/src/authorisation_rules_observer.py +++ b/src/authorisation_rules_observer.py @@ -118,7 +118,7 @@ def start_authorisation_rules_observer(self): def stop_authorisation_rules_observer(self): """Stop the authorisation rules observer process.""" if ( - self._charm.peers is None + self._charm._peers is None or "authorisation-rules-observer-pid" not in self._charm.unit_peer_data ): return diff --git a/src/backups.py b/src/backups.py index 7b1f4a09550..bb25408356c 100644 --- a/src/backups.py +++ b/src/backups.py @@ -224,6 +224,8 @@ def can_use_s3_repository(self) -> tuple[bool, str | None]: ]) if error != "": raise Exception(error) + if system_identifier_from_instance is None: + raise Exception("Failed to get system identifier from instance") system_identifier_from_instance = next( line for line in system_identifier_from_instance.splitlines() @@ -301,7 +303,7 @@ def _empty_data_files(self) -> None: self.container.exec(["rm", "-r", "/var/lib/postgresql/data/pgdata"]).wait_output() except ExecError as e: # If previous PITR restore was unsuccessful, there is no such directory. - if "No such file or directory" not in e.stderr: + if e.stderr is None or "No such file or directory" not in e.stderr: logger.exception( "Failed to empty data directory in prep for backup restore", exc_info=e ) @@ -387,6 +389,8 @@ def _generate_backup_list_output(self) -> str: """ backup_list = [] output, _ = self._execute_command(["pgbackrest", "info", "--output=json"]) + if output is None: + return "" backups = json.loads(output)[0]["backup"] for backup in backups: backup_id, backup_type = self._parse_backup_id(backup["label"]) @@ -452,6 +456,8 @@ def _list_backups(self, show_failed: bool, parse=True) -> dict[str, tuple[str, s the S3 bucket. """ output, _ = self._execute_command(["pgbackrest", "info", "--output=json"]) + if output is None: + return dict[str, tuple[str, str]]() repository_info = next(iter(json.loads(output)), None) # If there are no backups, returns an empty dict. @@ -486,6 +492,8 @@ def _list_timelines(self) -> dict[str, tuple[str, str]]: "\\.history$", "--output=json", ]) + if output is None: + return dict[str, tuple[str, str]]() repository = json.loads(output).items() if repository is None: @@ -778,7 +786,7 @@ def _on_s3_credentials_checks(self, event: CredentialsChangedEvent) -> bool: return False return True - def _on_s3_credential_changed(self, event: CredentialsChangedEvent) -> bool | None: + def _on_s3_credential_changed(self, event: CredentialsChangedEvent) -> None: """Call the stanza initialization when the credentials or the connection info change.""" if not self._on_s3_credentials_checks(event): return @@ -814,7 +822,9 @@ def _on_s3_credential_changed_primary(self, event: HookEvent) -> bool: can_use_s3_repository, validation_message = self.can_use_s3_repository() if not can_use_s3_repository: - self._s3_initialization_set_failure(validation_message) + self._s3_initialization_set_failure( + validation_message or ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE + ) return False if not self._initialise_stanza(event): @@ -912,7 +922,7 @@ def _on_create_backup_action(self, event) -> None: # noqa: C901 # Recover the backup id from the logs. backup_label_stdout_line = re.findall( - r"(new backup label = )([0-9]{8}[-][0-9]{6}[F])$", e.stdout, re.MULTILINE + r"(new backup label = )([0-9]{8}[-][0-9]{6}[F])$", e.stdout or "", re.MULTILINE ) if len(backup_label_stdout_line) > 0: backup_id = backup_label_stdout_line[0][1] @@ -1134,7 +1144,7 @@ def _generate_fake_backup_id(self, backup_type: str) -> str: if backup_type == "full": return datetime.strftime(datetime.now(), "%Y%m%d-%H%M%SF") if backup_type == "differential": - backups = self._list_backups(show_failed=False, parse=False).keys() + backups = list(self._list_backups(show_failed=False, parse=False).keys()) last_full_backup = None for label in backups[::-1]: if label.endswith("F"): @@ -1145,12 +1155,13 @@ def _generate_fake_backup_id(self, backup_type: str) -> str: raise TypeError("Differential backup requested but no previous full backup") return f"{last_full_backup}_{datetime.strftime(datetime.now(), '%Y%m%d-%H%M%SD')}" if backup_type == "incremental": - backups = self._list_backups(show_failed=False, parse=False).keys() + backups = list(self._list_backups(show_failed=False, parse=False).keys()) if not backups: raise TypeError("Incremental backup requested but no previous successful backup") return f"{backups[-1]}_{datetime.strftime(datetime.now(), '%Y%m%d-%H%M%SI')}" + raise ValueError(f"Unknown backup type: {backup_type}") - def _fetch_backup_from_id(self, backup_id: str) -> str: + def _fetch_backup_from_id(self, backup_id: str) -> str | None: """Fetches backup's pgbackrest label from backup id.""" timestamp = f"{datetime.strftime(datetime.strptime(backup_id, '%Y-%m-%dT%H:%M:%SZ'), '%Y%m%d-%H%M%S')}" backups = self._list_backups(show_failed=False, parse=False).keys() @@ -1169,7 +1180,7 @@ def _pre_restore_checks(self, event: ActionEvent) -> bool: are_backup_settings_ok, validation_message = self._are_backup_settings_ok() if not are_backup_settings_ok: logger.error(f"Restore failed: {validation_message}") - event.fail(validation_message) + event.fail(validation_message or "Backup settings check failed") return False if not event.params.get("backup-id") and event.params.get("restore-to-time") is None: diff --git a/src/charm.py b/src/charm.py index f7a6d9f0422..9ce2046bcea 100755 --- a/src/charm.py +++ b/src/charm.py @@ -12,11 +12,12 @@ import shutil import sys import time +from collections.abc import MutableMapping from datetime import datetime from functools import cached_property from hashlib import shake_128 from pathlib import Path -from typing import Literal, get_args +from typing import Literal, cast, get_args from urllib.parse import urlparse from authorisation_rules_observer import ( @@ -27,6 +28,7 @@ # First platform-specific import, will fail on wrong architecture try: import psycopg2 + import psycopg2.errors except ModuleNotFoundError: from ops.main import main @@ -84,6 +86,7 @@ ChangeError, ExecError, Layer, + LayerDict, PathError, ProtocolError, ServiceInfo, @@ -155,7 +158,7 @@ logging.getLogger("httpcore").setLevel(logging.ERROR) logging.getLogger("httpx").setLevel(logging.ERROR) -Scopes = Literal[APP_SCOPE, UNIT_SCOPE] +Scopes = Literal["app", "unit"] PASSWORD_USERS = [*SYSTEM_USERS, "patroni"] @@ -274,10 +277,11 @@ def _on_databases_change(self, _): self.update_config() logger.debug("databases changed") timestamp = datetime.now() - self._peers.data[self.unit].update({"pg_hba_needs_update_timestamp": str(timestamp)}) + if self._peers is not None: + self._peers.data[self.unit].update({"pg_hba_needs_update_timestamp": str(timestamp)}) logger.debug(f"authorisation rules changed at {timestamp}") - def _generate_metrics_jobs(self, enable_tls: bool) -> dict: + def _generate_metrics_jobs(self, enable_tls: bool) -> list: """Generate spec for Prometheus scraping.""" return [ {"static_configs": [{"targets": [f"*:{METRICS_PORT}"]}]}, @@ -297,15 +301,16 @@ def app_units(self) -> set[Unit]: return {self.unit, *self._peers.units} - def scoped_peer_data(self, scope: Scopes) -> dict | None: + def scoped_peer_data(self, scope: Scopes) -> MutableMapping[str, str] | None: """Returns peer data based on scope.""" if scope == APP_SCOPE: return self.app_peer_data elif scope == UNIT_SCOPE: return self.unit_peer_data + return None @property - def app_peer_data(self) -> dict: + def app_peer_data(self) -> MutableMapping[str, str]: """Application peer relation data object.""" relation = self.model.get_relation(PEER) if relation is None: @@ -314,7 +319,7 @@ def app_peer_data(self) -> dict: return relation.data[self.app] @property - def unit_peer_data(self) -> dict: + def unit_peer_data(self) -> MutableMapping[str, str]: """Unit peer relation data object.""" relation = self.model.get_relation(PEER) if relation is None: @@ -322,7 +327,7 @@ def unit_peer_data(self) -> dict: return relation.data[self.unit] - def _peer_data(self, scope: Scopes) -> dict: + def _peer_data(self, scope: Scopes) -> MutableMapping[str, str]: """Return corresponding databag for app/unit.""" relation = self.model.get_relation(PEER) if relation is None: @@ -340,8 +345,7 @@ def peer_relation_data(self, scope: Scopes) -> DataPeerData: """Returns the peer relation data per scope.""" if scope == APP_SCOPE: return self.peer_relation_app - elif scope == UNIT_SCOPE: - return self.peer_relation_unit + return self.peer_relation_unit def _translate_field_to_secret_key(self, key: str) -> str: """Change 'key' to secrets-compatible key field.""" @@ -379,7 +383,8 @@ def set_secret(self, scope: Scopes, key: str, value: str | None) -> str | None: secret_key = self._translate_field_to_secret_key(key) # Old translation in databag is to be deleted - self.scoped_peer_data(scope).pop(key, None) + if (scoped_data := self.scoped_peer_data(scope)) is not None: + scoped_data.pop(key, None) self.peer_relation_data(scope).set_secret(peers.id, secret_key, value) def remove_secret(self, scope: Scopes, key: str) -> None: @@ -426,7 +431,7 @@ def postgresql(self) -> PostgreSQL: primary_host=self.primary_endpoint, current_host=self.endpoint, user=USER, - password=self.get_secret(APP_SCOPE, f"{USER}-password"), + password=self.get_secret(APP_SCOPE, f"{USER}-password") or "", database=DATABASE_DEFAULT_NAME, system_users=SYSTEM_USERS, ) @@ -475,7 +480,7 @@ def get_unit_ip(self, unit: Unit) -> str | None: if unit == self.unit: return str(self.model.get_binding(PEER).network.bind_address) # Check if host is a peer. - elif unit in self._peers.data: + elif self._peers is not None and unit in self._peers.data: return str(self._peers.data[unit].get("private-address")) # Return None if the unit is not a peer neither the current unit. else: @@ -754,7 +759,7 @@ def _handle_enable_disable_extensions(self, original_status, extensions, databas self.unit.status = WaitingStatus("Updating extensions") try: self.postgresql.enable_disable_extensions(extensions, database) - except psycopg2.errors.DependentObjectsStillExist as e: + except psycopg2.errors.DependentObjectsStillExist as e: # type: ignore logger.error( "Failed to disable plugin: %s\nWas the plugin enabled manually? If so, update charm config with `juju config postgresql-k8s plugin__enable=True`", str(e), @@ -853,7 +858,7 @@ def _hosts(self) -> set: """ peers = self.model.get_relation(PEER) hosts = [self._unit_name_to_pod_name(self.unit.name)] + [ - self._unit_name_to_pod_name(unit.name) for unit in peers.units + self._unit_name_to_pod_name(unit.name) for unit in (peers.units if peers else set()) ] return set(hosts) @@ -910,7 +915,11 @@ def fix_leader_annotation(self) -> bool: client = Client() try: endpoint = client.get(Endpoints, name=self.cluster_name, namespace=self._namespace) - if "leader" not in endpoint.metadata.annotations: + if ( + endpoint.metadata is None + or not endpoint.metadata.annotations + or "leader" not in endpoint.metadata.annotations + ): patch = { "metadata": { "annotations": {"leader": self._unit_name_to_pod_name(self._unit)} @@ -941,6 +950,8 @@ def _create_pgdata(self, container: Container): make_parents=True, ) # Also, fix the permissions from the parent directory. + if self._storage_path is None: + return container.exec([ "chown", f"{WORKLOAD_OS_USER}:{WORKLOAD_OS_GROUP}", @@ -1045,7 +1056,7 @@ def _set_active_status(self): except (RetryError, RequestsConnectionError) as e: logger.error(f"failed to get primary with error {e}") - def _initialize_cluster(self, event: WorkloadEvent) -> bool: + def _initialize_cluster(self, event: HookEvent) -> bool: # Add the labels needed for replication in this pod. # This also enables the member as part of the cluster. try: @@ -1102,7 +1113,8 @@ def _initialize_cluster(self, event: WorkloadEvent) -> bool: self.postgresql.grant_internal_access_group_memberships() # Mark the cluster as initialised. - self._peers.data[self.app]["cluster_initialised"] = "True" + if self._peers is not None: + self._peers.data[self.app]["cluster_initialised"] = "True" return True @@ -1166,6 +1178,7 @@ def _create_services(self) -> None: name=f"{self.app.name}-0", namespace=self.model.name, ) + pod0_owner_refs = pod0.metadata.ownerReferences if pod0.metadata is not None else None services = { "primary": "primary", @@ -1176,7 +1189,7 @@ def _create_services(self) -> None: metadata=ObjectMeta( name=f"{self._name}-{service_name_suffix}", namespace=self.model.name, - ownerReferences=pod0.metadata.ownerReferences, + ownerReferences=pod0_owner_refs, labels={ "app.kubernetes.io/name": self.app.name, }, @@ -1203,8 +1216,6 @@ def _create_services(self) -> None: ) client.apply( obj=service, - name=service.metadata.name, - namespace=service.metadata.namespace, force=True, field_manager=self.model.app.name, ) @@ -1464,34 +1475,37 @@ def _on_stop(self, _): logger.exception("failed to get the k8s resources created by the charm and Patroni") return + pod0_meta = pod0.metadata for resource in resources_to_patch: + resource_meta = resource.metadata + if resource_meta is None: + continue # Ignore resources created by Juju or the charm # (which are already patched). + pod0_owner_refs = pod0_meta.ownerReferences if pod0_meta is not None else None if ( type(resource) is Service - and resource.metadata.name + and resource_meta.name in [ self._name, f"{self._name}-endpoints", f"{self._name}-primary", f"{self._name}-replicas", ] - ) or resource.metadata.ownerReferences == pod0.metadata.ownerReferences: + ) or resource_meta.ownerReferences == pod0_owner_refs: continue # Patch the resource. try: - resource.metadata.ownerReferences = pod0.metadata.ownerReferences - resource.metadata.managedFields = None + resource_meta.ownerReferences = pod0_owner_refs + resource_meta.managedFields = None client.apply( obj=resource, - name=resource.metadata.name, - namespace=resource.metadata.namespace, force=True, ) except ApiError: # Only log the exception. logger.exception( - f"failed to patch k8s {type(resource).__name__} {resource.metadata.name}" + f"failed to patch k8s {type(resource).__name__} {resource_meta.name}" ) def _on_update_status_early_exit_checks(self, container) -> bool: @@ -1637,7 +1651,7 @@ def _was_restore_successful(self, container: Container, service: ServiceInfo) -> "stanza": "", "s3-initialization-start": "", "s3-initialization-done": "", - "s3-initialization-block-message": validation_message, + "s3-initialization-block-message": validation_message or "", }) return True @@ -1645,6 +1659,8 @@ def _was_restore_successful(self, container: Container, service: ServiceInfo) -> @cached_property def _patroni(self): """Returns an instance of the Patroni object.""" + if self._storage_path is None: + raise RuntimeError("storage path must be set") return Patroni( self, self._endpoint, @@ -1652,10 +1668,10 @@ def _patroni(self): self.primary_endpoint, self._namespace, self._storage_path, - self.get_secret(APP_SCOPE, USER_PASSWORD_KEY), - self.get_secret(APP_SCOPE, REPLICATION_PASSWORD_KEY), - self.get_secret(APP_SCOPE, REWIND_PASSWORD_KEY), - self.get_secret(APP_SCOPE, PATRONI_PASSWORD_KEY), + self.get_secret(APP_SCOPE, USER_PASSWORD_KEY) or "", + self.get_secret(APP_SCOPE, REPLICATION_PASSWORD_KEY) or "", + self.get_secret(APP_SCOPE, REWIND_PASSWORD_KEY) or "", + self.get_secret(APP_SCOPE, PATRONI_PASSWORD_KEY) or "", ) @property @@ -1743,6 +1759,8 @@ def _update_endpoints( if not self.unit.is_leader(): return + if self._peers is None: + return endpoints = json.loads(self._peers.data[self.app].get("endpoints", "[]")) if endpoint_to_add: endpoints.append(endpoint_to_add) @@ -1882,10 +1900,10 @@ def _postgresql_layer(self) -> Layer: } }, } - return Layer(layer_config) + return Layer(cast("LayerDict", layer_config)) @property - def _peers(self) -> Relation: + def _peers(self) -> Relation | None: """Fetch the peer relation. Returns: @@ -2012,7 +2030,12 @@ def _restart_metrics_service(self) -> None: ): container.add_layer( self.metrics_service, - Layer({"services": {self.metrics_service: self._generate_metrics_service()}}), + Layer( + cast( + "LayerDict", + {"services": {self.metrics_service: self._generate_metrics_service()}}, + ) + ), combine=True, ) container.restart(self.metrics_service) @@ -2038,7 +2061,12 @@ def _restart_ldap_sync_service(self) -> None: if self.is_primary and self.is_ldap_enabled: container.add_layer( self.ldap_sync_service, - Layer({"services": {self.ldap_sync_service: self._generate_ldap_service()}}), + Layer( + cast( + "LayerDict", + {"services": {self.ldap_sync_service: self._generate_ldap_service()}}, + ) + ), combine=True, ) logger.debug("Starting LDAP sync service") @@ -2217,7 +2245,7 @@ def _api_update_config(self, available_cpu_cores: int) -> bool: else: max_connections = max(4 * available_cpu_cores, 100) - cfg_patch = { + cfg_patch: dict[str, object] = { "max_connections": max_connections, "max_prepared_transactions": self.config.memory_max_prepared_transactions, "max_replication_slots": 25, @@ -2266,7 +2294,7 @@ def _build_postgresql_parameters( # Build PostgreSQL parameters. pg_parameters = self.postgresql.build_postgresql_parameters( - self.model.config, available_memory, limit_memory + dict(self.model.config), available_memory, limit_memory ) # Calculate and merge worker process configurations @@ -2372,7 +2400,9 @@ def _validate_config_options(self) -> None: if not self.postgresql.validate_group_map(self.config.ldap_map): raise ValueError("ldap_map config option has an invalid value") - if not self.postgresql.validate_date_style(self.config.request_date_style): + if self.config.request_date_style is not None and not self.postgresql.validate_date_style( + self.config.request_date_style + ): raise ValueError("request_date_style config option has an invalid value") if self.config.request_time_zone not in self.postgresql.get_postgresql_timezones(): @@ -2475,6 +2505,8 @@ def _get_node_name_for_pod(self) -> str: pod = client.get( Pod, name=self._unit_name_to_pod_name(self.unit.name), namespace=self._namespace ) + if pod.spec is None or pod.spec.nodeName is None: + raise RuntimeError("pod spec or node name is not set") return pod.spec.nodeName def get_resources_limits(self, container_name: str) -> dict: @@ -2488,21 +2520,29 @@ def get_resources_limits(self, container_name: str) -> dict: Pod, self._unit_name_to_pod_name(self.unit.name), namespace=self._namespace ) + if pod.spec is None: + return {} for container in pod.spec.containers: if container.name == container_name: - return container.resources.limits or {} + return ( + container.resources.limits if container.resources is not None else None + ) or {} return {} def get_node_allocable_memory(self) -> int: """Return the allocable memory in bytes for the current K8S node.""" client = Client() - node = client.get(Node, name=self._get_node_name_for_pod(), namespace=self._namespace) + node = client.get(Node, name=self._get_node_name_for_pod()) + if node.status is None or node.status.allocatable is None: + raise RuntimeError("node status or allocatable is not set") return any_memory_to_bytes(node.status.allocatable["memory"]) def get_node_cpu_cores(self) -> int: """Return the number of CPU cores for the current K8S node.""" client = Client() - node = client.get(Node, name=self._get_node_name_for_pod(), namespace=self._namespace) + node = client.get(Node, name=self._get_node_name_for_pod()) + if node.status is None or node.status.allocatable is None: + raise RuntimeError("node status or allocatable is not set") return any_cpu_to_cores(node.status.allocatable["cpu"]) def get_available_resources(self) -> tuple[int, int]: @@ -2792,7 +2832,7 @@ def get_ldap_parameters(self) -> dict: if relation_data is None: return {} - params = { + params: dict[str, object] = { "ldapbasedn": relation_data.base_dn, "ldapbinddn": relation_data.bind_dn, "ldapbindpasswd": relation_data.bind_password, diff --git a/src/ldap.py b/src/ldap.py index a13b8c29fb5..eb154f729e8 100644 --- a/src/ldap.py +++ b/src/ldap.py @@ -31,7 +31,7 @@ def __init__(self, charm, relation_name: str): self.framework.observe(self.ldap.on.ldap_unavailable, self._on_ldap_unavailable) @property - def _relation(self) -> Relation: + def _relation(self) -> Relation | None: """Return the relation object.""" return self.model.get_relation(self.relation_name) diff --git a/src/patroni.py b/src/patroni.py index f499796f6a9..b49e7d97818 100644 --- a/src/patroni.py +++ b/src/patroni.py @@ -19,6 +19,7 @@ from httpx import AsyncClient, BasicAuth, HTTPError from jinja2 import Template from ops.pebble import Error +from requests.auth import HTTPBasicAuth from tenacity import ( Future, RetryError, @@ -100,10 +101,10 @@ def __init__( primary_endpoint: str, namespace: str, storage_path: str, - superuser_password: str, - replication_password: str, - rewind_password: str, - patroni_password: str, + superuser_password: str | None, + replication_password: str | None, + rewind_password: str | None, + patroni_password: str | None, ): self._charm = charm self._endpoint = endpoint @@ -125,8 +126,9 @@ def _verify(self) -> str | bool: return f"{self._storage_path}/{TLS_CA_FILE}" if self._charm.is_peer_data_tls_set else True @cached_property - def _patroni_auth(self) -> requests.auth.HTTPBasicAuth: - return requests.auth.HTTPBasicAuth("patroni", self._patroni_password) + def _patroni_auth(self) -> HTTPBasicAuth | None: + if self._patroni_password: + return HTTPBasicAuth("patroni", self._patroni_password) @cached_property def _patroni_async_auth(self) -> BasicAuth | None: @@ -391,6 +393,8 @@ def is_replication_healthy(self) -> bool: for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3)): with attempt: primary = self.get_primary() + if primary is None: + raise NotReadyError("primary not found") unit_id = primary.split("-")[-1] primary_endpoint = ( f"{self._charm.app.name}-{unit_id}.{self._charm.app.name}-endpoints" @@ -450,8 +454,9 @@ def get_patroni_health(self) -> dict[str, str]: timeout=PATRONI_TIMEOUT, auth=self._patroni_auth, ) + logger.debug("API get_patroni_health: %s (%s)", r, r.elapsed.total_seconds()) - return r.json() + return r.json() @property def member_started(self) -> bool: @@ -656,7 +661,7 @@ def render_patroni_yml_file( restore_stanza=restore_stanza, synchronous_node_count=self._synchronous_node_count, maximum_lag_on_failover=self._charm.config.durability_maximum_lag_on_failover, - version=self.rock_postgresql_version.split(".")[0], + version=(self.rock_postgresql_version or "").split(".")[0], pg_parameters=parameters, primary_cluster_endpoint=self._charm.async_replication.get_primary_cluster_endpoint(), extra_replication_endpoints=self._charm.async_replication.get_standby_endpoints(), diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 3d3f6f0a314..f3ecdf02756 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -177,6 +177,8 @@ def _configure_primary_cluster( def _configure_standby_cluster(self, event: RelationChangedEvent) -> bool: """Configure the standby cluster.""" relation = self._relation + if relation is None: + return False if relation.name == REPLICATION_CONSUMER_RELATION and not self._update_internal_secret(): logger.debug("Secret not found, deferring event") event.defer() @@ -240,6 +242,8 @@ def get_primary_cluster_endpoint(self) -> str | None: if primary_cluster is None or self.charm.app == primary_cluster: return None relation = self._relation + if relation is None: + return None primary_cluster_data = relation.data[relation.app].get("primary-cluster-data") if primary_cluster_data is None: return None @@ -253,14 +257,14 @@ def get_all_primary_cluster_endpoints(self) -> list[str]: if relation is None or primary_cluster is None or self.charm.app == primary_cluster: return [] return [ - relation.data[unit].get("unit-address") + addr for relation in [ self.model.get_relation(REPLICATION_OFFER_RELATION), self.model.get_relation(REPLICATION_CONSUMER_RELATION), ] if relation is not None for unit in relation.units - if relation.data[unit].get("unit-address") is not None + if (addr := relation.data[unit].get("unit-address")) is not None ] def _get_secret(self) -> Secret: @@ -295,14 +299,14 @@ def get_standby_endpoints(self) -> list[str]: if relation is None or primary_cluster is None or self.charm.app != primary_cluster: return [] return [ - relation.data[unit].get("unit-address") + addr for relation in [ self.model.get_relation(REPLICATION_OFFER_RELATION), self.model.get_relation(REPLICATION_CONSUMER_RELATION), ] if relation is not None for unit in relation.units - if relation.data[unit].get("unit-address") is not None + if (addr := relation.data[unit].get("unit-address")) is not None ] def get_system_identifier(self) -> tuple[str | None, str | None]: @@ -395,8 +399,14 @@ def _handle_forceful_promotion(self, event: ActionEvent) -> bool: except RetryError: pass if not primary_cluster_reachable: + relation = self._relation + relation_name = ( + relation.app.name + if relation is not None and relation.app is not None + else "primary cluster" + ) event.fail( - f"{self._relation.app.name} isn't reachable. Pass `force=true` to promote anyway." + f"{relation_name} isn't reachable. Pass `force=true` to promote anyway." ) return False else: @@ -419,6 +429,8 @@ def _handle_replication_change(self, event: ActionEvent) -> bool: return False relation = self._relation + if relation is None: + return False # Check if all units from the other cluster published their pod IPs in the relation data. # If not, fail the action telling that all units must publish their pod addresses in the @@ -538,7 +550,9 @@ def _on_async_relation_changed(self, event: RelationChangedEvent) -> None: def _on_async_relation_created(self, _) -> None: """Publish this unit address in the relation data.""" - self._relation.data[self.charm.unit].update({"unit-address": self._get_unit_ip()}) + relation = self._relation + if relation is not None: + relation.data[self.charm.unit].update({"unit-address": self._get_unit_ip()}) # Set the counter for new units. highest_promoted_cluster_counter = self._get_highest_promoted_cluster_counter_value() @@ -559,7 +573,7 @@ def _on_create_replication(self, event: ActionEvent) -> None: event.fail("There is already a replication set up.") return - if self._relation.name == REPLICATION_CONSUMER_RELATION: + if self._relation is not None and self._relation.name == REPLICATION_CONSUMER_RELATION: event.fail("This action must be run in the cluster where the offer was created.") return @@ -567,7 +581,9 @@ def _on_create_replication(self, event: ActionEvent) -> None: return # Set the replication name in the relation data. - self._relation.data[self.charm.app].update({"name": event.params["name"]}) + relation = self._relation + if relation is not None: + relation.data[self.charm.app].update({"name": event.params["name"]}) # Set the status. self.charm.unit.status = MaintenanceStatus("Creating replication...") @@ -631,6 +647,8 @@ def _primary_cluster_endpoint(self) -> str: def _re_emit_async_relation_changed_event(self) -> None: """Re-emit the async relation changed event.""" relation = self._relation + if relation is None: + return getattr(self.charm.on, f"{relation.name.replace('-', '_')}_relation_changed").emit( relation, app=relation.app, @@ -638,7 +656,7 @@ def _re_emit_async_relation_changed_event(self) -> None: ) @property - def _relation(self) -> Relation: + def _relation(self) -> Relation | None: """Return the relation object.""" for relation in [ self.model.get_relation(REPLICATION_OFFER_RELATION), @@ -646,6 +664,7 @@ def _relation(self) -> Relation: ]: if relation is not None: return relation + return None def _remove_previous_cluster_information(self) -> None: """Remove the previous cluster information.""" @@ -727,6 +746,8 @@ def update_async_replication_data(self) -> None: def _update_internal_secret(self) -> bool: # Update the secrets between the clusters. relation = self._relation + if relation is None: + return False primary_cluster_info = relation.data[relation.app].get("primary-cluster-data") secret_id = ( None @@ -751,12 +772,15 @@ def _update_primary_cluster_data( ) -> None: """Update the primary cluster data.""" async_relation = self._relation + if async_relation is None: + return if promoted_cluster_counter is not None: for relation in [async_relation, self.charm._peers]: - relation.data[self.charm.app].update({ - "promoted-cluster-counter": str(promoted_cluster_counter) - }) + if relation is not None: + relation.data[self.charm.app].update({ + "promoted-cluster-counter": str(promoted_cluster_counter) + }) primary_cluster_data = {"endpoint": self._primary_cluster_endpoint} @@ -764,7 +788,8 @@ def _update_primary_cluster_data( if async_relation.name == REPLICATION_OFFER_RELATION: secret = self._get_secret() secret.grant(async_relation) - primary_cluster_data["secret-id"] = secret.id + if secret.id is not None: + primary_cluster_data["secret-id"] = secret.id if system_identifier is not None: primary_cluster_data["system-id"] = system_identifier diff --git a/src/relations/db.py b/src/relations/db.py index 3b632b461a7..ea2b1280911 100644 --- a/src/relations/db.py +++ b/src/relations/db.py @@ -5,6 +5,7 @@ import logging from collections.abc import Iterable +from typing import TYPE_CHECKING from charms.postgresql_k8s.v0.postgresql import ( ACCESS_GROUP_RELATION, @@ -16,7 +17,6 @@ from ops import ( ActiveStatus, BlockedStatus, - CharmBase, Object, Relation, RelationBrokenEvent, @@ -24,6 +24,9 @@ RelationDepartedEvent, Unit, ) + +if TYPE_CHECKING: + from charm import PostgresqlOperatorCharm from pgconnstr import ConnectionString from constants import ( @@ -51,7 +54,7 @@ class DbProvides(Object): - relation-broken """ - def __init__(self, charm: CharmBase, admin: bool = False): + def __init__(self, charm: "PostgresqlOperatorCharm", admin: bool = False): """Constructor for DbProvides object. Args: @@ -302,12 +305,16 @@ def _on_relation_departed(self, event: RelationDepartedEvent) -> None: # is removed and receives relation broken events from related applications. # This is needed because of https://bugs.launchpad.net/juju/+bug/1979811. if event.departing_unit == self.charm.unit: - self.charm._peers.data[self.charm.unit].update({"departing": "True"}) + if self.charm._peers is not None: + self.charm._peers.data[self.charm.unit].update({"departing": "True"}) return if not self.charm.unit.is_leader(): return + if event.departing_unit is None: + return + if event.departing_unit.app == self.charm.app: # Just run for departing of remote units. return @@ -420,7 +427,7 @@ def _get_state(self) -> str: Returns: The state of this unit. Can be 'standalone', 'master', or 'standby'. """ - if len(self.charm._peers.units) == 0: + if self.charm._peers is None or len(self.charm._peers.units) == 0: return "standalone" if self.charm._patroni.get_primary(unit_name_pattern=True) == self.charm.unit.name: return "master" diff --git a/src/relations/postgresql_provider.py b/src/relations/postgresql_provider.py index c0106e68b3d..1341c5ad233 100644 --- a/src/relations/postgresql_provider.py +++ b/src/relations/postgresql_provider.py @@ -4,6 +4,7 @@ """Postgres client relation hooks & helpers.""" import logging +from typing import TYPE_CHECKING from charms.data_platform_libs.v0.data_interfaces import ( DatabaseProvides, @@ -21,7 +22,6 @@ from ops import ( ActiveStatus, BlockedStatus, - CharmBase, Object, Relation, RelationBrokenEvent, @@ -29,6 +29,9 @@ RelationDepartedEvent, ) +if TYPE_CHECKING: + from charm import PostgresqlOperatorCharm + from constants import DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE from utils import new_password @@ -43,7 +46,7 @@ class PostgreSQLProvider(Object): - relation-broken """ - def __init__(self, charm: CharmBase, relation_name: str = "database") -> None: + def __init__(self, charm: "PostgresqlOperatorCharm", relation_name: str = "database") -> None: """Constructor for PostgreSQLClientProvides object. Args: @@ -96,11 +99,12 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: return self.charm.update_config() - for key in self.charm._peers.data: + for key in self.charm._peers.data if self.charm._peers is not None else {}: # We skip the leader so we don't have to wait on the defer if ( key != self.charm.app and key != self.charm.unit + and self.charm._peers is not None and self.charm._peers.data[key].get("user_hash", "") != self.charm.generate_user_hash ): @@ -110,6 +114,9 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: # Retrieve the database name and extra user roles using the charm library. database = event.database + if database is None: + logger.error("database name must be set in the relation data") + return # Make sure the relation access-group is added to the list extra_user_roles = self._sanitize_extra_roles(event.extra_user_roles) @@ -174,7 +181,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: logger.exception(e) self.charm.unit.status = BlockedStatus( e.message - if issubclass(type(e), PostgreSQLCreateUserError) and e.message is not None + if isinstance(e, PostgreSQLCreateUserError) and e.message is not None else f"Failed to initialize {self.relation_name} relation" ) return @@ -241,7 +248,7 @@ def update_read_only_endpoint( # If there are no replicas, remove the read-only endpoint. endpoints = ( f"{self.charm.replicas_endpoint}:{DATABASE_PORT}" - if len(self.charm._peers.units) > 0 + if self.charm._peers is not None and len(self.charm._peers.units) > 0 else f"{self.charm.primary_endpoint}:{DATABASE_PORT}" ) diff --git a/src/upgrade.py b/src/upgrade.py index f7e4caf6c03..76e0ca9878a 100644 --- a/src/upgrade.py +++ b/src/upgrade.py @@ -5,6 +5,7 @@ import json import logging +from typing import TYPE_CHECKING from charms.data_platform_libs.v0.upgrade import ( ClusterNotReadyError, @@ -26,6 +27,9 @@ from patroni import SwitchoverFailedError from utils import new_password +if TYPE_CHECKING: + from charm import PostgresqlOperatorCharm + logger = logging.getLogger(__name__) @@ -46,7 +50,9 @@ def get_postgresql_k8s_dependencies_model() -> PostgreSQLDependencyModel: class PostgreSQLUpgrade(DataUpgrade): """PostgreSQL upgrade class.""" - def __init__(self, charm, model: BaseModel, **kwargs) -> None: + charm: "PostgresqlOperatorCharm" + + def __init__(self, charm: "PostgresqlOperatorCharm", model: BaseModel, **kwargs) -> None: """Initialize the class.""" super().__init__(charm, model, **kwargs) self.charm = charm @@ -68,7 +74,7 @@ def _handle_label_change(self) -> None: self.charm._patroni.switchover() except SwitchoverFailedError as e: logger.warning(f"Switchover failed: {e}") - if len(self.charm._peers.units) == 0 or unit_number == 1: + if (self.charm._peers is None or len(self.charm._peers.units) == 0) or unit_number == 1: # If the unit is the last to be upgraded before unit zero # or the only unit in the cluster, update the label. self.charm._create_services() @@ -186,6 +192,8 @@ def pre_upgrade_check(self) -> None: Raises: :class:`ClusterNotReadyError`: if cluster is not ready to upgrade """ + if self.peer_relation is None: + raise ClusterNotReadyError("peer relation must exist", "peer relation is missing") default_message = "Pre-upgrade check failed and cannot safely upgrade" if not self.charm._patroni.are_all_members_ready(): raise ClusterNotReadyError( @@ -239,6 +247,8 @@ def pre_upgrade_check(self) -> None: def _set_list_of_sync_standbys(self) -> None: """Set the list of desired sync-standbys in the relation data.""" if self.charm.app.planned_units() > 2: + if self.peer_relation is None: + return sync_standbys = self.charm._patroni.get_sync_standby_names() # Include the first unit as one of the sync-standbys. unit_to_become_sync_standby = f"{self.charm.app.name}/0" @@ -296,7 +306,7 @@ def _set_up_new_credentials_for_legacy(self) -> None: self.charm.postgresql.create_user( MONITORING_USER, self.charm.get_secret(APP_SCOPE, MONITORING_PASSWORD_KEY), - extra_user_roles="pg_monitor", + extra_user_roles=["pg_monitor"], ) def _patch_failsafe_mode(self): @@ -314,4 +324,6 @@ def _patch_max_timelines_history(self): @property def unit_upgrade_data(self) -> RelationDataContent: """Return the application upgrade data.""" + if self.peer_relation is None: + raise RuntimeError("peer relation must exist") return self.peer_relation.data[self.charm.unit] diff --git a/tox.ini b/tox.ini index 6c23630de10..5024b58f649 100644 --- a/tox.ini +++ b/tox.ini @@ -34,13 +34,14 @@ allowlist_externals = {[testenv]allowlist_externals} find commands_pre = - poetry install --only lint,format --no-root + poetry install --with lint,format --no-root commands = poetry check --lock poetry run codespell {[vars]all_path} poetry run ruff check {[vars]all_path} poetry run ruff format --check --diff {[vars]all_path} find {[vars]all_path} -type f \( -name "*.sh" -o -name "*.bash" \) -exec poetry run shellcheck --color=always \{\} + + poetry run ty check [testenv:unit] description = Run unit tests