Extend create-backup action checks

Author: sinclert-canonical

actions.yaml

@@ -45,6 +45,12 @@ set-tls-private-key:
 create-backup:
   description: Create a database backup using xtrabackup.
     S3 credentials are retrieved from a relation with the S3 integrator charm.
+  params:
+    force:
+      type: boolean
+      default: False
+      description: |
+        Whether to ignore cluster health concerns, and create the backup regardless.
 
 list-backups:
   description: List available backup_ids in the S3 bucket and path provided by the S3 integrator charm.

lib/charms/mysql/v0/backups.py

@@ -57,6 +57,7 @@ def is_unit_blocked(self) -> bool:
     S3Requirer,
 )
 from charms.mysql.v0.mysql import (
+    MySQLClusterState,
     MySQLConfigureInstanceError,
     MySQLCreateClusterError,
     MySQLCreateClusterSetError,
@@ -111,7 +112,7 @@ def is_unit_blocked(self) -> bool:
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 16
+LIBPATCH = 17
 
 ANOTHER_S3_CLUSTER_REPOSITORY_ERROR_MESSAGE = "S3 repository claimed by another cluster"
 MOVE_RESTORED_CLUSTER_TO_ANOTHER_S3_REPOSITORY_ERROR = (
@@ -280,6 +281,7 @@ def _pre_create_backup_checks(self, event: ActionEvent) -> bool:
     def _on_create_backup(self, event: ActionEvent) -> None:
         """Handle the create backup action."""
         logger.info("A backup has been requested on unit")
+        force = event.params.get("force", False)
 
         if not self._pre_create_backup_checks(event):
             return
@@ -295,6 +297,13 @@ def _on_create_backup(self, event: ActionEvent) -> None:
 
         backup_path = str(pathlib.Path(s3_parameters["path"]) / datetime_backup_requested)
 
+        # Check if this cluster can perform backup
+        can_cluster_perform_backup, validation_message = self._can_cluster_perform_backup(force)
+        if not can_cluster_perform_backup and not force:
+            logger.error(f"Backup failed: {validation_message}")
+            event.fail(validation_message or "")
+            return
+
         # Check if this unit can perform backup
         can_unit_perform_backup, validation_message = self._can_unit_perform_backup()
         if not can_unit_perform_backup:
@@ -355,6 +364,25 @@ def _on_create_backup(self, event: ActionEvent) -> None:
         })
         self.charm._on_update_status(None)
 
+    def _can_cluster_perform_backup(self, force: bool) -> tuple[bool, str | None]:
+        """Validates whether this cluster can perform a backup.
+
+        Returns: tuple of (success, error_message)
+        """
+        cluster_status = self.charm._mysql.get_cluster_status()
+        if not cluster_status:
+            return False, "Cluster status unknown"
+
+        cluster_status = cluster_status["defaultreplicaset"]["status"]
+        if (
+            not force
+            and cluster_status != MySQLClusterState.OK
+            and cluster_status != MySQLClusterState.OK_PARTIAL
+        ):
+            return False, "Cluster is not in a healthy state"
+
+        return True, None
+
     def _can_unit_perform_backup(self) -> tuple[bool, str | None]:
         """Validates whether this unit can perform a backup.
 

lib/charms/mysql/v0/mysql.py

@@ -127,7 +127,7 @@ def wait_until_mysql_connection(self) -> None:
 # Increment this major API version when introducing breaking changes
 LIBAPI = 0
 
-LIBPATCH = 93
+LIBPATCH = 94
 
 UNIT_TEARDOWN_LOCKNAME = "unit-teardown"
 UNIT_ADD_LOCKNAME = "unit-add"
@@ -916,11 +916,7 @@ def get_cluster_endpoints(self, relation_name: str) -> tuple[str, str, str]:
 
         return ",".join(rw_endpoints), ",".join(ro_endpoints), ",".join(no_endpoints)
 
-    def get_secret(
-        self,
-        scope: Scopes,
-        key: str,
-    ) -> str | None:
+    def get_secret(self, scope: Scopes, key: str) -> str | None:
         """Get secret from the secret storage.
 
         Retrieve secret from juju secrets backend if secret exists there.
@@ -1012,7 +1008,18 @@ class MySQLMemberState(str, enum.Enum):
 class MySQLClusterState(str, enum.Enum):
     """MySQL Cluster state."""
 
+    # TODO: python 3.11 has new enum.StrEnum
+    #       that can remove str inheritance
+
     OK = "ok"
+    OK_PARTIAL = "ok_partial"
+    OK_NO_TOLERANCE = "ok_no_tolerance"
+    OK_NO_TOLERANCE_PARTIAL = "ok_no_tolerance_partial"
+    NO_QUORUM = "no_quorum"
+    OFFLINE = "offline"
+    ERROR = "error"
+    UNREACHABLE = "unreachable"
+    UNKNOWN = "unknown"
     FENCED = "fenced_writes"
 
 

tests/unit/test_backups.py

