Merge lp:~jtv/maas/configure-dhcpv6 into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Implement RPC call to configure DHCPv6 server. This includes Gavin's patch to the live cluster-service fixture, and his help getting the tests working.

Discussed with Julian. It bugs us that the front-end may be waiting for cluster controllers, so filed this as bug 1361590 for further discussion.

Jeroen

[Gavin Panella (allenap)]

Tip top.

review: Approve

[Jeroen T. Vermeulen (jtv)]

Holding this branch back from landing because it could break things, which we're not supposed to do at the moment.

[Julian Edwards (julian-edwards)]

On Tuesday 26 August 2014 12:58:22 you wrote:
> > + client = getClientFor(nodegroup.uuid)
>
> Might be worth noting in the docstring that this can raise
> NoConnectionsAvailable.

Actually I think it should be catching them here and using the event log to
log errors. However as I feared would happen, the event log is node-based
which doesn't suit everything we need to log.

Maybe we need a new type of event log that logs system wide errors that need
to be in the admin's face, and add some UI for it.

At the very least, catch the error here and report it, otherwise pages like
the cluster interface edit page would 500 when clusters are down.

[Jeroen T. Vermeulen (jtv)]

At last got this through Q/A, after discovering that the problems were caused by bug 1363105. The IPv6 side may not be complete yet, but it doesn't appear to break IPv4, so I'm landing. A node can ping another node by name, which shows that DHCP tells it how to reach the DNS server.

=== modified file 'src/maasserver/dhcp.py'
--- src/maasserver/dhcp.py	2014-08-26 07:58:55 +0000
+++ src/maasserver/dhcp.py	2014-08-29 02:35:01 +0000
@@ -21,12 +21,15 @@
 from django.conf import settings
 from maasserver.enum import NODEGROUP_STATUS
 from maasserver.models import Config
+from maasserver.rpc import getClientFor
 from netaddr import IPAddress
+from provisioningserver.rpc.cluster import ConfigureDHCPv6
 from provisioningserver.tasks import (
     restart_dhcp_server,
     stop_dhcp_server,
     write_dhcp_config,
     )
+from provisioningserver.utils.twisted import synchronous
 
 
 def split_ipv4_ipv6_interfaces(interfaces):
@@ -93,9 +96,32 @@
         queue=nodegroup.work_queue, kwargs=task_kwargs)
 
 
+@synchronous
 def configure_dhcpv6(nodegroup, interfaces, dns_server, ntp_server):
-    """Write DHCPv6 configuration and restart the DHCPv6 server."""
-    # TODO: Implement.
+    """Write DHCPv6 configuration and restart the DHCPv6 server.
+
+    Delegates the work to the cluster controller, and waits for it
+    to complete.
+
+    :raise NoConnectionsAvailable: if the region controller could not get
+        an RPC connection to the cluster controller.
+    :raise CannotConfigureDHCP: if configuration could not be written, or
+        restart of the DHCP server fails.
+    """
+    # XXX jtv 2014-08-26 bug=1361590: UI/API requests to update cluster
+    # interfaces will block on this.  We may need an asynchronous error
+    # backchannel.
+    subnets = [
+        make_subnet_config(interface, dns_server, ntp_server)
+        for interface in interfaces
+        ]
+    client = getClientFor(nodegroup.uuid)
+    # XXX jtv 2014-08-26 bug=1361673: If this fails remotely, the error
+    # needs to be reported gracefully to the caller.
+    call = client(
+        ConfigureDHCPv6, omapi_key=nodegroup.dhcp_key, subnet_configs=subnets)
+    # Keep the timeout short: the UI may be waiting for completion.
+    call.wait(5)
 
 
 def configure_dhcp(nodegroup):

=== modified file 'src/maasserver/rpc/testing/fixtures.py'
--- src/maasserver/rpc/testing/fixtures.py	2014-08-26 09:23:08 +0000
+++ src/maasserver/rpc/testing/fixtures.py	2014-08-29 02:35:01 +0000
@@ -28,6 +28,7 @@
 from maasserver import eventloop
 from maasserver.enum import NODEGROUP_STATUS
 from maasserver.models.nodegroup import NodeGroup
