launchpad-reviewers team mailing list archive

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

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

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1044203 in MAAS: "celeryd repeatedly writing dns config every minute"
  https://bugs.launchpad.net/maas/+bug/1044203

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1044203/+merge/122495
-- 
https://code.launchpad.net/~jtv/maas/bug-1044203/+merge/122495
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/bug-1044203 into lp:maas.

=== modified file 'src/maasserver/dns_connect.py'
--- src/maasserver/dns_connect.py	2012-08-08 13:41:22 +0000
+++ src/maasserver/dns_connect.py	2012-09-03 11:22:18 +0000
@@ -21,16 +21,14 @@
 from django.dispatch import receiver
 from maasserver.models import (
     Config,
-    DHCPLease,
     Node,
     NodeGroup,
     )
-from maasserver.models.dhcplease import post_updates
 from maasserver.signals import connect_to_field_change
 
 
 def dns_config_changed(sender, config, created, **kwargs):
-    """Signal callback called when the DNS config changed."""
+    """Signal callback called when the DNS config has changed."""
     from maasserver.dns import write_full_dns_config
     write_full_dns_config(active=config.value)
 
@@ -55,13 +53,6 @@
     write_full_dns_config()
 
 
-@receiver(post_updates, sender=DHCPLease.objects)
-def dns_updated_DHCPLeaseManager(sender, **kwargs):
-    """Update all the zone files."""
-    from maasserver.dns import change_dns_zones
-    change_dns_zones(NodeGroup.objects.all())
-
-
 @receiver(post_delete, sender=Node)
 def dns_post_delete_Node(sender, instance, **kwargs):
     """When a Node is deleted, update the Node's zone file."""

=== modified file 'src/maasserver/models/dhcplease.py'
--- src/maasserver/models/dhcplease.py	2012-09-03 04:59:58 +0000
+++ src/maasserver/models/dhcplease.py	2012-09-03 11:22:18 +0000
@@ -12,7 +12,6 @@
 __metaclass__ = type
 __all__ = [
     'DHCPLease',
-    'post_updates'
     ]
 
 
@@ -23,14 +22,10 @@
     Manager,
     Model,
     )
-from django.dispatch import Signal
 from maasserver import DefaultMeta
 from maasserver.fields import MACAddressField
 from maasserver.models.cleansave import CleanSave
 
-# A signal indicating that the record of leases has changed.
-post_updates = Signal()
-
 
 class DHCPLeaseManager(Manager):
     """Utility that manages :class:`DHCPLease` objects.
@@ -102,9 +97,13 @@
             deleted.
         :return: Iterable of IP addresses that were newly leased.
         """
+        # Avoid circular imports.
+        from maasserver import dns
+
         self._delete_obsolete_leases(nodegroup, leases)
         new_leases = self._add_missing_leases(nodegroup, leases)
-        post_updates.send(sender=self)
+        if len(new_leases) > 0:
+            dns.change_dns_zones([nodegroup])
         return new_leases
 
     def get_hostname_ip_mapping(self, nodegroup):

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2012-09-03 06:12:55 +0000
+++ src/maasserver/models/nodegroup.py	2012-09-03 11:22:18 +0000
@@ -141,6 +141,9 @@
     ip_range_high = IPAddressField(
         editable=True, unique=True, blank=True, null=True, default='')
 
+    def __repr__(self):
+        return "<NodeGroup %r>" % self.name
+
     def set_up_dhcp(self):
         """Write the DHCP configuration file and restart the DHCP server."""
         # Circular imports.

=== modified file 'src/maasserver/tests/test_dhcplease.py'
--- src/maasserver/tests/test_dhcplease.py	2012-09-03 04:59:58 +0000
+++ src/maasserver/tests/test_dhcplease.py	2012-09-03 11:22:18 +0000
@@ -12,14 +12,11 @@
 __metaclass__ = type
 __all__ = []
 
-from maasserver.models.dhcplease import (
-    DHCPLease,
-    post_updates,
-    )
+from maasserver import dns
+from maasserver.models import DHCPLease
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
 from maasserver.utils import ignore_unused
-from maastesting.fakemethod import FakeMethod
 
 
 def get_leases(nodegroup):
@@ -153,16 +150,18 @@
             },
             map_leases(nodegroup))
 
-    def test_update_leases_fires_signal(self):
-        # A call to DHCPLease.objects.update_leases fires the 'post_updates'
-        # signal.
-        recorder = FakeMethod()
-        post_updates.connect(recorder, sender=DHCPLease.objects)
-        self.addCleanup(
-            post_updates.disconnect, recorder, sender=DHCPLease.objects)
+    def test_update_leases_updates_dns_zone(self):
+        self.patch(dns, 'change_dns_zones')
+        nodegroup = factory.make_node_group()
+        DHCPLease.objects.update_leases(
+            nodegroup, factory.make_random_leases())
+        dns.change_dns_zones.assert_called_once_with([nodegroup])
+
+    def test_update_leases_does_not_update_dns_zone_if_nothing_added(self):
+        self.patch(dns, 'change_dns_zones')
         nodegroup = factory.make_node_group()
         DHCPLease.objects.update_leases(nodegroup, {})
-        self.assertEqual(1, recorder.call_count)
+        self.assertFalse(dns.change_dns_zones.called)
 
     def test_get_hostname_ip_mapping_returns_mapping(self):
         nodegroup = factory.make_node_group()

=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py	2012-08-30 07:53:32 +0000
+++ src/maasserver/tests/test_dns.py	2012-09-03 11:22:18 +0000
@@ -25,10 +25,7 @@
     server_address,
     )
 from maasserver.models import Config
-from maasserver.models.dhcplease import (
-    DHCPLease,
-    post_updates,
-    )
+from maasserver.models.dhcplease import DHCPLease
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
 from maastesting.bindfixture import BINDServer
@@ -154,9 +151,8 @@
         lease = factory.make_dhcp_lease(
             nodegroup=nodegroup, mac=mac.mac_address, ip=lease_ip)
         # Simulate that this lease was created by
-        # DHCPLease.objects.update_leases: fire the 'post_updates'
-        # signal.
-        post_updates.send(sender=DHCPLease.objects)
+        # DHCPLease.objects.update_leases: update its DNS config.
+        dns.change_dns_zones([nodegroup])
         return nodegroup, node, lease
 
     def dig_resolve(self, fqdn):

=== modified file 'src/provisioningserver/tests/test_utils.py'
--- src/provisioningserver/tests/test_utils.py	2012-09-03 05:20:39 +0000
+++ src/provisioningserver/tests/test_utils.py	2012-09-03 11:22:18 +0000
@@ -22,7 +22,6 @@
 import StringIO
 from subprocess import (
     CalledProcessError,
-    PIPE,
     Popen,
     )
 import sys