launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14538
[Merge] lp:~rvb/maas/bug-1081696-e into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/bug-1081696-e into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1081696 in MAAS: "The value of 'domain-name-servers' in the dhcp config is wrong."
https://bugs.launchpad.net/maas/+bug/1081696
For more details, see:
https://code.launchpad.net/~rvb/maas/bug-1081696-e/+merge/136189
This is the part e) of the action plan described in https://bugs.launchpad.net/maas/+bug/1081701/comments/8 to fix bug 1081701 and bug 1081696.
Use the host from nodegroup.maas_url to populate the dns-servers slot in the dhcp config.
Drive-by fix: cleanup the remains of 'next_server'. (The main change was done in https://code.launchpad.net/~julian-edwards/maas/next-server-wrong-bug-1081692/+merge/135579 but the bare minimum was done: here I remove the left-overs)
--
https://code.launchpad.net/~rvb/maas/bug-1081696-e/+merge/136189
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/bug-1081696-e into lp:maas.
=== modified file 'src/maasserver/dhcp.py'
--- src/maasserver/dhcp.py 2012-10-11 09:34:16 +0000
+++ src/maasserver/dhcp.py 2012-11-26 14:50:26 +0000
@@ -20,7 +20,6 @@
NODEGROUP_STATUS,
NODEGROUPINTERFACE_MANAGEMENT,
)
-from maasserver.server_address import get_maas_facing_server_address
from netaddr import IPAddress
from provisioningserver.tasks import (
restart_dhcp_server,
@@ -52,12 +51,6 @@
# server.
nodegroup.ensure_dhcp_key()
- # 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()
-
interface = nodegroup.get_managed_interface()
subnet = str(
IPAddress(interface.ip_range_low) &
@@ -66,13 +59,12 @@
options={'queue': nodegroup.work_queue})
task_kwargs = dict(
subnet=subnet,
- next_server=next_server,
omapi_key=nodegroup.dhcp_key,
subnet_mask=interface.subnet_mask,
dhcp_interfaces=interface.interface,
broadcast_ip=interface.broadcast_ip,
router_ip=interface.router_ip,
- dns_servers=get_dns_server_address(),
+ dns_servers=get_dns_server_address(nodegroup),
ip_range_low=interface.ip_range_low,
ip_range_high=interface.ip_range_high,
callback=reload_dhcp_server_subtask,
=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py 2012-10-25 07:24:01 +0000
+++ src/maasserver/dns.py 2012-11-26 14:50:26 +0000
@@ -105,14 +105,13 @@
logging.getLogger('maas').warn(WARNING_MESSAGE % ip)
-def get_dns_server_address():
+def get_dns_server_address(nodegroup=None):
"""Return the DNS server's IP address.
- That address is derived from DEFAULT_MAAS_URL in order to get a sensible
- default and at the same time give a possibility to the user to change this.
+ That address is derived from DEFAULT_MAAS_URL or nodegroup.maas_url.
"""
try:
- ip = get_maas_facing_server_address()
+ ip = get_maas_facing_server_address(nodegroup)
except socket.error as e:
raise DNSException(
"Unable to find MAAS server IP address: %s. "
=== modified file 'src/maasserver/server_address.py'
--- src/maasserver/server_address.py 2012-08-06 10:11:02 +0000
+++ src/maasserver/server_address.py 2012-11-26 14:50:26 +0000
@@ -22,23 +22,29 @@
from django.conf import settings
-def get_maas_facing_server_host():
+def get_maas_facing_server_host(nodegroup=None):
"""Return configured MAAS server hostname, for use by nodes or workers.
- :return: Hostname or IP address, exactly as configured in the
- DEFAULT_MAAS_URL setting.
+ :param nodegroup: The nodegroup from the point of view of which the
+ server host should be computed.
+ :return: Hostname or IP address, as configured in the DEFAULT_MAAS_URL
+ setting or as configured on nodegroup.maas_url.
"""
- return urlparse(settings.DEFAULT_MAAS_URL).hostname
-
-
-def get_maas_facing_server_address():
+ if nodegroup is None or not nodegroup.maas_url:
+ maas_url = settings.DEFAULT_MAAS_URL
+ else:
+ maas_url = nodegroup.maas_url
+ return urlparse(maas_url).hostname
+
+
+def get_maas_facing_server_address(nodegroup=None):
"""Return address where nodes and workers can reach the MAAS server.
- The address is taken from DEFAULT_MAAS_URL, which in turn is based on the
- server's primary IP address by default, but can be overridden for
- multi-interface servers where this guess is wrong.
+ The address is taken from DEFAULT_MAAS_URL or nodegroup.maas_url.
+ :param nodegroup: The nodegroup from the point of view of which the
+ server address should be computed.
:return: An IP address. If the configured URL uses a hostname, this
function will resolve that hostname.
"""
- return gethostbyname(get_maas_facing_server_host())
+ return gethostbyname(get_maas_facing_server_host(nodegroup))
=== modified file 'src/maasserver/tests/test_dhcp.py'
--- src/maasserver/tests/test_dhcp.py 2012-10-11 09:39:49 +0000
+++ src/maasserver/tests/test_dhcp.py 2012-11-26 14:50:26 +0000
@@ -22,7 +22,6 @@
)
from maasserver.dns import get_dns_server_address
from maasserver.enum import NODEGROUP_STATUS
-from maasserver.server_address import get_maas_facing_server_address
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase
from maastesting.celery import CeleryFixture
@@ -81,10 +80,6 @@
param: getattr(interface, 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'
@@ -98,6 +93,22 @@
self.assertEqual(expected_params, result_params)
+ def test_dhcp_config_uses_dns_server_from_cluster_controller(self):
+ mocked_task = self.patch(dhcp, 'write_dhcp_config')
+ ip = factory.getRandomIPAddress()
+ maas_url = 'http://%s/' % ip
+ nodegroup = factory.make_node_group(
+ maas_url=maas_url,
+ status=NODEGROUP_STATUS.ACCEPTED,
+ dhcp_key=factory.getRandomString(),
+ interface=factory.make_name('eth'),
+ network=IPNetwork("192.168.102.0/22"))
+ self.patch(settings, "DHCP_CONNECT", True)
+ configure_dhcp(nodegroup)
+ kwargs = mocked_task.apply_async.call_args[1]['kwargs']
+
+ self.assertEqual(ip, kwargs['dns_servers'])
+
def test_configure_dhcp_restart_dhcp_server(self):
self.patch(tasks, "sudo_write_file")
mocked_check_call = self.patch(tasks, "check_call")
=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py 2012-11-01 15:59:22 +0000
+++ src/maasserver/tests/test_dns.py 2012-11-26 14:50:26 +0000
@@ -114,6 +114,17 @@
call(dns.WARNING_MESSAGE % '127.0.0.1'),
logger.return_value.warn.call_args)
+ def test_get_dns_server_address_uses_nodegroup_maas_url(self):
+ ip = factory.getRandomIPAddress()
+ resolver = FakeMethod(result=ip)
+ self.patch(server_address, 'gethostbyname', resolver)
+ hostname = factory.getRandomString().lower()
+ maas_url = 'http://%s' % hostname
+ nodegroup = factory.make_node_group(maas_url=maas_url)
+ self.assertEqual(
+ (ip, [(hostname, )]),
+ (dns.get_dns_server_address(nodegroup), resolver.extract_args()))
+
def test_is_dns_managed(self):
nodegroups_with_expected_results = {
factory.make_node_group(
=== modified file 'src/maasserver/tests/test_server_address.py'
--- src/maasserver/tests/test_server_address.py 2012-08-06 10:14:07 +0000
+++ src/maasserver/tests/test_server_address.py 2012-11-26 14:50:26 +0000
@@ -15,9 +15,9 @@
from django.conf import settings
from maasserver import server_address
from maasserver.server_address import get_maas_facing_server_address
-from maastesting.factory import factory
+from maasserver.testing.factory import factory
+from maasserver.testing.testcase import TestCase
from maastesting.fakemethod import FakeMethod
-from maastesting.testcase import TestCase
from netaddr import IPNetwork
@@ -48,6 +48,13 @@
self.set_DEFAULT_MAAS_URL(ip)
self.assertEqual(ip, server_address.get_maas_facing_server_host())
+ def test_get_maas_facing_server_host_returns_nodegroup_maas_url(self):
+ hostname = factory.make_name('host').lower()
+ maas_url = 'http://%s' % hostname
+ nodegroup = factory.make_node_group(maas_url=maas_url)
+ self.assertEqual(
+ hostname, server_address.get_maas_facing_server_host(nodegroup))
+
def test_get_maas_facing_server_host_strips_out_port(self):
hostname = self.make_hostname()
self.set_DEFAULT_MAAS_URL(hostname, with_port=True)
@@ -64,11 +71,18 @@
self.set_DEFAULT_MAAS_URL(hostname=ip)
self.assertEqual(ip, get_maas_facing_server_address())
+ def test_get_maas_facing_server_address_returns_nodegroup_maas_url(self):
+ ip = factory.getRandomIPInNetwork(IPNetwork('127.0.0.0/8'))
+ maas_url = 'http://%s' % ip
+ nodegroup = factory.make_node_group(maas_url=maas_url)
+ self.assertEqual(
+ ip, server_address.get_maas_facing_server_host(nodegroup))
+
def test_get_maas_facing_server_address_resolves_hostname(self):
ip = factory.getRandomIPAddress()
resolver = FakeMethod(result=ip)
self.patch(server_address, 'gethostbyname', resolver)
- hostname = self.make_hostname()
+ hostname = self.make_hostname().lower()
self.set_DEFAULT_MAAS_URL(hostname=hostname)
self.assertEqual(
(ip, [(hostname, )]),
=== modified file 'src/provisioningserver/dhcp/tests/test_config.py'
--- src/provisioningserver/dhcp/tests/test_config.py 2012-09-05 05:49:28 +0000
+++ src/provisioningserver/dhcp/tests/test_config.py 2012-11-26 14:50:26 +0000
@@ -27,7 +27,6 @@
{{omapi_key}}
{{subnet}}
{{subnet_mask}}
- {{next_server}}
{{broadcast_ip}}
{{dns_servers}}
{{router_ip}}
@@ -45,7 +44,6 @@
omapi_key="random",
subnet="10.0.0.0",
subnet_mask="255.0.0.0",
- next_server="10.0.0.1",
broadcast_ip="10.255.255.255",
dns_servers="10.1.0.1 10.1.0.2",
router_ip="10.0.0.2",
=== modified file 'src/provisioningserver/tests/test_auth.py'
--- src/provisioningserver/tests/test_auth.py 2012-11-26 02:24:46 +0000
+++ src/provisioningserver/tests/test_auth.py 2012-11-26 14:50:26 +0000
@@ -12,8 +12,6 @@
__metaclass__ = type
__all__ = []
-import os
-
from apiclient.creds import convert_tuple_to_string
from apiclient.testing.credentials import make_api_credentials
from fixtures import EnvironmentVariableFixture
=== modified file 'src/provisioningserver/tests/test_start_cluster_controller.py'
--- src/provisioningserver/tests/test_start_cluster_controller.py 2012-11-23 01:42:03 +0000
+++ src/provisioningserver/tests/test_start_cluster_controller.py 2012-11-26 14:50:26 +0000
@@ -295,4 +295,3 @@
MAAS_URL=server_url,
)
os.execvpe.assert_called_once_with(ANY, ANY, env=env)
-
=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py 2012-11-26 02:43:10 +0000
+++ src/provisioningserver/tests/test_tasks.py 2012-11-26 14:50:26 +0000
@@ -176,7 +176,6 @@
'omapi_key',
'subnet',
'subnet_mask',
- 'next_server',
'broadcast_ip',
'dns_servers',
'router_ip',
