launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06491
[Merge] lp:~jtv/maas/userdata into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/userdata into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/userdata/+merge/94542
Support storage of user-data (for the metadata service) in the database. The interface is simple: one method on the manager class stores data, in binary form, another retrieves it. You don't even need to care whether you're overwriting an older version or creating something new.
This includes a Field class for storing binary data. There are things to be said about my approach, but you'll find them in the code where they will be available to those who need them in the future. Perhaps we could improve on this once we're on psycopg2 2.4.4 or better — we're on 2.4.1 now — but perhaps not. Maybe I should have split out the addition of the new Field type. But it turned out the branch wasn't all that big, so dear reviewer, I hope you can live with the difference.
You may wonder why I put the user data in a separate table, rather than on Node. One reason is to avoid bloating the Node table, but more importantly, there are noises from the Server team that instance-id really shouldn't be tied to just Node. And user-data, in turn, is probably tied to the instance-id more than to the Node. A separate table gives us the freedom to add more items to the lookup key later.
--
https://code.launchpad.net/~jtv/maas/userdata/+merge/94542
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/userdata into lp:maas.
=== added file 'src/metadataserver/fields.py'
--- src/metadataserver/fields.py 1970-01-01 00:00:00 +0000
+++ src/metadataserver/fields.py 2012-02-24 13:28:17 +0000
@@ -0,0 +1,109 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Custom field types for the metadata server."""
+
+from __future__ import (
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = [
+ 'BinaryField',
+ ]
+
+from base64 import (
+ b64decode,
+ b64encode,
+ )
+
+from django.db.models import (
+ Field,
+ SubfieldBase,
+ )
+
+
+class Bin(str):
+ """Wrapper class to convince django that a string is really binary.
+
+ This is really just a "str," but gets around an idiosyncracy of Django
+ custom field conversions: they must be able to tell on the fly whether a
+ value was retrieved from the database (and needs to be converted to a
+ python-side value), or whether it's already a python-side object (which
+ can stay as it is). The line between str and unicode is dangerously
+ thin.
+
+ So, to store a value in a BinaryField, wrap it in a Bin:
+
+ my_model_object.binary_data = Bin(b"\x01\x02\x03")
+ """
+
+ def __init__(self, initializer):
+ """Wrap a str.
+
+ :param initializer: Binary string of data for this Bin. This must
+ be a str. Anything else is almost certainly a mistake, so e.g.
+ this constructor will refuse to render None as b'None'.
+ :type initializer: str
+ """
+ assert isinstance(initializer, str), (
+ "Not a binary string: '%s'" % initializer)
+ super(Bin, self).__init__(initializer)
+
+
+class BinaryField(Field):
+ """A field that stores binary data.
+
+ The data is base64-encoded internally, so this is not very efficient.
+ Do not use this for large blobs.
+
+ We do not have direct support for binary data in django at the moment.
+ It's possible to create a django model Field based by a postgres BYTEA,
+ but:
+
+ 1. Any data you save gets mis-interpreted as encoded text. This won't
+ be obvious until you test with data that can't be decoded.
+ 2. Any data you retrieve gets truncated at the first zero byte.
+ """
+
+ __metaclass__ = SubfieldBase
+
+ def to_python(self, value):
+ """Django overridable: convert database value to python-side value."""
+ if isinstance(value, unicode):
+ # Encoded binary data from the database. Convert.
+ return Bin(b64decode(value))
+ elif value is None or isinstance(value, Bin):
+ # Already in python-side form.
+ return value
+ else:
+ raise AssertionError(
+ "Invalid BinaryField value (expected unicode): '%s'"
+ % repr(value))
+
+ def get_db_prep_value(self, value):
+ """Django overridable: convert python-side value to database value."""
+ if value is None:
+ # Equivalent of a NULL.
+ return None
+ elif isinstance(value, Bin):
+ # Python-side form. Convert to database form.
+ return b64encode(value)
+ elif isinstance(value, str):
+ # Binary string. Require a Bin to make intent explicit.
+ raise AssertionError(
+ "Converting a binary string to BinaryField: "
+ "either conversion is going the wrong way, or the value "
+ "needs to be wrapped in a Bin.")
+ elif isinstance(value, unicode):
+ # Unicode here is almost certainly a sign of a mistake.
+ raise AssertionError(
+ "A unicode string is being mistaken for binary data.")
+ else:
+ raise AssertionError(
+ "Invalid BinaryField value (expected Bin): '%s'"
+ % repr(value))
+
+ def get_internal_type(self):
+ return 'TextField'
=== modified file 'src/metadataserver/models.py'
--- src/metadataserver/models.py 2012-02-22 12:51:50 +0000
+++ src/metadataserver/models.py 2012-02-24 13:28:17 +0000
@@ -23,6 +23,10 @@
create_auth_token,
Node,
)
+from metadataserver.fields import (
+ Bin,
+ BinaryField,
+ )
from metadataserver.nodeinituser import get_node_init_user
from piston.models import KEY_SIZE
@@ -69,3 +73,40 @@
node = ForeignKey(Node, null=False, editable=False)
key = CharField(max_length=KEY_SIZE, null=False, editable=False)
+
+
+class NodeUserDataManager(Manager):
+ """Utility for the collection of NodeUserData items."""
+
+ def set_user_data(self, node, data):
+ """Set user data for the given node."""
+ existing_entries = self.filter(node=node)
+ if len(existing_entries) == 1:
+ [entry] = existing_entries
+ entry.data = Bin(data)
+ entry.save()
+ elif len(existing_entries) == 0:
+ self.create(node=node, data=Bin(data))
+ else:
+ raise AssertionError("More than one user-data entry matches.")
+
+ def get_user_data(self, node):
+ """Retrieve user data for the given node."""
+ return self.get(node=node).data
+
+
+class NodeUserData(Model):
+ """User-data portion of a node's metadata.
+
+ When cloud-init sets up a node, it retrieves specific data for that node
+ from the metadata service. One portion of that is the "user-data" binary
+ blob.
+
+ :ivar node: Node that this is for.
+ :ivar data: base64-encoded data.
+ """
+
+ objects = NodeUserDataManager()
+
+ node = ForeignKey(Node, null=False, editable=False)
+ data = BinaryField(null=False)
=== added file 'src/metadataserver/tests/models.py'
--- src/metadataserver/tests/models.py 1970-01-01 00:00:00 +0000
+++ src/metadataserver/tests/models.py 2012-02-24 13:28:17 +0000
@@ -0,0 +1,23 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test model for testing BinaryField."""
+
+from __future__ import (
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = [
+ "BinaryFieldModel",
+ ]
+
+from django.db.models import Model
+from metadataserver.fields import BinaryField
+
+
+class BinaryFieldModel(Model):
+ """Test model for BinaryField. Contains nothing but a BinaryField."""
+
+ data = BinaryField(null=True)
=== added file 'src/metadataserver/tests/test_fields.py'
--- src/metadataserver/tests/test_fields.py 1970-01-01 00:00:00 +0000
+++ src/metadataserver/tests/test_fields.py 2012-02-24 13:28:17 +0000
@@ -0,0 +1,81 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test custom field types."""
+
+from __future__ import (
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+from maasserver.testing import (
+ TestCase,
+ TestModelTestCase,
+ )
+from metadataserver.fields import Bin
+from metadataserver.tests.models import BinaryFieldModel
+
+
+class TestBin(TestCase):
+ """Test Bin helper class."""
+
+ def test_is_basically_str(self):
+ self.assertEqual(str(b"Hello"), Bin(b"Hello"))
+
+ def test_refuses_to_construct_from_unicode(self):
+ self.assertRaises(AssertionError, Bin, u"Hello")
+
+ def test_refuses_to_construct_from_None(self):
+ self.assertRaises(AssertionError, Bin, None)
+
+
+class TestBinaryField(TestModelTestCase):
+ """Test BinaryField. Uses BinaryFieldModel test model."""
+
+ app = 'metadataserver.tests'
+
+ def test_stores_and_retrieves_None(self):
+ binary_item = BinaryFieldModel()
+ self.assertIsNone(binary_item.data)
+ binary_item.save()
+ self.assertIsNone(
+ BinaryFieldModel.objects.get(id=binary_item.id).data)
+
+ def test_stores_and_retrieves_empty_data(self):
+ binary_item = BinaryFieldModel(data=Bin(b''))
+ self.assertEqual(b'', binary_item.data)
+ binary_item.save()
+ self.assertEqual(
+ b'', BinaryFieldModel.objects.get(id=binary_item.id).data)
+
+ def test_does_not_truncate_at_zero_bytes(self):
+ data = b"BEFORE THE ZERO\x00AFTER THE ZERO"
+ binary_item = BinaryFieldModel(data=Bin(data))
+ self.assertEqual(data, binary_item.data)
+ binary_item.save()
+ self.assertEqual(
+ data, BinaryFieldModel.objects.get(id=binary_item.id).data)
+
+ def test_stores_and_retrieves_binary_data(self):
+ data = b"\x01\x02\xff\xff\xfe\xff\xff\xfe"
+ binary_item = BinaryFieldModel(data=Bin(data))
+ self.assertEqual(data, binary_item.data)
+ binary_item.save()
+ self.assertEqual(
+ data, BinaryFieldModel.objects.get(id=binary_item.id).data)
+
+ def test_returns_bytes_not_text(self):
+ binary_item = BinaryFieldModel(data=Bin(b"Data"))
+ binary_item.save()
+ retrieved_data = BinaryFieldModel.objects.get(id=binary_item.id).data
+ self.assertIsInstance(retrieved_data, str)
+
+ def test_looks_up_data(self):
+ data = b"Binary item"
+ binary_item = BinaryFieldModel(data=Bin(data))
+ binary_item.save()
+ self.assertEqual(
+ binary_item, BinaryFieldModel.objects.get(data=Bin(data)))
=== modified file 'src/metadataserver/tests/test_models.py'
--- src/metadataserver/tests/test_models.py 2012-02-21 12:20:00 +0000
+++ src/metadataserver/tests/test_models.py 2012-02-24 13:28:17 +0000
@@ -13,7 +13,10 @@
from maasserver.testing.factory import factory
from maastesting import TestCase
-from metadataserver.models import NodeKey
+from metadataserver.models import (
+ NodeKey,
+ NodeUserData,
+ )
class TestNodeKeyManager(TestCase):
@@ -34,3 +37,45 @@
non_key = factory.getRandomString()
self.assertRaises(
NodeKey.DoesNotExist, NodeKey.objects.get_node_for_key, non_key)
+
+
+class TestNodeUserDataManager(TestCase):
+ """Test NodeUserDataManager."""
+
+ def test_set_user_data_creates_new_nodeuserdata_if_needed(self):
+ node = factory.make_node()
+ data = b'foo'
+ NodeUserData.objects.set_user_data(node, data)
+ self.assertEqual(data, NodeUserData.objects.get(node=node).data)
+
+ def test_set_user_data_overwrites_existing_userdata(self):
+ node = factory.make_node()
+ data = b'bar'
+ NodeUserData.objects.set_user_data(node, b'old data')
+ NodeUserData.objects.set_user_data(node, data)
+ self.assertEqual(data, NodeUserData.objects.get(node=node).data)
+
+ def test_set_user_data_leaves_data_for_other_nodes_alone(self):
+ node = factory.make_node()
+ NodeUserData.objects.set_user_data(node, b'intact')
+ NodeUserData.objects.set_user_data(factory.make_node(), b'unrelated')
+ self.assertEqual(b'intact', NodeUserData.objects.get(node=node).data)
+
+ def test_get_user_data_retrieves_data(self):
+ node = factory.make_node()
+ data = b'splat'
+ NodeUserData.objects.set_user_data(node, data)
+ self.assertEqual(data, NodeUserData.objects.get_user_data(node))
+
+ def test_get_user_data_raises_DoesNotExist_if_not_found(self):
+ node = factory.make_node()
+ self.assertRaises(
+ NodeUserData.DoesNotExist,
+ NodeUserData.objects.get_user_data, node)
+
+ def test_get_user_data_ignores_other_nodes(self):
+ node = factory.make_node()
+ data = b'bzzz'
+ NodeUserData.objects.set_user_data(node, data)
+ NodeUserData.objects.set_user_data(factory.make_node(), b'unrelated')
+ self.assertEqual(data, NodeUserData.objects.get_user_data(node))