+from maasserver.rpc import getClientFor
 from maasserver.rpc.regionservice import RegionServer
 from maasserver.testing.eventloop import (
     RegionEventLoopFixture,
@@ -317,4 +318,9 @@
         ident_response = {"ident": nodegroup.uuid.decode("ascii")}
         protocol.Identify.side_effect = (
             lambda protocol: defer.succeed(ident_response.copy()))
-        return self.addCluster(protocol).wait(10)
+        self.addCluster(protocol).wait(10)
+        # The connection is now established, but there is a brief handshake
+        # that takes place immediately upon connection.  We wait for that to
+        # finish before returning.
+        getClientFor(nodegroup.uuid, timeout=5)
+        return protocol

=== modified file 'src/maasserver/tests/test_dhcp.py'
--- src/maasserver/tests/test_dhcp.py	2014-08-26 08:13:11 +0000
+++ src/maasserver/tests/test_dhcp.py	2014-08-29 02:35:01 +0000
@@ -21,6 +21,7 @@
 from maasserver.dhcp import (
     configure_dhcp,
     configure_dhcpv4,
+    configure_dhcpv6,
     make_subnet_config,
     split_ipv4_ipv6_interfaces,
     )
@@ -30,6 +31,11 @@
     NODEGROUPINTERFACE_MANAGEMENT,
     )
 from maasserver.models import Config
+from maasserver.rpc.testing.fixtures import MockLiveRegionToClusterRPCFixture
+from maasserver.testing.eventloop import (
+    RegionEventLoopFixture,
+    RunningEventLoopFixture,
+    )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from maastesting.celery import CeleryFixture
@@ -43,6 +49,7 @@
     IPNetwork,
     )
 from provisioningserver import tasks
