Merge ~ack/maas:1848043-websocket-storage-vol-numa into maas:master
- Git
- lp:~ack/maas
- 1848043-websocket-storage-vol-numa
- Merge into master
Proposed by
Alberto Donato
Status: | Merged |
---|---|
Approved by: | Alberto Donato |
Approved revision: | e216dc92901f7a0cb768ba0c96c0a99e9d20a041 |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~ack/maas:1848043-websocket-storage-vol-numa |
Merge into: | maas:master |
Diff against target: |
449 lines (+277/-6) 9 files modified
src/maasserver/models/cacheset.py (+23/-0) src/maasserver/models/filesystem.py (+16/-0) src/maasserver/models/filesystemgroup.py (+17/-0) src/maasserver/models/tests/test_cacheset.py (+34/-0) src/maasserver/models/tests/test_filesystem.py (+41/-1) src/maasserver/models/tests/test_filesystemgroup.py (+85/-0) src/maasserver/models/virtualblockdevice.py (+4/-5) src/maasserver/websockets/handlers/node.py (+2/-0) src/maasserver/websockets/handlers/tests/test_machine.py (+55/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Newell Jensen (community) | Approve | ||
MAAS Lander | Approve | ||
Review via email: mp+374429@code.launchpad.net |
Commit message
LP: #1848043 - Add numa node indexes to websocket dehydration of CacheSets and FilesystemGroups
Description of the change
To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b 1848043-
STATUS: FAILED
LOG: http://
COMMIT: 4a507375d22fcb9
review:
Needs Fixing
Revision history for this message
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b 1848043-
STATUS: SUCCESS
COMMIT: e216dc92901f7a0
review:
Approve
There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/src/maasserver/models/cacheset.py b/src/maasserver/models/cacheset.py |
2 | index 97996e2..5246454 100644 |
3 | --- a/src/maasserver/models/cacheset.py |
4 | +++ b/src/maasserver/models/cacheset.py |
5 | @@ -5,6 +5,7 @@ |
6 | |
7 | __all__ = ["CacheSet"] |
8 | |
9 | +from itertools import chain |
10 | |
11 | from django.core.exceptions import PermissionDenied |
12 | from django.db.models import Manager, Q |
13 | @@ -13,6 +14,7 @@ from django.http import Http404 |
14 | from maasserver import DefaultMeta |
15 | from maasserver.enum import FILESYSTEM_TYPE |
16 | from maasserver.models.cleansave import CleanSave |
17 | +from maasserver.models.numa import NUMANode |
18 | from maasserver.models.timestampedmodel import TimestampedModel |
19 | |
20 | |
21 | @@ -188,3 +190,24 @@ class CacheSet(CleanSave, TimestampedModel): |
22 | return None |
23 | else: |
24 | return filesystem.get_parent() |
25 | + |
26 | + def get_numa_node_indexes(self): |
27 | + """Return NUMA node indexes for physical devices making up the cacheset.""" |
28 | + numa_node_indexes = set( |
29 | + chain( |
30 | + *( |
31 | + fsgroup.get_numa_node_indexes() |
32 | + for fsgroup in self.filesystemgroup_set.all() |
33 | + ) |
34 | + ) |
35 | + ) |
36 | + filesystem = self.get_filesystem() |
37 | + if filesystem: |
38 | + block_devices = filesystem.get_physical_block_devices() |
39 | + numa_ids = set(device.numa_node_id for device in block_devices) |
40 | + numa_node_indexes.update( |
41 | + NUMANode.objects.filter(id__in=numa_ids) |
42 | + .values_list("index", flat=True) |
43 | + .order_by("index") |
44 | + ) |
45 | + return sorted(numa_node_indexes) |
46 | diff --git a/src/maasserver/models/filesystem.py b/src/maasserver/models/filesystem.py |
47 | index cc591c9..a34f4d7 100644 |
48 | --- a/src/maasserver/models/filesystem.py |
49 | +++ b/src/maasserver/models/filesystem.py |
50 | @@ -29,6 +29,7 @@ from maasserver.models.cacheset import CacheSet |
51 | from maasserver.models.cleansave import CleanSave |
52 | from maasserver.models.filesystemgroup import FilesystemGroup |
53 | from maasserver.models.partition import Partition |
54 | +from maasserver.models.physicalblockdevice import PhysicalBlockDevice |
55 | from maasserver.models.timestampedmodel import TimestampedModel |
56 | |
57 | |
58 | @@ -170,6 +171,21 @@ class Filesystem(CleanSave, TimestampedModel): |
59 | # XXX: Explode instead? |
60 | return None |
61 | |
62 | + def get_physical_block_devices(self): |
63 | + """Return PhysicalBlockDevices backing the filesystem.""" |
64 | + from maasserver.models.virtualblockdevice import VirtualBlockDevice |
65 | + |
66 | + devices = [] |
67 | + parent = self.get_parent() |
68 | + if isinstance(parent, PhysicalBlockDevice): |
69 | + devices.append(parent) |
70 | + elif isinstance(parent, VirtualBlockDevice): |
71 | + for grandparent in parent.get_parents(): |
72 | + device = grandparent.actual_instance |
73 | + if isinstance(device, PhysicalBlockDevice): |
74 | + devices.append(device) |
75 | + return devices |
76 | + |
77 | def get_size(self): |
78 | """Size of filesystem.""" |
79 | if self.partition is not None: |
80 | diff --git a/src/maasserver/models/filesystemgroup.py b/src/maasserver/models/filesystemgroup.py |
81 | index 319e3a0..c3b9bb3 100644 |
82 | --- a/src/maasserver/models/filesystemgroup.py |
83 | +++ b/src/maasserver/models/filesystemgroup.py |
84 | @@ -6,6 +6,7 @@ a virtual block device. E.g. LVM Volume Group.""" |
85 | |
86 | __all__ = ["Bcache", "FilesystemGroup", "RAID", "VMFS", "VolumeGroup"] |
87 | |
88 | +from itertools import chain |
89 | from uuid import uuid4 |
90 | |
91 | from django.core.exceptions import PermissionDenied, ValidationError |
92 | @@ -24,6 +25,7 @@ from maasserver.enum import ( |
93 | ) |
94 | from maasserver.models.cacheset import CacheSet |
95 | from maasserver.models.cleansave import CleanSave |
96 | +from maasserver.models.numa import NUMANode |
97 | from maasserver.models.timestampedmodel import TimestampedModel |
98 | from maasserver.utils.orm import get_one |
99 | |
100 | @@ -392,6 +394,21 @@ class FilesystemGroup(CleanSave, TimestampedModel): |
101 | # that cache will be used. |
102 | return get_one(self.virtual_devices.all()) |
103 | |
104 | + def get_numa_node_indexes(self): |
105 | + """Return NUMA node indexes for physical devices making the volume group.""" |
106 | + block_devices = chain( |
107 | + *( |
108 | + filesystem.get_physical_block_devices() |
109 | + for filesystem in self.filesystems.all() |
110 | + ) |
111 | + ) |
112 | + numa_ids = set(device.numa_node_id for device in block_devices) |
113 | + return list( |
114 | + NUMANode.objects.filter(id__in=numa_ids) |
115 | + .values_list("index", flat=True) |
116 | + .order_by("index") |
117 | + ) |
118 | + |
119 | def get_node(self): |
120 | """`Node` this filesystem group belongs to.""" |
121 | if self.filesystems.count() == 0: |
122 | diff --git a/src/maasserver/models/tests/test_cacheset.py b/src/maasserver/models/tests/test_cacheset.py |
123 | index 6009a11..6341ea5 100644 |
124 | --- a/src/maasserver/models/tests/test_cacheset.py |
125 | +++ b/src/maasserver/models/tests/test_cacheset.py |
126 | @@ -10,6 +10,7 @@ import random |
127 | from django.core.exceptions import PermissionDenied |
128 | from django.http import Http404 |
129 | |
130 | +from maasserver.enum import FILESYSTEM_GROUP_TYPE, FILESYSTEM_TYPE |
131 | from maasserver.models import CacheSet |
132 | from maasserver.permissions import NodePermission |
133 | from maasserver.testing.factory import factory |
134 | @@ -283,3 +284,36 @@ class TestCacheSet(MAASServerTestCase): |
135 | block_device = factory.make_PhysicalBlockDevice() |
136 | cache_set = factory.make_CacheSet(block_device=block_device) |
137 | self.assertEqual(block_device, cache_set.get_device()) |
138 | + |
139 | + def test_get_numa_nodes_indexes_only_own_device(self): |
140 | + block_device = factory.make_PhysicalBlockDevice() |
141 | + cache_set = factory.make_CacheSet(block_device=block_device) |
142 | + self.assertEqual(cache_set.get_numa_node_indexes(), [0]) |
143 | + |
144 | + def test_get_numa_nodes_indexes_many_devices(self): |
145 | + node = factory.make_Node() |
146 | + numa_nodes = [ |
147 | + node.default_numanode, |
148 | + factory.make_NUMANode(node=node), |
149 | + factory.make_NUMANode(node=node), |
150 | + ] |
151 | + block_devices = [ |
152 | + factory.make_PhysicalBlockDevice(numa_node=numa_node) |
153 | + for numa_node in numa_nodes |
154 | + ] |
155 | + filesystems = [ |
156 | + factory.make_Filesystem( |
157 | + fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device |
158 | + ) |
159 | + for block_device in block_devices |
160 | + ] |
161 | + fsgroup = factory.make_FilesystemGroup( |
162 | + node=node, |
163 | + filesystems=filesystems, |
164 | + group_type=FILESYSTEM_GROUP_TYPE.LVM_VG, |
165 | + ) |
166 | + virtual_block_device = factory.make_VirtualBlockDevice( |
167 | + filesystem_group=fsgroup |
168 | + ) |
169 | + cache_set = factory.make_CacheSet(block_device=virtual_block_device) |
170 | + self.assertEqual(cache_set.get_numa_node_indexes(), [0, 1, 2]) |
171 | diff --git a/src/maasserver/models/tests/test_filesystem.py b/src/maasserver/models/tests/test_filesystem.py |
172 | index e97865b..b2da0b5 100644 |
173 | --- a/src/maasserver/models/tests/test_filesystem.py |
174 | +++ b/src/maasserver/models/tests/test_filesystem.py |
175 | @@ -15,7 +15,11 @@ from testscenarios import multiply_scenarios |
176 | from testtools import ExpectedException |
177 | from testtools.matchers import Equals, Is, IsInstance, MatchesStructure |
178 | |
179 | -from maasserver.enum import FILESYSTEM_FORMAT_TYPE_CHOICES_DICT |
180 | +from maasserver.enum import ( |
181 | + FILESYSTEM_FORMAT_TYPE_CHOICES_DICT, |
182 | + FILESYSTEM_GROUP_TYPE, |
183 | + FILESYSTEM_TYPE, |
184 | +) |
185 | from maasserver.models.filesystem import Filesystem |
186 | from maasserver.testing.factory import factory |
187 | from maasserver.testing.testcase import MAASServerTestCase |
188 | @@ -71,6 +75,42 @@ class TestFilesystem(MAASServerTestCase): |
189 | fs = Filesystem() |
190 | self.assertIsNone(fs.get_node()) |
191 | |
192 | + def test_get_physical_block_devices_single(self): |
193 | + node = factory.make_Node() |
194 | + block_device = factory.make_PhysicalBlockDevice(node=node) |
195 | + filesystem = factory.make_Filesystem( |
196 | + fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device |
197 | + ) |
198 | + self.assertEqual( |
199 | + filesystem.get_physical_block_devices(), [block_device] |
200 | + ) |
201 | + |
202 | + def test_get_physical_block_devices_multiple(self): |
203 | + node = factory.make_Node() |
204 | + block_devices = [ |
205 | + factory.make_PhysicalBlockDevice(node=node) for _ in range(3) |
206 | + ] |
207 | + filesystems = [ |
208 | + factory.make_Filesystem( |
209 | + fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device |
210 | + ) |
211 | + for block_device in block_devices |
212 | + ] |
213 | + fsgroup = factory.make_FilesystemGroup( |
214 | + node=node, |
215 | + filesystems=filesystems, |
216 | + group_type=FILESYSTEM_GROUP_TYPE.LVM_VG, |
217 | + ) |
218 | + virtual_block_device = factory.make_VirtualBlockDevice( |
219 | + filesystem_group=fsgroup |
220 | + ) |
221 | + filesystem = factory.make_Filesystem( |
222 | + fstype=FILESYSTEM_TYPE.LVM_PV, block_device=virtual_block_device |
223 | + ) |
224 | + self.assertEqual( |
225 | + filesystem.get_physical_block_devices(), block_devices |
226 | + ) |
227 | + |
228 | def test_get_size_returns_partition_size(self): |
229 | partition = factory.make_Partition() |
230 | fs = factory.make_Filesystem(partition=partition) |
231 | diff --git a/src/maasserver/models/tests/test_filesystemgroup.py b/src/maasserver/models/tests/test_filesystemgroup.py |
232 | index 4fe263a..a84f46b 100644 |
233 | --- a/src/maasserver/models/tests/test_filesystemgroup.py |
234 | +++ b/src/maasserver/models/tests/test_filesystemgroup.py |
235 | @@ -684,6 +684,91 @@ class TestFilesystemGroup(MAASServerTestCase): |
236 | fsgroup.virtual_device, |
237 | ) |
238 | |
239 | + def test_get_numa_node_indexes_all_same(self): |
240 | + fsgroup = factory.make_FilesystemGroup( |
241 | + group_type=factory.pick_enum( |
242 | + FILESYSTEM_GROUP_TYPE, but_not=FILESYSTEM_GROUP_TYPE.VMFS6 |
243 | + ) |
244 | + ) |
245 | + self.assertEqual(fsgroup.get_numa_node_indexes(), [0]) |
246 | + |
247 | + def test_get_numa_node_indexes_multiple(self): |
248 | + node = factory.make_Node() |
249 | + numa_nodes = [ |
250 | + node.default_numanode, |
251 | + factory.make_NUMANode(node=node), |
252 | + factory.make_NUMANode(node=node), |
253 | + ] |
254 | + block_devices = [ |
255 | + factory.make_PhysicalBlockDevice(numa_node=numa_node) |
256 | + for numa_node in numa_nodes |
257 | + ] |
258 | + filesystems = [ |
259 | + factory.make_Filesystem( |
260 | + fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device |
261 | + ) |
262 | + for block_device in block_devices |
263 | + ] |
264 | + fsgroup = factory.make_FilesystemGroup( |
265 | + node=node, |
266 | + filesystems=filesystems, |
267 | + group_type=FILESYSTEM_GROUP_TYPE.LVM_VG, |
268 | + ) |
269 | + self.assertEqual(fsgroup.get_numa_node_indexes(), [0, 1, 2]) |
270 | + |
271 | + def test_get_numa_node_indexes_nested(self): |
272 | + node = factory.make_Node() |
273 | + numa_nodes = [ |
274 | + node.default_numanode, |
275 | + factory.make_NUMANode(node=node), |
276 | + factory.make_NUMANode(node=node), |
277 | + factory.make_NUMANode(node=node), |
278 | + factory.make_NUMANode(node=node), |
279 | + ] |
280 | + # 2 physical disks have filesystems on them directly |
281 | + filesystems = [ |
282 | + factory.make_Filesystem( |
283 | + fstype=FILESYSTEM_TYPE.LVM_PV, |
284 | + block_device=factory.make_PhysicalBlockDevice( |
285 | + numa_node=numa_node |
286 | + ), |
287 | + ) |
288 | + for numa_node in numa_nodes[:2] |
289 | + ] |
290 | + |
291 | + # the 3 remaining disks are part of another filesystem group which gets |
292 | + # added to the first |
293 | + nested_filesystems = [ |
294 | + factory.make_Filesystem( |
295 | + fstype=FILESYSTEM_TYPE.LVM_PV, |
296 | + block_device=factory.make_PhysicalBlockDevice( |
297 | + numa_node=numa_node |
298 | + ), |
299 | + ) |
300 | + for numa_node in numa_nodes[2:] |
301 | + ] |
302 | + nested_group = factory.make_FilesystemGroup( |
303 | + node=node, |
304 | + filesystems=nested_filesystems, |
305 | + group_type=FILESYSTEM_GROUP_TYPE.LVM_VG, |
306 | + ) |
307 | + virtual_block_device = factory.make_VirtualBlockDevice( |
308 | + filesystem_group=nested_group |
309 | + ) |
310 | + filesystems.append( |
311 | + factory.make_Filesystem( |
312 | + fstype=FILESYSTEM_TYPE.LVM_PV, |
313 | + block_device=virtual_block_device, |
314 | + ) |
315 | + ) |
316 | + |
317 | + fsgroup = factory.make_FilesystemGroup( |
318 | + node=node, |
319 | + filesystems=filesystems, |
320 | + group_type=FILESYSTEM_GROUP_TYPE.LVM_VG, |
321 | + ) |
322 | + self.assertEqual(fsgroup.get_numa_node_indexes(), [0, 1, 2, 3, 4]) |
323 | + |
324 | def test_get_node_returns_first_filesystem_node(self): |
325 | fsgroup = factory.make_FilesystemGroup() |
326 | self.assertEqual( |
327 | diff --git a/src/maasserver/models/virtualblockdevice.py b/src/maasserver/models/virtualblockdevice.py |
328 | index 9b9bb65..b59be68 100644 |
329 | --- a/src/maasserver/models/virtualblockdevice.py |
330 | +++ b/src/maasserver/models/virtualblockdevice.py |
331 | @@ -153,13 +153,12 @@ class VirtualBlockDevice(BlockDevice): |
332 | return False |
333 | else: |
334 | return False |
335 | - for virtual_device in fs_group.virtual_devices.all(): |
336 | - if virtual_device.id == self.id: |
337 | - return True |
338 | - return False |
339 | + |
340 | + # whether the device is part of the filesystem group |
341 | + return fs_group.virtual_devices.filter(id=self.id).exists() |
342 | |
343 | parents = [] |
344 | - # We need to check all of the nodes block devices incase |
345 | + # We need to check all of the nodes block devices in case |
346 | # we have nested virtual block devices. |
347 | for block_device in self.node.blockdevice_set.all(): |
348 | if block_device.id == self.id: |
349 | diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py |
350 | index 7b47bca..e214716 100644 |
351 | --- a/src/maasserver/websockets/handlers/node.py |
352 | +++ b/src/maasserver/websockets/handlers/node.py |
353 | @@ -553,6 +553,7 @@ class NodeHandler(TimestampedModelHandler): |
354 | "used_for": "volume group", |
355 | "filesystem": None, |
356 | "partitions": None, |
357 | + "numa_nodes": volume_group.get_numa_node_indexes(), |
358 | } |
359 | |
360 | def dehydrate_cache_set(self, cache_set): |
361 | @@ -582,6 +583,7 @@ class NodeHandler(TimestampedModelHandler): |
362 | "used_for": ", ".join(bcache_devices), |
363 | "filesystem": None, |
364 | "partitions": None, |
365 | + "numa_nodes": cache_set.get_numa_node_indexes(), |
366 | } |
367 | |
368 | def dehydrate_partitions(self, partition_table): |
369 | diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py |
370 | index 7144d1e..010e8bc 100644 |
371 | --- a/src/maasserver/websockets/handlers/tests/test_machine.py |
372 | +++ b/src/maasserver/websockets/handlers/tests/test_machine.py |
373 | @@ -989,10 +989,40 @@ class TestMachineHandler(MAASServerTestCase): |
374 | "used_for": "volume group", |
375 | "filesystem": None, |
376 | "partitions": None, |
377 | + "numa_nodes": [0], |
378 | }, |
379 | handler.dehydrate_volume_group(volume_group), |
380 | ) |
381 | |
382 | + def test_dehydrate_volume_group_multiple_numa_nodes(self): |
383 | + owner = factory.make_User() |
384 | + node = factory.make_Node(owner=owner) |
385 | + numa_nodes = [ |
386 | + node.default_numanode, |
387 | + factory.make_NUMANode(node=node), |
388 | + factory.make_NUMANode(node=node), |
389 | + ] |
390 | + block_devices = [ |
391 | + factory.make_PhysicalBlockDevice(numa_node=numa_node) |
392 | + for numa_node in numa_nodes |
393 | + ] |
394 | + filesystems = [ |
395 | + factory.make_Filesystem( |
396 | + fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device |
397 | + ) |
398 | + for block_device in block_devices |
399 | + ] |
400 | + volume_group = factory.make_FilesystemGroup( |
401 | + node=node, |
402 | + filesystems=filesystems, |
403 | + group_type=FILESYSTEM_GROUP_TYPE.LVM_VG, |
404 | + ) |
405 | + handler = MachineHandler(owner, {}, None) |
406 | + self.assertEqual( |
407 | + handler.dehydrate_volume_group(volume_group)["numa_nodes"], |
408 | + [0, 1, 2], |
409 | + ) |
410 | + |
411 | def test_dehydrate_cache_set(self): |
412 | owner = factory.make_User() |
413 | node = factory.make_Node(owner=owner) |
414 | @@ -1041,10 +1071,35 @@ class TestMachineHandler(MAASServerTestCase): |
415 | ), |
416 | "filesystem": None, |
417 | "partitions": None, |
418 | + "numa_nodes": [0], |
419 | }, |
420 | handler.dehydrate_cache_set(cache_set), |
421 | ) |
422 | |
423 | + def test_dehydrate_cache_set_multiple_numa_nodes(self): |
424 | + owner = factory.make_User() |
425 | + node = factory.make_Node(owner=owner) |
426 | + numa_nodes = [ |
427 | + node.default_numanode, |
428 | + factory.make_NUMANode(node=node), |
429 | + factory.make_NUMANode(node=node), |
430 | + ] |
431 | + cache_set = factory.make_CacheSet(node=node) |
432 | + for numa_node in numa_nodes: |
433 | + backing = factory.make_PhysicalBlockDevice(numa_node=numa_node) |
434 | + fs = factory.make_Filesystem( |
435 | + block_device=backing, fstype=FILESYSTEM_TYPE.BCACHE_BACKING |
436 | + ) |
437 | + factory.make_FilesystemGroup( |
438 | + group_type=FILESYSTEM_GROUP_TYPE.BCACHE, |
439 | + filesystems=[fs], |
440 | + cache_set=cache_set, |
441 | + ) |
442 | + handler = MachineHandler(owner, {}, None) |
443 | + self.assertEqual( |
444 | + handler.dehydrate_cache_set(cache_set)["numa_nodes"], [0, 1, 2] |
445 | + ) |
446 | + |
447 | def test_dehydrate_partitions_returns_None(self): |
448 | owner = factory.make_User() |
449 | handler = MachineHandler(owner, {}, None) |
UNIT TESTS websocket- storage- vol-numa lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas
-b 1848043-
STATUS: FAILED maas-ci- jenkins. internal: 8080/job/ maas/job/ branch- tester/ 6832/console fd2bd18c9ea26c1 32b9e7858c
LOG: http://
COMMIT: 81d12ff2826704b