[MISC] Miscellaneous cleanups

kubernetes/lib/charms/mysql/v0/backups.py

@@ -118,7 +118,7 @@ def is_unit_blocked(self) -> bool:
 )
 
 if typing.TYPE_CHECKING:
-    from mysql import MySQLCharmBase
+    from .mysql import MySQLCharmBase
 
 
 class MySQLBackups(Object):
@@ -265,7 +265,10 @@ def _pre_create_backup_checks(self, event: ActionEvent) -> bool:
             return False
 
         if "s3-block-message" in self.charm.app_peer_data:
-            logger.error("Backup failed: S3 relation is blocked for write")
+            logger.error(
+                "Backup failed: S3 relation is blocked for write: %s",
+                self.charm.app_peer_data["s3-block-message"],
+            )
             event.fail("S3 relation is blocked for write")
             return False
 
@@ -727,7 +730,7 @@ def _clean_data_dir_and_start_mysqld(self) -> tuple[bool, str]:
             self.charm._mysql.delete_temp_backup_directory()
             # Old backups may contain the temp backup directory (as previously, the temp
             # backup directory was created in the mysql data directory to reduce IOPS latency)
-            self.charm._mysql.delete_temp_backup_directory(from_directory=MYSQL_DATA_DIR)
+            self.charm._mysql.delete_temp_backup_directory(tmp_base_directory=MYSQL_DATA_DIR)
         except MySQLDeleteTempRestoreDirectoryError:
             return False, "Failed to delete the temp restore directory"
         except MySQLDeleteTempBackupDirectoryError:

kubernetes/lib/charms/mysql/v0/mysql.py

@@ -2448,7 +2448,7 @@ def get_available_memory(self) -> int:
         """Platform dependent method to get the available memory for mysql-server."""
         raise NotImplementedError
 
