Merge lp:~gmb/maas/add-ptr-records-bug-1382190 into lp:~maas-committers/maas/trunk

[Graham Binns]

Add DNS PTR entries in both forward and reverse zones for hosts which are not registered in MAAS. This is particularly useful for LXC containers, which can't be treated as nodes by MAAS, but which sometimes require a resolvable hostname.

There is a lag of up to 60 seconds between the DHCP lease being acquired by the host and the DNS entries appearing, but that's reasonably acceptable as the kinds of hosts that this is aimed at take a while to spin up, and the DNS entries should be in place by the time they need them.

I'm not wholly sure about some bits of this change, but it's late, I'm tired, and I'm happy to fix things up on Monday.

Particular things that niggle me:

 - Naming of the method on DHCPLeaseManager (yeah, yeah, three hard
   problems, etc.)
 - The query in get_dynamic_hostname_ip_mapping().
 - There's probably a builtin somewhere that does the same as
   make_dynamic_hostname_from_ip_address(), but I couldn't find it.

[Graham Binns (gmb)]

Oh, and: I've tested this on my NUCs as follows:

$ juju bootstrap
$ juju deploy mysql --to lxc:0

Check /etc/bind/maas/zone.<whatever your network looks like> and zone.maas.

zone.maas should contain a forward entry for dynamic-<IP of LXC>.maas; the reverse zone file should contain the PTR record.

[Graham Binns (gmb)]

Oh, and: I've tested this on my NUCs as follows:

$ juju bootstrap
$ juju deploy mysql --to lxc:0

Check /etc/bind/maas/zone.<whatever your network looks like> and zone.maas.

zone.maas should contain a forward entry for dynamic-<IP of LXC>.maas; the reverse zone file should contain the PTR record.

[Graham Binns (gmb)]

Oh, and: I've tested this on my NUCs as follows:

$ juju bootstrap
$ juju deploy mysql --to lxc:0

Check /etc/bind/maas/zone.<whatever your network looks like> and zone.maas.

zone.maas should contain a forward entry for dynamic-<IP of LXC>.maas; the reverse zone file should contain the PTR record.

[Graham Binns (gmb)]

Oh, and: I've tested this on my NUCs as follows:

$ juju bootstrap
$ juju deploy mysql --to lxc:0

Check /etc/bind/maas/zone.<whatever your network looks like> and zone.maas.

zone.maas should contain a forward entry for dynamic-<IP of LXC>.maas; the reverse zone file should contain the PTR record.

[Christian Reis (kiko)]

Incidentally, is there some guarantee that this will take "up to 60 seconds"?

[Julian Edwards (julian-edwards)]

On Thursday 23 Oct 2014 23:44:50 you wrote:
> Incidentally, is there some guarantee that this will take "up to 60
> seconds"?

Yes, that's how often the lease scanner runs.

[Julian Edwards (julian-edwards)]

Considering this was a late hour change it looks pretty cromulent to me! Minor nits inline.

review: Approve

[Andres Rodriguez (andreserl)]

And this needs a backport to 1.7

review: Approve

[Christian Reis (kiko)]

I have some minor comments below, but what I'm missing is the GENERATE piece; did we decide not to use that in the end, or is there something more magical I'm missing?

[Christian Reis (kiko)]

I came back to the bug and understood your point. But there may be a critical issue: the first LXC takes time, but the second and beyond, as Julian noted, can be created very fast, particularly if we are running btrfs clones. Are we kidding ourselves here by adding code which will race with LXC?

review: Needs Information

[Julian Edwards (julian-edwards)]

On Friday 24 Oct 2014 17:43:04 you wrote:
> > + # Avoid circular imports.
>
> Does this usually require an XXX in MAAS?

No, XXXes follow the old Launchpad protocol of referencing a bug. Avoiding
circular imports is not a bug, just a nuisance, and we prefer to leave a small
comment.

[Julian Edwards (julian-edwards)]

Me, on #juju-dev:

<bigjools> hey folks, what's the quickest you've seen an LXC come up, given its image is already cached?
<stokachu> bigjools: 32 seconds

[Graham Binns (gmb)]

