Merge lp:~jtv/maas/rpc-curtin-network-preseed into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Add (but do not call yet) an RPC call for composing a node's curtin network preseed.

This is tedious stuff. I've lost count of the number of pass-through functions I've had to define just to get my arguments to the right place. Some of my parameters did not fit the AMP mould, so I wrapped those into a JSON argument rather than come up with yet another data structure.

In my osystems test I still have a self.patch instead of self.patch_autospec. The reason is that self.patch_autospec broke for reasons that looked completely spurious. It told me that the call was receiving multiple values for the same keyword argument. How can that happen? I was passing **kwargs, where kwargs was a normal dict. But the errors were always for the arguments that were lists, never for the ones that were dicts. So I suspect the autospec does something clever along the lines of “if it's a list, expand it as multiple arguments.” The other test would have failed if this happened in real life, so I'm convinced this is a Mock bug.

Jeroen

[Andres Rodriguez (andreserl)]

lgtm! (but it is late and I might be missing any code correctness)

review: Approve

[Jeroen T. Vermeulen (jtv)]

Thanks for the review! I wasn't expecting that, but it reduces my dependency on the review backlog, and that is very nice.

=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2014-09-27 04:40:39 +0000
+++ src/provisioningserver/rpc/cluster.py	2014-10-01 01:47:10 +0000
@@ -185,6 +185,35 @@
     }
 
 
