Merge lp:~blake-rouse/maas/fix-1462507 into lp:maas/trunk

Proposed by Blake Rouse on 2015-06-05
Status: Merged
Approved by: Blake Rouse on 2015-06-05
Approved revision: 3983
Merged at revision: 3996
Proposed branch: lp:~blake-rouse/maas/fix-1462507
Merge into: lp:maas/trunk
Diff against target: 333 lines (+177/-19)
5 files modified
src/maasserver/api/blockdevices.py (+49/-9)
src/maasserver/api/tests/test_blockdevice.py (+83/-4)
src/maasserver/models/blockdevice.py (+19/-0)
src/maasserver/models/tests/test_blockdevice.py (+14/-0)
src/maasserver/urls_api.py (+12/-6)
To merge this branch: bzr merge lp:~blake-rouse/maas/fix-1462507
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) 2015-06-05 Approve on 2015-06-05
Review via email: mp+261284@code.launchpad.net

Commit message

Fix endpoint for BlockDevice API. Add ability for a user to list all the block devices on a node and read a block device on a node. Include the type of blockdevice in the output.

To post a comment you must log in.
lp:~blake-rouse/maas/fix-1462507 updated on 2015-06-05
3983. By Blake Rouse on 2015-06-05

Add blockdevice read operation.

Andres Rodriguez (andreserl) wrote :

