Merge lp:~mpontillo/maas/update-host-maps-cleanup-bug-1440102 into lp:~maas-committers/maas/trunk

Proposed by Mike Pontillo
Status: Merged
Approved by: Mike Pontillo
Approved revision: no longer in the source branch.
Merged at revision: 4020
Proposed branch: lp:~mpontillo/maas/update-host-maps-cleanup-bug-1440102
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 1794 lines (+399/-244)
15 files modified
src/maasserver/api/devices.py (+4/-23)
src/maasserver/api/ip_addresses.py (+7/-9)
src/maasserver/api/nodes.py (+1/-7)
src/maasserver/api/tests/test_devices.py (+17/-17)
src/maasserver/api/tests/test_ipaddresses.py (+19/-13)
src/maasserver/api/tests/test_node.py (+7/-6)
src/maasserver/models/macaddress.py (+141/-38)
src/maasserver/models/macipaddresslink.py (+2/-0)
src/maasserver/models/node.py (+41/-24)
src/maasserver/models/signals/dns.py (+10/-10)
src/maasserver/models/tests/test_macaddress.py (+65/-43)
src/maasserver/models/tests/test_node.py (+48/-35)
src/maasserver/websockets/handlers/device.py (+4/-4)
src/maasserver/websockets/handlers/tests/test_device.py (+23/-14)
src/maastesting/testcase.py (+10/-1)
To merge this branch: bzr merge lp:~mpontillo/maas/update-host-maps-cleanup-bug-1440102
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+261351@code.launchpad.net

Commit message

Move responsibility for calling update_host_maps() and update_dns_zones() away from the caller, and into the model.

To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote :

I'm putting this up for review even though there is a little more cleanup to be done, so people can get started looking at it. All the test cases are passing in this iteration, though I'll be doing some manual testing and running it through CI as well, to ensure it's okay.

This change consolidates the logic for updating host maps and DNS zones within the claim_static_ips() and set_static_ip() methods inside macaddress.py. (Sounds easy? Not really, because lots of tests depend on using a mock to forego updating DNS and/or host maps, depending on their particular use case.)

I'm not completely happy with this change, mainly because moving around dozens of mocks makes it extremely hard to refactor this code.

What I'd like to do, given the time, is to change all callers of update_host_maps(), update_dns_zones() (and any related functions) to call via a module, instead of importing a function. This way when we mock the calls, we already know what to mock, and we can refactor as much as we like without impacting the dozens of tests that care about it.

Also on the TODO list is changing the signature of the update_nodegroup_host_maps() function, which is currently an awkward (nodegroup, claims, nodegroups=None). (I extracted the method in a hurry.)

Revision history for this message
Mike Pontillo (mpontillo) wrote :

