diff --git a/e2e/e2e_tests/backup/test_backup_restore.py b/e2e/e2e_tests/backup/test_backup_restore.py index 13b5a8681..52864aad4 100755 --- a/e2e/e2e_tests/backup/test_backup_restore.py +++ b/e2e/e2e_tests/backup/test_backup_restore.py @@ -676,7 +676,8 @@ def _restore_backup(self, backup_id: str, lvol_name: str, pool_name: str = None) sleep_n_sec(60) pool = pool_name or self.pool_name out, err = self._sbcli( - f"-d backup restore {backup_id} --lvol {lvol_name} --pool {pool}") + f"-d backup restore {backup_id} --lvol {lvol_name} --pool {pool} " + f"--cluster-id {self.cluster_id}") assert not (err and "error" in err.lower()), \ f"backup restore failed: {err}" if out and "Error:" in out: diff --git a/simplyblock_cli/cli-reference.yaml b/simplyblock_cli/cli-reference.yaml index 6e65a40cc..b23b29961 100644 --- a/simplyblock_cli/cli-reference.yaml +++ b/simplyblock_cli/cli-reference.yaml @@ -2400,9 +2400,10 @@ commands: dest: node type: str - name: "--cluster-id" - help: "The cluster id." + help: "The target cluster id." dest: cluster_id type: str + required: true - name: export help: "Export backup metadata to a JSON file for cross-cluster restore." arguments: diff --git a/simplyblock_cli/cli.py b/simplyblock_cli/cli.py index 50197cef7..0464454de 100755 --- a/simplyblock_cli/cli.py +++ b/simplyblock_cli/cli.py @@ -985,7 +985,7 @@ def init_backup__restore(self, subparser): subcommand.add_argument('--lvol', help='The new logical volume name.', type=str, dest='lvol_name', required=True) subcommand.add_argument('--pool', help='The target pool name or id.', type=str, dest='pool', required=True) subcommand.add_argument('--node', help='The target storage node id.', type=str, dest='node') - subcommand.add_argument('--cluster-id', help='The cluster id.', type=str, dest='cluster_id') + subcommand.add_argument('--cluster-id', help='The target cluster id.', type=str, dest='cluster_id', required=True) def init_backup__export(self, subparser): subcommand = self.add_sub_command(subparser, 'export', 'Export backup metadata to a JSON file for cross-cluster restore.') diff --git a/simplyblock_cli/clibase.py b/simplyblock_cli/clibase.py index ea7076055..91b46f69e 100755 --- a/simplyblock_cli/clibase.py +++ b/simplyblock_cli/clibase.py @@ -928,7 +928,7 @@ def backup__delete(self, sub_command, args): def backup__restore(self, sub_command, args): result, error = backup_controller.restore_backup( args.backup_id, args.lvol_name, args.pool, - cluster_id=getattr(args, 'cluster_id', None), + cluster_id=args.cluster_id, target_node_id=getattr(args, 'node', None)) if error: print(f"Error: {error}") diff --git a/simplyblock_core/controllers/backup_controller.py b/simplyblock_core/controllers/backup_controller.py index 6bda8471d..517eabbe1 100644 --- a/simplyblock_core/controllers/backup_controller.py +++ b/simplyblock_core/controllers/backup_controller.py @@ -3,6 +3,7 @@ import re import time import uuid +from typing import Optional import boto3 from botocore.exceptions import BotoCoreError, ClientError @@ -388,8 +389,8 @@ def backup_snapshot(snapshot_id, cluster_id=None): return final_backup_id, None -def restore_backup(backup_id, lvol_name, pool_id_or_name, cluster_id, - target_node_id=None): +def restore_backup(backup_id: str, lvol_name: str, pool_id_or_name: str, cluster_id: str, + target_node_id: Optional[str] = None): """Restore a backup chain into a new fully-accessible lvol. Creates the volume (with subsystem, listeners, namespace) via @@ -419,8 +420,7 @@ def restore_backup(backup_id, lvol_name, pool_id_or_name, cluster_id, # If the backup came from an external cluster, the S3 bdev must be # switched to that cluster's bucket before restoring. backup_src = backup.source_cluster_id or backup.cluster_id - cl = db_controller.get_cluster_by_id(cluster_id) - active_src = cl.backup_source or cluster_id + active_src = cluster.backup_source or cluster.uuid if backup_src != active_src: return None, ( f"Backup source is {backup_src[:8]} but active S3 source " diff --git a/tests/integration/test_backup.py b/tests/integration/test_backup.py index 12c82018d..c9b511cc1 100644 --- a/tests/integration/test_backup.py +++ b/tests/integration/test_backup.py @@ -588,12 +588,14 @@ class TestRestoreBackup(unittest.TestCase): @patch("simplyblock_core.controllers.backup_controller.tasks_controller") @patch("simplyblock_core.controllers.backup_controller.db_controller") def test_success(self, mock_db, mock_tasks): - backup = _backup(s3_id=5) + cluster_uuid = "00000000-0000-0000-0000-000000000001" + backup = _backup(s3_id=5, cluster_id=cluster_uuid) mock_db.get_backup_by_id.return_value = backup mock_db.get_backup_chain.return_value = [backup] mock_tasks.add_backup_restore_task.return_value = True mock_cluster = MagicMock() + mock_cluster.uuid = cluster_uuid mock_cluster.backup_source = "" mock_db.get_cluster_by_id.return_value = mock_cluster @@ -610,7 +612,7 @@ def test_success(self, mock_db, mock_tasks): mock_add_ha.return_value = ("lvol-new", None) from simplyblock_core.controllers.backup_controller import restore_backup - result, error = restore_backup("backup-1", "restored_lvol", "pool-1", "cluster-1") + result, error = restore_backup("backup-1", "restored_lvol", "pool-1", cluster_uuid) self.assertEqual(result, "lvol-new") self.assertIsNone(error) @@ -630,11 +632,13 @@ def test_backup_not_found(self, mock_db): @patch("simplyblock_core.controllers.backup_controller.db_controller") def test_add_lvol_ha_fails(self, mock_db): - mock_db.get_backup_by_id.return_value = _backup() - mock_db.get_backup_chain.return_value = [_backup()] + cluster_uuid = "00000000-0000-0000-0000-000000000001" + mock_db.get_backup_by_id.return_value = _backup(cluster_id=cluster_uuid) + mock_db.get_backup_chain.return_value = [_backup(cluster_id=cluster_uuid)] mock_db.get_storage_node_by_id.return_value = _node() mock_cluster = MagicMock() + mock_cluster.uuid = cluster_uuid mock_cluster.backup_source = "" mock_db.get_cluster_by_id.return_value = mock_cluster @@ -642,7 +646,7 @@ def test_add_lvol_ha_fails(self, mock_db): mock_add_ha.return_value = (None, "Pool not found") from simplyblock_core.controllers.backup_controller import restore_backup - result, error = restore_backup("backup-1", "lvol", "bad-pool", "cluster-1") + result, error = restore_backup("backup-1", "lvol", "bad-pool", cluster_uuid) self.assertIsNone(result) self.assertIn("Failed to create restore volume", error)