]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
ceph-volume: address review comments
authorJan Fajerski <jfajerski@suse.com>
Fri, 11 Sep 2020 08:36:43 +0000 (10:36 +0200)
committerJan Fajerski <jfajerski@suse.com>
Fri, 2 Oct 2020 07:47:45 +0000 (09:47 +0200)
Signed-off-by: Jan Fajerski <jfajerski@suse.com>
(cherry picked from commit 6f1592a1146529d352184c795aae8ce12f66e554)

doc/ceph-volume/lvm/batch.rst
src/ceph-volume/ceph_volume/devices/lvm/batch.py
src/ceph-volume/ceph_volume/devices/lvm/common.py
src/ceph-volume/ceph_volume/devices/lvm/prepare.py
src/ceph-volume/ceph_volume/tests/devices/lvm/test_common.py [new file with mode: 0644]

index 05c51b26dc5f7997c8b26cf4a011ecb3fad14ac5..78f7abf6fa420be4ba9076dbd4737aec8782258a 100644 (file)
@@ -25,35 +25,112 @@ supported, for example: specifying where journals should be placed.
 
 Automatic sorting of disks
 --------------------------
-If ``batch`` receives only a single list of data devices and the ``--no-auto`` option
-   is *not* passed (currently the default), ``ceph-volume`` will auto-sort disks by its rotational
-   property and use non-rotating disks for ``block.db`` or ``journal`` depending
-   on the objectstore used.
-This default behavior is now DEPRECATED and will be removed in future releases. Instead
-   an ``auto`` option is introduced to retain this behavior.
-It is recommended to make use of the explicit device lists for ``block.db``,
-   ``block.wal`` and ``journal``.
+If ``batch`` receives only a single list of data devices and other options are
+passed , ``ceph-volume`` will auto-sort disks by its rotational
+property and use non-rotating disks for ``block.db`` or ``journal`` depending
+on the objectstore used. If all devices are to be used for standalone OSDs,
+no matter if rotating or solid state, pass ``--no-auto``.
 For example assuming :term:`bluestore` is used and ``--no-auto`` is not passed,
-   the deprecated behavior would deploy the following, depending on the devices
-   passed:
+the deprecated behavior would deploy the following, depending on the devices
+passed:
 
 #. Devices are all spinning HDDs: 1 OSD is created per device
 #. Devices are all SSDs: 2 OSDs are created per device
 #. Devices are a mix of HDDs and SSDs: data is placed on the spinning device,
    the ``block.db`` is created on the SSD, as large as possible.
 
+.. note:: Although operations in ``ceph-volume lvm create`` allow usage of
+          ``block.wal`` it isn't supported with the ``auto`` behavior.
+
+This default auto-sorting behavior is now DEPRECATED and will be changed in future releases.
+Instead devices are not automatically sorted unless the ``--auto`` option is passed
+
+It is recommended to make use of the explicit device lists for ``block.db``,
+   ``block.wal`` and ``journal``.
+
+.. _ceph-volume-lvm-batch_bluestore:
+
+Reporting
+=========
+By default ``batch`` will print a report of the computed OSD layout and ask the
+user to confirm. This can be overridden by passing ``--yes``.
+
+If one wants to try out several invocations with being asked to deploy
+``--report`` can be passed. ``ceph-volume`` will exit after printing the report.
+
+Consider the following invocation::
+
+    $ ceph-volume lvm batch --report /dev/sdb /dev/sdc /dev/sdd --db-devices /dev/nvme0n1
+
+This will deploy three OSDs with external ``db`` and ``wal`` volumes on
+an NVME device.
+
+**pretty reporting**
+The ``pretty`` report format (the default) would
+look like this::
+
+    $ ceph-volume lvm batch --report /dev/sdb /dev/sdc /dev/sdd --db-devices /dev/nvme0n1
+    --> passed data devices: 3 physical, 0 LVM
+    --> relative data size: 1.0
+    --> passed block_db devices: 1 physical, 0 LVM
+
+    Total OSDs: 3
+
+      Type            Path                                                    LV Size         % of device
+    ----------------------------------------------------------------------------------------------------
+      data            /dev/sdb                                              300.00 GB         100.00%
+      block_db        /dev/nvme0n1                                           66.67 GB         33.33%
+    ----------------------------------------------------------------------------------------------------
+      data            /dev/sdc                                              300.00 GB         100.00%
+      block_db        /dev/nvme0n1                                           66.67 GB         33.33%
+    ----------------------------------------------------------------------------------------------------
+      data            /dev/sdd                                              300.00 GB         100.00%
+      block_db        /dev/nvme0n1                                           66.67 GB         33.33%
+
 
 .. note:: Although operations in ``ceph-volume lvm create`` allow usage of
           ``block.wal`` it isn't supported with the ``batch`` sub-command
 
 