+class ComposeCurtinNetworkPreseed(amp.Command):
+    """Generate Curtin network preseed for a node.
+
+    :param osystem: Operating system identifier, e.g. `ubuntu`.
+    :param config: A dict detailing the network configuration:
+        `interfaces` maps to a list of pairs of interface name and MAC address.
+        `ips_mapping` maps to a dict which maps MAC addresses to lists of
+        IP addresses (at most one IPv4 and one IPv6 each) to be assigned to the
+        corresponding network interfaces.
+        `gateways_mapping` maps to a dict which maps MAC addresses to lists of
+        gateway IP addresses (at most one IPv4 and one IPv6) to be used by the
+        corresponding network interfaces.
+    :param disable_ipv4: Should IPv4 networking be disabled on the node?
+    :since: 1.7
+    """
+
+    arguments = [
+        (b"osystem", amp.Unicode()),
+        (b"config", StructureAsJSON()),
+        (b"disable_ipv4", amp.Boolean()),
+        ]
+    response = [
+        (b"data", StructureAsJSON()),
+        ]
+    errors = {
+        exceptions.NoSuchOperatingSystem: b"NoSuchOperatingSystem",
+    }
+
+
 class _Power(amp.Command):
     """Base class for power control commands.
 

=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2014-09-30 10:23:13 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2014-10-01 01:47:10 +0000
@@ -64,6 +64,7 @@
     start_monitors,
     )
 from provisioningserver.rpc.osystems import (
+    compose_curtin_network_preseed,
     gen_operating_systems,
     get_os_release_title,
     get_preseed_data,
@@ -200,6 +201,23 @@
                 consumer_key, token_key, token_secret, metadata_url),
         }
 
+    @cluster.ComposeCurtinNetworkPreseed.responder
+    def compose_curtin_network_preseed(self, osystem, config, disable_ipv4):
+        """compose_curtin_network_preseed()
+
+        Implementation of
+        :py:class:`~provisioningserver.rpc.cluster.ComposeCurtinNetworkPreseed`
+        """
+        interfaces = config.get('interfaces', [])
+        interfaces = [tuple(interface) for interface in interfaces]
+        ips_mapping = config.get('ips_mapping', {})
+        gateways_mapping = config.get('gateways_mapping', {})
+        return {
+            'data': compose_curtin_network_preseed(
+                osystem, interfaces, ips_mapping, gateways_mapping,
+                disable_ipv4=disable_ipv4),
+            }
+
     @log_call(level=logging.DEBUG)
     @cluster.PowerOn.responder
     def power_on(self, system_id, hostname, power_type, context):

=== modified file 'src/provisioningserver/rpc/osystems.py'
--- src/provisioningserver/rpc/osystems.py	2014-09-27 04:40:39 +0000
+++ src/provisioningserver/rpc/osystems.py	2014-10-01 01:47:10 +0000
@@ -115,3 +115,27 @@
             preseed_type, Node(node_system_id, node_hostname),
             Token(consumer_key, token_key, token_secret),
             metadata_url.geturl())
+
+
+def compose_curtin_network_preseed(os_name, interfaces, ips_mapping,
+                                   gateways_mapping, disable_ipv4):
+    """Compose Curtin network preseed for a node.
+
+    :param os_name: Identifying name of the operating system for which a
+        preseed should be generated.
+    :param interfaces: A list of interface/MAC pairs for the node.
+    :param ips_mapping: A dict mapping MAC addresses to containers of the
+        corresponding network interfaces' IP addresses.
+    :param gateways_mapping: A `defaultdict` mapping MAC addresses to
+        containers of the corresponding network interfaces' default gateways.
+    :param disable_ipv4: Should this node be installed without IPv4 networking?
+    :return: Preseed data, as JSON.
+    """
+    try:
+        osystem = OperatingSystemRegistry[os_name]
+    except KeyError:
+        raise exceptions.NoSuchOperatingSystem(os_name)
+    else:
+        return osystem.compose_curtin_network_preseed(
+            interfaces, ips_mapping, gateways_mapping,
+            disable_ipv4=disable_ipv4)

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2014-09-30 10:23:13 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-10-01 01:47:10 +0000
@@ -1043,6 +1043,55 @@
                 Cluster(), cluster.GetPreseedData, arguments)
 
 
+class TestClusterProtocol_ComposeCurtinNetworkPreseed(MAASTestCase):
+
+    run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)
+
+    def make_args(self, osystem=None):
+        if osystem is None:
+            osystem = factory.make_name('os')
+        mac = factory.make_mac_address()
+        return {
+            'osystem': osystem,
+            'config': {
+                'interfaces': [(factory.make_name('eth'), mac)],
+                'ips_mapping': {mac: [factory.make_ipv4_address()]},
+                'gateways_mapping': {mac: [factory.make_ipv4_address()]},
+                },
+            'disable_ipv4': factory.pick_bool(),
+            }
+
+    def test__is_registered(self):
+        protocol = Cluster()
+        responder = protocol.locateResponder(
+            cluster.ComposeCurtinNetworkPreseed.commandName)
+        self.assertIsNotNone(responder)
+
+    @inlineCallbacks
+    def test__calls_compose_curtin_network_preseed(self):
+        preseed = [factory.make_name('preseed')]
+        fake = self.patch_autospec(
+            clusterservice, 'compose_curtin_network_preseed')
+        fake.return_value = preseed
+        args = self.make_args()
+
+        response = yield call_responder(
+            Cluster(), cluster.ComposeCurtinNetworkPreseed, args)
+
+        self.expectThat(response, Equals({'data': preseed}))
+        self.expectThat(
+            fake,
+            MockCalledOnceWith(
+                args['osystem'], args['config'], args['disable_ipv4']))
+
+    @inlineCallbacks
+    def test__fails_for_unknown_OS(self):
+        args = self.make_args(osystem=factory.make_name('nonexistent-os'))
+        with ExpectedException(exceptions.NoSuchOperatingSystem):
+            yield call_responder(
+                Cluster(), cluster.ComposeCurtinNetworkPreseed, args)
+
+
 class TestClusterProtocol_PowerOn_PowerOff(MAASTestCase):
 
     run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)

=== modified file 'src/provisioningserver/rpc/tests/test_osystems.py'
--- src/provisioningserver/rpc/tests/test_osystems.py	2014-09-27 04:40:39 +0000
+++ src/provisioningserver/rpc/tests/test_osystems.py	2014-10-01 01:47:10 +0000
@@ -199,3 +199,41 @@
                 (sentinel.consumer_key, sentinel.token_key,
                  sentinel.token_secret),
                 metadata_url.geturl()))
+
+
+class TestComposeCurtinNetworkPreseed(MAASTestCase):
+
+    def make_args(self):
+        mac = factory.make_mac_address()
+        return {
+            'interfaces': [(factory.make_name('eth'), mac)],
+            'ips_mapping': {
+                mac: [
+                    factory.make_ipv4_address(),
+                    factory.make_ipv6_address(),
+                    ],
+                },
+            'gateways_mapping': {
+                mac: [
+                    factory.make_ipv4_address(),
+                    factory.make_ipv6_address(),
+                    ],
+                },
+            'disable_ipv4': factory.pick_bool(),
+            }
+
+    def test__forwards_to_OS_implementation(self):
+        args = self.make_args()
+        for os_name, osystem in OperatingSystemRegistry:
+            fake = self.patch(osystem, 'compose_curtin_network_preseed')
+            fake.return_value = factory.make_name('preseed-%s' % os_name)
+            self.assertEqual(
+                fake.return_value,
+                osystems.compose_curtin_network_preseed(os_name, **args))
+
+    def test__works_with_real_implementation(self):
+        ubuntu = OperatingSystemRegistry['ubuntu']
+        args = self.make_args()
+        self.assertEqual(
+            ubuntu.compose_curtin_network_preseed(**args),
+            osystems.compose_curtin_network_preseed('ubuntu', **args))