On 27 October 2014 00:59, Julian Edwards <email address hidden> wrote:
>
> <bigjools> hey folks, what's the quickest you've seen an LXC come up, given its image is already cached?
> <stokachu> bigjools: 32 seconds

Okay, we definitely need to test this further. One thing, though: it's
not LXC that we care about here so much as Juju: can we give a
container DNS before the juju agent comes up and the charm starts to
need it?

I'll test this this morning.

[Raphaël Badin (rvb)]

> I came back to the bug and understood your point. But there may be a critical
> issue: the first LXC takes time, but the second and beyond, as Julian noted,
> can be created very fast, particularly if we are running btrfs clones. Are we
> kidding ourselves here by adding code which will race with LXC?

The leases parser can be slow when the leases file is huge and this is why we only run it every minute. Now, the actual parsing is only done when the file has changed so if we could measure the performance of the parser on huge leases files (like the one we get from OIL) maybe it would make sense to run the parser more often (like every 10s) to avoid races.

[Graham Binns (gmb)]

> Me, on #juju-dev:
>
> <bigjools> hey folks, what's the quickest you've seen an LXC come up, given
> its image is already cached?
> <stokachu> bigjools: 32 seconds

I've been testing this for a couple of hours on my local MAAS. AFAICT although LXC containers come up very quickly, Juju agents (and charms) on those containers don't; in my tests it took well over five minutes from a `juju deploy` or `juju add-unit` command for the Juju agent on a container to transition out of "pending", and even longer for the charm to start. That's more than enough time for DNS to be updated by MAAS.

[Graham Binns (gmb)]

Sorry, "well over five minutes" is a bit misleading; I've just had one agent come up in 04:50. Still plenty of time for MAAS to sort out DNS for the container though.

[Gavin Panella (allenap)]

If the container is rebooted, how long does that take before it comes back up?

Does Juju start up a lot quicker after a reboot? IIRC, on first installation, Juju dist-upgrades the machine, which would account for much of the slow start. If it's not doing that after a reboot it might come up much quicker, and thus outrun MAAS.

Does a RabbitMQ server, for example, deployed via charm, actually have to wait for Juju to start after a reboot? Does Upstart bring it up? The point again being that it could start before MAAS has a chance to refresh DNS.

There's a race here whichever way we look at it. Unless MAAS can be made fast enough to always win that race, we're going to have unreliable behaviour. I don't think we can make MAAS quick enough right now, and so pre-generating names is the only viable approach.

[Graham Binns (gmb)]

On 27 October 2014 11:25, Gavin Panella <email address hidden> wrote:
> There's a race here whichever way we look at it. Unless MAAS can be made fast enough to always win that race, we're going to have unreliable behaviour. I don't think we can make MAAS quick enough right now, and so pre-generating names is the only viable approach.

I agree, but I'd rather get this approach tested in a real-world
environment to see if it's sufficient, rather than just dumping it and
going back to the old, less efficient, and more constrained way of
doing things.

[Gavin Panella (allenap)]

> > I came back to the bug and understood your point. But there may be a
> > critical issue: the first LXC takes time, but the second and beyond,
> > as Julian noted, can be created very fast, particularly if we are
> > running btrfs clones. Are we kidding ourselves here by adding code
> > which will race with LXC?
>
> The leases parser can be slow when the leases file is huge and this is
> why we only run it every minute. Now, the actual parsing is only done
> when the file has changed so if we could measure the performance of
> the parser on huge leases files (like the one we get from OIL) maybe
> it would make sense to run the parser more often (like every 10s) to
> avoid races.

With a limited set of MAC addresses -- like in a network with a fixed
number of machines -- the lease parser can be quite fast because it can
consider only the last lease for a particular MAC address.

When the set of MAC addresses is large and volatile -- like when there
are a lot of containers, and they're coming and going -- the lease
parser has to process a lot more of the leases file, and can get very
slow.

At core, I think we would all agree that the lease parser is slow and
deserves to be profiled and tuned, or rewritten. However, it is well
defined and tested so we have a good reference against which to prove a
new parser.

