Merge lp:~allenap/maas/rpc-get-preseed-data into lp:~maas-committers/maas/trunk

[Gavin Panella]

New RPC call GetPreseedData

[Blake Rouse (blake-rouse)]

Looks good. One comment inline.

You did not update the docstring for OperatingSystem for compose_preseed. Would be good to update it for Node and Token objects are now passed to that method.

review: Approve

[Gavin Panella (allenap)]

Thanks for the review! Good catches on the docstrings.

=== modified file 'src/maasserver/compose_preseed.py'
--- src/maasserver/compose_preseed.py	2014-06-20 13:38:39 +0000
+++ src/maasserver/compose_preseed.py	2014-07-14 21:39:26 +0000
@@ -20,7 +20,11 @@
 
 from maasserver.enum import PRESEED_TYPE
 from maasserver.utils import absolute_reverse
-from provisioningserver.drivers.osystem import OperatingSystemRegistry
+from provisioningserver.drivers.osystem import (
+    Node as OSystemNode,
+    OperatingSystemRegistry,
+    Token as OSystemToken,
+    )
 import yaml
 
 
@@ -108,9 +112,14 @@
         if preseed_type == PRESEED_TYPE.CURTIN:
             metadata_url = absolute_reverse(
                 'curtin-metadata', base_url=base_url)
+        node_for_osystem = OSystemNode(
+            node.system_id, node.hostname)
+        token_for_osystem = OSystemToken(
+            token.consumer.key, token.key, token.secret)
         try:
             return osystem.compose_preseed(
-                preseed_type, node, token, metadata_url)
+                preseed_type, node_for_osystem, token_for_osystem,
+                metadata_url)
         except NotImplementedError:
             pass
 

=== modified file 'src/maasserver/tests/test_compose_preseed.py'
--- src/maasserver/tests/test_compose_preseed.py	2014-06-11 15:26:45 +0000
+++ src/maasserver/tests/test_compose_preseed.py	2014-07-14 21:39:26 +0000
@@ -126,10 +126,13 @@
         mock_compose = self.patch(osystem, 'compose_preseed')
         node = factory.make_node(
             osystem=osystem.name, status=NODE_STATUS.READY)
-
         token = NodeKey.objects.get_token_for_node(node)
         url = absolute_reverse('curtin-metadata')
         compose_preseed(PRESEED_TYPE.CURTIN, node)
         self.assertThat(
             mock_compose,
-            MockCalledOnceWith(PRESEED_TYPE.CURTIN, node, token, url))
+            MockCalledOnceWith(
+                PRESEED_TYPE.CURTIN,
+                (node.system_id, node.hostname),
+                (token.consumer.key, token.key, token.secret),
+                url))

=== modified file 'src/provisioningserver/drivers/osystem/__init__.py'
--- src/provisioningserver/drivers/osystem/__init__.py	2014-07-11 14:07:08 +0000
+++ src/provisioningserver/drivers/osystem/__init__.py	2014-07-14 21:39:26 +0000
@@ -13,8 +13,10 @@
 
 __metaclass__ = type
 __all__ = [
+    "Node",
     "OperatingSystem",
     "OperatingSystemRegistry",
+    "Token",
     ]
 
 from abc import (
@@ -22,6 +24,7 @@
     abstractmethod,
     abstractproperty,
     )
+from collections import namedtuple
 
 from provisioningserver.utils.registry import Registry
 
@@ -36,6 +39,16 @@
     XINSTALL = 'xinstall'
 
 
+# A cluster-side representation of a Node, relevant to the osystem code,
+# with only minimal fields.
+Node = namedtuple("Node", ("system_id", "hostname"))
+
+
+# A cluster-side representation of a Token, relevant to the osystem code,
+# with only minimal fields.
+Token = namedtuple("Token", ("consumer_key", "token_key", "token_secret"))
+
+
 class OperatingSystem:
     """Skeleton for an operating system."""
 
@@ -139,10 +152,12 @@
         """Composes the preseed for the given node.
 
         :param preseed_type: Preseed type to compose.
-        :param node: Node preseed needs generating.
-        :param token: OAuth token for url.
-        :param metadata_url: Metdata url for node.
-        :returns: Preseed for node.
+        :param node: Node preseed needs generating for.
+        :type node: :py:class:`Node`
+        :param token: OAuth token for URL.
+        :type token: :py:class:`Token`
+        :param metadata_url: Metdata URL for node.
+        :returns: Preseed data for node.
         :raise:
             NotImplementedError: doesn't implement a custom preseed
         """

