cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #00633
[Merge] lp:~daniel-thewatkins/cloud-init/smartos-v2-metadata into lp:cloud-init
Dan Watkins has proposed merging lp:~daniel-thewatkins/cloud-init/smartos-v2-metadata into lp:cloud-init with lp:~daniel-thewatkins/cloud-init/fix-dmi as a prerequisite.
Requested reviews:
cloud init development team (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~daniel-thewatkins/cloud-init/smartos-v2-metadata/+merge/251720
Convert the SmartOS data source to use the new v2 metadata protocol as defined in http://eng.joyent.com/mdata/protocol.html.
Tested as working on Joyent.
--
Your team cloud init development team is requested to review the proposed merge of lp:~daniel-thewatkins/cloud-init/smartos-v2-metadata into lp:cloud-init.
=== modified file 'cloudinit/sources/DataSourceSmartOS.py'
--- cloudinit/sources/DataSourceSmartOS.py 2015-01-27 20:03:52 +0000
+++ cloudinit/sources/DataSourceSmartOS.py 2015-03-04 11:30:36 +0000
@@ -29,9 +29,10 @@
# http://us-east.manta.joyent.com/jmc/public/mdata/datadict.html
# Comments with "@datadictionary" are snippets of the definition
-import base64
import binascii
import os
+import random
+import re
import serial
from cloudinit import log as logging
@@ -301,6 +302,59 @@
return ser
+class JoyentMetadataFetchException(Exception):
+ pass
+
+
+class JoyentMetadataClient(object):
+
+ def __init__(self, serial):
+ self.serial = serial
+
+ def _checksum(self, body):
+ return '{0:08x}'.format(
+ binascii.crc32(body.encode('utf-8')) & 0xffffffff)
+
+ def _get_value_from_frame(self, expected_request_id, frame):
+ regex = (
+ r'V2 (?P<length>\d+) (?P<checksum>[0-9a-f]+)'
+ r' (?P<body>(?P<request_id>[0-9a-f]+) (?P<status>SUCCESS|NOTFOUND)'
+ r'( (?P<payload>.+))?)')
+ frame_data = re.match(regex, frame).groupdict()
+ if int(frame_data['length']) != len(frame_data['body']):
+ raise JoyentMetadataFetchException(
+ 'Incorrect frame length given ({0} != {1}).'.format(
+ frame_data['length'], len(frame_data['body'])))
+ expected_checksum = self._checksum(frame_data['body'])
+ if frame_data['checksum'] != expected_checksum:
+ raise JoyentMetadataFetchException(
+ 'Invalid checksum (expected: {0}; got {1}).'.format(
+ expected_checksum, frame_data['checksum']))
+ if frame_data['request_id'] != expected_request_id:
+ raise JoyentMetadataFetchException(
+ 'Request ID mismatch (expected: {0}; got {1}).'.format(
+ expected_request_id, frame_data['request_id']))
+ if not frame_data.get('payload', None):
+ LOG.info('No value found.')
+ return None
+ value = util.b64d(frame_data['payload'])
+ LOG.info('Value "%s" found.', value)
+ return value
+
+ def get_metadata(self, metadata_key):
+ LOG.info('Fetching metadata key "%s"...', metadata_key)
+ request_id = '{0:08x}'.format(random.randint(0, 0xffffffff))
+ message_body = '{0} GET {1}'.format(request_id,
+ util.b64e(metadata_key))
+ msg = 'V2 {0} {1} {2}\n'.format(
+ len(message_body), self._checksum(message_body), message_body)
+ LOG.debug('Writing "%s" to serial port.', msg)
+ self.serial.write(msg)
+ response = self.serial.readline()
+ LOG.debug('Read "%s" from serial port.', response)
+ return self._get_value_from_frame(request_id, response)
+
+
def query_data(noun, seed_device, seed_timeout, strip=False, default=None,
b64=None):
"""Makes a request to via the serial console via "GET <NOUN>"
@@ -314,33 +368,21 @@
encoded, so this method relies on being told if the data is base64 or
not.
"""
-
if not noun:
return False
ser = get_serial(seed_device, seed_timeout)
- ser.write("GET %s\n" % noun.rstrip())
- status = str(ser.readline()).rstrip()
- response = []
- eom_found = False
- if 'SUCCESS' not in status:
- ser.close()
+ client = JoyentMetadataClient(ser)
+ response = client.get_metadata(noun)
+ ser.close()
+ if response is None:
return default
- while not eom_found:
- m = ser.readline()
- if m.rstrip() == ".":
- eom_found = True
- else:
- response.append(m)
-
- ser.close()
-
if b64 is None:
b64 = query_data('b64-%s' % noun, seed_device=seed_device,
- seed_timeout=seed_timeout, b64=False,
- default=False, strip=True)
+ seed_timeout=seed_timeout, b64=False,
+ default=False, strip=True)
b64 = util.is_true(b64)
resp = None
=== modified file 'cloudinit/util.py'
--- cloudinit/util.py 2015-03-02 20:56:15 +0000
+++ cloudinit/util.py 2015-03-04 11:30:36 +0000
@@ -128,6 +128,28 @@
# Path for DMI Data
DMI_SYS_PATH = "/sys/class/dmi/id"
+# dmidecode and /sys/class/dmi/id/* use different names for the same value,
+# this allows us to refer to them by one canonical name
+DMIDECODE_TO_DMI_SYS_MAPPING = {
+ 'baseboard-asset-tag': 'board_asset_tag',
+ 'baseboard-manufacturer': 'board_vendor',
+ 'baseboard-product-name': 'board_name',
+ 'baseboard-serial-number': 'board_serial',
+ 'baseboard-version': 'board_version',
+ 'bios-release-date': 'bios_date',
+ 'bios-vendor': 'bios_vendor',
+ 'bios-version': 'bios_version',
+ 'chassis-asset-tag': 'chassis_asset_tag',
+ 'chassis-manufacturer': 'chassis_vendor',
+ 'chassis-serial-number': 'chassis_serial',
+ 'chassis-version': 'chassis_version',
+ 'system-manufacturer': 'sys_vendor',
+ 'system-product-name': 'product_name',
+ 'system-serial-number': 'product_serial',
+ 'system-uuid': 'product_uuid',
+ 'system-version': 'product_version',
+}
+
class ProcessExecutionError(IOError):
@@ -2103,24 +2125,26 @@
"""
Reads dmi data with from /sys/class/dmi/id
"""
-
- dmi_key = "{0}/{1}".format(DMI_SYS_PATH, key)
- LOG.debug("querying dmi data {0}".format(dmi_key))
+ if key not in DMIDECODE_TO_DMI_SYS_MAPPING:
+ return None
+ mapped_key = DMIDECODE_TO_DMI_SYS_MAPPING[key]
+ dmi_key_path = "{0}/{1}".format(DMI_SYS_PATH, mapped_key)
+ LOG.debug("querying dmi data {0}".format(dmi_key_path))
try:
- if not os.path.exists(dmi_key):
- LOG.debug("did not find {0}".format(dmi_key))
+ if not os.path.exists(dmi_key_path):
+ LOG.debug("did not find {0}".format(dmi_key_path))
return None
- key_data = load_file(dmi_key)
+ key_data = load_file(dmi_key_path)
if not key_data:
- LOG.debug("{0} did not return any data".format(key))
+ LOG.debug("{0} did not return any data".format(dmi_key_path))
return None
- LOG.debug("dmi data {0} returned {0}".format(dmi_key, key_data))
+ LOG.debug("dmi data {0} returned {1}".format(dmi_key_path, key_data))
return key_data.strip()
except Exception as e:
- logexc(LOG, "failed read of {0}".format(dmi_key), e)
+ logexc(LOG, "failed read of {0}".format(dmi_key_path), e)
return None
@@ -2134,18 +2158,27 @@
(result, _err) = subp(cmd)
LOG.debug("dmidecode returned '{0}' for '{0}'".format(result, key))
return result
- except OSError as _err:
+ except (IOError, OSError) as _err:
LOG.debug('failed dmidecode cmd: {0}\n{0}'.format(cmd, _err.message))
return None
def read_dmi_data(key):
"""
- Wrapper for reading DMI data. This tries to determine whether the DMI
- Data can be read directly, otherwise it will fallback to using dmidecode.
+ Wrapper for reading DMI data.
+
+ This will do the following (returning the first that produces a
+ result):
+ 1) Use a mapping to translate `key` from dmidecode naming to
+ sysfs naming and look in /sys/class/dmi/... for a value.
+ 2) Use `key` as a sysfs key directly and look in /sys/class/dmi/...
+ 3) Fall-back to passing `key` to `dmidecode --string`.
+
+ If all of the above fail to find a value, None will be returned.
"""
- if os.path.exists(DMI_SYS_PATH):
- return _read_dmi_syspath(key)
+ syspath_value = _read_dmi_syspath(key)
+ if syspath_value is not None:
+ return syspath_value
dmidecode_path = which('dmidecode')
if dmidecode_path:
@@ -2153,5 +2186,4 @@
LOG.warn("did not find either path {0} or dmidecode command".format(
DMI_SYS_PATH))
-
return None
=== modified file 'tests/unittests/test_datasource/test_smartos.py'
--- tests/unittests/test_datasource/test_smartos.py 2015-01-27 20:03:52 +0000
+++ tests/unittests/test_datasource/test_smartos.py 2015-03-04 11:30:36 +0000
@@ -24,18 +24,27 @@
from __future__ import print_function
-from cloudinit import helpers as c_helpers
-from cloudinit.sources import DataSourceSmartOS
-from cloudinit.util import b64e
-from .. import helpers
import os
import os.path
import re
import shutil
+import stat
import tempfile
-import stat
import uuid
-
+from binascii import crc32
+
+import serial
+
+from cloudinit import helpers as c_helpers
+from cloudinit.sources import DataSourceSmartOS
+from cloudinit.util import b64e
+
+from .. import helpers
+
+try:
+ from unittest import mock
+except ImportError:
+ import mock
MOCK_RETURNS = {
'hostname': 'test-host',
@@ -54,60 +63,15 @@
DMI_DATA_RETURN = (str(uuid.uuid4()), 'smartdc')
-class MockSerial(object):
- """Fake a serial terminal for testing the code that
- interfaces with the serial"""
-
- port = None
-
- def __init__(self, mockdata):
- self.last = None
- self.last = None
- self.new = True
- self.count = 0
- self.mocked_out = []
- self.mockdata = mockdata
-
- def open(self):
- return True
-
- def close(self):
- return True
-
- def isOpen(self):
- return True
-
- def write(self, line):
- line = line.replace('GET ', '')
- self.last = line.rstrip()
-
- def readline(self):
- if self.new:
- self.new = False
- if self.last in self.mockdata:
- return 'SUCCESS\n'
- else:
- return 'NOTFOUND %s\n' % self.last
-
- if self.last in self.mockdata:
- if not self.mocked_out:
- self.mocked_out = [x for x in self._format_out()]
-
- if len(self.mocked_out) > self.count:
- self.count += 1
- return self.mocked_out[self.count - 1]
-
- def _format_out(self):
- if self.last in self.mockdata:
- _mret = self.mockdata[self.last]
- try:
- for l in _mret.splitlines():
- yield "%s\n" % l.rstrip()
- except:
- yield "%s\n" % _mret.rstrip()
-
- yield '.'
- yield '\n'
+def get_mock_client(mockdata):
+ class MockMetadataClient(object):
+
+ def __init__(self, serial):
+ pass
+
+ def get_metadata(self, metadata_key):
+ return mockdata.get(metadata_key)
+ return MockMetadataClient
class TestSmartOSDataSource(helpers.FilesystemMockingTestCase):
@@ -155,9 +119,6 @@
if dmi_data is None:
dmi_data = DMI_DATA_RETURN
- def _get_serial(*_):
- return MockSerial(mockdata)
-
def _dmi_data():
return dmi_data
@@ -174,7 +135,9 @@
sys_cfg['datasource']['SmartOS'] = ds_cfg
self.apply_patches([(mod, 'LEGACY_USER_D', self.legacy_user_d)])
- self.apply_patches([(mod, 'get_serial', _get_serial)])
+ self.apply_patches([(mod, 'get_serial', mock.MagicMock())])
+ self.apply_patches([
+ (mod, 'JoyentMetadataClient', get_mock_client(mockdata))])
self.apply_patches([(mod, 'dmi_data', _dmi_data)])
self.apply_patches([(os, 'uname', _os_uname)])
self.apply_patches([(mod, 'device_exists', lambda d: True)])
@@ -453,3 +416,129 @@
setattr(ref, name, replace)
ret.append((ref, name, orig))
return ret
+
+
+class TestJoyentMetadataClient(helpers.FilesystemMockingTestCase):
+
+ def setUp(self):
+ super(TestJoyentMetadataClient, self).setUp()
+ self.serial = mock.MagicMock(spec=serial.Serial)
+ self.request_id = 0xabcdef12
+ self.metadata_value = 'value'
+ self.response_parts = {
+ 'command': 'SUCCESS',
+ 'crc': 'b5a9ff00',
+ 'length': 17 + len(b64e(self.metadata_value)),
+ 'payload': b64e(self.metadata_value),
+ 'request_id': '{0:08x}'.format(self.request_id),
+ }
+
+ def make_response():
+ payload = ''
+ if self.response_parts['payload']:
+ payload = ' {0}'.format(self.response_parts['payload'])
+ del self.response_parts['payload']
+ return (
+ 'V2 {length} {crc} {request_id} {command}{payload}\n'.format(
+ payload=payload, **self.response_parts))
+ self.serial.readline.side_effect = make_response
+ self.patched_funcs.enter_context(
+ mock.patch('cloudinit.sources.DataSourceSmartOS.random.randint',
+ mock.Mock(return_value=self.request_id)))
+
+ def _get_client(self):
+ return DataSourceSmartOS.JoyentMetadataClient(self.serial)
+
+ def assertEndsWith(self, haystack, prefix):
+ self.assertTrue(haystack.endswith(prefix),
+ "{0} does not end with '{1}'".format(
+ repr(haystack), prefix))
+
+ def assertStartsWith(self, haystack, prefix):
+ self.assertTrue(haystack.startswith(prefix),
+ "{0} does not start with '{1}'".format(
+ repr(haystack), prefix))
+
+ def test_get_metadata_writes_a_single_line(self):
+ client = self._get_client()
+ client.get_metadata('some_key')
+ self.assertEqual(1, self.serial.write.call_count)
+ written_line = self.serial.write.call_args[0][0]
+ self.assertEndsWith(written_line, '\n')
+ self.assertEqual(1, written_line.count('\n'))
+
+ def _get_written_line(self, key='some_key'):
+ client = self._get_client()
+ client.get_metadata(key)
+ return self.serial.write.call_args[0][0]
+
+ def test_get_metadata_line_starts_with_v2(self):
+ self.assertStartsWith(self._get_written_line(), 'V2')
+
+ def test_get_metadata_uses_get_command(self):
+ parts = self._get_written_line().strip().split(' ')
+ self.assertEqual('GET', parts[4])
+
+ def test_get_metadata_base64_encodes_argument(self):
+ key = 'my_key'
+ parts = self._get_written_line(key).strip().split(' ')
+ self.assertEqual(b64e(key), parts[5])
+
+ def test_get_metadata_calculates_length_correctly(self):
+ parts = self._get_written_line().strip().split(' ')
+ expected_length = len(' '.join(parts[3:]))
+ self.assertEqual(expected_length, int(parts[1]))
+
+ def test_get_metadata_uses_appropriate_request_id(self):
+ parts = self._get_written_line().strip().split(' ')
+ request_id = parts[3]
+ self.assertEqual(8, len(request_id))
+ self.assertEqual(request_id, request_id.lower())
+
+ def test_get_metadata_uses_random_number_for_request_id(self):
+ request_id = self._get_written_line().strip().split(' ')[3]
+ self.assertEqual('{0:08x}'.format(self.request_id), request_id)
+
+ def test_get_metadata_checksums_correctly(self):
+ parts = self._get_written_line().strip().split(' ')
+ expected_checksum = '{0:08x}'.format(
+ crc32(' '.join(parts[3:]).encode('utf-8')) & 0xffffffff)
+ checksum = parts[2]
+ self.assertEqual(expected_checksum, checksum)
+
+ def test_get_metadata_reads_a_line(self):
+ client = self._get_client()
+ client.get_metadata('some_key')
+ self.assertEqual(1, self.serial.readline.call_count)
+
+ def test_get_metadata_returns_valid_value(self):
+ client = self._get_client()
+ value = client.get_metadata('some_key')
+ self.assertEqual(self.metadata_value, value)
+
+ def test_get_metadata_throws_exception_for_incorrect_length(self):
+ self.response_parts['length'] = 0
+ client = self._get_client()
+ self.assertRaises(DataSourceSmartOS.JoyentMetadataFetchException,
+ client.get_metadata, 'some_key')
+
+ def test_get_metadata_throws_exception_for_incorrect_crc(self):
+ self.response_parts['crc'] = 'deadbeef'
+ client = self._get_client()
+ self.assertRaises(DataSourceSmartOS.JoyentMetadataFetchException,
+ client.get_metadata, 'some_key')
+
+ def test_get_metadata_throws_exception_for_request_id_mismatch(self):
+ self.response_parts['request_id'] = 'deadbeef'
+ client = self._get_client()
+ client._checksum = lambda _: self.response_parts['crc']
+ self.assertRaises(DataSourceSmartOS.JoyentMetadataFetchException,
+ client.get_metadata, 'some_key')
+
+ def test_get_metadata_returns_None_if_value_not_found(self):
+ self.response_parts['payload'] = ''
+ self.response_parts['command'] = 'NOTFOUND'
+ self.response_parts['length'] = 17
+ client = self._get_client()
+ client._checksum = lambda _: self.response_parts['crc']
+ self.assertIsNone(client.get_metadata('some_key'))
=== modified file 'tests/unittests/test_util.py'
--- tests/unittests/test_util.py 2015-02-13 20:49:40 +0000
+++ tests/unittests/test_util.py 2015-03-04 11:30:36 +0000
@@ -323,58 +323,67 @@
class TestReadDMIData(helpers.FilesystemMockingTestCase):
- def _patchIn(self, root):
- self.patchOS(root)
- self.patchUtils(root)
-
- def _write_key(self, key, content):
+ def setUp(self):
+ super(TestReadDMIData, self).setUp()
+ self.new_root = tempfile.mkdtemp()
+ self.addCleanup(shutil.rmtree, self.new_root)
+ self.patchOS(self.new_root)
+ self.patchUtils(self.new_root)
+
+ def _create_sysfs_parent_directory(self):
+ util.ensure_dir(os.path.join('sys', 'class', 'dmi', 'id'))
+
+ def _create_sysfs_file(self, key, content):
"""Mocks the sys path found on Linux systems."""
- new_root = tempfile.mkdtemp()
- self.addCleanup(shutil.rmtree, new_root)
- self._patchIn(new_root)
- util.ensure_dir(os.path.join('sys', 'class', 'dmi', 'id'))
-
+ self._create_sysfs_parent_directory()
dmi_key = "/sys/class/dmi/id/{0}".format(key)
util.write_file(dmi_key, content)
- def _no_syspath(self, key, content):
+ def _configure_dmidecode_return(self, key, content, error=None):
"""
In order to test a missing sys path and call outs to dmidecode, this
function fakes the results of dmidecode to test the results.
"""
- new_root = tempfile.mkdtemp()
- self.addCleanup(shutil.rmtree, new_root)
- self._patchIn(new_root)
- self.real_which = util.which
- self.real_subp = util.subp
-
- def _which(key):
- return True
- util.which = _which
-
- def _cdd(_key, error=None):
+ def _dmidecode_subp(cmd):
+ if cmd[-1] != key:
+ raise util.ProcessExecutionError()
return (content, error)
- util.subp = _cdd
-
- def test_key(self):
- key_content = "TEST-KEY-DATA"
- self._write_key("key", key_content)
- self.assertEquals(key_content, util.read_dmi_data("key"))
-
- def test_key_mismatch(self):
- self._write_key("test", "ABC")
- self.assertNotEqual("123", util.read_dmi_data("test"))
-
- def test_no_key(self):
- self._no_syspath(None, None)
- self.assertFalse(util.read_dmi_data("key"))
-
- def test_callout_dmidecode(self):
- """test to make sure that dmidecode is used when no syspath"""
- self._no_syspath("key", "stuff")
- self.assertEquals("stuff", util.read_dmi_data("key"))
- self._no_syspath("key", None)
- self.assertFalse(None, util.read_dmi_data("key"))
+
+ self.patched_funcs.enter_context(
+ mock.patch.object(util, 'which', lambda _: True))
+ self.patched_funcs.enter_context(
+ mock.patch.object(util, 'subp', _dmidecode_subp))
+
+ def patch_mapping(self, new_mapping):
+ self.patched_funcs.enter_context(
+ mock.patch('cloudinit.util.DMIDECODE_TO_DMI_SYS_MAPPING',
+ new_mapping))
+
+ def test_sysfs_used_with_key_in_mapping_and_file_on_disk(self):
+ self.patch_mapping({'mapped-key': 'mapped-value'})
+ expected_dmi_value = 'sys-used-correctly'
+ self._create_sysfs_file('mapped-value', expected_dmi_value)
+ self._configure_dmidecode_return('mapped-key', 'wrong-wrong-wrong')
+ self.assertEqual(expected_dmi_value, util.read_dmi_data('mapped-key'))
+
+ def test_dmidecode_used_if_no_sysfs_file_on_disk(self):
+ self.patch_mapping({})
+ self._create_sysfs_parent_directory()
+ expected_dmi_value = 'dmidecode-used'
+ self._configure_dmidecode_return('use-dmidecode', expected_dmi_value)
+ self.assertEqual(expected_dmi_value,
+ util.read_dmi_data('use-dmidecode'))
+
+ def test_none_returned_if_neither_source_has_data(self):
+ self.patch_mapping({})
+ self._configure_dmidecode_return('key', 'value')
+ self.assertEqual(None, util.read_dmi_data('expect-fail'))
+
+ def test_none_returned_if_dmidecode_not_in_path(self):
+ self.patched_funcs.enter_context(
+ mock.patch.object(util, 'which', lambda _: False))
+ self.patch_mapping({})
+ self.assertEqual(None, util.read_dmi_data('expect-fail'))
class TestMultiLog(helpers.FilesystemMockingTestCase):