Another option is to incrementally parse the leases file. Monitor it
(via polling or inotify) and parse only what changes. It's guaranteed to
be append-only, so it doesn't need to be particularly clever code,
though it is rotated every hour. QA on this is likely to be the bulk of
the work, to make sure it works correctly during start, stop, crash,
rotate, continuous running, upgrade, and so on.

[Gavin Panella (allenap)]

> > I came back to the bug and understood your point. But there may be a
> > critical issue: the first LXC takes time, but the second and beyond,
> > as Julian noted, can be created very fast, particularly if we are
> > running btrfs clones. Are we kidding ourselves here by adding code
> > which will race with LXC?
>
> The leases parser can be slow when the leases file is huge and this is
> why we only run it every minute. Now, the actual parsing is only done
> when the file has changed so if we could measure the performance of
> the parser on huge leases files (like the one we get from OIL) maybe
> it would make sense to run the parser more often (like every 10s) to
> avoid races.

With a limited set of MAC addresses -- like in a network with a fixed
number of machines -- the lease parser can be quite fast because it can
consider only the last lease for a particular MAC address.

When the set of MAC addresses is large and volatile -- like when there
are a lot of containers, and they're coming and going -- the lease
parser has to process a lot more of the leases file, and can get very
slow.

At core, I think we would all agree that the lease parser is slow and
deserves to be profiled and tuned, or rewritten. However, it is well
defined and tested so we have a good reference against which to prove a
new parser.

Another option is to incrementally parse the leases file. Monitor it
(via polling or inotify) and parse only what changes. It's guaranteed to
be append-only, so it doesn't need to be particularly clever code,
though it is rotated every hour. QA on this is likely to be the bulk of
the work, to make sure it works correctly during start, stop, crash,
rotate, continuous running, upgrade, and so on.

[Christian Reis (kiko)]

I don't understand why generating IPs using a GENERATE expression is more constrained. Is there a reason why you chose to not use that (and wildcard DNS IPv6) apart from the fact that it is painful to handle netmasks that aren't a multiple of 8?

Generally I also see it as a negative depending on the leases file for yet another critical piece of MAAS functionality.

[Graham Binns (gmb)]

On 27 October 2014 11:56, Christian Reis <email address hidden> wrote:
> I don't understand why generating IPs using a GENERATE expression is more constrained. Is there a reason why you chose to not use that (and wildcard DNS IPv6) apart from the fact that it is painful to handle netmasks that aren't a multiple of 8?

