launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1047061 into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1047061 in MAAS: "dhcpd.conf next-server set to 127.0.0.1"
  https://bugs.launchpad.net/maas/+bug/1047061

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1047061/+merge/123500

Discussed briefly with Julian, but the realization that there's a can of worms in the master nodegroup's worker_ip came after.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1047061/+merge/123500
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/bug-1047061 into lp:maas.

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2012-09-04 15:06:20 +0000
+++ src/maasserver/models/nodegroup.py	2012-09-10 08:01:39 +0000
@@ -25,6 +25,7 @@
 from maasserver.dhcp import is_dhcp_management_enabled
 from maasserver.models.timestampedmodel import TimestampedModel
 from maasserver.refresh_worker import refresh_worker
+from maasserver.server_address import get_maas_facing_server_address
 from netaddr import IPAddress
 from piston.models import (
     KEY_SIZE,
@@ -149,13 +150,19 @@
         """Write the DHCP configuration file and restart the DHCP server."""
         # Circular imports.
         from maasserver.dns import get_dns_server_address
+
+        # Use the server's address (which is where the central TFTP
+        # server is) for the next_server setting.  We'll want to proxy
+        # it on the local worker later, and then we can use
+        # next_server=self.worker_ip.
+        next_server = get_maas_facing_server_address()
+
         subnet = str(
             IPAddress(self.ip_range_low) & IPAddress(self.subnet_mask))
         write_dhcp_config.delay(
-            subnet=subnet, next_server=self.worker_ip,
-            omapi_key=self.dhcp_key, subnet_mask=self.subnet_mask,
-            broadcast_ip=self.broadcast_ip, router_ip=self.router_ip,
-            dns_servers=get_dns_server_address(),
+            subnet=subnet, next_server=next_server, omapi_key=self.dhcp_key,
+            subnet_mask=self.subnet_mask, broadcast_ip=self.broadcast_ip,
+            router_ip=self.router_ip, dns_servers=get_dns_server_address(),
             ip_range_low=self.ip_range_low, ip_range_high=self.ip_range_high)
 
     def is_dhcp_enabled(self):

=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py	2012-09-05 13:30:21 +0000
+++ src/maasserver/tests/test_nodegroup.py	2012-09-10 08:01:39 +0000
@@ -12,9 +12,11 @@
 __metaclass__ = type
 __all__ = []
 
+from django.conf import settings
 import maasserver
 from maasserver.dns import get_dns_server_address
 from maasserver.models import NodeGroup
+from maasserver.server_address import get_maas_facing_server_address
 from maasserver.testing import (
     disable_dhcp_management,
     enable_dhcp_management,
@@ -193,6 +195,9 @@
     def test_set_up_dhcp_writes_dhcp_config(self):
         mocked_task = self.patch(
             maasserver.models.nodegroup, 'write_dhcp_config')
+        self.patch(
+            settings, 'DEFAULT_MAAS_URL',
+            'http://%s/' % factory.getRandomIPAddress())
         nodegroup = factory.make_node_group(
             dhcp_key=factory.getRandomString(),
             ip_range_low='192.168.102.1', ip_range_high='192.168.103.254',
@@ -201,13 +206,19 @@
         dhcp_params = [
             'subnet_mask', 'broadcast_ip', 'router_ip',
             'ip_range_low', 'ip_range_high']
-        expected_params = {}
-        for param in dhcp_params:
-            expected_params[param] = getattr(nodegroup, param)
-        expected_params["next_server"] = nodegroup.worker_ip
+
+        expected_params = {
+            param: getattr(nodegroup, param)
+            for param in dhcp_params}
+
+        # Currently all nodes use the central TFTP server.  This will be
+        # decentralized to use NodeGroup.worker_ip later.
+        expected_params["next_server"] = get_maas_facing_server_address()
+
         expected_params["omapi_key"] = nodegroup.dhcp_key
         expected_params["dns_servers"] = get_dns_server_address()
         expected_params["subnet"] = '192.168.100.0'
+
         mocked_task.delay.assert_called_once_with(**expected_params)
 
     def test_add_dhcp_host_maps_adds_maps_if_managing_dhcp(self):