Fix/cluster activate stranded secondary node

simplyblock_core/cluster_ops.py

@@ -976,14 +976,16 @@ def _cluster_activate(cl_id, force=False, force_lvstore_create=False) -> None:
         set_cluster_status(cl_id, ols_status)
         raise
 
-    # Failure-domain coverage check (best-effort: warn, don't block). To
-    # survive losing a whole failure domain we need at least npcs+1 distinct
-    # domains; with fewer, placement falls back to host-disjoint and a domain
-    # outage may exceed the cluster's fault tolerance.
+    # Failure-domain coverage check (best-effort: warn, don't block). A 2-FD
+    # layout can never absorb a second independent failure once one domain
+    # is down, so the hard minimum below (enforced at fresh activation) is
+    # npcs+2, not npcs+1 -- this warning uses the same number so a
+    # reactivation that's short of it gets the same signal without being
+    # blocked (recovering a drifted layout must not turn into an outage).
     fd_desired_layout: t.Dict[str, t.Tuple[str, str]] = {}
     if cluster.enable_failure_domain:
         distinct_domains = {node.failure_domain for node in online_nodes if node.failure_domain >= 0}
-        min_domains = cluster.distr_npcs + 1
+        min_domains = cluster.distr_npcs + 2
         if len(distinct_domains) < min_domains:
             logger.warning(
                 "Failure-domain feature is enabled but only %d distinct failure "
@@ -1023,9 +1025,15 @@ def _fd_fail(msg: str) -> None:
                              f"a host must sit entirely in one domain")
 
             fd_host_counts = Counter(host_fd.values())
-            if len(fd_host_counts) < 2:
-                _fd_fail("failure domains are enabled but all hosts are in a "
-                         "single domain; at least two domains are required")
+            # See fd_activation_domain_count_violation's docstring: npcs+2
+            # domains, not just the bare rotation-correctness minimum, so a
+            # later single add/remove has a spare candidate instead of
+            # stranding another node's secondary/tertiary with none at all.
+            # This also subsumes the plain "at least two domains" floor.
+            domain_count_violation = fd_planner.fd_activation_domain_count_violation(
+                cluster.distr_npcs, len(fd_host_counts))
+            if domain_count_violation:
+                _fd_fail(domain_count_violation)
             if len(set(fd_host_counts.values())) != 1:
                 _fd_fail(
                     f"failure domains must hold an EQUAL number of hosts at "
@@ -1062,6 +1070,18 @@ def _fd_fail(msg: str) -> None:
     used_nodes_as_sec: t.List[str] = []
     used_nodes_as_tertiary: t.List[str] = []
     snodes = db_controller.get_storage_nodes_by_cluster_id(cl_id)
+    # Process primaries grouped by failure domain. get_secondary_nodes/
+    # get_secondary_nodes_2 (and their splice repairs) already sort their own
+    # candidate scan by domain, which alone is enough to keep the assignment
+    # domain-disjoint when domains are evenly sized. But once any node needs
+    # splice-repair (uneven domain sizes, some conflict unavoidable), the
+    # repair works off whatever partial assignment already exists -- so which
+    # primary gets processed first still changes the outcome. Grouping here
+    # too makes the result deterministic instead of order-dependent in that
+    # case. A no-op when FD is disabled (all nodes share one failure_domain).
+    # Fresh FD+HA activation bypasses this fallback via fd_desired_layout,
+    # but reactivation and non-HA/non-fresh paths still rely on it.
+    snodes = sorted(snodes, key=lambda n: n.failure_domain)
     if cluster.ha_type == "ha":
         for snode in snodes:
             # Do not assign secondary to removed node
@@ -1081,16 +1101,21 @@ def _fd_fail(msg: str) -> None:
                     secondary_nodes = [fd_desired_layout[snode.get_id()][0]]
                 else:
                     secondary_nodes = storage_node_ops.get_secondary_nodes(snode)
-                if not secondary_nodes:
+                if secondary_nodes:
+                    snode = db_controller.get_storage_node_by_id(snode.get_id())
+                    snode.secondary_node_id = secondary_nodes[0]
+                    snode.write_to_db()
+                    sec_node = db_controller.get_storage_node_by_id(snode.secondary_node_id)
+                    sec_node.lvstore_stack_secondary = snode.get_id()
+                    sec_node.write_to_db()
+                elif not storage_node_ops.splice_stranded_secondary(snode):
+                    # get_secondary_nodes()'s greedy walk closed a cycle that
+                    # excludes this node, and there isn't even one existing
+                    # pairing left to splice it into (only possible this early
+                    # in the pass, before 2+ pairings exist).
                     set_cluster_status(cl_id, ols_status)
                     raise ValueError("Failed to activate cluster, No enough secondary nodes")
-
                 snode = db_controller.get_storage_node_by_id(snode.get_id())
-                snode.secondary_node_id = secondary_nodes[0]
-                snode.write_to_db()
-                sec_node = db_controller.get_storage_node_by_id(snode.secondary_node_id)
-                sec_node.lvstore_stack_secondary = snode.get_id()
-                sec_node.write_to_db()
                 used_nodes_as_sec.append(snode.secondary_node_id)
 
             # Assign second secondary when max_fault_tolerance >= 2
@@ -1109,15 +1134,19 @@ def _fd_fail(msg: str) -> None:
                         exclude_failure_domains=[sec_node.failure_domain],
                         exclude_physical_labels=[sec_node.physical_label],
                     )