Updated update_nodegroup_host_maps() signature. (it now just takes a list of nodegroups, but the old update_host_maps() will remain the same and ignore the node's nodegroup if passed in, because it breaks test cases without that behavior; even though I'm not positive the behavior is correct, I don't want to risk it.)

Revision history for this message
Mike Pontillo (mpontillo) wrote :

The other thing I don't like about this change is that updating the model suddenly has far-reaching side effects.

I made the side effect explicit by passing in the "update_host_maps" boolean to a few functions. I might have preferred a "dependency injection" style approach, but at least this gives callers (especially tests) the option to avoid the side effect.

If the side-effect was easy to mock in a consistent way (as I suggested by saying it should be called via its own module), I think it would be okay to remove the boolean. But in this case, adding the boolean prevented an even worse explosion in terms of test cases that needed to be updated.

Revision history for this message
Mike Pontillo (mpontillo) wrote :

Note: CI failed with the following errors:

2015-06-11 18:40:58 ERROR juju.provider.common bootstrap.go:122 bootstrap failed: cannot start bootstrap instance: gomaasapi: got error back from server: 500 INTERNAL SERVER ERROR ('MAC' object has no attribute 'encode')
2015-06-11 18:40:58 INFO juju.cmd cmd.go:113 Bootstrap failed, destroying environment
2015-06-11 18:40:58 INFO juju.provider.common destroy.go:15 destroying environment "maas"
2015-06-11 18:41:02 ERROR juju.cmd supercommand.go:323 cannot start bootstrap instance: gomaasapi: got error back from server: 500 INTERNAL SERVER ERROR ('MAC' object has no attribute 'encode')

I'm looking into this...

Revision history for this message
Raphaël Badin (rvb) wrote :

> Note: CI failed with the following errors:
>
> 2015-06-11 18:40:58 ERROR juju.provider.common bootstrap.go:122 bootstrap
> failed: cannot start bootstrap instance: gomaasapi: got error back from
> server: 500 INTERNAL SERVER ERROR ('MAC' object has no attribute 'encode')
> 2015-06-11 18:40:58 INFO juju.cmd cmd.go:113 Bootstrap failed, destroying
> environment
> 2015-06-11 18:40:58 INFO juju.provider.common destroy.go:15 destroying
> environment "maas"
> 2015-06-11 18:41:02 ERROR juju.cmd supercommand.go:323 cannot start bootstrap
> instance: gomaasapi: got error back from server: 500 INTERNAL SERVER ERROR
> ('MAC' object has no attribute 'encode')
>
> I'm looking into this...

I found the problem: CreateHostMaps expects its 'mac_address' parameter(s)to be something that can be converted into unicode (see CreateHostMaps's signature in ps/rpc/cluster.py). But here you're passing the raw MAC (maasserver/fields.py) object instead of passing the mac address as a string. That is because you removed the code http://paste.ubuntu.com/11700836/. I assume the same problem will happen for the IP address.

Revision history for this message
Raphaël Badin (rvb) :
Revision history for this message
Gavin Panella (allenap) wrote :

No objections, but I don't understand what the problem was and how this improves the situation. Some background please :)

review: Needs Information
Revision history for this message
Raphaël Badin (rvb) wrote :

@Gavin: have a look at the related bug for some context. But in a nutshell this is trying to reduce some code duplication that was added at the end of last cycle. The code in question is the code that does: claim_ip/update host map. It's duplicated in a couple of places, we've got a bunch of slightly different code that does very similar things at different levels in the tree (similar code in the API and in the model). This code will be changed when we move to the networking model and this is an attempt at rationalizing it before we even have to change it.

Revision history for this message
Raphaël Badin (rvb) wrote :

@Mike: you missed the create() method in websockets/handlers/device.py. It needs refactoring as well. There is even a mention of the bug you're fixing in that code. If applying your refactoring there improves this code, this is definitely proof that this change is all for the better.

Revision history for this message
Gavin Panella (allenap) wrote :

I don't much like the update_host_maps boolean flag, but it's okay; if we figure out a better way it'll be easy to change.

review: Approve
Revision history for this message
Mike Pontillo (mpontillo) wrote :

@Gavin, yes, I don't like it either, but I think it's a step in the right direction. There are a couple of places where we need it still; specifically where we combine the use of this function with "commit_within_atomic_block()" and try to make (for example) the creation of an IP address + reservation "atomic" (manually).

Another thing I noticed is, this code would be much easier to refactor if we didn't import *specific functions* so much. If we call via indirection (i.e. import maasserver.dhcp, dhcp.update_host_maps) it would have made it orders of magnitude easier to update all these tests. (and update them again in the future.)

Revision history for this message
Mike Pontillo (mpontillo) wrote :

I think this is ready to land, but I'm doing some final testing first.

I've changed the way the mocks are done for most of the affected tests; rather than using string constants, I've added a way to "self.patch()" using just a symbol. This requires importing a module instead of a function, but I think it's worth it. (When I first joined the MAAS team, the way mocks were used really violated the principle of least surprise for me... and the alternatives don't seem DRY-compliant.)

Revision history for this message
Mike Pontillo (mpontillo) wrote :

This version didn't pass CI. I think I missed a spot where I need to do MAC(...) instead of assuming it's the proper type. (and not, say, unicode). Line 716 in the current diff:

ip_mapping = [(static_ip.ip, self.mac_address.get_raw())]

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (77.9 KiB)

The attempt to merge lp:~mpontillo/maas/update-host-maps-cleanup-bug-1440102 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Hit http://security.ubuntu.com trusty-security Release
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:1 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Hit http://security.ubuntu.com trusty-security/main Sources
Get:2 http://nova.clouds.archive.ubuntu.com trusty-updates Release [63.5 kB]
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [207 kB]
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [120 kB]
Get:5 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [537 kB]
Get:6 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [286 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/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
Fetched 1,214 kB in 3s (376 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 phantomjs postgresql pyflakes python-apt python-bson python-bzrlib python-convoy python-coverage 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-iscpy python-jinja2 python-jsonschema python-lockfile python-lxml python-mock python-netaddr python-netifaces python-nose python-oauth python-openssl python-paramiko ...

Revision history for this message
Mike Pontillo (mpontillo) wrote :

The failure was in:

provisioningserver.rpc.tests.test_clusterservice.TestClusterProtocol_PowerQuery.test_returns_power_state

This seems to be a flaky test case. It fails for me about ~10-20% of the time.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api/devices.py'
2--- src/maasserver/api/devices.py 2015-04-23 17:15:45 +0000
3+++ src/maasserver/api/devices.py 2015-06-15 08:11:13 +0000
4@@ -21,7 +21,6 @@
5 OperationsHandler,
6 )
7 from maasserver.api.utils import get_optional_list
8-from maasserver.dns.config import dns_update_zones
9 from maasserver.enum import (
10 IPADDRESS_TYPE,
11 NODE_PERMISSION,
12@@ -37,10 +36,7 @@
13 DeviceWithMACsForm,
14 ReleaseIPForm,
15 )
16-from maasserver.models import (
17- MACAddress,
18- NodeGroup,
19-)
20+from maasserver.models import MACAddress
21 from maasserver.models.node import Device
22 from piston.utils import rc
23
24@@ -185,27 +181,12 @@
25 if requested_address is None:
26 sticky_ips = mac_address.claim_static_ips(
27 alloc_type=IPADDRESS_TYPE.STICKY,
28- requested_address=requested_address)
29- claims = [
30- (static_ip.ip, mac_address.mac_address.get_raw())
31- for static_ip in sticky_ips]
32- device.update_host_maps(claims)
33+ requested_address=requested_address, update_host_maps=True)
34 else:
35 sticky_ip = mac_address.set_static_ip(
36- requested_address, request.user)
37+ requested_address, request.user, update_host_maps=True)
38 sticky_ips = [sticky_ip]
39- dhcp_managed_clusters = [
40- cluster
41- for cluster in NodeGroup.objects.all()
42- if cluster.manages_dhcp()
43- ]
44- device.update_host_maps(
45- [(sticky_ip.ip, mac_address.mac_address.get_raw())],
46- dhcp_managed_clusters)
47- # Use the master cluster DNS zone for devices.
48- # This is a temporary measure until we support setting a specific
49- # zone for each MAC.
50- dns_update_zones([NodeGroup.objects.ensure_master()])
51+
52 maaslog.info(
53 "%s: Sticky IP address(es) allocated: %s", device.hostname,
54 ', '.join(allocation.ip for allocation in sticky_ips))
55
56=== modified file 'src/maasserver/api/ip_addresses.py'
57--- src/maasserver/api/ip_addresses.py 2015-06-10 11:20:09 +0000
58+++ src/maasserver/api/ip_addresses.py 2015-06-15 08:11:13 +0000
59@@ -26,10 +26,7 @@
60 get_mandatory_param,
61 get_optional_param,
62 )
63-from maasserver.clusterrpc.dhcp import (
64- remove_host_maps,
65- update_host_maps,
66-)
67+from maasserver.clusterrpc import dhcp
68 from maasserver.enum import IPADDRESS_TYPE
69 from maasserver.exceptions import (
70 MAASAPIBadRequest,
71@@ -89,6 +86,8 @@
72 alloc_type=IPADDRESS_TYPE.USER_RESERVED,
73 requested_address=requested_address,
74 user=user, hostname=hostname)
75+ from maasserver.dns import config as dns_config
76+ dns_config.dns_update_zones([interface.nodegroup])
77 maaslog.info("User %s was allocated IP %s", user.username, sip.ip)
78 else:
79 # The user has requested a static IP linked to a MAC
80@@ -116,10 +115,11 @@
81 sip.ip: mac_address.mac_address,
82 }
83 }
84+
85 # Commit the DB changes before we do RPC calls.
86 commit_within_atomic_block()
87 update_host_maps_failures = list(
88- update_host_maps(host_map_updates))
89+ dhcp.update_host_maps(host_map_updates))
90 if len(update_host_maps_failures) > 0:
91 # Deallocate the static IPs and delete the MAC address
92 # if it doesn't have a Node attached.
93@@ -130,6 +130,7 @@
94
95 # There will only ever be one error, so raise that.
96 raise update_host_maps_failures[0].value
97+
98 maaslog.info(
99 "User %s was allocated IP %s for MAC address %s",
100 user.username, sip.ip, mac_address.mac_address)
101@@ -201,9 +202,6 @@
102 request.user, ngi, requested_address, mac_address,
103 hostname=hostname)
104
105- from maasserver.dns.config import dns_update_zones
106- dns_update_zones([ngi.nodegroup])
107-
108 return sip
109
110 @operation(idempotent=False)
111@@ -235,7 +233,7 @@
112 for interface in linked_mac_address_interfaces
113 }
114 remove_host_maps_failures = list(
115- remove_host_maps(host_maps_to_remove))
116+ dhcp.remove_host_maps(host_maps_to_remove))
117 if len(remove_host_maps_failures) > 0:
118 # There's only going to be one failure, so raise that.
119 raise remove_host_maps_failures[0].value
120
121=== modified file 'src/maasserver/api/nodes.py'
122--- src/maasserver/api/nodes.py 2015-05-28 09:38:40 +0000
123+++ src/maasserver/api/nodes.py 2015-06-15 08:11:13 +0000
124@@ -40,7 +40,6 @@
125 get_optional_param,
126 )
127 from maasserver.clusterrpc.power_parameters import get_power_types
128-from maasserver.dns.config import dns_update_zones
129 from maasserver.enum import (
130 IPADDRESS_TYPE,
131 NODE_PERMISSION,
132@@ -549,12 +548,7 @@
133
134 sticky_ips = mac_address.claim_static_ips(
135 alloc_type=IPADDRESS_TYPE.STICKY,
136- requested_address=requested_address)
137- claims = [
138- (static_ip.ip, mac_address.mac_address.get_raw())
139- for static_ip in sticky_ips]
140- node.update_host_maps(claims)
141- dns_update_zones([node.nodegroup])
142+ requested_address=requested_address, update_host_maps=True)
143 maaslog.info(
144 "%s: Sticky IP address(es) allocated: %s", node.hostname,
145 ', '.join(allocation.ip for allocation in sticky_ips))
146
147=== modified file 'src/maasserver/api/tests/test_devices.py'
148--- src/maasserver/api/tests/test_devices.py 2015-05-16 06:21:20 +0000
149+++ src/maasserver/api/tests/test_devices.py 2015-06-15 08:11:13 +0000
150@@ -21,7 +21,7 @@
151
152 from django.core.urlresolvers import reverse
153 from django.db import transaction
154-from maasserver.api import devices as api_devices
155+from maasserver.dns import config as dns_config
156 from maasserver.enum import (
157 IPADDRESS_TYPE,
158 NODE_STATUS,
159@@ -281,7 +281,7 @@
160 installable=False, parent=parent, mac=True, disable_ipv4=False,
161 owner=self.logged_in_user)
162 # Silence 'update_host_maps'.
163- self.patch(node_module, "update_host_maps")
164+ self.patch(Node.update_host_maps)
165 response = self.client.post(
166 get_device_uri(device), {'op': 'claim_sticky_ip_address'})
167 self.assertEqual(httplib.OK, response.status_code, response.content)
168@@ -299,8 +299,8 @@
169 device = factory.make_Node(
170 installable=False, parent=parent, mac=True, disable_ipv4=False,
171 owner=self.logged_in_user, nodegroup=parent.nodegroup)
172- dns_update_zones = self.patch(api_devices, 'dns_update_zones')
173- update_host_maps = self.patch(node_module, "update_host_maps")
174+ dns_update_zones = self.patch(dns_config.dns_update_zones)
175+ update_host_maps = self.patch(Node.update_host_maps)
176 update_host_maps.return_value = [] # No failures.
177 response = self.client.post(
178 get_device_uri(device), {'op': 'claim_sticky_ip_address'})
179@@ -327,7 +327,7 @@
180 device = factory.make_Node(
181 installable=False, parent=parent, mac=True, disable_ipv4=False,
182 owner=factory.make_User())
183- self.patch(node_module, "update_host_maps")
184+ self.patch(Node.update_host_maps)
185 response = self.client.post(
186 get_device_uri(device), {'op': 'claim_sticky_ip_address'})
187 self.assertEqual(httplib.FORBIDDEN, response.status_code)
188@@ -339,7 +339,7 @@
189 installable=False, mac=True, disable_ipv4=False,
190 owner=self.logged_in_user)
191 # Silence 'update_host_maps'.
192- self.patch(node_module, "update_host_maps")
193+ self.patch(Node.update_host_maps)
194 response = self.client.post(
195 get_device_uri(device),
196 {
197@@ -364,7 +364,7 @@
198 owner=self.logged_in_user)
199 second_mac = factory.make_MACAddress(node=device)
200 # Silence 'update_host_maps'.
201- self.patch(node_module, "update_host_maps")
202+ self.patch(Node.update_host_maps)
203 response = self.client.post(
204 get_device_uri(device),
205 {
206@@ -383,6 +383,8 @@
207 self.assertItemsEqual([second_mac], given_ip.macaddress_set.all())
208
209 def test_creates_host_DHCP_and_DNS_mappings_with_given_ip(self):
210+ dns_update_zones = self.patch(dns_config.dns_update_zones)
211+ update_host_maps = self.patch(Node.update_host_maps)
212 # Create a nodegroup for which we manage DHCP.
213 factory.make_NodeGroup(
214 status=NODEGROUP_STATUS.ENABLED,
215@@ -394,8 +396,6 @@
216 device = factory.make_Node(
217 installable=False, mac=True, disable_ipv4=False,
218 owner=self.logged_in_user)
219- dns_update_zones = self.patch(api_devices, 'dns_update_zones')
220- update_host_maps = self.patch(node_module, "update_host_maps")
221 update_host_maps.return_value = [] # No failures.
222 requested_address = factory.make_ip_address()
223 response = self.client.post(
224@@ -473,7 +473,7 @@
225 installable=False, mac=True, disable_ipv4=False,
226 owner=self.logged_in_user)
227 # Silence 'update_host_maps'.
228- self.patch(node_module, "update_host_maps")
229+ self.patch(Node.update_host_maps)
230 response = self.client.post(
231 get_device_uri(device),
232 {
233@@ -494,8 +494,8 @@
234 installable=False, parent=parent, mac=True, disable_ipv4=False,
235 owner=self.logged_in_user)
236 # Silence 'update_host_maps' and 'remove_host_maps'
237- self.patch(node_module, "update_host_maps")
238- self.patch(node_module, "remove_host_maps")
239+ self.patch(Node.update_host_maps)
240+ self.patch(node_module, node_module.remove_host_maps.__name__)
241 response = self.client.post(
242 get_device_uri(device), {'op': 'claim_sticky_ip_address'})
243 self.assertEqual(httplib.OK, response.status_code, response.content)
244@@ -549,8 +549,8 @@
245 installable=False, mac_count=5, network=network,
246 disable_ipv4=False, owner=self.logged_in_user)
247 # Silence 'update_host_maps' and 'remove_host_maps'
248- self.patch(node_module, "update_host_maps")
249- self.patch(node_module, "remove_host_maps")
250+ self.patch(Node.update_host_maps)
251+ self.patch(node_module, node_module.remove_host_maps.__name__)
252 self.assertThat(MACAddress.objects.all(), HasLength(5))
253 for mac in MACAddress.objects.all():
254 with transaction.atomic():
255@@ -569,8 +569,8 @@
256 installable=False, mac_count=2, network=network,
257 disable_ipv4=False, owner=self.logged_in_user)
258 # Silence 'update_host_maps' and 'remove_host_maps'
259- self.patch(node_module, "update_host_maps")
260- self.patch(node_module, "remove_host_maps")
261+ self.patch(Node.update_host_maps)
262+ self.patch(node_module, node_module.remove_host_maps.__name__)
263 self.assertThat(MACAddress.objects.all(), HasLength(2))
264 ips = []
265 for mac in MACAddress.objects.all():
266@@ -596,7 +596,7 @@
267 installable=False, parent=parent, mac=True, disable_ipv4=False,
268 owner=factory.make_User())
269 # Silence 'update_host_maps' and 'remove_host_maps'
270- self.patch(node_module, "update_host_maps")
271+ self.patch(Node.update_host_maps)
272 self.patch(node_module, "remove_host_maps")
273 with transaction.atomic():
274 device.claim_static_ip_addresses(alloc_type=IPADDRESS_TYPE.STICKY)
275
276=== modified file 'src/maasserver/api/tests/test_ipaddresses.py'
277--- src/maasserver/api/tests/test_ipaddresses.py 2015-05-16 06:21:20 +0000
278+++ src/maasserver/api/tests/test_ipaddresses.py 2015-06-15 08:11:13 +0000
279@@ -18,7 +18,7 @@
280 import json
281
282 from django.core.urlresolvers import reverse
283-from maasserver.api import ip_addresses as ip_addresses_module
284+from maasserver.clusterrpc import dhcp as dhcp_module
285 from maasserver.dns import config as dns_config_module
286 from maasserver.enum import (
287 IPADDRESS_TYPE,
288@@ -104,7 +104,7 @@
289 self.assertEqual(self.logged_in_user, staticipaddress.user)
290
291 def test_POST_reserve_with_MAC_links_MAC_to_ip_address(self):
292- update_host_maps = self.patch(ip_addresses_module, 'update_host_maps')
293+ update_host_maps = self.patch(dhcp_module, 'update_host_maps')
294 interface = self.make_interface()
295 net = interface.network
296 mac = factory.make_mac_address()
297@@ -124,7 +124,7 @@
298 {interface.nodegroup: {returned_address['ip']: mac}}))
299
300 def test_POST_reserve_with_MAC_returns_503_if_hostmap_update_fails(self):
301- update_host_maps = self.patch(ip_addresses_module, 'update_host_maps')
302+ update_host_maps = self.patch(dhcp_module, 'update_host_maps')
303 # We a specific exception here because update_host_maps() will
304 # fail with RPC-specific errors.
305 update_host_maps.return_value = [
306@@ -149,7 +149,7 @@
307 def test_POST_returns_CONFLICT_when_static_ip_for_MAC_already_exists(self):
308 interface = self.make_interface()
309 mac = factory.make_MACAddress(cluster_interface=interface)
310- mac.claim_static_ips()
311+ mac.claim_static_ips(update_host_maps=False)
312 net = interface.network
313
314 response = self.post_reservation_request(net=net, mac=mac.mac_address)
315@@ -161,7 +161,7 @@
316 Equals(1))
317
318 def test_POST_allows_claiming_of_new_static_ips_for_existing_MAC(self):
319- self.patch(ip_addresses_module, 'update_host_maps')
320+ self.patch(dhcp_module, 'update_host_maps')
321
322 interface = self.make_interface()
323 net = interface.network
324@@ -363,36 +363,40 @@
325 self.assertIsNone(reload_object(ipaddress))
326
327 def test_POST_release_deletes_floating_MAC_address(self):
328- self.patch(ip_addresses_module, 'remove_host_maps')
329+ self.patch(dhcp_module, dhcp_module.remove_host_maps.__name__)
330
331 interface = self.make_interface()
332 floating_mac = factory.make_MACAddress(cluster_interface=interface)
333 [ipaddress] = floating_mac.claim_static_ips(
334- alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=self.logged_in_user)
335+ alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=self.logged_in_user,
336+ update_host_maps=False)
337
338 self.post_release_request(ipaddress.ip)
339 self.assertIsNone(reload_object(floating_mac))
340
341 def test_POST_release_does_not_delete_MACs_linked_to_nodes(self):
342- self.patch(ip_addresses_module, 'remove_host_maps')
343+ self.patch(dhcp_module, dhcp_module.remove_host_maps.__name__)
344
345 interface = self.make_interface()
346 node = factory.make_Node(nodegroup=interface.nodegroup)
347 attached_mac = factory.make_MACAddress(
348 node=node, cluster_interface=interface)
349 [ipaddress] = attached_mac.claim_static_ips(
350- alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=self.logged_in_user)
351+ alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=self.logged_in_user,
352+ update_host_maps=False)
353
354 self.post_release_request(ipaddress.ip)
355 self.assertEqual(attached_mac, reload_object(attached_mac))
356
357 def test_POST_release_updates_DNS_and_DHCP(self):
358- remove_host_maps = self.patch(ip_addresses_module, 'remove_host_maps')
359+ remove_host_maps = self.patch(
360+ dhcp_module, dhcp_module.remove_host_maps.__name__)
361
362 interface = self.make_interface()
363 floating_mac = factory.make_MACAddress(cluster_interface=interface)
364 [ipaddress] = floating_mac.claim_static_ips(
365- alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=self.logged_in_user)
366+ alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=self.logged_in_user,
367+ update_host_maps=False)
368
369 self.post_release_request(ipaddress.ip)
370 self.expectThat(
371@@ -400,7 +404,8 @@
372 {interface.nodegroup: [ipaddress.ip]}))
373
374 def test_POST_release_raises_503_if_removing_host_maps_errors(self):
375- remove_host_maps = self.patch(ip_addresses_module, 'remove_host_maps')
376+ remove_host_maps = self.patch(
377+ dhcp_module, dhcp_module.remove_host_maps.__name__)
378 # Failures in remove_host_maps() will be RPC-related exceptions,
379 # so we use one of those explicitly.
380 remove_host_maps.return_value = [
381@@ -412,7 +417,8 @@
382 interface = self.make_interface()
383 floating_mac = factory.make_MACAddress(cluster_interface=interface)
384 [ipaddress] = floating_mac.claim_static_ips(
385- alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=self.logged_in_user)
386+ alloc_type=IPADDRESS_TYPE.USER_RESERVED, user=self.logged_in_user,
387+ update_host_maps=False)
388
389 response = self.post_release_request(ipaddress.ip)
390 self.expectThat(
391
392=== modified file 'src/maasserver/api/tests/test_node.py'
393--- src/maasserver/api/tests/test_node.py 2015-05-28 13:26:45 +0000
394+++ src/maasserver/api/tests/test_node.py 2015-06-15 08:11:13 +0000
395@@ -24,7 +24,7 @@
396 from django.core.urlresolvers import reverse
397 from django.db import transaction
398 from maasserver import forms
399-from maasserver.api import nodes as api_nodes
400+from maasserver.dns import config as dns_config
401 from maasserver.enum import (
402 IPADDRESS_TYPE,
403 NODE_BOOT,
404@@ -1195,9 +1195,9 @@
405 self.become_admin()
406 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface()
407 # Silence 'update_host_maps'.
408- self.patch(node_module, "update_host_maps")
409+ self.patch(Node.update_host_maps)
410 [existing_ip] = node.get_primary_mac().claim_static_ips(
411- alloc_type=IPADDRESS_TYPE.STICKY)
412+ alloc_type=IPADDRESS_TYPE.STICKY, update_host_maps=False)
413 response = self.client.post(
414 self.get_node_uri(node), {'op': 'claim_sticky_ip_address'})
415 self.assertEqual(httplib.OK, response.status_code, response.content)
416@@ -1214,7 +1214,8 @@
417 random_alloc_type = factory.pick_enum(
418 IPADDRESS_TYPE,
419 but_not=[IPADDRESS_TYPE.STICKY, IPADDRESS_TYPE.USER_RESERVED])
420- node.get_primary_mac().claim_static_ips(alloc_type=random_alloc_type)
421+ node.get_primary_mac().claim_static_ips(
422+ alloc_type=random_alloc_type, update_host_maps=False)
423 response = self.client.post(
424 self.get_node_uri(node), {'op': 'claim_sticky_ip_address'})
425 self.assertEqual(
426@@ -1265,7 +1266,7 @@
427 def test_claim_ip_address_creates_host_DHCP_and_DNS_mappings(self):
428 self.become_admin()
429 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface()
430- dns_update_zones = self.patch(api_nodes, 'dns_update_zones')
431+ dns_update_zones = self.patch(dns_config.dns_update_zones)
432 update_host_maps = self.patch(node_module, "update_host_maps")
433 update_host_maps.return_value = [] # No failures.
434 response = self.client.post(
435@@ -1371,7 +1372,7 @@
436 requested_address = IPAddress(ngi.static_ip_range_low) + 1
437 requested_address = requested_address.format()
438 other_node.get_primary_mac().claim_static_ips(
439- requested_address=requested_address)
440+ requested_address=requested_address, update_host_maps=False)
441
442 # Use the API to try to duplicate the same IP on the other node.
443 response = self.client.post(
444
445=== modified file 'src/maasserver/models/macaddress.py'
446--- src/maasserver/models/macaddress.py 2015-05-13 11:33:20 +0000
447+++ src/maasserver/models/macaddress.py 2015-06-15 08:11:13 +0000
448@@ -43,6 +43,7 @@
449 from maasserver.models.macipaddresslink import MACStaticIPAddressLink
450 from maasserver.models.managers import BulkManager
451 from maasserver.models.network import Network
452+from maasserver.models.nodegroup import NodeGroup
453 from maasserver.models.nodegroupinterface import NodeGroupInterface
454 from maasserver.models.staticipaddress import StaticIPAddress
455 from maasserver.models.timestampedmodel import TimestampedModel
456@@ -267,24 +268,48 @@
457 return self.node.parent.get_pxe_mac().cluster_interface
458 return None
459
460- def claim_static_ips(self, alloc_type=IPADDRESS_TYPE.AUTO,
461- requested_address=None, user=None):
462+ def _get_attached_clusters_with_static_ranges(self):
463+ """Returns a list of cluster interfaces attached to this MAC address,
464+ where each cluster interface has a defined static range.
465+ """
466+ return [
467+ interface
468+ for interface in self.get_cluster_interfaces()
469+ if interface.get_static_ip_range()
470+ ]
471+
472+ def _get_hostname_log_prefix(self):
473+ """Returns a string that represents the hostname for this MAC address,
474+ suitable for prepending to a log statement.
475+ """
476+ if self.node is not None:
477+ hostname_string = "%s: " % self.node.hostname
478+ else:
479+ hostname_string = ""
480+ return hostname_string
481+
482+ def claim_static_ips(
483+ self, alloc_type=IPADDRESS_TYPE.AUTO, requested_address=None,
484+ fabric=None, user=None, update_host_maps=True):
485 """Assign static IP addresses to this MAC.
486
487 Allocates one address per managed cluster interface connected to this
488- MAC. Typically this will be either just one IPv4 address, or an IPv4
489+ MAC. Typically this will be either just one IPv4 address, or an IPv4
490 address and an IPv6 address.
491+ Calls update_host_maps() on the related Node in order to update
492
493- It is the caller's responsibility to update the DHCP server.
494+ any DHCP mappings.
495
496 :param alloc_type: See :class:`StaticIPAddress`.alloc_type.
497 This parameter musn't be IPADDRESS_TYPE.USER_RESERVED.
498 :param requested_address: Optional IP address to claim. Must be in
499 the range defined on some cluster interface to which this
500- MACAddress is related. If given, no allocations will be made on
501+ MACAddress is related. If given, no allocations will be made on
502 any other cluster interfaces the MAC may be connected to.
503 :param user: Optional User who will be given ownership of any
504 `StaticIPAddress`es claimed.
505+ :param update_host_maps: If True, will update any relevant DHCP
506+ mappings in addition to allocating the address.
507 :return: A list of :class:`StaticIPAddress`. Returns empty if
508 the cluster_interface is not yet known, or the
509 static_ip_range_low/high values values are not set on the
510@@ -297,7 +322,12 @@
511 the cluster interface's defined range.
512 :raises: StaticIPAddressUnavailable if the requested_address is already
513 allocated.
514+ :raises: StaticIPAddressForbidden if the address occurs within
515+ an existing dynamic range within the specified fabric.
516 """
517+ if fabric is not None:
518+ raise NotImplementedError("Fabrics are not yet supported.")
519+
520 # This method depends on a database isolation level of SERIALIZABLE
521 # (or perhaps REPEATABLE READ) to avoid race conditions.
522
523@@ -307,19 +337,12 @@
524 # different representations for "none" values in IP addresses.
525 if self.get_cluster_interface() is None:
526 # No known cluster interface. Nothing we can do.
527- if self.node is not None:
528- hostname_string = "%s: " % self.node.hostname
529- else:
530- hostname_string = ""
531+ hostname_string = self._get_hostname_log_prefix()
532 maaslog.error(
533- "%s tried to allocate an IP to MAC %s but its cluster "
534+ "%sTried to allocate an IP to MAC %s, but its cluster "
535 "interface is not known", hostname_string, self)
536 return []
537- cluster_interfaces = [
538- interface
539- for interface in self.get_cluster_interfaces()
540- if interface.get_static_ip_range()
541- ]
542+ cluster_interfaces = self._get_attached_clusters_with_static_ranges()
543 if len(cluster_interfaces) == 0:
544 # There were cluster interfaces, but none of them had a static
545 # range. Can't allocate anything.
546@@ -328,6 +351,10 @@
547 if requested_address is not None:
548 # A specific IP address was requested. We restrict our attention
549 # to the cluster interface that is responsible for that address.
550+ # In addition, claiming addresses inside a dynamic range on the
551+ # requested fabric is not allowed.
552+ self._raise_if_address_inside_dynamic_range(
553+ requested_address, fabric)
554 cluster_interface = find_cluster_interface_responsible_for_ip(
555 cluster_interfaces, IPAddress(requested_address))
556 if cluster_interface is None:
557@@ -338,25 +365,34 @@
558
559 allocations = self._map_allocated_addresses(cluster_interfaces)
560
561- if None not in allocations.values():
562- # We already have a full complement of static IP addresses
563- # allocated. Check for a clash.
564- types = [sip.alloc_type for sip in allocations.values()]
565- if alloc_type not in types:
566- # None of the prior allocations are for the same type that's
567- # being requested now. This is a complete clash.
568- raise StaticIPAddressTypeClash(
569- "MAC address %s already has IP adresses of different "
570- "types than the ones requested." % self)
571+ # Check if we already have a full complement of static IP addresses
572+ # allocated, none of which are the same type.
573+ if (None not in allocations.values() and alloc_type not in
574+ [a.alloc_type for a in allocations.values()]):
575+ raise StaticIPAddressTypeClash(
576+ "MAC address %s already has IP addresses of different "
577+ "types than the ones requested." % self)
578
579+ new_allocations = []
580 # Allocate IP addresses on all relevant cluster interfaces where this
581 # MAC does not have any address allocated yet.
582 for interface in cluster_interfaces:
583 if allocations[interface] is None:
584- # No IP address yet on this cluster interface. Get one.
585- allocations[interface] = self._allocate_static_address(
586+ # No IP address yet on this cluster interface. Get one.
587+ static_ip = self._allocate_static_address(
588 interface, alloc_type, requested_address, user=user)
589+ allocations[interface] = static_ip
590+ mac_address = MAC(self.mac_address)
591+ new_allocations.append(
592+ (static_ip.ip, mac_address.get_raw()))
593
594+ # Note: the previous behavior of the product (MAAS < 1.8) was to
595+ # update host maps with *every* address, not just changed addresses.
596+ # This should only impact separately-claimed IPv6 and IPv4 addresses.
597+ if update_host_maps:
598+ if self.node is not None:
599+ self.node.update_host_maps(new_allocations)
600+ self.update_related_dns_zones()
601 # We now have a static IP allocated to each of our cluster interfaces.
602 # Ignore the clashes. Return the ones that have the right type: those
603 # are either matching pre-existing allocations or fresh ones.
604@@ -366,14 +402,69 @@
605 if sip.alloc_type == alloc_type
606 ]
607
608- def set_static_ip(self, requested_address, user):
609+ def _get_device_cluster_or_default(self):
610+ """Returns a cluster interface for this MAC, first by checking for a
611+ direct link, then by checking the parent node,
612+ (via get_cluster_interface()) and finally by getting the default,
613+ if all else fails.
614+ """
615+ cluster_interface = self.get_cluster_interface()
616+ if cluster_interface is not None:
617+ return cluster_interface.nodegroup
618+ else:
619+ return NodeGroup.objects.ensure_master()
620+
621+ def update_related_dns_zones(self):
622+ """Updates DNS for the cluster related to this MAC."""
623+ # Prevent circular imports
624+ from maasserver.dns import config as dns_config
625+ dns_config.dns_update_zones([self._get_device_cluster_or_default()])
626+
627+ def _raise_if_address_inside_dynamic_range(
628+ self, requested_address, fabric=None):
629+ """
630+ Checks if the specified IP address, inside the specified fabric,
631+ is inside a MAAS-managed dynamic range.
632+
633+ :raises: StaticIPAddressForbidden if the address occurs within
634+ an existing dynamic range within the specified fabric.
635+ """
636+ if fabric is not None:
637+ raise NotImplementedError("Fabrics are not yet supported.")
638+
639+ requested_address_ip = IPAddress(requested_address)
640+ for interface in NodeGroupInterface.objects.all():
641+ if interface.is_managed:
642+ dynamic_range = interface.get_dynamic_ip_range()
643+ if requested_address_ip in dynamic_range:
644+ raise StaticIPAddressForbidden(
645+ "Requested IP address %s is in a dynamic range." %
646+ requested_address)
647+
648+ def _get_dhcp_managed_clusters(self, fabric=None):
649+ """Returns the DHCP-managed clusters relevant to the specified fabric.
650+
651+ :param fabric: The fabric whose DHCP-managed clusters to update.
652+ """
653+ if fabric is not None:
654+ raise NotImplementedError("Fabrics are not yet supported.")
655+
656+ return [
657+ cluster
658+ for cluster in NodeGroup.objects.all()
659+ if cluster.manages_dhcp()
660+ ]
661+
662+ def set_static_ip(
663+ self, requested_address, user, fabric=None, update_host_maps=True):
664 """Assign a static (sticky) IP address to this MAC.
665
666 This is meant to be called on a device's MAC address: the IP address
667- can be anything. Only if the MAC is linked to a network will this
668+ can be anything. Only if the MAC is linked to a network will this
669 method enforce that the IP address if part of the referenced network.
670
671- It is the caller's responsibility to update the DHCP server.
672+ Calls update_host_maps() on the related Node in order to update
673+ any DHCP mappings.
674
675 :param requested_address: IP address to claim. Must not be in
676 the dynamic range of any cluster interface.
677@@ -390,6 +481,9 @@
678 :raises: StaticIPAddressUnavailable if the requested_address is already
679 allocated.
680 """
681+ if fabric is not None:
682+ raise NotImplementedError("Fabrics are not yet supported.")
683+
684 # If this MAC is linked to a cluster interface, make sure the
685 # requested_address is part of the cluster interface's network.
686 cluster_interface = self.get_cluster_interface()
687@@ -402,14 +496,7 @@
688
689 # Raise a StaticIPAddressForbidden exception if the requested_address
690 # is in a dynamic range.
691- requested_address_ip = IPAddress(requested_address)
692- for interface in NodeGroupInterface.objects.all():
693- if interface.is_managed:
694- dynamic_range = interface.get_dynamic_ip_range()
695- if requested_address_ip in dynamic_range:
696- raise StaticIPAddressForbidden(
697- "Requested IP address %s is in a dynamic range." %
698- requested_address)
699+ self._raise_if_address_inside_dynamic_range(requested_address, fabric)
700
701 # Allocate IP if it isn't allocated already.
702 static_ip, created = StaticIPAddress.objects.get_or_create(
703@@ -434,4 +521,20 @@
704 "Requested IP address %s is already allocated "
705 "to a different MAC address." %
706 requested_address)
707+
708+ if update_host_maps:
709+ # XXX:fabric We need to restrict this to cluster interfaces in the
710+ # appropriate fabric!
711+ if cluster_interface is not None:
712+ relevant_clusters = [cluster_interface.nodegroup]
713+ else:
714+ relevant_clusters = self._get_dhcp_managed_clusters(fabric)
715+
716+ mac_address = MAC(self.mac_address)
717+ ip_mapping = [(static_ip.ip, mac_address.get_raw())]
718+
719+ self.node.update_host_maps(
720+ ip_mapping, nodegroups=relevant_clusters)
721+ self.update_related_dns_zones()
722+
723 return static_ip
724
725=== modified file 'src/maasserver/models/macipaddresslink.py'
726--- src/maasserver/models/macipaddresslink.py 2015-05-07 18:14:38 +0000
727+++ src/maasserver/models/macipaddresslink.py 2015-06-15 08:11:13 +0000
728@@ -33,6 +33,8 @@
729
730 class MACStaticIPAddressLink(CleanSave, TimestampedModel):
731
732+ # XXX:fabric IP addresses are unique within each fabric, so this
733+ # code needs to be updated.
734 class Meta(DefaultMeta):
735 unique_together = ('ip_address', 'mac_address')
736
737
738=== modified file 'src/maasserver/models/node.py'
739--- src/maasserver/models/node.py 2015-05-22 15:03:29 +0000
740+++ src/maasserver/models/node.py 2015-06-15 08:11:13 +0000
741@@ -1742,7 +1742,7 @@
742
743 def claim_static_ip_addresses(
744 self, mac=None, alloc_type=IPADDRESS_TYPE.AUTO,
745- requested_address=None):
746+ requested_address=None, update_host_maps=True):
747 """Assign static IPs to a node's MAC.
748 If no MAC is specified, defaults to its PXE MAC.
749
750@@ -1759,7 +1759,6 @@
751 :raises: `StaticIPAddressUnavailable` if the supplied
752 requested_address is already in use.
753 """
754-
755 if mac is None:
756 mac = self.get_pxe_mac()
757 if mac is None:
758@@ -1767,7 +1766,8 @@
759
760 try:
761 static_ips = mac.claim_static_ips(
762- alloc_type=alloc_type, requested_address=requested_address)
763+ alloc_type=alloc_type, requested_address=requested_address,
764+ update_host_maps=update_host_maps)
765 except StaticIPAddressTypeClash:
766 # There's already a non-AUTO IP.
767 return []
768@@ -1776,31 +1776,49 @@
769 # because it's all-or-nothing (hence the atomic context).
770 return [(static_ip.ip, unicode(mac)) for static_ip in static_ips]
771
772- def update_host_maps(self, claims, nodegroups=None):
773+ @staticmethod
774+ def update_nodegroup_host_maps(nodegroups, claims):
775 """Update host maps for the given MAC->IP mappings.
776
777 If a nodegroup list is given, update all of them. If not, only
778 update the nodegroup of the node.
779 """
780 static_mappings = defaultdict(dict)
781- if nodegroups is None:
782- static_mappings[self.nodegroup].update(claims)
783- else:
784- for nodegroup in nodegroups:
785- static_mappings[nodegroup].update(claims)
786+ for nodegroup in nodegroups:
787+ static_mappings[nodegroup].update(claims)
788 update_host_maps_failures = list(
789 update_host_maps(static_mappings))
790- if len(update_host_maps_failures) != 0:
791+ num_failures = len(update_host_maps_failures)
792+ if num_failures != 0:
793 # We've hit an error, so release any IPs we've claimed
794 # and then raise the error for the call site to
795 # handle.
796- StaticIPAddress.objects.deallocate_by_node(self)
797+ # StaticIPAddress.objects.deallocate_by_node(self)
798+ # StaticIPAddress.objects.deallocate_by_node()
799+ for claim in claims:
800+ StaticIPAddress.objects.get(ip=claim[0]).deallocate()
801+
802 # We know there's only one error because we only
803 # sent one mapping to update_host_maps(), so we
804 # extract the exception from the Failure and raise
805 # it.
806 raise update_host_maps_failures[0].raiseException()
807
808+ def update_host_maps(self, claims, nodegroups=None):
809+ """Update host maps for the given MAC->IP mappings.
810+
811+ If a nodegroup list is given, update all of them. If not, only
812+ update the nodegroup of the node.
813+ """
814+
815+ # For some reason, we can call this method on a Node, but the intent
816+ # is to update nodegroups not on this node. That's why it's not
817+ # an "append" here.
818+ if nodegroups is None:
819+ nodegroups = [self.nodegroup]
820+
821+ self.update_nodegroup_host_maps(nodegroups, claims)
822+
823 def deallocate_static_ip_addresses(
824 self, alloc_type=IPADDRESS_TYPE.AUTO, ip=None):
825 """Release the `StaticIPAddress` that is assigned to this node and
826@@ -1813,9 +1831,10 @@
827 self, alloc_type=alloc_type, ip=ip)
828
829 if len(deallocated_ips) > 0:
830+ # Prevent circular imports
831+ from maasserver.dns import config as dns_config
832 self.delete_host_maps(deallocated_ips)
833- from maasserver.dns.config import dns_update_zones
834- dns_update_zones([self.nodegroup])
835+ dns_config.dns_update_zones([self.nodegroup])
836
837 return deallocated_ips
838
839@@ -1916,7 +1935,7 @@
840 return False
841
842 @transactional
843- def start(self, by_user, user_data=None):
844+ def start(self, by_user, user_data=None, update_host_maps=True):
845 """Request on given user's behalf that the node be started up.
846
847 :param by_user: Requesting user.
848@@ -1927,7 +1946,7 @@
849 :type user_data: unicode
850
851 :raise StaticIPAddressExhaustion: if there are not enough IP addresses
852- left in the static range for this node toget all the addresses it
853+ left in the static range for this node to get all the addresses it
854 needs.
855 :raise PermissionDenied: If `by_user` does not have permission to
856 start this node.
857@@ -1941,7 +1960,6 @@
858 """
859 # Avoid circular imports.
860 from metadataserver.models import NodeUserData
861- from maasserver.dns.config import dns_update_zones
862
863 if not by_user.has_perm(NODE_PERMISSION.EDIT, self):
864 # You can't start a node you don't own unless you're an admin.
865@@ -1954,11 +1972,13 @@
866
867 # Claim static IP addresses for the node if it's ALLOCATED.
868 if self.status == NODE_STATUS.ALLOCATED:
869- claims = self.claim_static_ip_addresses()
870- # If the PXE mac is on a managed interface then we can ask
871- # the cluster to generate the DHCP host map(s).
872- if self.is_pxe_mac_on_managed_interface():
873- self.update_host_maps(claims)
874+
875+ # Don't update host maps if we're not on a managed interface.
876+ if not self.is_pxe_mac_on_managed_interface() and update_host_maps:
877+ update_host_maps = False
878+
879+ self.claim_static_ip_addresses(
880+ update_host_maps=update_host_maps)
881
882 if self.status == NODE_STATUS.ALLOCATED:
883 transition_monitor = (
884@@ -1969,9 +1989,6 @@
885 else:
886 transition_monitor = None
887
888- # Update the DNS zone with the new static IP info as necessary.
889- dns_update_zones(self.nodegroup)
890-
891 power_info = self.get_effective_power_info()
892 if not power_info.can_be_started:
893 # The node can't be powered on by MAAS, so return early.
894
895=== modified file 'src/maasserver/models/signals/dns.py'
896--- src/maasserver/models/signals/dns.py 2015-06-10 11:24:44 +0000
897+++ src/maasserver/models/signals/dns.py 2015-06-15 08:11:13 +0000
898@@ -81,8 +81,8 @@
899 def dns_post_delete_Node(sender, instance, **kwargs):
900 """When a Node is deleted, update the Node's zone file."""
901 try:
902- from maasserver.dns.config import dns_update_zones
903- dns_update_zones(instance.nodegroup)
904+ from maasserver.dns import config as dns_config
905+ dns_config.dns_update_zones(instance.nodegroup)
906 except NodeGroup.DoesNotExist:
907 # If this Node is being deleted because the whole NodeGroup
908 # has been deleted, no need to update the zone file because
909@@ -92,29 +92,29 @@
910
911 def dns_post_edit_hostname_Node(instance, old_values, **kwargs):
912 """When a Node has been flagged, update the related zone."""
913- from maasserver.dns.config import dns_update_zones
914- dns_update_zones(instance.nodegroup)
915+ from maasserver.dns import config as dns_config
916+ dns_config.dns_update_zones(instance.nodegroup)
917
918
919 connect_to_field_change(dns_post_edit_hostname_Node, Node, ['hostname'])
920
921
922 def dns_setting_changed(sender, instance, created, **kwargs):
923- from maasserver.dns.config import dns_update_all_zones
924- dns_update_all_zones()
925+ from maasserver.dns import config as dns_config
926+ dns_config.dns_update_all_zones()
927
928
929 @receiver(post_save, sender=Network)
930 def dns_post_save_Network(sender, instance, **kwargs):
931 """When a network is added/changed, put it in the DNS trusted networks."""
932- from maasserver.dns.config import dns_update_all_zones
933- dns_update_all_zones()
934+ from maasserver.dns import config as dns_config
935+ dns_config.dns_update_all_zones()
936
937
938 @receiver(post_delete, sender=Network)
939 def dns_post_delete_Network(sender, instance, **kwargs):
940- from maasserver.dns.config import dns_update_all_zones
941- dns_update_all_zones()
942+ from maasserver.dns import config as dns_config
943+ dns_config.dns_update_all_zones()
944
945
946 Config.objects.config_changed_connect("upstream_dns", dns_setting_changed)
947
948=== modified file 'src/maasserver/models/tests/test_macaddress.py'
949--- src/maasserver/models/tests/test_macaddress.py 2015-05-16 06:21:20 +0000
950+++ src/maasserver/models/tests/test_macaddress.py 2015-06-15 08:11:13 +0000
951@@ -370,12 +370,12 @@
952 def test__returns_empty_if_no_cluster_interface(self):
953 # If mac.cluster_interface is None, we can't allocate any IP.
954 mac = factory.make_MACAddress_with_Node()
955- self.assertEquals([], mac.claim_static_ips())
956+ self.assertEquals([], mac.claim_static_ips(update_host_maps=False))
957
958 def test__reserves_an_ip_address(self):
959 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface()
960 mac = node.get_primary_mac()
961- [claimed_ip] = mac.claim_static_ips()
962+ [claimed_ip] = mac.claim_static_ips(update_host_maps=False)
963 self.assertIsInstance(claimed_ip, StaticIPAddress)
964 self.assertNotEqual([], list(node.static_ip_addresses()))
965 self.assertEqual(
966@@ -387,7 +387,8 @@
967 node = make_node_attached_to_cluster_interfaces(
968 ipv4_network=ipv4_network, ipv6_network=ipv6_network)
969
970- allocation = node.get_primary_mac().claim_static_ips()
971+ allocation = node.get_primary_mac().claim_static_ips(
972+ update_host_maps=False)
973
974 # Allocated one IPv4 address and one IPv6 address.
975 self.assertThat(allocation, HasLength(2))
976@@ -401,7 +402,8 @@
977 def test__sets_type_as_required(self):
978 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface()
979 mac = node.get_primary_mac()
980- [claimed_ip] = mac.claim_static_ips(alloc_type=IPADDRESS_TYPE.STICKY)
981+ [claimed_ip] = mac.claim_static_ips(
982+ alloc_type=IPADDRESS_TYPE.STICKY, update_host_maps=False)
983 self.assertEqual(IPADDRESS_TYPE.STICKY, claimed_ip.alloc_type)
984
985 def test__returns_empty_if_no_static_range_defined(self):
986@@ -410,7 +412,7 @@
987 mac.cluster_interface.static_ip_range_low = None
988 mac.cluster_interface.static_ip_range_high = None
989 mac.cluster_interface.save()
990- self.assertEqual([], mac.claim_static_ips())
991+ self.assertEqual([], mac.claim_static_ips(update_host_maps=False))
992
993 def test__returns_only_addresses_for_interfaces_with_static_ranges(self):
994 ipv6_network = factory.make_ipv6_network()
995@@ -423,7 +425,8 @@
996 ipv6_interface.static_ip_range_high = None
997 ipv6_interface.save()
998
999- allocation = node.get_primary_mac().claim_static_ips()
1000+ allocation = node.get_primary_mac().claim_static_ips(
1001+ update_host_maps=False)
1002 self.assertThat(allocation, HasLength(1))
1003 [sip] = allocation
1004 self.assertEqual(4, IPAddress(sip.ip).version)
1005@@ -432,7 +435,7 @@
1006 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface()
1007 mac = node.get_primary_mac()
1008 iptype, iptype2 = pick_two_allocation_types()
1009- mac.claim_static_ips(alloc_type=iptype)
1010+ mac.claim_static_ips(alloc_type=iptype, update_host_maps=False)
1011 self.assertRaises(
1012 StaticIPAddressTypeClash,
1013 mac.claim_static_ips, alloc_type=iptype2)
1014@@ -448,7 +451,8 @@
1015 iptype, iptype2 = pick_two_allocation_types()
1016 mac.claim_static_ips(
1017 alloc_type=iptype,
1018- requested_address=ipv4_interface.static_ip_range_low)
1019+ requested_address=ipv4_interface.static_ip_range_low,
1020+ update_host_maps=False)
1021
1022 self.assertRaises(
1023 StaticIPAddressTypeClash,
1024@@ -467,7 +471,8 @@
1025 iptype, iptype2 = pick_two_allocation_types()
1026 mac.claim_static_ips(
1027 alloc_type=iptype,
1028- requested_address=ipv6_interface.static_ip_range_low)
1029+ requested_address=ipv6_interface.static_ip_range_low,
1030+ update_host_maps=False)
1031
1032 self.assertRaises(
1033 StaticIPAddressTypeClash,
1034@@ -487,9 +492,11 @@
1035 iptype, iptype2 = pick_two_allocation_types()
1036 mac.claim_static_ips(
1037 alloc_type=iptype,
1038- requested_address=ipv4_interface.static_ip_range_low)
1039+ requested_address=ipv4_interface.static_ip_range_low,
1040+ update_host_maps=False)
1041
1042- allocation = mac.claim_static_ips(alloc_type=iptype2)
1043+ allocation = mac.claim_static_ips(
1044+ alloc_type=iptype2, update_host_maps=False)
1045
1046 self.assertThat(allocation, HasLength(1))
1047 [sip] = allocation
1048@@ -507,9 +514,11 @@
1049 iptype, iptype2 = pick_two_allocation_types()
1050 mac.claim_static_ips(
1051 alloc_type=iptype,
1052- requested_address=ipv6_interface.static_ip_range_low)
1053+ requested_address=ipv6_interface.static_ip_range_low,
1054+ update_host_maps=False)
1055
1056- allocation = mac.claim_static_ips(alloc_type=iptype2)
1057+ allocation = mac.claim_static_ips(
1058+ alloc_type=iptype2, update_host_maps=False)
1059
1060 self.assertThat(allocation, HasLength(1))
1061 [sip] = allocation
1062@@ -524,7 +533,7 @@
1063 ipv4_network=ipv4_network, ipv6_network=ipv6_network)
1064 mac = node.get_primary_mac()
1065 iptype, iptype2 = pick_two_allocation_types()
1066- mac.claim_static_ips(alloc_type=iptype)
1067+ mac.claim_static_ips(alloc_type=iptype, update_host_maps=False)
1068
1069 self.assertRaises(
1070 StaticIPAddressTypeClash,
1071@@ -542,15 +551,17 @@
1072 # Clashing IPv6 address:
1073 mac.claim_static_ips(
1074 alloc_type=iptype,
1075- requested_address=ipv6_interface.static_ip_range_low)
1076+ requested_address=ipv6_interface.static_ip_range_low,
1077+ update_host_maps=False)
1078 # Pre-existing, but non-clashing, IPv4 address:
1079 [ipv4_sip] = mac.claim_static_ips(
1080 alloc_type=iptype2,
1081- requested_address=ipv4_interface.static_ip_range_low)
1082+ requested_address=ipv4_interface.static_ip_range_low,
1083+ update_host_maps=False)
1084
1085 self.assertItemsEqual(
1086 [ipv4_sip],
1087- mac.claim_static_ips(alloc_type=iptype2))
1088+ mac.claim_static_ips(alloc_type=iptype2, update_host_maps=False))
1089
1090 def test__returns_existing_IPv6_if_IPv4_clashes(self):
1091 # If the MAC is attached to IPv4 and IPv6 cluster interfaces, there's
1092@@ -564,15 +575,17 @@
1093 # Clashing IPv4 address:
1094 mac.claim_static_ips(
1095 alloc_type=iptype,
1096- requested_address=ipv4_interface.static_ip_range_low)
1097+ requested_address=ipv4_interface.static_ip_range_low,
1098+ update_host_maps=False)
1099 # Pre-existing, but non-clashing, IPv6 address:
1100 [ipv6_sip] = mac.claim_static_ips(
1101 alloc_type=iptype2,
1102- requested_address=ipv6_interface.static_ip_range_low)
1103+ requested_address=ipv6_interface.static_ip_range_low,
1104+ update_host_maps=False)
1105
1106 self.assertItemsEqual(
1107 [ipv6_sip],
1108- mac.claim_static_ips(alloc_type=iptype2))
1109+ mac.claim_static_ips(alloc_type=iptype2, update_host_maps=False))
1110
1111 def test__ignores_clashing_IPv4_when_requesting_IPv6(self):
1112 node = make_node_attached_to_cluster_interfaces()
1113@@ -582,11 +595,13 @@
1114 iptype, iptype2 = pick_two_allocation_types()
1115 mac.claim_static_ips(
1116 alloc_type=iptype,
1117- requested_address=ipv4_interface.static_ip_range_low)
1118+ requested_address=ipv4_interface.static_ip_range_low,
1119+ update_host_maps=False)
1120
1121 allocation = mac.claim_static_ips(
1122 alloc_type=iptype2,
1123- requested_address=ipv6_interface.static_ip_range_low)
1124+ requested_address=ipv6_interface.static_ip_range_low,
1125+ update_host_maps=False)
1126
1127 self.assertThat(allocation, HasLength(1))
1128 [ipv6_sip] = allocation
1129@@ -600,11 +615,13 @@
1130 iptype, iptype2 = pick_two_allocation_types()
1131 mac.claim_static_ips(
1132 alloc_type=iptype,
1133- requested_address=ipv6_interface.static_ip_range_low)
1134+ requested_address=ipv6_interface.static_ip_range_low,
1135+ update_host_maps=False)
1136
1137 allocation = mac.claim_static_ips(
1138 alloc_type=iptype2,
1139- requested_address=ipv4_interface.static_ip_range_low)
1140+ requested_address=ipv4_interface.static_ip_range_low,
1141+ update_host_maps=False)
1142
1143 self.assertThat(allocation, HasLength(1))
1144 [ipv4_sip] = allocation
1145@@ -615,22 +632,24 @@
1146 mac = node.get_primary_mac()
1147 iptype = factory.pick_enum(
1148 IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED])
1149- [ip] = mac.claim_static_ips(alloc_type=iptype)
1150+ [ip] = mac.claim_static_ips(alloc_type=iptype, update_host_maps=False)
1151 self.assertEqual(
1152- [ip], mac.claim_static_ips(alloc_type=iptype))
1153+ [ip], mac.claim_static_ips(
1154+ alloc_type=iptype, update_host_maps=False))
1155
1156 def test__combines_existing_and_new_addresses(self):
1157 node = make_node_attached_to_cluster_interfaces()
1158- # node = factory.make_node_with_mac_attached_to_nodegroupinterface()
1159 mac = node.get_primary_mac()
1160 ipv4_interface = find_cluster_interface(node.nodegroup, 4)
1161 iptype = factory.pick_enum(
1162 IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED])
1163 [ipv4_sip] = mac.claim_static_ips(
1164 alloc_type=iptype,
1165- requested_address=ipv4_interface.static_ip_range_low)
1166+ requested_address=ipv4_interface.static_ip_range_low,
1167+ update_host_maps=False)
1168
1169- allocation = mac.claim_static_ips(alloc_type=iptype)
1170+ allocation = mac.claim_static_ips(
1171+ alloc_type=iptype, update_host_maps=False)
1172
1173 self.assertIn(ipv4_sip, allocation)
1174 self.assertThat(allocation, HasLength(2))
1175@@ -639,7 +658,8 @@
1176 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface()
1177 mac = node.get_primary_mac()
1178 ip = node.get_primary_mac().cluster_interface.static_ip_range_high
1179- [allocation] = mac.claim_static_ips(requested_address=ip)
1180+ [allocation] = mac.claim_static_ips(
1181+ requested_address=ip, update_host_maps=False)
1182 self.assertEqual(ip, allocation.ip)
1183
1184 def test__allocates_only_IPv4_if_IPv4_address_requested(self):
1185@@ -648,7 +668,7 @@
1186 requested_ip = ipv4_interface.static_ip_range_low
1187
1188 allocation = node.get_primary_mac().claim_static_ips(
1189- requested_address=requested_ip)
1190+ requested_address=requested_ip, update_host_maps=False)
1191
1192 self.assertThat(allocation, HasLength(1))
1193 [sip] = allocation
1194@@ -660,7 +680,7 @@
1195 requested_ip = ipv6_interface.static_ip_range_low
1196
1197 allocation = node.get_primary_mac().claim_static_ips(
1198- requested_address=requested_ip)
1199+ requested_address=requested_ip, update_host_maps=False)
1200
1201 self.assertThat(allocation, HasLength(1))
1202 [sip] = allocation
1203@@ -673,7 +693,8 @@
1204 cluster_interface=cluster_interface)
1205 user = factory.make_User()
1206 [sip] = mac_address.claim_static_ips(
1207- user=user, alloc_type=IPADDRESS_TYPE.USER_RESERVED)
1208+ user=user, alloc_type=IPADDRESS_TYPE.USER_RESERVED,
1209+ update_host_maps=False)
1210 self.assertEqual(sip.user, user)
1211
1212
1213@@ -881,7 +902,7 @@
1214 mac = factory.make_MACAddress_with_Node()
1215 user = factory.make_User()
1216 ip_address = factory.make_ip_address()
1217- static_ip = mac.set_static_ip(ip_address, user)
1218+ static_ip = mac.set_static_ip(ip_address, user, update_host_maps=False)
1219
1220 matcher = MatchesStructure(
1221 id=Not(Is(None)), # IP persisted in the DB.
1222@@ -905,7 +926,7 @@
1223 static_range = IPRange(
1224 ngi.static_ip_range_low, ngi.static_ip_range_high)
1225 ip_address = factory.pick_ip_in_network(static_range)
1226- static_ip = mac.set_static_ip(ip_address, user)
1227+ static_ip = mac.set_static_ip(ip_address, user, update_host_maps=False)
1228
1229 matcher = MatchesStructure(
1230 id=Not(Is(None)), # IP persisted in the DB.
1231@@ -927,7 +948,7 @@
1232 dynamic_range = IPRange(ngi.ip_range_low, ngi.ip_range_high)
1233 ip_address = factory.pick_ip_in_network(dynamic_range)
1234 with ExpectedException(StaticIPAddressForbidden):
1235- mac.set_static_ip(ip_address, user)
1236+ mac.set_static_ip(ip_address, user, update_host_maps=False)
1237
1238 def test_rejects_ip_if_ip_not_part_of_connected_network(self):
1239 node = factory.make_Node(mac=True)
1240@@ -945,15 +966,16 @@
1241 other_network = factory.make_ipv4_network(disjoint_from=[network])
1242 ip_address = factory.pick_ip_in_network(other_network)
1243 with ExpectedException(StaticIPAddressConflict):
1244- mac.set_static_ip(ip_address, user)
1245+ mac.set_static_ip(ip_address, user, update_host_maps=False)
1246
1247 def test_returns_existing_allocation_if_it_exists(self):
1248 mac = factory.make_MACAddress_with_Node()
1249 user = factory.make_User()
1250 ip_address = factory.make_ip_address()
1251- static_ip = mac.set_static_ip(ip_address, user)
1252+ static_ip = mac.set_static_ip(ip_address, user, update_host_maps=False)
1253
1254- static_ip2 = mac.set_static_ip(ip_address, user)
1255+ static_ip2 = mac.set_static_ip(
1256+ ip_address, user, update_host_maps=False)
1257
1258 self.assertEquals(static_ip.id, static_ip2.id)
1259
1260@@ -961,11 +983,11 @@
1261 other_mac = factory.make_MACAddress_with_Node()
1262 user = factory.make_User()
1263 ip_address = factory.make_ip_address()
1264- other_mac.set_static_ip(ip_address, user)
1265+ other_mac.set_static_ip(ip_address, user, update_host_maps=False)
1266
1267 mac = factory.make_MACAddress_with_Node()
1268 with ExpectedException(StaticIPAddressUnavailable):
1269- mac.set_static_ip(ip_address, user)
1270+ mac.set_static_ip(ip_address, user, update_host_maps=False)
1271
1272 def test_rejects_ip_if_allocation_with_other_type_already_exists(self):
1273 nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
1274@@ -977,7 +999,7 @@
1275 mac.cluster_interface = ngi
1276 mac.save()
1277 # Create an AUTO IP allocation.
1278- static_ip = mac.claim_static_ips()[0]
1279+ static_ip = mac.claim_static_ips(update_host_maps=False)[0]
1280
1281 with ExpectedException(StaticIPAddressUnavailable):
1282- mac.set_static_ip(static_ip.ip, user)
1283+ mac.set_static_ip(static_ip.ip, user, update_host_maps=False)
1284
1285=== modified file 'src/maasserver/models/tests/test_node.py'
1286--- src/maasserver/models/tests/test_node.py 2015-06-10 11:24:44 +0000
1287+++ src/maasserver/models/tests/test_node.py 2015-06-15 08:11:13 +0000
1288@@ -92,7 +92,6 @@
1289 )
1290 from maastesting.djangotestcase import count_queries
1291 from maastesting.matchers import (
1292- MockAnyCall,
1293 MockCalledOnceWith,
1294 MockNotCalled,
1295 )
1296@@ -351,7 +350,8 @@
1297 primary_mac = node.get_primary_mac()
1298 random_alloc_type = factory.pick_enum(
1299 IPADDRESS_TYPE, but_not=[IPADDRESS_TYPE.USER_RESERVED])
1300- primary_mac.claim_static_ips(alloc_type=random_alloc_type)
1301+ primary_mac.claim_static_ips(
1302+ alloc_type=random_alloc_type, update_host_maps=False)
1303 node.delete()
1304 self.assertItemsEqual([], StaticIPAddress.objects.all())
1305
1306@@ -362,7 +362,8 @@
1307 primary_mac = node.get_primary_mac()
1308 static_ip_addresses = set(
1309 static_ip_address.ip for static_ip_address in
1310- primary_mac.claim_static_ips(alloc_type=IPADDRESS_TYPE.STICKY))
1311+ primary_mac.claim_static_ips(
1312+ alloc_type=IPADDRESS_TYPE.STICKY, update_host_maps=False))
1313 node.delete()
1314 self.assertThat(
1315 remove_host_maps, MockCalledOnceWith(
1316@@ -1244,7 +1245,7 @@
1317 user = factory.make_User()
1318 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface(
1319 owner=user, status=NODE_STATUS.ALLOCATED)
1320- sips = node.get_primary_mac().claim_static_ips()
1321+ sips = node.get_primary_mac().claim_static_ips(update_host_maps=False)
1322 node.deallocate_static_ip_addresses()
1323 expected = {sip.ip.format() for sip in sips}
1324 self.assertThat(
1325@@ -1254,14 +1255,14 @@
1326 def test_deallocate_static_ip_updates_dns(self):
1327 # silence remove_host_maps
1328 self.patch_autospec(node_module, "remove_host_maps")
1329- dns_update_zones = self.patch(dns_config, 'dns_update_zones')
1330+ dns_update_zones = self.patch(dns_config.dns_update_zones)
1331 nodegroup = factory.make_NodeGroup(
1332 management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
1333 status=NODEGROUP_STATUS.ENABLED)
1334 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface(
1335 nodegroup=nodegroup, status=NODE_STATUS.ALLOCATED,
1336 owner=factory.make_User(), power_type='ether_wake')
1337- node.get_primary_mac().claim_static_ips()
1338+ node.get_primary_mac().claim_static_ips(update_host_maps=False)
1339 node.deallocate_static_ip_addresses()
1340 self.assertThat(dns_update_zones, MockCalledOnceWith([node.nodegroup]))
1341
1342@@ -2422,7 +2423,8 @@
1343
1344 def test__returns_mapping_for_iface_on_managed_network(self):
1345 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface()
1346- static_mappings = node.claim_static_ip_addresses()
1347+ static_mappings = node.claim_static_ip_addresses(
1348+ update_host_maps=False)
1349 [static_ip] = node.static_ip_addresses()
1350 [mac_address] = node.macaddress_set.all()
1351 self.assertEqual(
1352@@ -2436,7 +2438,8 @@
1353 [managed_interface] = node.nodegroup.get_managed_interfaces()
1354 node.pxe_mac.cluster_interface = managed_interface
1355 node.pxe_mac.save()
1356- static_mappings = node.claim_static_ip_addresses()
1357+ static_mappings = node.claim_static_ip_addresses(
1358+ update_host_maps=False)
1359 [static_ip] = node.static_ip_addresses()
1360 mac_address = node.get_pxe_mac()
1361 self.assertEqual(
1362@@ -2446,7 +2449,8 @@
1363 def test__ignores_mac_address_with_non_auto_addresses(self):
1364 node = factory.make_Node_with_MACAddress_and_NodeGroupInterface()
1365 mac_address = node.macaddress_set.first()
1366- mac_address.claim_static_ips(IPADDRESS_TYPE.STICKY)
1367+ mac_address.claim_static_ips(
1368+ IPADDRESS_TYPE.STICKY, update_host_maps=False)
1369 self.assertRaises(
1370 StaticIPAddressTypeClash, mac_address.claim_static_ips)
1371 static_mappings = node.claim_static_ip_addresses()
1372@@ -2464,7 +2468,7 @@
1373 static_range = ngi.get_static_ip_range()
1374
1375 pxe_ip, pxe_mac = node.claim_static_ip_addresses(
1376- alloc_type=IPADDRESS_TYPE.STICKY)[0]
1377+ alloc_type=IPADDRESS_TYPE.STICKY, update_host_maps=False)[0]
1378
1379 self.expectThat(static_range, Contains(IPAddress(pxe_ip)))
1380
1381@@ -2489,7 +2493,8 @@
1382 first_ip = ngi.get_static_ip_range()[0]
1383
1384 pxe_ip, pxe_mac = node.claim_static_ip_addresses(
1385- alloc_type=IPADDRESS_TYPE.STICKY, requested_address=first_ip)[0]
1386+ alloc_type=IPADDRESS_TYPE.STICKY, requested_address=first_ip,
1387+ update_host_maps=False)[0]
1388 mac = MACAddress.objects.get(mac_address=pxe_mac)
1389 ip = mac.ip_addresses.first()
1390 self.expectThat(IPAddress(ip.ip), Equals(first_ip))
1391@@ -2507,10 +2512,12 @@
1392 first_ip = ngi.get_static_ip_range()[0]
1393
1394 node.claim_static_ip_addresses(
1395- alloc_type=IPADDRESS_TYPE.STICKY, requested_address=first_ip)
1396+ alloc_type=IPADDRESS_TYPE.STICKY, requested_address=first_ip,
1397+ update_host_maps=False)
1398 with ExpectedException(StaticIPAddressUnavailable):
1399 node2.claim_static_ip_addresses(
1400- alloc_type=IPADDRESS_TYPE.STICKY, requested_address=first_ip)
1401+ alloc_type=IPADDRESS_TYPE.STICKY, requested_address=first_ip,
1402+ update_host_maps=False)
1403
1404
1405 class TestAsyncDellocateStaticIPAddress(MAASTransactionServerTestCase):
1406@@ -2554,7 +2561,7 @@
1407
1408 with transaction.atomic():
1409 pxe_ip, pxe_mac = node.claim_static_ip_addresses(
1410- alloc_type=IPADDRESS_TYPE.STICKY)[0]
1411+ alloc_type=IPADDRESS_TYPE.STICKY, update_host_maps=False)[0]
1412
1413 mac = MACAddress.objects.get(mac_address=pxe_mac)
1414 ip = mac.ip_addresses.all()[0]
1415@@ -2565,7 +2572,8 @@
1416 for mac in macs:
1417 with transaction.atomic():
1418 sips.append(node.claim_static_ip_addresses(
1419- mac=mac, alloc_type=IPADDRESS_TYPE.STICKY)[0])
1420+ mac=mac, alloc_type=IPADDRESS_TYPE.STICKY,
1421+ update_host_maps=False)[0])
1422
1423 # try removing just the IP on the PXE MAC first
1424 deallocated = node.deallocate_static_ip_addresses(
1425@@ -2639,7 +2647,7 @@
1426 user_data = factory.make_bytes()
1427
1428 with post_commit_hooks:
1429- node.start(user, user_data=user_data)
1430+ node.start(user, user_data=user_data, update_host_maps=False)
1431
1432 nud = NodeUserData.objects.get(node=node)
1433 self.assertEqual(user_data, nud.data)
1434@@ -2653,7 +2661,7 @@
1435 NodeUserData.objects.set_user_data(node, user_data)
1436
1437 with post_commit_hooks:
1438- node.start(user, user_data=None)
1439+ node.start(user, user_data=None, update_host_maps=False)
1440
1441 self.assertFalse(NodeUserData.objects.filter(node=node).exists())
1442
1443@@ -2664,13 +2672,15 @@
1444 node = self.make_acquired_node_with_mac(user, nodegroup)
1445
1446 claim_static_ip_addresses = self.patch_autospec(
1447- node, "claim_static_ip_addresses", spec_set=False)
1448+ node, "claim_static_ip_addresses")
1449 claim_static_ip_addresses.return_value = {}
1450
1451 with post_commit_hooks:
1452 node.start(user)
1453
1454- self.expectThat(node.claim_static_ip_addresses, MockAnyCall())
1455+ self.expectThat(
1456+ claim_static_ip_addresses, MockCalledOnceWith(
1457+ update_host_maps=True))
1458
1459 def test__only_claims_static_addresses_when_allocated(self):
1460 user = factory.make_User()
1461@@ -2685,7 +2695,7 @@
1462 claim_static_ip_addresses.return_value = {}
1463
1464 with post_commit_hooks:
1465- node.start(user)
1466+ node.start(user, update_host_maps=False)
1467
1468 # No calls are made to claim_static_ip_addresses, since the node
1469 # isn't ALLOCATED.
1470@@ -2734,16 +2744,17 @@
1471 self.assertRaises(exception_type, node.start, user)
1472
1473 def test__updates_dns(self):
1474+ dns_update_zones = self.patch(dns_config.dns_update_zones)
1475+ self.patch(Node, "update_host_maps")
1476+
1477 user = factory.make_User()
1478 node = self.make_acquired_node_with_mac(user)
1479
1480- dns_update_zones = self.patch(dns_config, "dns_update_zones")
1481-
1482 with post_commit_hooks:
1483 node.start(user)
1484
1485 self.assertThat(
1486- dns_update_zones, MockCalledOnceWith(node.nodegroup))
1487+ dns_update_zones, MockCalledOnceWith([node.nodegroup]))
1488
1489 def test__set_zone(self):
1490 """Verifies whether the set_zone sets the node's zone"""
1491@@ -2762,7 +2773,7 @@
1492 self.patch_autospec(node, '_start_transition_monitor_async')
1493
1494 with post_commit_hooks:
1495- node.start(user)
1496+ node.start(user, update_host_maps=False)
1497
1498 # If the following fails the diff is big, but it's useful.
1499 self.maxDiff = None
1500@@ -2792,13 +2803,13 @@
1501
1502 with ExpectedException(PraiseBeToJTVException):
1503 with post_commit_hooks:
1504- node.start(user)
1505+ node.start(user, update_host_maps=False)
1506
1507 def test__marks_allocated_node_as_deploying(self):
1508 user = factory.make_User()
1509 node = self.make_acquired_node_with_mac(user)
1510 with post_commit_hooks:
1511- node.start(user)
1512+ node.start(user, update_host_maps=False)
1513 self.assertEqual(
1514 NODE_STATUS.DEPLOYING, reload_object(node).status)
1515
1516@@ -2813,7 +2824,7 @@
1517 power_on_node = self.patch(node_module, "power_on_node")
1518 power_on_node.return_value = defer.succeed(None)
1519 with post_commit_hooks:
1520- node.start(user)
1521+ node.start(user, update_host_maps=False)
1522 self.assertEqual(
1523 NODE_STATUS.DEPLOYED, reload_object(node).status)
1524
1525@@ -2829,7 +2840,7 @@
1526 )
1527 self.patch(node, 'get_effective_power_info').return_value = power_info
1528 power_on_node = self.patch(node_module, "power_on_node")
1529- node.start(user)
1530+ node.start(user, update_host_maps=False)
1531 self.assertThat(power_on_node, MockNotCalled())
1532
1533 def test__does_not_start_nodes_the_user_cannot_edit(self):
1534@@ -2838,8 +2849,9 @@
1535 node = self.make_acquired_node_with_mac(owner)
1536
1537 user = factory.make_User()
1538- self.assertRaises(PermissionDenied, node.start, user)
1539- self.assertThat(power_on_node, MockNotCalled())
1540+ with ExpectedException(PermissionDenied):
1541+ node.start(user, update_host_maps=False)
1542+ self.assertThat(power_on_node, MockNotCalled())
1543
1544 def test__allows_admin_to_start_any_node(self):
1545 power_on_node = self.patch_autospec(node_module, "power_on_node")
1546@@ -2848,7 +2860,7 @@
1547
1548 admin = factory.make_admin()
1549 with post_commit_hooks:
1550- node.start(admin)
1551+ node.start(admin, update_host_maps=False)
1552
1553 self.expectThat(
1554 power_on_node, MockCalledOnceWith(
1555@@ -2870,7 +2882,7 @@
1556
1557 with ExpectedException(exception_type):
1558 with post_commit_hooks:
1559- node.start(user)
1560+ node.start(user, update_host_maps=False)
1561
1562 self.assertThat(deallocate_ips, MockCalledOnceWith(node))
1563
1564@@ -2880,8 +2892,8 @@
1565 update_host_maps.return_value = [
1566 Failure(exception_type("You steaming nit, you!"))
1567 ]
1568- deallocate_ips = self.patch(
1569- node_module.StaticIPAddress.objects, 'deallocate_by_node')
1570+ deallocate = self.patch(
1571+ node_module.StaticIPAddress, 'deallocate')
1572
1573 user = factory.make_User()
1574 node = self.make_acquired_node_with_mac(user)
1575@@ -2890,7 +2902,8 @@
1576 with post_commit_hooks:
1577 node.start(user)
1578
1579- self.assertThat(deallocate_ips, MockCalledOnceWith(node))
1580+ self.assertThat(
1581+ deallocate, MockCalledOnceWith())
1582
1583 def test_update_host_maps_updates_given_nodegroup_list(self):
1584 user = factory.make_User()
1585
1586=== modified file 'src/maasserver/websockets/handlers/device.py'
1587--- src/maasserver/websockets/handlers/device.py 2015-05-27 16:17:55 +0000
1588+++ src/maasserver/websockets/handlers/device.py 2015-06-15 08:11:13 +0000
1589@@ -17,7 +17,6 @@
1590 ]
1591
1592 from maasserver.clusterrpc import dhcp
1593-from maasserver.dns.config import dns_update_zones
1594 from maasserver.enum import (
1595 IPADDRESS_TYPE,
1596 NODE_PERMISSION,
1597@@ -312,7 +311,7 @@
1598 ip_assignment = nic["ip_assignment"]
1599 if ip_assignment == DEVICE_IP_ASSIGNMENT.EXTERNAL:
1600 sticky_ip = mac_address.set_static_ip(
1601- nic["ip_address"], self.user)
1602+ nic["ip_address"], self.user, update_host_maps=False)
1603 external_static_ips.append(
1604 (sticky_ip, mac_address))
1605 elif ip_assignment == DEVICE_IP_ASSIGNMENT.DYNAMIC:
1606@@ -335,7 +334,7 @@
1607 # Claim the static ip.
1608 sticky_ips = mac_address.claim_static_ips(
1609 alloc_type=IPADDRESS_TYPE.STICKY,
1610- requested_address=ip_address)
1611+ requested_address=ip_address, update_host_maps=False)
1612 assigned_sticky_ips.extend([
1613 (static_ip, mac_address)
1614 for static_ip in sticky_ips
1615@@ -393,7 +392,8 @@
1616 log_static_allocations(
1617 device_obj, external_static_ips, assigned_sticky_ips)
1618 if len(external_static_ips) > 0 or len(assigned_sticky_ips) > 0:
1619- dns_update_zones([NodeGroup.objects.ensure_master()])
1620+ from maasserver.dns import config as dns_config
1621+ dns_config.dns_update_zones([NodeGroup.objects.ensure_master()])
1622
1623 return self.full_dehydrate(device_obj)
1624
1625
1626=== modified file 'src/maasserver/websockets/handlers/tests/test_device.py'
1627--- src/maasserver/websockets/handlers/tests/test_device.py 2015-05-26 23:55:25 +0000
1628+++ src/maasserver/websockets/handlers/tests/test_device.py 2015-06-15 08:11:13 +0000
1629@@ -17,6 +17,7 @@
1630 import re
1631
1632 from maasserver.clusterrpc import dhcp as dhcp_module
1633+from maasserver.dns import config as dns_config
1634 from maasserver.exceptions import NodeActionError
1635 from maasserver.fields import MAC
1636 from maasserver.forms import (
1637@@ -155,7 +156,7 @@
1638 interface = factory.make_NodeGroupInterface(nodegroup)
1639 primary_mac.cluster_interface = interface
1640 primary_mac.save()
1641- primary_mac.claim_static_ips()
1642+ primary_mac.claim_static_ips(update_host_maps=False)
1643 return device
1644
1645 def make_devices(self, nodegroup, number, owner=None):
1646@@ -190,6 +191,7 @@
1647 handler.list({}))
1648
1649 def test_list_num_queries_is_independent_of_num_devices(self):
1650+ self.patch(Node, "update_host_maps")
1651 owner = factory.make_User()
1652 handler = DeviceHandler(owner, {})
1653 nodegroup = factory.make_NodeGroup()
1654@@ -211,6 +213,7 @@
1655 "Number of queries is not independent to the number of nodes.")
1656
1657 def test_list_returns_devices_only_viewable_by_user(self):
1658+ self.patch(Node, "update_host_maps")
1659 user = factory.make_User()
1660 # Create another user.
1661 factory.make_User()
1662@@ -327,7 +330,7 @@
1663 hostname = factory.make_name("hostname")
1664 ip_address = factory.make_ipv4_address()
1665 self.patch(dhcp_module, "update_host_maps").return_value = []
1666- mock_dns_update_zones = self.patch(device_module, "dns_update_zones")
1667+ mock_dns_update_zones = self.patch(dns_config.dns_update_zones)
1668 handler.create({
1669 "hostname": hostname,
1670 "primary_mac": mac,
1671@@ -371,6 +374,7 @@
1672 Equals(0), "Created StaticIPAddress was not deleted.")
1673
1674 def test_create_creates_device_with_static_ip_assignment_implicit(self):
1675+ self.patch(Node, "update_host_maps")
1676 user = factory.make_User()
1677 handler = DeviceHandler(user, {})
1678 mac = factory.make_mac_address()
1679@@ -400,6 +404,7 @@
1680 Equals(1), "StaticIPAddress was not created.")
1681
1682 def test_create_creates_device_with_static_ip_assignment_explicit(self):
1683+ self.patch(Node, "update_host_maps")
1684 user = factory.make_User()
1685 handler = DeviceHandler(user, {})
1686 mac = factory.make_mac_address()
1687@@ -431,14 +436,15 @@
1688 Equals(1), "StaticIPAddress was not created.")
1689
1690 def test_create_with_static_ip_calls_dns_update_zones(self):
1691+ self.patch(device_module.update_host_maps).return_value = []
1692+ # self.patch(Node.update_host_maps).return_value = []
1693 user = factory.make_User()
1694 handler = DeviceHandler(user, {})
1695 mac = factory.make_mac_address()
1696 hostname = factory.make_name("hostname")
1697 nodegroup = factory.make_NodeGroup()
1698 nodegroup_interface = factory.make_NodeGroupInterface(nodegroup)
1699- self.patch(dhcp_module, "update_host_maps").return_value = []
1700- mock_dns_update_zones = self.patch(device_module, "dns_update_zones")
1701+ mock_dns_update_zones = self.patch(dns_config.dns_update_zones)
1702 handler.create({
1703 "hostname": hostname,
1704 "primary_mac": mac,
1705@@ -453,6 +459,8 @@
1706 MockCalledOnceWith([NodeGroup.objects.ensure_master()]))
1707
1708 def test_create_raises_failure_static_ip_update_hostmaps_fails(self):
1709+ self.patch(Node, "update_host_maps")
1710+ mock_update_host_maps = self.patch(dhcp_module, "update_host_maps")
1711 user = factory.make_User()
1712 handler = DeviceHandler(user, {})
1713 mac = factory.make_mac_address()
1714@@ -460,7 +468,6 @@
1715 nodegroup = factory.make_NodeGroup()
1716 nodegroup_interface = factory.make_NodeGroupInterface(nodegroup)
1717 ip_address = nodegroup_interface.static_ip_range_low
1718- mock_update_host_maps = self.patch(dhcp_module, "update_host_maps")
1719 mock_update_host_maps.return_value = [
1720 Failure(factory.make_exception()),
1721 ]
1722@@ -485,6 +492,7 @@
1723 Equals(0), "Created StaticIPAddress was not deleted.")
1724
1725 def test_create_creates_device_with_static_and_external_ip(self):
1726+ self.patch(Node, "update_host_maps")
1727 user = factory.make_User()
1728 handler = DeviceHandler(user, {})
1729 hostname = factory.make_name("hostname")
1730@@ -539,18 +547,19 @@
1731 Equals(1), "External StaticIPAddress was not created.")
1732
1733 def test_create_deletes_device_and_ips_when_only_one_errors(self):
1734- user = factory.make_User()
1735- handler = DeviceHandler(user, {})
1736- hostname = factory.make_name("hostname")
1737- nodegroup = factory.make_NodeGroup()
1738- nodegroup_interface = factory.make_NodeGroupInterface(nodegroup)
1739- mac_static = factory.make_mac_address()
1740- static_ip_address = nodegroup_interface.static_ip_range_low
1741- mac_external = factory.make_mac_address()
1742- external_ip_address = factory.make_ipv4_address()
1743+ self.patch(Node, "update_host_maps")
1744 self.patch(dhcp_module, "update_host_maps").side_effect = [
1745 [], [Failure(factory.make_exception())],
1746 ]
1747+ user = factory.make_User()
1748+ handler = DeviceHandler(user, {})
1749+ hostname = factory.make_name("hostname")
1750+ nodegroup = factory.make_NodeGroup()
1751+ nodegroup_interface = factory.make_NodeGroupInterface(nodegroup)
1752+ mac_static = factory.make_mac_address()
1753+ static_ip_address = nodegroup_interface.static_ip_range_low
1754+ mac_external = factory.make_mac_address()
1755+ external_ip_address = factory.make_ipv4_address()
1756 self.assertRaises(HandlerError, handler.create, {
1757 "hostname": hostname,
1758 "primary_mac": mac_static,
1759
1760=== modified file 'src/maastesting/testcase.py'
1761--- src/maastesting/testcase.py 2015-05-07 18:14:38 +0000
1762+++ src/maastesting/testcase.py 2015-06-15 08:11:13 +0000
1763@@ -19,6 +19,7 @@
1764
1765 from contextlib import contextmanager
1766 import doctest
1767+from importlib import import_module
1768 import types
1769 import unittest
1770
1771@@ -177,7 +178,7 @@
1772 with active_test(result, self):
1773 super(MAASTestCase, self).__call__(result)
1774
1775- def patch(self, obj, attribute, value=mock.sentinel.unset):
1776+ def patch(self, obj, attribute=None, value=mock.sentinel.unset):
1777 """Patch `obj.attribute` with `value`.
1778
1779 If `value` is unspecified, a new `MagicMock` will be created and
1780@@ -188,6 +189,14 @@
1781
1782 :return: The patched-in object.
1783 """
1784+
1785+ # If 'attribute' is None, assume 'obj' is a 'fully-qualified' object,
1786+ # and assume that its __module__ is what we want to patch. For more
1787+ # complex use cases, the two-paramerter 'patch' will still need to
1788+ # be used.
1789+ if attribute is None:
1790+ attribute = obj.__name__
1791+ obj = import_module(obj.__module__)
1792 if value is mock.sentinel.unset:
1793 value = mock.MagicMock()
1794 super(MAASTestCase, self).patch(obj, attribute, value)