launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06521
[Merge] lp:~jtv/maas/pass-metadata into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/pass-metadata into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/pass-metadata/+merge/94974
The nodes need to know how to access the metadata service: its URL and the node's oauth credentials. This information needs to be produced by the maasserver and from there, handed to provisioningserver and on to cobbler. The format for this string is documented in the metadata server requirements document, based on a call I had with Scott: in Cobbler terms, a system's ks_meta should contain a MAAS_PRESEED string of the form...
cloud-init cloud-init/datasources multiselect Maas
cloud-init cloud-init/maas-metadata-url string http://192.168.0.5/metadata/
cloud-init cloud-init/maas-metadata-credentials string oauth_consumer_key=Xh234sdkljf&oauth_token_key=kjhfgb3n&oauth_token_secret=24uysdfx1w4
This will get fed directly into debconf, hence the format.
In this branch I pass the information through the provisioning API (which should encapsulate Cobbler as much as possible) as a “metadata dict.” The provisioning server then uses that to generate the specified debconf stanza.
I can really only test this whole bucket chain against a real provisioning API implementation. It in turn can run against a fake cobbler just fine, since by that stage ks_meta is just another attribute. The structure of the API tests allowed for tests that only run against the fake API, and for tests that run against a fake or a real implementation, but not for tests that only run against the real one. I fixed that by putting the tests in an abstract base class and having the two concrete test cases (respectively against real and fake provisioning API implementations) derive from that and mix in TestCase. The concrete test cases are now siblings in the class family.
The way this branch produces the metadata service URL is a bit iffy. We may at some point abstract away the starting point of the metadata API on the http tree, but more importantly, where does the “host” part of the URL come from? We can't just pick an IP address because there may be multiple (and the right one may be virtual). For now I made this a configuration item and had it default to `/bin/hostname`. Not really good enough in my opinion, but improving on it is work for a later ticket.
Jeroen
--
https://code.launchpad.net/~jtv/maas/pass-metadata/+merge/94974
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/pass-metadata into lp:maas.
=== modified file 'src/maasserver/models.py'
--- src/maasserver/models.py 2012-02-27 16:39:57 +0000
+++ src/maasserver/models.py 2012-02-28 14:53:45 +0000
@@ -585,6 +585,9 @@
# Default values for config options.
DEFAULT_CONFIG = {
+ # The address by which this server can be accessed. The nodes need
+ # this information to access the metadata service.
+ "server_address": None,
}
=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py 2012-02-16 12:12:09 +0000
+++ src/maasserver/provisioning.py 2012-02-28 14:53:45 +0000
@@ -13,6 +13,8 @@
'get_provisioning_api_proxy',
]
+import subprocess
+from urllib import urlencode
import warnings
import xmlrpclib
@@ -23,9 +25,11 @@
)
from django.dispatch import receiver
from maasserver.models import (
+ Config,
MACAddress,
Node,
)
+from metadataserver.models import NodeKey
def get_provisioning_api_proxy():
@@ -52,6 +56,39 @@
url, allow_none=True, use_datetime=True)
+def get_server_name():
+ """Hostname under which the metadata service can be reached."""
+ setting = Config.objects.get_config('server_address')
+ if setting is not None:
+ return setting
+ return subprocess.check_output('/bin/hostname').strip()
+
+
+def get_metadata_server_url():
+ """Return the URL where nodes can reach the metadata service."""
+ return "http://%s/metadata/"; % get_server_name()
+
+
+def compose_metadata(node):
+ """Put together metadata information for `node`.
+
+ :param node: The node to provide with metadata.
+ :type node: Node
+ :return: A dict containing metadata information that will be seeded to
+ the node, so that it can access the metadata service.
+ """
+ token = NodeKey.objects.get_token_for_node(node)
+ credentials = urlencode({
+ 'oauth_consumer_key': token.consumer.key,
+ 'oauth_token_key': token.key,
+ 'oauth_token_secret': token.secret,
+ })
+ return {
+ 'maas-metadata-url': get_metadata_server_url(),
+ 'maas-metadata-credentials': credentials,
+ }
+
+
@receiver(post_save, sender=Node)
def provision_post_save_Node(sender, instance, created, **kwargs):
"""Create or update nodes in the provisioning server."""
@@ -66,7 +103,7 @@
"No profiles defined in Cobbler; has "
"cobbler-ubuntu-import been run?")
profile = sorted(profiles)[0]
- papi.add_node(instance.system_id, profile)
+ papi.add_node(instance.system_id, profile, compose_metadata(instance))
def set_node_mac_addresses(node):
=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py 2012-02-16 11:51:13 +0000
+++ src/maasserver/tests/test_provisioning.py 2012-02-28 14:53:45 +0000
@@ -11,10 +11,22 @@
__metaclass__ = type
__all__ = []
+import subprocess
+from urlparse import parse_qs
+
from maasserver import provisioning
-from maasserver.models import Node
+from maasserver.models import (
+ DEFAULT_CONFIG,
+ Node,
+ )
+from maasserver.provisioning import (
+ compose_metadata,
+ get_metadata_server_url,
+ get_server_name,
+ )
from maasserver.testing import TestCase
from maasserver.testing.factory import factory
+from metadataserver.models import NodeKey
class ProvisioningTests:
@@ -94,6 +106,39 @@
node = self.papi.get_nodes_by_name(["frank"])["frank"]
self.assertEqual([], node["mac_addresses"])
+ def test_get_server_name_reads_name_from_config(self):
+ DEFAULT_CONFIG['server_address'] = factory.getRandomString()
+ self.assertEqual(DEFAULT_CONFIG['server_address'], get_server_name())
+
+ def test_get_server_name_defaults_to_hostname(self):
+ DEFAULT_CONFIG['server_address'] = None
+ server_name = get_server_name()
+ self.assertIsInstance(server_name, basestring)
+ self.assertEqual(
+ subprocess.check_output('/bin/hostname').strip(), server_name)
+
+ def test_metadata_server_url_refers_to_own_metadata_service(self):
+ self.assertEqual(
+ "http://%s/metadata/"; % get_server_name(),
+ get_metadata_server_url())
+
+ def test_compose_metadata_includes_metadata_url(self):
+ node = factory.make_node()
+ self.assertEqual(
+ get_metadata_server_url(),
+ compose_metadata(node)['maas-metadata-url'])
+
+ def test_compose_metadata_includes_node_oauth_token(self):
+ node = factory.make_node()
+ metadata = compose_metadata(node)
+ token = NodeKey.objects.get_token_for_node(node)
+ self.assertEqual({
+ 'oauth_consumer_key': [token.consumer.key],
+ 'oauth_token_key': [token.key],
+ 'oauth_token_secret': [token.secret],
+ },
+ parse_qs(metadata['maas-metadata-credentials']))
+
class TestProvisioningWithFake(ProvisioningTests, TestCase):
"""Tests for the Provisioning API using a fake."""
=== modified file 'src/provisioningserver/api.py'
--- src/provisioningserver/api.py 2012-02-14 13:55:08 +0000
+++ src/provisioningserver/api.py 2012-02-28 14:53:45 +0000
@@ -146,6 +146,18 @@
}
+# Preseed data to send to cloud-init. We set this as MAAS_PRESEED in
+# ks_meta, and it gets fed straight into debconf.
+metadata_preseed_items = [
+ ('datasources', 'multiselect', 'Maas'),
+ ('maas-metadata-url', 'string', '%(maas-metadata-url)s'),
+ ('maas-metadata-credentials', 'string', '%(maas-metadata-credentials)s'),
+ ]
+metadata_preseed = '\n'.join([
+ "cloud-init cloud-init/%s %s %s" % (item_name, item_type, item_value)
+ for item_name, item_type, item_value in metadata_preseed_items])
+
+
class ProvisioningAPI:
implements(IProvisioningAPI)
@@ -175,11 +187,15 @@
returnValue(profile.name)
@inlineCallbacks
- def add_node(self, name, profile):
+ def add_node(self, name, profile, metadata):
assert isinstance(name, basestring)
assert isinstance(profile, basestring)
- system = yield CobblerSystem.new(
- self.session, name, {"profile": profile})
+ assert isinstance(metadata, dict)
+ attributes = {
+ "profile": profile,
+ "ks_meta": {"MAAS_PRESEED": metadata_preseed % metadata},
+ }
+ system = yield CobblerSystem.new(self.session, name, attributes)
returnValue(system.name)
@inlineCallbacks
=== modified file 'src/provisioningserver/interfaces.py'
--- src/provisioningserver/interfaces.py 2012-02-13 18:05:35 +0000
+++ src/provisioningserver/interfaces.py 2012-02-28 14:53:45 +0000
@@ -50,9 +50,16 @@
:return: The name of the new profile.
"""
- def add_node(name, profile):
- """Add a node with the given `name`, and `profile`.
+ def add_node(name, profile, metadata):
+ """Add a node with the given `name`.
+ :param profile: Name of profile to associate the node with.
+ :param metadata: Dict of ks_meta items to pre-seed into the node.
+ Should include maas-metadata-url (URL for the metadata service)
+ and maas-metadata-credentials (OAuth token for accessing the
+ metadata service). The maas-metadata-credentials entry details
+ oauth_consumer_key, oauth_token_key, and oauth_token_secret
+ encoded as a URL query section suitable for urlparse.parse_qs.
:return: The name of the new node.
"""
=== modified file 'src/provisioningserver/testing/fakeapi.py'
--- src/provisioningserver/testing/fakeapi.py 2012-02-14 14:10:14 +0000
+++ src/provisioningserver/testing/fakeapi.py 2012-02-28 14:53:45 +0000
@@ -88,9 +88,10 @@
self.profiles[name]["distro"] = distro
return name
- def add_node(self, name, profile):
+ def add_node(self, name, profile, metadata):
self.nodes[name]["profile"] = profile
self.nodes[name]["mac_addresses"] = []
+ self.nodes[name]["metadata"] = metadata
return name
def modify_distros(self, deltas):
=== modified file 'src/provisioningserver/tests/test_api.py'
--- src/provisioningserver/tests/test_api.py 2012-02-14 14:01:33 +0000
+++ src/provisioningserver/tests/test_api.py 2012-02-28 14:53:45 +0000
@@ -14,6 +14,11 @@
__metaclass__ = type
__all__ = []
+from abc import (
+ ABCMeta,
+ abstractmethod,
+ )
+
from provisioningserver.api import (
cobbler_to_papi_distro,
cobbler_to_papi_node,
@@ -22,6 +27,7 @@
postprocess_mapping,
ProvisioningAPI,
)
+from provisioningserver.cobblerclient import CobblerSystem
from provisioningserver.interfaces import IProvisioningAPI
from provisioningserver.testing.fakeapi import FakeAsynchronousProvisioningAPI
from provisioningserver.testing.fakecobbler import make_fake_cobbler_session
@@ -185,14 +191,32 @@
self.assertEqual(expected, observed)
-class TestProvisioningAPI(TestCase):
- """Tests for `provisioningserver.api.ProvisioningAPI`."""
+class ProvisioningAPITestScenario:
+ """Tests for `provisioningserver.api.ProvisioningAPI`.
+
+ Abstract base class. To exercise these tests, derive a test case from
+ this class as well as from TestCase. Provide it with a
+ get_provisioning_api method that returns a ProvisioningAPI implementation
+ that you want to test against.
+ """
+
+ __metaclass__ = ABCMeta
run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
+ @abstractmethod
def get_provisioning_api(self):
- session = make_fake_cobbler_session()
- return ProvisioningAPI(session)
+ """Create a real, or faked, ProvisoningAPI to run tests against.
+
+ Override this in the test case that exercises this scenario.
+ """
+
+ def fake_metadata(self):
+ """Produce fake metadata parameters for adding a node."""
+ return {
+ 'maas-metadata-url': 'http://localhost:8000/metadata/',
+ 'maas-metadata-credentials': 'Fake metadata credentials',
+ }
def test_ProvisioningAPI_interfaces(self):
papi = self.get_provisioning_api()
@@ -219,10 +243,9 @@
def test_add_node(self):
# Create a system/node via the Provisioning API.
papi = self.get_provisioning_api()
- distro = yield papi.add_distro(
- "distro", "an_initrd", "a_kernel")
+ distro = yield papi.add_distro("distro", "an_initrd", "a_kernel")
profile = yield papi.add_profile("profile", distro)
- node = yield papi.add_node("node", profile)
+ node = yield papi.add_node("node", profile, self.fake_metadata())
self.assertEqual("node", node)
@inlineCallbacks
@@ -255,7 +278,8 @@
"distro", "an_initrd", "a_kernel")
profile1_name = yield papi.add_profile("profile1", distro_name)
profile2_name = yield papi.add_profile("profile2", distro_name)
- node_name = yield papi.add_node("node", profile1_name)
+ node_name = yield papi.add_node(
+ "node", profile1_name, self.fake_metadata())
yield papi.modify_nodes({node_name: {"profile": profile2_name}})
values = yield papi.get_nodes_by_name([node_name])
self.assertEqual(profile2_name, values[node_name]["profile"])
@@ -266,7 +290,8 @@
distro_name = yield papi.add_distro(
"distro", "an_initrd", "a_kernel")
profile_name = yield papi.add_profile("profile1", distro_name)
- node_name = yield papi.add_node("node", profile_name)
+ node_name = yield papi.add_node(
+ "node", profile_name, self.fake_metadata())
yield papi.modify_nodes(
{node_name: {"mac_addresses": ["55:55:55:55:55:55"]}})
values = yield papi.get_nodes_by_name([node_name])
@@ -279,7 +304,8 @@
distro_name = yield papi.add_distro(
"distro", "an_initrd", "a_kernel")
profile_name = yield papi.add_profile("profile1", distro_name)
- node_name = yield papi.add_node("node", profile_name)
+ node_name = yield papi.add_node(
+ "node", profile_name, self.fake_metadata())
mac_addresses_from = ["55:55:55:55:55:55", "66:66:66:66:66:66"]
mac_addresses_to = ["66:66:66:66:66:66"]
yield papi.modify_nodes(
@@ -322,7 +348,7 @@
distro = yield papi.add_distro(
"distro", "an_initrd", "a_kernel")
profile = yield papi.add_profile("profile", distro)
- node = yield papi.add_node("node", profile)
+ node = yield papi.add_node("node", profile, self.fake_metadata())
# Delete it again via the Provisioning API.
yield papi.delete_nodes_by_name([node])
# It has gone, checked via the Cobbler session.
@@ -366,25 +392,39 @@
self.assertEqual(expected, profiles)
@inlineCallbacks
- def test_get_nodes(self):
- papi = self.get_provisioning_api()
+ def test_get_nodes_returns_empty_dict_when_no_nodes_exist(self):
+ papi = self.get_provisioning_api()
+ nodes = yield papi.get_nodes()
+ self.assertEqual({}, nodes)
+
+ @inlineCallbacks
+ def test_get_nodes_returns_all_nodes(self):
+ papi = self.get_provisioning_api()
+ node_names = [self.getUniqueString() for counter in range(3)]
distro = yield papi.add_distro(
"distro", "an_initrd", "a_kernel")
profile = yield papi.add_profile("profile", distro)
- nodes = yield papi.get_nodes()
- self.assertEqual({}, nodes)
- # Create some nodes via the Provisioning API.
- expected = {}
- for num in xrange(3):
- name = self.getUniqueString()
- yield papi.add_node(name, profile)
- expected[name] = {
- 'name': name,
- 'profile': 'profile',
- 'mac_addresses': [],
- }
- nodes = yield papi.get_nodes()
- self.assertEqual(expected, nodes)
+ for name in node_names:
+ yield papi.add_node(name, profile, self.fake_metadata())
+ nodes = yield papi.get_nodes()
+ self.assertItemsEqual(node_names, nodes.keys())
+
+ @inlineCallbacks
+ def test_get_nodes_includes_node_attributes(self):
+ papi = self.get_provisioning_api()
+ distro = self.getUniqueString('distro')
+ initrd = self.getUniqueString('initrd')
+ kernel = self.getUniqueString('kernel')
+ distro = yield papi.add_distro(distro, initrd, kernel)
+ profile = yield papi.add_profile(self.getUniqueString(), distro)
+ node_name = self.getUniqueString()
+ yield papi.add_node(node_name, profile, self.fake_metadata())
+ nodes = yield papi.get_nodes()
+ self.assertItemsEqual([node_name], nodes.keys())
+ attrs = nodes[node_name]
+ self.assertEqual(
+ (node_name, profile, []),
+ (attrs['name'], attrs['profile'], attrs['mac_addresses']))
@inlineCallbacks
def test_get_nodes_by_name(self):
@@ -394,7 +434,7 @@
# Create a node via the Provisioning API.
distro = yield papi.add_distro("distro", "initrd", "kernel")
profile = yield papi.add_profile("profile", distro)
- yield papi.add_node("alice", profile)
+ yield papi.add_node("alice", profile, self.fake_metadata())
nodes = yield papi.get_nodes_by_name(["alice", "bob"])
# The response contains keys for all systems found.
self.assertSequenceEqual(["alice"], sorted(nodes))
@@ -427,7 +467,7 @@
papi = self.get_provisioning_api()
distro = yield papi.add_distro("distro", "initrd", "kernel")
profile = yield papi.add_profile("profile", distro)
- yield papi.add_node("alice", profile)
+ yield papi.add_node("alice", profile, self.fake_metadata())
yield papi.stop_nodes(["alice"])
# The test is that we get here without error.
pass
@@ -437,14 +477,43 @@
papi = self.get_provisioning_api()
distro = yield papi.add_distro("distro", "initrd", "kernel")
profile = yield papi.add_profile("profile", distro)
- yield papi.add_node("alice", profile)
+ yield papi.add_node("alice", profile, self.fake_metadata())
yield papi.start_nodes(["alice"])
# The test is that we get here without error.
pass
-class TestFakeProvisioningAPI(TestProvisioningAPI):
- """Test :class:`FakeAsynchronousProvisioningAPI`."""
-
- def get_provisioning_api(self):
+class TestProvisioningAPI(ProvisioningAPITestScenario, TestCase):
+ """Test :class:`ProvisioningAPI`.
+
+ Includes by inheritance all the tests in ProvisioningAPITestScenario.
+ """
+
+ def get_provisioning_api(self):
+ """Return a real ProvisioningAPI, but using a fake Cobbler session."""
+ return ProvisioningAPI(make_fake_cobbler_session())
+
+ @inlineCallbacks
+ def test_add_node_preseeds_metadata(self):
+ papi = self.get_provisioning_api()
+ distro = yield papi.add_distro("distro", "an_initrd", "a_kernel")
+ profile = yield papi.add_profile("profile", distro)
+ metadata = self.fake_metadata()
+ node_name = self.getUniqueString("node")
+ yield papi.add_node(node_name, profile, metadata)
+
+ attrs = yield CobblerSystem(papi.session, node_name).get_values()
+ preseed = attrs['ks_meta']['MAAS_PRESEED']
+ self.assertIn(metadata['maas-metadata-url'], preseed)
+ self.assertIn(metadata['maas-metadata-credentials'], preseed)
+
+
+class TestFakeProvisioningAPI(ProvisioningAPITestScenario, TestCase):
+ """Test :class:`FakeAsynchronousProvisioningAPI`.
+
+ Includes by inheritance all the tests in ProvisioningAPITestScenario.
+ """
+
+ def get_provisioning_api(self):
+ """Return a fake ProvisioningAPI."""
return FakeAsynchronousProvisioningAPI()
Follow ups
