Merge ~newell-jensen/maas:lp1789521 into maas:master

Proposed by Newell Jensen on 2018-08-31
Status: Merged
Approved by: Newell Jensen on 2018-09-07
Approved revision: aad988a3e435e4f38ed4f3e52e79c890dec4f0b6
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~newell-jensen/maas:lp1789521
Merge into: maas:master
Prerequisite: ~newell-jensen/maas:pod-drivers-add-known-host-interface
Diff against target: 1230 lines (+563/-210)
5 files modified
src/maasserver/forms/pods.py (+52/-28)
src/maasserver/forms/tests/test_pods.py (+185/-103)
src/provisioningserver/drivers/pod/tests/test_virsh.py (+198/-53)
src/provisioningserver/drivers/pod/virsh.py (+116/-26)
src/provisioningserver/enum.py (+12/-0)
Reviewer Review Type Date Requested Status
Mike Pontillo (community) 2018-08-31 Approve on 2018-09-06
MAAS Lander Approve on 2018-09-06
Review via email: mp+354151@code.launchpad.net

Commit message

LP: #1789521 -- Make virsh pod driver smarter about assiging default networks.

To post a comment you must log in.
Mike Pontillo (mpontillo) wrote :

Looks good so far; nice job. I've made a few comments below for you to think about, but nothing major.

Newell Jensen (newell-jensen) wrote :

Thanks, I also just realized that I am not validating if the networks are attached a bridge that has a MAAS DHCP enabled VLAN.

MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1789521 lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: faea65562c7267a2cb269b5608ac93a2b99dca19

review: Approve
Mike Pontillo (mpontillo) wrote :

I tested this branch and found that it doesn't work when composing VMs from the Web UI. I get the following error:

    Database connections in this thread (MainThread) are disabled.

The code that was added inside get_requested_machine() will need to be placed in a separate function and called within a deferToDatabase function when isInIOThread() turns out to be true.

review: Needs Fixing
Mike Pontillo (mpontillo) wrote :

I tested this branch again using the command line (maas admin pod 1 compose) and hit the following error:

    'NoneType' object has no attribute 'dhcp_on'

I've pasted the full stack trace here: https://paste.ubuntu.com/p/wnRhGcTpZC/

It seems to be happening because the relay_vlan is not being set.

