Merge ~bjornt/maas:bug-1772906-fix-pod-update-api into maas:master

Proposed by Björn Tillenius
Status: Merged
Approved by: Björn Tillenius
Approved revision: f1a1cdfde86f33481b60e7ef2dd9b92cf37a4018
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~bjornt/maas:bug-1772906-fix-pod-update-api
Merge into: maas:master
Diff against target: 343 lines (+64/-119)
6 files modified
src/maasserver/api/tests/test_pods.py (+20/-0)
src/maasserver/forms/pods.py (+5/-1)
src/maasserver/forms/tests/test_pods.py (+23/-12)
src/maasserver/models/tests/test_bmc.py (+2/-2)
src/maasserver/testing/factory.py (+4/-1)
src/maasserver/websockets/handlers/tests/test_pod.py (+10/-103)
Reviewer Review Type Date Requested Status
MAAS Lander Needs Fixing
Alberto Donato (community) Approve
Review via email: mp+346735@code.launchpad.net

Commit message

LP: #1772906 - Pods can't be updated via the API without passing power_address

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

+1

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :
Revision history for this message
MAAS Lander (maas-lander) wrote :
Revision history for this message
MAAS Lander (maas-lander) wrote :
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b bug-1772906-fix-pod-update-api lp:~bjornt/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/2911/console
COMMIT: aa1df742fc57ac9acb2b9254284ce89fda6ef091

review: Needs Fixing
f1a1cdf... by Björn Tillenius