=== modified file 'src/provisioningserver/drivers/osystem/tests/test_windows.py'
--- src/provisioningserver/drivers/osystem/tests/test_windows.py	2014-07-10 17:08:58 +0000
+++ src/provisioningserver/drivers/osystem/tests/test_windows.py	2014-07-14 21:39:26 +0000
@@ -18,7 +18,10 @@
 
 from maastesting.factory import factory
 from maastesting.testcase import MAASTestCase
-from mock import MagicMock
+from provisioningserver.drivers.osystem import (
+    Node,
+    Token,
+    )
 from provisioningserver.drivers.osystem.windows import (
     BOOT_IMAGE_PURPOSE,
     Config,
@@ -135,9 +138,10 @@
             machine = factory.make_name('hostname')
             dns = factory.make_name('dns')
             hostname = '%s.%s' % (machine, dns)
-        node = MagicMock()
-        node.hostname = hostname
-        return node
+        return Node(
+            system_id=factory.make_name("system_id"),
+            hostname=hostname,
+        )
 
     def make_token(self, consumer_key=None, token_key=None, token_secret=None):
         if consumer_key is None:
@@ -146,11 +150,11 @@
             token_key = factory.make_name('token_key')
         if token_secret is None:
             token_secret = factory.make_name('secret_key')
-        token = MagicMock()
-        token.consumer.key = consumer_key
-        token.key = token_key
-        token.secret = token_secret
-        return token
+        return Token(
+            consumer_key=consumer_key,
+            token_key=token_key,
+            token_secret=token_secret,
+        )
 
     def test_compose_pressed_not_implemented_for_curtin(self):
         osystem = WindowsOS()

=== modified file 'src/provisioningserver/drivers/osystem/windows.py'
--- src/provisioningserver/drivers/osystem/windows.py	2014-07-10 17:08:58 +0000
+++ src/provisioningserver/drivers/osystem/windows.py	2014-07-14 21:39:26 +0000
@@ -98,9 +98,9 @@
         credentials = {
             'maas_metadata_url': metadata_url,
             'maas_oauth_consumer_secret': '',
-            'maas_oauth_consumer_key': token.consumer.key,
-            'maas_oauth_token_key': token.key,
-            'maas_oauth_token_secret': token.secret,
+            'maas_oauth_consumer_key': token.consumer_key,
+            'maas_oauth_token_key': token.token_key,
+            'maas_oauth_token_secret': token.token_secret,
             'hostname': hostname,
             }
         return credentials

=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2014-07-09 21:45:26 +0000
+++ src/provisioningserver/rpc/cluster.py	2014-07-14 21:39:26 +0000
@@ -24,7 +24,10 @@
 ]
 
 from provisioningserver.rpc import exceptions
-from provisioningserver.rpc.arguments import StructureAsJSON
+from provisioningserver.rpc.arguments import (
+    ParsedURL,
+    StructureAsJSON,
+    )
 from provisioningserver.rpc.common import Identify
 from twisted.protocols import amp
 
@@ -119,3 +122,30 @@
         exceptions.NoSuchOperatingSystem: (
             b"NoSuchOperatingSystem"),
     }
+
+
+class GetPreseedData(amp.Command):
+    """Get OS-specific preseed data.
+
+    :since: 1.7
+    """
+
+    arguments = [
+        (b"osystem", amp.Unicode()),
+        (b"preseed_type", amp.Unicode()),
+        (b"node_system_id", amp.Unicode()),
+        (b"node_hostname", amp.Unicode()),
+        (b"consumer_key", amp.Unicode()),
+        (b"token_key", amp.Unicode()),
+        (b"token_secret", amp.Unicode()),
+        (b"metadata_url", ParsedURL()),
+    ]
+    response = [
+        (b"data", StructureAsJSON()),
+    ]
+    errors = {
+        exceptions.NoSuchOperatingSystem: (
+            b"NoSuchOperatingSystem"),
+        NotImplementedError: (
+            b"NotImplementedError"),
+    }

