From ffa19cbd80ba10453f0ef448c1c10dbcbf5be504 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 5 Oct 2012 13:38:54 -0700 Subject: Ensure that config drive datasource attempts to translate the device name to a actual device using logic that will try the ec2 metadata (if avail) or will try using 'blkid' to find a corresponding label. LP: #1062540 --- cloudinit/sources/DataSourceConfigDrive.py | 58 ++++++++++++++++++++++++++++++ cloudinit/sources/DataSourceEc2.py | 16 --------- cloudinit/sources/__init__.py | 17 +++++++++ 3 files changed, 75 insertions(+), 16 deletions(-) diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index b1cf942e..495eee82 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -48,6 +48,7 @@ class DataSourceConfigDrive(sources.DataSource): self.dsmode = 'local' self.seed_dir = os.path.join(paths.seed_dir, 'config_drive') self.version = None + self.ec2_metadata = None def __str__(self): mstr = "%s [%s,ver=%s]" % (util.obj_name(self), self.dsmode, @@ -55,6 +56,62 @@ class DataSourceConfigDrive(sources.DataSource): mstr += "[source=%s]" % (self.source) return mstr + def _ec2_name_to_device(self, name): + if not self.ec2_metadata: + return None + bdm = self.ec2_metadata.get('block-device-mapping', {}) + for (ent_name, device) in bdm.items(): + if name == ent_name: + return device + return None + + def _os_name_to_device(self, name): + device = None + try: + dev_entries = util.find_devs_with('LABEL=%s' % (name)) + if dev_entries: + device = dev_entries[0] + except util.ProcessExecutionError: + pass + return device + + def device_name_to_device(self, name): + # Translate a 'name' to a 'physical' device + if not name: + return None + # Try the ec2 mapping first + names = [name] + if name == 'root': + names.insert(0, 'ami') + if name == 'ami': + names.append('root') + device = None + for n in names: + device = self._ec2_name_to_device(n) + if device: + break + # Try the openstack way second + if not device: + for n in names: + device = self._os_name_to_device(n) + if device: + break + # Ok give up... + if not device: + return None + # Ensure translated ok + if not device.startswith("/"): + device = "/dev/%s" % device + if os.path.exists(device): + return device + # Durn, try adjusting the mapping + remapped = self._remap_device(os.path.basename(device)) + if remapped: + LOG.debug("Remapped device name %s => %s", device, remapped) + return remapped + # Really give up now + return None + def get_data(self): found = None md = {} @@ -143,6 +200,7 @@ class DataSourceConfigDrive(sources.DataSource): self.source = found self.metadata = md + self.ec2_metadata = results.get('ec2-metadata') self.userdata_raw = results.get('userdata') self.version = results['cfgdrive_ver'] diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index c7ad6d54..3686fa10 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -151,22 +151,6 @@ class DataSourceEc2(sources.DataSource): self.metadata_address = url2base.get(url) return bool(url) - def _remap_device(self, short_name): - # LP: #611137 - # the metadata service may believe that devices are named 'sda' - # when the kernel named them 'vda' or 'xvda' - # we want to return the correct value for what will actually - # exist in this instance - mappings = {"sd": ("vd", "xvd")} - for (nfrom, tlist) in mappings.iteritems(): - if not short_name.startswith(nfrom): - continue - for nto in tlist: - cand = "/dev/%s%s" % (nto, short_name[len(nfrom):]) - if os.path.exists(cand): - return cand - return None - def device_name_to_device(self, name): # Consult metadata service, that has # ephemeral0: sdb diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 04083d0c..b22369a8 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -23,6 +23,7 @@ from email.mime.multipart import MIMEMultipart import abc +import os from cloudinit import importer from cloudinit import log as logging @@ -128,6 +129,22 @@ class DataSource(object): return keys + def _remap_device(self, short_name): + # LP: #611137 + # the metadata service may believe that devices are named 'sda' + # when the kernel named them 'vda' or 'xvda' + # we want to return the correct value for what will actually + # exist in this instance + mappings = {"sd": ("vd", "xvd")} + for (nfrom, tlist) in mappings.iteritems(): + if not short_name.startswith(nfrom): + continue + for nto in tlist: + cand = "/dev/%s%s" % (nto, short_name[len(nfrom):]) + if os.path.exists(cand): + return cand + return None + def device_name_to_device(self, _name): # translate a 'name' to a device # the primary function at this point is on ec2 -- cgit v1.2.3 From 5b4fa81016f487fb6e041cef5a3b4ac0bd0863c5 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 5 Oct 2012 14:50:22 -0700 Subject: Add tests to show that the assigned bug is fixed. Also fix the extraction of the metadata key name since it actually uses 'dashes' instead of being a single word. --- cloudinit/sources/DataSourceConfigDrive.py | 2 +- .../unittests/test_datasource/test_configdrive.py | 147 +++++++++++++++++++-- 2 files changed, 140 insertions(+), 9 deletions(-) diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index 495eee82..eebe44ec 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -285,7 +285,7 @@ def read_config_drive_dir_v2(source_dir, version="2012-08-10"): ('metadata', "openstack/%s/meta_data.json" % version, True, json.loads), ('userdata', "openstack/%s/user_data" % version, False, None), - ('ec2-metadata', "ec2/latest/metadata.json", False, json.loads), + ('ec2-metadata', "ec2/latest/meta-data.json", False, json.loads), ) results = {'userdata': None} diff --git a/tests/unittests/test_datasource/test_configdrive.py b/tests/unittests/test_datasource/test_configdrive.py index 55573114..99936d92 100644 --- a/tests/unittests/test_datasource/test_configdrive.py +++ b/tests/unittests/test_datasource/test_configdrive.py @@ -4,10 +4,15 @@ import os import os.path import shutil import tempfile -from unittest import TestCase + +import mocker +from mocker import MockerTestCase from cloudinit.sources import DataSourceConfigDrive as ds +from cloudinit import settings from cloudinit import util +from cloudinit import helpers + PUBKEY = u'ssh-rsa AAAAB3NzaC1....sIkJhq8wdX+4I3A4cYbYP ubuntu@server-460\n' @@ -60,17 +65,143 @@ CFG_DRIVE_FILES_V2 = { 'openstack/latest/user_data': USER_DATA} -class TestConfigDriveDataSource(TestCase): +class TestConfigDriveDataSource(MockerTestCase): def setUp(self): super(TestConfigDriveDataSource, self).setUp() - self.tmp = tempfile.mkdtemp() + self.tmp = self.makeDir() - def tearDown(self): - try: - shutil.rmtree(self.tmp) - except OSError: - pass + def test_ec2_metadata(self): + populate_dir(self.tmp, CFG_DRIVE_FILES_V2) + cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, + None, + helpers.Paths({})) + found = ds.read_config_drive_dir(self.tmp) + self.assertTrue('ec2-metadata' in found) + ec2_md = found['ec2-metadata'] + self.assertEqual(EC2_META, ec2_md) + + def test_dev_os_remap(self): + populate_dir(self.tmp, CFG_DRIVE_FILES_V2) + cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, + None, + helpers.Paths({})) + found = ds.read_config_drive_dir(self.tmp) + cfg_ds.metadata = found['metadata'] + name_tests = { + 'ami': '/dev/vda1', + 'root': '/dev/vda1', + 'ephemeral0': '/dev/vda2', + 'swap': '/dev/vda3', + } + for name, dev_name in name_tests.items(): + my_mock = mocker.Mocker() + find_mock = my_mock.replace(util.find_devs_with, + spec=False, passthrough=False) + provided_name = dev_name[len('/dev/'):] + provided_name = "s" + provided_name[1:] + find_mock(mocker.ARGS) + my_mock.result([provided_name]) + exists_mock = my_mock.replace(os.path.exists, + spec=False, passthrough=False) + exists_mock(mocker.ARGS) + my_mock.result(False) + exists_mock(mocker.ARGS) + my_mock.result(True) + my_mock.replay() + device = cfg_ds.device_name_to_device(name) + my_mock.restore() + self.assertEquals(dev_name, device) + + def test_dev_os_map(self): + populate_dir(self.tmp, CFG_DRIVE_FILES_V2) + cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, + None, + helpers.Paths({})) + found = ds.read_config_drive_dir(self.tmp) + os_md = found['metadata'] + cfg_ds.metadata = os_md + name_tests = { + 'ami': '/dev/vda1', + 'root': '/dev/vda1', + 'ephemeral0': '/dev/vda2', + 'swap': '/dev/vda3', + } + for name, dev_name in name_tests.items(): + my_mock = mocker.Mocker() + find_mock = my_mock.replace(util.find_devs_with, + spec=False, passthrough=False) + find_mock(mocker.ARGS) + my_mock.result([dev_name]) + exists_mock = my_mock.replace(os.path.exists, + spec=False, passthrough=False) + exists_mock(mocker.ARGS) + my_mock.result(True) + my_mock.replay() + device = cfg_ds.device_name_to_device(name) + my_mock.restore() + self.assertEquals(dev_name, device) + + def test_dev_ec2_remap(self): + populate_dir(self.tmp, CFG_DRIVE_FILES_V2) + cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, + None, + helpers.Paths({})) + found = ds.read_config_drive_dir(self.tmp) + ec2_md = found['ec2-metadata'] + os_md = found['metadata'] + cfg_ds.ec2_metadata = ec2_md + cfg_ds.metadata = os_md + name_tests = { + 'ami': '/dev/vda1', + 'root': '/dev/vda1', + 'ephemeral0': '/dev/vda2', + 'swap': '/dev/vda3', + None: None, + 'bob': None, + 'root2k': None, + } + for name, dev_name in name_tests.items(): + my_mock = mocker.Mocker() + exists_mock = my_mock.replace(os.path.exists, + spec=False, passthrough=False) + exists_mock(mocker.ARGS) + my_mock.result(False) + exists_mock(mocker.ARGS) + my_mock.result(True) + my_mock.replay() + device = cfg_ds.device_name_to_device(name) + self.assertEquals(dev_name, device) + my_mock.restore() + + def test_dev_ec2_map(self): + populate_dir(self.tmp, CFG_DRIVE_FILES_V2) + cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, + None, + helpers.Paths({})) + found = ds.read_config_drive_dir(self.tmp) + exists_mock = self.mocker.replace(os.path.exists, + spec=False, passthrough=False) + exists_mock(mocker.ARGS) + self.mocker.count(0, None) + self.mocker.result(True) + self.mocker.replay() + ec2_md = found['ec2-metadata'] + os_md = found['metadata'] + cfg_ds.ec2_metadata = ec2_md + cfg_ds.metadata = os_md + name_tests = { + 'ami': '/dev/sda1', + 'root': '/dev/sda1', + 'ephemeral0': '/dev/sda2', + 'swap': '/dev/sda3', + None: None, + 'bob': None, + 'root2k': None, + } + for name, dev_name in name_tests.items(): + device = cfg_ds.device_name_to_device(name) + self.assertEquals(dev_name, device) def test_dir_valid(self): """Verify a dir is read as such.""" -- cgit v1.2.3 From f510d8f5762f3c9d27afcc57f63d7614ec6c05cd Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 5 Oct 2012 15:43:54 -0700 Subject: Add checks around the device names that are found to ensure that even if they are found that they are also valid, before they are assumed to be the correct device name. --- cloudinit/sources/DataSourceConfigDrive.py | 36 ++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index eebe44ec..4af2e5ae 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -68,13 +68,30 @@ class DataSourceConfigDrive(sources.DataSource): def _os_name_to_device(self, name): device = None try: - dev_entries = util.find_devs_with('LABEL=%s' % (name)) + criteria = 'LABEL=%s' % (name) + if name in ['swap']: + criteria = 'TYPE=%s' % (name) + dev_entries = util.find_devs_with(criteria) if dev_entries: device = dev_entries[0] except util.ProcessExecutionError: pass return device + def _validate_device_name(self, device): + if not device: + return None + if not device.startswith("/"): + device = "/dev/%s" % device + if os.path.exists(device): + return device + # Durn, try adjusting the mapping + remapped = self._remap_device(os.path.basename(device)) + if remapped: + LOG.debug("Remapped device name %s => %s", device, remapped) + return remapped + return None + def device_name_to_device(self, name): # Translate a 'name' to a 'physical' device if not name: @@ -86,31 +103,26 @@ class DataSourceConfigDrive(sources.DataSource): if name == 'ami': names.append('root') device = None + LOG.debug("Using ec2 metadata lookup to find device %s", names) for n in names: device = self._ec2_name_to_device(n) + device = self._validate_device_name(device) if device: break # Try the openstack way second if not device: + LOG.debug("Using os lookup to find device %s", names) for n in names: device = self._os_name_to_device(n) + device = self._validate_device_name(device) if device: break # Ok give up... if not device: return None - # Ensure translated ok - if not device.startswith("/"): - device = "/dev/%s" % device - if os.path.exists(device): + else: + LOG.debug("Using cfg drive lookup mapped to device %s", device) return device - # Durn, try adjusting the mapping - remapped = self._remap_device(os.path.basename(device)) - if remapped: - LOG.debug("Remapped device name %s => %s", device, remapped) - return remapped - # Really give up now - return None def get_data(self): found = None -- cgit v1.2.3 From 40a54a4f4a486bc196ee0eac53ef630c828aef8e Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 5 Oct 2012 15:46:20 -0700 Subject: Pylint cleanups. --- tests/unittests/test_datasource/test_configdrive.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/unittests/test_datasource/test_configdrive.py b/tests/unittests/test_datasource/test_configdrive.py index 99936d92..4fa13db8 100644 --- a/tests/unittests/test_datasource/test_configdrive.py +++ b/tests/unittests/test_datasource/test_configdrive.py @@ -2,8 +2,6 @@ from copy import copy import json import os import os.path -import shutil -import tempfile import mocker from mocker import MockerTestCase @@ -73,9 +71,6 @@ class TestConfigDriveDataSource(MockerTestCase): def test_ec2_metadata(self): populate_dir(self.tmp, CFG_DRIVE_FILES_V2) - cfg_ds = ds.DataSourceConfigDrive(settings.CFG_BUILTIN, - None, - helpers.Paths({})) found = ds.read_config_drive_dir(self.tmp) self.assertTrue('ec2-metadata' in found) ec2_md = found['ec2-metadata'] -- cgit v1.2.3