Merge lp:~mpontillo/maas/update-host-maps-cleanup-bug-1440102 into lp:~maas-committers/maas/trunk
- update-host-maps-cleanup-bug-1440102
- Merge into trunk
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 |
Related bugs: |
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.
Description of the change
Mike Pontillo (mpontillo) wrote : | # |
Mike Pontillo (mpontillo) wrote : | # |
Updated update_
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.
Mike Pontillo (mpontillo) wrote : | # |
Note: CI failed with the following errors:
2015-06-11 18:40:58 ERROR juju.provider.
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.
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...
Raphaël Badin (rvb) wrote : | # |
> Note: CI failed with the following errors:
>
> 2015-06-11 18:40:58 ERROR juju.provider.
> 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.
> 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/
Raphaël Badin (rvb) : | # |
Gavin Panella (allenap) wrote : | # |
No objections, but I don't understand what the problem was and how this improves the situation. Some background please :)
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.
Raphaël Badin (rvb) wrote : | # |
@Mike: you missed the create() method in websockets/
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.
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_
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_
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.)
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_
MAAS Lander (maas-lander) wrote : | # |
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://
Hit http://
Hit http://
Ign http://
Ign http://
Hit http://
Get:1 http://
Hit http://
Hit http://
Get:2 http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Get:3 http://
Get:4 http://
Get:5 http://
Get:6 http://
Hit http://
Hit http://
Ign http://
Ign http://
Fetched 1,214 kB in 3s (376 kB/s)
Reading package lists...
sudo DEBIAN_
--
Mike Pontillo (mpontillo) wrote : | # |
The failure was in:
provisioningser
This seems to be a flaky test case. It fails for me about ~10-20% of the time.
Preview Diff
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) |
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.)