Merge ~adam-collard/maas:lxd-numa-memory-1878923 into maas:master

Proposed by Adam Collard
Status: Merged
Approved by: Adam Collard
Approved revision: 46dea51cc575a72e4d210606c04a50e6e845e42f
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~adam-collard/maas:lxd-numa-memory-1878923
Merge into: maas:master
Diff against target: 148 lines (+64/-10)
4 files modified
src/maasserver/models/numa.py (+3/-0)
src/metadataserver/builtin_scripts/hooks.py (+2/-5)
src/metadataserver/builtin_scripts/tests/test_hooks.py (+47/-0)
src/provisioningserver/utils/lxd.py (+12/-5)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Lee Trager (community) Approve
Review via email: mp+386396@code.launchpad.net

Commit message

Improve robustness of NUMA node identification

Fixes LP:1885157

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

UNIT TESTS
-b lxd-numa-memory-1878923 lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/7820/console
COMMIT: 245d125f97afbe54d50fe792bf0412cb1f4f0214

review: Needs Fixing
Revision history for this message
Lee Trager (ltrager) wrote :

LGTM!

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

UNIT TESTS
-b lxd-numa-memory-1878923 lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 46dea51cc575a72e4d210606c04a50e6e845e42f

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/models/numa.py b/src/maasserver/models/numa.py
2index ee82464..f6e347c 100644
3--- a/src/maasserver/models/numa.py
4+++ b/src/maasserver/models/numa.py
5@@ -19,6 +19,9 @@ class NUMANode(CleanSave, TimestampedModel):
6 class Meta:
7 unique_together = [("node", "index")]
8
9+ def __repr__(self):
10+ return f"<NUMANode of {self.index} {self.node!r} cores: {self.cores!r} {self.memory}>"
11+
12
13 def create_default_numanode(machine):
14 """Create the default "0" NUMA node for a machine."""
15diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
16index c7a3580..26c7e11 100644
17--- a/src/metadataserver/builtin_scripts/hooks.py
18+++ b/src/metadataserver/builtin_scripts/hooks.py
19@@ -464,10 +464,7 @@ def _process_lxd_resources(node, data):
20 NUMANode.objects.update_or_create(
21 node=node,
22 index=numa_index,
23- defaults={
24- "memory": numa_data["memory"],
25- "cores": numa_data["cores"],
26- },
27+ defaults={"memory": numa_data.memory, "cores": numa_data.cores},
28 )[0]
29 for numa_index, numa_data in numa_nodes.items()
30 ]
31@@ -494,7 +491,7 @@ def _parse_memory(memory, numa_nodes):
32 default_numa_node = {"numa_node": 0, "total": total_memory}
33
34 for memory_node in memory.get("nodes", [default_numa_node]):
35- numa_nodes[memory_node["numa_node"]]["memory"] = int(
36+ numa_nodes[memory_node["numa_node"]].memory = int(
37 memory_node.get("total", 0) / 1024 ** 2
38 )
39
40diff --git a/src/metadataserver/builtin_scripts/tests/test_hooks.py b/src/metadataserver/builtin_scripts/tests/test_hooks.py
41index 872b5ae..be1a517 100644
42--- a/src/metadataserver/builtin_scripts/tests/test_hooks.py
43+++ b/src/metadataserver/builtin_scripts/tests/test_hooks.py
44@@ -1486,6 +1486,53 @@ class TestProcessLXDResults(MAASServerTestCase):
45 [32158, 0], [numa.memory for numa in node.numanode_set.all()]
46 )
47
48+ def test_updates_memory_no_corresponding_cpu_numa_node(self):
49+ # Regression test for LP:1885157
50+ node = factory.make_Node()
51+ self.patch(hooks_module, "update_node_network_information")
52+ data = make_lxd_output()
53+ data["resources"]["memory"] = {
54+ "nodes": [
55+ {
56+ "numa_node": 252,
57+ "hugepages_used": 0,
58+ "hugepages_total": 0,
59+ "used": 1314131968,
60+ "total": 33720463360,
61+ },
62+ {
63+ "numa_node": 253,
64+ "hugepages_used": 0,
65+ "hugepages_total": 0,
66+ "used": 0,
67+ "total": 0,
68+ },
69+ {
70+ "numa_node": 254,
71+ "hugepages_used": 0,
72+ "hugepages_total": 0,
73+ "used": 0,
74+ "total": 0,
75+ },
76+ {
77+ "numa_node": 255,
78+ "hugepages_used": 0,
79+ "hugepages_total": 0,
80+ "used": 0,
81+ "total": 0,
82+ },
83+ ],
84+ "hugepages_total": 0,
85+ "hugepages_used": 0,
86+ "hugepages_size": 2097152,
87+ "used": 602902528,
88+ "total": 33720463360,
89+ }
90+
91+ process_lxd_results(node, json.dumps(data).encode(), 0)
92+ numa_nodes = NUMANode.objects.filter(node=node).order_by("index")
93+ self.assertEqual(6, len(numa_nodes))
94+
95 def test_updates_cpu_numa_nodes(self):
96 node = factory.make_Node()
97 self.patch(hooks_module, "update_node_network_information")
98diff --git a/src/provisioningserver/utils/lxd.py b/src/provisioningserver/utils/lxd.py
99index 0b29b64..f45ffd1 100644
100--- a/src/provisioningserver/utils/lxd.py
101+++ b/src/provisioningserver/utils/lxd.py
102@@ -5,8 +5,11 @@
103
104 __all__ = ["parse_lxd_cpuinfo"]
105
106+from collections import defaultdict
107 import re
108
109+import attr
110+
111
112 # This is needed on the rack controller in the LXDPodDriver to set
113 # the cpu_speed for discovered machines as the lxd resources are
114@@ -16,6 +19,13 @@ def lxd_cpu_speed(data):
115 return cpu_speed
116
117
118+@attr.s
119+class NUMANode:
120+
121+ memory: int = attr.ib(default=0)
122+ cores: list = attr.ib(factory=list)
123+
124+
125 def parse_lxd_cpuinfo(data):
126 cpu_speed = 0
127 cpu_model = None
128@@ -23,7 +33,7 @@ def parse_lxd_cpuinfo(data):
129 # Only update the cpu_model if all the socket names match.
130 sockets = data.get("cpu", {}).get("sockets", [])
131 names = []
132- numa_nodes = {}
133+ numa_nodes = defaultdict(NUMANode)
134 for socket in sockets:
135 name = socket.get("name")
136 if name is not None:
137@@ -32,10 +42,7 @@ def parse_lxd_cpuinfo(data):
138 for thread in core.get("threads", []):
139 thread_id = thread["id"]
140 numa_node = thread["numa_node"]
141- if numa_node in numa_nodes:
142- numa_nodes[numa_node]["cores"].append(thread_id)
143- else:
144- numa_nodes[numa_node] = {"cores": [thread_id]}
145+ numa_nodes[numa_node].cores.append(thread_id)
146
147 if len(names) > 0 and all(name == names[0] for name in names):
148 cpu = names[0]

Subscribers

People subscribed via source and target branches