It makes it painful to handle any network bigger than a /16, (although
as Gavin pointed out, that makes whoever's setting up the /8 or
whatever a bit crazy, and we can't code against crazy). The suggestion
was that we'd re-introduce the max-size of /16 for an IPv4 network to
accommodate this. Wildcard DNS for IPv6 is lovely, though, and I'd be
happy to use it.

> Generally I also see it as a negative depending on the leases file for yet another critical piece of MAAS functionality.

That's a fair point.

[Christian Reis (kiko)]

On Mon, Oct 27, 2014 at 12:03:23PM -0000, Graham Binns wrote:
> On 27 October 2014 11:56, Christian Reis <email address hidden> wrote:
> > I don't understand why generating IPs using a GENERATE expression is more constrained. Is there a reason why you chose to not use that (and wildcard DNS IPv6) apart from the fact that it is painful to handle netmasks that aren't a multiple of 8?
>
> It makes it painful to handle any network bigger than a /16, (although
> as Gavin pointed out, that makes whoever's setting up the /8 or
> whatever a bit crazy, and we can't code against crazy). The suggestion
> was that we'd re-introduce the max-size of /16 for an IPv4 network to
> accommodate this. Wildcard DNS for IPv6 is lovely, though, and I'd be
> happy to use it.

I'm +1 on restricting the dynamic pool to the max size of a /16.
--
Christian Robottom Reis | [+1] 612 888 4935 | http://launchpad.net/~kiko
Canonical VP Hyperscale | [+55 16] 9 9112 6430 | http://async.com.br/~kiko

[Graham Binns (gmb)]

> On Mon, Oct 27, 2014 at 12:03:23PM -0000, Graham Binns wrote:
> > On 27 October 2014 11:56, Christian Reis <email address hidden> wrote:
> > > I don't understand why generating IPs using a GENERATE expression is more
> constrained. Is there a reason why you chose to not use that (and wildcard DNS
> IPv6) apart from the fact that it is painful to handle netmasks that aren't a
> multiple of 8?
> >
> > It makes it painful to handle any network bigger than a /16, (although
> > as Gavin pointed out, that makes whoever's setting up the /8 or
> > whatever a bit crazy, and we can't code against crazy). The suggestion
> > was that we'd re-introduce the max-size of /16 for an IPv4 network to
> > accommodate this. Wildcard DNS for IPv6 is lovely, though, and I'd be
> > happy to use it.
>
> I'm +1 on restricting the dynamic pool to the max size of a /16.

Okay. I'm on it.

[Andres Rodriguez (andreserl)]

Ok, what about the time it takes to actually install everything needed
before actually running the charm?
On Oct 26, 2014 8:59 PM, "Julian Edwards" <email address hidden>
wrote:

> Me, on #juju-dev:
>
> <bigjools> hey folks, what's the quickest you've seen an LXC come up,
> given its image is already cached?
> <stokachu> bigjools: 32 seconds
>
> --
>
> https://code.launchpad.net/~gmb/maas/add-ptr-records-bug-1382190/+merge/239493
> You are reviewing the proposed merge of
> lp:~gmb/maas/add-ptr-records-bug-1382190 into lp:maas.
>

[Graham Binns (gmb)]

> If the container is rebooted, how long does that take before it comes back up?
>
> Does Juju start up a lot quicker after a reboot? IIRC, on first installation,
> Juju dist-upgrades the machine, which would account for much of the slow
> start. If it's not doing that after a reboot it might come up much quicker,
> and thus outrun MAAS.
>
> Does a RabbitMQ server, for example, deployed via charm, actually have to wait
> for Juju to start after a reboot? Does Upstart bring it up? The point again
> being that it could start before MAAS has a chance to refresh DNS.

I've tested this locally this afternoon and rebooting the container is very, very fast… Rabbit seemed to come up almost immediately (though it didn't break, because the DHCP lease was still current and the container's MAC hadn't changed).

I think that this approach, whilst neat and elegant, *does* in fact run the risk of breakages down the line, maybe if an LXC is stopped and then restarted across lease boundaries.

The fix that Kiko et. al. are suggesting is less pleasing to the eye, but it has the advantage of being far more robust in the face of failure, and also causes far less churn of the DNS zone files.

I'm coming to the conclusion that, unless there's significant evidence to the contrary, they're right this approach is not quite up to the standard we require for robustness.

[Christian Reis (kiko)]

Here's what I am thinking. If we are able to implement the GENERATE/wildcard approach and are happy with it by this Wednesday then we will take that into 1.7RC1 on Thursday. If we are not at the point where it fits, we could land the patch in this MP onto 1.7 and work on the proper fix for the next point release. This latter scenario means that there will be a corner case in 1.7 (that the LXC container may race with MAAS) that we'd release note accordingly.

[Graham Binns (gmb)]

On 27 October 2014 15:04, Christian Reis <email address hidden> wrote:
> Here's what I am thinking. If we are able to implement the GENERATE/wildcard approach and are happy with it by this Wednesday then we will take that into 1.7RC1 on Thursday. If we are not at the point where it fits, we could land the patch in this MP onto 1.7 and work on the proper fix for the next point release. This latter scenario means that there will be a corner case in 1.7 (that the LXC container may race with MAAS) that we'd release note accordingly.

I'm happy with that. Thanks for clarifying your thoughts.

I'm already working on a branch to create the $GENERATE lines; should
be done by lunchtime tomorrow with that. I'll then do the wildcard
entries.

[Julian Edwards (julian-edwards)]

On Monday 27 Oct 2014 12:09:17 you wrote:
> On Mon, Oct 27, 2014 at 12:03:23PM -0000, Graham Binns wrote:
> > On 27 October 2014 11:56, Christian Reis <email address hidden> wrote:
> > > I don't understand why generating IPs using a GENERATE expression is
> > > more constrained. Is there a reason why you chose to not use that (and
> > > wildcard DNS IPv6) apart from the fact that it is painful to handle
> > > netmasks that aren't a multiple of 8?>
> > It makes it painful to handle any network bigger than a /16, (although
> > as Gavin pointed out, that makes whoever's setting up the /8 or
> > whatever a bit crazy, and we can't code against crazy). The suggestion
> > was that we'd re-introduce the max-size of /16 for an IPv4 network to
> > accommodate this. Wildcard DNS for IPv6 is lovely, though, and I'd be
> > happy to use it.
>
> I'm +1 on restricting the dynamic pool to the max size of a /16.

I think we need to avoid doing this.

For a /16 network, it added a huge amount of overhead and load onto the
process that writes the DNS zone files as it's now having to write the pre-
generated entries *every time another entry changes*.

We are still yet to explore a couple more options:

1. Juju starts using the API to get a static IP and passes it to cloud-init.

2. Use DDNS on the DHCPD to update our own DNS server directly, since MAAS
doesn't need to know the IPs itself for LXCs.

I think #2 is a trivial solution here, it's a simple piece of config.

[Julian Edwards (julian-edwards)]

On Monday 27 Oct 2014 11:45:09 you wrote:
> Another option is to incrementally parse the leases file. Monitor it
> (via polling or inotify) and parse only what changes. It's guaranteed to
> be append-only, so it doesn't need to be particularly clever code,
> though it is rotated every hour. QA on this is likely to be the bulk of
> the work, to make sure it works correctly during start, stop, crash,
> rotate, continuous running, upgrade, and so on.

Right, I've said this for a while now :)

Make a separate daemon or thread that uses inotify to keep up with the leases
file; watch for file rewrites (which is harded coded in dhcpd as one hour);
keep track of active leases in the daemon.

However, I reckon we can get rid of the leases parser altogether if we use
DDNS as per my previous suggestion. I'll experiment with this today to see
what I can do.

[Julian Edwards (julian-edwards)]

I just played around with DDNS. It turns out you can't use it with zones where you are using rndc to reload the zone, the rndc command is refused :(

This seems odd. Anyway I tried adding a new zone and it all gets complex quite quickly, so I'm going to discount this as a quick solution.

[Christian Reis (kiko)]

> On Monday 27 Oct 2014 12:09:17 you wrote:
> > I'm +1 on restricting the dynamic pool to the max size of a /16.
>
> I think we need to avoid doing this.
>
> For a /16 network, it added a huge amount of overhead and load onto the
> process that writes the DNS zone files as it's now having to write the pre-
> generated entries *every time another entry changes*.

I don't think that's actually true if you are using GENERATE (for IPv4) and wildcard (for IPv6), is it?

> 1. Juju starts using the API to get a static IP and passes it to cloud-init.

We considered this, and it should happen, but it won't happen for 1.7.

> 2. Use DDNS on the DHCPD to update our own DNS server directly, since MAAS
> doesn't need to know the IPs itself for LXCs.
>
> I think #2 is a trivial solution here, it's a simple piece of config.

This may be a good solution, but have we ever seen that working?

[Julian Edwards (julian-edwards)]

On Tuesday 28 Oct 2014 12:52:05 you wrote:
> > On Monday 27 Oct 2014 12:09:17 you wrote:
> > > I'm +1 on restricting the dynamic pool to the max size of a /16.
> >
> > I think we need to avoid doing this.
> >
> > For a /16 network, it added a huge amount of overhead and load onto the
> > process that writes the DNS zone files as it's now having to write the
> > pre-
> > generated entries *every time another entry changes*.
>
> I don't think that's actually true if you are using GENERATE (for IPv4) and
> wildcard (for IPv6), is it?

It should be OK for that, but I thought gmb had eschewed it in favour of
writing zone files? Or did that flip again.

> > 1. Juju starts using the API to get a static IP and passes it to
> > cloud-init.
> We considered this, and it should happen, but it won't happen for 1.7.

Right. I am worried as to how long we'll have to support this legacy GENERATE
stuff.

>
> > 2. Use DDNS on the DHCPD to update our own DNS server directly, since MAAS
> > doesn't need to know the IPs itself for LXCs.
> >
> > I think #2 is a trivial solution here, it's a simple piece of config.
>
> This may be a good solution, but have we ever seen that working?

As per my other emails, it's a PITA to get working.

=== modified file 'src/maasserver/dns/tests/test_zonegenerator.py'
--- src/maasserver/dns/tests/test_zonegenerator.py	2014-09-15 14:28:28 +0000
+++ src/maasserver/dns/tests/test_zonegenerator.py	2014-10-27 11:52:58 +0000
@@ -36,6 +36,7 @@
 from maasserver.models import Config
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
+from maasserver.utils import make_dynamic_hostname_from_ip_address
 from maastesting.fakemethod import FakeMethod
 from maastesting.matchers import (
     MockAnyCall,
@@ -199,10 +200,17 @@
         node2 = factory.make_node_with_mac_attached_to_nodegroupinterface(
             nodegroup=nodegroup)
         staticip = factory.make_StaticIPAddress(mac=node2.get_primary_mac())
+        # Create dynamic mapping for a host that's not registered as a Node
+        # in MAAS (e.g. an LXC container).
+        non_node_lease = factory.make_DHCPLease(
+            nodegroup=nodegroup, mac=factory.make_mac_address())
+        non_node_hostname = make_dynamic_hostname_from_ip_address(
+            non_node_lease.ip)
 
         expected_mapping = {
             node1.hostname: [lease.ip],
             node2.hostname: [staticip.ip],
+            non_node_hostname: [non_node_lease.ip],
         }
         self.assertEqual(
             expected_mapping, get_hostname_ip_mapping(nodegroup))

=== modified file 'src/maasserver/dns/zonegenerator.py'
--- src/maasserver/dns/zonegenerator.py	2014-08-28 12:00:15 +0000
+++ src/maasserver/dns/zonegenerator.py	2014-10-27 11:52:58 +0000
@@ -84,9 +84,12 @@
     from maasserver.models.staticipaddress import StaticIPAddress
     from maasserver.models.dhcplease import DHCPLease
     mapping = DHCPLease.objects.get_hostname_ip_mapping(nodegroup)
-    static_mapping = StaticIPAddress.objects.get_hostname_ip_mapping(nodegroup)
+    dynamic_mapping = DHCPLease.objects.get_dynamic_hostname_ip_mapping(
+        nodegroup)
+    mapping.update(dynamic_mapping)
     # The static mapping entries take precedence over the dynamic mapping
     # entries.
+    static_mapping = StaticIPAddress.objects.get_hostname_ip_mapping(nodegroup)
     mapping.update(static_mapping)
     return mapping
 

=== modified file 'src/maasserver/models/dhcplease.py'
--- src/maasserver/models/dhcplease.py	2014-09-01 13:32:35 +0000
+++ src/maasserver/models/dhcplease.py	2014-10-27 11:52:58 +0000
@@ -33,7 +33,10 @@
     )
 from maasserver.models.cleansave import CleanSave
 from maasserver.models.macaddress import MACAddress
-from maasserver.utils import strip_domain
+from maasserver.utils import (
+    make_dynamic_hostname_from_ip_address,
+    strip_domain,
+    )
 
 
 class DHCPLeaseManager(Manager):
@@ -106,8 +109,13 @@
             deleted.
         :return: Iterable of IP addresses that were newly leased.
         """
+        # Avoid circular imports.
+        from maasserver.dns.config import change_dns_zones
+
         self._delete_obsolete_leases(nodegroup, leases)
         new_leases = self._add_missing_leases(nodegroup, leases)
+        if len(new_leases) > 0:
+            change_dns_zones(nodegroup)
         return new_leases
 
     def get_hostname_ip_mapping(self, nodegroup):
@@ -146,6 +154,26 @@
             for hostname, ip in cursor.fetchall()
             }
 
+    def get_dynamic_hostname_ip_mapping(self, nodegroup):
+        """Return a mapping {hostnames -> ips} for the currently leased
+        IP addresses for MAC addresses not attached to a Node in `nodegroup`.
+
+        This allows us to generate DNS records on-the-fly for hosts
+        which take a dynamic address but aren't treated as Nodes in MAAS
+        (e.g. LXC containers)..
+
+        Any domain will be stripped from the hostnames.
+        """
+        mac_addresses_with_nodes = [
+            mac.mac_address for mac in MACAddress.objects.all()]
+        leases_on_cluster = DHCPLease.objects.filter(nodegroup=nodegroup)
+        dynamic_leases = leases_on_cluster.exclude(
+            mac__in=mac_addresses_with_nodes)
+        return {
+            make_dynamic_hostname_from_ip_address(lease.ip): [lease.ip]
+            for lease in dynamic_leases
+            }
+
 
 class DHCPLease(CleanSave, Model):
     """A known mapping of an IP address to a MAC address.

=== modified file 'src/maasserver/models/tests/test_dhcplease.py'
--- src/maasserver/models/tests/test_dhcplease.py	2014-09-18 12:44:38 +0000
+++ src/maasserver/models/tests/test_dhcplease.py	2014-10-27 11:52:58 +0000
@@ -24,7 +24,16 @@
 from maasserver.models import DHCPLease
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
-from maasserver.utils import ignore_unused
+from maasserver.utils import (
+    ignore_unused,
+    make_dynamic_hostname_from_ip_address,
+    )
+from maastesting.matchers import MockCalledOnceWith
+from testtools.matchers import (
+    Contains,
+    Equals,
+    Not,
+    )
 
 
 def get_leases(nodegroup):
@@ -166,10 +175,20 @@
             map_leases(nodegroup))
 
     def test_update_leases_does_not_update_dns_zone_if_nothing_added(self):
-        self.patch(dns, 'change_dns_zones')
+        self.patch(dns.config, 'change_dns_zones')
         nodegroup = factory.make_NodeGroup()
         DHCPLease.objects.update_leases(nodegroup, {})
-        self.assertFalse(dns.change_dns_zones.called)
+        self.assertFalse(dns.config.change_dns_zones.called)
+
+    def test_update_leases_updates_dns_zone_if_leases_added(self):
+        change_dns_zones = self.patch(dns.config, 'change_dns_zones')
+        nodegroup = factory.make_NodeGroup()
+        ip_address = factory.make_ipv4_address()
+        mac_address = factory.make_mac_address()
+        DHCPLease.objects.update_leases(
+            nodegroup, {ip_address: mac_address})
+        self.assertThat(
+            change_dns_zones, MockCalledOnceWith(nodegroup))
 
     def test_get_hostname_ip_mapping_returns_mapping(self):
         nodegroup = factory.make_NodeGroup()
@@ -255,3 +274,45 @@
         mapping = DHCPLease.objects.get_hostname_ip_mapping(
             another_nodegroup)
         self.assertEqual({}, mapping)
+
+
+class TestDHCPLeaseManager_GetDynamicHostnameMapping(MAASServerTestCase):
+
+    def test_get_dynamic_hostname_ip_mapping_returns_mapping(self):
+        nodegroup = factory.make_NodeGroup()
+        expected_mapping = {}
+        for _ in range(3):
+            mac = factory.make_MAC()
+            lease = factory.make_DHCPLease(
+                nodegroup=nodegroup, mac=unicode(mac))
+            hostname = make_dynamic_hostname_from_ip_address(lease.ip)
+            expected_mapping[hostname] = [lease.ip]
+        mapping = DHCPLease.objects.get_dynamic_hostname_ip_mapping(nodegroup)
+        self.assertEqual(expected_mapping, mapping)
+
+    def test_get_dynamic_hostname_ip_mapping_ignores_known_nodes(self):
+        nodegroup = factory.make_NodeGroup()
+        expected_mapping = {}
+
+        node = factory.make_Node(nodegroup=nodegroup)
+        node_mac = factory.make_MACAddress(node=node)
+        factory.make_DHCPLease(
+            nodegroup=nodegroup, mac=node_mac.mac_address)
+
+        mac = factory.make_MAC()
+        lease = factory.make_DHCPLease(
+            nodegroup=nodegroup, mac=unicode(mac))
+        hostname = make_dynamic_hostname_from_ip_address(lease.ip)
+        expected_mapping[hostname] = [lease.ip]
+
+        mapping = DHCPLease.objects.get_dynamic_hostname_ip_mapping(nodegroup)
+        self.expectThat(mapping, Equals(expected_mapping))
+
+    def test_get_dynamic_hostname_ip_mapping_considers_given_nodegroup(self):
+        nodegroup = factory.make_NodeGroup()
+        mac = factory.make_MAC()
+        factory.make_DHCPLease(nodegroup=nodegroup, mac=unicode(mac))
+        another_nodegroup = factory.make_NodeGroup()
+        mapping = DHCPLease.objects.get_dynamic_hostname_ip_mapping(
+            another_nodegroup)
+        self.assertEqual({}, mapping)

=== modified file 'src/maasserver/utils/__init__.py'
--- src/maasserver/utils/__init__.py	2014-10-04 08:36:12 +0000
+++ src/maasserver/utils/__init__.py	2014-10-27 11:52:58 +0000
@@ -19,6 +19,7 @@
     'get_db_state',
     'get_local_cluster_UUID',
     'ignore_unused',
+    'make_dynamic_hostname_from_ip_address',
     'make_validation_error_message',
     'strip_domain',
     'synchronised',
@@ -35,6 +36,10 @@
 from maasserver.enum import NODEGROUPINTERFACE_MANAGEMENT
 from maasserver.exceptions import NodeGroupMisconfiguration
 from maasserver.utils.orm import get_one
+from netaddr import (
+    AddrConversionError,
+    IPAddress,
+    )
 from provisioningserver.utils.text import make_bullet_list
 
 
@@ -214,3 +219,17 @@
     The message takes the form of a textual bullet-list.
     """
     return make_bullet_list(gen_validation_error_messages(error))
+
+
+def make_dynamic_hostname_from_ip_address(ip):
+    ip_address = IPAddress(ip)
+    try:
+        address = ip_address.ipv4().format()
+    except AddrConversionError:
+        address = ip_address.ipv6().format()
+        hostname = address.lstrip("::")
+        hostname = hostname.replace("::", "-")
+        hostname = hostname.replace(":", "-")
+        return "dynamic-%s" % hostname
+    else:
+        return "dynamic-%s" % address.replace('.', '-')

=== modified file 'src/maasserver/utils/tests/test_utils.py'
--- src/maasserver/utils/tests/test_utils.py	2014-10-04 08:36:12 +0000
+++ src/maasserver/utils/tests/test_utils.py	2014-10-27 11:52:58 +0000
@@ -36,6 +36,7 @@
     find_nodegroup,
     get_db_state,
     get_local_cluster_UUID,
+    make_dynamic_hostname_from_ip_address,
     make_validation_error_message,
     strip_domain,
     synchronised,
@@ -348,3 +349,29 @@
             "* alice: bob\n"
             "* foo: bar",
             make_validation_error_message(error))
+
+
+class TestMakeDynamicHostnameFromIPAddress(MAASTestCase):
+
+    def test_makes_hostname_from_ipv4_address(self):
+        ip_address = factory.make_ipv4_address()
+        ip_parts = ip_address.split('.')
+        expected_hostname = "dynamic-%s-%s-%s-%s" % tuple(ip_parts)
+        self.assertEqual(
+            expected_hostname,
+            make_dynamic_hostname_from_ip_address(ip_address))
+
+    def test_makes_hostname_from_ipv6_address(self):
+        ip_address = factory.make_ipv6_address()
+        ip_parts = ip_address.split(':')
+        expected_hostname = "dynamic-%s" % "-".join(ip_parts)
+        self.assertEqual(
+            expected_hostname,
+            make_dynamic_hostname_from_ip_address(ip_address))
+
+    def test_makes_hostname_from_truncated_ipv6_address(self):
+        ip_address = '::ffff:ffff:ffff:ffff:7'
+        expected_hostname = "dynamic-ffff-ffff-ffff-ffff-7"
+        self.assertEqual(
+            expected_hostname,
+            make_dynamic_hostname_from_ip_address(ip_address))