cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #00643
Re: [Merge] lp:~daniel-thewatkins/cloud-init/smartos-v2-metadata into lp:cloud-init
Diff comments:
> === modified file 'cloudinit/sources/DataSourceSmartOS.py'
> --- cloudinit/sources/DataSourceSmartOS.py 2015-01-27 20:03:52 +0000
> +++ cloudinit/sources/DataSourceSmartOS.py 2015-03-04 16:40:58 +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]+)'
Is this xml (sorta looks like it from your tag names)? Why not just use one of the built-in xml parsers that exist in python instead of something custom....
> + 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)
Seems like the above should just use contextlib.closing to ensure its safely closed....
> + 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 '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 16:40:58 +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'))
>
--
https://code.launchpad.net/~daniel-thewatkins/cloud-init/smartos-v2-metadata/+merge/251775
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.
References