Fix test failure.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/maasserver/api/tests/test_pods.py b/src/maasserver/api/tests/test_pods.py
index fed3570..863df22 100644
--- a/src/maasserver/api/tests/test_pods.py
+++ b/src/maasserver/api/tests/test_pods.py
@@ -276,6 +276,26 @@ class TestPodAPI(APITestCase.ForUser, PodMixin):
276 self.assertEqual(new_name, parsed_output['name'])276 self.assertEqual(new_name, parsed_output['name'])
277 self.assertEqual(discovered_pod.cores, parsed_output['total']['cores'])277 self.assertEqual(discovered_pod.cores, parsed_output['total']['cores'])
278278
279 def test_PUT_update_minimal(self):
280 self.become_admin()
281 pod_info = self.make_pod_info()
282 power_parameters = {
283 'power_address': pod_info['power_address'],
284 'power_pass': pod_info['power_pass'],
285 }
286 pod = factory.make_Pod(
287 pod_type=pod_info['type'], parameters=power_parameters)
288 new_name = factory.make_name('pool')
289 self.fake_pod_discovery()
290 response = self.client.put(get_pod_uri(pod), {
291 'name': new_name,
292 })
293 self.assertEqual(
294 http.client.OK, response.status_code, response.content)
295 pod.refresh_from_db()
296 self.assertEqual(new_name, pod.name)
297 self.assertEqual(power_parameters, pod.power_parameters)
298
279 def test_refresh_requires_admin(self):299 def test_refresh_requires_admin(self):
280 pod = factory.make_Pod()300 pod = factory.make_Pod()
281 response = self.client.post(get_pod_uri(pod), {301 response = self.client.post(get_pod_uri(pod), {
diff --git a/src/maasserver/forms/pods.py b/src/maasserver/forms/pods.py
index 654f5f6..89e3b2b 100644
--- a/src/maasserver/forms/pods.py
+++ b/src/maasserver/forms/pods.py
@@ -165,10 +165,14 @@ class PodForm(MAASModelForm):
165 # selected type for the pod.165 # selected type for the pod.
166 if len(self.errors) == 0:166 if len(self.errors) == 0:
167 driver_fields = get_driver_parameters_from_json(167 driver_fields = get_driver_parameters_from_json(
168 self.drivers_orig, None, scope=SETTING_SCOPE.BMC)168 self.drivers_orig, scope=SETTING_SCOPE.BMC)
169 self.param_fields = (169 self.param_fields = (
170 driver_fields[self.cleaned_data['type']].field_dict)170 driver_fields[self.cleaned_data['type']].field_dict)
171 self.fields.update(self.param_fields)171 self.fields.update(self.param_fields)
172 if not self.is_new:
173 for key, value in self.instance.power_parameters.items():
174 if key not in self.data:
175 self.data[key] = value
172 super(PodForm, self)._clean_fields()176 super(PodForm, self)._clean_fields()
173177
174 def clean(self):178 def clean(self):
diff --git a/src/maasserver/forms/tests/test_pods.py b/src/maasserver/forms/tests/test_pods.py
index 64bfae4..a567f31 100644
--- a/src/maasserver/forms/tests/test_pods.py
+++ b/src/maasserver/forms/tests/test_pods.py
@@ -85,7 +85,7 @@ def make_pod_with_hints():
85 pod = factory.make_Pod(85 pod = factory.make_Pod(
86 architectures=architectures, cores=random.randint(8, 16),86 architectures=architectures, cores=random.randint(8, 16),
87 memory=random.randint(4096, 8192),87 memory=random.randint(4096, 8192),
88 cpu_speed=random.randint(2000, 3000))88 cpu_speed=random.randint(2000, 3000), parameters={})
89 for _ in range(3):89 for _ in range(3):
90 pool = factory.make_PodStoragePool(pod)90 pool = factory.make_PodStoragePool(pod)
91 pod.default_storage_pool = pool91 pod.default_storage_pool = pool
@@ -369,18 +369,22 @@ class TestPodForm(MAASTransactionServerTestCase):
369369
370 def test_updates_existing_pod_minimal(self):370 def test_updates_existing_pod_minimal(self):
371 self.fake_pod_discovery()371 self.fake_pod_discovery()
372 pod_info = self.make_pod_info()
373 zone = factory.make_Zone()372 zone = factory.make_Zone()
374 pool = factory.make_ResourcePool()373 pool = factory.make_ResourcePool()
375 cpu_over_commit = random.randint(0, 10)374 cpu_over_commit = random.randint(0, 10)
376 memory_over_commit = random.randint(0, 10)375 memory_over_commit = random.randint(0, 10)
376 power_parameters = {
377 'power_address': 'qemu+ssh://1.2.3.4/system',
378 'power_pass': 'pass',
379 }
377 orig_pod = factory.make_Pod(380 orig_pod = factory.make_Pod(
378 pod_type=pod_info['type'], zone=zone, default_pool=pool,381 pod_type='virsh', zone=zone, default_pool=pool,
379 cpu_over_commit_ratio=cpu_over_commit,382 cpu_over_commit_ratio=cpu_over_commit,
380 memory_over_commit_ratio=memory_over_commit)383 memory_over_commit_ratio=memory_over_commit,
384 parameters=power_parameters)
381 new_name = factory.make_name("pod")385 new_name = factory.make_name("pod")
382 pod_info['name'] = new_name386 form = PodForm(
383 form = PodForm(data=pod_info, request=self.request, instance=orig_pod)387 data={'name': new_name}, request=self.request, instance=orig_pod)
384 self.assertTrue(form.is_valid(), form._errors)388 self.assertTrue(form.is_valid(), form._errors)
385 pod = form.save()389 pod = form.save()
386 self.assertEqual(new_name, pod.name)390 self.assertEqual(new_name, pod.name)
@@ -388,6 +392,8 @@ class TestPodForm(MAASTransactionServerTestCase):
388 self.assertEqual(pool, pod.default_pool)392 self.assertEqual(pool, pod.default_pool)
389 self.assertEqual(cpu_over_commit, pod.cpu_over_commit_ratio)393 self.assertEqual(cpu_over_commit, pod.cpu_over_commit_ratio)
390 self.assertEqual(memory_over_commit, pod.memory_over_commit_ratio)394 self.assertEqual(memory_over_commit, pod.memory_over_commit_ratio)
395 self.assertEqual(memory_over_commit, pod.memory_over_commit_ratio)
396 self.assertEqual(power_parameters, pod.power_parameters)
391397
392 def test_updates_existing_pod(self):398 def test_updates_existing_pod(self):
393 discovered_pod, discovered_racks, failed_racks = (399 discovered_pod, discovered_racks, failed_racks = (
@@ -533,7 +539,10 @@ class TestPodForm(MAASTransactionServerTestCase):
533 self.fake_pod_discovery())539 self.fake_pod_discovery())
534 zone = factory.make_Zone()540 zone = factory.make_Zone()
535 pod_info = self.make_pod_info()541 pod_info = self.make_pod_info()
536 orig_pod = factory.make_Pod(zone=zone, pod_type=pod_info['type'])542 power_parameters = {'power_address': pod_info['power_address']}
543 orig_pod = factory.make_Pod(
544 zone=zone, pod_type=pod_info['type'],
545 parameters=power_parameters)
537 form = PodForm(data=pod_info, request=self.request, instance=orig_pod)546 form = PodForm(data=pod_info, request=self.request, instance=orig_pod)
538 pod = form.discover_and_sync_pod()547 pod = form.discover_and_sync_pod()
539 self.assertThat(pod, MatchesStructure(548 self.assertThat(pod, MatchesStructure(
@@ -546,8 +555,8 @@ class TestPodForm(MAASTransactionServerTestCase):
546 cpu_speed=Equals(discovered_pod.cpu_speed),555 cpu_speed=Equals(discovered_pod.cpu_speed),
547 zone=Equals(zone),556 zone=Equals(zone),
548 power_type=Equals(pod_info['type']),557 power_type=Equals(pod_info['type']),
549 power_parameters=Equals({}),558 power_parameters=Equals(power_parameters),
550 ip_address=Is(None),559 ip_address=MatchesStructure(ip=Equals(pod_info['ip_address'])),
551 ))560 ))
552 routable_racks = [561 routable_racks = [
553 relation.rack_controller562 relation.rack_controller
@@ -571,8 +580,10 @@ class TestPodForm(MAASTransactionServerTestCase):
571 pods_module.discover_pod.return_value)580 pods_module.discover_pod.return_value)
572 zone = yield deferToDatabase(factory.make_Zone)581 zone = yield deferToDatabase(factory.make_Zone)
573 pod_info = yield deferToDatabase(self.make_pod_info)582 pod_info = yield deferToDatabase(self.make_pod_info)
583 power_parameters = {'power_address': pod_info['power_address']}
574 orig_pod = yield deferToDatabase(584 orig_pod = yield deferToDatabase(
575 factory.make_Pod, zone=zone, pod_type=pod_info['type'])585 factory.make_Pod, zone=zone, pod_type=pod_info['type'],
586 parameters=power_parameters)
576 form = yield deferToDatabase(587 form = yield deferToDatabase(
577 PodForm, data=pod_info, request=self.request, instance=orig_pod)588 PodForm, data=pod_info, request=self.request, instance=orig_pod)
578 pod = yield form.discover_and_sync_pod()589 pod = yield form.discover_and_sync_pod()
@@ -586,8 +597,8 @@ class TestPodForm(MAASTransactionServerTestCase):
586 cpu_speed=Equals(discovered_pod.cpu_speed),597 cpu_speed=Equals(discovered_pod.cpu_speed),
587 zone=Equals(zone),598 zone=Equals(zone),
588 power_type=Equals(pod_info['type']),599 power_type=Equals(pod_info['type']),
589 power_parameters=Equals({}),600 power_parameters=Equals(power_parameters),
590 ip_address=Is(None),601 ip_address=MatchesStructure(ip=Equals(pod_info['ip_address'])),
591 ))602 ))
592603
593 def validate_rack_routes():604 def validate_rack_routes():
diff --git a/src/maasserver/models/tests/test_bmc.py b/src/maasserver/models/tests/test_bmc.py
index 9bc3cfb..84db146 100644
--- a/src/maasserver/models/tests/test_bmc.py
+++ b/src/maasserver/models/tests/test_bmc.py
@@ -1509,8 +1509,8 @@ class TestPodDelete(MAASTransactionServerTestCase):
1509 yield pod.async_delete()1509 yield pod.async_delete()
1510 self.assertThat(1510 self.assertThat(
1511 client, MockCalledOnceWith(1511 client, MockCalledOnceWith(
1512 DecomposeMachine, type=pod.power_type, context={},1512 DecomposeMachine, type=pod.power_type,
1513 pod_id=pod.id, name=pod.name))1513 context=pod.power_parameters, pod_id=pod.id, name=pod.name))
1514 decomposable_machine = yield deferToDatabase(1514 decomposable_machine = yield deferToDatabase(
1515 reload_object, decomposable_machine)1515 reload_object, decomposable_machine)
1516 delete_machine = yield deferToDatabase(reload_object, delete_machine)1516 delete_machine = yield deferToDatabase(reload_object, delete_machine)
diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py
index 0204588..e5bd2b7 100644
--- a/src/maasserver/testing/factory.py
+++ b/src/maasserver/testing/factory.py
@@ -610,7 +610,10 @@ class Factory(maastesting.factory.Factory):
610 if pod_type is None:610 if pod_type is None:
611 pod_type = 'virsh'611 pod_type = 'virsh'
612 if parameters is None:612 if parameters is None:
613 parameters = {}613 parameters = {
614 'power_address': 'qemu+ssh://{}/system'.format(
615 self.make_ip_address())
616 }
614 pod = Pod(617 pod = Pod(
615 power_type=pod_type, power_parameters=parameters,618 power_type=pod_type, power_parameters=parameters,
616 ip_address=ip_address, **kwargs)619 ip_address=ip_address, **kwargs)
diff --git a/src/maasserver/websockets/handlers/tests/test_pod.py b/src/maasserver/websockets/handlers/tests/test_pod.py
index 7539a5a..d3a4621 100644
--- a/src/maasserver/websockets/handlers/tests/test_pod.py
+++ b/src/maasserver/websockets/handlers/tests/test_pod.py
@@ -9,14 +9,12 @@ import random
9from unittest.mock import MagicMock9from unittest.mock import MagicMock
1010
11from crochet import wait_for11from crochet import wait_for
12from maasserver.enum import NODE_TYPE
13from maasserver.forms import pods12from maasserver.forms import pods
14from maasserver.forms.pods import PodForm13from maasserver.forms.pods import PodForm
15from maasserver.testing.factory import factory14from maasserver.testing.factory import factory
16from maasserver.testing.testcase import MAASTransactionServerTestCase15from maasserver.testing.testcase import MAASTransactionServerTestCase
17from maasserver.utils.orm import reload_object16from maasserver.utils.orm import reload_object
18from maasserver.utils.threads import deferToDatabase17from maasserver.utils.threads import deferToDatabase
19from maasserver.websockets.base import dehydrate_datetime
20from maasserver.websockets.handlers.pod import (18from maasserver.websockets.handlers.pod import (
21 ComposeMachineForm,19 ComposeMachineForm,
22 PodHandler,20 PodHandler,
@@ -113,116 +111,22 @@ class TestPodHandler(MAASTransactionServerTestCase):
113 cpu_speed=random.randint(1000, 3000), local_storage=0)111 cpu_speed=random.randint(1000, 3000), local_storage=0)
114 return composed_machine, pod_hints112 return composed_machine, pod_hints
115113
116 def dehydrate_storage_pool(self, pool):
117 used = pool.get_used_storage()
118 return {
119 'id': pool.pool_id,
120 'name': pool.name,
121 'type': pool.pool_type,
122 'path': pool.path,
123 'total': pool.storage,
124 'used': used,
125 'available': pool.storage - used,
126 }
127
128 def dehydrate_pod(self, pod, admin=True, for_list=True):
129 data = {
130 "id": pod.id,
131 "name": pod.name,
132 "cpu_speed": pod.cpu_speed,
133 "type": pod.power_type,
134 "ip_address": pod.ip_address,
135 "updated": dehydrate_datetime(pod.updated),
136 "created": dehydrate_datetime(pod.created),
137 "composed_machines_count": pod.node_set.filter(
138 node_type=NODE_TYPE.MACHINE).count(),
139 "total": {
140 "cores": pod.cores,
141 "memory": pod.memory,
142 'memory_gb': '%.1f' % (pod.memory / 1024.0),
143 "local_storage": pod.local_storage,
144 'local_storage_gb': '%.1f' % (pod.local_storage / (1024 ** 3)),
145 },
146 "used": {
147 "cores": pod.get_used_cores(),
148 "memory": pod.get_used_memory(),
149 'memory_gb': '%.1f' % (pod.get_used_memory() / 1024.0),
150 "local_storage": pod.get_used_local_storage(),
151 'local_storage_gb': '%.1f' % (
152 pod.get_used_local_storage() / (1024 ** 3)),
153 },
154 "available": {
155 "cores": pod.cores - pod.get_used_cores(),
156 "memory": pod.memory - pod.get_used_memory(),
157 'memory_gb': '%.1f' % (
158 (pod.memory - pod.get_used_memory()) / 1024.0),
159 "local_storage": (
160 pod.local_storage - pod.get_used_local_storage()),
161 'local_storage_gb': '%.1f' % (
162 (pod.local_storage - pod.get_used_local_storage()) / (
163 (1024 ** 3))),
164 },
165 "default_pool": pod.default_pool.id,
166 "capabilities": pod.capabilities,
167 "architectures": pod.architectures,
168 "hints": {
169 'cores': pod.hints.cores,
170 'cpu_speed': pod.hints.cpu_speed,
171 'memory': pod.hints.memory,
172 'memory_gb': '%.1f' % (pod.hints.memory / 1024.0),
173 'local_storage': pod.hints.local_storage,
174 'local_storage_gb': '%.1f' % (
175 pod.hints.local_storage / (1024 ** 3)),
176 'local_disks': pod.hints.local_disks,
177 'iscsi_storage': pod.hints.iscsi_storage,
178 'iscsi_storage_gb': '%.1f' % (
179 pod.hints.iscsi_storage / (1024 ** 3)),
180 },
181 "tags": pod.tags,
182 "zone": pod.zone.id,
183 "cpu_over_commit_ratio": pod.cpu_over_commit_ratio,
184 "memory_over_commit_ratio": pod.memory_over_commit_ratio
185 }
186 if Capabilities.FIXED_LOCAL_STORAGE in pod.capabilities:
187 data['total']['local_disks'] = pod.local_disks
188 data['used']['local_disks'] = pod.get_used_local_disks()
189 data['available']['local_disks'] = (
190 pod.local_disks - pod.get_used_local_disks())
191 if Capabilities.ISCSI_STORAGE in pod.capabilities:
192 data['total']['iscsi_storage'] = pod.iscsi_storage
193 data['total']['iscsi_storage_gb'] = '%.1f' % (
194 pod.iscsi_storage / (1024 ** 3))
195 data['used']['iscsi_storage'] = pod.get_used_iscsi_storage()
196 data['used']['iscsi_storage_gb'] = '%.1f' % (
197 pod.get_used_iscsi_storage() / (1024 ** 3))
198 data['available']['iscsi_storage'] = (
199 pod.iscsi_storage - pod.get_used_iscsi_storage())
200 data['available']['iscsi_storage_gb'] = '%.1f' % (
201 (pod.iscsi_storage - pod.get_used_iscsi_storage()) / (
202 1024 ** 3))
203 if admin:
204 data.update(pod.power_parameters)
205 if not for_list and pod.storage_pools.all():
206 data["storage_pools"] = [
207 self.dehydrate_storage_pool(pool)
208 for pool in pod.storage_pools.all()
209 ]
210 data["default_storage_pool"] = pod.default_storage_pool.pool_id
211 return data
212
213 def test_get(self):114 def test_get(self):
214 admin = factory.make_admin()115 admin = factory.make_admin()
215 handler = PodHandler(admin, {})116 handler = PodHandler(admin, {})
216 pod = self.make_pod_with_hints()117 pod = self.make_pod_with_hints()
217 expected_data = self.dehydrate_pod(pod, for_list=False)118 expected_data = handler.full_dehydrate(pod)
218 result = handler.get({"id": pod.id})119 result = handler.get({"id": pod.id})
120 self.assertItemsEqual(expected_data.keys(), result.keys())
121 for key in expected_data:
122 self.assertEqual(expected_data[key], result[key], key)
219 self.assertThat(result, Equals(expected_data))123 self.assertThat(result, Equals(expected_data))
220124
221 def test_get_as_standard_user(self):125 def test_get_as_standard_user(self):
222 user = factory.make_User()126 user = factory.make_User()
223 handler = PodHandler(user, {})127 handler = PodHandler(user, {})
224 pod = self.make_pod_with_hints()128 pod = self.make_pod_with_hints()
225 expected_data = self.dehydrate_pod(pod, admin=False, for_list=False)129 expected_data = handler.full_dehydrate(pod)
226 result = handler.get({"id": pod.id})130 result = handler.get({"id": pod.id})
227 self.assertThat(result, Equals(expected_data))131 self.assertThat(result, Equals(expected_data))
228132
@@ -230,7 +134,7 @@ class TestPodHandler(MAASTransactionServerTestCase):
230 admin = factory.make_admin()134 admin = factory.make_admin()
231 handler = PodHandler(admin, {})135 handler = PodHandler(admin, {})
232 pod = self.make_pod_with_hints()136 pod = self.make_pod_with_hints()
233 expected_data = [self.dehydrate_pod(pod, for_list=True)]137 expected_data = [handler.full_dehydrate(pod, for_list=True)]
234 result = handler.list({"id": pod.id})138 result = handler.list({"id": pod.id})
235 self.assertThat(result, Equals(expected_data))139 self.assertThat(result, Equals(expected_data))
236140
@@ -244,10 +148,13 @@ class TestPodHandler(MAASTransactionServerTestCase):
244 PodForm, 'discover_and_sync_pod')148 PodForm, 'discover_and_sync_pod')
245 mock_discover_and_sync_pod.return_value = succeed(pod)149 mock_discover_and_sync_pod.return_value = succeed(pod)
246 expected_data = yield deferToDatabase(150 expected_data = yield deferToDatabase(
247 self.dehydrate_pod, pod, for_list=False)151 handler.full_dehydrate, pod, for_list=False)
248 observed_data = yield handler.refresh({"id": pod.id})152 observed_data = yield handler.refresh({"id": pod.id})
249 self.assertThat(153 self.assertThat(
250 mock_discover_and_sync_pod, MockCalledOnceWith())154 mock_discover_and_sync_pod, MockCalledOnceWith())
155 self.assertItemsEqual(expected_data.keys(), observed_data.keys())
156 for key in expected_data:
157 self.assertEqual(expected_data[key], observed_data[key], key)
251 self.assertEqual(expected_data, observed_data)158 self.assertEqual(expected_data, observed_data)
252159
253 @wait_for_reactor160 @wait_for_reactor

Subscribers

People subscribed via source and target branches