=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2014-07-09 20:59:05 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2014-07-14 21:39:26 +0000
@@ -41,6 +41,7 @@
 from provisioningserver.rpc.interfaces import IConnection
 from provisioningserver.rpc.osystems import (
     gen_operating_systems,
+    get_preseed_data,
     validate_license_key,
     )
 from twisted.application.internet import (
@@ -130,6 +131,21 @@
         """
         return {"is_valid": validate_license_key(osystem, release, key)}
 
+    @cluster.GetPreseedData.responder
+    def get_preseed_data(
+            self, osystem, preseed_type, node_system_id, node_hostname,
+            consumer_key, token_key, token_secret, metadata_url):
+        """get_preseed_data()
+
+        Implementation of
+        :py:class:`~provisioningserver.rpc.cluster.GetPreseedData`.
+        """
+        return {
+            "data": get_preseed_data(
+                osystem, preseed_type, node_system_id, node_hostname,
+                consumer_key, token_key, token_secret, metadata_url),
+        }
+
     @amp.StartTLS.responder
     def get_tls_parameters(self):
         """get_tls_parameters()

=== modified file 'src/provisioningserver/rpc/osystems.py'
--- src/provisioningserver/rpc/osystems.py	2014-07-11 16:57:48 +0000
+++ src/provisioningserver/rpc/osystems.py	2014-07-14 21:39:26 +0000
@@ -17,7 +17,11 @@
     "validate_license_key",
 ]
 
-from provisioningserver.drivers.osystem import OperatingSystemRegistry
+from provisioningserver.drivers.osystem import (
+    Node,
+    OperatingSystemRegistry,
+    Token,
+    )
 from provisioningserver.rpc import exceptions
 
 
@@ -70,3 +74,26 @@
         raise exceptions.NoSuchOperatingSystem(osystem)
     else:
         return osystem.validate_license_key(release, key)
+
+
+def get_preseed_data(
+        osystem, preseed_type, node_system_id, node_hostname,
+        consumer_key, token_key, token_secret, metadata_url):
+    """Composes preseed data for the given node.
+
+    :param preseed_type: The preseed type being composed.
+    :param node: The node for which a preseed is being composed.
+    :param token: OAuth token for the metadata URL.
+    :param metadata_url: The metdata URL for the node.
+    :returns: Preseed data for the given node.
+    :raise NotImplementedError: when the specified operating system does
+        not require custom preseed data.
+    """
+    try:
+        osystem = OperatingSystemRegistry[osystem]
+    except KeyError:
+        raise exceptions.NoSuchOperatingSystem(osystem)
+    else:
+        return osystem.compose_preseed(
+            preseed_type, Node(node_system_id, node_hostname),
+            Token(consumer_key, token_key, token_secret), metadata_url)

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2014-07-11 16:57:48 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-07-14 21:39:26 +0000
@@ -18,6 +18,7 @@
 import json
 import os.path
 from random import randint
+from urlparse import urlparse
 
 from fixtures import EnvironmentVariable
 from maastesting.factory import factory
@@ -36,6 +37,10 @@
     )
 from provisioningserver.boot import tftppath
 from provisioningserver.boot.tests.test_tftppath import make_osystem
