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

Proposed by Julian Edwards on 2014-10-20
Status: Merged
Approved by: Julian Edwards on 2014-10-21
Approved revision: 3277
Merged at revision: 3275
Proposed branch: lp:~julian-edwards/maas/no-hostmaps-when-unmanaged-bug-1382108
Merge into: lp:maas/trunk
Diff against target: 88 lines (+49/-4)
2 files modified
src/maasserver/models/node.py (+18/-4)
src/maasserver/models/tests/test_node.py (+31/-0)
To merge this branch: bzr merge lp:~julian-edwards/maas/no-hostmaps-when-unmanaged-bug-1382108
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve on 2014-10-20
Graham Binns 2014-10-20 Pending
Review via email: mp+238846@code.launchpad.net

Commit message

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)

To post a comment you must log in.
Raphaël Badin (rvb) :
review: Approve
Julian Edwards (julian-edwards) wrote :

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) wrote :
Download full text (32.7 KiB)

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...

3277. By Julian Edwards on 2014-10-21

damn you linter

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/node.py'
2--- src/maasserver/models/node.py 2014-10-17 13:14:58 +0000
3+++ src/maasserver/models/node.py 2014-10-21 01:17:12 +0000
4@@ -1553,6 +1553,14 @@
5
6 return self.macaddress_set.first()
7
8+ def is_pxe_mac_on_managed_interface(self):
9+ pxe_mac = self.get_pxe_mac()
10+ if pxe_mac is not None:
11+ cluster_interface = pxe_mac.cluster_interface
12+ if cluster_interface is not None:
13+ return cluster_interface.is_managed
14+ return False
15+
16 def start(self, by_user, user_data=None):
17 """Request on given user's behalf that the node be started up.
18
19@@ -1584,10 +1592,16 @@
20 if self.status == NODE_STATUS.ALLOCATED:
21 static_mappings = defaultdict(dict)
22 claims = self.claim_static_ip_addresses()
23- static_mappings[self.nodegroup].update(claims)
24- update_host_maps_failures = list(update_host_maps(static_mappings))
25- if len(update_host_maps_failures) != 0:
26- raise MultipleFailures(*update_host_maps_failures)
27+ # If the PXE mac is on a managed interface then we can ask
28+ # the cluster to generate the DHCP host map(s).
29+ if self.is_pxe_mac_on_managed_interface():
30+ static_mappings[self.nodegroup].update(claims)
31+ update_host_maps_failures = list(
32+ update_host_maps(static_mappings))
33+ if len(update_host_maps_failures) != 0:
34+ raise MultipleFailures(*update_host_maps_failures)
35+
36+ if self.status == NODE_STATUS.ALLOCATED:
37 self.start_deployment()
38
39 # Update the DNS zone with the new static IP info as necessary.
40
41=== modified file 'src/maasserver/models/tests/test_node.py'
42--- src/maasserver/models/tests/test_node.py 2014-10-20 02:42:53 +0000
43+++ src/maasserver/models/tests/test_node.py 2014-10-21 01:17:12 +0000
44@@ -1843,6 +1843,28 @@
45 self.assertEqual(node.macaddress_set.first(), node.get_pxe_mac())
46
47
48+class TestNode_pxe_mac_on_managed_interface(MAASServerTestCase):
49+
50+ def test__returns_true_if_managed(self):
51+ node = factory.make_node_with_mac_attached_to_nodegroupinterface()
52+ self.assertTrue(node.is_pxe_mac_on_managed_interface())
53+
54+ def test__returns_false_if_no_pxe_mac(self):
55+ node = factory.make_Node()
56+ self.assertFalse(node.is_pxe_mac_on_managed_interface())
57+
58+ def test__returns_false_if_no_attached_cluster_interface(self):
59+ node = factory.make_Node()
60+ node.pxe_mac = factory.make_MACAddress(node=node)
61+ node.save()
62+ self.assertFalse(node.is_pxe_mac_on_managed_interface())
63+
64+ def test__returns_false_if_cluster_interface_unmanaged(self):
65+ node = factory.make_node_with_mac_attached_to_nodegroupinterface(
66+ management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
67+ self.assertFalse(node.is_pxe_mac_on_managed_interface())
68+
69+
70 class NodeRoutersTest(MAASServerTestCase):
71
72 def test_routers_stores_mac_address(self):
73@@ -2676,6 +2698,15 @@
74 # isn't ALLOCATED.
75 self.assertThat(claim_static_ip_addresses, MockNotCalled())
76
77+ def test__does_not_generate_host_maps_if_not_on_managed_interface(self):
78+ user = factory.make_User()
79+ node = self.make_acquired_node_with_mac(user)
80+ self.patch(
81+ node, 'is_pxe_mac_on_managed_interface').return_value = False
82+ update_host_maps = self.patch(node_module, "update_host_maps")
83+ node.start(user)
84+ self.assertThat(update_host_maps, MockNotCalled())
85+
86 def test__updates_host_maps(self):
87 user = factory.make_User()
88 node = self.make_acquired_node_with_mac(user)