launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #14680
[Merge] lp:~julian-edwards/maas/backport-r1300 into lp:maas/1.2
Julian Edwards has proposed merging lp:~julian-edwards/maas/backport-r1300 into lp:maas/1.2.
Commit message:
Write the DNS config only if at least one of the interfaces is configured to manage DNS. (backport r1300 from trunk)
Requested reviews:
MAAS Maintainers (maas-maintainers)
Related bugs:
Bug #1067977 in MAAS: "write_full_dns_config runs on a fresh installation even before dns is installed or configured"
https://bugs.launchpad.net/maas/+bug/1067977
For more details, see:
https://code.launchpad.net/~julian-edwards/maas/backport-r1300/+merge/137494
--
https://code.launchpad.net/~julian-edwards/maas/backport-r1300/+merge/137494
Your team MAAS Maintainers is requested to review the proposed merge of lp:~julian-edwards/maas/backport-r1300 into lp:maas/1.2.
=== modified file 'src/maasserver/dhcp_connect.py'
--- src/maasserver/dhcp_connect.py 2012-10-03 07:48:03 +0000
+++ src/maasserver/dhcp_connect.py 2012-12-03 07:32:19 +0000
@@ -31,7 +31,7 @@
configure_dhcp(instance.nodegroup)
-def dhcp_post_edit_status_NodeGroup(instance, old_field):
+def dhcp_post_edit_status_NodeGroup(instance, old_field, **kwargs):
"""The status of a NodeGroup changed."""
# This could be optimized a bit by detecting if the status change is
# actually a change from 'do not manage DHCP' to 'manage DHCP'.
=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py 2012-10-01 16:34:43 +0000
+++ src/maasserver/dns.py 2012-12-03 07:32:19 +0000
@@ -37,6 +37,7 @@
from maasserver.models import (
DHCPLease,
NodeGroup,
+ NodeGroupInterface,
)
from maasserver.sequence import (
INT_MAX,
@@ -64,6 +65,14 @@
return '%0.10d' % zone_serial.nextval()
+def is_dns_in_use():
+ """Is there at least one interface configured to manage DNS?"""
+ interfaces_with_dns = (
+ NodeGroupInterface.objects.filter(
+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS))
+ return interfaces_with_dns.exists()
+
+
def is_dns_enabled():
"""Is MAAS configured to manage DNS?"""
return settings.DNS_CONNECT
@@ -82,17 +91,18 @@
interface.management == NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+WARNING_MESSAGE = (
+ "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's DNS features or if you don't rely on this information. "
+ "Be sure to configure the DEFAULT_MAAS_URL setting in MAAS's "
+ "/etc/maas/maas_local_settings.py.")
+
+
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 will use the address '%s', which is inside the "
- "loopback network. This may not be a problem if you're not using "
- "MAAS's DNS features or if you don't rely on this information. "
- "Be sure to configure the DEFAULT_MAAS_URL setting in MAAS's "
- "settings.py (or demo.py/development.py if you are running a "
- "development system)."
- % ip)
+ logging.getLogger('maas').warn(WARNING_MESSAGE % ip)
def get_dns_server_address():
@@ -251,7 +261,7 @@
zone should be updated.
:type nodegroups: list (or :class:`NodeGroup`)
"""
- if not is_dns_enabled():
+ if not (is_dns_enabled() and is_dns_in_use()):
return
serial = next_zone_serial()
for zone in ZoneGenerator(nodegroups, serial):
@@ -271,7 +281,7 @@
:param nodegroup: The nodegroup for which the zone should be added.
:type nodegroup: :class:`NodeGroup`
"""
- if not is_dns_enabled():
+ if not (is_dns_enabled() and is_dns_in_use()):
return
zones_to_write = ZoneGenerator(nodegroup).as_list()
if len(zones_to_write) == 0:
@@ -286,23 +296,21 @@
zones=zones_to_write, callback=write_dns_config_subtask)
-def write_full_dns_config(active=True, reload_retry=False):
+def write_full_dns_config(reload_retry=False, force=False):
"""Write the DNS configuration.
- :param active: If True, write the DNS config for all the nodegroups.
- Otherwise write an empty DNS config (with no zones). Defaults
- to `True`.
- :type active: bool
:param reload_retry: Should the reload rndc command be retried in case
of failure? Defaults to `False`.
:type reload_retry: bool
+ :param force: Write the configuration even if no interface is
+ configured to manage DNS.
+ :type force: bool
"""
- if not is_dns_enabled():
+ write_conf = (
+ is_dns_enabled() and (force or is_dns_in_use()))
+ if not write_conf:
return
- if active:
- zones = ZoneGenerator(NodeGroup.objects.all()).as_list()
- else:
- zones = []
+ zones = ZoneGenerator(NodeGroup.objects.all()).as_list()
tasks.write_full_dns_config.delay(
zones=zones,
callback=tasks.rndc_command.subtask(
=== modified file 'src/maasserver/dns_connect.py'
--- src/maasserver/dns_connect.py 2012-10-08 12:50:32 +0000
+++ src/maasserver/dns_connect.py 2012-12-03 07:32:19 +0000
@@ -19,6 +19,7 @@
post_save,
)
from django.dispatch import receiver
+from maasserver.enum import NODEGROUPINTERFACE_MANAGEMENT
from maasserver.models import (
Node,
NodeGroup,
@@ -50,11 +51,19 @@
write_full_dns_config()
-@receiver(post_delete, sender=NodeGroup)
-def dns_post_delete_NodeGroup(sender, instance, **kwargs):
- """Delete DNS zones related to the nodegroup."""
+def dns_post_edit_management_NodeGroupInterface(instance, old_field, deleted):
+ """Delete DNS zones related to the interface."""
from maasserver.dns import write_full_dns_config
- write_full_dns_config()
+ if old_field == NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS:
+ # Force the dns config to be written as this might have been
+ # triggered by the last DNS-enabled interface being deleted
+ # or switched off (i.e. management set to DHCP or UNMANAGED).
+ write_full_dns_config(force=True)
+
+
+connect_to_field_change(
+ dns_post_edit_management_NodeGroupInterface,
+ NodeGroupInterface, 'management', delete=True)
@receiver(post_delete, sender=Node)
@@ -70,7 +79,7 @@
pass
-def dns_post_edit_hostname_Node(instance, old_field):
+def dns_post_edit_hostname_Node(instance, old_field, **kwargs):
"""When a Node has been flagged, update the related zone."""
from maasserver.dns import change_dns_zones
change_dns_zones(instance.nodegroup)
=== modified file 'src/maasserver/signals.py'
--- src/maasserver/signals.py 2012-08-03 18:57:08 +0000
+++ src/maasserver/signals.py 2012-12-03 07:32:19 +0000
@@ -15,13 +15,15 @@
]
from django.db.models.signals import (
+ post_delete,
post_init,
post_save,
+ pre_delete,
pre_save,
)
-def connect_to_field_change(callback, model, field_name):
+def connect_to_field_change(callback, model, field_name, delete=False):
"""Call the provided callback when a field is modified on a model.
The provided `callback` method will be called when the value of the field
@@ -29,13 +31,24 @@
The signature of the callback method should be the following:
- >>> def callback(instance, old_value):
+ >>> def callback(instance, old_value, deleted):
...
pass
- Where `instance` is the object which has just being saved to the database
- and `old_value` is the old value of the field (different from the value of
- the field in `instance`).
+ Where `instance` is the object which has just being saved to the
+ database, `old_value` is the old value of the field (different from the
+ value of the field in `instance`) and `deleted` indicates whether or not
+ the callback was called because the object was deleted.
+
+ :param callback: The callback function.
+ :type callback: callable
+ :param model: Specifies a particular sender to receive signals from.
+ :type model: class
+ :param field_name: Name of the field to monitor.
+ :type field_name: basestring
+ :param delete: Should the deletion of an object be considered a change
+ in the field?
+ :type delete: bool
"""
last_seen_flag = '_field_last_seen_value_%s' % field_name
delta_flag = '_field_delta_%s' % field_name
@@ -47,17 +60,27 @@
post_init.connect(post_init_callback, sender=model, weak=False)
# Set 'delta_flag' with the new and the old value of the field.
- def pre_save_callback(sender, instance, **kwargs):
+ def record_delta_flag(sender, instance, **kwargs):
original_value = getattr(instance, last_seen_flag)
new_value = getattr(instance, field_name)
setattr(instance, delta_flag, (new_value, original_value))
- pre_save.connect(pre_save_callback, sender=model, weak=False)
+ pre_save.connect(record_delta_flag, sender=model, weak=False)
# Call the `callback` if the field has changed.
def post_save_callback(sender, instance, created, **kwargs):
(new_value, original_value) = getattr(instance, delta_flag)
# Call the callback method is the field has changed.
if original_value != new_value:
- callback(instance, original_value)
+ callback(instance, original_value, deleted=False)
setattr(instance, last_seen_flag, new_value)
+
+ if delete:
+ pre_delete.connect(record_delta_flag, sender=model, weak=False)
+
+ def post_delete_callback(sender, instance, **kwargs):
+ (new_value, original_value) = getattr(instance, delta_flag)
+ callback(instance, original_value, deleted=True)
+
+ post_delete.connect(post_delete_callback, sender=model, weak=False)
+
post_save.connect(post_save_callback, sender=model, weak=False)
=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py 2012-11-16 13:50:43 +0000
+++ src/maasserver/tests/test_dns.py 2012-12-03 07:32:19 +0000
@@ -15,6 +15,7 @@
from functools import partial
from itertools import islice
+import logging
import socket
from celery.task import task
@@ -35,6 +36,10 @@
from maastesting.celery import CeleryFixture
from maastesting.fakemethod import FakeMethod
from maastesting.tests.test_bindfixture import dig_call
+from mock import (
+ call,
+ Mock,
+ )
from netaddr import (
IPNetwork,
IPRange,
@@ -99,6 +104,16 @@
self.patch_DEFAULT_MAAS_URL_with_random_values()
self.assertRaises(dns.DNSException, dns.get_dns_server_address)
+ def test_get_dns_server_address_logs_warning_if_ip_is_localhost(self):
+ logger = self.patch(logging, 'getLogger')
+ self.patch(
+ dns, 'get_maas_facing_server_address',
+ Mock(return_value='127.0.0.1'))
+ dns.get_dns_server_address()
+ self.assertEqual(
+ call(dns.WARNING_MESSAGE % '127.0.0.1'),
+ logger.return_value.warn.call_args)
+
def test_is_dns_managed(self):
nodegroups_with_expected_results = {
factory.make_node_group(
@@ -149,12 +164,15 @@
# Reload BIND.
self.bind.runner.rndc('reload')
+ def create_managed_nodegroup(self):
+ return factory.make_node_group(
+ network=IPNetwork('192.168.0.1/24'),
+ status=NODEGROUP_STATUS.ACCEPTED,
+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+
def create_nodegroup_with_lease(self, lease_number=1, nodegroup=None):
if nodegroup is None:
- nodegroup = factory.make_node_group(
- network=IPNetwork('192.168.0.1/24'),
- status=NODEGROUP_STATUS.ACCEPTED,
- management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+ nodegroup = self.create_managed_nodegroup()
interface = nodegroup.get_managed_interface()
node = factory.make_node(
nodegroup=nodegroup)
@@ -225,25 +243,27 @@
self.patch(settings, 'DNS_CONNECT', False)
self.assertFalse(dns.is_dns_enabled())
- def test_is_dns_enabled_return_True(self):
+ def test_is_dns_enabled_return_True_if_DNS_CONNECT_True(self):
self.patch(settings, 'DNS_CONNECT', True)
self.assertTrue(dns.is_dns_enabled())
+ def test_is_dns_in_use_return_False_no_configured_interface(self):
+ self.assertFalse(dns.is_dns_in_use())
+
+ def test_is_dns_in_use_return_True_if_configured_interface(self):
+ self.create_managed_nodegroup()
+ self.assertTrue(dns.is_dns_in_use())
+
def test_write_full_dns_loads_full_dns_config(self):
nodegroup, node, lease = self.create_nodegroup_with_lease()
self.patch(settings, 'DNS_CONNECT', True)
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()
- self.patch(settings, 'DNS_CONNECT', True)
- dns.write_full_dns_config(active=False)
- self.assertEqual([''], self.dig_resolve(generated_hostname(lease.ip)))
-
def test_write_full_dns_passes_reload_retry_parameter(self):
self.patch(settings, 'DNS_CONNECT', True)
recorder = FakeMethod()
+ self.create_managed_nodegroup()
@task
def recorder_task(*args, **kwargs):
@@ -253,6 +273,12 @@
self.assertEqual(
([(['reload'], True)]), recorder.extract_args())
+ def test_write_full_dns_doesnt_call_task_it_no_interface_configured(self):
+ self.patch(settings, 'DNS_CONNECT', True)
+ patched_task = self.patch(tasks, 'write_full_dns_config')
+ dns.write_full_dns_config()
+ self.assertEqual(0, patched_task.call_count)
+
def test_dns_config_has_NS_record(self):
ip = factory.getRandomIPAddress()
self.patch(settings, 'DEFAULT_MAAS_URL', 'http://%s/' % ip)
@@ -268,11 +294,6 @@
port=self.bind.config.port, commands=[ns_record, '+short'])
self.assertEqual(ip, ip_of_ns_record)
- def test_is_dns_enabled_follows_DNS_CONNECT(self):
- rand_bool = factory.getRandomBoolean()
- self.patch(settings, "DNS_CONNECT", rand_bool)
- self.assertEqual(rand_bool, dns.is_dns_enabled())
-
def test_add_nodegroup_creates_DNS_zone(self):
self.patch(settings, "DNS_CONNECT", True)
network = IPNetwork('192.168.7.1/24')
@@ -305,6 +326,19 @@
# The ip from the new network resolves.
self.assertDNSMatches(generated_hostname(ip), nodegroup.name, ip)
+ def test_changing_interface_management_updates_DNS_zone(self):
+ self.patch(settings, "DNS_CONNECT", True)
+ network = IPNetwork('192.168.7.1/24')
+ ip = factory.getRandomIPInNetwork(network)
+ nodegroup = factory.make_node_group(
+ network=network, status=NODEGROUP_STATUS.ACCEPTED,
+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+ interface = nodegroup.get_managed_interface()
+ interface.management = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
+ interface.save()
+ self.assertEqual([''], self.dig_resolve(generated_hostname(ip)))
+ self.assertEqual([''], self.dig_reverse_resolve(ip))
+
def test_delete_nodegroup_disables_DNS_zone(self):
self.patch(settings, "DNS_CONNECT", True)
network = IPNetwork('192.168.7.1/24')
=== modified file 'src/maasserver/tests/test_signals.py'
--- src/maasserver/tests/test_signals.py 2012-08-03 18:57:08 +0000
+++ src/maasserver/tests/test_signals.py 2012-12-03 07:32:19 +0000
@@ -16,7 +16,10 @@
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestModelTestCase
from maasserver.tests.models import FieldChangeTestModel
-from maastesting.fakemethod import FakeMethod
+from mock import (
+ call,
+ Mock,
+ )
class ConnectToFieldChangeTest(TestModelTestCase):
@@ -25,7 +28,7 @@
app = 'maasserver.tests'
def test_connect_to_field_change_calls_callback(self):
- callback = FakeMethod()
+ callback = Mock()
connect_to_field_change(callback, FieldChangeTestModel, 'name1')
old_name1_value = factory.getRandomString()
obj = FieldChangeTestModel(name1=old_name1_value)
@@ -33,11 +36,11 @@
obj.name1 = factory.getRandomString()
obj.save()
self.assertEqual(
- (1, [(obj, old_name1_value)]),
- (callback.call_count, callback.extract_args()))
+ (1, call(obj, old_name1_value, deleted=False)),
+ (callback.call_count, callback.call_args))
def test_connect_to_field_change_calls_callback_for_each_save(self):
- callback = FakeMethod()
+ callback = Mock()
connect_to_field_change(callback, FieldChangeTestModel, 'name1')
old_name1_value = factory.getRandomString()
obj = FieldChangeTestModel(name1=old_name1_value)
@@ -49,7 +52,7 @@
self.assertEqual(2, callback.call_count)
def test_connect_to_field_change_calls_callback_for_each_real_save(self):
- callback = FakeMethod()
+ callback = Mock()
connect_to_field_change(callback, FieldChangeTestModel, 'name1')
old_name1_value = factory.getRandomString()
obj = FieldChangeTestModel(name1=old_name1_value)
@@ -60,9 +63,9 @@
self.assertEqual(1, callback.call_count)
def test_connect_to_field_change_calls_multiple_callbacks(self):
- callback1 = FakeMethod()
+ callback1 = Mock()
connect_to_field_change(callback1, FieldChangeTestModel, 'name1')
- callback2 = FakeMethod()
+ callback2 = Mock()
connect_to_field_change(callback2, FieldChangeTestModel, 'name1')
old_name1_value = factory.getRandomString()
obj = FieldChangeTestModel(name1=old_name1_value)
@@ -74,15 +77,35 @@
def test_connect_to_field_change_ignores_changes_to_other_fields(self):
obj = FieldChangeTestModel(name2=factory.getRandomString())
obj.save()
- callback = FakeMethod()
+ callback = Mock()
connect_to_field_change(callback, FieldChangeTestModel, 'name1')
obj.name2 = factory.getRandomString()
obj.save()
self.assertEqual(0, callback.call_count)
def test_connect_to_field_change_ignores_object_creation(self):
- callback = FakeMethod()
+ callback = Mock()
connect_to_field_change(callback, FieldChangeTestModel, 'name1')
obj = FieldChangeTestModel(name1=factory.getRandomString())
obj.save()
self.assertEqual(0, callback.call_count)
+
+ def test_connect_to_field_change_ignores_deletion_by_default(self):
+ obj = FieldChangeTestModel(name2=factory.getRandomString())
+ obj.save()
+ callback = Mock()
+ connect_to_field_change(callback, FieldChangeTestModel, 'name1')
+ obj.delete()
+ self.assertEqual(0, callback.call_count)
+
+ def test_connect_to_field_change_listens_to_deletion_if_delete_True(self):
+ old_name1_value = factory.getRandomString()
+ obj = FieldChangeTestModel(name1=old_name1_value)
+ obj.save()
+ callback = Mock()
+ connect_to_field_change(
+ callback, FieldChangeTestModel, 'name1', delete=True)
+ obj.delete()
+ self.assertEqual(
+ (1, call(obj, old_name1_value, deleted=True)),
+ (callback.call_count, callback.call_args))
Follow ups
