Merge pull request #68414 from guits/has_bs_repl

ceph-volume: has_bluestore_label checks all bluestore label replica o…

qa/tasks/cephadm.py

@@ -1184,23 +1184,28 @@ def ceph_osds(ctx, config):
             cur += 1
 
         if cur == 0:
-            if raw:
-                for remote, devs in devs_by_remote.items():
-                    for dev in devs:
-                        log.info(f'Zapping device {dev} on {remote.shortname} before raw OSD deployment')
-                        remote.run(
-                            args=[
-                                'sudo', 'ceph-bluestore-tool', 'zap-device',
-                                '--dev', dev,
-                                '--yes-i-really-really-mean-it',
-                            ],
-                            check_status=False,
-                        )
-                        remote.run(args=['sudo', 'wipefs', '--all', dev], check_status=False)
-                        remote.run(
-                            args=['sudo', 'dd', 'if=/dev/zero', f'of={dev}', 'bs=1M', 'count=10', 'conv=fsync'],
-                            check_status=False,
-                        )
+            for remote, devs in devs_by_remote.items():
+                for dev in devs:
+                    log.info(f'Zapping device {dev} on {remote.shortname} before OSD deployment')
+                    remote.run(
+                        args=[
+                            'sudo',
+                            ctx.cephadm,
+                            '--image', ctx.ceph[cluster_name].image,
+                            'ceph-volume',
+                            '-c', '/etc/ceph/{}.conf'.format(cluster_name),
+                            '-k', '/etc/ceph/{}.client.admin.keyring'.format(cluster_name),
+                            '--fsid', ctx.ceph[cluster_name].fsid,
+                            '--', 'lvm', 'zap', dev
+                        ],
+                        check_status=False,
+                     )
+                    remote.run(args=['sudo', 'wipefs', '--all', dev], check_status=False)
+                    remote.run(
+                        args=['sudo', 'dd', 'if=/dev/zero', f'of={dev}', 'bs=1M', 'count=10', 'conv=fsync'],
+                        check_status=False,
+                    )
+            _shell(ctx, cluster_name, remote, ['ceph', 'orch', 'device', 'ls', '--refresh'])
             osd_cmd = ['ceph', 'orch', 'apply', 'osd', '--all-available-devices']
             if raw:
                 osd_cmd.extend(['--method', 'raw'])

src/ceph-volume/ceph_volume/tests/util/test_disk.py

@@ -1,3 +1,4 @@
+import errno
 import pytest
 import stat
 from typing import Any, Callable, ClassVar, Optional
@@ -679,6 +680,91 @@ class TestHasBlueStoreLabel(object):
         fake_filesystem.create_dir(device_path)
         assert not disk.has_bluestore_label(device_path)
 
+    def test_bluestore_label_at_replica_offset(self):
+        class _FakeBlockDev:
+            def __init__(self, label_offset):
+                self._label_offset = label_offset
+                self._pos = 0
+
+            def seek(self, offset, whence=0):
+                if whence != 0:
+                    raise NotImplementedError
+                self._pos = offset
+                return offset
+
+            def read(self, n):
+                buf_start = self._pos
+                self._pos += n
+                sig = disk.BLUESTORE_BDEV_LABEL_SIGNATURE
+                if buf_start == self._label_offset:
+                    return sig[:n] + (b'\x00' * max(0, n - len(sig)))
+                return b'\x00' * n
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return None
+
+        for off in disk.BLUESTORE_BDEV_LABEL_OFFSETS[1:3]:
+            with patch('builtins.open', lambda *a, **k: _FakeBlockDev(off)):
+                assert disk.has_bluestore_label('/dev/fake'), off
+
+    def test_bluestore_label_non_seekable_returns_false(self):
+        class _NonSeekable:
+            def seek(self, offset, whence=0):
+                raise OSError(errno.EINVAL, 'Invalid argument')
+
+            def read(self, n):
+                return b'\x00' * n
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return None
+
+        with patch('builtins.open', lambda *a, **k: _NonSeekable()):
+            assert not disk.has_bluestore_label('/dev/fake')
+
+    def test_bluestore_label_non_seekable_fallback_read_matches(self):
+        class _NonSeekableWithLabelAtStart:
+            def seek(self, offset, whence=0):
+                raise OSError(errno.EINVAL, 'Invalid argument')
+
+            def read(self, n):
+                return disk.BLUESTORE_BDEV_LABEL_SIGNATURE[:n] + (
+                    b'\x00' * max(0, n - len(disk.BLUESTORE_BDEV_LABEL_SIGNATURE))
+                )
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return None
+
+        with patch('builtins.open', lambda *a, **k: _NonSeekableWithLabelAtStart()):
+            assert disk.has_bluestore_label('/dev/fake')
+
+    def test_bluestore_label_io_error_propagates(self):
+        class _EIOOnSeek:
+            def seek(self, offset, whence=0):
+                raise OSError(errno.EIO, 'Input/output error')
+
+            def read(self, n):
+                return b'\x00' * n
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *args):
+                return None
+
+        with patch('builtins.open', lambda *a, **k: _EIOOnSeek()):
+            with pytest.raises(OSError) as excinfo:
+                disk.has_bluestore_label('/dev/fake')
+            assert excinfo.value.errno == errno.EIO
+
 
 class TestBlockSysFs(TestCase):
     def setUp(self) -> None:

src/ceph-volume/ceph_volume/util/disk.py

@@ -1,3 +1,4 @@
+import errno
 import logging
 import os
 import re
@@ -12,6 +13,16 @@ from typing import Dict, List, Any, Union, Optional
 
 logger = logging.getLogger(__name__)
 
+_ONE_GIB = 1024 * 1024 * 1024
+BLUESTORE_BDEV_LABEL_OFFSETS = (
+    0,
+    _ONE_GIB,
+    10 * _ONE_GIB,
+    100 * _ONE_GIB,
+    1000 * _ONE_GIB,
+)
+BLUESTORE_BDEV_LABEL_SIGNATURE = b'bluestore block device'
+
 
 # The blkid CLI tool has some oddities which prevents having one common call
 # to extract the information instead of having separate utilities. The `udev`
@@ -884,21 +895,30 @@ def get_devices(_sys_block_path='/sys/block', device=''):
     return device_facts
 
 def has_bluestore_label(device_path: str) -> bool:
-    isBluestore = False
-    bluestoreDiskSignature = 'bluestore block device' # 22 bytes long
-
-    # throws OSError on failure
     logger.info("opening device {} to check for BlueStore label".format(device_path))
+    sig_len = len(BLUESTORE_BDEV_LABEL_SIGNATURE)
     try:
         with open(device_path, "rb") as fd:
-            # read first 22 bytes looking for bluestore disk signature
-            signature = fd.read(22)
-            if signature.decode('ascii', 'replace') == bluestoreDiskSignature:
-                isBluestore = True
+            for position in BLUESTORE_BDEV_LABEL_OFFSETS:
+                try:
+                    fd.seek(position)
+                except OSError as exc:
+                    err = exc.errno
+                    if err is None or err not in (
+                        errno.EINVAL,
+                        errno.ESPIPE,
+                        errno.ENOTTY,
+                    ):
+                        raise
+                    if position != 0:
+                        continue
+                signature = fd.read(sig_len)
+                if signature == BLUESTORE_BDEV_LABEL_SIGNATURE:
+                    return True
     except IsADirectoryError:
         logger.info(f'{device_path} is a directory, skipping.')
 
-    return isBluestore
+    return False
 
 def has_seastore_label(device_path: str) -> bool:
     is_seastore = False