MAAS

Merge ~andreserl/maas:virsh_reduce_nodeinfo_queries into maas:master

  1. Git
  2. lp:~andreserl/maas
  3. virsh_reduce_nodeinfo_queries
  4. Merge into master
Proposed by Andres Rodriguez
Status: Merged
Approved by: Andres Rodriguez
Approved revision: 43a32b1de93560a7b0678025961bfa32eea9cfed
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~andreserl/maas:virsh_reduce_nodeinfo_queries
Merge into: maas:master
Diff against target: 197 lines (+42/-27)
2 files modified
src/provisioningserver/drivers/pod/tests/test_virsh.py (+21/-8)
src/provisioningserver/drivers/pod/virsh.py (+21/-19)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Newell Jensen (community) Approve
Review via email: mp+359964@code.launchpad.net

Commit message

Improve virsh communication with the host to reduce 'nodeinfo' queries from 4 to 1.

To post a comment you must log in.
Revision history for this message
Newell Jensen (newell-jensen) wrote :

Looks good!

review: Approve
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Nice optimization!

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

UNIT TESTS
-b virsh_reduce_nodeinfo_queries lp:~andreserl/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 43a32b1de93560a7b0678025961bfa32eea9cfed

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

LANDING
-b virsh_reduce_nodeinfo_queries lp:~andreserl/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/4627/consoleText

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/provisioningserver/drivers/pod/tests/test_virsh.py b/src/provisioningserver/drivers/pod/tests/test_virsh.py
2index 0c9498a..e741ff4 100644
3--- a/src/provisioningserver/drivers/pod/tests/test_virsh.py
4+++ b/src/provisioningserver/drivers/pod/tests/test_virsh.py
5@@ -672,12 +672,14 @@ class TestVirshSSH(MAASTestCase):
6
7 def test_get_pod_cpu_count(self):
8 conn = self.configure_virshssh(SAMPLE_NODEINFO)
9- expected = conn.get_pod_cpu_count()
10+ nodeinfo = conn.get_pod_nodeinfo()
11+ expected = conn.get_pod_cpu_count(nodeinfo)
12 self.assertEqual(8, expected)
13
14 def test_get_pod_cpu_count_returns_zero_if_info_not_available(self):
15 conn = self.configure_virshssh('')
16- expected = conn.get_pod_cpu_count()
17+ nodeinfo = conn.get_pod_nodeinfo()
18+ expected = conn.get_pod_cpu_count(nodeinfo)
19 self.assertEqual(0, expected)
20
21 def test_get_machine_cpu_count(self):
22@@ -692,7 +694,8 @@ class TestVirshSSH(MAASTestCase):
23
24 def test_get_pod_cpu_speed(self):
25 conn = self.configure_virshssh(SAMPLE_NODEINFO)
26- expected = conn.get_pod_cpu_speed()
27+ nodeinfo = conn.get_pod_nodeinfo()
28+ expected = conn.get_pod_cpu_speed(nodeinfo)
29 self.assertEqual(2400, expected)
30
31 def test_get_pod_cpu_speed_returns_zero_if_info_not_available(self):
32@@ -700,17 +703,20 @@ class TestVirshSSH(MAASTestCase):
33 mock_get_key_value_unitless = self.patch(
34 virsh.VirshSSH, "get_key_value_unitless")
35 mock_get_key_value_unitless.return_value = None
36- expected = conn.get_pod_cpu_speed()
37+ nodeinfo = conn.get_pod_nodeinfo()
38+ expected = conn.get_pod_cpu_speed(nodeinfo)
39 self.assertEqual(0, expected)
40
41 def test_get_pod_memory(self):
42 conn = self.configure_virshssh(SAMPLE_NODEINFO)
43- expected = conn.get_pod_memory()
44+ nodeinfo = conn.get_pod_nodeinfo()
45+ expected = conn.get_pod_memory(nodeinfo)
46 self.assertEqual(int(16307176 / 1024), expected)
47
48 def test_get_pod_memory_returns_zero_if_info_not_available(self):
49 conn = self.configure_virshssh('')
50- expected = conn.get_pod_memory()
51+ nodeinfo = conn.get_pod_nodeinfo()
52+ expected = conn.get_pod_memory(nodeinfo)
53 self.assertEqual(0, expected)
54
55 def test_get_machine_memory(self):
56@@ -768,12 +774,13 @@ class TestVirshSSH(MAASTestCase):
57
58 def test_get_pod_arch(self):
59 conn = self.configure_virshssh(SAMPLE_NODEINFO)
60- expected = conn.get_pod_arch()
61+ nodeinfo = conn.get_pod_nodeinfo()
62+ expected = conn.get_pod_arch(nodeinfo)
63 self.assertEqual('amd64/generic', expected)
64
65 def test_get_pod_arch_raises_error_if_not_found(self):
66 conn = self.configure_virshssh('')
67- self.assertRaises(PodInvalidResources, conn.get_pod_arch)
68+ self.assertRaises(PodInvalidResources, conn.get_pod_arch, None)
69
70 def test_get_machine_arch_returns_valid(self):
71 arch = factory.make_name('arch')
72@@ -806,6 +813,8 @@ class TestVirshSSH(MAASTestCase):
73 for _ in range(3)
74 ]
75 local_storage = sum(pool.storage for pool in storage_pools)
76+ mock_get_pod_nodeinfo = self.patch(
77+ virsh.VirshSSH, 'get_pod_nodeinfo')
78 mock_get_pod_arch = self.patch(
79 virsh.VirshSSH, 'get_pod_arch')
80 mock_get_pod_cpu_count = self.patch(
81@@ -816,6 +825,7 @@ class TestVirshSSH(MAASTestCase):
82 virsh.VirshSSH, 'get_pod_memory')
83 mock_get_pod_storage_pools = self.patch(
84 virsh.VirshSSH, 'get_pod_storage_pools')
85+ mock_get_pod_nodeinfo.return_value = SAMPLE_NODEINFO
86 mock_get_pod_arch.return_value = architecture
87 mock_get_pod_cpu_count.return_value = cores
88 mock_get_pod_cpu_speed.return_value = cpu_speed
89@@ -841,11 +851,14 @@ class TestVirshSSH(MAASTestCase):
90 memory = random.randint(4096, 8192)
91 cpu_speed = random.randint(2000, 3000)
92 local_storage = random.randint(4096, 8192)
93+ mock_get_pod_nodeinfo = self.patch(
94+ virsh.VirshSSH, 'get_pod_nodeinfo')
95 mock_get_pod_cores = self.patch(
96 virsh.VirshSSH, 'get_pod_cpu_count')
97 mock_get_pod_cores.return_value = cores
98 mock_get_pod_memory = self.patch(
99 virsh.VirshSSH, 'get_pod_memory')
100+ mock_get_pod_nodeinfo.return_value = SAMPLE_NODEINFO
101 mock_get_pod_memory.return_value = memory
102 mock_get_pod_cpu_speed = self.patch(
103 virsh.VirshSSH, 'get_pod_cpu_speed')
104diff --git a/src/provisioningserver/drivers/pod/virsh.py b/src/provisioningserver/drivers/pod/virsh.py
105index bc947c5..ad0c69e 100644
106--- a/src/provisioningserver/drivers/pod/virsh.py
107+++ b/src/provisioningserver/drivers/pod/virsh.py
108@@ -495,10 +495,9 @@ class VirshSSH(pexpect.spawn):
109 output = output.splitlines()[2:]
110 return [InterfaceInfo(*line.split()[1:5]) for line in output]
111
112- def get_pod_cpu_count(self):
113+ def get_pod_cpu_count(self, nodeinfo):
114 """Gets number of CPUs in the pod."""
115- output = self.run(['nodeinfo']).strip()
116- cpu_count = self.get_key_value(output, "CPU(s)")
117+ cpu_count = self.get_key_value(nodeinfo, "CPU(s)")
118 if cpu_count is None:
119 maaslog.error("Failed to get pod CPU count")
120 return 0
121@@ -513,19 +512,17 @@ class VirshSSH(pexpect.spawn):
122 return 0
123 return int(cpu_count)
124
125- def get_pod_cpu_speed(self):
126+ def get_pod_cpu_speed(self, nodeinfo):
127 """Gets CPU speed (MHz) in the pod."""
128- output = self.run(['nodeinfo']).strip()
129- cpu_speed = self.get_key_value_unitless(output, "CPU frequency")
130+ cpu_speed = self.get_key_value_unitless(nodeinfo, "CPU frequency")
131 if cpu_speed is None:
132 maaslog.error("Failed to get pod CPU speed")
133 return 0
134 return int(cpu_speed)
135
136- def get_pod_memory(self):
137+ def get_pod_memory(self, nodeinfo):
138 """Gets the total memory of the pod."""
139- output = self.run(['nodeinfo']).strip()
140- KiB = self.get_key_value_unitless(output, "Memory size")
141+ KiB = self.get_key_value_unitless(nodeinfo, "Memory size")
142 if KiB is None:
143 maaslog.error("Failed to get pod memory")
144 return 0
145@@ -602,10 +599,13 @@ class VirshSSH(pexpect.spawn):
146 except TypeError:
147 return None
148
149- def get_pod_arch(self):
150+ def get_pod_nodeinfo(self):
151+ """Gets the general information of the node via 'nodeinfo'"""
152+ return self.run(['nodeinfo']).strip()
153+
154+ def get_pod_arch(self, nodeinfo):
155 """Gets architecture of the pod."""
156- output = self.run(['nodeinfo']).strip()
157- arch = self.get_key_value(output, "CPU model")
158+ arch = self.get_key_value(nodeinfo, "CPU model")
159 if arch is None:
160 maaslog.error("Failed to get pod architecture")
161 raise PodInvalidResources(
162@@ -639,16 +639,17 @@ class VirshSSH(pexpect.spawn):
163 architectures=[], cores=0, cpu_speed=0, memory=0, local_storage=0,
164 hints=DiscoveredPodHints(
165 cores=0, cpu_speed=0, memory=0, local_storage=0))
166- discovered_pod.architectures = [self.get_pod_arch()]
167 discovered_pod.capabilities = [
168 Capabilities.COMPOSABLE,
169 Capabilities.DYNAMIC_LOCAL_STORAGE,
170 Capabilities.OVER_COMMIT,
171 Capabilities.STORAGE_POOLS,
172 ]
173- discovered_pod.cores = self.get_pod_cpu_count()
174- discovered_pod.cpu_speed = self.get_pod_cpu_speed()
175- discovered_pod.memory = self.get_pod_memory()
176+ nodeinfo = self.get_pod_nodeinfo()
177+ discovered_pod.architectures = [self.get_pod_arch(nodeinfo)]
178+ discovered_pod.cores = self.get_pod_cpu_count(nodeinfo)
179+ discovered_pod.cpu_speed = self.get_pod_cpu_speed(nodeinfo)
180+ discovered_pod.memory = self.get_pod_memory(nodeinfo)
181 discovered_pod.storage_pools = self.get_pod_storage_pools()
182 discovered_pod.local_storage = sum(
183 pool.storage for pool in discovered_pod.storage_pools)
184@@ -661,9 +662,10 @@ class VirshSSH(pexpect.spawn):
185 # You can always create a domain up to the size of total cores,
186 # memory, and cpu_speed even if that amount is already in use.
187 # Not a good idea, but possible.
188- discovered_pod_hints.cores = self.get_pod_cpu_count()
189- discovered_pod_hints.cpu_speed = self.get_pod_cpu_speed()
190- discovered_pod_hints.memory = self.get_pod_memory()
191+ nodeinfo = self.get_pod_nodeinfo()
192+ discovered_pod_hints.cores = self.get_pod_cpu_count(nodeinfo)
193+ discovered_pod_hints.cpu_speed = self.get_pod_cpu_speed(nodeinfo)
194+ discovered_pod_hints.memory = self.get_pod_memory(nodeinfo)
195 discovered_pod_hints.local_storage = (
196 self.get_pod_available_local_storage())
197 return discovered_pod_hints

Subscribers

People subscribed via source and target branches