-                if not secondary_nodes_2:
+                if secondary_nodes_2:
+                    snode.tertiary_node_id = secondary_nodes_2[0]
+                    snode.write_to_db()
+                    sec_node_2 = db_controller.get_storage_node_by_id(snode.tertiary_node_id)
+                    sec_node_2.lvstore_stack_tertiary = snode.get_id()
+                    sec_node_2.write_to_db()
+                elif not storage_node_ops.splice_stranded_tertiary(snode):
+                    # get_secondary_nodes_2()'s greedy walk closed a cycle that
+                    # excludes this node, and there isn't even one existing
+                    # tertiary pairing left to splice it into.
                     set_cluster_status(cl_id, ols_status)
                     raise ValueError("Failed to activate cluster, not enough nodes for dual fault tolerance")
-
-                snode.tertiary_node_id = secondary_nodes_2[0]
-                snode.write_to_db()
-                sec_node_2 = db_controller.get_storage_node_by_id(snode.tertiary_node_id)
-                sec_node_2.lvstore_stack_tertiary = snode.get_id()
-                sec_node_2.write_to_db()
+                snode = db_controller.get_storage_node_by_id(snode.get_id())
                 used_nodes_as_tertiary.append(snode.tertiary_node_id)
 
     # Pass 1: bring up the primary LVS on every online primary node.

simplyblock_core/controllers/cluster_expansion/planner.py