@@ -155,7 +155,12 @@ def test_on_list_backups_failure(self, _list_backups_in_s3_path, _retrieve_s3_pa
         return_value=({"path": "/path"}, []),
     )
     @patch(
-        "charms.mysql.v0.backups.MySQLBackups._can_unit_perform_backup", return_value=(True, None)
+        "charms.mysql.v0.backups.MySQLBackups._can_cluster_perform_backup",
+        return_value=(True, None),
+    )
+    @patch(
+        "charms.mysql.v0.backups.MySQLBackups._can_unit_perform_backup",
+        return_value=(True, None),
     )
     @patch("charms.mysql.v0.backups.upload_content_to_s3")
     @patch("charms.mysql.v0.backups.MySQLBackups._pre_backup", return_value=(True, None))
@@ -170,6 +175,7 @@ def test_on_create_backup(
         _pre_backup,
         _upload_content_to_s3,
         _can_unit_perform_backup,
+        _can_cluster_perform_backup,
         _retrieve_s3_parameters,
         _datetime,
         _update_status,
@@ -191,6 +197,7 @@ def test_on_create_backup(
         self.mysql_backups._on_create_backup(event)
 
         _retrieve_s3_parameters.assert_called_once()
+        _can_cluster_perform_backup.assert_called_once()
         _can_unit_perform_backup.assert_called_once()
         _upload_content_to_s3.assert_called_once_with(
             expected_metadata, f"{expected_backup_path}.metadata", expected_s3_params
@@ -208,7 +215,12 @@ def test_on_create_backup(
         return_value=({"path": "/path"}, []),
     )
     @patch(
-        "charms.mysql.v0.backups.MySQLBackups._can_unit_perform_backup", return_value=(True, None)
+        "charms.mysql.v0.backups.MySQLBackups._can_cluster_perform_backup",
+        return_value=(True, None),
+    )
+    @patch(
+        "charms.mysql.v0.backups.MySQLBackups._can_unit_perform_backup",
+        return_value=(True, None),
     )
     @patch("charms.mysql.v0.backups.upload_content_to_s3")
     @patch("charms.mysql.v0.backups.MySQLBackups._pre_backup", return_value=(True, None))
@@ -223,6 +235,7 @@ def test_on_create_backup_failure(
         _pre_backup,
         _upload_content_to_s3,
         _can_unit_perform_backup,
+        _can_cluster_perform_backup,
         _retrieve_s3_parameters,
         _datetime,
     ):
@@ -269,6 +282,17 @@ def test_on_create_backup_failure(
         event.fail.assert_called_once_with("Failed to upload metadata to provided S3")
         self.assertTrue(isinstance(self.harness.model.unit.status, ActiveStatus))
 
+        # test failure with _can_cluster_perform_backup
+        _can_cluster_perform_backup.return_value = False, "can cluster perform backup failure"
+        event = MagicMock()
+        type(event).params = PropertyMock(return_value={"force": False})
+        self.charm.unit.status = ActiveStatus()
+
+        self.mysql_backups._on_create_backup(event)
+        event.set_results.assert_not_called()
+        event.fail.assert_called_once_with("can cluster perform backup failure")
+        self.assertTrue(isinstance(self.harness.model.unit.status, ActiveStatus))
+
         # test failure with _can_unit_perform_backup
         _can_unit_perform_backup.return_value = False, "can unit perform backup failure"
         event = MagicMock()
@@ -309,6 +333,42 @@ def test_on_create_backup_failure(
         event.fail.assert_called_once_with("Missing relation with S3 integrator charm")
         self.assertTrue(isinstance(self.harness.model.unit.status, ActiveStatus))
 
+    @patch("mysql_vm_helpers.MySQL.get_cluster_status")
+    def test_can_cluster_perform_backup(self, _get_cluster_status):
+        """Test _can_cluster_perform_backup()."""
+        _get_cluster_status.return_value = {"defaultreplicaset": {"status": "ok"}}
+
+        success, error_message = self.mysql_backups._can_cluster_perform_backup(force=False)
+        self.assertTrue(success)
+        self.assertIsNone(error_message)
+
+    @patch("mysql_vm_helpers.MySQL.get_cluster_status")
+    def test_can_cluster_perform_backup_failure(self, _get_cluster_status):
+        """Test failure of _can_unit_perform_backup()."""
+        # test unknown state
+        _get_cluster_status.reset_mock()
+        _get_cluster_status.return_value = None
+
+        success, error_message = self.mysql_backups._can_cluster_perform_backup(force=False)
+        self.assertFalse(success)
+        self.assertEqual(error_message, "Cluster status unknown")
+
+        # test error state (without force)
+        _get_cluster_status.reset_mock()
+        _get_cluster_status.return_value = {"defaultreplicaset": {"status": "error"}}
+
+        success, error_message = self.mysql_backups._can_cluster_perform_backup(force=False)
+        self.assertFalse(success)
+        self.assertEqual(error_message, "Cluster is not in a healthy state")
+
+        # test error state (with force)
+        _get_cluster_status.reset_mock()
+        _get_cluster_status.return_value = {"defaultreplicaset": {"status": "error"}}
+
+        success, error_message = self.mysql_backups._can_cluster_perform_backup(force=True)
+        self.assertTrue(success)
+        self.assertEqual(error_message, None)
+
     @patch("mysql_vm_helpers.MySQL.offline_mode_and_hidden_instance_exists", return_value=False)
     @patch("mysql_vm_helpers.MySQL.get_member_state", return_value=("online", "replica"))
     def test_can_unit_perform_backup(