Merge lp:~julian-edwards/maas/no-hostmaps-when-unmanaged-bug-1382108 into lp:~maas-committers/maas/trunk

[Julian Edwards]

Don't write DHCP host maps when starting a node if its PXE MAC is not on a managed cluster interface, they are not needed. This also removes a related crash (see bug 1382108)

[Raphaël Badin (rvb)]

review: Approve

[Julian Edwards (julian-edwards)]

On Monday 20 Oct 2014 09:47:19 you wrote:
> Review: Approve

Thanks for reviewing.

> Best to use cluster_interface.is_managed here.

D'oh, good spot!

[MAAS Lander (maas-lander)]

The attempt to merge lp:~julian-edwards/maas/no-hostmaps-when-unmanaged-bug-1382108 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://security.ubuntu.com trusty-security Release
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools gjs ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python-openssl python-paramiko python-pexpect python-pip python-pocket-lint python-psycopg2 python-pyinotify python-seamicroclient python-simplejson python-s...

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2014-10-17 13:14:58 +0000
+++ src/maasserver/models/node.py	2014-10-21 01:17:12 +0000
@@ -1553,6 +1553,14 @@
 
         return self.macaddress_set.first()
 
+    def is_pxe_mac_on_managed_interface(self):
+        pxe_mac = self.get_pxe_mac()
+        if pxe_mac is not None:
+            cluster_interface = pxe_mac.cluster_interface
+            if cluster_interface is not None:
+                return cluster_interface.is_managed
+        return False
+
     def start(self, by_user, user_data=None):
         """Request on given user's behalf that the node be started up.
 
@@ -1584,10 +1592,16 @@
         if self.status == NODE_STATUS.ALLOCATED:
             static_mappings = defaultdict(dict)
             claims = self.claim_static_ip_addresses()
-            static_mappings[self.nodegroup].update(claims)
-            update_host_maps_failures = list(update_host_maps(static_mappings))
-            if len(update_host_maps_failures) != 0:
-                raise MultipleFailures(*update_host_maps_failures)
+            # If the PXE mac is on a managed interface then we can ask
+            # the cluster to generate the DHCP host map(s).
+            if self.is_pxe_mac_on_managed_interface():
+                static_mappings[self.nodegroup].update(claims)
+                update_host_maps_failures = list(
+                    update_host_maps(static_mappings))
+                if len(update_host_maps_failures) != 0:
+                    raise MultipleFailures(*update_host_maps_failures)
+
+        if self.status == NODE_STATUS.ALLOCATED:
             self.start_deployment()
 
         # Update the DNS zone with the new static IP info as necessary.

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2014-10-20 02:42:53 +0000
+++ src/maasserver/models/tests/test_node.py	2014-10-21 01:17:12 +0000
@@ -1843,6 +1843,28 @@
         self.assertEqual(node.macaddress_set.first(), node.get_pxe_mac())
 
 
+class TestNode_pxe_mac_on_managed_interface(MAASServerTestCase):
+
+    def test__returns_true_if_managed(self):
+        node = factory.make_node_with_mac_attached_to_nodegroupinterface()
+        self.assertTrue(node.is_pxe_mac_on_managed_interface())
+
+    def test__returns_false_if_no_pxe_mac(self):
+        node = factory.make_Node()
+        self.assertFalse(node.is_pxe_mac_on_managed_interface())
+
+    def test__returns_false_if_no_attached_cluster_interface(self):
+        node = factory.make_Node()
+        node.pxe_mac = factory.make_MACAddress(node=node)
+        node.save()
+        self.assertFalse(node.is_pxe_mac_on_managed_interface())
+
+    def test__returns_false_if_cluster_interface_unmanaged(self):
+        node = factory.make_node_with_mac_attached_to_nodegroupinterface(
+            management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
+        self.assertFalse(node.is_pxe_mac_on_managed_interface())
+
+
 class NodeRoutersTest(MAASServerTestCase):
 
     def test_routers_stores_mac_address(self):
@@ -2676,6 +2698,15 @@
         # isn't ALLOCATED.
         self.assertThat(claim_static_ip_addresses, MockNotCalled())
 
+    def test__does_not_generate_host_maps_if_not_on_managed_interface(self):
+        user = factory.make_User()
+        node = self.make_acquired_node_with_mac(user)
+        self.patch(
+            node, 'is_pxe_mac_on_managed_interface').return_value = False
+        update_host_maps = self.patch(node_module, "update_host_maps")
+        node.start(user)
+        self.assertThat(update_host_maps, MockNotCalled())
+
     def test__updates_host_maps(self):
         user = factory.make_User()
         node = self.make_acquired_node_with_mac(user)