I would recommend pulling the new logic out into a separate function (possibly with the pod.host as the input parameter, so that it doesn't have to be inside the class), that way you can unit test the logic separately from the other pieces of the form.

Newell Jensen (newell-jensen) wrote :

Thanks for the tests Mike!

MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1789521 lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 6d5ffe80be273df42fa2b0eb0a3fc49c308ada11

review: Approve
Mike Pontillo (mpontillo) wrote :

Tested the latest version. Looking better, but I now see this when composing in the UI:

    Pod unable to compose machine: too many values to unpack (expected 2)

Testing composition from the command-line results in:

    # maas admin pod compose 1
    Unable to compose machine because: Failed talking to pod: check_network_maas_dhcp_enabled() takes 3 positional arguments but 4 were given

So, still some bugs to fix - and tests to enhance...

MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b lp1789521 lp:~newell-jensen/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: aad988a3e435e4f38ed4f3e52e79c890dec4f0b6

review: Approve
Mike Pontillo (mpontillo) wrote :

This is working great now. Thanks for the fixes!

In a follow-on branch it would be nice to refactor that code to use a single path instead of two separate paths for async and non-async, and use inlineCallbacks. I think the code would be a lot easier to test and maintain that way.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/forms/pods.py b/src/maasserver/forms/pods.py
2index c64a690..9b457b3 100644
3--- a/src/maasserver/forms/pods.py
4+++ b/src/maasserver/forms/pods.py
5@@ -58,15 +58,13 @@ from maasserver.node_constraint_filter_forms import (
6 from maasserver.rpc import getClientFromIdentifiers
7 from maasserver.utils.forms import set_form_error
8 from maasserver.utils.orm import transactional
9-from maasserver.utils.threads import (
10- callOutToDatabase,
11- deferToDatabase,
12-)
13+from maasserver.utils.threads import deferToDatabase
14 import petname
15 from provisioningserver.drivers import SETTING_SCOPE
16 from provisioningserver.drivers.pod import (
17 Capabilities,
18 InterfaceAttachType,
19+ KnownHostInterface,
20 RequestedMachine,
21 RequestedMachineBlockDevice,
22 RequestedMachineInterface,
23@@ -414,7 +412,7 @@ class ComposeMachineForm(forms.Form):
24 else:
25 return None
26
27- def get_requested_machine(self):
28+ def get_requested_machine(self, known_host_interfaces):
29 """Return the `RequestedMachine`."""
30 block_devices = []
31
32@@ -442,6 +440,7 @@ class ComposeMachineForm(forms.Form):
33 cpu_speed=self.get_value_for('cpu_speed'),
34 block_devices=block_devices,
35 interfaces=requested_machine_interfaces,
36+ known_host_interfaces=known_host_interfaces,
37 )
38
39 def _get_requested_machine_interfaces_via_constraints(
40@@ -515,16 +514,44 @@ class ComposeMachineForm(forms.Form):
41 if skip_commissioning is None:
42 skip_commissioning = self.get_value_for('skip_commissioning')
43
44- def check_over_commit_ratios(result):
45+ def db_work(client):
46 # Check over commit ratios.
47 over_commit_message = self.pod.check_over_commit_ratios(
48 requested_cores=self.get_value_for('cores'),
49 requested_memory=self.get_value_for('memory'))
50 if over_commit_message:
51 raise PodProblem(over_commit_message)
52- return result
53
54- def create_and_sync(requested_machine, result):
55+ # Update the default storage pool.
56+ if self.pod.default_storage_pool is not None:
57+ power_parameters['default_storage_pool_id'] = (
58+ self.pod.default_storage_pool.pool_id)
59+
60+ # Find the pod's known host interfaces.
61+ if self.pod.host is not None:
62+ result = [
63+ KnownHostInterface(
64+ ifname=interface.name,
65+ attach_type=(
66+ InterfaceAttachType.BRIDGE
67+ if interface.type == INTERFACE_TYPE.BRIDGE
68+ else InterfaceAttachType.MACVLAN),
69+ dhcp_enabled=(
70+ True
71+ if (interface.vlan is not None and
72+ interface.vlan.dhcp_on) or (
73+ interface.vlan.relay_vlan is not None and
74+ interface.vlan.relay_vlan.dhcp_on)
75+ else False))
76+ for interface in self.pod.host.interface_set.all()
77+ ]
78+ else:
79+ result = []
80+
81+ return (client, result)
82+
83+ def create_and_sync(result):
84+ requested_machine, result = result
85 discovered_machine, pod_hints = result
86 created_machine = self.pod.create_machine(
87 discovered_machine, self.request.user,
88@@ -539,30 +566,30 @@ class ComposeMachineForm(forms.Form):
89 self.pod.sync_hints(pod_hints)
90 return created_machine
91
92- power_parameters = self.pod.power_parameters.copy()
93+ def async_compose_machine(
94+ result, power_type, power_paramaters, **kwargs):
95+ client, result = result
96+ requested_machine = self.get_requested_machine(result)
97+ d = compose_machine(
98+ client, power_type, power_paramaters, requested_machine,
99+ **kwargs)
100+ d.addCallback(lambda result: (requested_machine, result))
101+ return d
102
103- def _set_default_pool_id():
104- if self.pod.default_storage_pool is not None:
105- power_parameters['default_storage_pool_id'] = (
106- self.pod.default_storage_pool.pool_id)
107+ power_parameters = self.pod.power_parameters.copy()
108
109 if isInIOThread():
110 # Running under the twisted reactor, before the work from inside.
111 d = deferToDatabase(transactional(self.pod.get_client_identifiers))
112 d.addCallback(getClientFromIdentifiers)
113 d.addCallback(
114- partial(deferToDatabase, transactional(
115- check_over_commit_ratios)))
116- d.addCallback(callOutToDatabase, _set_default_pool_id)
117- requested_machine = self.get_requested_machine()
118+ partial(deferToDatabase, transactional(db_work)))
119 d.addCallback(
120- compose_machine, self.pod.power_type,
121- power_parameters, requested_machine,
122- pod_id=self.pod.id, name=self.pod.name)
123+ async_compose_machine, self.pod.power_type,
124+ power_parameters, pod_id=self.pod.id, name=self.pod.name)
125 d.addCallback(
126 partial(
127- deferToDatabase, transactional(create_and_sync),
128- requested_machine))
129+ deferToDatabase, transactional(create_and_sync)))
130 return d
131 else:
132 # Running outside of reactor. Do the work inside and then finish
133@@ -574,16 +601,13 @@ class ComposeMachineForm(forms.Form):
134 """Wrapper to get the client."""
135 d = getClientFromIdentifiers(client_idents)
136 d.addCallback(
137- partial(deferToDatabase, transactional(
138- check_over_commit_ratios)))
139- d.addCallback(
140 compose_machine, pod_type, parameters, request,
141 pod_id=pod_id, name=name)
142 return d
143
144- _set_default_pool_id()
145+ _, result = db_work(None)
146 try:
147- requested_machine = self.get_requested_machine()
148+ requested_machine = self.get_requested_machine(result)
149 result = wrap_compose_machine(
150 self.pod.get_client_identifiers(),
151 self.pod.power_type,
152@@ -596,7 +620,7 @@ class ComposeMachineForm(forms.Form):
153 "Unable to compose a machine because '%s' driver "
154 "timed out after %d seconds." % (
155 self.pod.power_type, timeout))
156- return create_and_sync(requested_machine, result)
157+ return create_and_sync((requested_machine, result))
158
159
160 class ComposeMachineForPodsForm(forms.Form):
161diff --git a/src/maasserver/forms/tests/test_pods.py b/src/maasserver/forms/tests/test_pods.py
162index c28755a..e795c06 100644
163--- a/src/maasserver/forms/tests/test_pods.py
164+++ b/src/maasserver/forms/tests/test_pods.py
165@@ -20,6 +20,7 @@ from django.core.validators import (
166 )
167 from maasserver.enum import (
168 BMC_TYPE,
169+ INTERFACE_TYPE,
170 NODE_CREATION_TYPE,
171 NODE_STATUS,
172 )
173@@ -59,6 +60,7 @@ from provisioningserver.drivers.pod import (
174 DiscoveredPodHints,
175 DiscoveredPodStoragePool,
176 InterfaceAttachType,
177+ KnownHostInterface,
178 RequestedMachine,
179 RequestedMachineBlockDevice,
180 RequestedMachineInterface,
181@@ -88,6 +90,27 @@ from twisted.internet.defer import (
182 wait_for_reactor = crochet.wait_for(30) # 30 seconds.
183
184
185+def make_known_host_interfaces(pod):
186+ return [
187+ KnownHostInterface(
188+ ifname=interface.name,
189+ attach_type=(
190+ InterfaceAttachType.BRIDGE
191+ if interface.type == INTERFACE_TYPE.BRIDGE
192+ else InterfaceAttachType.MACVLAN),
193+ dhcp_enabled=(
194+ True
195+ if (interface.vlan is not None and
196+ interface.vlan.dhcp_on) or (
197+ interface.vlan.relay_vlan is not None and
198+ interface.vlan.relay_vlan.dhcp_on)
199+ else False))
200+ for interface in (
201+ pod.host.interface_set.all()
202+ if pod.host is not None else [])
203+ ]
204+
205+
206 def make_pod_with_hints():
207 architectures = [
208 "amd64/generic", "i386/generic", "arm64/generic",
209@@ -732,6 +755,16 @@ class TestPodForm(MAASTransactionServerTestCase):
210
211 class TestComposeMachineForm(MAASTransactionServerTestCase):
212
213+ def make_requested_machine_result(self, pod):
214+ return RequestedMachine(
215+ hostname=factory.make_name('hostname'),
216+ architecture=factory.make_name('architecture'),
217+ cores=random.randint(1, 8),
218+ memory=random.randint(1024, 4096),
219+ block_devices=[RequestedMachineBlockDevice(size=4096)],
220+ interfaces=[RequestedMachineInterface()],
221+ known_host_interfaces=make_known_host_interfaces(pod))
222+
223 def make_compose_machine_result(self, pod):
224 composed_machine = DiscoveredMachine(
225 hostname=factory.make_name('hostname'),
226@@ -813,12 +846,13 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
227 form = ComposeMachineForm(request=request, pod=pod)
228 self.assertEqual(pool, form.initial['pool'])
229
230- def test__get_requested_machine_uses_all_initial_values(self):
231+ def test__get_machine_uses_all_initial_values(self):
232 request = MagicMock()
233 pod = make_pod_with_hints()
234 form = ComposeMachineForm(data={}, request=request, pod=pod)
235 self.assertTrue(form.is_valid())
236- request_machine = form.get_requested_machine()
237+ request_machine = form.get_requested_machine(
238+ make_known_host_interfaces(pod))
239 self.assertThat(request_machine, MatchesAll(
240 IsInstance(RequestedMachine),
241 MatchesStructure(
242@@ -831,9 +865,10 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
243 IsInstance(RequestedMachineBlockDevice),
244 MatchesStructure(size=Equals(8 * (1000 ** 3))))]),
245 interfaces=MatchesListwise([
246- IsInstance(RequestedMachineInterface)]))))
247+ IsInstance(RequestedMachineInterface)]),
248+ known_host_interfaces=MatchesListwise([]))))
249
250- def test__get_requested_machine_uses_passed_values(self):
251+ def test__get_machine_uses_passed_values(self):
252 request = MagicMock()
253 pod = make_pod_with_hints()
254 architecture = random.choice(pod.architectures)
255@@ -861,7 +896,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
256 'storage': storage,
257 }, request=request, pod=pod)
258 self.assertTrue(form.is_valid(), form.errors)
259- request_machine = form.get_requested_machine()
260+ request_machine = form.get_requested_machine(
261+ make_known_host_interfaces(pod))
262 self.assertThat(request_machine, MatchesAll(
263 IsInstance(RequestedMachine),
264 MatchesStructure(
265@@ -880,9 +916,10 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
266 size=Equals(disk_2), tags=Equals(disk_2_tags))),
267 ]),
268 interfaces=MatchesListwise([
269- IsInstance(RequestedMachineInterface)]))))
270+ IsInstance(RequestedMachineInterface)]),
271+ known_host_interfaces=MatchesListwise([]))))
272
273- def test__get_requested_machine_handles_no_tags_in_storage(self):
274+ def test__get_machine_handles_no_tags_in_storage(self):
275 request = MagicMock()
276 pod = make_pod_with_hints()
277 disk_1 = random.randint(8, 16) * (1000 ** 3)
278@@ -894,7 +931,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
279 'storage': storage,
280 }, request=request, pod=pod)
281 self.assertTrue(form.is_valid(), form.errors)
282- request_machine = form.get_requested_machine()
283+ request_machine = form.get_requested_machine(
284+ make_known_host_interfaces(pod))
285 self.assertThat(request_machine, MatchesAll(
286 IsInstance(RequestedMachine),
287 MatchesStructure(
288@@ -926,7 +964,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
289 with ExpectedException(
290 ValidationError,
291 ".*DHCP must be enabled on at least one VLAN*"):
292- form.get_requested_machine()
293+ form.get_requested_machine(
294+ make_known_host_interfaces(pod))
295
296 def test__get_machine_with_interfaces_fails_for_no_matching_network(self):
297 request = MagicMock()
298@@ -945,7 +984,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
299 with ExpectedException(
300 ValidationError,
301 ".*does not match the specified network.*"):
302- form.get_requested_machine()
303+ form.get_requested_machine(
304+ make_known_host_interfaces(pod))
305
306 def test__get_machine_with_interfaces_by_subnet(self):
307 request = MagicMock()
308@@ -963,7 +1003,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
309 'interfaces': interfaces,
310 }, request=request, pod=pod)
311 self.assertTrue(form.is_valid(), form.errors)
312- request_machine = form.get_requested_machine()
313+ request_machine = form.get_requested_machine(
314+ make_known_host_interfaces(pod))
315 self.assertThat(request_machine, MatchesAll(
316 IsInstance(RequestedMachine),
317 MatchesStructure(
318@@ -991,7 +1032,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
319 form = ComposeMachineForm(
320 data=dict(interfaces=''), request=request, pod=pod)
321 self.assertTrue(form.is_valid(), form.errors)
322- request_machine = form.get_requested_machine()
323+ request_machine = form.get_requested_machine(
324+ make_known_host_interfaces(pod))
325 self.assertThat(request_machine, MatchesAll(
326 IsInstance(RequestedMachine),
327 MatchesStructure(
328@@ -1004,45 +1046,6 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
329 attach_options=Is(None)))
330 ]))))
331
332- def test__compose_with_interfaces_with_reserved_ip_fails(self):
333- # Mock the RPC client.
334- client = MagicMock()
335- mock_getClient = self.patch(pods_module, "getClientFromIdentifiers")
336- mock_getClient.return_value = succeed(client)
337-
338- pod = make_pod_with_hints()
339-
340- # Mock start_commissioning so it doesn't use post commit hooks.
341- self.patch(Machine, "start_commissioning")
342-
343- # Mock the result of the composed machine.
344- composed_machine, pod_hints = self.make_compose_machine_result(pod)
345- composed_machine.interfaces = [
346- DiscoveredMachineInterface(mac_address='00:01:02:03:04:05')
347- ]
348- mock_compose_machine = self.patch(pods_module, "compose_machine")
349- mock_compose_machine.return_value = succeed(
350- (composed_machine, pod_hints))
351-
352- request = MagicMock()
353- subnet = factory.make_Subnet()
354- host = factory.make_Machine_with_Interface_on_Subnet(subnet=subnet)
355- space = factory.make_Space('dmz')
356- host.boot_interface.vlan.dhcp_on = True
357- host.boot_interface.vlan.space = space
358- host.boot_interface.vlan.save()
359- ip = factory.make_StaticIPAddress(
360- interface=host.get_boot_interface(), subnet=subnet)
361- pod.ip_address = host.boot_interface.ip_addresses.first()
362- pod.save()
363- interfaces = "eth0:ip=%s" % str(ip.ip)
364- form = ComposeMachineForm(data={
365- 'interfaces': interfaces,
366- }, request=request, pod=pod)
367- self.assertTrue(form.is_valid(), form.errors)
368- with ExpectedException(StaticIPAddressUnavailable):
369- form.compose()
370-
371 def test__get_machine_with_interfaces_with_unreserved_ip(self):
372 # Mock the RPC client.
373 client = MagicMock()
374@@ -1081,7 +1084,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
375 'interfaces': interfaces,
376 }, request=request, pod=pod)
377 self.assertTrue(form.is_valid(), form.errors)
378- request_machine = form.get_requested_machine()
379+ request_machine = form.get_requested_machine(
380+ make_known_host_interfaces(pod))
381 self.assertThat(request_machine, MatchesAll(
382 IsInstance(RequestedMachine),
383 MatchesStructure(
384@@ -1096,48 +1100,6 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
385 attach_options=Equals(MACVLAN_MODE.BRIDGE)))
386 ]))))
387
388- def test__compose_with_interfaces_with_unreserved_ip(self):
389- # Mock the RPC client.
390- client = MagicMock()
391- mock_getClient = self.patch(pods_module, "getClientFromIdentifiers")
392- mock_getClient.return_value = succeed(client)
393-
394- pod = make_pod_with_hints()
395-
396- # Mock start_commissioning so it doesn't use post commit hooks.
397- self.patch(Machine, "start_commissioning")
398-
399- # Mock the result of the composed machine.
400- composed_machine, pod_hints = self.make_compose_machine_result(pod)
401- composed_machine.interfaces = [
402- DiscoveredMachineInterface(
403- mac_address='00:01:02:03:04:05', boot=True)
404- ]
405- mock_compose_machine = self.patch(pods_module, "compose_machine")
406- mock_compose_machine.return_value = succeed(
407- (composed_machine, pod_hints))
408- request = MagicMock()
409- subnet = factory.make_Subnet()
410- host = factory.make_Machine_with_Interface_on_Subnet(subnet=subnet)
411- space = factory.make_Space('dmz')
412- host.boot_interface.vlan.dhcp_on = True
413- host.boot_interface.vlan.space = space
414- host.boot_interface.vlan.save()
415- ip = factory.make_StaticIPAddress(
416- interface=host.get_boot_interface(), subnet=subnet)
417- expected_ip = str(ip.ip)
418- ip.delete()
419- pod.ip_address = host.boot_interface.ip_addresses.first()
420- pod.save()
421- interfaces = "eth0:ip=%s" % expected_ip
422- form = ComposeMachineForm(data={
423- 'interfaces': interfaces,
424- }, request=request, pod=pod)
425- self.assertTrue(form.is_valid(), form.errors)
426- machine = form.compose()
427- ip = StaticIPAddress.objects.filter(ip=expected_ip).first()
428- self.assertThat(ip.get_interface().node, Equals(machine))
429-
430 def test__get_machine_with_interfaces_by_subnet_with_default_mode(self):
431 request = MagicMock()
432 pod_host = factory.make_Machine_with_Interface_on_Subnet()
433@@ -1155,7 +1117,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
434 'interfaces': interfaces,
435 }, request=request, pod=pod)
436 self.assertTrue(form.is_valid(), form.errors)
437- request_machine = form.get_requested_machine()
438+ request_machine = form.get_requested_machine(
439+ make_known_host_interfaces(pod))
440 self.assertThat(request_machine, MatchesAll(
441 IsInstance(RequestedMachine),
442 MatchesStructure(
443@@ -1187,7 +1150,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
444 'interfaces': interfaces,
445 }, request=request, pod=pod)
446 self.assertTrue(form.is_valid(), form.errors)
447- request_machine = form.get_requested_machine()
448+ request_machine = form.get_requested_machine(
449+ make_known_host_interfaces(pod))
450 self.assertThat(request_machine, MatchesAll(
451 IsInstance(RequestedMachine),
452 MatchesStructure(
453@@ -1214,7 +1178,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
454 'interfaces': interfaces,
455 }, request=request, pod=pod)
456 self.assertTrue(form.is_valid(), form.errors)
457- request_machine = form.get_requested_machine()
458+ request_machine = form.get_requested_machine(
459+ make_known_host_interfaces(pod))
460 self.assertThat(request_machine, MatchesAll(
461 IsInstance(RequestedMachine),
462 MatchesStructure(
463@@ -1245,7 +1210,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
464 'interfaces': interfaces,
465 }, request=request, pod=pod)
466 self.assertTrue(form.is_valid(), form.errors)
467- request_machine = form.get_requested_machine()
468+ request_machine = form.get_requested_machine(
469+ make_known_host_interfaces(pod))
470 self.assertThat(request_machine, MatchesAll(
471 IsInstance(RequestedMachine),
472 MatchesStructure(
473@@ -1282,7 +1248,8 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
474 'interfaces': interfaces,
475 }, request=request, pod=pod)
476 self.assertTrue(form.is_valid(), form.errors)
477- request_machine = form.get_requested_machine()
478+ request_machine = form.get_requested_machine(
479+ make_known_host_interfaces(pod))
480 self.assertThat(request_machine, MatchesAll(
481 IsInstance(RequestedMachine),
482 MatchesStructure(
483@@ -1296,12 +1263,117 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
484 attach_options=Is(None)))
485 ]))))
486
487- def test__save_raises_AttributeError(self):
488+ def test__get_machine_with_known_host_interfaces(self):
489+ # Need to test that it can actually find the pod host's data correctly
490+ # and that this matches what is expected.
491 request = MagicMock()
492+ pod_host = factory.make_Machine_with_Interface_on_Subnet(
493+ interface_count=3)
494 pod = make_pod_with_hints()
495+ pod.ip_address = pod_host.boot_interface.ip_addresses.first()
496+ pod.save()
497 form = ComposeMachineForm(data={}, request=request, pod=pod)
498- self.assertTrue(form.is_valid())
499- self.assertRaises(AttributeError, form.save)
500+ self.assertTrue(form.is_valid(), form.errors)
501+ request_machine = form.get_requested_machine(
502+ make_known_host_interfaces(pod))
503+ self.assertThat(request_machine, MatchesAll(
504+ IsInstance(RequestedMachine),
505+ MatchesStructure(
506+ known_host_interfaces=MatchesListwise([
507+ MatchesAll(
508+ IsInstance(KnownHostInterface),
509+ MatchesStructure(
510+ ifname=Equals(interface.name),
511+ attach_type=Equals(
512+ InterfaceAttachType.BRIDGE
513+ if interface.type == INTERFACE_TYPE.BRIDGE
514+ else InterfaceAttachType.MACVLAN),
515+ dhcp_enabled=Equals(
516+ interface.vlan.dhcp_on or
517+ interface.vlan.relay_vlan.dhcp_on)))
518+ for interface in pod.host.interface_set.all()
519+ ]))))
520+
521+ def test__compose_with_interfaces_with_reserved_ip_fails(self):
522+ # Mock the RPC client.
523+ client = MagicMock()
524+ mock_getClient = self.patch(pods_module, "getClientFromIdentifiers")
525+ mock_getClient.return_value = succeed(client)
526+
527+ pod = make_pod_with_hints()
528+
529+ # Mock start_commissioning so it doesn't use post commit hooks.
530+ self.patch(Machine, "start_commissioning")
531+
532+ # Mock the result of the composed machine.
533+ composed_machine, pod_hints = self.make_compose_machine_result(pod)
534+ composed_machine.interfaces = [
535+ DiscoveredMachineInterface(mac_address='00:01:02:03:04:05')
536+ ]
537+ mock_compose_machine = self.patch(pods_module, "compose_machine")
538+ mock_compose_machine.return_value = succeed(
539+ (composed_machine, pod_hints))
540+
541+ request = MagicMock()
542+ subnet = factory.make_Subnet()
543+ host = factory.make_Machine_with_Interface_on_Subnet(subnet=subnet)
544+ space = factory.make_Space('dmz')
545+ host.boot_interface.vlan.dhcp_on = True
546+ host.boot_interface.vlan.space = space
547+ host.boot_interface.vlan.save()
548+ ip = factory.make_StaticIPAddress(
549+ interface=host.get_boot_interface(), subnet=subnet)
550+ pod.ip_address = host.boot_interface.ip_addresses.first()
551+ pod.save()
552+ interfaces = "eth0:ip=%s" % str(ip.ip)
553+ form = ComposeMachineForm(data={
554+ 'interfaces': interfaces,
555+ }, request=request, pod=pod)
556+ self.assertTrue(form.is_valid(), form.errors)
557+ with ExpectedException(StaticIPAddressUnavailable):
558+ form.compose()
559+
560+ def test__compose_with_interfaces_with_unreserved_ip(self):
561+ # Mock the RPC client.
562+ client = MagicMock()
563+ mock_getClient = self.patch(pods_module, "getClientFromIdentifiers")
564+ mock_getClient.return_value = succeed(client)
565+
566+ pod = make_pod_with_hints()
567+
568+ # Mock start_commissioning so it doesn't use post commit hooks.
569+ self.patch(Machine, "start_commissioning")
570+
571+ # Mock the result of the composed machine.
572+ composed_machine, pod_hints = self.make_compose_machine_result(pod)
573+ composed_machine.interfaces = [
574+ DiscoveredMachineInterface(
575+ mac_address='00:01:02:03:04:05', boot=True)
576+ ]
577+ mock_compose_machine = self.patch(pods_module, "compose_machine")
578+ mock_compose_machine.return_value = succeed(
579+ (composed_machine, pod_hints))
580+ request = MagicMock()
581+ subnet = factory.make_Subnet()
582+ host = factory.make_Machine_with_Interface_on_Subnet(subnet=subnet)
583+ space = factory.make_Space('dmz')
584+ host.boot_interface.vlan.dhcp_on = True
585+ host.boot_interface.vlan.space = space
586+ host.boot_interface.vlan.save()
587+ ip = factory.make_StaticIPAddress(
588+ interface=host.get_boot_interface(), subnet=subnet)
589+ expected_ip = str(ip.ip)
590+ ip.delete()
591+ pod.ip_address = host.boot_interface.ip_addresses.first()
592+ pod.save()
593+ interfaces = "eth0:ip=%s" % expected_ip
594+ form = ComposeMachineForm(data={
595+ 'interfaces': interfaces,
596+ }, request=request, pod=pod)
597+ self.assertTrue(form.is_valid(), form.errors)
598+ machine = form.compose()
599+ ip = StaticIPAddress.objects.filter(ip=expected_ip).first()
600+ self.assertThat(ip.get_interface().node, Equals(machine))
601
602 def test__compose_with_commissioning(self):
603 request = MagicMock()
604@@ -1357,7 +1429,9 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
605 ANY, pod.power_type, {
606 'default_storage_pool_id': pod.default_storage_pool.pool_id,
607 },
608- form.get_requested_machine(), pod_id=pod.id, name=pod.name))
609+ form.get_requested_machine(
610+ make_known_host_interfaces(pod)),
611+ pod_id=pod.id, name=pod.name))
612
613 def test__compose_duplicated_hostname(self):
614 factory.make_Node(hostname='test')
615@@ -1576,6 +1650,7 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
616 mock_getClient.return_value = succeed(client)
617
618 # Mock the result of the composed machine.
619+# requested_machine = self.make_requested_machine_result(pod)
620 composed_machine, pod_hints = self.make_compose_machine_result(pod)
621 mock_compose_machine = self.patch(pods_module, "compose_machine")
622 mock_compose_machine.return_value = succeed(
623@@ -1597,6 +1672,13 @@ class TestComposeMachineForm(MAASTransactionServerTestCase):
624 cpu_speed=Equals(300))))
625 self.assertThat(mock_commissioning, MockCalledOnce())
626
627+ def test__save_raises_AttributeError(self):
628+ request = MagicMock()
629+ pod = make_pod_with_hints()
630+ form = ComposeMachineForm(data={}, request=request, pod=pod)
631+ self.assertTrue(form.is_valid())
632+ self.assertRaises(AttributeError, form.save)
633+
634
635 class TestComposeMachineForPodsForm(MAASServerTestCase):
636
637diff --git a/src/provisioningserver/drivers/pod/tests/test_virsh.py b/src/provisioningserver/drivers/pod/tests/test_virsh.py
638index 3e2b9a8..9b338ea 100644
639--- a/src/provisioningserver/drivers/pod/tests/test_virsh.py
640+++ b/src/provisioningserver/drivers/pod/tests/test_virsh.py
641@@ -31,6 +31,7 @@ from provisioningserver.drivers.pod import (
642 Capabilities,
643 DiscoveredPodStoragePool,
644 InterfaceAttachType,
645+ KnownHostInterface,
646 RequestedMachine,
647 RequestedMachineBlockDevice,
648 RequestedMachineInterface,
649@@ -48,7 +49,10 @@ from provisioningserver.drivers.pod.virsh import (
650 InterfaceInfo,
651 VirshPodDriver,
652 )
653-from provisioningserver.enum import MACVLAN_MODE_CHOICES
654+from provisioningserver.enum import (
655+ LIBVIRT_NETWORK,
656+ MACVLAN_MODE_CHOICES,
657+)
658 from provisioningserver.rpc.exceptions import PodInvalidResources
659 from provisioningserver.utils.shell import (
660 get_env_with_locale,
661@@ -159,6 +163,42 @@ SAMPLE_IFLIST = dedent("""
662 - bridge br1 e1000 %s
663 """)
664
665+SAMPLE_NETWORK_DUMPXML = dedent("""
666+ <network>
667+ <name>default</name>
668+ <uuid>6d477dbc-c6d6-46c1-97c8-665ecab001f3</uuid>
669+ <forward mode='nat'>
670+ <nat>
671+ <port start='1024' end='65535'/>
672+ </nat>
673+ </forward>
674+ <bridge name='virbr0' stp='on' delay='0'/>
675+ <mac address='52:54:00:85:fc:da'/>
676+ <ip address='192.168.123.1' netmask='255.255.255.0'>
677+ <dhcp>
678+ <range start='192.168.123.2' end='192.168.123.254'/>
679+ </dhcp>
680+ </ip>
681+ </network>
682+ """)
683+
684+SAMPLE_NETWORK_DUMPXML_2 = dedent("""
685+ <network>
686+ <name>maas</name>
687+ <uuid>8ef77750-edb2-11e7-b8c7-b3f6673ef6b2</uuid>
688+ <forward mode='nat'>
689+ <nat>
690+ <port start='1024' end='65535'/>
691+ </nat>
692+ </forward>
693+ <bridge name='virbr1' stp='on' delay='0'/>
694+ <mac address='52:54:00:df:83:6c'/>
695+ <domain name='testnet'/>
696+ <ip address='172.16.99.1' netmask='255.255.255.0'>
697+ </ip>
698+ </network>
699+ """)
700+
701 SAMPLE_DUMPXML = dedent("""
702 <domain type='kvm'>
703 <name>test</name>
704@@ -1298,39 +1338,164 @@ class TestVirshSSH(MAASTestCase):
705 conn = self.configure_virshssh('\n'.join(networks))
706 self.assertEquals(networks, conn.get_network_list())
707
708- def test_get_best_network_returns_maas(self):
709+ def test_check_network_maas_dhcp_enabled_returns_None_virsh_dhcp(self):
710+ bridge = 'virbr0'
711+ conn = self.configure_virshssh('')
712+ host_interfaces = {
713+ factory.make_name('ifname'): False,
714+ bridge: True,
715+ }
716+ mock_get_network_xml = self.patch(virsh.VirshSSH, "get_network_xml")
717+ mock_get_network_xml.return_value = SAMPLE_NETWORK_DUMPXML
718+ expected = conn.check_network_maas_dhcp_enabled(
719+ bridge, host_interfaces)
720+ self.assertIsNone(expected)
721+
722+ def test_check_network_maas_dhcp_enabled_returns_network(self):
723+ bridge = 'virbr1'
724+ conn = self.configure_virshssh('')
725+ host_interfaces = {
726+ factory.make_name('ifname'): False,
727+ bridge: True,
728+ }
729+ mock_get_network_xml = self.patch(virsh.VirshSSH, "get_network_xml")
730+ mock_get_network_xml.return_value = SAMPLE_NETWORK_DUMPXML_2
731+ bridge_name = conn.check_network_maas_dhcp_enabled(
732+ bridge, host_interfaces)
733+ self.assertEquals(bridge_name, bridge)
734+
735+ def test_get_default_interface_attachment_no_host_interfaces_maas(self):
736+ conn = self.configure_virshssh('')
737+ self.patch(virsh.VirshSSH, "get_network_list").return_value = [
738+ LIBVIRT_NETWORK.MAAS, LIBVIRT_NETWORK.DEFAULT, 'other']
739+ network, attach_type = conn.get_default_interface_attachment(None)
740+ self.assertEquals(LIBVIRT_NETWORK.MAAS, network)
741+ self.assertEquals(InterfaceAttachType.NETWORK, attach_type)
742+
743+ def test_get_default_interface_attachment_no_host_interfaces_default(self):
744+ conn = self.configure_virshssh('')
745+ self.patch(virsh.VirshSSH, "get_network_list").return_value = [
746+ LIBVIRT_NETWORK.DEFAULT, 'other']
747+ network, attach_type = conn.get_default_interface_attachment(None)
748+ self.assertEquals(LIBVIRT_NETWORK.DEFAULT, network)
749+ self.assertEquals(InterfaceAttachType.NETWORK, attach_type)
750+
751+ def test_get_default_interface_attachment_errors_no_nics_or_networks(self):
752+ conn = self.configure_virshssh('')
753+ self.patch(virsh.VirshSSH, "get_network_list").return_value = []
754+ self.assertRaises(
755+ PodInvalidResources, conn.get_default_interface_attachment, None)
756+
757+ def test_get_default_interface_attachment_maas_bridge_no_virsh_dhcp(self):
758+ conn = self.configure_virshssh('')
759+ self.patch(virsh.VirshSSH, "get_network_list").return_value = [
760+ LIBVIRT_NETWORK.MAAS, LIBVIRT_NETWORK.DEFAULT, 'other']
761+ self.patch(
762+ virsh.VirshSSH, "check_network_maas_dhcp_enabled").return_value = (
763+ 'virbr0')
764+ host_interfaces = [
765+ KnownHostInterface(
766+ ifname=LIBVIRT_NETWORK.MAAS,
767+ attach_type=InterfaceAttachType.NETWORK,
768+ dhcp_enabled=True)
769+ ]
770+ network, attach_type = conn.get_default_interface_attachment(
771+ host_interfaces)
772+ # This shows us that the method returned with these values.
773+ self.assertEquals(LIBVIRT_NETWORK.MAAS, network)
774+ self.assertEquals(InterfaceAttachType.NETWORK, attach_type)
775+
776+ def test_get_default_interface_attachment_default_brd_no_virsh_dhcp(self):
777 conn = self.configure_virshssh('')
778 self.patch(virsh.VirshSSH, "get_network_list").return_value = [
779- 'maas', 'default', 'other']
780- self.assertEquals('maas', conn.get_best_network())
781+ LIBVIRT_NETWORK.DEFAULT, 'other']
782+ self.patch(
783+ virsh.VirshSSH, "check_network_maas_dhcp_enabled").return_value = (
784+ 'virbr1')
785+ host_interfaces = [
786+ KnownHostInterface(
787+ ifname=LIBVIRT_NETWORK.DEFAULT,
788+ attach_type=InterfaceAttachType.NETWORK,
789+ dhcp_enabled=True)
790+ ]
791+ network, attach_type = conn.get_default_interface_attachment(
792+ host_interfaces)
793+ # This shows us that the method returned with these values.
794+ self.assertEquals(LIBVIRT_NETWORK.DEFAULT, network)
795+ self.assertEquals(InterfaceAttachType.NETWORK, attach_type)
796
797- def test_get_best_network_returns_default(self):
798+ def test_get_default_interface_attachment_vlan_bridge_no_virsh_dhcp(self):
799 conn = self.configure_virshssh('')
800 self.patch(virsh.VirshSSH, "get_network_list").return_value = [
801- 'default', 'other']
802- self.assertEquals('default', conn.get_best_network())
803+ LIBVIRT_NETWORK.MAAS, LIBVIRT_NETWORK.DEFAULT, 'other']
804+ self.patch(
805+ virsh.VirshSSH, "check_network_maas_dhcp_enabled").side_effect = (
806+ None, None, 'br0')
807+ host_interfaces = [
808+ KnownHostInterface(
809+ ifname='br0',
810+ attach_type=InterfaceAttachType.BRIDGE,
811+ dhcp_enabled=True)
812+ ]
813+ ifname, attach_type = conn.get_default_interface_attachment(
814+ host_interfaces)
815+ # This shows us that the method returned with these values.
816+ self.assertEquals('br0', ifname)
817+ self.assertEquals(InterfaceAttachType.BRIDGE, attach_type)
818
819- def test_get_best_network_returns_first(self):
820+ def test_get_default_interface_attachment_macvlan_no_virsh_dhcp(self):
821 conn = self.configure_virshssh('')
822 self.patch(virsh.VirshSSH, "get_network_list").return_value = [
823- 'first', 'second']
824- self.assertEquals('first', conn.get_best_network())
825+ LIBVIRT_NETWORK.MAAS, LIBVIRT_NETWORK.DEFAULT, 'other']
826+ self.patch(
827+ virsh.VirshSSH, "check_network_maas_dhcp_enabled").side_effect = (
828+ None, None, 'br0')
829+ host_interfaces = [
830+ KnownHostInterface(
831+ ifname='eth0',
832+ attach_type=InterfaceAttachType.MACVLAN,
833+ dhcp_enabled=True)
834+ ]
835+ ifname, attach_type = conn.get_default_interface_attachment(
836+ host_interfaces)
837+ # This shows us that the method returned with these values.
838+ self.assertEquals('eth0', ifname)
839+ self.assertEquals(InterfaceAttachType.MACVLAN, attach_type)
840
841- def test_get_best_network_no_network(self):
842+ def test_get_default_interface_attachment_errors_no_match(self):
843 conn = self.configure_virshssh('')
844 self.patch(virsh.VirshSSH, "get_network_list").return_value = []
845- self.assertRaises(PodInvalidResources, conn.get_best_network)
846+ self.patch(
847+ virsh.VirshSSH, "check_network_maas_dhcp_enabled").side_effect = (
848+ None, None, 'br0')
849+ host_interfaces = [
850+ KnownHostInterface(
851+ ifname=factory.make_name('error'),
852+ attach_type=InterfaceAttachType.MACVLAN,
853+ dhcp_enabled=False)
854+ ]
855+ self.assertRaises(
856+ PodInvalidResources,
857+ conn.get_default_interface_attachment,
858+ host_interfaces)
859
860 def test_attach_interface_defaults_to_network_attachment(self):
861 conn = self.configure_virshssh('')
862- domain = factory.make_name('domain')
863+ request = make_requested_machine()
864 network = factory.make_name('network')
865- self.patch(virsh.VirshSSH, "get_best_network").return_value = network
866+ request.known_host_interfaces = {
867+ network: True,
868+ }
869+ domain = factory.make_name('domain')
870+ mock_get_default_interface_attachment = self.patch(
871+ virsh.VirshSSH, "get_default_interface_attachment")
872+ mock_get_default_interface_attachment.return_value = (
873+ network, InterfaceAttachType.NETWORK)
874 mock_run = self.patch(virsh.VirshSSH, "run")
875 fake_mac = factory.make_mac_address()
876 interface = RequestedMachineInterface()
877 self.patch(virsh, 'generate_mac_address').return_value = fake_mac
878- conn.attach_interface(interface, domain)
879+ conn.attach_interface(request, interface, domain)
880 self.assertThat(mock_run, MockCalledOnceWith([
881 'attach-interface', domain, 'network', network,
882 '--mac', fake_mac,
883@@ -1338,15 +1503,22 @@ class TestVirshSSH(MAASTestCase):
884
885 def test_attach_interface_calls_attaches_network(self):
886 conn = self.configure_virshssh('')
887- domain = factory.make_name('domain')
888+ request = make_requested_machine()
889 network = factory.make_name('network')
890- self.patch(virsh.VirshSSH, "get_best_network").return_value = network
891+ request.known_host_interfaces = {
892+ network: True,
893+ }
894+ domain = factory.make_name('domain')
895+ mock_get_default_interface_attachment = self.patch(
896+ virsh.VirshSSH, "get_default_interface_attachment")
897+ mock_get_default_interface_attachment.return_value = (
898+ network, InterfaceAttachType.NETWORK)
899 mock_run = self.patch(virsh.VirshSSH, "run")
900 fake_mac = factory.make_mac_address()
901 interface = RequestedMachineInterface(
902 attach_name=network, attach_type='network')
903 self.patch(virsh, 'generate_mac_address').return_value = fake_mac
904- conn.attach_interface(interface, domain)
905+ conn.attach_interface(request, interface, domain)
906 self.assertThat(mock_run, MockCalledOnceWith([
907 'attach-interface', domain, 'network', network,
908 '--mac', fake_mac,
909@@ -1354,6 +1526,7 @@ class TestVirshSSH(MAASTestCase):
910
911 def test_attach_interface_attaches_macvlan(self):
912 conn = self.configure_virshssh('')
913+ request = make_requested_machine()
914 domain = factory.make_name('domain')
915 mock_run = self.patch(virsh.VirshSSH, "run")
916 fake_mac = factory.make_mac_address()
917@@ -1366,7 +1539,7 @@ class TestVirshSSH(MAASTestCase):
918 tmpfile = NamedTemporaryFile.return_value
919 tmpfile.__enter__.return_value = tmpfile
920 tmpfile.name = factory.make_name("filename")
921- conn.attach_interface(interface, domain)
922+ conn.attach_interface(request, interface, domain)
923
924 device_params = {
925 'mac_address': fake_mac,
926@@ -1387,6 +1560,7 @@ class TestVirshSSH(MAASTestCase):
927
928 def test_attach_interface_attaches_bridge(self):
929 conn = self.configure_virshssh('')
930+ request = make_requested_machine()
931 domain = factory.make_name('domain')
932 mock_run = self.patch(virsh.VirshSSH, "run")
933 fake_mac = factory.make_mac_address()
934@@ -1399,7 +1573,7 @@ class TestVirshSSH(MAASTestCase):
935 tmpfile = NamedTemporaryFile.return_value
936 tmpfile.__enter__.return_value = tmpfile
937 tmpfile.name = factory.make_name("filename")
938- conn.attach_interface(interface, domain)
939+ conn.attach_interface(request, interface, domain)
940
941 device_params = {
942 'mac_address': fake_mac,
943@@ -1506,35 +1680,6 @@ class TestVirshSSH(MAASTestCase):
944 PodInvalidResources, conn.create_domain, request)
945 self.assertEqual("not enough space for disk 0.", str(error))
946
947- def test_create_domain_handles_no_network_for_requested_interface(self):
948- conn = self.configure_virshssh('')
949- request = make_requested_machine()
950- request.block_devices = request.block_devices[:1]
951- request.interfaces = request.interfaces[:1]
952- disk_info = (factory.make_name('pool'), factory.make_name('vol'))
953- domain_params = {
954- "type": "kvm",
955- "emulator": "/usr/bin/qemu-system-x86_64",
956- }
957- self.patch(
958- virsh.VirshSSH, "create_local_volume").return_value = disk_info
959- self.patch(virsh.VirshSSH, "get_domain_capabilities").return_value = (
960- domain_params)
961- mock_uuid = self.patch(virsh_module, "uuid4")
962- mock_uuid.return_value = str(uuid4())
963- domain_params['name'] = request.hostname
964- domain_params['uuid'] = mock_uuid.return_value
965- domain_params['arch'] = ARCH_FIX_REVERSE[request.architecture]
966- domain_params['cores'] = str(request.cores)
967- domain_params['memory'] = str(request.memory)
968- NamedTemporaryFile = self.patch(virsh_module, "NamedTemporaryFile")
969- tmpfile = NamedTemporaryFile.return_value
970- tmpfile.__enter__.return_value = tmpfile
971- tmpfile.name = factory.make_name("filename")
972- self.patch(virsh.VirshSSH, "run")
973- self.patch(virsh.VirshSSH, "get_network_list").return_value = []
974- self.assertRaises(PodInvalidResources, conn.create_domain, request)
975-
976 def test_create_domain_calls_correct_methods_with_amd64_arch(self):
977 conn = self.configure_virshssh('')
978 request = make_requested_machine()
979@@ -1585,7 +1730,7 @@ class TestVirshSSH(MAASTestCase):
980 mock_attach_disk,
981 MockCalledOnceWith(ANY, disk_info[0], disk_info[1], 'vda'))
982 self.assertThat(
983- mock_attach_nic, MockCalledOnceWith(ANY, ANY))
984+ mock_attach_nic, MockCalledOnceWith(request, ANY, ANY))
985 self.assertThat(
986 mock_check_machine_can_startup,
987 MockCalledOnceWith(request.hostname))
988@@ -1648,7 +1793,7 @@ class TestVirshSSH(MAASTestCase):
989 mock_attach_disk,
990 MockCalledOnceWith(ANY, disk_info[0], disk_info[1], 'vda'))
991 self.assertThat(
992- mock_attach_nic, MockCalledOnceWith(ANY, ANY))
993+ mock_attach_nic, MockCalledOnceWith(request, ANY, ANY))
994 self.assertThat(
995 mock_check_machine_can_startup,
996 MockCalledOnceWith(request.hostname))
997@@ -1711,7 +1856,7 @@ class TestVirshSSH(MAASTestCase):
998 mock_attach_disk,
999 MockCalledOnceWith(ANY, disk_info[0], disk_info[1], 'vda'))
1000 self.assertThat(
1001- mock_attach_nic, MockCalledOnceWith(ANY, ANY))
1002+ mock_attach_nic, MockCalledOnceWith(request, ANY, ANY))
1003 self.assertThat(
1004 mock_check_machine_can_startup,
1005 MockCalledOnceWith(request.hostname))
1006@@ -1774,7 +1919,7 @@ class TestVirshSSH(MAASTestCase):
1007 mock_attach_disk,
1008 MockCalledOnceWith(ANY, disk_info[0], disk_info[1], 'vda'))
1009 self.assertThat(
1010- mock_attach_nic, MockCalledOnceWith(ANY, ANY))
1011+ mock_attach_nic, MockCalledOnceWith(request, ANY, ANY))
1012 self.assertThat(
1013 mock_check_machine_can_startup,
1014 MockCalledOnceWith(request.hostname))
1015diff --git a/src/provisioningserver/drivers/pod/virsh.py b/src/provisioningserver/drivers/pod/virsh.py
1016index 451b38a..e738001 100644
1017--- a/src/provisioningserver/drivers/pod/virsh.py
1018+++ b/src/provisioningserver/drivers/pod/virsh.py
1019@@ -34,6 +34,7 @@ from provisioningserver.drivers.pod import (
1020 InterfaceAttachType,
1021 PodDriver,
1022 )
1023+from provisioningserver.enum import LIBVIRT_NETWORK
1024 from provisioningserver.logger import get_maas_logger
1025 from provisioningserver.rpc.exceptions import PodInvalidResources
1026 from provisioningserver.rpc.utils import (
1027@@ -56,6 +57,11 @@ from twisted.internet.threads import deferToThread
1028
1029 maaslog = get_maas_logger("drivers.pod.virsh")
1030
1031+ADD_DEFAULT_NETWORK = dedent("""
1032+ Please add a 'default' or 'maas' network whose bridge is
1033+ on a MAAS DHCP enabled VLAN. Ensure that libvirt DHCP is
1034+ not enabled.
1035+ """)
1036
1037 XPATH_ARCH = "/domain/os/type/@arch"
1038 XPATH_BOOT = "/domain/os/boot"
1039@@ -316,6 +322,13 @@ class VirshSSH(pexpect.spawn):
1040 cmd = 'virsh --connect %s' % poweraddr
1041 self._spawn(cmd)
1042
1043+ def get_network_xml(self, network):
1044+ output = self.run(['net-dumpxml', network]).strip()
1045+ if output.startswith("error:"):
1046+ maaslog.error("%s: Failed to get XML for network", network)
1047+ return None
1048+ return output
1049+
1050 def get_machine_xml(self, machine):
1051 # Check if we have a cached version of the XML.
1052 # This is a short-lived object, so we don't need to worry about
1053@@ -875,46 +888,123 @@ class VirshSSH(pexpect.spawn):
1054 output = self.run(['net-list', '--name'])
1055 return output.strip().splitlines()
1056
1057- def get_best_network(self):
1058- """Return the best possible network."""
1059+ def check_network_maas_dhcp_enabled(
1060+ self, network, host_interfaces):
1061+ xml = self.get_network_xml(network)
1062+ tree = etree.fromstring(xml)
1063+ # Check that libvirt DHCP is not enabled.
1064+ if not tree.xpath('//dhcp'):
1065+ # Validate that network is attached to a bridge
1066+ # whose VLAN is enabled for MAAS DHCP.
1067+ bridge_element = tree.xpath('//bridge')[0]
1068+ ifname = bridge_element.get('name')
1069+ if (ifname in host_interfaces and host_interfaces[ifname]):
1070+ return ifname
1071+
1072+ def get_default_interface_attachment(self, known_host_interfaces):
1073+ """Return the interface attachment for the best attachment option
1074+ currenlty available for the libvirt networks.
1075+
1076+ The criteria for retrieving the best possible interface attachments
1077+ here is done by:
1078+
1079+ (1) If a `maas` or `default` network is defined in libvirt, continue
1080+ to prefer network attachments in that order of preference However,
1081+ DO NOT attach to either network by default if libvirt's DHCP is
1082+ enabled (this is virtually guaranteed to break the operation of MAAS).
1083+
1084+ (2) If a `maas` or `default` network is selected, validate that it is
1085+ attached to a bridge whose VLAN is enabled for MAAS DHCP (or DHCP
1086+ relay). If no libvirt network is found to be attached to a
1087+ DHCP-enabled network in MAAS, skip to (c).
1088+
1089+ (3) If no MAAS-managed DHCP network was found after checking (1) and
1090+ (2), prefer attachments in the following order of precedence:
1091+ (3.1) A bridge interface on the pod host whose VLAN is DHCP-enabled.
1092+ (3.2) A macvlan attachment on the pod host whose VLAN is
1093+ DHCP-enabled.
1094+
1095+ (4) If known_host_interfaces is None, return interface attachments for
1096+ `maas` or `default` network, checked in that order, if they exist
1097+ within the available libvirt networks.
1098+ """
1099 networks = self.get_network_list()
1100- if 'maas' in networks:
1101- return 'maas'
1102- elif 'default' in networks:
1103- return 'default'
1104- elif not networks:
1105- raise PodInvalidResources(
1106- "Pod does not have a network defined. "
1107- "Please add a 'default' or 'maas' network.")
1108
1109- return networks[0]
1110+ # When there are not any known_host_interfaces,
1111+ # default to checking for the `maas` and `default` libvirt networks.
1112+ if known_host_interfaces is None:
1113+ if LIBVIRT_NETWORK.MAAS in networks:
1114+ return LIBVIRT_NETWORK.MAAS, InterfaceAttachType.NETWORK
1115+ elif LIBVIRT_NETWORK.DEFAULT in networks:
1116+ return LIBVIRT_NETWORK.DEFAULT, InterfaceAttachType.NETWORK
1117+ raise PodInvalidResources(ADD_DEFAULT_NETWORK)
1118+
1119+ host_interfaces = {
1120+ host_interface.ifname: host_interface.dhcp_enabled
1121+ for host_interface in known_host_interfaces
1122+ }
1123
1124- def attach_interface(self, interface, domain):
1125+ # Check the 'maas' network.
1126+ if LIBVIRT_NETWORK.MAAS in networks:
1127+ maas_network_ifname = self.check_network_maas_dhcp_enabled(
1128+ LIBVIRT_NETWORK.MAAS, host_interfaces)
1129+ if maas_network_ifname is not None:
1130+ return LIBVIRT_NETWORK.MAAS, InterfaceAttachType.NETWORK
1131+
1132+ # Check the 'default' network.
1133+ if LIBVIRT_NETWORK.DEFAULT in networks:
1134+ default_network_ifname = self.check_network_maas_dhcp_enabled(
1135+ LIBVIRT_NETWORK.DEFAULT, host_interfaces)
1136+ if default_network_ifname is not None:
1137+ return LIBVIRT_NETWORK.DEFAULT, InterfaceAttachType.NETWORK
1138+
1139+ # Check for a bridge interface on the pod host whose VLAN
1140+ # is DHCP enabled.
1141+ for host_interface in known_host_interfaces:
1142+ if (host_interface.attach_type == InterfaceAttachType.BRIDGE and
1143+ host_interface.dhcp_enabled):
1144+ return host_interface.ifname, InterfaceAttachType.BRIDGE
1145+
1146+ # Check for a macvlan interface on the pod host whose VLAN
1147+ # is DHCP enabled.
1148+ for host_interface in known_host_interfaces:
1149+ if (host_interface.attach_type == InterfaceAttachType.MACVLAN and
1150+ host_interface.dhcp_enabled):
1151+ return host_interface.ifname, InterfaceAttachType.MACVLAN
1152+
1153+ # If no interface attachments were found, raise an error.
1154+ raise PodInvalidResources(ADD_DEFAULT_NETWORK)
1155+
1156+ def attach_interface(self, request, interface, domain):
1157 """Attach new network interface on `domain` to `network`."""
1158 mac = generate_mac_address()
1159+ attach_type = interface.attach_type
1160+ attach_name = interface.attach_name
1161 # If attachment type is not specified, default to network.
1162- if interface.attach_type in (None, InterfaceAttachType.NETWORK):
1163- # Set the network if we are explicity attaching a network
1164- # specified by the user.
1165- network = self.get_best_network()
1166- if interface.attach_type is not None:
1167- network = interface.attach_name
1168- return self.run([
1169- 'attach-interface', domain, 'network', network,
1170- '--mac', mac, '--model', 'virtio', '--config'])
1171+ if attach_type in (None, InterfaceAttachType.NETWORK):
1172+ # Retreive the best possible interface attachments.
1173+ # If a viable network cannot be found, resort to
1174+ # trying to attach to a bridge or macvlan interface,
1175+ # which is used below.
1176+ attach_name, attach_type = self.get_default_interface_attachment(
1177+ request.known_host_interfaces)
1178+ if attach_type == InterfaceAttachType.NETWORK:
1179+ return self.run([
1180+ 'attach-interface', domain, 'network', attach_name,
1181+ '--mac', mac, '--model', 'virtio', '--config'])
1182
1183 # For macvlans and bridges, we need to pass an XML template to
1184 # virsh's attach-device command since the attach-interface
1185 # command doesn't have a flag for setting the macvlan's mode.
1186 device_params = {
1187 'mac_address': mac,
1188- 'attach_name': interface.attach_name,
1189+ 'attach_name': attach_name,
1190 }
1191- if interface.attach_type == InterfaceAttachType.MACVLAN:
1192+ if attach_type == InterfaceAttachType.BRIDGE:
1193+ device_xml = DOM_TEMPLATE_BRIDGE_INTERFACE.format(**device_params)
1194+ elif attach_type == InterfaceAttachType.MACVLAN:
1195 device_params['attach_options'] = interface.attach_options
1196 device_xml = DOM_TEMPLATE_MACVLAN_INTERFACE.format(**device_params)
1197- elif interface.attach_type == InterfaceAttachType.BRIDGE:
1198- device_xml = DOM_TEMPLATE_BRIDGE_INTERFACE.format(**device_params)
1199
1200 # Rewrite the XML in a temporary file to use with 'virsh define'.
1201 with NamedTemporaryFile() as f:
1202@@ -1048,7 +1138,7 @@ class VirshSSH(pexpect.spawn):
1203 # Attach the requested interfaces.
1204 for interface in request.interfaces:
1205 try:
1206- self.attach_interface(interface, request.hostname)
1207+ self.attach_interface(request, interface, request.hostname)
1208 except PodInvalidResources as error:
1209 # Delete the defined domain in virsh.
1210 self.run(['destroy', request.hostname])
1211diff --git a/src/provisioningserver/enum.py b/src/provisioningserver/enum.py
1212index ef8ff9d..7e53c46 100644
1213--- a/src/provisioningserver/enum.py
1214+++ b/src/provisioningserver/enum.py
1215@@ -28,3 +28,15 @@ MACVLAN_MODE_CHOICES = (
1216 (MACVLAN_MODE.PRIVATE, "private"),
1217 (MACVLAN_MODE.VEPA, "vepa"),
1218 )
1219+
1220+
1221+class LIBVIRT_NETWORK:
1222+
1223+ DEFAULT = "default"
1224+ MAAS = "maas"
1225+
1226+
1227+LIBVIRT_NETWORK_CHOICES = (
1228+ (LIBVIRT_NETWORK.DEFAULT, "default"),
1229+ (LIBVIRT_NETWORK.MAAS, "maas"),
1230+)

Subscribers

People subscribed via source and target branches

to all changes: