ignore devices that have already been processed

Similar to how osdize in charms.ceph checks for already processed
devices we need to avoid checking if they are pristine or not.

Additionally, mapped LUKS devices need to be filtered from being zapped
as they may hold valuable data. They are only used as underlying devices
for device mapper and dmcrypt to provide a decrypted block device
abstration so if they really need to be zapped a mapping needs to be
removed first.

This change also pulls charms.ceph modifications.

Change-Id: I96b3d40b3f9e56681be142377e454b15f9e22be3
Co-Authored-By: Dmitrii Shcherbakov <dmitrii.shcherbakov@canonical.com>
Co-Authored-By: Chris Procter <chris.procter@canonical.com>
Closes-Bug: 1781453

actions/zap_disk.py

@@ -28,6 +28,7 @@ from charmhelpers.contrib.storage.linux.utils import (
 )
 from charmhelpers.core.unitdata import kv
 from ceph.utils import is_active_bluestore_device
+from ceph.utils import is_mapped_luks_device
 
 
 def get_devices():
@@ -53,7 +54,9 @@ def zap():
     for device in devices:
         if not is_block_device(device):
             not_block_devices.append(device)
-        if is_device_mounted(device) or is_active_bluestore_device(device):
+        if (is_device_mounted(device) or
+                is_active_bluestore_device(device) or
+                is_mapped_luks_device(device)):
             failed_devices.append(device)
 
     if failed_devices or not_block_devices:

hooks/ceph_hooks.py

