← Back to team overview

sts-sponsors team mailing list archive

[Merge] ~troyanov/maas:fix-2003940-3.3 into maas:3.3

 

Anton Troyanov has proposed merging ~troyanov/maas:fix-2003940-3.3 into maas:3.3.

Commit message:
fix(ws): incorrect storage amount

Combining multiple aggregations with annotate() will yield the wrong results
because joins are used instead of subqueries.

See related Django bug: https://code.djangoproject.com/ticket/10060

Resolves LP:2003940

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #2003940 in MAAS: "MAAS 3.3 RC shows incorrect storage amount"
  https://bugs.launchpad.net/maas/+bug/2003940

For more details, see:
https://code.launchpad.net/~troyanov/maas/+git/maas/+merge/442024
-- 
Your team MAAS Committers is subscribed to branch maas:3.3.
diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py
index d5ae491..9365a64 100644
--- a/src/maasserver/websockets/handlers/machine.py
+++ b/src/maasserver/websockets/handlers/machine.py
@@ -15,6 +15,7 @@ from django.db.models import (
     Count,
     Exists,
     F,
+    IntegerField,
     OuterRef,
     Subquery,
     Sum,
@@ -127,6 +128,15 @@ class MachineHandler(NodeHandler):
                 "partitiontable_set__partitions"
             )
         )
+        node_storage_query_set = (
+            Machine.objects.select_related("current_config")
+            .annotate(
+                storage=Sum(
+                    "current_config__blockdevice__physicalblockdevice__size"
+                )
+            )
+            .filter(pk=OuterRef("pk"))
+        )
         list_queryset = (
             Machine.objects.all()
             .select_related("owner", "zone", "domain", "bmc", "current_config")
@@ -157,10 +167,12 @@ class MachineHandler(NodeHandler):
             .prefetch_related("ownerdata_set")
             .alias(
                 physical_disk_count=Count(
-                    "current_config__blockdevice__physicalblockdevice"
+                    "current_config__blockdevice__physicalblockdevice",
+                    distinct=True,
                 ),
-                storage=Sum(
-                    "current_config__blockdevice__physicalblockdevice__size"
+                storage=Subquery(
+                    node_storage_query_set.values("storage"),
+                    output_field=IntegerField(),
                 ),
                 pxe_mac=F("boot_interface__mac_address"),
                 fabric_name=F("boot_interface__vlan__fabric__name"),
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index 08afc62..ebac3a3 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -6041,6 +6041,30 @@ class TestMachineHandlerNewSchema(MAASServerTestCase):
         self.assertEqual(2, result["count"])
         self.assertEqual(2, result["groups"][0]["count"])
 
+    def test_filter_storage_counters(self):
+        user = factory.make_User()
+        nodes = [factory.make_Machine(owner=user) for _ in range(2)]
+        for node in nodes:
+            # factory.make_PhysicalBlockDevice(node=node)
+            # _ = [node.tags.add(factory.make_Tag()) for _ in range(3)]
+            [node.tags.add(factory.make_Tag()) for _ in range(2)]
+            # node.tags.add(factory.make_Tag())
+
+        handler = MachineHandler(user, {}, None)
+        result = handler.list({"filter": {"free_text": nodes[0].hostname}})
+
+        self.assertEqual(1, result["count"])
+        self.assertEqual(
+            nodes[0].physicalblockdevice_set.count(),
+            result["groups"][0]["items"][0]["physical_disk_count"],
+        )
+        self.assertEqual(
+            round(
+                nodes[0].physicalblockdevice_set.first().size / (1000**3), 1
+            ),
+            result["groups"][0]["items"][0]["storage"],
+        )
+
     def test_sort_alias(self):
         user = factory.make_User()
         fabrics = [factory.make_Fabric() for _ in range(2)]

Follow ups