Merge ~ack/maas:1848043-websocket-storage-vol-numa into maas:master

[Alberto Donato]

LP: #1848043 - Add numa node indexes to websocket dehydration of CacheSets and FilesystemGroups

[MAAS Lander (maas-lander)]

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

[MAAS Lander (maas-lander)]

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

[MAAS Lander (maas-lander)]

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

[Newell Jensen (newell-jensen)]

LGTM

review: Approve

diff --git a/src/maasserver/models/cacheset.py b/src/maasserver/models/cacheset.py
index 97996e2..5246454 100644
--- a/src/maasserver/models/cacheset.py
+++ b/src/maasserver/models/cacheset.py
@@ -5,6 +5,7 @@
 
 __all__ = ["CacheSet"]
 
+from itertools import chain
 
 from django.core.exceptions import PermissionDenied
 from django.db.models import Manager, Q
@@ -13,6 +14,7 @@ from django.http import Http404
 from maasserver import DefaultMeta
 from maasserver.enum import FILESYSTEM_TYPE
 from maasserver.models.cleansave import CleanSave
+from maasserver.models.numa import NUMANode
 from maasserver.models.timestampedmodel import TimestampedModel
 
 
@@ -188,3 +190,24 @@ class CacheSet(CleanSave, TimestampedModel):
             return None
         else:
             return filesystem.get_parent()
+
+    def get_numa_node_indexes(self):
+        """Return NUMA node indexes for physical devices making up the cacheset."""
+        numa_node_indexes = set(
+            chain(
+                *(
+                    fsgroup.get_numa_node_indexes()
+                    for fsgroup in self.filesystemgroup_set.all()
+                )
+            )
+        )
+        filesystem = self.get_filesystem()
+        if filesystem:
+            block_devices = filesystem.get_physical_block_devices()
+            numa_ids = set(device.numa_node_id for device in block_devices)
+            numa_node_indexes.update(
+                NUMANode.objects.filter(id__in=numa_ids)
+                .values_list("index", flat=True)
+                .order_by("index")
+            )
+        return sorted(numa_node_indexes)
diff --git a/src/maasserver/models/filesystem.py b/src/maasserver/models/filesystem.py
index cc591c9..a34f4d7 100644
--- a/src/maasserver/models/filesystem.py
+++ b/src/maasserver/models/filesystem.py
@@ -29,6 +29,7 @@ from maasserver.models.cacheset import CacheSet
 from maasserver.models.cleansave import CleanSave
 from maasserver.models.filesystemgroup import FilesystemGroup
 from maasserver.models.partition import Partition
+from maasserver.models.physicalblockdevice import PhysicalBlockDevice
 from maasserver.models.timestampedmodel import TimestampedModel
 
 
@@ -170,6 +171,21 @@ class Filesystem(CleanSave, TimestampedModel):
             # XXX: Explode instead?
             return None
 
+    def get_physical_block_devices(self):
+        """Return PhysicalBlockDevices backing the filesystem."""
+        from maasserver.models.virtualblockdevice import VirtualBlockDevice
+
+        devices = []
+        parent = self.get_parent()
+        if isinstance(parent, PhysicalBlockDevice):
+            devices.append(parent)
+        elif isinstance(parent, VirtualBlockDevice):
+            for grandparent in parent.get_parents():
+                device = grandparent.actual_instance
+                if isinstance(device, PhysicalBlockDevice):
+                    devices.append(device)
+        return devices
+
     def get_size(self):
         """Size of filesystem."""
         if self.partition is not None:
