From 8181abc4bec3484d71f64c6705d35dfeb5ddcced Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Tue, 21 Jun 2022 11:28:30 -0600 Subject: [PATCH] ceph-volume: Rename env var; add warning So that we can have a nice big warning that fires once per invocation, I think using a callable class with a class attribute seems like a decent approach. A closure could work too. Signed-off-by: Zack Cerza (cherry picked from commit 69f58f51a2d7967d597a8d61c0ab20b52e8dc374) --- .../ceph_volume/tests/util/test_device.py | 4 +-- .../ceph_volume/tests/util/test_disk.py | 11 ++++++ src/ceph-volume/ceph_volume/util/device.py | 5 +-- src/ceph-volume/ceph_volume/util/disk.py | 35 +++++++++++++++---- 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/ceph-volume/ceph_volume/tests/util/test_device.py b/src/ceph-volume/ceph_volume/tests/util/test_device.py index e3e3b30c4be2a..457142cbc661a 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_device.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py @@ -91,11 +91,11 @@ class TestDevice(object): def test_loop_device_is_device(self, fake_call, device_info): data = {"/dev/loop0": {"foo": "bar"}} lsblk = {"TYPE": "loop"} - os.environ["CEPH_VOLUME_USE_LOOP_DEVICES"] = "1" + os.environ["CEPH_VOLUME_ALLOW_LOOP_DEVICES"] = "1" device_info(devices=data, lsblk=lsblk) disk = device.Device("/dev/loop0") assert disk.is_device is True - del os.environ["CEPH_VOLUME_USE_LOOP_DEVICES"] + del os.environ["CEPH_VOLUME_ALLOW_LOOP_DEVICES"] def test_device_is_rotational(self, fake_call, device_info): data = {"/dev/sda": {"rotational": "1"}} diff --git a/src/ceph-volume/ceph_volume/tests/util/test_disk.py b/src/ceph-volume/ceph_volume/tests/util/test_disk.py index 44f19e036fa47..65fd994a795aa 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_disk.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_disk.py @@ -549,3 +549,14 @@ class TestSizeSpecificFormatting(object): result = "%s" % size.tb assert "%s" % size.tb == "%s" % size.terabytes assert result == "1027.00 TB" + + +class TestAllowLoopDevsWarning(object): + def test_loop_dev_warning(self, fake_call, caplog): + assert disk.allow_loop_devices() is False + assert not caplog.records + os.environ['CEPH_VOLUME_ALLOW_LOOP_DEVICES'] = "y" + assert disk.allow_loop_devices() is True + log = caplog.records[0] + assert log.levelname == "WARNING" + assert "will never be supported in production" in log.message diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py index c556e56768ddd..8ccb320238f46 100644 --- a/src/ceph-volume/ceph_volume/util/device.py +++ b/src/ceph-volume/ceph_volume/util/device.py @@ -8,6 +8,7 @@ from ceph_volume.api import lvm from ceph_volume.util import disk, system from ceph_volume.util.lsmdisk import LSMDisk from ceph_volume.util.constants import ceph_disk_guids +from ceph_volume.util.disk import allow_loop_devices logger = logging.getLogger(__name__) @@ -214,7 +215,7 @@ class Device(object): device_type = dev.get('TYPE', '') # always check is this is an lvm member valid_types = ['part', 'disk'] - if os.environ.get("CEPH_VOLUME_USE_LOOP_DEVICES", False): + if allow_loop_devices(): valid_types.append('loop') if device_type in valid_types: self._set_lvm_membership() @@ -465,7 +466,7 @@ class Device(object): api = self.blkid_api if api: valid_types = ['disk', 'device', 'mpath'] - if os.environ.get("CEPH_VOLUME_USE_LOOP_DEVICES", False): + if allow_loop_devices(): valid_types.append('loop') return self.device_type in valid_types return False diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py index 923c12f5246cd..00e9f2a44f983 100644 --- a/src/ceph-volume/ceph_volume/util/disk.py +++ b/src/ceph-volume/ceph_volume/util/disk.py @@ -343,12 +343,8 @@ def is_device(dev): if not dev.startswith('/dev/'): return False if dev[len('/dev/'):].startswith('loop'): - if os.environ.get("CEPH_VOLUME_USE_LOOP_DEVICES", False) is False: + if not allow_loop_devices(): return False - logger.info( - "Allowing the use of loop devices since " - "CEPH_VOLUME_USE_LOOP_DEVICES is set." - ) # fallback to stat return _stat_is_device(os.lstat(dev).st_mode) @@ -759,6 +755,31 @@ def get_block_devs_lsblk(device=''): return [re.split(r'\s+', line) for line in stdout] +class AllowLoopDevices(object): + allow = False + warned = False + + @classmethod + def __call__(cls): + val = os.environ.get("CEPH_VOLUME_ALLOW_LOOP_DEVICES", "false").lower() + if val not in ("false", 'no', '0'): + cls.allow = True + if not cls.warned: + logger.warning( + "CEPH_VOLUME_ALLOW_LOOP_DEVICES is set in your " + "environment, so we will allow the use of unattached loop" + " devices as disks. This feature is intended for " + "development purposes only and will never be supported in" + " production. Issues filed based on this behavior will " + "likely be ignored." + ) + cls.warned = True + return cls.allow + + +allow_loop_devices = AllowLoopDevices() + + def get_block_devs_sysfs(_sys_block_path='/sys/block', _sys_dev_block_path='/sys/dev/block'): # First, get devices that are _not_ partitions result = list() @@ -774,7 +795,7 @@ def get_block_devs_sysfs(_sys_block_path='/sys/block', _sys_dev_block_path='/sys basename = get_file_contents(os.path.join(dm_dir_path, 'name')) name = os.path.join("/dev/mapper", basename) if dev.startswith('loop'): - if os.environ.get("CEPH_VOLUME_USE_LOOP_DEVICES", False) is False: + if not allow_loop_devices(): continue # Skip loop devices that are not attached if not os.path.exists(os.path.join(_sys_block_path, dev, 'loop')): @@ -808,7 +829,7 @@ def get_devices(_sys_block_path='/sys/block', device=''): block_devs = get_block_devs_sysfs(_sys_block_path) block_types = ['disk', 'mpath'] - if os.environ.get("CEPH_VOLUME_USE_LOOP_DEVICES", False) is not False: + if allow_loop_devices(): block_types.append('loop') for block in block_devs: -- 2.39.5