-    def create_sa_pem_file(
+    def create_ca_pem_file(
         self,
         ca_chain: list[str],
         tmp_base_directory: str,
@@ -2475,7 +2475,7 @@ def create_sa_pem_file(
     def execute_backup_commands(
         self,
         s3_path: str,
-        s3_parameters: dict[str, str],
+        s3_parameters: dict,
         xtrabackup_location: str,
         xbcloud_location: str,
         xtrabackup_plugin_dir: str,
@@ -2488,26 +2488,34 @@ def execute_backup_commands(
         """Executes commands to create a backup with the given args."""
         nproc_command = ["nproc"]
         make_temp_dir_command = f"mktemp --directory {tmp_base_directory}/xtra_backup_XXXX".split()
-        ca_chain = s3_parameters.get("tls-ca-chain")
-        ca_file_location = None
         try:
             nproc, _ = self._execute_commands(nproc_command)
-            tmp_dir, _ = self._execute_commands(
-                make_temp_dir_command, user=user, group=group
-            )
-            if ca_chain:
-                ca_file_location = self.create_sa_pem_file(
-                    ca_chain, tmp_base_directory, user=user, group=group
-                )
+            tmp_dir, _ = self._execute_commands(make_temp_dir_command, user=user, group=group)
         except MySQLExecError as e:
-            logger.error("Failed to execute commands prior to running backup")
+            logger.error(f"Failed to execute commands prior to running backup, reason: {e}")
             raise MySQLExecuteBackupCommandsError from e
         except Exception as e:
             # Catch all other exceptions to prevent the database being stuck in
             # a bad state due to pre-backup operations
             logger.error("Failed unexpectedly to execute commands prior to running backup")
             raise MySQLExecuteBackupCommandsError from e
 
+        ca_chain: list[str] | None = s3_parameters.get("tls-ca-chain")
+        ca_file_location = None
+        if ca_chain:
+            try:
+                ca_file_location = self.create_ca_pem_file(
+                    ca_chain, tmp_base_directory, user=user, group=group
+                )
+            except MySQLExecError as e:
+                logger.error(f"Failed to execute commands prior to running backup, reason: {e}")
+                raise MySQLExecuteBackupCommandsError from e
+            except Exception as e:
+                # Catch all other exceptions to prevent the database being stuck in
+                # a bad state due to pre-backup operations
+                logger.error("Failed unexpectedly to execute commands prior to running backup")
+                raise MySQLExecuteBackupCommandsError from e
+
         # TODO: remove flags --no-server-version-check
         # when MySQL and XtraBackup versions are in sync
         xtrabackup_commands = [
@@ -2536,7 +2544,9 @@ def execute_backup_commands(
             f"--s3-api-version={s3_parameters['s3-api-version']}",
             f"--s3-bucket-lookup={s3_parameters['s3-uri-style']}",
         ]
-        xtrabackup_commands.append(f"--cacert={ca_file_location}") if ca_chain and ca_file_location else None
+        xtrabackup_commands.append(
+            f"--cacert={ca_file_location}"
+        ) if ca_chain and ca_file_location else None
         xtrabackup_commands.append(f"{s3_path}")
         try:
             logger.debug(
@@ -2593,7 +2603,7 @@ def delete_temp_backup_directory(
     def retrieve_backup_with_xbcloud(
         self,
         backup_id: str,
-        s3_parameters: dict[str, str],
+        s3_parameters: dict,
         temp_restore_directory: str,
         xbcloud_location: str,
         xbstream_location: str,
@@ -2605,22 +2615,28 @@ def retrieve_backup_with_xbcloud(
         make_temp_dir_command = (
             f"mktemp --directory {temp_restore_directory}/#mysql_sst_XXXX".split()
         )
-        ca_chain = s3_parameters.get("tls-ca-chain")
-        ca_file_location = None
         try:
             nproc, _ = self._execute_commands(nproc_command)
             tmp_dir, _ = self._execute_commands(
                 make_temp_dir_command,
                 user=user,
                 group=group,
             )
-            if ca_chain:
-                ca_file_location = self.create_sa_pem_file(
-                    ca_chain, temp_restore_directory, user=user, group=group
-                )
         except MySQLExecError as e:
             logger.error("Failed to execute commands prior to running xbcloud get")
             raise MySQLRetrieveBackupWithXBCloudError(e.message) from e
+
+        ca_chain: list[str] | None = s3_parameters.get("tls-ca-chain")
+        ca_file_location = None
+        if ca_chain:
+            try:
+                ca_file_location = self.create_ca_pem_file(
+                    ca_chain, temp_restore_directory, user=user, group=group
+                )
+            except MySQLExecError as e:
+                logger.error("Failed to execute commands prior to running xbcloud get")
+                raise MySQLRetrieveBackupWithXBCloudError(e.message) from e
+
         retrieve_backup_command = [
             f"{xbcloud_location} get",
             "--curl-retriable-errors=7",
@@ -2633,14 +2649,17 @@ def retrieve_backup_with_xbcloud(
             f"--s3-api-version={s3_parameters['s3-api-version']}",
             f"{s3_parameters['path']}/{backup_id}",
         ]
-        retrieve_backup_command.append(f"--cacert={ca_file_location}") if ca_chain and ca_file_location else None
+        retrieve_backup_command.append(
+            f"--cacert={ca_file_location}"
+        ) if ca_chain and ca_file_location else None
         retrieve_backup_command += [
             f"| {xbstream_location}",
             "--decompress",
             "-x",
             f"-C {tmp_dir}",
             f"--parallel={nproc}",
         ]
+
         try:
             logger.debug(f"Command to retrieve backup: {' '.join(retrieve_backup_command)}")
 
@@ -2868,6 +2887,7 @@ def _execute_commands(
         user: str | None = None,
         group: str | None = None,
         env_extra: dict | None = None,
+        timeout: float | None = None,
         stream_output: str | None = None,
     ) -> tuple[str, str]:
         """Execute commands on the server where MySQL is running."""

kubernetes/lib/charms/mysql/v0/s3_helpers.py

@@ -110,7 +110,7 @@ def upload_content_to_s3(content: str, content_path: str, s3_parameters: dict) -
     try:
         logger.info(f"Uploading content to bucket={s3_parameters['bucket']}, path={content_path}")
 
-        ca_chain = s3_parameters.get("tls-ca-chain")
+        ca_chain: list[str] | None = s3_parameters.get("tls-ca-chain")
         with tempfile.NamedTemporaryFile() as content_file, tempfile.NamedTemporaryFile() if ca_chain else nullcontext() as ca_file:
             content_file.write(content.encode("utf-8"))
             content_file.flush()
@@ -146,7 +146,7 @@ def _read_content_from_s3(content_path: str, s3_parameters: dict) -> str | None:
     """
     try:
         logger.info(f"Reading content from bucket={s3_parameters['bucket']}, path={content_path}")
-        ca_chain = s3_parameters.get("tls-ca-chain")
+        ca_chain: list[str] | None = s3_parameters.get("tls-ca-chain")
         with tempfile.NamedTemporaryFile() if ca_chain else nullcontext() as ca_file, BytesIO() as buf:
             if ca_file:
                 ca = "\n".join(ca_chain)
@@ -277,7 +277,7 @@ def list_backups_in_s3_path(s3_parameters: dict) -> list[tuple[str, str]]:
         logger.info(
             f"Listing subdirectories from S3 bucket={s3_parameters['bucket']}, path={s3_parameters['path']}"
         )
-        ca_chain = s3_parameters.get("tls-ca-chain")
+        ca_chain: list[str] | None = s3_parameters.get("tls-ca-chain")
         with _temporary_ca_file(ca_chain) as ca_file_name:
             s3_client = boto3.client(
                 "s3",
@@ -306,7 +306,7 @@ def list_backups_in_s3_path(s3_parameters: dict) -> list[tuple[str, str]]:
         raise
 
 
-def fetch_and_check_existence_of_s3_path(path: str, s3_parameters: dict[str, str]) -> bool:
+def fetch_and_check_existence_of_s3_path(path: str, s3_parameters: dict) -> bool:
     """Checks the existence of a provided S3 path by fetching the object.
 
     Args:
@@ -319,7 +319,7 @@ def fetch_and_check_existence_of_s3_path(path: str, s3_parameters: dict[str, str
 
     Raises: any exceptions raised by boto3
     """
-    ca_chain = s3_parameters.get("tls-ca-chain")
+    ca_chain: list[str] | None = s3_parameters.get("tls-ca-chain")
     with _temporary_ca_file(ca_chain) as ca_file_name:
         s3_client = boto3.client(
             "s3",

kubernetes/metadata.yaml

@@ -14,7 +14,7 @@ docs: https://canonical-charmed-mysql-k8s.readthedocs-hosted.com/
 source: https://github.com/canonical/mysql-operators
 issues: https://github.com/canonical/mysql-operators/issues
 website:
-  - https://ubuntu.com/data/mysql
+  - https://canonical.com/data/mysql
   - https://charmhub.io/mysql-k8s
   - https://github.com/canonical/mysql-operators
 maintainers:

kubernetes/src/mysql_k8s_helpers.py

@@ -319,12 +319,17 @@ def execute_backup_commands(
             group,
         )
 
-    def delete_temp_backup_directory(self, from_directory: str = MYSQL_DATA_DIR) -> None:
+    def delete_temp_backup_directory(
+        self,
+        tmp_base_directory: str = MYSQL_DATA_DIR,
+        user=MYSQL_SYSTEM_USER,
+        group=MYSQL_SYSTEM_GROUP,
+    ) -> None:
         """Delete the temp backup directory in the data directory."""
         super().delete_temp_backup_directory(
-            from_directory,
-            user=MYSQL_SYSTEM_USER,
-            group=MYSQL_SYSTEM_GROUP,
+            tmp_base_directory,
+            user,
+            group,
         )
 
     def retrieve_backup_with_xbcloud(
@@ -334,8 +339,8 @@ def retrieve_backup_with_xbcloud(
         temp_restore_directory: str = MYSQL_DATA_DIR,
         xbcloud_location: str = CHARMED_MYSQL_XBCLOUD_LOCATION,
         xbstream_location: str = CHARMED_MYSQL_XBSTREAM_LOCATION,
-        user: str = MYSQL_SYSTEM_USER,
-        group: str = MYSQL_SYSTEM_GROUP,
+        user: str | None = MYSQL_SYSTEM_USER,
+        group: str | None = MYSQL_SYSTEM_GROUP,
     ) -> tuple[str, str, str]:
         """Retrieve the specified backup from S3.
 
@@ -352,42 +357,68 @@ def retrieve_backup_with_xbcloud(
             group,
         )
 
-    def prepare_backup_for_restore(self, backup_location: str) -> tuple[str, str]:
+    def prepare_backup_for_restore(
+        self,
+        backup_location: str,
+        xtrabackup_location: str = CHARMED_MYSQL_XTRABACKUP_LOCATION,
+        xtrabackup_plugin_dir: str = XTRABACKUP_PLUGIN_DIR,
+        user=MYSQL_SYSTEM_USER,
+        group=MYSQL_SYSTEM_GROUP,
+    ) -> tuple[str, str]:
         """Prepare the backup in the provided dir for restore."""
         return super().prepare_backup_for_restore(
             backup_location,
-            CHARMED_MYSQL_XTRABACKUP_LOCATION,
-            XTRABACKUP_PLUGIN_DIR,
-            user=MYSQL_SYSTEM_USER,
-            group=MYSQL_SYSTEM_GROUP,
+            xtrabackup_location,
+            xtrabackup_plugin_dir,
+            user,
+            group,
         )
 
-    def empty_data_files(self) -> None:
+    def empty_data_files(
+        self,
+        mysql_data_directory=MYSQL_DATA_DIR,
+        user=MYSQL_SYSTEM_USER,
+        group=MYSQL_SYSTEM_GROUP,
+    ) -> None:
         """Empty the mysql data directory in preparation of backup restore."""
         super().empty_data_files(
-            MYSQL_DATA_DIR,
-            user=MYSQL_SYSTEM_USER,
-            group=MYSQL_SYSTEM_GROUP,
+            mysql_data_directory,
+            user,
+            group,
         )
 
-    def restore_backup(self, backup_location: str) -> tuple[str, str]:
+    def restore_backup(
+        self,
+        backup_location: str,
+        xtrabackup_location: str = CHARMED_MYSQL_XTRABACKUP_LOCATION,
+        defaults_config_file: str = MYSQLD_DEFAULTS_CONFIG_FILE,
+        mysql_data_directory: str = MYSQL_DATA_DIR,
+        xtrabackup_plugin_directory: str = XTRABACKUP_PLUGIN_DIR,
+        user=MYSQL_SYSTEM_USER,
+        group=MYSQL_SYSTEM_GROUP,
+    ) -> tuple[str, str]:
         """Restore the provided prepared backup."""
         return super().restore_backup(
             backup_location,
-            CHARMED_MYSQL_XTRABACKUP_LOCATION,
-            MYSQLD_DEFAULTS_CONFIG_FILE,
-            MYSQL_DATA_DIR,
-            XTRABACKUP_PLUGIN_DIR,
-            user=MYSQL_SYSTEM_USER,
-            group=MYSQL_SYSTEM_GROUP,
+            xtrabackup_location,
+            defaults_config_file,
+            mysql_data_directory,
+            xtrabackup_plugin_directory,
+            user,
+            group,
         )
 
-    def delete_temp_restore_directory(self) -> None:
+    def delete_temp_restore_directory(
+        self,
+        temp_restore_directory: str = MYSQL_DATA_DIR,
+        user=MYSQL_SYSTEM_USER,
+        group=MYSQL_SYSTEM_GROUP,
+    ) -> None:
         """Delete the temp restore directory from the mysql data directory."""
         super().delete_temp_restore_directory(
-            MYSQL_DATA_DIR,
-            user=MYSQL_SYSTEM_USER,
-            group=MYSQL_SYSTEM_GROUP,
+            temp_restore_directory,
+            user,
+            group,
         )
 
     @retry(