launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07023
[Merge] lp:~jtv/maas/bug-974175 into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-974175 into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #974175 in MAAS: "Set MAAS_PRESEED on commissioning node"
https://bugs.launchpad.net/maas/+bug/974175
For more details, see:
https://code.launchpad.net/~jtv/maas/bug-974175/+merge/100998
Design as agreed with Daviey and Scott. Implementation as pre-imped with Gavin.
We need a special, different preseed for commissioning nodes. That was a bit of a downer, because it's currently the provisioning server's responsibility to compose a preseed variable and feed it to Cobbler in the right way. And alas, the provisioning server has no inkling of whether a node is commissioning or not. It works at a lower level.
This branch does not yet ensure that we actually use the new preseed for commissioning; that's work for a separate branch. In this branch I merely rearrange how and where we produce it.
→ The maasserver now composes the preseed string directly, and passes it to pserv's add_node().
→ And so the metadata dict it used to pass is supplanted by a preseed_data string.
→ Preseeds are computed for a specific node. It now produces the desired special format for Commissioning nodes.
Now maasserver knows how the preseed is put together, which is part of its contract with cloud-init; pserv knows how it fits in with ksmeta etc. and how to feed all that to Cobbler; and Cobbler knows how to feed it into the node.
Things get simpler, all in all, but having two formats is a bit warty and we still need to work out how to apply it. It may happen as a matter of course because we call add_node() whenever we save a node in maasserver, or it may need special effort. Test first.
--
https://code.launchpad.net/~jtv/maas/bug-974175/+merge/100998
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-974175 into lp:maas.
=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py 2012-04-05 14:50:39 +0000
+++ src/maasserver/provisioning.py 2012-04-05 16:04:19 +0000
@@ -187,26 +187,65 @@
return urljoin(maas_url, path)
-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.
- """
- # Circular import.
- from metadataserver.models import NodeKey
- token = NodeKey.objects.get_token_for_node(node)
+def compose_cloud_init_preseed(token):
+ """Compose the preseed value for a node in any state but Commissioning."""
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,
- }
+
+ # 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', get_metadata_server_url()),
+ ('maas-metadata-credentials', 'string', credentials),
+ ]
+
+ return '\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)
+
+
+def compose_commissioning_preseed(token):
+ """Compose the preseed value for a Commissioning node."""
+ template = dedent("""
+ #cloud-config
+ datasource:
+ MAAS:
+ metadata_url: %s
+ consumer_key: %s
+ token_key: %s
+ token_secret: %s
+ """.lstrip('\n'))
+ return template % (
+ get_metadata_server_url(),
+ token.consumer.key,
+ token.key,
+ token.secret,
+ )
+
+
+def compose_preseed(node):
+ """Put together preseed data for `node`.
+
+ :param node: The node to compose preseed data for.
+ :type node: Node
+ :return: Preseed data containing the information the node needs in order
+ to acces the metadata service: its URL and auth token.
+ """
+ # Circular import.
+ from metadataserver.models import NodeKey
+ token = NodeKey.objects.get_token_for_node(node)
+ if node.status == NODE_STATUS.COMMISSIONING:
+ return compose_commissioning_preseed(token)
+ else:
+ return compose_cloud_init_preseed(token)
def name_arch_in_cobbler_style(architecture):
@@ -245,9 +284,10 @@
papi = get_provisioning_api_proxy()
profile = select_profile_for_node(instance)
power_type = instance.get_effective_power_type()
- metadata = compose_metadata(instance)
+ preseed_data = compose_preseed(instance)
papi.add_node(
instance.system_id, instance.hostname,
+<<<<<<< TREE
profile, power_type, metadata)
# When the node is allocated this must not modify the netboot_enabled
# parameter. The node, once it has booted and installed itself, asks the
@@ -265,6 +305,9 @@
}
}
papi.modify_nodes(deltas)
+=======
+ profile, power_type, preseed_data)
+>>>>>>> MERGE-SOURCE
def set_node_mac_addresses(node):
=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py 2012-04-05 14:40:40 +0000
+++ src/maasserver/tests/test_provisioning.py 2012-04-05 16:04:19 +0000
@@ -12,7 +12,6 @@
__all__ = []
from abc import ABCMeta
-from urlparse import parse_qs
from xmlrpclib import Fault
from django.conf import settings
@@ -27,7 +26,7 @@
NODE_STATUS_CHOICES,
)
from maasserver.provisioning import (
- compose_metadata,
+ compose_preseed,
get_metadata_server_url,
name_arch_in_cobbler_style,
present_user_friendly_fault,
@@ -230,22 +229,31 @@
% Config.objects.get_config('maas_url').rstrip('/'),
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']))
+ def test_compose_preseed_includes_metadata_url(self):
+ node = factory.make_node(status=NODE_STATUS.READY)
+ self.assertIn(get_metadata_server_url(), compose_preseed(node))
+
+ def test_compose_preseed_for_commissioning_includes_metadata_url(self):
+ node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
+ self.assertIn(
+ 'metadata_url: %s\n' % get_metadata_server_url(),
+ compose_preseed(node))
+
+ def test_compose_preseed_includes_node_oauth_token(self):
+ node = factory.make_node(status=NODE_STATUS.READY)
+ preseed = compose_preseed(node)
+ token = NodeKey.objects.get_token_for_node(node)
+ self.assertIn('oauth_consumer_key=%s' % token.consumer.key, preseed)
+ self.assertIn('oauth_token_key=%s' % token.key, preseed)
+ self.assertIn('oauth_token_secret=%s' % token.secret, preseed)
+
+ def test_compose_preseed_for_commissioning_includes_auth_token(self):
+ node = factory.make_node(status=NODE_STATUS.COMMISSIONING)
+ preseed = compose_preseed(node)
+ token = NodeKey.objects.get_token_for_node(node)
+ self.assertIn('consumer_key: %s\n' % token.consumer.key, preseed)
+ self.assertIn('token_key: %s\n' % token.key, preseed)
+ self.assertIn('token_secret: %s\n' % token.secret, preseed)
def test_papi_xmlrpc_faults_are_reported_helpfully(self):
@@ -255,7 +263,7 @@
self.papi.patch('add_node', raise_fault)
with ExpectedException(MAASAPIException, ".*provisioning server.*"):
- self.papi.add_node('node', 'profile', 'power', {})
+ self.papi.add_node('node', 'profile', 'power', '')
def test_provisioning_errors_are_reported_helpfully(self):
@@ -265,7 +273,7 @@
self.papi.patch('add_node', raise_provisioning_error)
with ExpectedException(MAASAPIException, ".*Cobbler.*"):
- self.papi.add_node('node', 'profile', 'power', {})
+ self.papi.add_node('node', 'profile', 'power', '')
def test_present_user_friendly_fault_describes_pserv_fault(self):
self.assertIn(
=== modified file 'src/provisioningserver/api.py'
--- src/provisioningserver/api.py 2012-04-05 13:58:19 +0000
+++ src/provisioningserver/api.py 2012-04-05 16:04:19 +0000
@@ -162,18 +162,6 @@
pass # No change.
-# 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)
@@ -203,13 +191,13 @@
returnValue(profile.name)
@inlineCallbacks
- def add_node(self, name, hostname, profile, power_type, metadata):
+ def add_node(self, name, hostname, profile, power_type, preseed_data):
assert isinstance(name, basestring)
assert isinstance(hostname, basestring)
assert isinstance(profile, basestring)
assert power_type in (POWER_TYPE.VIRSH, POWER_TYPE.WAKE_ON_LAN)
- assert isinstance(metadata, dict)
- preseed = b64encode(metadata_preseed % metadata)
+ assert isinstance(preseed_data, basestring)
+ preseed = b64encode(preseed_data)
attributes = {
"hostname": hostname,
"profile": profile,
=== modified file 'src/provisioningserver/interfaces.py'
--- src/provisioningserver/interfaces.py 2012-03-29 10:18:58 +0000
+++ src/provisioningserver/interfaces.py 2012-04-05 16:04:19 +0000
@@ -50,19 +50,17 @@
:return: The name of the new profile.
"""
- def add_node(name, hostname, profile, power_type, metadata):
+ def add_node(name, hostname, profile, power_type, preseed_data):
"""Add a node with the given `name`.
:param hostname: The fully-qualified hostname for the node.
:param profile: Name of profile to associate the node with.
:param power_type: A choice of power-control method, as in
:class:`POWER_TYPE`.
- :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.
+ :param preseed_data: Data to pre-seed into the node as MAAS_PRESEED.
+ Should include the URL for the metadata service, and credentials
+ for accessing it. The credentials consist of oauth_consumer_key,
+ oauth_token_key, and oauth_token_secret.
:return: The name of the new node.
"""
=== modified file 'src/provisioningserver/testing/factory.py'
--- src/provisioningserver/testing/factory.py 2012-04-04 13:40:05 +0000
+++ src/provisioningserver/testing/factory.py 2012-04-05 16:04:19 +0000
@@ -10,7 +10,6 @@
__metaclass__ = type
__all__ = [
- 'fake_node_metadata',
'ProvisioningFakeFactory',
]
@@ -34,14 +33,6 @@
return next(names)
-def fake_node_metadata():
- """Produce fake metadata parameters for adding a node."""
- return {
- 'maas-metadata-url': 'http://%s:5240/metadata/' % fake_name(),
- 'maas-metadata-credentials': 'fake/%s' % fake_name(),
- }
-
-
class ProvisioningFakeFactory:
"""Mixin for test cases: factory of fake provisioning objects.
@@ -110,14 +101,12 @@
@inlineCallbacks
def add_node(self, papi, name=None, hostname=None, profile_name=None,
- power_type=None, metadata=None):
+ power_type=None, preseed_data=None):
"""Creates a new node object via `papi`.
Arranges for it to be deleted during test clean-up. If `name` is not
specified, `fake_name` will be called to obtain one. If `profile_name`
- is not specified, one will be obtained by calling `add_profile`. If
- `metadata` is not specified, it will be obtained by calling
- `fake_node_metadata`.
+ is not specified, one will be obtained by calling `add_profile`.
"""
if name is None:
name = fake_name()
@@ -127,10 +116,10 @@
profile_name = yield self.add_profile(papi)
if power_type is None:
power_type = POWER_TYPE.WAKE_ON_LAN
- if metadata is None:
- metadata = fake_node_metadata()
+ if preseed_data is None:
+ preseed_data = ""
node_name = yield papi.add_node(
- name, hostname, profile_name, power_type, metadata)
+ name, hostname, profile_name, power_type, preseed_data)
self.addCleanup(
self.clean_up_objects,
papi.delete_nodes_by_name,
=== modified file 'src/provisioningserver/testing/fakeapi.py'
--- src/provisioningserver/testing/fakeapi.py 2012-04-05 14:05:52 +0000
+++ src/provisioningserver/testing/fakeapi.py 2012-04-05 16:04:19 +0000
@@ -88,11 +88,11 @@
self.profiles[name]["distro"] = distro
return name
- def add_node(self, name, hostname, profile, power_type, metadata):
+ def add_node(self, name, hostname, profile, power_type, preseed_data):
self.nodes[name]["hostname"] = hostname
self.nodes[name]["profile"] = profile
self.nodes[name]["mac_addresses"] = []
- self.nodes[name]["metadata"] = metadata
+ self.nodes[name]["preseed_data"] = preseed_data
self.nodes[name]["power_type"] = power_type
return name
=== modified file 'src/provisioningserver/tests/test_api.py'
--- src/provisioningserver/tests/test_api.py 2012-04-05 13:58:19 +0000
+++ src/provisioningserver/tests/test_api.py 2012-04-05 16:04:19 +0000
@@ -34,10 +34,7 @@
from provisioningserver.cobblerclient import CobblerSystem
from provisioningserver.enum import POWER_TYPE
from provisioningserver.interfaces import IProvisioningAPI
-from provisioningserver.testing.factory import (
- fake_node_metadata,
- ProvisioningFakeFactory,
- )
+from provisioningserver.testing.factory import ProvisioningFakeFactory
from provisioningserver.testing.fakeapi import FakeAsynchronousProvisioningAPI
from provisioningserver.testing.fakecobbler import make_fake_cobbler_session
from provisioningserver.testing.realcobbler import RealCobbler
@@ -626,15 +623,13 @@
return ProvisioningAPI(make_fake_cobbler_session())
@inlineCallbacks
- def test_add_node_preseeds_metadata(self):
+ def test_add_node_provides_preseed(self):
papi = self.get_provisioning_api()
- metadata = fake_node_metadata()
- node_name = yield self.add_node(papi, metadata=metadata)
+ preseed_data = factory.getRandomString()
+ node_name = yield self.add_node(papi, preseed_data=preseed_data)
attrs = yield CobblerSystem(papi.session, node_name).get_values()
- preseed = attrs['ks_meta']['MAAS_PRESEED']
- preseed = b64decode(preseed)
- self.assertIn(metadata['maas-metadata-url'], preseed)
- self.assertIn(metadata['maas-metadata-credentials'], preseed)
+ self.assertEqual(
+ preseed_data, b64decode(attrs['ks_meta']['MAAS_PRESEED']))
class TestProvisioningAPIWithRealCobbler(ProvisioningAPITests,