-.. _ceph-volume-lvm-batch_filestore:
 
-``filestore``
--------------
-The :term:`filestore` objectstore can be used when creating multiple OSDs
-with the ``batch`` sub-command. It allows two different scenarios depending
-on the input of devices:
+**JSON reporting**
+Reporting can produce a structured output with ``--format json`` or
+``--format json-pretty``::
+
+    $ ceph-volume lvm batch --report --format json-pretty /dev/sdb /dev/sdc /dev/sdd --db-devices /dev/nvme0n1
+    --> passed data devices: 3 physical, 0 LVM
+    --> relative data size: 1.0
+    --> passed block_db devices: 1 physical, 0 LVM
+    [
+        {
+            "block_db": "/dev/nvme0n1",
+            "block_db_size": "66.67 GB",
+            "data": "/dev/sdb",
+            "data_size": "300.00 GB",
+            "encryption": "None"
+        },
+        {
+            "block_db": "/dev/nvme0n1",
+            "block_db_size": "66.67 GB",
+            "data": "/dev/sdc",
+            "data_size": "300.00 GB",
+            "encryption": "None"
+        },
+        {
+            "block_db": "/dev/nvme0n1",
+            "block_db_size": "66.67 GB",
+            "data": "/dev/sdd",
+            "data_size": "300.00 GB",
+            "encryption": "None"
+        }
+    ]
+
+Sizing
+======
+When no sizing arguments are passed, `ceph-volume` will derive the sizing from
+the passed device lists (or the sorted lists when using the automatic sorting).
+`ceph-volume batch` will attempt to fully utilize a device's available capacity.
+Relying on automatic sizing is recommended.
 
 #. Devices are all the same type (for example all spinning HDD or all SSDs):
    1 OSD is created per device, collocating the journal in the same HDD.
@@ -62,14 +139,14 @@ on the input of devices:
    ceph.conf and falling back to the default journal size of 5GB.
 
 
-When a mix of solid and spinning devices are used, ``ceph-volume`` will try to
-detect existing volume groups on the solid devices. If a VG is found, it will
-try to create the logical volume from there, otherwise raising an error if
-space is insufficient.
+* ``--block-db-slots``
+* ``--block-wal-slots``
+* ``--journal-slots``
 
-If a raw solid device is used along with a device that has a volume group in
-addition to some spinning devices, ``ceph-volume`` will try to extend the
-existing volume group and then create a logical volume.
+For example, consider an OSD host that is supposed to contain 5 data devices and
+one device for wal/db volumes. However, one data device is currently broken and
+is being replaced. Instead of calculating the explicit sizes for the wal/db
+volume, one can simply call::
 
 .. _ceph-volume-lvm-batch_report:
 
@@ -80,10 +157,9 @@ continue if the pre-computed output is acceptable. This output is useful to
 understand the outcome of the received devices. Once confirmation is accepted,
 the process continues.
 
-Although prompts are good to understand outcomes, it is incredibly useful to
-try different inputs to find the best product possible. With the ``--report``
-flag, one can prevent any actual operations and just verify outcomes from
-inputs.
+* ``--block-db-size``
+* ``--block-wal-size``
+* ``--journal-size``
 
 **pretty reporting**
 For two spinning devices, this is how the ``pretty`` report (the default) would