+from provisioningserver.rpc.cluster import ConfigureDHCPv6
 from testresources import FixtureResource
 from testtools.matchers import (
     ContainsAll,
@@ -297,6 +304,37 @@
         self.assertEqual(nodegroup.work_queue, kwargs['options']['queue'])
 
 
+class TestConfigureDHCPv6(MAASServerTestCase):
+    """Tests for `configure_dhcpv6`."""
+
+    def prepare_rpc(self, nodegroup):
+        """Set up test case for speaking RPC to `nodegroup`.
+
+        :param nodegroup: A cluster.  It will "run" a mock RPC service.
+        :return: Protocol.
+        """
+        self.useFixture(RegionEventLoopFixture('rpc'))
+        self.useFixture(RunningEventLoopFixture())
+        fixture = self.useFixture(MockLiveRegionToClusterRPCFixture())
+        return fixture.makeCluster(nodegroup, ConfigureDHCPv6)
+
+    def test__configures_dhcpv6(self):
+        nodegroup = factory.make_node_group(
+            status=NODEGROUP_STATUS.ACCEPTED,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP,
+            dhcp_key=factory.make_name('key'),
+            network=factory.make_ipv6_network())
+        protocol = self.prepare_rpc(nodegroup)
+
+        configure_dhcpv6(
+            nodegroup, [], factory.make_name('dns'), factory.make_name('ntp'))
+
+        self.assertThat(
+            protocol.ConfigureDHCPv6,
+            MockCalledOnceWith(
+                ANY, omapi_key=nodegroup.dhcp_key, subnet_configs=[]))
+
+
 class TestConfigureDHCP(MAASServerTestCase):
     """Tests for `configure_dhcp`."""
 
@@ -455,103 +493,116 @@
         )
 
     def test_dhcp_config_gets_written_when_nodegroup_becomes_active(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
+        self.patch(dhcp, 'configure_dhcpv6')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.PENDING,
             management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
         self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(dhcp, 'write_dhcp_config')
         nodegroup.accept()
-        self.assertEqual(1, dhcp.write_dhcp_config.apply_async.call_count)
+        self.assertThat(
+            configure_dhcpv4, MockCalledOnceWith(nodegroup, ANY, ANY, ANY))
 
     def test_dhcp_config_gets_written_when_nodegroup_name_changes(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
+        self.patch(dhcp, 'configure_dhcpv6')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.ACCEPTED,
             management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
         self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(dhcp, 'write_dhcp_config')
-        new_name = factory.make_name('dns name'),
 
-        nodegroup.name = new_name
+        nodegroup.name = factory.make_name('domain')
         nodegroup.save()
 
-        self.assertEqual(1, dhcp.write_dhcp_config.apply_async.call_count)
+        self.assertThat(
+            configure_dhcpv4, MockCalledOnceWith(nodegroup, ANY, ANY, ANY))
 
     def test_dhcp_config_gets_written_when_interface_IP_changes(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
+        self.patch(dhcp, 'configure_dhcpv6')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.ACCEPTED,
             management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
         [interface] = nodegroup.nodegroupinterface_set.all()
         self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(dhcp, 'write_dhcp_config')
 
         interface.ip = factory.pick_ip_in_network(
             interface.network, but_not=[interface.ip])
         interface.save()
 
-        self.assertEqual(1, dhcp.write_dhcp_config.apply_async.call_count)
+        self.assertThat(
+            configure_dhcpv4, MockCalledOnceWith(nodegroup, ANY, ANY, ANY))
 
     def test_dhcp_config_gets_written_when_interface_management_changes(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
+        self.patch(dhcp, 'configure_dhcpv6')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.ACCEPTED,
             management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
         [interface] = nodegroup.nodegroupinterface_set.all()
         self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(dhcp, 'write_dhcp_config')
 
         interface.management = NODEGROUPINTERFACE_MANAGEMENT.DHCP
         interface.save()
 
-        self.assertEqual(1, dhcp.write_dhcp_config.apply_async.call_count)
+        self.assertThat(
+            configure_dhcpv4, MockCalledOnceWith(nodegroup, ANY, ANY, ANY))
 
     def test_dhcp_config_gets_written_when_interface_name_changes(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
+        self.patch(dhcp, 'configure_dhcpv6')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.ACCEPTED,
             management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
         [interface] = nodegroup.get_managed_interfaces()
         self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(dhcp, 'write_dhcp_config')
 
         interface.interface = factory.make_name('itf')
         interface.save()
 
-        self.assertEqual(1, dhcp.write_dhcp_config.apply_async.call_count)
+        self.assertThat(
+            configure_dhcpv4, MockCalledOnceWith(nodegroup, ANY, ANY, ANY))
 
     def test_dhcp_config_gets_written_when_netmask_changes(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
+        self.patch(dhcp, 'configure_dhcpv6')
         network = factory.getRandomNetwork(slash='255.255.255.0')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.ACCEPTED, network=network,
             management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
         [interface] = nodegroup.get_managed_interfaces()
         self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(dhcp, 'write_dhcp_config')
 
         interface.subnet_mask = '255.255.0.0'
         interface.save()
 
-        self.assertEqual(1, dhcp.write_dhcp_config.apply_async.call_count)
+        self.assertThat(
+            configure_dhcpv4, MockCalledOnceWith(nodegroup, ANY, ANY, ANY))
 
     def test_dhcp_config_gets_written_when_interface_router_ip_changes(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
+        self.patch(dhcp, 'configure_dhcpv6')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.ACCEPTED,
             management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
         [interface] = nodegroup.get_managed_interfaces()
         self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(dhcp, 'write_dhcp_config')
-        new_router_ip = factory.pick_ip_in_network(
+
+        interface.router_ip = factory.pick_ip_in_network(
             interface.network, but_not=[interface.router_ip])
-
-        interface.router_ip = new_router_ip
         interface.save()
 
-        self.assertEqual(1, dhcp.write_dhcp_config.apply_async.call_count)
+        self.assertThat(
+            configure_dhcpv4, MockCalledOnceWith(nodegroup, ANY, ANY, ANY))
 
     def test_dhcp_config_gets_written_when_ip_range_changes(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
+        self.patch(dhcp, 'configure_dhcpv6')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.ACCEPTED,
             management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
         [interface] = nodegroup.get_managed_interfaces()
         self.patch(settings, "DHCP_CONNECT", True)
-        self.patch(dhcp, 'write_dhcp_config')
 
         interface.ip_range_low = unicode(
             IPAddress(interface.ip_range_low) + 1)
@@ -559,25 +610,26 @@
             IPAddress(interface.ip_range_high) - 1)
         interface.save()
 
-        self.assertEqual(1, dhcp.write_dhcp_config.apply_async.call_count)
+        self.assertThat(
+            configure_dhcpv4, MockCalledOnceWith(nodegroup, ANY, ANY, ANY))
 
     def test_dhcp_config_is_not_written_when_foreign_dhcp_changes(self):
+        configure_dhcpv4 = self.patch(dhcp, 'configure_dhcpv4')
         nodegroup = factory.make_node_group(
             status=NODEGROUP_STATUS.ACCEPTED,
             management=NODEGROUPINTERFACE_MANAGEMENT.DHCP)
         [interface] = nodegroup.get_managed_interfaces()
-        self.patch(dhcp, 'write_dhcp_config')
         self.patch(settings, "DHCP_CONNECT", True)
 
-        interface.foreign_dhcp = factory.pick_ip_in_network(
-            interface.network)
+        interface.foreign_dhcp = factory.pick_ip_in_network(interface.network)
         interface.save()
 
-        self.assertEqual([], dhcp.write_dhcp_config.apply_async.mock_calls)
+        self.assertThat(configure_dhcpv4, MockNotCalled())
 
     def test_dhcp_config_gets_written_when_ntp_server_changes(self):
         # When the "ntp_server" Config item is changed, check that all
         # nodegroups get their DHCP config re-written.
+        self.patch(dhcp, 'configure_dhcpv6')
         num_active_nodegroups = random.randint(1, 10)
         num_inactive_nodegroups = random.randint(1, 10)
         for x in range(num_active_nodegroups):