From: Jan Fajerski Date: Tue, 10 Dec 2019 13:56:37 +0000 (+0100) Subject: lvm: add sizing arguments to prepare and create. X-Git-Tag: v14.2.8~52^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=31fb57fccd5e46d540771eec2fadfa38a7fa007f;p=ceph.git lvm: add sizing arguments to prepare and create. This adds options to size to-be-created LVs in the prepare and create subcommands. Sizing can be done explicitly by passing a sizes or implicitly by specifying the number of slots per [data|journal|wal|db] device. The former will try to create a LV of the specified size and use that to create OSDs if it succeeds. The latter will carve up the device size into $n slots and use one of those slots for the to-be-created OSD. If partitions or LVs are passed these options are ignored. This also creates the foundation to move to byte-based sizing, by moving VolumeGroup lvm querying and size calculation to bytes as the base unit. Fixes: https://tracker.ceph.com/issues/43299 Signed-off-by: Jan Fajerski (cherry picked from commit 8b8913ad3c0b8ceae9f458fd8d3ec292c5ff5eb1) --- diff --git a/src/ceph-volume/ceph_volume/api/lvm.py b/src/ceph-volume/ceph_volume/api/lvm.py index 9849c4d6f761..bb0638148d89 100644 --- a/src/ceph-volume/ceph_volume/api/lvm.py +++ b/src/ceph-volume/ceph_volume/api/lvm.py @@ -126,7 +126,7 @@ def sizing(device_size, parts=None, size=None): return { 'parts': parts, 'percentages': percentages, - 'sizes': int(sizes), + 'sizes': int(sizes/1024/1024/1024), } @@ -516,8 +516,8 @@ def get_pv(pv_name=None, pv_uuid=None, pv_tags=None, pvs=None): # ############################# -#TODO add vg_extent_size here to have that available in VolumeGroup class -VG_FIELDS = 'vg_name,pv_count,lv_count,snap_count,vg_attr,vg_size,vg_free,vg_free_count' +VG_FIELDS = 'vg_name,pv_count,lv_count,vg_attr,vg_extent_count,vg_free_count,vg_extent_size' +VG_CMD_OPTIONS = ['--noheadings', '--readonly', '--units=b', '--nosuffix', '--separator=";"'] def get_api_vgs(): @@ -527,17 +527,15 @@ def get_api_vgs(): Command and sample delimited output should look like:: - $ vgs --noheadings --units=g --readonly --separator=';' \ - -o vg_name,pv_count,lv_count,snap_count,vg_attr,vg_size,vg_free - ubuntubox-vg;1;2;0;wz--n-;299.52g;12.00m - osd_vg;3;1;0;wz--n-;29.21g;9.21g + $ vgs --noheadings --units=b --readonly --separator=';' \ + -o vg_name,pv_count,lv_count,vg_attr,vg_free_count,vg_extent_size + ubuntubox-vg;1;2;wz--n-;12; To normalize sizing, the units are forced in 'g' which is equivalent to gigabytes, which uses multiples of 1024 (as opposed to 1000) """ stdout, stderr, returncode = process.call( - ['vgs', '--noheadings', '--readonly', '--units=g', '--separator=";"', - '-o', VG_FIELDS], + ['vgs'] + VG_CMD_OPTIONS + ['-o', VG_FIELDS], verbose_on_failure=False ) return _output_parser(stdout, VG_FIELDS) @@ -560,43 +558,19 @@ class VolumeGroup(object): def __repr__(self): return self.__str__() - def _parse_size(self, size): - error_msg = "Unable to convert vg size to integer: '%s'" % str(size) - try: - integer, _ = size.split('g') - except ValueError: - logger.exception(error_msg) - raise RuntimeError(error_msg) - - return util.str_to_int(integer) - @property def free(self): """ - Parse the available size in gigabytes from the ``vg_free`` attribute, that - will be a string with a character ('g') to indicate gigabytes in size. - Returns a rounded down integer to ease internal operations:: - - >>> data_vg.vg_free - '0.01g' - >>> data_vg.size - 0 + Return free space in VG in bytes """ - return self._parse_size(self.vg_free) + return int(self.vg_extent_size) * int(self.vg_free_count) @property def size(self): """ - Parse the size in gigabytes from the ``vg_size`` attribute, that - will be a string with a character ('g') to indicate gigabytes in size. - Returns a rounded down integer to ease internal operations:: - - >>> data_vg.vg_size - '1024.9g' - >>> data_vg.size - 1024 + Returns VG size in bytes """ - return self._parse_size(self.vg_size) + return int(self.vg_extent_size) * int(self.vg_extent_count) def sizing(self, parts=None, size=None): """ @@ -635,7 +609,8 @@ class VolumeGroup(object): vg_free_count = util.str_to_int(self.vg_free_count) if size: - extents = int(size * vg_free_count / self.free) + size = size * 1024 * 1024 * 1024 + extents = int(size / int(self.vg_extent_size)) disk_sizing = sizing(self.free, size=size, parts=parts) else: if parts is not None: @@ -651,6 +626,18 @@ class VolumeGroup(object): disk_sizing['percentages'] = extent_sizing['percentages'] return disk_sizing + def bytes_to_extents(self, size): + ''' + Return a how many extents we can fit into a size in bytes. + ''' + return int(size / int(self.vg_extent_size)) + + def slots_to_extents(self, slots): + ''' + Return how many extents fit the VG slot times + ''' + return int(int(self.vg_free_count) / slots) + class VolumeGroups(list): """ @@ -768,8 +755,6 @@ def create_vg(devices, name=None, name_prefix=None): name = "ceph-%s" % str(uuid.uuid4()) process.run([ 'vgcreate', - '-s', - '1G', '--force', '--yes', name] + devices @@ -864,8 +849,7 @@ def get_vg(vg_name=None, vg_tags=None, vgs=None): def get_device_vgs(device, name_prefix=''): stdout, stderr, returncode = process.call( - ['pvs', '--noheadings', '--readonly', '--units=g', '--separator=";"', - '-o', VG_FIELDS, device], + ['pvs'] + VG_CMD_OPTIONS + ['-o', VG_FIELDS, device], verbose_on_failure=False ) vgs = _output_parser(stdout, VG_FIELDS) @@ -1112,14 +1096,21 @@ class Volumes(list): return lvs[0] -def create_lv(name_prefix, uuid, vg=None, device=None, extents=None, size=None, tags=None): +def create_lv(name_prefix, + uuid, + vg=None, + device=None, + slots=None, + extents=None, + size=None, + tags=None): """ Create a Logical Volume in a Volume Group. Command looks like:: lvcreate -L 50G -n gfslv vg0 - ``name_prefix`` is required. If ``size`` is provided it must follow - lvm's size notation (like 1G, or 20M). Tags are an optional dictionary and is expected to + ``name_prefix`` is required. If ``size`` is provided its expected to be a + byte count. Tags are an optional dictionary and is expected to conform to the convention of prefixing them with "ceph." like:: {"ceph.block_device": "/dev/ceph/osd-1"} @@ -1129,9 +1120,12 @@ def create_lv(name_prefix, uuid, vg=None, device=None, extents=None, size=None, form the LV name :param vg: optional, pass an existing VG to create LV :param device: optional, device to use. Either device of vg must be passed - :param extends: optional, how many lvm extends to use - :param size: optional, LV size, must follow lvm's size notation, supersedes - extends + :param slots: optional, number of slots to divide vg up, LV will occupy one + one slot if enough space is available + :param extends: optional, how many lvm extends to use, supersedes slots + :param size: optional, target LV size in bytes, supersedes extents, + resulting LV might be smaller depending on extent + size of the underlying VG :param tags: optional, a dict of lvm tags to set on the LV """ name = '{}-{}'.format(name_prefix, uuid) @@ -1141,27 +1135,26 @@ def create_lv(name_prefix, uuid, vg=None, device=None, extents=None, size=None, # check if a vgs starting with ceph already exists vgs = get_device_vgs(device, 'ceph') if vgs: - vg = vgs[0].vg_name + vg = vgs[0] else: # create on if not - vg = create_vg(device, name_prefix='ceph').vg_name + vg = create_vg(device, name_prefix='ceph') assert(vg) if size: - command = [ - 'lvcreate', - '--yes', - '-L', - '{}'.format(size), - '-n', name, vg - ] - elif extents: + extents = vg.bytes_to_extents(size) + logger.debug('size was passed: {} -> {}'.format(size, extents)) + elif slots and not extents: + extents = vg.slots_to_extents(slots) + logger.debug('slots was passed: {} -> {}'.format(slots, extents)) + + if extents: command = [ 'lvcreate', '--yes', '-l', '{}'.format(extents), - '-n', name, vg + '-n', name, vg.vg_name ] # create the lv with all the space available, this is needed because the # system call is different for LVM @@ -1171,11 +1164,11 @@ def create_lv(name_prefix, uuid, vg=None, device=None, extents=None, size=None, '--yes', '-l', '100%FREE', - '-n', name, vg + '-n', name, vg.vg_name ] process.run(command) - lv = get_lv(lv_name=name, vg_name=vg) + lv = get_lv(lv_name=name, vg_name=vg.vg_name) if tags is None: tags = { diff --git a/src/ceph-volume/ceph_volume/devices/lvm/common.py b/src/ceph-volume/ceph_volume/devices/lvm/common.py index 1b4eebe74804..06fc33541a30 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/common.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/common.py @@ -56,6 +56,19 @@ def common_parser(prog, description): help='OSD data path. A physical device or logical volume', ) + required_group.add_argument( + '--data-size', + help='Size of data LV in case a device was passed in --data', + default=0, + ) + + required_group.add_argument( + '--data-slots', + help=('Intended number of slots on data device. The new OSD gets one' + 'of those slots or 1/nth of the available capacity'), + default=1, + ) + filestore_group.add_argument( '--filestore', action='store_true', @@ -67,6 +80,12 @@ def common_parser(prog, description): help='(REQUIRED) A logical volume (vg_name/lv_name), or path to a device', ) + filestore_group.add_argument( + '--journal-size', + help='Size of journal LV in case a raw block device was passed in --journal', + default=0, + ) + bluestore_group.add_argument( '--bluestore', action='store_true', @@ -79,12 +98,42 @@ def common_parser(prog, description): help='Path to bluestore block.db logical volume or device', ) + bluestore_group.add_argument( + '--block.db-size', + dest='block_db_size', + help='Size of block.db LV in case device was passed in --block.db', + default=0, + ) + + required_group.add_argument( + '--block.db-slots', + dest='block_db_slots', + help=('Intended number of slots on db device. The new OSD gets one' + 'of those slots or 1/nth of the available capacity'), + default=1, + ) + bluestore_group.add_argument( '--block.wal', dest='block_wal', help='Path to bluestore block.wal logical volume or device', ) + bluestore_group.add_argument( + '--block.wal-size', + dest='block_wal_size', + help='Size of block.wal LV in case device was passed in --block.wal', + default=0, + ) + + required_group.add_argument( + '--block.wal-slots', + dest='block_wal_slots', + help=('Intended number of slots on wal device. The new OSD gets one' + 'of those slots or 1/nth of the available capacity'), + default=1, + ) + parser.add_argument( '--osd-id', help='Reuse an existing OSD id', diff --git a/src/ceph-volume/ceph_volume/devices/lvm/prepare.py b/src/ceph-volume/ceph_volume/devices/lvm/prepare.py index 157dc8604e55..d46308df44fb 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/prepare.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/prepare.py @@ -150,7 +150,7 @@ class Prepare(object): return None return api.get_lv(lv_name=lv_name, vg_name=vg_name) - def setup_device(self, device_type, device_name, tags): + def setup_device(self, device_type, device_name, tags, size): """ Check if ``device`` is an lv, if so, set the tags, making sure to update the tags with the lv_uuid and lv_path which the incoming tags @@ -174,13 +174,18 @@ class Prepare(object): # We got a disk, create an lv lv_type = "osd-{}".format(device_type) uuid = system.generate_uuid() + tags['ceph.{}_uuid'.format(device_type)] = uuid + kwargs = { + 'device': device_name, + 'tags': tags, + } + if size != 0: + kwargs['size'] = disk.Size.parse(size) lv = api.create_lv( lv_type, uuid, - device=device_name, - tags={'ceph.type': device_type}) + **kwargs) path = lv.lv_path - tags['ceph.{}_uuid'.format(device_type)] = uuid tags['ceph.{}_device'.format(device_type)] = path lv.set_tags(tags) else: @@ -191,7 +196,7 @@ class Prepare(object): tags['ceph.%s_device' % device_type] = path return path, uuid, tags - def prepare_device(self, device, device_type, osd_uuid): + def prepare_data_device(self, device_type, osd_uuid): """ Check if ``arg`` is a device or partition to create an LV out of it with a distinct volume group name, assigning LV tags on it and @@ -202,14 +207,20 @@ class Prepare(object): :param device_type: Usually, either ``data`` or ``block`` (filestore vs. bluestore) :param osd_uuid: The OSD uuid """ + device = self.args.data if disk.is_partition(device) or disk.is_device(device): # we must create a vg, and then a single lv lv_name_prefix = "osd-{}".format(device_type) + kwargs = {'device': device, + 'tags': {'ceph.type': device_type}, + } + logger.debug('data device size: {}'.format(self.args.data_size)) + if self.args.data_size != 0: + kwargs['size'] = disk.Size.parse(self.args.data_size) return api.create_lv( lv_name_prefix, osd_uuid, - device=device, - tags={'ceph.type': device_type}) + **kwargs) else: error = [ 'Cannot use device ({}).'.format(device), @@ -285,7 +296,7 @@ class Prepare(object): data_lv = self.get_lv(self.args.data) if not data_lv: - data_lv = self.prepare_device(self.args.data, 'data', osd_fsid) + data_lv = self.prepare_data_device('data', osd_fsid) tags['ceph.data_device'] = data_lv.lv_path tags['ceph.data_uuid'] = data_lv.lv_uuid @@ -294,8 +305,7 @@ class Prepare(object): tags['ceph.vdo'] = api.is_vdo(data_lv.lv_path) journal_device, journal_uuid, tags = self.setup_device( - 'journal', self.args.journal, tags - ) + 'journal', self.args.journal, tags, self.args.journal_size) tags['ceph.type'] = 'data' data_lv.set_tags(tags) @@ -311,7 +321,7 @@ class Prepare(object): elif self.args.bluestore: block_lv = self.get_lv(self.args.data) if not block_lv: - block_lv = self.prepare_device(self.args.data, 'block', osd_fsid) + block_lv = self.prepare_data_device('block', osd_fsid) tags['ceph.block_device'] = block_lv.lv_path tags['ceph.block_uuid'] = block_lv.lv_uuid @@ -319,8 +329,10 @@ class Prepare(object): tags['ceph.encrypted'] = encrypted tags['ceph.vdo'] = api.is_vdo(block_lv.lv_path) - wal_device, wal_uuid, tags = self.setup_device('wal', self.args.block_wal, tags) - db_device, db_uuid, tags = self.setup_device('db', self.args.block_db, tags) + wal_device, wal_uuid, tags = self.setup_device( + 'wal', self.args.block_wal, tags, self.args.block_wal_size) + db_device, db_uuid, tags = self.setup_device( + 'db', self.args.block_db, tags, self.args.block_db_size) tags['ceph.type'] = 'block' block_lv.set_tags(tags) @@ -379,4 +391,4 @@ class Prepare(object): # cause both to be True if not self.args.bluestore and not self.args.filestore: self.args.bluestore = True - self.safe_prepare(self.args) + self.safe_prepare() diff --git a/src/ceph-volume/ceph_volume/tests/api/test_lvm.py b/src/ceph-volume/ceph_volume/tests/api/test_lvm.py index c2af9d4d197f..a3b83a9e6bd1 100644 --- a/src/ceph-volume/ceph_volume/tests/api/test_lvm.py +++ b/src/ceph-volume/ceph_volume/tests/api/test_lvm.py @@ -366,44 +366,9 @@ class TestVolumeGroups(object): class TestVolumeGroupFree(object): - def test_no_g_in_output(self): - vg = api.VolumeGroup(vg_name='nosize', vg_free='') - with pytest.raises(RuntimeError): - vg.free - - def test_g_without_size(self): - vg = api.VolumeGroup(vg_name='nosize', vg_free='g') - with pytest.raises(RuntimeError): - vg.free - - def test_size_without_g(self): - vg = api.VolumeGroup(vg_name='nosize', vg_free='1') - with pytest.raises(RuntimeError): - vg.free - - def test_error_message(self): - vg = api.VolumeGroup(vg_name='nosize', vg_free='F') - with pytest.raises(RuntimeError) as error: - vg.free - assert "Unable to convert vg size to integer: 'F'" in str(error.value) - - def test_invalid_float(self): - vg = api.VolumeGroup(vg_name='nosize', vg_free=' g') - with pytest.raises(RuntimeError) as error: - vg.free - assert "Unable to convert to integer: ' '" in str(error.value) - def test_integer_gets_produced(self): - vg = api.VolumeGroup(vg_name='nosize', vg_free='100g') - assert vg.free == 100 - - def test_integer_gets_produced_whitespace(self): - vg = api.VolumeGroup(vg_name='nosize', vg_free=' 100g ') - assert vg.free == 100 - - def test_integer_gets_rounded_down(self): - vg = api.VolumeGroup(vg_name='nosize', vg_free='100.99g') - assert vg.free == 100 + vg = api.VolumeGroup(vg_name='nosize', vg_free_count=100, vg_extent_size=4194304) + assert vg.free == 100 * 4194304 class TestCreateLVs(object): @@ -560,14 +525,17 @@ class TestCreateLV(object): def setup(self): self.foo_volume = api.Volume(lv_name='foo', lv_path='/path', vg_name='foo_group', lv_tags='') + self.foo_group = api.VolumeGroup(vg_name='foo_group', + vg_extent_size=4194304, + vg_free_count=100) @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') @patch('ceph_volume.api.lvm.get_lv') def test_uses_size(self, m_get_lv, m_call, m_run, monkeypatch): m_get_lv.return_value = self.foo_volume - api.create_lv('foo', 0, vg='foo_group', size='5G', tags={'ceph.type': 'data'}) - expected = ['lvcreate', '--yes', '-L', '5G', '-n', 'foo-0', 'foo_group'] + api.create_lv('foo', 0, vg=self.foo_group, size=5368709120, tags={'ceph.type': 'data'}) + expected = ['lvcreate', '--yes', '-l', '1280', '-n', 'foo-0', 'foo_group'] m_run.assert_called_with(expected) @patch('ceph_volume.api.lvm.process.run') @@ -575,16 +543,28 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.get_lv') def test_uses_extents(self, m_get_lv, m_call, m_run, monkeypatch): m_get_lv.return_value = self.foo_volume - api.create_lv('foo', 0, vg='foo_group', extents='50', tags={'ceph.type': 'data'}) + api.create_lv('foo', 0, vg=self.foo_group, extents='50', tags={'ceph.type': 'data'}) expected = ['lvcreate', '--yes', '-l', '50', '-n', 'foo-0', 'foo_group'] m_run.assert_called_with(expected) + @pytest.mark.parametrize("test_input,expected", + [(2, 50), + (3, 33),]) + @patch('ceph_volume.api.lvm.process.run') + @patch('ceph_volume.api.lvm.process.call') + @patch('ceph_volume.api.lvm.get_lv') + def test_uses_slots(self, m_get_lv, m_call, m_run, monkeypatch, test_input, expected): + m_get_lv.return_value = self.foo_volume + api.create_lv('foo', 0, vg=self.foo_group, slots=test_input, tags={'ceph.type': 'data'}) + expected = ['lvcreate', '--yes', '-l', str(expected), '-n', 'foo-0', 'foo_group'] + m_run.assert_called_with(expected) + @patch('ceph_volume.api.lvm.process.run') @patch('ceph_volume.api.lvm.process.call') @patch('ceph_volume.api.lvm.get_lv') def test_uses_all(self, m_get_lv, m_call, m_run, monkeypatch): m_get_lv.return_value = self.foo_volume - api.create_lv('foo', 0, vg='foo_group', tags={'ceph.type': 'data'}) + api.create_lv('foo', 0, vg=self.foo_group, tags={'ceph.type': 'data'}) expected = ['lvcreate', '--yes', '-l', '100%FREE', '-n', 'foo-0', 'foo_group'] m_run.assert_called_with(expected) @@ -594,7 +574,7 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.get_lv') def test_calls_to_set_tags_default(self, m_get_lv, m_set_tags, m_call, m_run, monkeypatch): m_get_lv.return_value = self.foo_volume - api.create_lv('foo', 0, vg='foo_group', size='5G') + api.create_lv('foo', 0, vg=self.foo_group) tags = { "ceph.osd_id": "null", "ceph.type": "null", @@ -609,7 +589,7 @@ class TestCreateLV(object): @patch('ceph_volume.api.lvm.get_lv') def test_calls_to_set_tags_arg(self, m_get_lv, m_set_tags, m_call, m_run, monkeypatch): m_get_lv.return_value = self.foo_volume - api.create_lv('foo', 0, vg='foo_group', size='5G', tags={'ceph.type': 'data'}) + api.create_lv('foo', 0, vg=self.foo_group, tags={'ceph.type': 'data'}) tags = { "ceph.type": "data", "ceph.data_device": "/path" @@ -774,7 +754,7 @@ class TestCreateVG(object): monkeypatch.setattr(api, 'get_vg', lambda **kw: True) api.create_vg(['/dev/sda', '/dev/sdb'], name='ceph') result = fake_run.calls[0]['args'][0] - expected = ['vgcreate', '-s', '1G', '--force', '--yes', 'ceph', '/dev/sda', '/dev/sdb'] + expected = ['vgcreate', '--force', '--yes', 'ceph', '/dev/sda', '/dev/sdb'] assert result == expected def test_name_prefix(self, monkeypatch, fake_run): diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py index 56985dbad269..b7a69e993a1e 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_prepare.py @@ -24,10 +24,12 @@ class TestLVM(object): class TestPrepareDevice(object): - def test_cannot_use_device(self): + def test_cannot_use_device(self, factory): + args = factory(data='/dev/var/foo') with pytest.raises(RuntimeError) as error: - lvm.prepare.Prepare([]).prepare_device( - '/dev/var/foo', 'data', '0') + p = lvm.prepare.Prepare([]) + p.args = args + p.prepare_data_device( 'data', '0') assert 'Cannot use device (/dev/var/foo)' in str(error.value) assert 'A vg/lv path or an existing device is needed' in str(error.value)