launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #10612
[Merge] lp:~jtv/maas/get_maas_facing_server_address into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/get_maas_facing_server_address into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/get_maas_facing_server_address/+merge/118323
This was done in coördination with Raphaël. For another branch I'm doing, I need to obtain the MAAS server's IP address as seen from the perspective of the workers and nodes. The implementation could probably be more solid in the face of NAT, but it should at least be possible to configure this such that it will work. The code also subsumes get_maas_server_host, so as to avoid duplication.
We just decided to drop the maas_url config item. That is why this new code goes straight to DEFAULT_MAAS_URL, which we will rename to MAAS_URL in a separate branch.
Jeroen
--
https://code.launchpad.net/~jtv/maas/get_maas_facing_server_address/+merge/118323
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/get_maas_facing_server_address into lp:maas.
=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py 2012-08-02 15:47:39 +0000
+++ src/maasserver/dns.py 2012-08-06 09:36:00 +0000
@@ -21,7 +21,7 @@
import collections
import logging
import socket
-from urlparse import urlparse
+from textwrap import dedent
from django.conf import settings
from django.db.models.signals import (
@@ -41,6 +41,7 @@
INT_MAX,
Sequence,
)
+from maasserver.server_address import get_maas_facing_server_address
from maasserver.signals import connect_to_field_change
from netaddr import (
IPAddress,
@@ -68,15 +69,20 @@
"""An error occured when setting up MAAS' DNS server."""
+def concatenate_line(text):
+ """Return `text` concatenated into a single line of text."""
+ return dedent(text.rstrip('\n')).replace('\n', ' ').strip()
+
+
def warn_loopback(ip):
"""Warn if the given IP address is in the loopback network."""
if IPAddress(ip) in IPNetwork('127.0.0.1/8'):
- logging.getLogger('maas').warn(
- "The DNS server address used will be '%s'. That address is "
- "in the loopback network. This might not be a problem "
- "if you're not using MAAS' DNS features or if "
- "you don't rely on this information. Be sure to set the "
- "setting DEFAULT_MAAS_URL." % ip)
+ logging.getLogger('maas').warn(concatenate_line("""
+ The DNS server will use the address '%s', which is inside the
+ loopback network. This may not be a problem if you're not using
+ MAAS' DNS features or if you don't rely on this information. Be
+ sure to configure the DEFAULT_MAAS_URL setting.
+ """ % ip))
def get_dns_server_address():
@@ -85,24 +91,18 @@
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.
"""
- host = urlparse(settings.DEFAULT_MAAS_URL).netloc.split(':')[0]
-
- # Try to resolve the hostname, if `host` is alread an IP address, it
- # will simpply be returned by `socket.gethostbyname`.
try:
- ip = socket.gethostbyname(host)
- warn_loopback(ip)
- return ip
- except socket.error:
- pass
+ ip = get_maas_facing_server_address()
+ except socket.error as e:
+ raise DNSException(concatenate_line("""
+ Unable to find MAAS server IP address: %s.
+ MAAS' DNS server requires this IP address for the NS records
+ in its zone files. Make sure that the DEFAULT_MAAS_URL setting
+ has the correct hostname.
+ """ % e.strerror))
- # No suitable address has been found.
- raise DNSException(
- "Unable to find a suitable IP for the MAAS server. Such an IP "
- "is required for MAAS' DNS features to work. Make sure that the "
- "setting DEFAULT_MAAS_URL is defined properly. The IP in "
- "DEFAULT_MAAS_URL is the one which will be used for the NS record "
- "in MAAS' zone files.")
+ warn_loopback(ip)
+ return ip
def dns_config_changed(sender, config, created, **kwargs):
=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py 2012-08-03 15:17:01 +0000
+++ src/maasserver/preseed.py 2012-08-06 09:36:00 +0000
@@ -19,15 +19,14 @@
from os.path import join
from pipes import quote
from urllib import urlencode
-from urlparse import urlparse
from django.conf import settings
from maasserver.enum import (
NODE_STATUS,
PRESEED_TYPE,
)
-from maasserver.models import Config
from maasserver.provisioning import compose_preseed
+from maasserver.server_address import get_maas_facing_server_host
from maasserver.utils import absolute_reverse
import tempita
@@ -200,12 +199,6 @@
return get_template(prefix, None, default=True)
-def get_maas_server_host():
- """Return MAAS' server name."""
- maas_url = Config.objects.get_config('maas_url')
- return urlparse(maas_url).netloc.split(':')[0]
-
-
# XXX: rvb 2012-06-19 bug=1013146: 'precise' is hardcoded here.
def get_preseed_context(node, release="precise"):
"""Return the context dictionary to be used to render preseed templates
@@ -217,7 +210,7 @@
:return: The context dictionary.
:rtype: dict.
"""
- server_host = get_maas_server_host()
+ server_host = get_maas_facing_server_host()
context = {
'release': release,
'server_host': server_host,
=== added file 'src/maasserver/server_address.py'
--- src/maasserver/server_address.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/server_address.py 2012-08-06 09:36:00 +0000
@@ -0,0 +1,44 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Helper to obtain the MAAS server's address."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = [
+ 'get_maas_facing_server_address',
+ 'get_maas_facing_server_host',
+ ]
+
+
+from socket import gethostbyname
+from urlparse import urlparse
+
+from django.conf import settings
+
+
+def get_maas_facing_server_host():
+ """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.
+ """
+ return urlparse(settings.DEFAULT_MAAS_URL).netloc.split(':')[0]
+
+
+def get_maas_facing_server_address():
+ """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.
+
+ :return: An IP address. If the configured URL uses a hostname, this
+ function will resolve that hostname.
+ """
+ return gethostbyname(get_maas_facing_server_host())
=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py 2012-08-03 12:09:36 +0000
+++ src/maasserver/tests/test_dns.py 2012-08-06 09:36:00 +0000
@@ -19,16 +19,9 @@
from django.conf import settings
from django.core.management import call_command
-from maasserver.dns import (
- add_zone,
- change_dns_zones,
- DNSException,
- get_dns_server_address,
- get_zone,
- is_dns_enabled,
- next_zone_serial,
- write_full_dns_config,
- zone_serial,
+from maasserver import (
+ dns,
+ server_address,
)
from maasserver.models import Config
from maasserver.models.dhcplease import (
@@ -59,7 +52,7 @@
def test_zone_serial_parameters(self):
self.assertThat(
- zone_serial,
+ dns.zone_serial,
MatchesStructure.byEquality(
maxvalue=2 ** 32 - 1,
minvalue=1,
@@ -68,10 +61,10 @@
)
def test_next_zone_serial_returns_sequence(self):
- initial = int(next_zone_serial())
+ initial = int(dns.next_zone_serial())
self.assertSequenceEqual(
['%0.10d' % i for i in range(initial + 1, initial + 11)],
- [next_zone_serial() for i in range(initial, initial + 10)])
+ [dns.next_zone_serial() for i in range(initial, initial + 10)])
def patch_DEFAULT_MAAS_URL_with_random_values(self, hostname=None):
if hostname is None:
@@ -80,36 +73,27 @@
hostname, factory.getRandomPort(), factory.getRandomString())
self.patch(settings, 'DEFAULT_MAAS_URL', url)
- def test_get_dns_server_address_returns_IP(self):
- ip = factory.getRandomIPAddress()
- self.patch_DEFAULT_MAAS_URL_with_random_values(hostname=ip)
- self.assertEqual(ip, get_dns_server_address())
-
- def test_get_dns_server_address_returns_IP_if_IP_is_localhost(self):
- ip = factory.getRandomIPInNetwork(IPNetwork('127.0.0.1/8'))
- self.patch_DEFAULT_MAAS_URL_with_random_values(hostname=ip)
- self.assertEqual(ip, get_dns_server_address())
-
def test_get_dns_server_address_resolves_hostname(self):
ip = factory.getRandomIPAddress()
resolver = FakeMethod(result=ip)
- self.patch(socket, 'gethostbyname', resolver)
+ self.patch(server_address, 'gethostbyname', resolver)
hostname = factory.getRandomString()
self.patch_DEFAULT_MAAS_URL_with_random_values(hostname=hostname)
self.assertEqual(
(ip, [(hostname, )]),
- (get_dns_server_address(), resolver.extract_args()))
+ (dns.get_dns_server_address(), resolver.extract_args()))
def test_get_dns_server_address_raises_if_hostname_doesnt_resolve(self):
- resolver = FakeMethod(failure=socket.error)
- self.patch(socket, 'gethostbyname', resolver)
+ self.patch(
+ dns, 'get_maas_facing_server_address',
+ FakeMethod(failure=socket.error))
self.patch_DEFAULT_MAAS_URL_with_random_values()
- self.assertRaises(DNSException, get_dns_server_address)
+ self.assertRaises(dns.DNSException, dns.get_dns_server_address)
def test_get_zone_creates_DNSZoneConfig(self):
nodegroup = factory.make_node_group()
serial = random.randint(1, 100)
- zone = get_zone(nodegroup, serial)
+ zone = dns.get_zone(nodegroup, serial)
self.assertAttributes(
zone,
dict(
@@ -126,7 +110,7 @@
nodegroup = factory.make_node_group()
nodegroup.subnet_mask = None
nodegroup.save()
- self.assertIsNone(get_zone(nodegroup))
+ self.assertIsNone(dns.get_zone(nodegroup))
class TestDNSConfigModifications(TestCase):
@@ -210,7 +194,7 @@
def test_add_zone_loads_dns_zone(self):
nodegroup, node, lease = self.create_nodegroup_with_lease()
- add_zone(nodegroup)
+ dns.add_zone(nodegroup)
self.assertDNSMatches(node.hostname, nodegroup.name, lease.ip)
def test_add_zone_doesnt_write_config_if_dhcp_disabled(self):
@@ -219,30 +203,30 @@
nodegroup = factory.make_node_group()
nodegroup.subnet_mask = None
nodegroup.save()
- add_zone(nodegroup)
+ dns.add_zone(nodegroup)
self.assertEqual(0, recorder.call_count)
def test_change_dns_zone_changes_dns_zone(self):
nodegroup, _, _ = self.create_nodegroup_with_lease()
- write_full_dns_config()
+ dns.write_full_dns_config()
nodegroup, new_node, new_lease = (
self.create_nodegroup_with_lease(
nodegroup=nodegroup, lease_number=2))
- change_dns_zones(nodegroup)
+ dns.change_dns_zones(nodegroup)
self.assertDNSMatches(new_node.hostname, nodegroup.name, new_lease.ip)
def test_is_dns_enabled_return_false_if_DNS_CONNECT_False(self):
self.patch(settings, 'DNS_CONNECT', False)
- self.assertFalse(is_dns_enabled())
+ self.assertFalse(dns.is_dns_enabled())
def test_is_dns_enabled_return_false_if_confif_enable_dns_False(self):
Config.objects.set_config('enable_dns', False)
- self.assertFalse(is_dns_enabled())
+ self.assertFalse(dns.is_dns_enabled())
def test_is_dns_enabled_return_True(self):
self.patch(settings, 'DNS_CONNECT', True)
Config.objects.set_config('enable_dns', True)
- self.assertTrue(is_dns_enabled())
+ self.assertTrue(dns.is_dns_enabled())
def test_change_dns_zone_changes_doesnt_write_conf_if_dhcp_disabled(self):
recorder = FakeMethod()
@@ -250,7 +234,7 @@
nodegroup = factory.make_node_group()
nodegroup.subnet_mask = None
nodegroup.save()
- change_dns_zones(nodegroup)
+ dns.change_dns_zones(nodegroup)
self.assertEqual(0, recorder.call_count)
def test_write_full_dns_doesnt_write_config_if_dhcp_disabled(self):
@@ -259,24 +243,24 @@
nodegroup = factory.make_node_group()
nodegroup.subnet_mask = None
nodegroup.save()
- write_full_dns_config()
+ dns.write_full_dns_config()
self.assertEqual(0, recorder.call_count)
def test_write_full_dns_loads_full_dns_config(self):
nodegroup, node, lease = self.create_nodegroup_with_lease()
- write_full_dns_config()
+ dns.write_full_dns_config()
self.assertDNSMatches(node.hostname, nodegroup.name, lease.ip)
def test_write_full_dns_can_write_inactive_config(self):
nodegroup, node, lease = self.create_nodegroup_with_lease()
- write_full_dns_config(active=False)
+ dns.write_full_dns_config(active=False)
self.assertEqual([''], self.dig_resolve(generated_hostname(lease.ip)))
def test_dns_config_has_NS_record(self):
ip = factory.getRandomIPAddress()
self.patch(settings, 'DEFAULT_MAAS_URL', 'http://%s/' % ip)
nodegroup, node, lease = self.create_nodegroup_with_lease()
- write_full_dns_config()
+ dns.write_full_dns_config()
# Get the NS record for the zone 'nodegroup.name'.
ns_record = dig_call(
port=self.bind.config.port,
@@ -289,7 +273,7 @@
def test_is_dns_enabled_follows_DNS_CONNECT(self):
rand_bool = factory.getRandomBoolean()
self.patch(settings, "DNS_CONNECT", rand_bool)
- self.assertEqual(rand_bool, is_dns_enabled())
+ self.assertEqual(rand_bool, dns.is_dns_enabled())
def test_add_nodegroup_creates_DNS_zone(self):
self.patch(settings, "DNS_CONNECT", True)
=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py 2012-08-03 15:21:28 +0000
+++ src/maasserver/tests/test_preseed.py 2012-08-06 09:36:00 +0000
@@ -20,11 +20,9 @@
NODE_STATUS,
PRESEED_TYPE,
)
-from maasserver.models import Config
from maasserver.preseed import (
GENERIC_FILENAME,
get_enlist_preseed,
- get_maas_server_host,
get_preseed,
get_preseed_context,
get_preseed_filenames,
@@ -300,19 +298,6 @@
TemplateNotFoundError, template.substitute)
-class TestGetMAASServerHost(TestCase):
- """Tests for `get_maas_server_host`."""
-
- def test_get_maas_server_host_returns_host(self):
- Config.objects.set_config('maas_url', 'http://example.com/path')
- self.assertEqual('example.com', get_maas_server_host())
-
- def test_get_maas_server_host_strips_out_port(self):
- Config.objects.set_config(
- 'maas_url', 'http://example.com:%d' % factory.getRandomPort())
- self.assertEqual('example.com', get_maas_server_host())
-
-
class TestPreseedContext(TestCase):
"""Tests for `get_preseed_context`."""
=== added file 'src/maasserver/tests/test_server_address.py'
--- src/maasserver/tests/test_server_address.py 1970-01-01 00:00:00 +0000
+++ src/maasserver/tests/test_server_address.py 2012-08-06 09:36:00 +0000
@@ -0,0 +1,75 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the server_address module."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+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 maastesting.fakemethod import FakeMethod
+from maastesting.testcase import TestCase
+from netaddr import IPNetwork
+
+
+class TestServerAddress(TestCase):
+
+ def make_hostname(self):
+ return '%s.example.com' % factory.make_name('host')
+
+ def set_DEFAULT_MAAS_URL(self, hostname=None, with_port=False):
+ """Patch DEFAULT_MAAS_URL to be a (partly) random URL."""
+ if hostname is None:
+ hostname = self.make_hostname()
+ if with_port:
+ location = "%s:%d" % (hostname, factory.getRandomPort())
+ else:
+ location = hostname
+ url = 'http://%s/%s' % (location, factory.make_name("path"))
+ self.patch(settings, 'DEFAULT_MAAS_URL', url)
+
+ def test_get_maas_facing_server_host_returns_host_name(self):
+ hostname = self.make_hostname()
+ self.set_DEFAULT_MAAS_URL(hostname)
+ self.assertEqual(
+ hostname, server_address.get_maas_facing_server_host())
+
+ def test_get_maas_facing_server_host_returns_ip_if_ip_configured(self):
+ ip = factory.getRandomIPAddress()
+ self.set_DEFAULT_MAAS_URL(ip)
+ self.assertEqual(ip, server_address.get_maas_facing_server_host())
+
+ def test_get_maas_facing_server_host_strips_out_port(self):
+ hostname = self.make_hostname()
+ self.set_DEFAULT_MAAS_URL(hostname, with_port=True)
+ self.assertEqual(
+ hostname, server_address.get_maas_facing_server_host())
+
+ def test_get_maas_facing_server_address_returns_IP(self):
+ ip = factory.getRandomIPAddress()
+ self.set_DEFAULT_MAAS_URL(hostname=ip)
+ self.assertEqual(ip, get_maas_facing_server_address())
+
+ def test_get_maas_facing_server_address_returns_local_IP(self):
+ ip = factory.getRandomIPInNetwork(IPNetwork('127.0.0.0/8'))
+ self.set_DEFAULT_MAAS_URL(hostname=ip)
+ self.assertEqual(ip, get_maas_facing_server_address())
+
+ def test_get_maas_facing_server_address_resolves_hostname(self):
+ ip = factory.getRandomIPAddress()
+ resolver = FakeMethod(result=ip)
+ self.patch(server_address, 'gethostbyname', resolver)
+ hostname = factory.getRandomString()
+ self.set_DEFAULT_MAAS_URL(hostname=hostname)
+ self.assertEqual(
+ (ip, [(hostname, )]),
+ (get_maas_facing_server_address(), resolver.extract_args()))
