Merge lp:~ltrager/maas/parse_proc into lp:~maas-committers/maas/trunk

[Lee Trager]

Get cpu_count from /proc/cpuinfo and memory from dmesg

lshw isn't giving us correct data. With this MP MAAS no longer processes the data from lshw, however it will still be captured.

With some CPU's, such as the ones in the NEC lab, lshw is only displaying the number of physical CPU's and not mentioning anything about how many cores they have. As /proc/cpuinfo shows all the cores the kernel has access to upload and process it to get the correct number of CPU cores.

lshw CPU is no longer reporting the correct amount of RAM. This isn't entirely lshw's fault as newer kernels are only reporting the amount of available RAM. RAM can be reserved by the BIOS(e.g for video cards) or by the kernel itself. I searched around a bit to try to find the closest number I could get. On boot the kernel itself prints the number closest to the amount of actual physical RAM.

As per our discussion this morning I modified Node.display_memory() to show the amount of RAM in GiB with one decimal place. Because Python automatically rounds, and the memory value is so close to the actual value, the UI now displays the correct amount of RAM in GiB.

[Andres Rodriguez (andreserl)]

Can you please be more specific when you say that lshw is not providing the correct information? The big problem here is that MAAS tagging based on definition feature depends on LSHW output. I'd think that instead of doing a work around because LSHW is broken, we should actually fix LSHW.

Can you please provide examples of LSHW being broken?

Also, for example, you can create a tag with a definition that has X amount of cores. This definition will scrape lshw output on all machines to tag all machines with that definition. As such, we can't just not rely on lshw without completely affecting that feature. So we really need to fix lshw.

review: Needs Information

[Andres Rodriguez (andreserl)]

actually! Gonna reject this branch because this would completely break definition based tagging.

review: Disapprove

[Lee Trager (ltrager)]

The reason MAAS isn't showing the correct amount of CPU cores is because lshw isn't detecting all of them. http://pastebin.com/1Fwd8U8U is the output of lshw -xml from antileague-adolfo in the NEC lab. As you can see its only showing 1 CPU and lists nothing about the amount of cores. http://paste.ubuntu.com/16232408/ is /proc/cpuinfo which lists all usable CPU cores.

The amount of memory lshw is the same as what the MemTotal field from /proc/meminfo is showing. I found that the kernel lists a bit more in dmesg on boot. I assume the amount listed is dmesg is the total amount of memory available to the kernel while /proc/meminfo shows the total amount of memory available to the user. I'm trying to capture the most correct answer without guessing.

lshw is still captured and stored, it is NOT removed. What this MP changes is where commissioning pulls the cpu_count and memory from. Tagging looks at the stored lshw data and is unaffected by this change.

[Blake Rouse (blake-rouse)]

review: Needs Information

[Blake Rouse (blake-rouse)]

Oh and as Lee says it does not affect tagging or anything that depends on LSHW, as that information is still returned. We just use proc to get the other information.

[Lee Trager (ltrager)]

lshw-B.02.18 fixes both showing the correct amount of cores and physical RAM. As per the discussion I had with Andres I will be seeing if we can RSU lshw-B.02.18 into Xenial or backport the patch which fixes both bugs. Until then we will parse lshw as well as /proc/cpuinfo and choose the higher number for the amount of CPU cores.

[Andres Rodriguez (andreserl)]

review: Needs Fixing

[Lee Trager (ltrager)]

I've reordered the commissioning scripts so both lshw and /proc/cpuinfo are both 01. After uploading this I noticed trunk skipped 05 while this branch doesn't. Let me know if you want me to restore the original order or if closing the gap is alright.

[Andres Rodriguez (andreserl)]

review: Needs Fixing

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

=== modified file 'src/metadataserver/models/commissioningscript.py'
--- src/metadataserver/models/commissioningscript.py	2016-05-10 16:58:17 +0000
+++ src/metadataserver/models/commissioningscript.py	2016-05-11 21:32:07 +0000
@@ -19,6 +19,7 @@
 import logging
 import math
 import os.path
+import re
 import tarfile
 from time import time as now
 
@@ -175,15 +176,34 @@
     else:
         # Same document, many queries: use XPathEvaluator.
         evaluator = etree.XPathEvaluator(doc)