lgtm!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api/blockdevices.py'
2--- src/maasserver/api/blockdevices.py 2015-06-01 13:00:47 +0000
3+++ src/maasserver/api/blockdevices.py 2015-06-05 20:24:38 +0000
4@@ -20,12 +20,18 @@
5 OperationsHandler,
6 )
7 from maasserver.api.utils import get_mandatory_param
8-from maasserver.models import BlockDevice
9+from maasserver.enum import NODE_PERMISSION
10+from maasserver.exceptions import MAASAPINotFound
11+from maasserver.models import (
12+ BlockDevice,
13+ Node,
14+)
15
16
17 DISPLAYED_BLOCKDEVICE_FIELDS = (
18 'id',
19 'name',
20+ 'type',
21 'path',
22 'id_path',
23 'size',
24@@ -34,36 +40,70 @@
25 )
26
27
28+class BlockDevicesHandler(OperationsHandler):
29+ """Manage block devices on a node."""
30+ api_doc_section_name = "Block devices"
31+ create = replace = update = delete = None
32+ model = BlockDevice
33+ fields = DISPLAYED_BLOCKDEVICE_FIELDS
34+
35+ def read(self, request, system_id):
36+ """List all block devices belonging to node.
37+
38+ Returns 404 if the node is not found.
39+ """
40+ node = Node.nodes.get_node_or_404(
41+ system_id, request.user, NODE_PERMISSION.VIEW)
42+ return node.blockdevice_set.all()
43+
44+
45 class BlockDeviceHandler(OperationsHandler):
46- """Manage a BlockDevice.
47-
48- The device is identified by its database id.
49- """
50- api_doc_section_name = "BlockDevice"
51- create = replace = update = read = None
52+ """Manage a block device on a node."""
53+ api_doc_section_name = "Block device"
54+ create = replace = update = None
55 model = BlockDevice
56 fields = DISPLAYED_BLOCKDEVICE_FIELDS
57
58+ def read(self, request, system_id, device_id):
59+ """Read block device on node.
60+
61+ Returns 404 if the node or block device is not found.
62+ """
63+ node = Node.nodes.get_node_or_404(
64+ system_id, request.user, NODE_PERMISSION.VIEW)
65+ device = get_object_or_404(BlockDevice, id=device_id)
66+ if device.node != node:
67+ raise MAASAPINotFound()
68+ return device
69+
70 @admin_method
71 @operation(idempotent=True)
72- def add_tag(self, request, device_id):
73+ def add_tag(self, request, system_id, device_id):
74 """Add a tag to a BlockDevice.
75
76 :param tag: The tag being added.
77 """
78+ node = Node.nodes.get_node_or_404(
79+ system_id, request.user, NODE_PERMISSION.ADMIN)
80 device = get_object_or_404(BlockDevice, id=device_id)
81+ if device.node != node:
82+ raise MAASAPINotFound()
83 device.add_tag(get_mandatory_param(request.GET, 'tag'))
84 device.save()
85 return device
86
87 @admin_method
88 @operation(idempotent=True)
89- def remove_tag(self, request, device_id):
90+ def remove_tag(self, request, system_id, device_id):
91 """Remove a tag from a BlockDevice.
92
93 :param tag: The tag being removed.
94 """
95+ node = Node.nodes.get_node_or_404(
96+ system_id, request.user, NODE_PERMISSION.ADMIN)
97 device = get_object_or_404(BlockDevice, id=device_id)
98+ if device.node != node:
99+ raise MAASAPINotFound()
100 device.remove_tag(get_mandatory_param(request.GET, 'tag'))
101 device.save()
102 return device
103
104=== modified file 'src/maasserver/api/tests/test_blockdevice.py'
105--- src/maasserver/api/tests/test_blockdevice.py 2015-06-01 13:00:47 +0000
106+++ src/maasserver/api/tests/test_blockdevice.py 2015-06-05 20:24:38 +0000
107@@ -23,16 +23,73 @@
108 from maasserver.testing.orm import reload_object
109
110
111-def get_blockdevice_uri(device):
112+def get_blockdevices_uri(node):
113+ """Return a Node's BlockDevice URI on the API."""
114+ return reverse(
115+ 'blockdevices_handler', args=[node.system_id])
116+
117+
118+def get_blockdevice_uri(device, node=None):
119 """Return a BlockDevice's URI on the API."""
120- return reverse('blockdevice_handler', args=[device.id])
121+ if node is None:
122+ node = device.node
123+ return reverse(
124+ 'blockdevice_handler', args=[node.system_id, device.id])
125+
126+
127+class TestBlockDevices(APITestCase):
128+
129+ def test_read(self):
130+ node = factory.make_Node()
131+ devices = [
132+ factory.make_PhysicalBlockDevice(node=node)
133+ for _ in range(3)
134+ ]
135+ uri = get_blockdevices_uri(node)
136+ response = self.client.get(uri)
137+
138+ # Ensure the response status is OK
139+ self.assertEqual(httplib.OK, response.status_code, response.content)
140+
141+ # Ensure all the device ids match.
142+ expected_device_ids = [
143+ device.id
144+ for device in devices
145+ ]
146+ result_device_ids = [
147+ device["id"]
148+ for device in json.loads(response.content)
149+ ]
150+ self.assertItemsEqual(expected_device_ids, result_device_ids)
151
152
153 class TestBlockDeviceAPI(APITestCase):
154
155+ def test_read(self):
156+ device = factory.make_PhysicalBlockDevice()
157+ uri = get_blockdevice_uri(device)
158+ response = self.client.get(uri)
159+
160+ # Ensure the response status is OK
161+ self.assertEqual(httplib.OK, response.status_code, response.content)
162+
163+ parsed_device = json.loads(response.content)
164+ self.assertEquals(device.id, parsed_device["id"])
165+ self.assertEquals(device.type, parsed_device["type"])
166+
167+ def test_add_tag_returns_403_for_non_admin(self):
168+ device = factory.make_PhysicalBlockDevice()
169+ uri = get_blockdevice_uri(device)
170+ response = self.client.get(
171+ uri, {'op': 'add_tag', 'tag': factory.make_name('tag')})
172+
173+ # Ensure the response status is FORBIDDEN
174+ self.assertEqual(
175+ httplib.FORBIDDEN, response.status_code, response.content)
176+
177 def test_add_tag_to_block_device(self):
178 self.become_admin()
179- device = factory.make_BlockDevice()
180+ device = factory.make_PhysicalBlockDevice()
181 tag_to_be_added = factory.make_name('tag')
182 uri = get_blockdevice_uri(device)
183 response = self.client.get(
184@@ -49,9 +106,31 @@
185 parsed_device = json.loads(response.content)
186 self.assertIn(tag_to_be_added, parsed_device['tags'])
187
188+ def test_add_tag_returns_404_when_system_id_doesnt_match(self):
189+ self.become_admin()
190+ device = factory.make_PhysicalBlockDevice()
191+ other_node = factory.make_Node()
192+ uri = get_blockdevice_uri(device, node=other_node)
193+ response = self.client.get(
194+ uri, {'op': 'add_tag', 'tag': factory.make_name('tag')})
195+
196+ # Ensure the response status is NOT_FOUND.
197+ self.assertEqual(
198+ httplib.NOT_FOUND, response.status_code, response.content)
199+
200+ def test_remove_tag_returns_403_for_non_admin(self):
201+ device = factory.make_PhysicalBlockDevice()
202+ uri = get_blockdevice_uri(device)
203+ response = self.client.get(
204+ uri, {'op': 'remove_tag', 'tag': factory.make_name('tag')})
205+
206+ # Ensure the response status is FORBIDDEN
207+ self.assertEqual(
208+ httplib.FORBIDDEN, response.status_code, response.content)
209+
210 def test_remove_tag_from_block_device(self):
211 self.become_admin()
212- device = factory.make_BlockDevice()
213+ device = factory.make_PhysicalBlockDevice()
214 tag_to_be_removed = device.tags[0]
215 uri = get_blockdevice_uri(device)
216 response = self.client.get(
217
218=== modified file 'src/maasserver/models/blockdevice.py'
219--- src/maasserver/models/blockdevice.py 2015-05-07 18:14:38 +0000
220+++ src/maasserver/models/blockdevice.py 2015-06-05 20:24:38 +0000
221@@ -84,6 +84,25 @@
222 tags = ArrayField(
223 dbtype="text", blank=True, null=False, default=[])
224
225+ @property
226+ def type(self):
227+ # Circular imports, since PhysicalBlockDevice and VirtualBlockDevice
228+ # extend from this calss.
229+ from maasserver.models.physicalblockdevice import PhysicalBlockDevice
230+ from maasserver.models.virtualblockdevice import VirtualBlockDevice
231+ try:
232+ self.physicalblockdevice
233+ return "physical"
234+ except PhysicalBlockDevice.DoesNotExist:
235+ try:
236+ self.virtualblockdevice
237+ return "virtual"
238+ except VirtualBlockDevice.DoesNotExist:
239+ pass
240+ raise ValueError(
241+ "BlockDevice is not a subclass of "
242+ "PhysicalBlockDevice or VirtualBlockDevice")
243+
244 def display_size(self, include_suffix=True):
245 return human_readable_bytes(self.size, include_suffix=include_suffix)
246
247
248=== modified file 'src/maasserver/models/tests/test_blockdevice.py'
249--- src/maasserver/models/tests/test_blockdevice.py 2015-05-07 18:14:38 +0000
250+++ src/maasserver/models/tests/test_blockdevice.py 2015-06-05 20:24:38 +0000
251@@ -17,6 +17,7 @@
252 from maasserver.models import BlockDevice
253 from maasserver.testing.factory import factory
254 from maasserver.testing.testcase import MAASServerTestCase
255+from testtools import ExpectedException
256 from testtools.matchers import Equals
257
258
259@@ -87,6 +88,19 @@
260 class TestBlockDevice(MAASServerTestCase):
261 """Tests for the `BlockDevice` model."""
262
263+ def test_type_physical(self):
264+ block_device = factory.make_PhysicalBlockDevice()
265+ self.assertEquals("physical", block_device.type)
266+
267+ def test_type_virtual(self):
268+ block_device = factory.make_VirtualBlockDevice()
269+ self.assertEquals("virtual", block_device.type)
270+
271+ def test_type_raise_ValueError(self):
272+ block_device = factory.make_BlockDevice()
273+ with ExpectedException(ValueError):
274+ block_device.type
275+
276 def test_display_size(self):
277 sizes = (
278 (45, '45.0 bytes'),
279
280=== modified file 'src/maasserver/urls_api.py'
281--- src/maasserver/urls_api.py 2015-06-01 13:00:47 +0000
282+++ src/maasserver/urls_api.py 2015-06-05 20:24:38 +0000
283@@ -20,7 +20,10 @@
284 )
285 from maasserver.api.account import AccountHandler
286 from maasserver.api.auth import api_auth
287-from maasserver.api.blockdevices import BlockDeviceHandler
288+from maasserver.api.blockdevices import (
289+ BlockDeviceHandler,
290+ BlockDevicesHandler,
291+)
292 from maasserver.api.boot_images import BootImagesHandler
293 from maasserver.api.boot_resources import (
294 BootResourceFileUploadHandler,
295@@ -133,6 +136,10 @@
296 node_mac_handler = RestrictedResource(NodeMacHandler, authentication=api_auth)
297 node_macs_handler = RestrictedResource(
298 NodeMacsHandler, authentication=api_auth)
299+blockdevices_handler = RestrictedResource(
300+ BlockDevicesHandler, authentication=api_auth)
301+blockdevice_handler = RestrictedResource(
302+ BlockDeviceHandler, authentication=api_auth)
303 nodegroup_handler = RestrictedResource(
304 NodeGroupHandler, authentication=api_auth)
305 nodegroups_handler = RestrictedResource(
306@@ -184,8 +191,6 @@
307 LicenseKeyHandler, authentication=api_auth)
308 license_keys_handler = AdminRestrictedResource(
309 LicenseKeysHandler, authentication=api_auth)
310-blockdevice_handler = AdminRestrictedResource(
311- BlockDeviceHandler, authentication=api_auth)
312
313
314 # API URLs accessible to anonymous users.
315@@ -207,7 +212,10 @@
316 url(
317 r'^nodes/(?P<system_id>[^/]+)/macs/$', node_macs_handler,
318 name='node_macs_handler'),
319-
320+ url(r'^nodes/(?P<system_id>[^/]+)/blockdevices/$',
321+ blockdevices_handler, name='blockdevices_handler'),
322+ url(r'^nodes/(?P<system_id>[^/]+)/blockdevices/(?P<device_id>[^/]+)/$',
323+ blockdevice_handler, name='blockdevice_handler'),
324 url(
325 r'^nodes/(?P<system_id>[^/]+)/$', node_handler,
326 name='node_handler'),
327@@ -308,8 +316,6 @@
328 'selections/(?P<id>[^/]+)/$',
329 boot_source_selection_backward_handler,
330 name='boot_source_selection_backward_handler'),
331- url(r'^blockdevice/(?P<device_id>[^/]+)/$',
332- blockdevice_handler, name='blockdevice_handler'),
333 )
334
335