+from provisioningserver.drivers.osystem import (
+    OperatingSystem,
+    OperatingSystemRegistry,
+    )
 from provisioningserver.power_schema import JSON_POWER_TYPE_PARAMETERS
 from provisioningserver.rpc import (
     cluster,
@@ -761,3 +766,68 @@
         with ExpectedException(exceptions.NoSuchOperatingSystem):
             yield call_responder(
                 Cluster(), cluster.ValidateLicenseKey, arguments)
+
+
+class TestClusterProtocol_GetPreseedData(MAASTestCase):
+
+    run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
+
+    def make_arguments(self):
+        return {
+            "osystem": factory.make_name("osystem"),
+            "preseed_type": factory.make_name("preseed_type"),
+            "node_system_id": factory.make_name("system_id"),
+            "node_hostname": factory.make_name("hostname"),
+            "consumer_key": factory.make_name("consumer_key"),
+            "token_key": factory.make_name("token_key"),
+            "token_secret": factory.make_name("token_secret"),
+            "metadata_url": urlparse(
+                "https://%s/path/to/metadata" % factory.make_hostname()),
+        }
+
+    def test_is_registered(self):
+        protocol = Cluster()
+        responder = protocol.locateResponder(
+            cluster.GetPreseedData.commandName)
+        self.assertIsNot(responder, None)
+
+    @inlineCallbacks
+    def test_calls_get_preseed_data(self):
+        get_preseed_data = self.patch(clusterservice, "get_preseed_data")
+        get_preseed_data.return_value = factory.make_name("data")
+        arguments = self.make_arguments()
+        observed = yield call_responder(
+            Cluster(), cluster.GetPreseedData, arguments)
+        expected = {"data": get_preseed_data.return_value}
+        self.assertEqual(expected, observed)
+        # The arguments are passed to the responder positionally.
+        self.assertThat(get_preseed_data, MockCalledOnceWith(
+            arguments["osystem"], arguments["preseed_type"],
+            arguments["node_system_id"], arguments["node_hostname"],
+            arguments["consumer_key"], arguments["token_key"],
+            arguments["token_secret"], arguments["metadata_url"]))
+
+    @inlineCallbacks
+    def test_exception_when_os_does_not_exist(self):
+        # A remote NoSuchOperatingSystem exception is re-raised locally.
+        get_preseed_data = self.patch(
+            clusterservice, "get_preseed_data")
+        get_preseed_data.side_effect = exceptions.NoSuchOperatingSystem()
+        arguments = self.make_arguments()
+        with ExpectedException(exceptions.NoSuchOperatingSystem):
+            yield call_responder(
+                Cluster(), cluster.GetPreseedData, arguments)
+
+    @inlineCallbacks
+    def test_exception_when_preseed_not_implemented(self):
+        # A remote NotImplementedError exception is re-raised locally.
+        # Choose an operating system which has not overridden the
+        # default compose_preseed.
+        osystem_name = next(
+            osystem_name for osystem_name, osystem in OperatingSystemRegistry
+            if osystem.compose_preseed == OperatingSystem.compose_preseed)
+        arguments = self.make_arguments()
+        arguments["osystem"] = osystem_name
+        with ExpectedException(exceptions.NoSuchOperatingSystem):
+            yield call_responder(
+                Cluster(), cluster.GetPreseedData, arguments)

=== modified file 'src/provisioningserver/rpc/tests/test_osystems.py'
--- src/provisioningserver/rpc/tests/test_osystems.py	2014-07-11 20:16:44 +0000
+++ src/provisioningserver/rpc/tests/test_osystems.py	2014-07-14 21:39:26 +0000
@@ -128,3 +128,43 @@
         self.assertThat(
             os_specific_validate_license_key,
             MockCalledOnceWith(self.release, sentinel.key))
+
+
+class TestGetPreseedDataErrors(MAASTestCase):
+
+    def test_throws_exception_when_os_does_not_exist(self):
+        self.assertRaises(
+            exceptions.NoSuchOperatingSystem,
+            osystems.get_preseed_data, factory.make_name("no-such-os"),
+            sentinel.preseed_type, sentinel.node_system_id,
+            sentinel.node_hostname, sentinel.consumer_key,
+            sentinel.token_key, sentinel.token_secret,
+            sentinel.metadata_url)
+
+
+class TestGetPreseedData(MAASTestCase):
+
+    # Check for every OS.
+    scenarios = [
+        (osystem.name, {"osystem": osystem})
+        for _, osystem in OperatingSystemRegistry
+    ]
+
+    def test_get_preseed_data_calls_compose_preseed(self):
+        # get_preseed_data() calls compose_preseed() on the
+        # OperatingSystem instances.
+        os_specific_compose_preseed = self.patch(
+            self.osystem, "compose_preseed")
+        osystems.get_preseed_data(
+            self.osystem.name, sentinel.preseed_type,
+            sentinel.node_system_id, sentinel.node_hostname,
+            sentinel.consumer_key, sentinel.token_key,
+            sentinel.token_secret, sentinel.metadata_url)
+        self.assertThat(
+            os_specific_compose_preseed,
+            MockCalledOnceWith(
+                sentinel.preseed_type,
+                (sentinel.node_system_id, sentinel.node_hostname),
+                (sentinel.consumer_key, sentinel.token_key,
+                 sentinel.token_secret),
+                sentinel.metadata_url))