-        cpu_count = evaluator(_xpath_processor_count)
+        cpu_count = evaluator(_xpath_processor_count) or 0
         memory = evaluator(_xpath_memory_bytes)
         if not memory or math.isnan(memory):
             memory = 0
-        node.cpu_count = cpu_count or 0
+        # XXX ltrager 2016-05-09 - Work around for LP:1579996. On some
+        # CPU's lshw doesn't detect all CPU cores. MAAS captures and
+        # processes /proc/cpuinfo so MAAS chooses the highest number.
+        if node.cpu_count is None or cpu_count > node.cpu_count:
+            node.cpu_count = cpu_count
         node.memory = memory
         node.save()
 
 
+def parse_cpuinfo(node, output, exit_status):
+    """Parse the output of /proc/cpuinfo."""
+    assert isinstance(output, bytes)
+    if exit_status != 0:
+        return
+    decoded_output = output.decode('ascii')
+    cpu_count = len([
+        m.start()
+        for m in re.finditer('processor\t:', decoded_output)
+    ])
+    if node.cpu_count is None or cpu_count > node.cpu_count:
+        node.cpu_count = cpu_count
+        node.save()
+
+
 def set_virtual_tag(node, output, exit_status):
     """Process the results of `VIRTUALITY_SCRIPT`.
 
@@ -354,6 +374,7 @@
 
 # Register the post processing hooks.
 NODE_INFO_SCRIPTS[LSHW_OUTPUT_NAME]['hook'] = update_hardware_details
+NODE_INFO_SCRIPTS['00-maas-01-cpuinfo.out']['hook'] = parse_cpuinfo
 NODE_INFO_SCRIPTS['00-maas-02-virtuality.out']['hook'] = set_virtual_tag
 NODE_INFO_SCRIPTS['00-maas-07-block-devices.out']['hook'] = (
     update_node_physical_block_devices)

=== modified file 'src/metadataserver/models/tests/test_commissioningscript.py'
--- src/metadataserver/models/tests/test_commissioningscript.py	2016-05-10 20:10:34 +0000
+++ src/metadataserver/models/tests/test_commissioningscript.py	2016-05-11 21:32:07 +0000
@@ -52,6 +52,7 @@
     inject_lldp_result,
     inject_lshw_result,
     inject_result,
+    parse_cpuinfo,
     set_virtual_tag,
     update_hardware_details,
     update_node_network_information,
@@ -414,6 +415,23 @@
         self.assertEqual("", logger.output)
 
 
+class TestParseCPUInfo(MAASServerTestCase):
+
+    doctest_flags = doctest.ELLIPSIS | doctest.NORMALIZE_WHITESPACE
+
+    def test_parse_cpuinfo(self):
+        node = factory.make_Node()
+        cpuinfo = dedent("""\
+        processor\t: 0
+        vendor_id\t: GenuineIntel
+
+        processor\t: 1
+        vendor_id\t: GenuineIntel
+        """).encode('utf-8')
+        parse_cpuinfo(node, cpuinfo, 0)
+        self.assertEqual(2, reload_object(node).cpu_count)
+
+
 class TestUpdateNodePhysicalBlockDevices(MAASServerTestCase):
 
     def make_block_device(

=== modified file 'src/provisioningserver/refresh/node_info_scripts.py'
--- src/provisioningserver/refresh/node_info_scripts.py	2016-03-28 13:54:47 +0000
+++ src/provisioningserver/refresh/node_info_scripts.py	2016-05-11 21:32:07 +0000
@@ -83,6 +83,11 @@
     fi
     """)
 
+CPUINFO_SCRIPT = dedent("""\
+    #!/bin/sh
+    cat /proc/cpuinfo
+    """)
+
 
 # Run `dhclient` on all the unconfigured interfaces.
 # This is done to create records in the leases file for the
@@ -336,6 +341,11 @@
         'hook': null_hook,
         'run_on_controller': True,
     }),
+    ('00-maas-01-cpuinfo.out', {
+        'content': CPUINFO_SCRIPT.encode('ascii'),
+        'hook': null_hook,
+        'run_on_controller': True,
+    }),
     ('00-maas-02-virtuality.out', {
         'content': VIRTUALITY_SCRIPT.encode('ascii'),
         'hook': null_hook,