From 9d6a224291575b1332509f1314526d0664203df9 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Wed, 6 Aug 2025 16:12:26 +0100 Subject: [PATCH] ceph-volume: avoid RuntimeError on ceph-volume raw list with non-existent loop devices Signed-off-by: Parfait Detchenou --- .../ceph_volume/devices/raw/list.py | 15 +++++- .../ceph_volume/tests/devices/lvm/test_zap.py | 9 ++++ .../tests/devices/raw/test_list.py | 50 ++++++++++++++++++- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/ceph-volume/ceph_volume/devices/raw/list.py b/src/ceph-volume/ceph_volume/devices/raw/list.py index 68923216a4116..00eda056c451b 100644 --- a/src/ceph-volume/ceph_volume/devices/raw/list.py +++ b/src/ceph-volume/ceph_volume/devices/raw/list.py @@ -1,13 +1,15 @@ from __future__ import print_function +from typing import Any, Dict, Optional, List as _List +import os import argparse import json import logging from textwrap import dedent +from concurrent.futures import ThreadPoolExecutor + from ceph_volume import decorators, process from ceph_volume.util import disk from ceph_volume.util.device import Device -from typing import Any, Dict, Optional, List as _List -from concurrent.futures import ThreadPoolExecutor logger = logging.getLogger(__name__) @@ -57,6 +59,13 @@ class List(object): self.info_devices: _List[Dict[str, str]] = [] self.devices_to_scan: _List[str] = [] + def exclude_invalid_devices(self, devices: _List[Dict[str, str]]) -> _List[Dict[str, str]]: + return [ + dev + for dev in devices + if (dev_name := dev["NAME"]) and os.path.exists(dev_name) + ] + def exclude_atari_partitions(self) -> None: result: _List[str] = [] for info_device in self.info_devices: @@ -92,6 +101,8 @@ class List(object): # the parent disk has a bluestore header, but children may be the most appropriate # devices to return if the parent disk does not have a bluestore header. self.info_devices = disk.lsblk_all(abspath=True) + self.info_devices = self.exclude_invalid_devices(self.info_devices) + # Linux kernels built with CONFIG_ATARI_PARTITION enabled can falsely interpret # bluestore's on-disk format as an Atari partition table. These false Atari partitions # can be interpreted as real OSDs if a bluestore OSD was previously created on the false diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py index a6747f7cca2d0..66c93ee04e930 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py @@ -26,6 +26,11 @@ def process_call(command, **kw): result = [], [], 0 return result +@pytest.fixture() +def prevent_exclude_invalid_devices(): + with patch('ceph_volume.devices.raw.list.List.exclude_invalid_devices') as mock: + mock.side_effect = lambda x: x + yield class TestZap: def test_invalid_osd_id_passed(self) -> None: @@ -128,6 +133,7 @@ class TestZap: mock_zap.assert_called_once() # @patch('ceph_volume.devices.lvm.zap.disk.has_bluestore_label', Mock(return_value=True)) + @pytest.mark.usefixtures("prevent_exclude_invalid_devices") @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.devices.raw.list.List.filter_lvm_osd_devices', Mock(return_value='/dev/sdb')) @patch('ceph_volume.process.call', Mock(side_effect=process_call)) @@ -156,6 +162,7 @@ class TestZap: assert z.args.devices[0].path == '/dev/VolGroup/lv' mock_zap.assert_called_once + @pytest.mark.usefixtures("prevent_exclude_invalid_devices") @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.devices.raw.list.List.filter_lvm_osd_devices', Mock(return_value='/dev/sdb')) @patch('ceph_volume.process.call', Mock(side_effect=process_call)) @@ -186,6 +193,7 @@ class TestZap: assert z.args.devices[0].path == '/dev/VolGroup/lv' mock_zap.assert_called_once + @pytest.mark.usefixtures("prevent_exclude_invalid_devices") @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.devices.raw.list.List.filter_lvm_osd_devices', Mock(return_value='/dev/sdb')) @patch('ceph_volume.process.call', Mock(side_effect=process_call)) @@ -199,6 +207,7 @@ class TestZap: assert z.args.devices[0].path == '/dev/sdb' mock_zap.assert_called_once + @pytest.mark.usefixtures("prevent_exclude_invalid_devices") @patch('ceph_volume.devices.lvm.zap.Zap.zap') @patch('ceph_volume.devices.raw.list.List.filter_lvm_osd_devices', Mock(side_effect=['/dev/vdx', '/dev/vdy', '/dev/vdz', None])) @patch('ceph_volume.process.call', Mock(side_effect=process_call)) diff --git a/src/ceph-volume/ceph_volume/tests/devices/raw/test_list.py b/src/ceph-volume/ceph_volume/tests/devices/raw/test_list.py index d0d68d61116d4..aa06982a65f68 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/raw/test_list.py +++ b/src/ceph-volume/ceph_volume/tests/devices/raw/test_list.py @@ -3,6 +3,7 @@ import pytest from .data_list import ceph_bluestore_tool_show_label_output from unittest.mock import patch, Mock from ceph_volume.devices import raw +from ceph_volume.devices.raw import list as list_command # Sample lsblk output is below that overviews the test scenario. (--json output for reader clarity) # - sda and all its children are used for the OS @@ -127,12 +128,14 @@ class TestList(object): @patch('ceph_volume.util.disk.has_bluestore_label') @patch('ceph_volume.process.call') @patch('ceph_volume.util.disk.lsblk_all') - def test_raw_list(self, patched_disk_lsblk, patched_call, patched_bluestore_label, patched_get_devices): + @patch('ceph_volume.devices.raw.list.os.path.exists') + def test_raw_list(self, patched_path_exists, patched_disk_lsblk, patched_call, patched_bluestore_label, patched_get_devices): raw.list.logger.setLevel("DEBUG") patched_call.side_effect = _process_call_side_effect patched_disk_lsblk.side_effect = _lsblk_all_devices patched_bluestore_label.side_effect = _has_bluestore_label_side_effect patched_get_devices.side_effect = _devices_side_effect + patched_path_exists.return_value = True result = raw.list.List([]).generate() assert len(result) == 3 @@ -161,13 +164,15 @@ class TestList(object): @patch('ceph_volume.util.disk.has_bluestore_label') @patch('ceph_volume.process.call') @patch('ceph_volume.util.disk.lsblk_all') - def test_raw_list_with_OSError(self, patched_disk_lsblk, patched_call, patched_bluestore_label, patched_get_devices): + @patch('ceph_volume.devices.raw.list.os.path.exists') + def test_raw_list_with_OSError(self, patched_path_exists, patched_disk_lsblk, patched_call, patched_bluestore_label, patched_get_devices): def _has_bluestore_label_side_effect_with_OSError(device_path): if device_path == "/dev/sdd": raise OSError('fake OSError') return _has_bluestore_label_side_effect(device_path) raw.list.logger.setLevel("DEBUG") + patched_path_exists.return_value = True patched_disk_lsblk.side_effect = _lsblk_all_devices patched_call.side_effect = _process_call_side_effect patched_bluestore_label.side_effect = _has_bluestore_label_side_effect_with_OSError @@ -176,3 +181,44 @@ class TestList(object): result = raw.list.List([]).generate() assert len(result) == 2 assert {'sdb-uuid', 'sde1-uuid'} == set(result.keys()) + + @patch('ceph_volume.devices.raw.list.os.path.exists') + def test_raw_list_exclude_non_existent_loop_devices(self, path_exists_patch): + def path_exists_side_effect(path): + return path in ["/dev/sda"] + path_exists_patch.side_effect = path_exists_side_effect + + devices = [ + {"NAME": "/dev/loop0", "KNAME": "/dev/loop0", "PKNAME": "", "TYPE": "loop"}, + {"NAME": "/dev/nvme1n1", "KNAME": "/dev/nvme1n1", "PKNAME": "", "TYPE": "disk"}, + {"NAME": "/dev/sda", "KNAME": "/dev/sda", "PKNAME": "", "TYPE": "disk"}, + ] + cmd = list_command.List([]) + + assert cmd.exclude_invalid_devices(devices) == [ + {"NAME": "/dev/sda", "KNAME": "/dev/sda", "PKNAME": "", "TYPE": "disk"} + ] + + @patch("ceph_volume.devices.raw.list.List.exclude_lvm_osd_devices", Mock()) + @patch("ceph_volume.util.device.disk.get_devices") + @patch("ceph_volume.util.disk.has_bluestore_label") + @patch("ceph_volume.process.call") + @patch("ceph_volume.util.disk.lsblk_all") + def test_exclude_invalid_devices_is_called( + self, + patched_disk_lsblk, + patched_call, + patched_bluestore_label, + patched_get_devices, + ): + patched_disk_lsblk.side_effect = _lsblk_all_devices + patched_call.side_effect = _process_call_side_effect + patched_bluestore_label.side_effect = _has_bluestore_label_side_effect + patched_get_devices.side_effect = _devices_side_effect + + with patch( + "ceph_volume.devices.raw.list.List.exclude_invalid_devices" + ) as mock: + list_command.List([]).generate() + mock.assert_called_once() + -- 2.39.5