diff --git a/src/maasserver/models/filesystemgroup.py b/src/maasserver/models/filesystemgroup.py
index 319e3a0..c3b9bb3 100644
--- a/src/maasserver/models/filesystemgroup.py
+++ b/src/maasserver/models/filesystemgroup.py
@@ -6,6 +6,7 @@ a virtual block device. E.g. LVM Volume Group."""
 
 __all__ = ["Bcache", "FilesystemGroup", "RAID", "VMFS", "VolumeGroup"]
 
+from itertools import chain
 from uuid import uuid4
 
 from django.core.exceptions import PermissionDenied, ValidationError
@@ -24,6 +25,7 @@ from maasserver.enum import (
 )
 from maasserver.models.cacheset import CacheSet
 from maasserver.models.cleansave import CleanSave
+from maasserver.models.numa import NUMANode
 from maasserver.models.timestampedmodel import TimestampedModel
 from maasserver.utils.orm import get_one
 
@@ -392,6 +394,21 @@ class FilesystemGroup(CleanSave, TimestampedModel):
             # that cache will be used.
             return get_one(self.virtual_devices.all())
 
+    def get_numa_node_indexes(self):
+        """Return NUMA node indexes for physical devices making the volume group."""
+        block_devices = chain(
+            *(
+                filesystem.get_physical_block_devices()
+                for filesystem in self.filesystems.all()
+            )
+        )
+        numa_ids = set(device.numa_node_id for device in block_devices)
+        return list(
+            NUMANode.objects.filter(id__in=numa_ids)
+            .values_list("index", flat=True)
+            .order_by("index")
+        )
+
     def get_node(self):
         """`Node` this filesystem group belongs to."""
         if self.filesystems.count() == 0:
diff --git a/src/maasserver/models/tests/test_cacheset.py b/src/maasserver/models/tests/test_cacheset.py
index 6009a11..6341ea5 100644
--- a/src/maasserver/models/tests/test_cacheset.py
+++ b/src/maasserver/models/tests/test_cacheset.py
@@ -10,6 +10,7 @@ import random
 from django.core.exceptions import PermissionDenied
 from django.http import Http404
 
+from maasserver.enum import FILESYSTEM_GROUP_TYPE, FILESYSTEM_TYPE
 from maasserver.models import CacheSet
 from maasserver.permissions import NodePermission
 from maasserver.testing.factory import factory
@@ -283,3 +284,36 @@ class TestCacheSet(MAASServerTestCase):
         block_device = factory.make_PhysicalBlockDevice()
         cache_set = factory.make_CacheSet(block_device=block_device)
         self.assertEqual(block_device, cache_set.get_device())
+
+    def test_get_numa_nodes_indexes_only_own_device(self):
+        block_device = factory.make_PhysicalBlockDevice()
+        cache_set = factory.make_CacheSet(block_device=block_device)
+        self.assertEqual(cache_set.get_numa_node_indexes(), [0])
+
+    def test_get_numa_nodes_indexes_many_devices(self):
+        node = factory.make_Node()
+        numa_nodes = [
+            node.default_numanode,
+            factory.make_NUMANode(node=node),
+            factory.make_NUMANode(node=node),
+        ]
+        block_devices = [
+            factory.make_PhysicalBlockDevice(numa_node=numa_node)
+            for numa_node in numa_nodes
+        ]
+        filesystems = [
+            factory.make_Filesystem(
+                fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device
+            )
+            for block_device in block_devices
+        ]
+        fsgroup = factory.make_FilesystemGroup(
+            node=node,
+            filesystems=filesystems,
+            group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
+        )
+        virtual_block_device = factory.make_VirtualBlockDevice(
+            filesystem_group=fsgroup
+        )
+        cache_set = factory.make_CacheSet(block_device=virtual_block_device)
+        self.assertEqual(cache_set.get_numa_node_indexes(), [0, 1, 2])
diff --git a/src/maasserver/models/tests/test_filesystem.py b/src/maasserver/models/tests/test_filesystem.py
index e97865b..b2da0b5 100644
--- a/src/maasserver/models/tests/test_filesystem.py
+++ b/src/maasserver/models/tests/test_filesystem.py
@@ -15,7 +15,11 @@ from testscenarios import multiply_scenarios
 from testtools import ExpectedException
 from testtools.matchers import Equals, Is, IsInstance, MatchesStructure
 
-from maasserver.enum import FILESYSTEM_FORMAT_TYPE_CHOICES_DICT
+from maasserver.enum import (
+    FILESYSTEM_FORMAT_TYPE_CHOICES_DICT,
+    FILESYSTEM_GROUP_TYPE,
+    FILESYSTEM_TYPE,
+)
 from maasserver.models.filesystem import Filesystem
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -71,6 +75,42 @@ class TestFilesystem(MAASServerTestCase):
         fs = Filesystem()
         self.assertIsNone(fs.get_node())
 
+    def test_get_physical_block_devices_single(self):
+        node = factory.make_Node()
+        block_device = factory.make_PhysicalBlockDevice(node=node)
+        filesystem = factory.make_Filesystem(
+            fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device
+        )
+        self.assertEqual(
+            filesystem.get_physical_block_devices(), [block_device]
+        )
+
+    def test_get_physical_block_devices_multiple(self):
+        node = factory.make_Node()
+        block_devices = [
+            factory.make_PhysicalBlockDevice(node=node) for _ in range(3)
+        ]
+        filesystems = [
+            factory.make_Filesystem(
+                fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device
+            )
+            for block_device in block_devices
+        ]
+        fsgroup = factory.make_FilesystemGroup(
+            node=node,
+            filesystems=filesystems,
+            group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
+        )
+        virtual_block_device = factory.make_VirtualBlockDevice(
+            filesystem_group=fsgroup
+        )
+        filesystem = factory.make_Filesystem(
+            fstype=FILESYSTEM_TYPE.LVM_PV, block_device=virtual_block_device
+        )
+        self.assertEqual(
+            filesystem.get_physical_block_devices(), block_devices
+        )
+
     def test_get_size_returns_partition_size(self):
         partition = factory.make_Partition()
         fs = factory.make_Filesystem(partition=partition)
diff --git a/src/maasserver/models/tests/test_filesystemgroup.py b/src/maasserver/models/tests/test_filesystemgroup.py
index 4fe263a..a84f46b 100644
--- a/src/maasserver/models/tests/test_filesystemgroup.py
+++ b/src/maasserver/models/tests/test_filesystemgroup.py
@@ -684,6 +684,91 @@ class TestFilesystemGroup(MAASServerTestCase):
             fsgroup.virtual_device,
         )
 
+    def test_get_numa_node_indexes_all_same(self):
+        fsgroup = factory.make_FilesystemGroup(
+            group_type=factory.pick_enum(
+                FILESYSTEM_GROUP_TYPE, but_not=FILESYSTEM_GROUP_TYPE.VMFS6
+            )
+        )
+        self.assertEqual(fsgroup.get_numa_node_indexes(), [0])
+
+    def test_get_numa_node_indexes_multiple(self):
+        node = factory.make_Node()
+        numa_nodes = [
+            node.default_numanode,
+            factory.make_NUMANode(node=node),
+            factory.make_NUMANode(node=node),
+        ]
+        block_devices = [
+            factory.make_PhysicalBlockDevice(numa_node=numa_node)
+            for numa_node in numa_nodes
+        ]
+        filesystems = [
+            factory.make_Filesystem(
+                fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device
+            )
+            for block_device in block_devices
+        ]
+        fsgroup = factory.make_FilesystemGroup(
+            node=node,
+            filesystems=filesystems,
+            group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
+        )
+        self.assertEqual(fsgroup.get_numa_node_indexes(), [0, 1, 2])
+
+    def test_get_numa_node_indexes_nested(self):
+        node = factory.make_Node()
+        numa_nodes = [
+            node.default_numanode,
+            factory.make_NUMANode(node=node),
+            factory.make_NUMANode(node=node),
+            factory.make_NUMANode(node=node),
+            factory.make_NUMANode(node=node),
+        ]
+        # 2 physical disks have filesystems on them directly
+        filesystems = [
+            factory.make_Filesystem(
+                fstype=FILESYSTEM_TYPE.LVM_PV,
+                block_device=factory.make_PhysicalBlockDevice(
+                    numa_node=numa_node
+                ),
+            )
+            for numa_node in numa_nodes[:2]
+        ]
+
+        # the 3 remaining disks are part of another filesystem group which gets
+        # added to the first
+        nested_filesystems = [
+            factory.make_Filesystem(
+                fstype=FILESYSTEM_TYPE.LVM_PV,
+                block_device=factory.make_PhysicalBlockDevice(
+                    numa_node=numa_node
+                ),
+            )
+            for numa_node in numa_nodes[2:]
+        ]
+        nested_group = factory.make_FilesystemGroup(
+            node=node,
+            filesystems=nested_filesystems,
+            group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
+        )
+        virtual_block_device = factory.make_VirtualBlockDevice(
+            filesystem_group=nested_group
+        )
+        filesystems.append(
+            factory.make_Filesystem(
+                fstype=FILESYSTEM_TYPE.LVM_PV,
+                block_device=virtual_block_device,
+            )
+        )
+
+        fsgroup = factory.make_FilesystemGroup(
+            node=node,
+            filesystems=filesystems,
+            group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
+        )
+        self.assertEqual(fsgroup.get_numa_node_indexes(), [0, 1, 2, 3, 4])
+
     def test_get_node_returns_first_filesystem_node(self):
         fsgroup = factory.make_FilesystemGroup()
         self.assertEqual(
diff --git a/src/maasserver/models/virtualblockdevice.py b/src/maasserver/models/virtualblockdevice.py
index 9b9bb65..b59be68 100644
--- a/src/maasserver/models/virtualblockdevice.py
+++ b/src/maasserver/models/virtualblockdevice.py
@@ -153,13 +153,12 @@ class VirtualBlockDevice(BlockDevice):
                     return False
             else:
                 return False
-            for virtual_device in fs_group.virtual_devices.all():
-                if virtual_device.id == self.id:
-                    return True
-            return False
+
+            # whether the device is part of the filesystem group
+            return fs_group.virtual_devices.filter(id=self.id).exists()
 
         parents = []
-        # We need to check all of the nodes block devices incase
+        # We need to check all of the nodes block devices in case
         # we have nested virtual block devices.
         for block_device in self.node.blockdevice_set.all():
             if block_device.id == self.id:
diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
index 7b47bca..e214716 100644
--- a/src/maasserver/websockets/handlers/node.py
+++ b/src/maasserver/websockets/handlers/node.py
@@ -553,6 +553,7 @@ class NodeHandler(TimestampedModelHandler):
             "used_for": "volume group",
             "filesystem": None,
             "partitions": None,
+            "numa_nodes": volume_group.get_numa_node_indexes(),
         }
 
     def dehydrate_cache_set(self, cache_set):
@@ -582,6 +583,7 @@ class NodeHandler(TimestampedModelHandler):
             "used_for": ", ".join(bcache_devices),
             "filesystem": None,
             "partitions": None,
+            "numa_nodes": cache_set.get_numa_node_indexes(),
         }
 
     def dehydrate_partitions(self, partition_table):
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index 7144d1e..010e8bc 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -989,10 +989,40 @@ class TestMachineHandler(MAASServerTestCase):
                 "used_for": "volume group",
                 "filesystem": None,
                 "partitions": None,
+                "numa_nodes": [0],
             },
             handler.dehydrate_volume_group(volume_group),
         )
 
+    def test_dehydrate_volume_group_multiple_numa_nodes(self):
+        owner = factory.make_User()
+        node = factory.make_Node(owner=owner)
+        numa_nodes = [
+            node.default_numanode,
+            factory.make_NUMANode(node=node),
+            factory.make_NUMANode(node=node),
+        ]
+        block_devices = [
+            factory.make_PhysicalBlockDevice(numa_node=numa_node)
+            for numa_node in numa_nodes
+        ]
+        filesystems = [
+            factory.make_Filesystem(
+                fstype=FILESYSTEM_TYPE.LVM_PV, block_device=block_device
+            )
+            for block_device in block_devices
+        ]
+        volume_group = factory.make_FilesystemGroup(
+            node=node,
+            filesystems=filesystems,
+            group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
+        )
+        handler = MachineHandler(owner, {}, None)
+        self.assertEqual(
+            handler.dehydrate_volume_group(volume_group)["numa_nodes"],
+            [0, 1, 2],
+        )
+
     def test_dehydrate_cache_set(self):
         owner = factory.make_User()
         node = factory.make_Node(owner=owner)
@@ -1041,10 +1071,35 @@ class TestMachineHandler(MAASServerTestCase):
                 ),
                 "filesystem": None,
                 "partitions": None,
+                "numa_nodes": [0],
             },
             handler.dehydrate_cache_set(cache_set),
         )
 
+    def test_dehydrate_cache_set_multiple_numa_nodes(self):
+        owner = factory.make_User()
+        node = factory.make_Node(owner=owner)
+        numa_nodes = [
+            node.default_numanode,
+            factory.make_NUMANode(node=node),
+            factory.make_NUMANode(node=node),
+        ]
+        cache_set = factory.make_CacheSet(node=node)
+        for numa_node in numa_nodes:
+            backing = factory.make_PhysicalBlockDevice(numa_node=numa_node)
+            fs = factory.make_Filesystem(
+                block_device=backing, fstype=FILESYSTEM_TYPE.BCACHE_BACKING
+            )
+            factory.make_FilesystemGroup(
+                group_type=FILESYSTEM_GROUP_TYPE.BCACHE,
+                filesystems=[fs],
+                cache_set=cache_set,
+            )
+        handler = MachineHandler(owner, {}, None)
+        self.assertEqual(
+            handler.dehydrate_cache_set(cache_set)["numa_nodes"], [0, 1, 2]
+        )
+
     def test_dehydrate_partitions_returns_None(self):
         owner = factory.make_User()
         handler = MachineHandler(owner, {}, None)