@@ -503,6 +503,47 @@ def fd_balance_violation(
     return None
 
 
+def fd_activation_domain_count_violation(
+    npcs: int, distinct_domain_count: int,
+) -> Optional[str]:
+    """Validate the number of distinct failure domains for fresh activation.
+
+    A 2-FD layout can never absorb a second independent failure once one
+    domain is fully down (confirmed with the backend team), so it is not
+    supported at any npcs level.
+
+    The bare *correctness* minimum for the rotation layout itself is
+    npcs+1 (e.g. 2 domains for npcs=1, 3 for npcs=2 -- below that even the
+    initial static placement is wrong: at exactly 2 domains the tertiary
+    role mathematically always lands back in the primary's own domain,
+    since "2 steps ahead" in a period-2 round-robin wraps to where it
+    started; verified directly against rotation_layout()). But a
+    minimum-correct STATIC layout has zero spare hosts per domain, and the
+    moment a single node is added or removed, the relocation logic
+    (_pick_replica_relocation_node) has no spare candidate left to
+    reassign the stranded role to -- verified directly: removing one node
+    from a bare-minimum npcs=1/2-domain or npcs=2/3-domain layout strands
+    another node's secondary/tertiary with no replacement at all, blocking
+    the removal outright rather than just degrading placement quality.
+
+    Requiring npcs+2 domains (3 for npcs=1, 4 for npcs=2, which also rules
+    out exactly 2 for both) keeps one domain of spare capacity beyond the
+    bare correctness floor, so a single add/remove has somewhere to place
+    the relocated role instead of failing immediately. Returns a
+    human-readable reason on violation, ``None`` when the count is
+    acceptable.
+    """
+    min_domains = npcs + 2
+    if distinct_domain_count < min_domains:
+        return (
+            f"failure domains are enabled with npcs={npcs}, which requires at "
+            f"least {min_domains} distinct failure domains (2 domains is not "
+            f"supported at any npcs level); currently have "
+            f"{distinct_domain_count}. Add hosts in additional domains, or "
+            f"disable failure domains, then activate.")
+    return None
+
+
 # ---------------------------------------------------------------------------
 # Persistence helpers for ``Cluster.expand_state``.
 #

simplyblock_core/models/nvme_device.py

@@ -128,9 +128,6 @@ class JMDevice(NVMeDevice):
     # the per-leg member partitions. Empty for single-device (no-raid) JMs.
     jm_leg_bdevs: List[str] = []
     jm_leg_members: List = []
-    # When attaching this JM to a node, override the device name on that node.
-    # This is needed when a JM device is removed and needed to be replaced, but the name must be the same.
-    override_name_on_node: dict[str, str] = {} # node_id: new_name
 
 
 class RemoteDevice(BaseModel):

simplyblock_core/rpc_client.py

@@ -1418,6 +1418,31 @@ def jc_explicit_synchronization(self, jm_vuid):
         }
         return self._request("jc_explicit_synchronization", params)
 
+    def jc_replace_jm(self, name_old: str, name_new: str):
+        """Swap the JM bdev backing a live JC member from ``name_old`` to
+        ``name_new`` in place -- JC re-syncs the new JM's journal in the
+        background and, from then on, treats it as the member for this slot.
+        Replaces the old override_name_on_node naming trick (which faked the
+        replacement under the removed peer's old name so JC wouldn't need
+        touching): this RPC updates JC's live state directly, so the caller
+        can connect the replacement under its own natural name.
+
+        ``name_new`` must already exist as a bdev (the caller connects it
+        first) and must not already be in use by JC. Raises RPCRemoteError
+        with one of the documented codes on rejection/failure:
+            -10 JC is closing
+            -11 invalid JM names (empty, or name_old == name_new)
+            -12 another JM replacement is already in progress
+            -13 name_old is not currently used by JC
+            -14 name_new is already used by JC
+            -15 the JM of name_old is being removed
+            -3  JC started closing during the operation
+            -4  the JM context disappeared during the operation
+            -5  failed to re-key the JM lists (OOM) -- affected vuids stopped
+            -6  timed out connecting to the new JM bdev
+        """
+        return self._request3("jc_replace_jm", name_old=name_old, name_new=name_new)
+
     def listeners_del(self, nqn, trtype, traddr, trsvcid):
         """"
             nqn: Subsystem NQN.

simplyblock_core/services/tasks_runner_migration.py

@@ -86,6 +86,12 @@ def task_runner(task):
             _migration_retry_allowed(task, unavailable)
         return False
 
+    if snode.status == StorageNode.STATUS_REMOVED:
+        task.status = JobSchedule.STATUS_DONE
+        task.function_result = f"Node found in r: {task.node_id}"
+        task.write_to_db(db.kv_store)
+        return True
+
     cluster = db.get_cluster_by_id(task.cluster_id)
     if cluster.status not in Cluster.OPERABLE_STATUSES:
         task.function_result = "cluster is not active, retrying"