update and improve pool.replace api test

(cherry picked from commit 0026995)

src/middlewared/middlewared/test/integration/assets/pool.py

@@ -6,28 +6,19 @@
 from middlewared.service_exception import InstanceNotFound
 from middlewared.test.integration.utils import call, fail, pool
 
-
-mirror_topology = (2, lambda disks: {
-    "data": [
-        {"type": "MIRROR", "disks": disks[0:2]}
-    ],
+_1_disk_stripe_topology = (1, lambda disks: {
+    "data": [{"type": "STRIPE", "disks": disks[0:1]}],
+})
+_2_disk_mirror_topology = (2, lambda disks: {
+    "data": [{"type": "MIRROR", "disks": disks[0:2]}],
+})
+_4_disk_raidz2_topology = (4, lambda disks: {
+    "data": [{"type": "RAIDZ2", "disks": disks[0:4]}],
 })
-
 another_pool_topologies = [
-    (1, lambda disks: {
-        "data": [
-            {"type": "STRIPE", "disks": disks[0:1]},
-        ],
-    }),
-    mirror_topology,
-    (4, lambda disks: {
-        "data": [
-            {
-                "type": "RAIDZ2",
-                "disks": disks[0:4]
-            }
-        ],
-    }),
+    _1_disk_stripe_topology,
+    _2_disk_mirror_topology,
+    _4_disk_raidz2_topology,
 ]
 
 

tests/api2/test_pool_replace_disk.py

@@ -1,77 +1,68 @@
-import pytest
-
 from time import sleep
-from middlewared.test.integration.assets.pool import mirror_topology, another_pool_topologies, another_pool
-from middlewared.test.integration.utils import call
-from auto_config import ha
-pytestmark = [
-    pytest.mark.skipif(ha, reason='Skipping for HA testing'),
-]
-
-
-def disks(topology):
-    flat = call("pool.flatten_topology", topology)
-    return [vdev for vdev in flat if vdev["type"] == "DISK"]
 
+import pytest
 
-@pytest.mark.parametrize("topology", another_pool_topologies[1:])
-@pytest.mark.parametrize("i", list(range(0, max(topology[0] for topology in another_pool_topologies))))
-def test_pool_replace_disk(topology, i):
-    count = topology[0]
-    if i >= count:
-        return
+from middlewared.test.integration.assets.pool import _2_disk_mirror_topology, _4_disk_raidz2_topology, another_pool
+from middlewared.test.integration.utils import call
 
-    with another_pool(topology=topology) as pool:
-        assert len(disks(pool["topology"])) == count
 
-        to_replace_vdev = disks(pool["topology"])[i]
-        to_replace_disk = call("disk.query", [["devname", "=", to_replace_vdev["disk"]]],
-                               {"get": True, "extra": {"pools": True}})
+@pytest.mark.parametrize("topology", [_2_disk_mirror_topology, _4_disk_raidz2_topology])
+def test_pool_replace_disk(topology):
+    """This tests the following:
+        1. create a zpool based on the `topology`
+        2. flatten the newly created zpools topology
+        3. verify the zpool vdev size matches reality
+        4. choose 1st vdev from newly created zpool
+        5. choose 1st disk in vdev from step #4
+        6. choose 1st disk in disk.get_unused as replacement disk
+        7. call pool.replace using disk from step #5 with disk from step #6
+        8. validate that the disk being replaced still has zfs partitions
+        9. validate pool.get_instance topology info shows the replacement disk
+        10. validate disk.get_instance associates the replacement disk with the zpool
+    """
+    with another_pool(topology=topology) as pool:  # step 1
+        # step 2
+        flat_top = call("pool.flatten_topology", pool["topology"])
+        pool_top = [vdev for vdev in flat_top if vdev["type"] == "DISK"]
+        # step 3
+        assert len(pool_top) == topology[0]
+
+        # step 4
+        to_replace_vdev = pool_top[0]
+        # step 5
+        to_replace_disk = call(
+            "disk.query", [["devname", "=", to_replace_vdev["disk"]]], {"get": True, "extra": {"pools": True}}
+        )
         assert to_replace_disk["pool"] == pool["name"]
 
+        # step 6
         new_disk = call("disk.get_unused")[0]
 
-        call("pool.replace", pool["id"], {
-            "label": to_replace_vdev["guid"],
-            "disk": new_disk["identifier"],
-            "force": True,
-        }, job=True)
-
-        # Sometimes the VM is slow so look 5 times with 1 second in between
-        for _ in range(5):
-            pool = call("pool.get_instance", pool["id"])
-            if len(disks(pool["topology"])) == count:
+        # step 7
+        call("pool.replace", pool["id"], {"label": to_replace_vdev["guid"], "disk": new_disk["identifier"]}, job=True)
+
+        # step 8
+        assert call("disk.gptid_from_part_type", to_replace_disk["devname"], call("disk.get_zfs_part_type"))
+
+        # step 9
+        found = False
+        for _ in range(10):
+            if not found:
+                for i in call("pool.flatten_topology", call("pool.get_instance", pool["id"])["topology"]):
+                    if i["type"] == "DISK" and i["disk"] == new_disk["devname"]:
+                        found = True
+                        break
+                else:
+                    sleep(1)
+
+        assert found, f'Failed to detect replacement disk {new_disk["devname"]!r} in zpool {pool["name"]!r}'
+
+        # step 10 (NOTE: disk.sync_all takes awhile so we retry a few times here)
+        for _ in range(30):
+            cmd = ("disk.get_instance", new_disk["identifier"], {"extra": {"pools": True}})
+            if call(*cmd)["pool"] == pool["name"]:
                 break
-            sleep(1)
-
-        assert len(disks(pool["topology"])) == count
-        assert disks(pool["topology"])[i]["disk"] == new_disk["devname"]
-
-        assert call("disk.get_instance", new_disk["identifier"], {"extra": {"pools": True}})["pool"] == pool["name"]
-        assert call("disk.get_instance", to_replace_disk["identifier"], {"extra": {"pools": True}})["pool"] is None
-
-
-@pytest.mark.parametrize("swaps", [[0, 0], [2, 2], [2, 4]])
-def test_pool_replace_disk_swap(swaps):
-    unused = call("disk.get_unused")
-    if len(unused) < 3:
-        raise RuntimeError(f"At least 3 unused disks required to run this test")
-
-    test_disks = unused[:3]
-
-    sizes = {disk["name"]: disk["size"] for disk in test_disks}
-    assert len(set(sizes.values())) == 1, sizes
-
-    call("system.advanced.update", {"swapondrive": swaps[0]})
-    try:
-        with another_pool(topology=mirror_topology) as pool:
-            to_replace_vdev = disks(pool["topology"])[0]
-
-            call("system.advanced.update", {"swapondrive": swaps[1]})
-            call("pool.replace", pool["id"], {
-                "label": to_replace_vdev["guid"],
-                "disk": test_disks[2]["identifier"],
-                "force": True,
-            }, job=True)
-    finally:
-        call("system.advanced.update", {"swapondrive": 2})
+            else:
+                sleep(1)
+        else:
+            assert False, f"{' '.join(cmd)} failed to update with pool information"