Merge ~ack/maas:1848043-websocket-storage-vol-numa into maas: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)
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

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b 1848043-websocket-storage-vol-numa lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6832/console
COMMIT: 81d12ff2826704bfd2bd18c9ea26c132b9e7858c

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

UNIT TESTS
-b 1848043-websocket-storage-vol-numa lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6835/console
COMMIT: 4a507375d22fcb9789dc7b7aaa12f2463d36a929

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

UNIT TESTS
-b 1848043-websocket-storage-vol-numa lp:~ack/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e216dc92901f7a0cb768ba0c96c0a99e9d20a041

review: Approve
Revision history for this message
Newell Jensen (newell-jensen) wrote :

LGTM

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
1diff --git a/src/maasserver/models/cacheset.py b/src/maasserver/models/cacheset.py
2index 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)
46diff --git a/src/maasserver/models/filesystem.py b/src/maasserver/models/filesystem.py
47index 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:
80diff --git a/src/maasserver/models/filesystemgroup.py b/src/maasserver/models/filesystemgroup.py
81index 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:
122diff --git a/src/maasserver/models/tests/test_cacheset.py b/src/maasserver/models/tests/test_cacheset.py
123index 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])
171diff --git a/src/maasserver/models/tests/test_filesystem.py b/src/maasserver/models/tests/test_filesystem.py
172index 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)
231diff --git a/src/maasserver/models/tests/test_filesystemgroup.py b/src/maasserver/models/tests/test_filesystemgroup.py
232index 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(
327diff --git a/src/maasserver/models/virtualblockdevice.py b/src/maasserver/models/virtualblockdevice.py
328index 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:
349diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
350index 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):
369diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
370index 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)

Subscribers

People subscribed via source and target branches