index 877fd58e674761748ea3d15a93539ec47a2eb659..7b8b7b65595ea3af56e05e5a4bfa612880c437b0 100644 (file)
@@ -117,7 +117,8 @@ def get_physical_fast_allocs(devices, type_, fast_slots_per_device, new_osds, ar
             continue
         # any LV present is considered a taken slot
         occupied_slots = len(dev.lvs)
-        # TODO this only looks at the first vg on device
+        # this only looks at the first vg on device, unsure if there is a better
+        # way
         dev_size = dev.vg_size[0]
         abs_size = disk.Size(b=int(dev_size / requested_slots))
         free_size = dev.vg_free[0]
@@ -289,8 +290,12 @@ class Batch(object):
             help='Provision slots on WAL device, can remain unoccupied'
         )
         def journal_size_in_mb_hack(size):
-            # give user time to adjust, then remove here
+            # TODO give user time to adjust, then remove this
             if size and size[-1].isdigit():
+                mlogger.warning('DEPRECATION NOTICE')
+                mlogger.warning('--journal-size as integer is parsed as megabytes')
+                mlogger.warning('A future release will parse integers as bytes')
+                mlogger.warning('Add a "M" to explicitly pass a megabyte size')
                 size += 'M'
             return disk.Size.parse(size)
         parser.add_argument(
index 2ad8562ae61d7dd40ac231e24cac7745ee9e5fff..06369e479d4c72a8731ef63c74f7d406bbec8dd8 100644 (file)
@@ -145,18 +145,10 @@ filestore_args = {
 
 def get_default_args():
     defaults = {}
-    defaults.update({name.strip('-').replace('-', '_').replace('.', '_'): val['default'] for name, val in common_args.items()
-                     if 'default' in val})
-    defaults.update({name.strip('-').replace('-', '_').replace('.', '_'): val['default'] for name, val in filestore_args.items()
-                     if 'default' in val})
-    defaults.update({name.strip('-').replace('-', '_').replace('.', '_'): val['default'] for name, val in bluestore_args.items()
-                     if 'default' in val})
-    defaults.update({name.strip('-').replace('-', '_').replace('.', '_'): None for name, val in common_args.items()
-                     if 'default' not in val})
-    defaults.update({name.strip('-').replace('-', '_').replace('.', '_'): None for name, val in filestore_args.items()
-                     if 'default' not in val})
-    defaults.update({name.strip('-').replace('-', '_').replace('.', '_'): None for name, val in bluestore_args.items()
-                     if 'default' not in val})
+    def format_name(name):
+        return name.strip('-').replace('-', '_').replace('.', '_')
+    for argset in (common_args, filestore_args, bluestore_args):
+        defaults.update({format_name(name): val.get('default', None) for name, val in argset.items()})
     return defaults
 
 
index 4f43bdcb39d93f231c63fde78e28a790abbc8372..9cb306fa9f228e43ba27cc1e3ff6f1e1d34d7da6 100644 (file)
@@ -299,7 +299,7 @@ class Prepare(object):
         if self.args.filestore:
             if not self.args.journal:
                 logger.info(('no journal was specifed, creating journal lv '
-                            'on {}').format(self.args.data))
+                             'on {}').format(self.args.data))
                 self.args.journal = self.args.data
                 self.args.journal_size = disk.Size(g=5)
                 # need to adjust data size/slots for colocated journal
@@ -308,9 +308,9 @@ class Prepare(object):
                 if self.args.data_slots == 1:
                     self.args.data_slots = 0
                 else:
-                    raise RuntimeError(('Can\'t handle multiple filestore OSDs '
+                    raise RuntimeError('Can\'t handle multiple filestore OSDs '
                                        'with colocated journals yet. Please '
-                                       'create journal LVs manually'))
+                                       'create journal LVs manually')
             tags['ceph.cephx_lockbox_secret'] = cephx_lockbox_secret
             tags['ceph.encrypted'] = encrypted
 
diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_common.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_common.py
new file mode 100644 (file)
index 0000000..c6b8a3d
--- /dev/null
@@ -0,0 +1,9 @@
+import pytest
+from ceph_volume.devices.lvm import common
+
+
+class TestCommon(object):
+
+    def test_get_default_args_smoke(self):
+        default_args = common.get_default_args()
+        assert default_args