From 421a67ea04ff4f32d26f5f9220d344eaa0240f46 Mon Sep 17 00:00:00 2001 From: Caleb Date: Tue, 26 Nov 2024 11:51:09 -0500 Subject: [PATCH 1/2] address review (cherry picked from commit 4595210a68ccedc88138350f8f35ef9baaf375ac) --- .../middlewared/plugins/zfs_/pool_status.py | 53 +++-- tests/api2/test_alert.py | 74 ++++++ tests/api2/test_zpool_status.py | 220 +++++++++--------- 3 files changed, 211 insertions(+), 136 deletions(-) create mode 100644 tests/api2/test_alert.py diff --git a/src/middlewared/middlewared/plugins/zfs_/pool_status.py b/src/middlewared/middlewared/plugins/zfs_/pool_status.py index d73782e862c0..4901d2f717e4 100644 --- a/src/middlewared/middlewared/plugins/zfs_/pool_status.py +++ b/src/middlewared/middlewared/plugins/zfs_/pool_status.py @@ -95,47 +95,46 @@ def status(self, data): ] } }, - "evo": { - "spares": {}, - "logs": {}, - "dedup": {}, - "special": {}, - "l2cache": {}, - "data": { - "/dev/disk/by-partuuid/d9cfa346-8623-402f-9bfe-a8256de902ec": { - "pool_name": "evo", - "disk_status": "ONLINE", - "disk_read_errors": 0, - "disk_write_errors": 0, - "disk_checksum_errors": 0, - "vdev_name": "stripe", - "vdev_type": "data", - "vdev_disks": [ - "/dev/disk/by-partuuid/d9cfa346-8623-402f-9bfe-a8256de902ec" - ] - } + "pools": { + "evo": { + "spares": {}, + "logs": {}, + "dedup": {}, + "special": {}, + "l2cache": {}, + "data": { + "/dev/disk/by-partuuid/d9cfa346-8623-402f-9bfe-a8256de902ec": { + "pool_name": "evo", + "disk_status": "ONLINE", + "disk_read_errors": 0, + "disk_write_errors": 0, + "disk_checksum_errors": 0, + "vdev_name": "stripe", + "vdev_type": "data", + "vdev_disks": [ + "/dev/disk/by-partuuid/d9cfa346-8623-402f-9bfe-a8256de902ec" + ] + } + } } - } } """ - pools = get_zpool_status(data.get('name')) - - final = {'disks': dict()} - for pool_name, pool_info in pools.items(): - final[pool_name] = dict() + final = {'disks': dict(), 'pools': dict()} + for pool_name, pool_info in get_zpool_status(data.get('name')).items(): + final['pools'][pool_name] = dict() # We need some normalization for data vdev here pool_info['data'] = pool_info.get('vdevs', {}).get(pool_name, {}).get('vdevs', {}) for vdev_type in ('spares', 'logs', 'dedup', 'special', 'l2cache', 'data'): vdev_members = pool_info.get(vdev_type, {}) if not vdev_members: - final[pool_name][vdev_type] = dict() + final['pools'][pool_name][vdev_type] = dict() continue info = self.status_impl(pool_name, vdev_type, vdev_members, **data) # we key on pool name and disk id because # this was designed, primarily, for the # `webui.enclosure.dashboard` endpoint - final[pool_name][vdev_type] = info + final['pools'][pool_name][vdev_type] = info final['disks'].update(info) return final diff --git a/tests/api2/test_alert.py b/tests/api2/test_alert.py new file mode 100644 index 000000000000..4222a75de3ad --- /dev/null +++ b/tests/api2/test_alert.py @@ -0,0 +1,74 @@ +from time import sleep + +import pytest + +from auto_config import pool_name +from middlewared.test.integration.utils import call, ssh + + +ID_PATH = "/dev/disk/by-partuuid/" + + +def get_alert_by_id(alert_id): + return next(filter(lambda alert: alert["id"] == alert_id, call("alert.list")), None) + + +def wait_for_alert(timeout=120): + for _ in range(timeout): + for alert in call("alert.list"): + if ( + alert["source"] == "VolumeStatus" and + alert["args"]["volume"] == pool_name and + alert["args"]["state"] == "DEGRADED" + ): + return alert["id"] + sleep(1) + + +@pytest.fixture(scope="module") +def degraded_pool_gptid(): + get_pool = call("pool.query", [["name", "=", pool_name]], {"get": True}) + gptid = get_pool["topology"]["data"][0]["path"].replace(ID_PATH, "") + ssh(f"zinject -d {gptid} -A fault {pool_name}") + return gptid + + +@pytest.fixture(scope="module") +def alert_id(degraded_pool_gptid): + call("alert.process_alerts") + result = wait_for_alert() + if result is None: + pytest.fail("Timed out while waiting for alert.") + return result + + +def test_verify_the_pool_is_degraded(degraded_pool_gptid): + status = call("zpool.status", {"name": pool_name}) + disk_status = status["pools"][pool_name]["data"][ID_PATH + degraded_pool_gptid]["disk_status"] + assert disk_status == "DEGRADED" + + +def test_dismiss_alert(alert_id): + call("alert.dismiss", alert_id) + alert = get_alert_by_id(alert_id) + assert alert["dismissed"] is True, alert + + +def test_restore_alert(alert_id): + call("alert.restore", alert_id) + alert = get_alert_by_id(alert_id) + assert alert["dismissed"] is False, alert + + +def test_clear_the_pool_degradation(degraded_pool_gptid): + ssh(f"zpool clear {pool_name}") + status = call("zpool.status", {"name": pool_name}) + disk_status = status["pools"][pool_name]["data"][ID_PATH + degraded_pool_gptid]["disk_status"] + assert disk_status != "DEGRADED" + + +@pytest.mark.timeout(120) +def test_wait_for_the_alert_to_disappear(alert_id): + call("alert.process_alerts") + while get_alert_by_id(alert_id) is not None: + sleep(1) diff --git a/tests/api2/test_zpool_status.py b/tests/api2/test_zpool_status.py index 9ca5e462ff4e..3951c9f4cd72 100644 --- a/tests/api2/test_zpool_status.py +++ b/tests/api2/test_zpool_status.py @@ -104,117 +104,119 @@ def get_pool_status(unused_disks, real_paths=False, replaced=False): ] } }, - POOL_NAME: { - 'spares': { - f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}': { - 'pool_name': POOL_NAME, - 'disk_status': 'AVAIL' if not replaced else 'INUSE', - 'disk_read_errors': 0, - 'disk_write_errors': 0, - 'disk_checksum_errors': 0, - 'vdev_name': 'stripe', - 'vdev_type': 'spares', - 'vdev_disks': [ - f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}' - ] - } - }, - 'logs': { - f'{disk_uuid_mapping[unused_disks[3]] if not real_paths else unused_disks[3]}': { - 'pool_name': POOL_NAME, - 'disk_status': 'ONLINE', - 'disk_read_errors': 0, - 'disk_write_errors': 0, - 'disk_checksum_errors': 0, - 'vdev_name': 'stripe', - 'vdev_type': 'logs', - 'vdev_disks': [ - f'{disk_uuid_mapping[unused_disks[3]] if not real_paths else unused_disks[3]}' - ] - } - }, - 'dedup': { - f'{disk_uuid_mapping[unused_disks[2]] if not real_paths else unused_disks[2]}': { - 'pool_name': POOL_NAME, - 'disk_status': 'ONLINE', - 'disk_read_errors': 0, - 'disk_write_errors': 0, - 'disk_checksum_errors': 0, - 'vdev_name': 'stripe', - 'vdev_type': 'dedup', - 'vdev_disks': [ - f'{disk_uuid_mapping[unused_disks[2]] if not real_paths else unused_disks[2]}' - ] - } - }, - 'special': { - f'{disk_uuid_mapping[unused_disks[5]] if not real_paths else unused_disks[5]}': { - 'pool_name': POOL_NAME, - 'disk_status': 'ONLINE', - 'disk_read_errors': 0, - 'disk_write_errors': 0, - 'disk_checksum_errors': 0, - 'vdev_name': 'stripe', - 'vdev_type': 'special', - 'vdev_disks': [ - f'{disk_uuid_mapping[unused_disks[5]] if not real_paths else unused_disks[5]}' - ] - } - }, - 'l2cache': { - f'{disk_uuid_mapping[unused_disks[0]] if not real_paths else unused_disks[0]}': { - 'pool_name': POOL_NAME, - 'disk_status': 'ONLINE', - 'disk_read_errors': 0, - 'disk_write_errors': 0, - 'disk_checksum_errors': 0, - 'vdev_name': 'stripe', - 'vdev_type': 'l2cache', - 'vdev_disks': [ - f'{disk_uuid_mapping[unused_disks[0]] if not real_paths else unused_disks[0]}' - ] - } - }, - 'data': { - f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}': { - 'pool_name': POOL_NAME, - 'disk_status': 'ONLINE', - 'disk_read_errors': 0, - 'disk_write_errors': 0, - 'disk_checksum_errors': 0, - 'vdev_name': 'stripe', - 'vdev_type': 'data', - 'vdev_disks': [ - f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}' - ] - } - } if not replaced else { - f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}': { - 'pool_name': POOL_NAME, - 'disk_status': 'ONLINE', - 'disk_read_errors': 0, - 'disk_write_errors': 0, - 'disk_checksum_errors': 0, - 'vdev_name': 'spare-0', - 'vdev_type': 'data', - 'vdev_disks': [ - f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}', - f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}' - ] + 'pools': { + POOL_NAME: { + 'spares': { + f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}': { + 'pool_name': POOL_NAME, + 'disk_status': 'AVAIL' if not replaced else 'INUSE', + 'disk_read_errors': 0, + 'disk_write_errors': 0, + 'disk_checksum_errors': 0, + 'vdev_name': 'stripe', + 'vdev_type': 'spares', + 'vdev_disks': [ + f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}' + ] + } + }, + 'logs': { + f'{disk_uuid_mapping[unused_disks[3]] if not real_paths else unused_disks[3]}': { + 'pool_name': POOL_NAME, + 'disk_status': 'ONLINE', + 'disk_read_errors': 0, + 'disk_write_errors': 0, + 'disk_checksum_errors': 0, + 'vdev_name': 'stripe', + 'vdev_type': 'logs', + 'vdev_disks': [ + f'{disk_uuid_mapping[unused_disks[3]] if not real_paths else unused_disks[3]}' + ] + } }, - f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}': { - 'pool_name': POOL_NAME, - 'disk_status': 'ONLINE', - 'disk_read_errors': 0, - 'disk_write_errors': 0, - 'disk_checksum_errors': 0, - 'vdev_name': 'spare-0', - 'vdev_type': 'data', - 'vdev_disks': [ - f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}', - f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}' - ] + 'dedup': { + f'{disk_uuid_mapping[unused_disks[2]] if not real_paths else unused_disks[2]}': { + 'pool_name': POOL_NAME, + 'disk_status': 'ONLINE', + 'disk_read_errors': 0, + 'disk_write_errors': 0, + 'disk_checksum_errors': 0, + 'vdev_name': 'stripe', + 'vdev_type': 'dedup', + 'vdev_disks': [ + f'{disk_uuid_mapping[unused_disks[2]] if not real_paths else unused_disks[2]}' + ] + } }, + 'special': { + f'{disk_uuid_mapping[unused_disks[5]] if not real_paths else unused_disks[5]}': { + 'pool_name': POOL_NAME, + 'disk_status': 'ONLINE', + 'disk_read_errors': 0, + 'disk_write_errors': 0, + 'disk_checksum_errors': 0, + 'vdev_name': 'stripe', + 'vdev_type': 'special', + 'vdev_disks': [ + f'{disk_uuid_mapping[unused_disks[5]] if not real_paths else unused_disks[5]}' + ] + } + }, + 'l2cache': { + f'{disk_uuid_mapping[unused_disks[0]] if not real_paths else unused_disks[0]}': { + 'pool_name': POOL_NAME, + 'disk_status': 'ONLINE', + 'disk_read_errors': 0, + 'disk_write_errors': 0, + 'disk_checksum_errors': 0, + 'vdev_name': 'stripe', + 'vdev_type': 'l2cache', + 'vdev_disks': [ + f'{disk_uuid_mapping[unused_disks[0]] if not real_paths else unused_disks[0]}' + ] + } + }, + 'data': { + f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}': { + 'pool_name': POOL_NAME, + 'disk_status': 'ONLINE', + 'disk_read_errors': 0, + 'disk_write_errors': 0, + 'disk_checksum_errors': 0, + 'vdev_name': 'stripe', + 'vdev_type': 'data', + 'vdev_disks': [ + f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}' + ] + } + } if not replaced else { + f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}': { + 'pool_name': POOL_NAME, + 'disk_status': 'ONLINE', + 'disk_read_errors': 0, + 'disk_write_errors': 0, + 'disk_checksum_errors': 0, + 'vdev_name': 'spare-0', + 'vdev_type': 'data', + 'vdev_disks': [ + f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}', + f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}' + ] + }, + f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}': { + 'pool_name': POOL_NAME, + 'disk_status': 'ONLINE', + 'disk_read_errors': 0, + 'disk_write_errors': 0, + 'disk_checksum_errors': 0, + 'vdev_name': 'spare-0', + 'vdev_type': 'data', + 'vdev_disks': [ + f'{disk_uuid_mapping[unused_disks[1]] if not real_paths else unused_disks[1]}', + f'{disk_uuid_mapping[unused_disks[4]] if not real_paths else unused_disks[4]}' + ] + }, + } } } } From 43b8c8bd28b2fbadf1b149804efa05a638e31d0b Mon Sep 17 00:00:00 2001 From: Caleb Date: Tue, 26 Nov 2024 15:52:22 -0500 Subject: [PATCH 2/2] remove test_alert.py --- tests/api2/test_alert.py | 74 ---------------------------------------- 1 file changed, 74 deletions(-) delete mode 100644 tests/api2/test_alert.py diff --git a/tests/api2/test_alert.py b/tests/api2/test_alert.py deleted file mode 100644 index 4222a75de3ad..000000000000 --- a/tests/api2/test_alert.py +++ /dev/null @@ -1,74 +0,0 @@ -from time import sleep - -import pytest - -from auto_config import pool_name -from middlewared.test.integration.utils import call, ssh - - -ID_PATH = "/dev/disk/by-partuuid/" - - -def get_alert_by_id(alert_id): - return next(filter(lambda alert: alert["id"] == alert_id, call("alert.list")), None) - - -def wait_for_alert(timeout=120): - for _ in range(timeout): - for alert in call("alert.list"): - if ( - alert["source"] == "VolumeStatus" and - alert["args"]["volume"] == pool_name and - alert["args"]["state"] == "DEGRADED" - ): - return alert["id"] - sleep(1) - - -@pytest.fixture(scope="module") -def degraded_pool_gptid(): - get_pool = call("pool.query", [["name", "=", pool_name]], {"get": True}) - gptid = get_pool["topology"]["data"][0]["path"].replace(ID_PATH, "") - ssh(f"zinject -d {gptid} -A fault {pool_name}") - return gptid - - -@pytest.fixture(scope="module") -def alert_id(degraded_pool_gptid): - call("alert.process_alerts") - result = wait_for_alert() - if result is None: - pytest.fail("Timed out while waiting for alert.") - return result - - -def test_verify_the_pool_is_degraded(degraded_pool_gptid): - status = call("zpool.status", {"name": pool_name}) - disk_status = status["pools"][pool_name]["data"][ID_PATH + degraded_pool_gptid]["disk_status"] - assert disk_status == "DEGRADED" - - -def test_dismiss_alert(alert_id): - call("alert.dismiss", alert_id) - alert = get_alert_by_id(alert_id) - assert alert["dismissed"] is True, alert - - -def test_restore_alert(alert_id): - call("alert.restore", alert_id) - alert = get_alert_by_id(alert_id) - assert alert["dismissed"] is False, alert - - -def test_clear_the_pool_degradation(degraded_pool_gptid): - ssh(f"zpool clear {pool_name}") - status = call("zpool.status", {"name": pool_name}) - disk_status = status["pools"][pool_name]["data"][ID_PATH + degraded_pool_gptid]["disk_status"] - assert disk_status != "DEGRADED" - - -@pytest.mark.timeout(120) -def test_wait_for_the_alert_to_disappear(alert_id): - call("alert.process_alerts") - while get_alert_by_id(alert_id) is not None: - sleep(1)