Merge ~blake-rouse/maas:fix-1793340 into maas:master

Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: 537d6bfa4c147fc26c674d3c46e0aa8fc6935284
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~blake-rouse/maas:fix-1793340
Merge into: maas:master
Diff against target: 448 lines (+114/-21)
26 files modified
src/maasserver/migrations/builtin/maasserver/0177_break_apart_linked_bmcs.py (+42/-0)
src/maasserver/models/bmc.py (+4/-4)
src/maasserver/models/node.py (+23/-13)
src/maasserver/models/tests/test_bmc.py (+6/-4)
src/maasserver/models/tests/test_node.py (+13/-0)
src/provisioningserver/drivers/pod/__init__.py (+2/-0)
src/provisioningserver/drivers/pod/tests/test_base.py (+1/-0)
src/provisioningserver/drivers/power/__init__.py (+4/-0)
src/provisioningserver/drivers/power/amt.py (+1/-0)
src/provisioningserver/drivers/power/apc.py (+1/-0)
src/provisioningserver/drivers/power/dli.py (+1/-0)
src/provisioningserver/drivers/power/fence_cdu.py (+1/-0)
src/provisioningserver/drivers/power/hmc.py (+1/-0)
src/provisioningserver/drivers/power/ipmi.py (+1/-0)
src/provisioningserver/drivers/power/manual.py (+1/-0)
src/provisioningserver/drivers/power/moonshot.py (+1/-0)
src/provisioningserver/drivers/power/mscm.py (+1/-0)
src/provisioningserver/drivers/power/msftocs.py (+1/-0)
src/provisioningserver/drivers/power/nova.py (+1/-0)
src/provisioningserver/drivers/power/recs.py (+1/-0)
src/provisioningserver/drivers/power/seamicro.py (+1/-0)
src/provisioningserver/drivers/power/tests/test_base.py (+2/-0)
src/provisioningserver/drivers/power/ucsm.py (+1/-0)
src/provisioningserver/drivers/power/virsh.py (+1/-0)
src/provisioningserver/drivers/power/vmware.py (+1/-0)
src/provisioningserver/drivers/power/wedge.py (+1/-0)
Reviewer Review Type Date Requested Status
Newell Jensen (community) Approve
Review via email: mp+355483@code.launchpad.net

Commit message

Fixes LP: #1793340 - Never merge together power drivers that are not for a chassis.

This includes a data migration that splits BMC's apart that was originally linked together incorrectly on power drivers that are not for chassis.

To post a comment you must log in.
Revision history for this message
Newell Jensen (newell-jensen) wrote :

LGTM

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :
~blake-rouse/maas:fix-1793340 updated
0d24bcd... by Blake Rouse

