From a3d9be48f2ae30bfcb1544f7a091c98981702be1 Mon Sep 17 00:00:00 2001 From: Raunak Jalan Date: Wed, 12 Aug 2026 04:35:18 +0530 Subject: [PATCH 1/3] Fix get_backup_by_id to filter by cluster_id in cross-cluster scenarios After backup import, both clusters share the same UUID in the KV store. get_backup_by_id() scanned all clusters, hitting 'Multiple values present' on restore. Add optional cluster_id parameter to scope the lookup. Co-Authored-By: Claude Opus 4.6 --- simplyblock_core/controllers/backup_controller.py | 9 ++++----- simplyblock_core/db_controller.py | 9 +++++---- simplyblock_core/env_var | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/simplyblock_core/controllers/backup_controller.py b/simplyblock_core/controllers/backup_controller.py index 86965bcbbd..3177161424 100644 --- a/simplyblock_core/controllers/backup_controller.py +++ b/simplyblock_core/controllers/backup_controller.py @@ -411,7 +411,7 @@ def restore_backup(backup_id: str, lvol_name: str, pool_id_or_name: str, cluster from simplyblock_core.models.lvol_model import LVol try: - backup = db_controller.get_backup_by_id(backup_id) + backup = db_controller.get_backup_by_id(backup_id, cluster_id) cluster = db_controller.get_cluster_by_id(cluster_id) except KeyError as e: return None, str(e) @@ -428,7 +428,7 @@ def restore_backup(backup_id: str, lvol_name: str, pool_id_or_name: str, cluster f"{backup_src}' first.") # Build the backup chain - chain = db_controller.get_backup_chain(backup_id) + chain = db_controller.get_backup_chain(backup_id, cluster_id) if not chain: return None, f"Could not build backup chain for {backup_id}" @@ -655,9 +655,8 @@ def import_backups(s3_metadata_list, cluster_id=None): # Skip only if already registered for the target cluster try: - existing = db_controller.get_backup_by_id(backup_id) - if existing.cluster_id == target_cluster: - continue # already imported for this cluster + db_controller.get_backup_by_id(backup_id, target_cluster) + continue # already imported for this cluster except KeyError: pass diff --git a/simplyblock_core/db_controller.py b/simplyblock_core/db_controller.py index 0269987fca..006a55f8c7 100644 --- a/simplyblock_core/db_controller.py +++ b/simplyblock_core/db_controller.py @@ -1357,8 +1357,9 @@ def get_backups(self, cluster_id: Optional[str] = None) -> List[Backup]: prefix = cluster_id if cluster_id else " " return Backup().read_from_db(self.kv_store, id=prefix) - def get_backup_by_id(self, backup_id: str) -> Backup: - backup = single_or_none(b for b in self.get_backups() if b.uuid == backup_id) + def get_backup_by_id(self, backup_id: str, cluster_id: Optional[str] = None) -> Backup: + backup = single_or_none( + b for b in self.get_backups(cluster_id) if b.uuid == backup_id) if backup is None: raise KeyError(f'Backup {backup_id} not found') return backup @@ -1369,7 +1370,7 @@ def get_backups_by_lvol_id(self, lvol_id: str) -> List[Backup]: def get_backups_by_snapshot_id(self, snapshot_id: str) -> List[Backup]: return [b for b in self.get_backups() if b.snapshot_id == snapshot_id] - def get_backup_chain(self, backup_id: str) -> List[Backup]: + def get_backup_chain(self, backup_id: str, cluster_id: Optional[str] = None) -> List[Backup]: """Return the full backup chain ending at backup_id, oldest first.""" chain = [] current_id = backup_id @@ -1377,7 +1378,7 @@ def get_backup_chain(self, backup_id: str) -> List[Backup]: while current_id and current_id not in visited: visited.add(current_id) try: - backup = self.get_backup_by_id(current_id) + backup = self.get_backup_by_id(current_id, cluster_id) except KeyError: break chain.append(backup) diff --git a/simplyblock_core/env_var b/simplyblock_core/env_var index 5abdecd440..7c155b1077 100644 --- a/simplyblock_core/env_var +++ b/simplyblock_core/env_var @@ -1,5 +1,5 @@ SIMPLY_BLOCK_COMMAND_NAME=sbcli-dev SIMPLY_BLOCK_VERSION=19.2.34 -SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:main +SIMPLY_BLOCK_DOCKER_IMAGE=public.ecr.aws/simply-block/simplyblock:fix-backup-get-by-id-cluster-filter SIMPLY_BLOCK_SPDK_ULTRA_IMAGE=public.ecr.aws/simply-block/ultra:main-latest From 6e74eeca6ba29f30787178980f9a93a52b5f38bb Mon Sep 17 00:00:00 2001 From: Raunak Jalan Date: Wed, 12 Aug 2026 16:43:56 +0530 Subject: [PATCH 2/3] Fix cross-cluster restore placing lvol on source cluster nodes When restoring a backup on a different cluster, restore_backup() fell back to backup.node_id which belongs to the source cluster, causing the restored lvol to be created on the wrong cluster's nodes. Now detect when the backup's node doesn't belong to the target cluster and automatically select an online node from the target cluster. Co-Authored-By: Claude Opus 4.6 --- .../controllers/backup_controller.py | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/simplyblock_core/controllers/backup_controller.py b/simplyblock_core/controllers/backup_controller.py index 3177161424..c012d550ea 100644 --- a/simplyblock_core/controllers/backup_controller.py +++ b/simplyblock_core/controllers/backup_controller.py @@ -436,8 +436,39 @@ def restore_backup(backup_id: str, lvol_name: str, pool_id_or_name: str, cluster if size <= 0: return None, "Backup has no size information" - # Determine target node: use explicit target, or fall back to backup node - restore_node_id = target_node_id or backup.node_id + # Determine target node: use explicit target, or fall back to backup node. + # In cross-cluster restore, backup.node_id belongs to the source cluster, + # so we must pick an online node from the target cluster instead. + if target_node_id: + restore_node_id = target_node_id + else: + # Check if backup's original node belongs to the target cluster + backup_node_in_target = False + try: + orig_node = db_controller.get_storage_node_by_id(backup.node_id) + if orig_node.cluster_id == cluster_id: + backup_node_in_target = True + except KeyError: + pass + + if backup_node_in_target: + restore_node_id = backup.node_id + else: + # Cross-cluster: pick an online node from the target cluster + target_nodes = db_controller.get_storage_nodes_by_cluster_id(cluster_id) + online_nodes = [ + n for n in target_nodes + if n.status == StorageNode.STATUS_ONLINE and n.lvstore + ] + if not online_nodes: + return None, ( + f"No online storage node with lvstore found in " + f"cluster {cluster_id} for cross-cluster restore") + restore_node_id = online_nodes[0].get_id() + logger.info( + f"Cross-cluster restore: backup node {backup.node_id} " + f"belongs to a different cluster, using target cluster " + f"node {restore_node_id}") # Validate target node is online and has an S3 bdev try: From 1c6225a21be4edcac1a26f6931dae3619c743f60 Mon Sep 17 00:00:00 2001 From: Raunak Jalan Date: Wed, 12 Aug 2026 16:51:03 +0530 Subject: [PATCH 3/3] Revert "Fix cross-cluster restore placing lvol on source cluster nodes" This reverts commit 6e74eeca6ba29f30787178980f9a93a52b5f38bb. --- .../controllers/backup_controller.py | 35 ++----------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/simplyblock_core/controllers/backup_controller.py b/simplyblock_core/controllers/backup_controller.py index c012d550ea..3177161424 100644 --- a/simplyblock_core/controllers/backup_controller.py +++ b/simplyblock_core/controllers/backup_controller.py @@ -436,39 +436,8 @@ def restore_backup(backup_id: str, lvol_name: str, pool_id_or_name: str, cluster if size <= 0: return None, "Backup has no size information" - # Determine target node: use explicit target, or fall back to backup node. - # In cross-cluster restore, backup.node_id belongs to the source cluster, - # so we must pick an online node from the target cluster instead. - if target_node_id: - restore_node_id = target_node_id - else: - # Check if backup's original node belongs to the target cluster - backup_node_in_target = False - try: - orig_node = db_controller.get_storage_node_by_id(backup.node_id) - if orig_node.cluster_id == cluster_id: - backup_node_in_target = True - except KeyError: - pass - - if backup_node_in_target: - restore_node_id = backup.node_id - else: - # Cross-cluster: pick an online node from the target cluster - target_nodes = db_controller.get_storage_nodes_by_cluster_id(cluster_id) - online_nodes = [ - n for n in target_nodes - if n.status == StorageNode.STATUS_ONLINE and n.lvstore - ] - if not online_nodes: - return None, ( - f"No online storage node with lvstore found in " - f"cluster {cluster_id} for cross-cluster restore") - restore_node_id = online_nodes[0].get_id() - logger.info( - f"Cross-cluster restore: backup node {backup.node_id} " - f"belongs to a different cluster, using target cluster " - f"node {restore_node_id}") + # Determine target node: use explicit target, or fall back to backup node + restore_node_id = target_node_id or backup.node_id # Validate target node is online and has an S3 bdev try: