diff --git a/hooks/swift_storage_hooks.py b/hooks/swift_storage_hooks.py index c6463ef4..b1612b63 100755 --- a/hooks/swift_storage_hooks.py +++ b/hooks/swift_storage_hooks.py @@ -16,8 +16,10 @@ setup_storage, assert_charm_supports_ipv6, setup_rsync, + remember_devices, REQUIRED_INTERFACES, assess_status, + ensure_devs_tracked, ) from lib.misc_utils import pause_aware_restart_on_change @@ -27,6 +29,7 @@ config, log, relation_get, + relation_ids, relation_set, relations_of_type, status_set, @@ -85,6 +88,12 @@ def config_changed(): openstack_upgrade_available('swift'): status_set('maintenance', 'Running openstack upgrade') do_openstack_upgrade(configs=CONFIGS) + + setup_storage() + + for rid in relation_ids('swift-storage'): + swift_storage_relation_joined(rid=rid) + CONFIGS.write_all() save_script_rc() @@ -96,23 +105,28 @@ def config_changed(): def upgrade_charm(): apt_install(filter_installed_packages(PACKAGES), fatal=True) update_nrpe_config() + ensure_devs_tracked() @hooks.hook() -def swift_storage_relation_joined(): - devs = [os.path.basename(dev) for dev in determine_block_devices()] +def swift_storage_relation_joined(rid=None): rel_settings = { 'zone': config('zone'), 'object_port': config('object-server-port'), 'container_port': config('container-server-port'), 'account_port': config('account-server-port'), - 'device': ':'.join(devs), } + devs = determine_block_devices() or [] + devs = [os.path.basename(d) for d in devs] + rel_settings['device'] = ':'.join(devs) + # Keep a reference of devices we are adding to the ring + remember_devices(devs) + if config('prefer-ipv6'): rel_settings['private-address'] = get_ipv6_addr()[0] - relation_set(**rel_settings) + relation_set(relation_id=rid, **rel_settings) @hooks.hook('swift-storage-relation-changed') diff --git a/lib/swift_storage_utils.py b/lib/swift_storage_utils.py index 73c31f6d..b0c6aac8 100644 --- a/lib/swift_storage_utils.py +++ b/lib/swift_storage_utils.py @@ -1,5 +1,7 @@ +import json import os import re +import subprocess import shutil import tempfile @@ -24,6 +26,10 @@ apt_update ) +from charmhelpers.core.unitdata import ( + Storage as KVStore, +) + from charmhelpers.core.host import ( mkdir, mount, @@ -37,8 +43,12 @@ log, DEBUG, INFO, + WARNING, ERROR, unit_private_ip, + local_unit, + relation_get, + relation_ids, ) from charmhelpers.contrib.storage.linux.utils import ( @@ -106,6 +116,16 @@ SWIFT_CONF_DIR = '/etc/swift' SWIFT_RING_EXT = 'ring.gz' +# NOTE(hopem): we intentionally place this database outside of unit context so +# that if the unit, service or even entire environment is +# destroyed, there will still be a record of what devices were in +# use so that when the swift charm next executes, already used +# devices will not be unintentionally reformatted. If devices are +# to be recycled, they will need to be manually removed from this +# database. +# FIXME: add charm support for removing devices (see LP: #1448190) +KV_DB_PATH = '/var/lib/juju/swift_storage/charm_kvdata.db' + def ensure_swift_directories(): ''' @@ -226,9 +246,8 @@ def guess_block_devices(): def determine_block_devices(): block_device = config('block-device') - - if not block_device or block_device in ['None', 'none']: - log('No storage devices specified in config as block-device', + if not block_device or block_device.lower() == 'none': + log("No storage devices specified in 'block_device' config", level=ERROR) return None @@ -238,31 +257,192 @@ def determine_block_devices(): bdevs = block_device.split(' ') # attempt to ensure block devices, but filter out missing devs - _none = ['None', 'none', None] + _none = ['None', 'none'] valid_bdevs = \ - [x for x in map(ensure_block_device, bdevs) if x not in _none] + [x for x in map(ensure_block_device, bdevs) if str(x).lower() not in + _none] log('Valid ensured block devices: %s' % valid_bdevs) return valid_bdevs -def mkfs_xfs(bdev): - cmd = ['mkfs.xfs', '-f', '-i', 'size=1024', bdev] +def mkfs_xfs(bdev, force=False): + """Format device with XFS filesystem. + + By default this should fail if the device already has a filesystem on it. + """ + cmd = ['mkfs.xfs'] + if force: + cmd.append("-f") + + cmd += ['-i', 'size=1024', bdev] check_call(cmd) +def devstore_safe_load(devstore): + """Attempt to decode json data and return None if an error occurs while + also printing a log. + """ + if not devstore: + return None + + try: + return json.loads(devstore) + except ValueError: + log("Unable to decode JSON devstore", level=DEBUG) + + return None + + +def is_device_in_ring(dev, skip_rel_check=False, ignore_deactivated=True): + """Check if device has been added to the ring. + + First check local KV store then check storage rel with proxy. + """ + d = os.path.dirname(KV_DB_PATH) + if not os.path.isdir(d): + mkdir(d) + log("Device '%s' does not appear to be in use by Swift" % (dev), + level=INFO) + return False + + # First check local KV store + kvstore = KVStore(KV_DB_PATH) + devstore = devstore_safe_load(kvstore.get(key='devices')) + kvstore.close() + deactivated = [] + if devstore: + blk_uuid = get_device_blkid("/dev/%s" % (dev)) + env_uuid = os.environ['JUJU_ENV_UUID'] + masterkey = "%s@%s" % (dev, env_uuid) + if (masterkey in devstore and + devstore[masterkey].get('blkid') == blk_uuid and + devstore[masterkey].get('status') == 'active'): + log("Device '%s' appears to be in use by Swift (found in local " + "devstore)" % (dev), level=INFO) + return True + + for key, val in devstore.iteritems(): + if key != masterkey and val.get('blkid') == blk_uuid: + log("Device '%s' appears to be in use by Swift (found in " + "local devstore) but has a different JUJU_ENV_UUID " + "(current=%s, expected=%s). " + "This could indicate that the device was added as part of " + "a previous deployment and will require manual removal or " + "updating if it needs to be reformatted." + % (dev, key, masterkey), level=INFO) + return True + + if ignore_deactivated: + deactivated = [k == masterkey and v.get('blkid') == blk_uuid and + v.get('status') != 'active' + for k, v in devstore.iteritems()] + + if skip_rel_check: + log("Device '%s' does not appear to be in use by swift (searched " + "local devstore only)" % (dev), level=INFO) + return False + + # Then check swift-storage relation with proxy + for rid in relation_ids('swift-storage'): + devstore = relation_get(attribute='device', rid=rid, unit=local_unit()) + if devstore and dev in devstore.split(':'): + if not ignore_deactivated or dev not in deactivated: + log("Device '%s' appears to be in use by swift (found on " + "proxy relation) but was not found in local devstore so " + "will be added to the cache" % (dev), level=INFO) + remember_devices([dev]) + return True + + log("Device '%s' does not appear to be in use by swift (searched local " + "devstore and proxy relation)" % (dev), level=INFO) + return False + + +def get_device_blkid(dev): + blk_uuid = subprocess.check_output(['blkid', '-s', 'UUID', dev]) + blk_uuid = re.match(r'^%s:\s+UUID="(.+)"$' % (dev), blk_uuid.strip()) + if blk_uuid: + return blk_uuid.group(1) + else: + log("Failed to obtain device UUID for device '%s' - returning None" % + dev, level=WARNING) + + return None + + +def remember_devices(devs): + """Add device to local store of ringed devices.""" + d = os.path.dirname(KV_DB_PATH) + if not os.path.isdir(d): + mkdir(d) + + kvstore = KVStore(KV_DB_PATH) + devstore = devstore_safe_load(kvstore.get(key='devices')) or {} + env_uuid = os.environ['JUJU_ENV_UUID'] + for dev in devs: + blk_uuid = get_device_blkid("/dev/%s" % (dev)) + key = "%s@%s" % (dev, env_uuid) + if key in devstore and devstore[key].get('blkid') == blk_uuid: + log("Device '%s' already in devstore (status:%s)" % + (dev, devstore[key].get('status')), level=DEBUG) + else: + existing = [(k, v) for k, v in devstore.iteritems() + if v.get('blkid') == blk_uuid and + re.match("^(.+)@(.+)$", k).group(1) == dev] + if existing: + log("Device '%s' already in devstore but has a different " + "JUJU_ENV_UUID (%s)" % + (dev, re.match(".+@(.+)$", existing[0][0]).group(1)), + level=WARNING) + else: + log("Adding device '%s' with blkid='%s' to devstore" % + (blk_uuid, dev), + level=DEBUG) + devstore[key] = {'blkid': blk_uuid, 'status': 'active'} + + if devstore: + kvstore.set(key='devices', value=json.dumps(devstore)) + + kvstore.flush() + kvstore.close() + + +def ensure_devs_tracked(): + for rid in relation_ids('swift-storage'): + devs = relation_get(attribute='device', rid=rid, unit=local_unit()) + if devs: + for dev in devs.split(':'): + # this will migrate if not already in the local store + is_device_in_ring(dev, skip_rel_check=True) + + def setup_storage(): # Ensure /srv/node exists just in case no disks # are detected and used. mkdir(os.path.join('/srv', 'node'), owner='swift', group='swift', perms=0o755) + reformat = str(config('overwrite')).lower() == "true" for dev in determine_block_devices(): - if config('overwrite') in ['True', 'true']: + if is_device_in_ring(os.path.basename(dev)): + log("Device '%s' already in the ring - ignoring" % (dev)) + continue + + if reformat: clean_storage(dev) - # if not cleaned and in use, mkfs should fail. - mkfs_xfs(dev) - _dev = os.path.basename(dev) - _mp = os.path.join('/srv', 'node', _dev) + + try: + # If not cleaned and in use, mkfs should fail. + mkfs_xfs(dev, force=reformat) + except subprocess.CalledProcessError as exc: + # This is expected is a formatted device is provided and we are + # forcing the format. + log("Format device '%s' failed (%s) - continuing to next device" % + (dev, exc), level=WARNING) + continue + + basename = os.path.basename(dev) + _mp = os.path.join('/srv', 'node', basename) mkdir(_mp, owner='swift', group='swift') options = None @@ -272,7 +452,7 @@ def setup_storage(): dev = loopback_device options = "loop, defaults" - mountpoint = '/srv/node/%s' % _dev + mountpoint = '/srv/node/%s' % basename filesystem = "xfs" mount(dev, mountpoint, filesystem=filesystem) diff --git a/unit_tests/test_swift_storage_relations.py b/unit_tests/test_swift_storage_relations.py index c19e45aa..7206e9af 100644 --- a/unit_tests/test_swift_storage_relations.py +++ b/unit_tests/test_swift_storage_relations.py @@ -1,4 +1,7 @@ from mock import patch +import os +import json +import uuid from test_utils import CharmTestCase, patch_open @@ -15,6 +18,7 @@ 'config', 'log', 'relation_set', + 'relation_ids', 'relation_get', 'relations_of_type', # charmhelpers.core.host @@ -111,42 +115,170 @@ def test_config_changed_ipv6(self, mock_assert_charm_supports_ipv6): self.assertTrue(self.CONFIGS.write_all.called) self.assertTrue(self.setup_rsync.called) - def test_upgrade_charm(self): + @patch.object(hooks, 'ensure_devs_tracked') + def test_upgrade_charm(self, mock_ensure_devs_tracked): self.filter_installed_packages.return_value = [ 'python-psutil'] hooks.upgrade_charm() self.apt_install.assert_called_with([ 'python-psutil'], fatal=True) self.assertTrue(self.update_nrpe_config.called) + self.assertTrue(mock_ensure_devs_tracked.called) + + @patch('hooks.lib.swift_storage_utils.get_device_blkid', + lambda dev: str(uuid.uuid4())) + @patch.object(hooks.os, 'environ') + @patch('hooks.lib.swift_storage_utils.os.path.isdir', lambda *args: True) + @patch.object(hooks, 'relation_set') + @patch('hooks.lib.swift_storage_utils.local_unit') + @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: []) + @patch('hooks.lib.swift_storage_utils.KVStore') + @patch.object(uuid, 'uuid4', lambda: 'a-test-uuid') + def test_storage_joined_single_device(self, mock_kvstore, mock_local_unit, + mock_rel_set, mock_environ): + mock_environ.get.side_effect = {'JUJU_ENV_UUID': uuid.uuid4()} + mock_local_unit.return_value = 'test/0' + kvstore = mock_kvstore.return_value + kvstore.__enter__.return_value = kvstore + kvstore.get.return_value = None + self.determine_block_devices.return_value = ['/dev/vdb'] - def test_storage_joined_single_device(self): - self.determine_block_devices.return_value = [ - '/dev/vdb'] hooks.swift_storage_relation_joined() - self.relation_set.assert_called_with( + + mock_rel_set.assert_called_with( + relation_id=None, device='vdb', object_port=6000, account_port=6002, zone=1, container_port=6001 ) - def test_storage_joined_ipv6(self): + kvstore.get.return_value = None + rel_settings = {} + + def fake_kv_set(key, value): + rel_settings[key] = value + + kvstore.set.side_effect = fake_kv_set + + def fake_kv_get(key): + return rel_settings.get(key) + + kvstore.get.side_effect = fake_kv_get + devices = {"vdb@%s" % (mock_environ['JUJU_ENV_UUID']): + {"status": "active", + "blkid": 'a-test-uuid'}} + kvstore.set.assert_called_with(key='devices', + value=json.dumps(devices)) + + @patch('hooks.lib.swift_storage_utils.get_device_blkid', + lambda dev: '%s-blkid-uuid' % os.path.basename(dev)) + @patch.object(hooks.os, 'environ') + @patch('hooks.lib.swift_storage_utils.os.path.isdir', lambda *args: True) + @patch.object(hooks, 'relation_set') + @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: []) + @patch('hooks.lib.swift_storage_utils.KVStore') + @patch.object(uuid, 'uuid4', lambda: 'a-test-uuid') + def test_storage_joined_ipv6(self, mock_kvstore, mock_rel_set, + mock_environ): + kvstore = mock_kvstore.return_value + kvstore.__enter__.return_value = kvstore + kvstore.get.return_value = None + self.determine_block_devices.return_value = ['/dev/vdb'] self.test_config.set('prefer-ipv6', True) self.get_ipv6_addr.return_value = ['2001:db8:1::1'] + hooks.swift_storage_relation_joined() args = { + 'relation_id': None, 'device': 'vdb', 'object_port': 6000, 'account_port': 6002, 'zone': 1, 'container_port': 6001, 'private-address': '2001:db8:1::1', } - self.relation_set.assert_called_with(**args) + mock_rel_set.assert_called_with(**args) + kvstore.get.assert_called_with(key='devices') + + @patch('hooks.lib.swift_storage_utils.get_device_blkid', + lambda dev: '%s-blkid-uuid' % os.path.basename(dev)) + @patch.object(hooks.os, 'environ') + @patch('hooks.lib.swift_storage_utils.os.path.isdir', lambda *args: True) + @patch('hooks.lib.swift_storage_utils.local_unit') + @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: []) + @patch('hooks.lib.swift_storage_utils.KVStore') + @patch.object(uuid, 'uuid4', lambda: 'a-test-uuid') + def test_storage_joined_multi_device(self, mock_kvstore, mock_local_unit, + mock_environ): + mock_environ.get.side_effect = {'JUJU_ENV_UUID': uuid.uuid4()} + self.determine_block_devices.return_value = ['/dev/vdb', '/dev/vdc', + '/dev/vdd'] + mock_local_unit.return_value = 'test/0' + kvstore = mock_kvstore.return_value + kvstore.__enter__.return_value = kvstore + kvstore.get.return_value = None + rel_settings = {} - def test_storage_joined_multi_device(self): + def fake_kv_set(key, value): + rel_settings[key] = value + + kvstore.set.side_effect = fake_kv_set + + def fake_kv_get(key): + return rel_settings.get(key) + + kvstore.get.side_effect = fake_kv_get + + hooks.swift_storage_relation_joined() + env_uuid = mock_environ['JUJU_ENV_UUID'] + devices = {"vdb@%s" % (env_uuid): {"status": "active", + "blkid": 'vdb-blkid-uuid'}, + "vdd@%s" % (env_uuid): {"status": "active", + "blkid": 'vdd-blkid-uuid'}, + "vdc@%s" % (env_uuid): {"status": "active", + "blkid": 'vdc-blkid-uuid'}} + kvstore.set.assert_called_with( + key='devices', value=json.dumps(devices) + ) + + @patch('hooks.lib.swift_storage_utils.get_device_blkid', + lambda dev: '%s-blkid-uuid' % os.path.basename(dev)) + @patch.object(hooks.os, 'environ') + @patch('hooks.lib.swift_storage_utils.os.path.isdir', lambda *args: True) + @patch('hooks.lib.swift_storage_utils.local_unit') + @patch('hooks.lib.swift_storage_utils.relation_ids', lambda *args: []) + @patch('hooks.lib.swift_storage_utils.KVStore') + def test_storage_joined_dev_exists_unknown_juju_env_uuid(self, + mock_kvstore, + mock_local_unit, + mock_environ): + mock_environ.get.return_value = {'JUJU_ENV_UUID': uuid.uuid4()} self.determine_block_devices.return_value = ['/dev/vdb', '/dev/vdc', '/dev/vdd'] + mock_local_unit.return_value = 'test/0' + kvstore = mock_kvstore.return_value + kvstore.__enter__.return_value = kvstore + kvstore.get.return_value = None + store = {'vdb@%s' % (uuid.uuid4()): {"status": "active", + "blkid": 'vdb-blkid-uuid'}} + + def fake_kv_set(key, value): + store[key] = value + + kvstore.set.side_effect = fake_kv_set + + def fake_kv_get(key): + return store.get(key) + + kvstore.get.side_effect = fake_kv_get + hooks.swift_storage_relation_joined() - self.relation_set.assert_called_with( - device='vdb:vdc:vdd', object_port=6000, account_port=6002, - zone=1, container_port=6001 + env_uuid = mock_environ['JUJU_ENV_UUID'] + devices = {"vdb@%s" % (env_uuid): {"status": "active", + "blkid": 'vdb-blkid-uuid'}, + "vdd@%s" % (env_uuid): {"status": "active", + "blkid": 'vdd-blkid-uuid'}, + "vdc@%s" % (env_uuid): {"status": "active", + "blkid": 'vdc-blkid-uuid'}} + kvstore.set.assert_called_with( + key='devices', value=json.dumps(devices) ) @patch('sys.exit') diff --git a/unit_tests/test_swift_storage_utils.py b/unit_tests/test_swift_storage_utils.py index d86f80f8..8e482035 100644 --- a/unit_tests/test_swift_storage_utils.py +++ b/unit_tests/test_swift_storage_utils.py @@ -201,16 +201,24 @@ def _findmnt(cmd): def test_mkfs_xfs(self): swift_utils.mkfs_xfs('/dev/sdb') + self.check_call.assert_called_with( + ['mkfs.xfs', '-i', 'size=1024', '/dev/sdb'] + ) + + def test_mkfs_xfs_force(self): + swift_utils.mkfs_xfs('/dev/sdb', force=True) self.check_call.assert_called_with( ['mkfs.xfs', '-f', '-i', 'size=1024', '/dev/sdb'] ) + @patch.object(swift_utils, 'is_device_in_ring') @patch.object(swift_utils, 'clean_storage') @patch.object(swift_utils, 'mkfs_xfs') @patch.object(swift_utils, 'determine_block_devices') - def test_setup_storage_no_overwrite(self, determine, mkfs, clean): + def test_setup_storage_no_overwrite(self, determine, mkfs, clean, + mock_is_device_in_ring): + mock_is_device_in_ring.return_value = False determine.return_value = ['/dev/vdb'] - self.test_config.set('overwrite', 'false') swift_utils.setup_storage() self.assertFalse(clean.called) calls = [call(['chown', '-R', 'swift:swift', '/srv/node/']), @@ -222,15 +230,20 @@ def test_setup_storage_no_overwrite(self, determine, mkfs, clean): call('/srv/node/vdb', group='swift', owner='swift') ]) + @patch.object(swift_utils, 'is_device_in_ring') @patch.object(swift_utils, 'clean_storage') @patch.object(swift_utils, 'mkfs_xfs') @patch.object(swift_utils, 'determine_block_devices') - def test_setup_storage_overwrite(self, determine, mkfs, clean): + def test_setup_storage_overwrite(self, determine, mkfs, clean, + mock_is_device_in_ring): + self.test_config.set('overwrite', True) + mock_is_device_in_ring.return_value = False self.is_mapped_loopback_device.return_value = None determine.return_value = ['/dev/vdb'] - self.test_config.set('overwrite', 'True') swift_utils.setup_storage() clean.assert_called_with('/dev/vdb') + self.mkdir.assert_called_with('/srv/node/vdb', owner='swift', + group='swift') self.mount.assert_called_with('/dev/vdb', '/srv/node/vdb', filesystem='xfs') self.fstab_add.assert_called_with('/dev/vdb', '/srv/node/vdb', @@ -359,9 +372,11 @@ def test_do_upgrade(self): for service in services: self.assertIn(call(service), self.service_restart.call_args_list) + @patch.object(swift_utils, "is_device_in_ring") @patch.object(swift_utils, "mkfs_xfs") @patch.object(swift_utils, "determine_block_devices") - def test_setup_storage_img(self, determine, mkfs): + def test_setup_storage_img(self, determine, mkfs, mock_is_device_in_ring): + mock_is_device_in_ring.return_value = False determine.return_value = ["/srv/test.img", ] self.is_mapped_loopback_device.return_value = "/srv/test.img" swift_utils.setup_storage() @@ -376,9 +391,19 @@ def test_setup_storage_img(self, determine, mkfs): 'xfs', options='loop, defaults' ) + self.mkdir.assert_has_calls([ call('/srv/node', owner='swift', group='swift', perms=0o755), call('/srv/node/test.img', group='swift', owner='swift') ]) + @patch.object(swift_utils.subprocess, "check_output") + def test_get_device_blkid(self, mock_check_output): + dev = '/dev/vdb' + cmd = ['blkid', '-s', 'UUID', dev] + ret = '/dev/vdb: UUID="808bc298-0609-4619-aaef-ed7a5ab0ebb7" \n' + mock_check_output.return_value = ret + uuid = swift_utils.get_device_blkid(dev) + self.assertEquals(uuid, "808bc298-0609-4619-aaef-ed7a5ab0ebb7") + mock_check_output.assert_called_with(cmd)