Fix issue in test.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/migrations/builtin/maasserver/0177_break_apart_linked_bmcs.py b/src/maasserver/migrations/builtin/maasserver/0177_break_apart_linked_bmcs.py
2new file mode 100644
3index 0000000..13c26a9
4--- /dev/null
5+++ b/src/maasserver/migrations/builtin/maasserver/0177_break_apart_linked_bmcs.py
6@@ -0,0 +1,42 @@
7+# -*- coding: utf-8 -*-
8+# Generated by Django 1.11.11 on 2018-09-18 12:28
9+from __future__ import unicode_literals
10+
11+import re
12+
13+from django.db import migrations
14+from provisioningserver.drivers.power.registry import PowerDriverRegistry
15+
16+
17+def get_none_chassis_power_types():
18+ """Return the power_types that are not chassis."""
19+ return [
20+ ptype
21+ for ptype, driver in PowerDriverRegistry
22+ if not driver.chassis
23+ ]
24+
25+
26+def break_apart_linked_bmcs(apps, schema_editor):
27+ BMC = apps.get_model("maasserver", "BMC")
28+ power_types = get_none_chassis_power_types()
29+ for bmc in BMC.objects.filter(power_type__in=power_types).prefetch_related(
30+ 'node_set'):
31+ nodes = list(bmc.node_set.all())
32+ for node in nodes[1:]:
33+ bmc.id = None
34+ bmc._state.adding = True
35+ bmc.save()
36+ node.bmc = bmc
37+ node.save()
38+
39+
40+class Migration(migrations.Migration):
41+
42+ dependencies = [
43+ ('maasserver', '0176_rename_user_id_migrate_to_user_id_for_events'),
44+ ]
45+
46+ operations = [
47+ migrations.RunPython(break_apart_linked_bmcs),
48+ ]
49diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
50index f813057..addc187 100644
51--- a/src/maasserver/models/bmc.py
52+++ b/src/maasserver/models/bmc.py
53@@ -336,15 +336,15 @@ class BMC(CleanSave, TimestampedModel):
54 node-specific ones."""
55 if not power_type:
56 # If there is no power type, treat all params as node params.
57- return ({}, power_params)
58+ return (False, {}, power_params)
59 power_driver = PowerDriverRegistry.get_item(power_type)
60 if power_driver is None:
61 # If there is no power driver, treat all params as node params.
62- return ({}, power_params)
63+ return (False, {}, power_params)
64 power_fields = power_driver.settings
65 if not power_fields:
66 # If there is no parameter info, treat all params as node params.
67- return ({}, power_params)
68+ return (False, {}, power_params)
69 bmc_params = {}
70 node_params = {}
71 for param_name in power_params:
72@@ -354,7 +354,7 @@ class BMC(CleanSave, TimestampedModel):
73 bmc_params[param_name] = power_params[param_name]
74 else:
75 node_params[param_name] = power_params[param_name]
76- return (bmc_params, node_params)
77+ return (power_driver.chassis, bmc_params, node_params)
78
79 @staticmethod
80 def extract_ip_address(power_type, power_parameters):
81diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
82index 1945476..b431d10 100644
83--- a/src/maasserver/models/node.py
84+++ b/src/maasserver/models/node.py
85@@ -1208,27 +1208,37 @@ class Node(CleanSave, TimestampedModel):
86
87 # Circular imports.
88 from maasserver.models.bmc import BMC
89- bmc_params, node_params = BMC.scope_power_parameters(
90+ chassis, bmc_params, node_params = BMC.scope_power_parameters(
91 self.bmc.power_type, power_params)
92 self.instance_power_parameters = node_params
93
94 if self.bmc.power_parameters == bmc_params:
95 return
96
97- conflicts = len(BMC.objects.filter(
98- power_type=self.bmc.power_type, power_parameters=bmc_params)) > 0
99- if not conflicts:
100+ if chassis:
101+ # Update either our BMC or link this node to another existing
102+ # of the same type.
103+ another_exists = BMC.objects.filter(
104+ power_type=self.bmc.power_type,
105+ power_parameters=bmc_params).count() > 0
106+ if not another_exists:
107+ self.bmc.power_parameters = bmc_params
108+ self.bmc.save()
109+ else:
110+ (bmc, _) = BMC.objects.get_or_create(
111+ power_type=self.bmc.power_type,
112+ power_parameters=bmc_params)
113+ # Point all nodes using old BMC at the new one.
114+ if self.bmc is not None and self.bmc_id != bmc.id:
115+ for node in self.bmc.node_set.exclude(id=self.id):
116+ node.bmc = bmc
117+ node.save()
118+ self.bmc = bmc
119+ else:
120+ # This power_type is not linked to more than 1 BMC per node, just
121+ # update the BMC power parameters.
122 self.bmc.power_parameters = bmc_params
123 self.bmc.save()
124- else:
125- (bmc, _) = BMC.objects.get_or_create(
126- power_type=self.bmc.power_type, power_parameters=bmc_params)
127- # Point all nodes using old BMC at the new one.
128- if self.bmc is not None and self.bmc_id != bmc.id:
129- for node in self.bmc.node_set.exclude(id=self.id):
130- node.bmc = bmc
131- node.save()
132- self.bmc = bmc
133
134 @property
135 def fqdn(self):
136diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
137index 00a5ad2..8fb20b5 100644
138--- a/src/maasserver/models/tests/test_bmc.py
139+++ b/src/maasserver/models/tests/test_bmc.py
140@@ -344,8 +344,9 @@ class TestBMC(MAASServerTestCase):
141 )
142 parameters = {**bmc_parameters, **node_parameters}
143 result = BMC.scope_power_parameters('vmware', parameters)
144- self.assertEqual(bmc_parameters, result[0])
145- self.assertEqual(node_parameters, result[1])
146+ self.assertTrue(result[0])
147+ self.assertEqual(bmc_parameters, result[1])
148+ self.assertEqual(node_parameters, result[2])
149
150 def test_scope_power_parameters_unknown_parameter(self):
151 bmc_parameters = dict(power_address=factory.make_string())
152@@ -354,8 +355,9 @@ class TestBMC(MAASServerTestCase):
153 node_parameters[factory.make_string()] = factory.make_string()
154 parameters = {**bmc_parameters, **node_parameters}
155 result = BMC.scope_power_parameters('hmc', parameters)
156- self.assertEqual(bmc_parameters, result[0])
157- self.assertEqual(node_parameters, result[1])
158+ self.assertTrue(result[0])
159+ self.assertEqual(bmc_parameters, result[1])
160+ self.assertEqual(node_parameters, result[2])
161
162 def test_bmc_extract_ip_address_whole_value(self):
163 power_parameters = {'power_address': "192.168.1.1"}
164diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
165index 0bb9308..5b367ae 100644
166--- a/src/maasserver/models/tests/test_node.py
167+++ b/src/maasserver/models/tests/test_node.py
168@@ -4593,6 +4593,19 @@ class TestNodePowerParameters(MAASServerTestCase):
169 self.assertEqual(node_parameters, node.instance_power_parameters)
170 self.assertEqual(bmc_parameters, node.bmc.power_parameters)
171
172+ def test_none_chassis_bmc_doesnt_consolidate(self):
173+ parameters = {
174+ 'power_address': factory.make_ipv4_address(),
175+ 'power_pass': factory.make_string(),
176+ }
177+ for _ in range(3):
178+ node = factory.make_Node(power_type='amt')
179+ node.power_parameters = parameters
180+ node.save()
181+
182+ # Should be 3 BMC's even though they all have the same information.
183+ self.assertEqual(3, BMC.objects.count())
184+
185 def test_bmc_consolidation(self):
186 nodes = []
187 for _ in range(3):
188diff --git a/src/provisioningserver/drivers/pod/__init__.py b/src/provisioningserver/drivers/pod/__init__.py
189index ab5c39c..0c691ad 100644
190--- a/src/provisioningserver/drivers/pod/__init__.py
191+++ b/src/provisioningserver/drivers/pod/__init__.py
192@@ -420,3 +420,5 @@ def get_error_message(err):
193
194 class PodDriver(PowerDriver, PodDriverBase):
195 """Default pod driver."""
196+
197+ chassis = True # Pods are always a chassis
198diff --git a/src/provisioningserver/drivers/pod/tests/test_base.py b/src/provisioningserver/drivers/pod/tests/test_base.py
199index 128b90c..f0f9cc1 100644
200--- a/src/provisioningserver/drivers/pod/tests/test_base.py
201+++ b/src/provisioningserver/drivers/pod/tests/test_base.py
202@@ -706,6 +706,7 @@ class TestRequestClasses(MAASTestCase):
203 class FakePodDriverBase(PodDriverBase):
204
205 name = ""
206+ chassis = True
207 description = ""
208 settings = []
209 ip_extractor = None
210diff --git a/src/provisioningserver/drivers/power/__init__.py b/src/provisioningserver/drivers/power/__init__.py
211index 1a1a602..71945a0 100644
212--- a/src/provisioningserver/drivers/power/__init__.py
213+++ b/src/provisioningserver/drivers/power/__init__.py
214@@ -187,6 +187,10 @@ class PowerDriverBase(metaclass=ABCMeta):
215 def queryable(self):
216 """Whether or not the power driver is queryable."""
217
218+ @abstractproperty
219+ def chassis(self):
220+ """Return True if the power driver is for a chassis."""
221+
222 @abstractmethod
223 def detect_missing_packages(self):
224 """Implement this method for the actual implementation
225diff --git a/src/provisioningserver/drivers/power/amt.py b/src/provisioningserver/drivers/power/amt.py
226index 338b8cd..20d1259 100644
227--- a/src/provisioningserver/drivers/power/amt.py
228+++ b/src/provisioningserver/drivers/power/amt.py
229@@ -57,6 +57,7 @@ REQUIRED_PACKAGES = [["amttool", "amtterm"], ["wsman", "wsmancli"]]
230 class AMTPowerDriver(PowerDriver):
231
232 name = 'amt'
233+ chassis = False
234 description = "Intel AMT"
235 settings = [
236 make_setting_field(
237diff --git a/src/provisioningserver/drivers/power/apc.py b/src/provisioningserver/drivers/power/apc.py
238index 0c5ab77..368f308 100644
239--- a/src/provisioningserver/drivers/power/apc.py
240+++ b/src/provisioningserver/drivers/power/apc.py
241@@ -39,6 +39,7 @@ class APCState:
242 class APCPowerDriver(PowerDriver):
243
244 name = 'apc'
245+ chassis = True
246 description = "American Power Conversion (APC) PDU"
247 settings = [
248 make_setting_field('power_address', "IP for APC PDU", required=True),
249diff --git a/src/provisioningserver/drivers/power/dli.py b/src/provisioningserver/drivers/power/dli.py
250index 6b269c9..f000b61 100644
251--- a/src/provisioningserver/drivers/power/dli.py
252+++ b/src/provisioningserver/drivers/power/dli.py
253@@ -28,6 +28,7 @@ from provisioningserver.utils.shell import (
254
255 class DLIPowerDriver(PowerDriver):
256 name = 'dli'
257+ chassis = True
258 description = "Digital Loggers, Inc. PDU"
259 settings = [
260 make_setting_field(
261diff --git a/src/provisioningserver/drivers/power/fence_cdu.py b/src/provisioningserver/drivers/power/fence_cdu.py
262index 3894bc6..d408629 100644
263--- a/src/provisioningserver/drivers/power/fence_cdu.py
264+++ b/src/provisioningserver/drivers/power/fence_cdu.py
265@@ -28,6 +28,7 @@ from provisioningserver.utils.shell import (
266 class FenceCDUPowerDriver(PowerDriver):
267
268 name = 'fence_cdu'
269+ chassis = True
270 description = "Sentry Switch CDU"
271 settings = [
272 make_setting_field('power_address', "Power address", required=True),
273diff --git a/src/provisioningserver/drivers/power/hmc.py b/src/provisioningserver/drivers/power/hmc.py
274index a653fa2..495bd70 100644
275--- a/src/provisioningserver/drivers/power/hmc.py
276+++ b/src/provisioningserver/drivers/power/hmc.py
277@@ -37,6 +37,7 @@ class HMCState:
278 class HMCPowerDriver(PowerDriver):
279
280 name = 'hmc'
281+ chassis = True
282 description = "IBM Hardware Management Console (HMC)"
283 settings = [
284 make_setting_field('power_address', "IP for HMC", required=True),
285diff --git a/src/provisioningserver/drivers/power/ipmi.py b/src/provisioningserver/drivers/power/ipmi.py
286index e3b1561..bcab917 100644
287--- a/src/provisioningserver/drivers/power/ipmi.py
288+++ b/src/provisioningserver/drivers/power/ipmi.py
289@@ -190,6 +190,7 @@ IPMI_BOOT_TYPE_MAPPING = {
290 class IPMIPowerDriver(PowerDriver):
291
292 name = 'ipmi'
293+ chassis = False
294 description = "IPMI"
295 settings = [
296 make_setting_field(
297diff --git a/src/provisioningserver/drivers/power/manual.py b/src/provisioningserver/drivers/power/manual.py
298index 472546a..b9a0b64 100644
299--- a/src/provisioningserver/drivers/power/manual.py
300+++ b/src/provisioningserver/drivers/power/manual.py
301@@ -16,6 +16,7 @@ maaslog = get_maas_logger("drivers.power.manual")
302 class ManualPowerDriver(PowerDriver):
303
304 name = 'manual'
305+ chassis = False
306 description = "Manual"
307 settings = []
308 ip_extractor = None
309diff --git a/src/provisioningserver/drivers/power/moonshot.py b/src/provisioningserver/drivers/power/moonshot.py
310index ad272ff..9c451a3 100644
311--- a/src/provisioningserver/drivers/power/moonshot.py
312+++ b/src/provisioningserver/drivers/power/moonshot.py
313@@ -28,6 +28,7 @@ from provisioningserver.utils.shell import (
314 class MoonshotIPMIPowerDriver(PowerDriver):
315
316 name = 'moonshot'
317+ chassis = True
318 description = "HP Moonshot - iLO4 (IPMI)"
319 settings = [
320 make_setting_field('power_address', "Power address", required=True),
321diff --git a/src/provisioningserver/drivers/power/mscm.py b/src/provisioningserver/drivers/power/mscm.py
322index 2318620..b5a6632 100644
323--- a/src/provisioningserver/drivers/power/mscm.py
324+++ b/src/provisioningserver/drivers/power/mscm.py
325@@ -64,6 +64,7 @@ class MSCMState:
326 class MSCMPowerDriver(PowerDriver):
327
328 name = 'mscm'
329+ chassis = True
330 description = "HP Moonshot - iLO Chassis Manager"
331 settings = [
332 make_setting_field(
333diff --git a/src/provisioningserver/drivers/power/msftocs.py b/src/provisioningserver/drivers/power/msftocs.py
334index 4a457a0..56d4e98 100644
335--- a/src/provisioningserver/drivers/power/msftocs.py
336+++ b/src/provisioningserver/drivers/power/msftocs.py
337@@ -38,6 +38,7 @@ class MicrosoftOCSState(object):
338 class MicrosoftOCSPowerDriver(PowerDriver):
339
340 name = 'msftocs'
341+ chassis = True
342 description = "Microsoft OCS - Chassis Manager"
343 settings = [
344 make_setting_field('power_address', "Power address", required=True),
345diff --git a/src/provisioningserver/drivers/power/nova.py b/src/provisioningserver/drivers/power/nova.py
346index 49774f2..2607975 100644
347--- a/src/provisioningserver/drivers/power/nova.py
348+++ b/src/provisioningserver/drivers/power/nova.py
349@@ -42,6 +42,7 @@ class NovaPowerState:
350 class NovaPowerDriver(PowerDriver):
351
352 name = 'nova'
353+ chassis = True
354 description = "OpenStack Nova"
355 settings = [
356 make_setting_field(
357diff --git a/src/provisioningserver/drivers/power/recs.py b/src/provisioningserver/drivers/power/recs.py
358index 311bb4e..5a5537b 100644
359--- a/src/provisioningserver/drivers/power/recs.py
360+++ b/src/provisioningserver/drivers/power/recs.py
361@@ -202,6 +202,7 @@ class RECSAPI:
362 class RECSPowerDriver(PowerDriver):
363
364 name = 'recs_box'
365+ chassis = True
366 description = "Christmann RECS|Box Power Driver"
367 settings = [
368 make_setting_field(
369diff --git a/src/provisioningserver/drivers/power/seamicro.py b/src/provisioningserver/drivers/power/seamicro.py
370index cf4491d..585ae4d 100644
371--- a/src/provisioningserver/drivers/power/seamicro.py
372+++ b/src/provisioningserver/drivers/power/seamicro.py
373@@ -45,6 +45,7 @@ def extract_seamicro_parameters(context):
374 class SeaMicroPowerDriver(PowerDriver):
375
376 name = 'sm15k'
377+ chassis = True
378 description = "SeaMicro 15000"
379 settings = [
380 make_setting_field(
381diff --git a/src/provisioningserver/drivers/power/tests/test_base.py b/src/provisioningserver/drivers/power/tests/test_base.py
382index f38b052..e0928e9 100644
383--- a/src/provisioningserver/drivers/power/tests/test_base.py
384+++ b/src/provisioningserver/drivers/power/tests/test_base.py
385@@ -50,6 +50,7 @@ from twisted.internet.defer import (
386 class FakePowerDriverBase(PowerDriverBase):
387
388 name = ""
389+ chassis = False
390 description = ""
391 settings = []
392 ip_extractor = None
393@@ -219,6 +220,7 @@ class TestGetErrorMessage(MAASTestCase):
394 class FakePowerDriver(PowerDriver):
395
396 name = ""
397+ chassis = False
398 description = ""
399 settings = []
400 ip_extractor = None
401diff --git a/src/provisioningserver/drivers/power/ucsm.py b/src/provisioningserver/drivers/power/ucsm.py
402index 164f23f..1f51095 100644
403--- a/src/provisioningserver/drivers/power/ucsm.py
404+++ b/src/provisioningserver/drivers/power/ucsm.py
405@@ -29,6 +29,7 @@ def extract_ucsm_parameters(context):
406 class UCSMPowerDriver(PowerDriver):
407
408 name = 'ucsm'
409+ chassis = True
410 description = "Cisco UCS Manager"
411 settings = [
412 make_setting_field(
413diff --git a/src/provisioningserver/drivers/power/virsh.py b/src/provisioningserver/drivers/power/virsh.py
414index 0bd1e93..cccf310 100644
415--- a/src/provisioningserver/drivers/power/virsh.py
416+++ b/src/provisioningserver/drivers/power/virsh.py
417@@ -33,6 +33,7 @@ def extract_virsh_parameters(context):
418 class VirshPowerDriver(PowerDriver):
419
420 name = 'virsh'
421+ chassis = True
422 description = "Virsh (virtual systems)"
423 settings = [
424 make_setting_field('power_address', "Power address", required=True),
425diff --git a/src/provisioningserver/drivers/power/vmware.py b/src/provisioningserver/drivers/power/vmware.py
426index cbc4b39..691db88 100644
427--- a/src/provisioningserver/drivers/power/vmware.py
428+++ b/src/provisioningserver/drivers/power/vmware.py
429@@ -37,6 +37,7 @@ def extract_vmware_parameters(context):
430 class VMwarePowerDriver(PowerDriver):
431
432 name = 'vmware'
433+ chassis = True
434 description = "VMware"
435 settings = [
436 make_setting_field(
437diff --git a/src/provisioningserver/drivers/power/wedge.py b/src/provisioningserver/drivers/power/wedge.py
438index cb26ca9..61f89ff 100644
439--- a/src/provisioningserver/drivers/power/wedge.py
440+++ b/src/provisioningserver/drivers/power/wedge.py
441@@ -32,6 +32,7 @@ class WedgeState:
442 class WedgePowerDriver(PowerDriver):
443
444 name = 'wedge'
445+ chassis = True
446 description = "Facebook's Wedge"
447 settings = [
448 make_setting_field('power_address', "IP address", required=True),

Subscribers

People subscribed via source and target branches