launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06555
[Merge] lp:~jtv/maas/node-start-user-data into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/node-start-user-data into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/node-start-user-data/+merge/95377
When Juju tells us to start up a node, it gives us a user_data blob¹ that the metadata service should provide to the node on request.
To this end, the Node's “start” API call stores the data in the NodeUserData model, where the metadata server can find it.
Since we couldn't figure out in reasonable time how to process a binary POST parameter in piston, and doing so would require changes on both the client side and the server side anyway, we opted for a braindead solution: base64-encode the data on the client side, send it as if it were regular form data, and base64-decode it on the server side.
Jeroen
—
¹) Binary Large OBJect. There was some confusion over whether we really need to support binary user_data, but Scott asks us to keep supporting it.
--
https://code.launchpad.net/~jtv/maas/node-start-user-data/+merge/95377
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/node-start-user-data into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-02-29 10:40:06 +0000
+++ src/maasserver/api.py 2012-03-01 14:59:17 +0000
@@ -21,6 +21,7 @@
"NodeMacsHandler",
]
+from base64 import b64decode
import httplib
import sys
import types
@@ -251,8 +252,21 @@
@api_exported('start', 'POST')
def start(self, request, system_id):
- """Power up a node."""
- nodes = Node.objects.start_nodes([system_id], request.user)
+ """Power up a node.
+
+ The user_data parameter, if set in the POST data, is taken as
+ base64-encoded binary data.
+
+ Ideally we'd have MIME multipart and content-transfer-encoding etc.
+ deal with the encapsulation of binary data, but couldn't make it work
+ with the framework in reasonable time so went for a dumb, manual
+ encoding instead.
+ """
+ user_data = request.POST.get('user_data', None)
+ if user_data is not None:
+ user_data = b64decode(user_data)
+ nodes = Node.objects.start_nodes(
+ [system_id], request.user, user_data=user_data)
if len(nodes) == 0:
raise PermissionDenied(
"You are not allowed to start up this node.")
=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py 2012-02-29 19:00:41 +0000
+++ src/maasserver/models.py 2012-03-01 14:59:17 +0000
@@ -292,7 +292,7 @@
self.provisioning_proxy.stop_nodes([node.system_id for node in nodes])
return nodes
- def start_nodes(self, ids, by_user):
+ def start_nodes(self, ids, by_user, user_data=None):
"""Request on given user's behalf that the given nodes be started up.
Power-on is only requested for nodes that the user has ownership
@@ -302,11 +302,19 @@
:type ids: Sequence
:param by_user: Requesting user.
:type by_user: User_
+ :param user_data: Optional blob of user-data to be made available to
+ the nodes through the metadata service. If not given, any
+ previous user data is used.
+ :type user_data: str
:return: Those Nodes for which power-on was actually requested.
:rtype: list
"""
+ from metadataserver.models import NodeUserData
self._set_provisioning_proxy()
nodes = self.get_editable_nodes(by_user, ids=ids)
+ if user_data is not None:
+ for node in nodes:
+ NodeUserData.objects.set_user_data(node, user_data)
self.provisioning_proxy.start_nodes(
[node.system_id for node in nodes])
return nodes
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-02-29 11:33:07 +0000
+++ src/maasserver/tests/test_api.py 2012-03-01 14:59:17 +0000
@@ -11,6 +11,7 @@
__metaclass__ = type
__all__ = []
+from base64 import b64encode
import httplib
import json
import os
@@ -29,7 +30,10 @@
)
from maasserver.testing.factory import factory
from maasserver.testing.oauthclient import OAuthAuthenticatedClient
-from metadataserver.models import NodeKey
+from metadataserver.models import (
+ NodeKey,
+ NodeUserData,
+ )
from metadataserver.nodeinituser import get_node_init_user
@@ -293,6 +297,19 @@
response = self.client.post(self.get_node_uri(node), {'op': 'start'})
self.assertEqual(httplib.OK, response.status_code)
+ def test_POST_stores_user_data(self):
+ node = factory.make_node(owner=self.logged_in_user)
+ user_data = (
+ b'\xff\x00\xff\xfe\xff\xff\xfe' +
+ factory.getRandomString().encode('ascii'))
+ response = self.client.post(
+ self.get_node_uri(node), {
+ 'op': 'start',
+ 'user_data': b64encode(user_data),
+ })
+ self.assertEqual(httplib.OK, response.status_code)
+ self.assertEqual(user_data, NodeUserData.objects.get_user_data(node))
+
def test_PUT_updates_node(self):
# The api allows to update a Node.
node = factory.make_node(hostname='diane')
=== modified file 'src/maasserver/tests/test_models.py'
--- src/maasserver/tests/test_models.py 2012-02-29 19:00:41 +0000
+++ src/maasserver/tests/test_models.py 2012-03-01 14:59:17 +0000
@@ -37,6 +37,7 @@
)
from maasserver.testing import TestCase
from maasserver.testing.factory import factory
+from metadataserver.models import NodeUserData
from piston.models import (
Consumer,
KEY_SIZE,
@@ -95,6 +96,10 @@
status = NODE_STATUS.ALLOCATED
return factory.make_node(set_hostname=True, status=status, owner=user)
+ def make_user_data(self):
+ """Create a blob of arbitrary user-data."""
+ return factory.getRandomString().encode('ascii')
+
def test_filter_by_ids_filters_nodes_by_ids(self):
nodes = [factory.make_node() for counter in range(5)]
ids = [node.system_id for node in nodes]
@@ -254,6 +259,38 @@
[startable_node],
Node.objects.start_nodes(ids, startable_node.owner))
+ def test_start_nodes_stores_user_data(self):
+ node = factory.make_node(owner=factory.make_user())
+ user_data = self.make_user_data()
+ Node.objects.start_nodes(
+ [node.system_id], node.owner, user_data=user_data)
+ self.assertEqual(user_data, NodeUserData.objects.get_user_data(node))
+
+ def test_start_nodes_does_not_store_user_data_for_uneditable_nodes(self):
+ node = factory.make_node(owner=factory.make_user())
+ original_user_data = self.make_user_data()
+ NodeUserData.objects.set_user_data(node, original_user_data)
+ Node.objects.start_nodes(
+ [node.system_id], factory.make_user(),
+ user_data=self.make_user_data())
+ self.assertEqual(
+ original_user_data, NodeUserData.objects.get_user_data(node))
+
+ def test_start_nodes_without_user_data_leaves_existing_data_alone(self):
+ node = factory.make_node(owner=factory.make_user())
+ user_data = self.make_user_data()
+ NodeUserData.objects.set_user_data(node, user_data)
+ Node.objects.start_nodes([node.system_id], node.owner, user_data=None)
+ self.assertEqual(user_data, NodeUserData.objects.get_user_data(node))
+
+ def test_start_nodes_with_user_data_overwrites_existing_data(self):
+ node = factory.make_node(owner=factory.make_user())
+ NodeUserData.objects.set_user_data(node, self.make_user_data())
+ user_data = self.make_user_data()
+ Node.objects.start_nodes(
+ [node.system_id], node.owner, user_data=user_data)
+ self.assertEqual(user_data, NodeUserData.objects.get_user_data(node))
+
class MACAddressTest(TestCase):
=== modified file 'src/metadataserver/fields.py'
--- src/metadataserver/fields.py 2012-02-24 13:13:55 +0000
+++ src/metadataserver/fields.py 2012-03-01 14:59:17 +0000
@@ -24,14 +24,14 @@
)
-class Bin(str):
+class Bin(bytes):
"""Wrapper class to convince django that a string is really binary.
- This is really just a "str," but gets around an idiosyncracy of Django
+ This is really just a "bytes," 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
+ can stay as it is). The line between bytes and unicode is dangerously
thin.
So, to store a value in a BinaryField, wrap it in a Bin:
@@ -40,15 +40,15 @@
"""
def __init__(self, initializer):
- """Wrap a str.
+ """Wrap a bytes.
:param initializer: Binary string of data for this Bin. This must
- be a str. Anything else is almost certainly a mistake, so e.g.
+ be a bytes. Anything else is almost certainly a mistake, so e.g.
this constructor will refuse to render None as b'None'.
- :type initializer: str
+ :type initializer: bytes
"""
- assert isinstance(initializer, str), (
- "Not a binary string: '%s'" % initializer)
+ assert isinstance(initializer, bytes), (
+ "Not a binary string: '%s'" % repr(initializer))
super(Bin, self).__init__(initializer)
@@ -90,7 +90,7 @@
elif isinstance(value, Bin):
# Python-side form. Convert to database form.
return b64encode(value)
- elif isinstance(value, str):
+ elif isinstance(value, bytes):
# Binary string. Require a Bin to make intent explicit.
raise AssertionError(
"Converting a binary string to BinaryField: "
=== modified file 'src/metadataserver/models.py'
--- src/metadataserver/models.py 2012-02-29 10:40:45 +0000
+++ src/metadataserver/models.py 2012-03-01 14:59:17 +0000
@@ -138,7 +138,8 @@
entry.data = Bin(data)
entry.save()
elif len(existing_entries) == 0:
- self.create(node=node, data=Bin(data))
+ wrapped_data = Bin(data)
+ self.create(node=node, data=wrapped_data)
else:
raise AssertionError("More than one user-data entry matches.")
