diff --git a/config.yaml b/config.yaml index 1220334b..6fe04ffe 100644 --- a/config.yaml +++ b/config.yaml @@ -63,6 +63,15 @@ options: . For ceph >= 0.56.6 these can also be directories instead of devices - the charm assumes anything not starting with /dev is a directory instead. + bdev-enable-discard: + type: string + default: auto + description: | + Enables async discard on devices. This option will enable/disable both + bdev-enable-discard and bdev-async-discard options in ceph configuration + at the same time. The default value "auto" will try to autodetect and + should work in most cases. If you need to force a behaviour you can + set it to "enable" or "disable". Only applies for Ceph Mimic or later. osd-journal: type: string default: diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py index c6b925fd..187c6c56 100755 --- a/hooks/ceph_hooks.py +++ b/hooks/ceph_hooks.py @@ -80,6 +80,7 @@ from utils import ( get_cluster_addr, get_blacklist, get_journal_devices, + should_enable_discard, ) from charmhelpers.contrib.openstack.alternatives import install_alternative from charmhelpers.contrib.network.ip import ( @@ -388,6 +389,13 @@ def get_ceph_context(upgrading=False): 'bluestore_block_db_size': config('bluestore-block-db-size'), } + if config('bdev-enable-discard').lower() == 'enabled': + cephcontext['bdev_discard'] = True + elif config('bdev-enable-discard').lower() == 'auto': + cephcontext['bdev_discard'] = should_enable_discard(get_devices()) + else: + cephcontext['bdev_discard'] = False + if config('prefer-ipv6'): dynamic_ipv6_address = get_ipv6_addr()[0] if not public_network: diff --git a/hooks/utils.py b/hooks/utils.py index b773e2d1..3064f7a5 100644 --- a/hooks/utils.py +++ b/hooks/utils.py @@ -15,6 +15,7 @@ import re import os import socket +import subprocess from charmhelpers.core.hookenv import ( unit_get, @@ -23,6 +24,7 @@ from charmhelpers.core.hookenv import ( network_get_primary_address, log, DEBUG, + WARNING, status_set, storage_get, storage_list, @@ -194,3 +196,34 @@ def get_journal_devices(): _blacklist = get_blacklist() return set(device for device in devices if device not in _blacklist and os.path.exists(device)) + + +def should_enable_discard(devices): + """ + Tries to autodetect if we can enable discard on devices and if that + discard can be asynchronous. We want to enable both options if there's + any SSDs unless any of them are using SATA <= 3.0, in which case + discard is supported but is a blocking operation. + """ + discard_enable = True + for device in devices: + # whitelist some devices that do not need checking + if (device.startswith("/dev/nvme") or + device.startswith("/dev/vd")): + continue + if (device.startswith("/dev/") and + os.path.exists(device) and + is_sata30orless(device)): + discard_enable = False + log("SSD Discard autodetection: {} is forcing discard off" + "(sata <= 3.0)".format(device), level=WARNING) + return discard_enable + + +def is_sata30orless(device): + result = subprocess.check_output(["/usr/sbin/smartctl", "-i", device]) + print(result) + for line in str(result).split("\\n"): + if re.match("SATA Version is: *SATA (1\.|2\.|3\.0)", str(line)): + return True + return False diff --git a/lib/ceph/utils.py b/lib/ceph/utils.py index 83388c9b..dac98c99 100644 --- a/lib/ceph/utils.py +++ b/lib/ceph/utils.py @@ -82,7 +82,7 @@ QUORUM = [LEADER, PEON] PACKAGES = ['ceph', 'gdisk', 'btrfs-tools', 'radosgw', 'xfsprogs', - 'lvm2', 'parted'] + 'lvm2', 'parted', 'smartmontools'] CEPH_KEY_MANAGER = 'ceph' VAULT_KEY_MANAGER = 'vault' diff --git a/templates/ceph.conf b/templates/ceph.conf index 77fce613..2682be01 100644 --- a/templates/ceph.conf +++ b/templates/ceph.conf @@ -75,6 +75,8 @@ osd journal size = {{ osd_journal_size }} filestore xattr use omap = true journal dio = {{ dio }} {%- endif %} +bdev enable discard = {{ bdev_discard }} +bdev async discard = {{ bdev_discard }} {%- if short_object_len %} osd max object name len = 256 diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py index 23e9d851..b55b87ae 100644 --- a/unit_tests/test_ceph_hooks.py +++ b/unit_tests/test_ceph_hooks.py @@ -37,6 +37,8 @@ CHARM_CONFIG = {'config-flags': '', 'customize-failure-domain': False, 'bluestore': False, 'crush-initial-weight': '0', + 'bdev-enable-discard': 'enabled', + 'osd-devices': '/dev/vdb', 'bluestore': False, 'bluestore-block-wal-size': 0, 'bluestore-block-db-size': 0, @@ -84,6 +86,7 @@ class CephHooksTestCase(unittest.TestCase): 'short_object_len': True, 'upgrade_in_progress': False, 'use_syslog': 'true', + 'bdev_discard': True, 'bluestore': False, 'bluestore_experimental': False, 'bluestore_block_wal_size': 0, @@ -123,6 +126,7 @@ class CephHooksTestCase(unittest.TestCase): 'short_object_len': True, 'upgrade_in_progress': False, 'use_syslog': 'true', + 'bdev_discard': True, 'bluestore': False, 'bluestore_experimental': True, 'bluestore_block_wal_size': 0, @@ -168,6 +172,7 @@ class CephHooksTestCase(unittest.TestCase): 'short_object_len': True, 'upgrade_in_progress': False, 'use_syslog': 'true', + 'bdev_discard': True, 'bluestore': True, 'bluestore_experimental': False, 'bluestore_block_wal_size': BLUESTORE_WAL_TEST_SIZE, @@ -210,6 +215,7 @@ class CephHooksTestCase(unittest.TestCase): 'short_object_len': True, 'upgrade_in_progress': False, 'use_syslog': 'true', + 'bdev_discard': True, 'bluestore': True, 'bluestore_experimental': True, 'bluestore_block_wal_size': BLUESTORE_WAL_TEST_SIZE, @@ -250,6 +256,7 @@ class CephHooksTestCase(unittest.TestCase): 'short_object_len': True, 'upgrade_in_progress': False, 'use_syslog': 'true', + 'bdev_discard': True, 'bluestore': False, 'bluestore_experimental': False, 'bluestore_block_wal_size': 0, @@ -292,6 +299,7 @@ class CephHooksTestCase(unittest.TestCase): 'short_object_len': True, 'upgrade_in_progress': False, 'use_syslog': 'true', + 'bdev_discard': True, 'bluestore': False, 'bluestore_experimental': False, 'bluestore_block_wal_size': 0, diff --git a/unit_tests/test_ceph_utils.py b/unit_tests/test_ceph_utils.py index f58ae070..88dfefe4 100644 --- a/unit_tests/test_ceph_utils.py +++ b/unit_tests/test_ceph_utils.py @@ -61,3 +61,57 @@ class CephUtilsTestCase(unittest.TestCase): mock_os_path_exists.assert_called() mock_get_blacklist.assert_called() self.assertEqual(devices, set(['/dev/vdb'])) + + @patch('os.path.exists') + @patch.object(utils, 'is_sata30orless') + def test_should_enable_discard_yes(self, mock_is_sata30orless, + mock_os_path_exists): + devices = ['/dev/sda', '/dev/vda', '/dev/nvme0n1'] + mock_os_path_exists.return_value = True + mock_is_sata30orless.return_value = False + ret = utils.should_enable_discard(devices) + mock_os_path_exists.assert_called() + mock_is_sata30orless.assert_called() + self.assertEqual(ret, True) + + @patch('os.path.exists') + @patch.object(utils, 'is_sata30orless') + def test_should_enable_discard_no(self, mock_is_sata30orless, + mock_os_path_exists): + devices = ['/dev/sda', '/dev/vda', '/dev/nvme0n1'] + mock_os_path_exists.return_value = True + mock_is_sata30orless.return_value = True + ret = utils.should_enable_discard(devices) + mock_os_path_exists.assert_called() + mock_is_sata30orless.assert_called() + self.assertEqual(ret, False) + + @patch('subprocess.check_output') + def test_is_sata30orless_sata31(self, mock_subprocess_check_output): + extcmd_output = (b'supressed text\nSATA Version is: ' + b'SATA 3.1, 6.0 Gb/s (current: 6.0 Gb/s)\n' + b'supressed text\n\n') + mock_subprocess_check_output.return_value = extcmd_output + ret = utils.is_sata30orless('/dev/sda') + mock_subprocess_check_output.assert_called() + self.assertEqual(ret, False) + + @patch('subprocess.check_output') + def test_is_sata30orless_sata30(self, mock_subprocess_check_output): + extcmd_output = (b'supressed text\nSATA Version is: ' + b'SATA 3.0, 6.0 Gb/s (current: 6.0 Gb/s)\n' + b'supressed text\n\n') + mock_subprocess_check_output.return_value = extcmd_output + ret = utils.is_sata30orless('/dev/sda') + mock_subprocess_check_output.assert_called() + self.assertEqual(ret, True) + + @patch('subprocess.check_output') + def test_is_sata30orless_sata26(self, mock_subprocess_check_output): + extcmd_output = (b'supressed text\nSATA Version is: ' + b'SATA 2.6, 3.0 Gb/s (current: 3.0 Gb/s)\n' + b'supressed text\n\n') + mock_subprocess_check_output.return_value = extcmd_output + ret = utils.is_sata30orless('/dev/sda') + mock_subprocess_check_output.assert_called() + self.assertEqual(ret, True)