@@ -93,6 +93,8 @@ from charmhelpers.contrib.storage.linux.utils import (
 from charmhelpers.contrib.charmsupport import nrpe
 from charmhelpers.contrib.hardening.harden import harden
 
+from charmhelpers.core.unitdata import kv
+
 import charmhelpers.contrib.openstack.vaultlocker as vaultlocker
 
 hooks = Hooks()
@@ -426,6 +428,15 @@ def prepare_disks_and_activate():
 
     # pre-flight check of eligible device pristinity
     devices = get_devices()
+
+    # if a device has been previously touched we need to consider it as
+    # non-pristine. If it needs to be re-processed it has to be zapped
+    # via the respective action which also clears the unitdata entry.
+    db = kv()
+    touched_devices = db.get('osd-devices', [])
+    devices = [dev for dev in devices if dev not in touched_devices]
+    log('Skipping osd devices previously processed by this unit: {}'
+        .format(touched_devices))
     # filter osd-devices that are file system paths
     devices = [dev for dev in devices if dev.startswith('/dev')]
     # filter osd-devices that does not exist on this unit
@@ -435,6 +446,7 @@ def prepare_disks_and_activate():
     # filter osd-devices that are active bluestore devices
     devices = [dev for dev in devices
                if not ceph.is_active_bluestore_device(dev)]
+
     log('Checking for pristine devices: "{}"'.format(devices), level=DEBUG)
     if not all(ceph.is_pristine_disk(dev) for dev in devices):
         status_set('blocked',

lib/ceph/broker.py

@@ -81,6 +81,10 @@ POOL_KEYS = {
     "cache_min_flush_age": [int],
     "cache_min_evict_age": [int],
     "fast_read": [bool],
+    "allow_ec_overwrites": [bool],
+    "compression_mode": [str, ["none", "passive", "aggressive", "force"]],
+    "compression_algorithm": [str, ["lz4", "snappy", "zlib", "zstd"]],
+    "compression_required_ratio": [float, [0.0, 1.0]],
 }
 
 CEPH_BUCKET_TYPES = [
@@ -251,7 +255,8 @@ def pool_permission_list_for_service(service):
         for prefix in prefixes:
             permissions.append("allow {} object_prefix {}".format(permission,
                                                                   prefix))
-    return ["mon", "allow r", "osd", ', '.join(permissions)]
+    return ['mon', 'allow r, allow command "osd blacklist"',
+            'osd', ', '.join(permissions)]
 
 
 def get_service_groups(service, namespace=None):

lib/ceph/utils.py

@@ -1096,7 +1096,8 @@ def get_mds_bootstrap_key():
 
 
 _default_caps = collections.OrderedDict([
-    ('mon', ['allow r']),
+    ('mon', ['allow r',
+             'allow command "osd blacklist"']),
     ('osd', ['allow rwx']),
 ])
 
@@ -1163,6 +1164,7 @@ def get_named_key(name, caps=None, pool_list=None):
     :param caps: dict of cephx capabilities
     :returns: Returns a cephx key
     """
+    key_name = 'client.{}'.format(name)
     try:
         # Does the key already exist?
         output = str(subprocess.check_output(
@@ -1177,8 +1179,14 @@ def get_named_key(name, caps=None, pool_list=None):
                 ),
                 'auth',
                 'get',
-                'client.{}'.format(name),
+                key_name,
             ]).decode('UTF-8')).strip()
+        # NOTE(jamespage);
+        # Apply any changes to key capabilities, dealing with
+        # upgrades which requires new caps for operation.
+        upgrade_key_caps(key_name,
+                         caps or _default_caps,
+                         pool_list)
         return parse_key(output)
     except subprocess.CalledProcessError:
         # Couldn't get the key, time to create it!
@@ -1194,7 +1202,7 @@ def get_named_key(name, caps=None, pool_list=None):
         '/var/lib/ceph/mon/ceph-{}/keyring'.format(
             socket.gethostname()
         ),
-        'auth', 'get-or-create', 'client.{}'.format(name),
+        'auth', 'get-or-create', key_name,
     ]
     # Add capabilities
     for subsystem, subcaps in caps.items():
@@ -1213,7 +1221,7 @@ def get_named_key(name, caps=None, pool_list=None):
                      .strip())  # IGNORE:E1103
 
 
-def upgrade_key_caps(key, caps):
+def upgrade_key_caps(key, caps, pool_list=None):
     """ Upgrade key to have capabilities caps """
     if not is_leader():
         # Not the MON leader OR not clustered
@@ -1222,6 +1230,12 @@ def upgrade_key_caps(key, caps):
         "sudo", "-u", ceph_user(), 'ceph', 'auth', 'caps', key
     ]
     for subsystem, subcaps in caps.items():
+        if subsystem == 'osd':
+            if pool_list:
+                # This will output a string similar to:
+                # "pool=rgw pool=rbd pool=something"
+                pools = " ".join(['pool={0}'.format(i) for i in pool_list])
+                subcaps[0] = subcaps[0] + " " + pools
         cmd.extend([subsystem, '; '.join(subcaps)])
     subprocess.check_call(cmd)
 
@@ -1453,6 +1467,11 @@ def osdize_dev(dev, osd_format, osd_journal, ignore_errors=False,
             ' skipping.'.format(dev))
         return
 
+    if is_mapped_luks_device(dev):
+        log('{} is a mapped LUKS device,'
+            ' skipping.'.format(dev))
+        return
+
     if cmp_pkgrevno('ceph', '12.2.4') >= 0:
         cmd = _ceph_volume(dev,
                            osd_journal,
@@ -1664,6 +1683,29 @@ def is_active_bluestore_device(dev):
     return False
 
 
+def is_luks_device(dev):
+    """
+    Determine if dev is a LUKS-formatted block device.
+
+    :param: dev: A full path to a block device to check for LUKS header
+    presence
+    :returns: boolean: indicates whether a device is used based on LUKS header.
+    """
+    return True if _luks_uuid(dev) else False
+
+
+def is_mapped_luks_device(dev):
+    """
+    Determine if dev is a mapped LUKS device
+    :param: dev: A full path to a block device to be checked
+    :returns: boolean: indicates whether a device is mapped
+    """
+    _, dirs, _ = next(os.walk('/sys/class/block/{}/holders/'
+                              .format(os.path.basename(dev))))
+    is_held = len(dirs) > 0
+    return is_held and is_luks_device(dev)
+
+
 def get_conf(variable):
     """
     Get the value of the given configuration variable from the

unit_tests/test_actions_zap_disk.py

@@ -26,10 +26,12 @@ class ZapDiskActionTests(CharmTestCase):
                        'is_block_device',
                        'is_device_mounted',
                        'is_active_bluestore_device',
+                       'is_mapped_luks_device',
                        'kv'])
         self.is_device_mounted.return_value = False
         self.is_block_device.return_value = True
         self.is_active_bluestore_device.return_value = False
+        self.is_mapped_luks_device.return_value = False
         self.kv.return_value = self.kv
         self.hookenv.local_unit.return_value = "ceph-osd-test/0"
 
@@ -127,3 +129,44 @@ class ZapDiskActionTests(CharmTestCase):
         _zap_disk.assert_not_called()
         self.hookenv.action_fail.assert_called_with(
             "1 devices are mounted: /dev/vdb")
+
+    @mock.patch.object(zap_disk, 'zap_disk')
+    def test_wont_zap__mapped_luks_device(self, _zap_disk):
+        """Will not zap a disk that has a LUKS header"""
+        def side_effect(arg):
+            return {
+                'devices': '/dev/vdb',
+                'i-really-mean-it': True,
+            }.get(arg)
+        self.hookenv.action_get.side_effect = side_effect
+        self.is_active_bluestore_device.return_value = False
+        self.is_mapped_luks_device.return_value = True
+        zap_disk.zap()
+        _zap_disk.assert_not_called()
+        self.hookenv.action_fail.assert_called_with(
+            "1 devices are mounted: /dev/vdb")
+
+    @mock.patch.object(zap_disk, 'zap_disk')
+    def test_zap_luks_not_mapped(self, _zap_disk):
+        """Will zap disk with extra config set"""
+        def side_effect(arg):
+            return {
+                'devices': '/dev/vdb',
+                'i-really-mean-it': True,
+            }.get(arg)
+
+        self.is_active_bluestore_device.return_value = False
+        self.is_mapped_luks_device.return_value = False
+
+        self.hookenv.action_get.side_effect = side_effect
+        self.kv.get.return_value = ['/dev/vdb', '/dev/vdz']
+        zap_disk.zap()
+        _zap_disk.assert_called_with('/dev/vdb')
+        self.kv.get.assert_called_with('osd-devices', [])
+        self.kv.set.assert_called_with('osd-devices', ['/dev/vdz'])
+        self.hookenv.action_set.assert_called_with({
+            'message': "1 disk(s) have been zapped, to use "
+                       "them as OSDs, run: \njuju "
+                       "run-action ceph-osd-test/0 add-disk "
+                       "osd-devices=\"/dev/vdb\""
+        })