Merge lp:~nelson-chu/opencompute/add-ocp-cpu-memory-job into lp:opencompute/checkbox

[Nelson Chu]

Scripts and annotations have been modified.

[Jeff Lane  (bladernr)]

Hi Nelson,

Thank you for breaking this into smaller pieces... it makes review a LOT easier. Now, there are some things that need fixing.

I'll take them one file at a time:
data/whitelists/opencompute-certify-local.whitelist looks fine.

jobs/TC-001-0001-CPU_Memory.txt.in:
1: any job that calls a script that needs root permissions to run must include the 'user: root' definition. See the file 'jobs/cpu.txt.in' for some examples where this is necessary. When I run the script cpu_info without root, the cache data is not returned and the test will fail. So this job needs 'user:root'
2: TC-001-0001-003-Memory_Information also needs 'user: root' added to properly run.

jobs/local.txt.in
1: The word "Verified" should be "Verify" in the description field.

scripts/cpu_info:
1: When I manually run the cpu_info script for the test TC-001-0001-001-CPU_information with and without root permissions, I get the following different outputs:
bladernr@klaatu:~/development/ocp-nelson-memory-cpu-test$ ./scripts/cpu_infoIntel(R) Core(TM) i7 CPU Q 720 @ 1.60GHz
Can not found any CPU cache.
bladernr@klaatu:~/development/ocp-nelson-memory-cpu-test$ echo $?
30
bladernr@klaatu:~/development/ocp-nelson-memory-cpu-test$ sudo ./scripts/cpu_info
Intel(R) Core(TM) i7 CPU Q 720 @ 1.60GHz
L1 Cache 32 KB
L2 Cache 256 KB
L3 Cache 6 MB
L3 cache size less than 20MB.
bladernr@klaatu:~/development/ocp-nelson-memory-cpu-test$ echo $?
50

I don't have a system that meets the test criteria, so it will always fail for me. First, the explanation for failure should be more explicit. Example: 'FAIL: Can not find any CPU cache.' for the first example. This is more important in the second example where 'L3 cache size less than 20MB' looks like part of the lshw output. It would be better more explicitly stated as 'FAIL: L3 cache size less than 20MB.'

2: You don't need all those explicit error codes. Checkbox only knows and stores 0 and Not 0 exit codes. A 0 exit code indicates a test passed. A non-zero exit code indicates a failure. Checkbox does not store the actual exit codes. This is not necessarily something you need to change, but you DO need to be aware of the behaviour in case a script behaves differently than expected when you run it via checkbox.

3: The description and spec for this test says: "CPU model should belong to Intel Xeon processor E5-2600 family..." but your test does not fail on non-Xeon processors. For example, when I comment out the error code return for my cache limit on my laptop, I get this output:
bladernr@klaatu:~/development/ocp-nelson-memory-cpu-test$ sudo ./scripts/cpu_info; echo $?
Intel(R) Core(TM) i7 CPU Q 720 @ 1.60GHz
L1 Cache 32 KB
L2 Cache 256 KB
L3 Cache 6 MB
L3 cache size less than 20MB.
0

but my laptop should clearly fail the test case since it's not up to OCP spec.

4: The output needs to be cleaned up a bit. Sorry, I know English is a second language for you, so I'll try to help as much as I can.
    "Can not parser" should be "Can not parse"
    "Can not found" should be "Can not find"
    "# Parser lshw XML for gather" should be "# Parse lshw XML for gathering"

scripts/memory_info:
1: Needs to be run as ...

review: Needs Fixing

[Nelson Chu (nelson-chu)]

OK, Thank you for your suggestion.
I will revise it accordingly.

[Nelson Chu (nelson-chu)]

Hi Jeff,

I have modified scripts. Please help me review them.
Any suggestion will be appreciated.

Thanks,
Nelson

review: Needs Resubmitting

[Jeff Lane  (bladernr)]

Before I can go any further, you have a conflict in data/whitelists/opencompute-certify-local.whitelist

To see this, you should do the following:

bzr branch lp:opencompute/checkbox ocp-checkbox
cd ocp-checkbox
bzr merge lp:~nelson-chu/opencompute/add-ocp-cpu-memory-job

That MAY actually just clean it up... but there's a file conflict in there. so please resolve that and I'll review the rest at that time.

review: Needs Fixing

[lp:~nelson-chu/opencompute/add-ocp-cpu-memory-job]

2170. By Nelson Chu on 2014-02-13

Merge add-ocp-cpu-memory-job to lp:opencompute/checkbox and fixed some conflicts

2171. By Nelson Chu on 2014-02-14

Modify cpu_info and memory_info scripts. Revise debian/changelog file.

2172. By Nelson Chu on 2014-02-21

debian/changelog file and memory_info script

Unmerged revisions

=== added file 'data/whitelists/opencompute-certify-local.whitelist'
--- data/whitelists/opencompute-certify-local.whitelist	1970-01-01 00:00:00 +0000
+++ data/whitelists/opencompute-certify-local.whitelist	2014-02-13 07:27:19 +0000
@@ -0,0 +1,47 @@
+# Resource Jobs
+block_device
+cdimage
+cpuinfo
+device
+dmi
+dpkg
+efi
+environment
+gconf
+lsb
+meminfo
+module
+optical_drive
+package
+sleep
+uname
+#Info attachment jobs
+__info__
+cpuinfo_attachment
+dmesg_attachment
+dmi_attachment
+dmidecode_attachment
+efi_attachment
+lspci_attachment
+lshw_attachment
+mcelog_attachment
+meminfo_attachment
+modprobe_attachment
+modules_attachment
+sysctl_attachment
+sysfs_attachment
+udev_attachment
+lsmod_attachment
+acpi_sleep_attachment
+info/hdparm
+info/hdparm_.*.txt
+installer_debug.gz
+info/disk_partitions
+# Actual test cases
+__TC-001-0001-CPU_Memory__
+TC-001-0001-001-CPU_Information
+TC-001-0001-002-Processor_Topology
+TC-001-0001-003-Memory_Information
+__TC-001-0002-Platform_Controller_Hub__
+TC-001-0002-001-SATA_port
+TC-001-0002-002-USB_2.0

=== removed file 'data/whitelists/opencompute-certify-local.whitelist'
--- data/whitelists/opencompute-certify-local.whitelist	2014-02-11 12:16:41 +0000
+++ data/whitelists/opencompute-certify-local.whitelist	1970-01-01 00:00:00 +0000
@@ -1,43 +0,0 @@
-# Resource Jobs
-block_device
-cdimage
-cpuinfo
-device
-dmi
-dpkg
-efi
-environment
-gconf
-lsb
-meminfo
-module
-optical_drive
-package
-sleep
-uname
-#Info attachment jobs
-__info__
-cpuinfo_attachment
-dmesg_attachment
-dmi_attachment
-dmidecode_attachment
-efi_attachment
-lspci_attachment
-lshw_attachment
-mcelog_attachment
-meminfo_attachment
-modprobe_attachment
-modules_attachment
-sysctl_attachment
-sysfs_attachment
-udev_attachment
-lsmod_attachment
-acpi_sleep_attachment
-info/hdparm
-info/hdparm_.*.txt
-installer_debug.gz
-info/disk_partitions
-# Actual test cases
-__TC-001-0002-Platform_Controller_Hub__
-TC-001-0002-001-SATA_port
-TC-001-0002-002-USB_2.0

=== modified file 'debian/changelog'
--- debian/changelog	2013-11-05 20:04:15 +0000
+++ debian/changelog	2014-02-13 07:27:19 +0000
@@ -1,3 +1,22 @@
+  [ Nelson Chu ]
+  * data/whitelists/opencompute-certify-local.whitelist - Added new jobs to
+    certification whitelist
+  * jobs/TC-001-0002-Platform_Controller_Hub.txt - Added new jobs for PCH
+    test cases
+  * jobs/local.txt.in - Added job to parse new PCH job file
+  * scripts/check_sata_port - new script to verify SATA port speed
+  * scripts/check_usb_port - new script to verify USB version
+
+  [ Nelson Chu ]
+  * data/whitelists/opencompute-certify-local.whitelist - Added new jobs to
+    certification whitelist
+  * jobs/TC-001-0001-CPU_Memory.txt.in - Added new jobs for CPU and Memory
+    test cases
+  * jobs/local.txt.in - Added job to parse new CPU and Memory job file
+  * scripts/cpu_info - new script to gather CPU information
+  * scripts/memory_info - new script to gather memory information
+  * scripts/processor_topology - Revised script to match certification criteria
+
 checkbox (1.16.13~OCP) UNRELEASED; urgency=low
 
   [ Jeff Marcom ]

=== added file 'jobs/TC-001-0001-CPU_Memory.txt.in'
--- jobs/TC-001-0001-CPU_Memory.txt.in	1970-01-01 00:00:00 +0000
+++ jobs/TC-001-0001-CPU_Memory.txt.in	2014-02-13 07:27:19 +0000
@@ -0,0 +1,31 @@
+plugin: shell
+name: TC-001-0001-001-CPU_Information
+requires: package.name == 'lshw'
+user: root
+command: cpu_info -p Xeon -f E5
+description:
+ 1. Use lshw command to gather CPU information.
+ 2. The program will output CPU model and L1, L2, L3 cache size.
+ 3. Criteria: CPU model must be Intel Xeon processor E5-2600 product family and L3 cache size must be up to 20MB.
+
+plugin: shell
+name: TC-001-0001-002-Processor_Topology
+command: processor_topology
+description:
+ 1. This test checks CPU topology for accuracy.
+ 2. Use lscpu command to gather CPU information.
+ 3. The program will output the total number of CPUs, the number of threads per core, the number of cores per socket, and the number of sockets.
+ 4. Criteria: It should be 8-12 cores per CPU and 2 threads per core.
+
+plugin: shell
+name: TC-001-0001-003-Memory_Information
+requires: package.name == 'lshw'
+user: root
+command: memory_info
+description:
+ 1. Use lshw command to gather memory information.
+ 2. Testing prerequisites:
+    4 channels DDR3 registered memory interface on each processor 0 and processor 1.
+    2 DDR3 slots per channel per processor. (total of 16 DIMMs on the motherboard)
+ 3. The program will output memory module, vendor, size and slot.
+ 4. Criteria: Total of 16 DIMMs on the motherboard.

=== renamed file 'jobs/TC-001-0002-Platform_Controller_Hub.txt' => 'jobs/TC-001-0002-Platform_Controller_Hub.txt.in'
=== modified file 'jobs/local.txt.in'
--- jobs/local.txt.in	2014-02-11 12:16:41 +0000
+++ jobs/local.txt.in	2014-02-13 07:27:19 +0000
@@ -110,6 +110,13 @@
  shopt -s extglob
  cat $CHECKBOX_SHARE/jobs/sniff.txt?(.in)
 
+name: __TC-001-0001-CPU_Memory__
+plugin: local
+_description: Verify CPU and memory
+command:
+ shopt -s extglob
+ cat $CHECKBOX_SHARE/jobs/TC-001-0001-CPU_Memory.txt?(.in)
+
 name: __TC-001-0002-Platform_Controller_Hub__
 plugin: local
 _description: Verify platform controller hub functionality

=== added file 'scripts/cpu_info'
--- scripts/cpu_info	1970-01-01 00:00:00 +0000
+++ scripts/cpu_info	2014-02-13 07:27:19 +0000
@@ -0,0 +1,97 @@
+#!/usr/bin/env python3
+"""
+Copyright (C) 2010-2013 by Cloud Computing Center for Mobile Applications 
+Industrial Technology Research Institute
+
+cpu_info
+  Use lshw command to gather CPU information.
+  The program will output CPU model and L1, L2, L3 cache size.
+  Criteria: CPU model and product family must match user's input
+            and L3 cache should be larger than 20MB.
+
+Authors
+  Nelson Chu <[e-mail]>
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License version 3,
+as published by the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""
+
+import os
+import re
+import sys
+import xml.etree.ElementTree as ET
+from subprocess import check_output
+from argparse import ArgumentParser, RawTextHelpFormatter
+
+def run(product, family):
+    command = "lshw -xml"
+    with open(os.devnull, 'w') as NULL:
+       hwinfo_xml = check_output(command, stderr=NULL, shell=True)
+    root = ET.fromstring(hwinfo_xml)
+
+    # Parse lshw XML for gathering processor information.
+    processor = root.findall(".//product/..[@class='processor']")
+
+    if not processor:
+        print("Fail: Cannot parse any processor information.")
+        return 10
+
+    for cpu in processor:
+        if cpu.find('product') is None:
+            print("Fail: Cannot find processor product.")
+            return 20
+        print(cpu.find('product').text)
+        match = re.search(product + '.*' + family, cpu.find('product').text)
+        if not match:
+            print("Fail: Cannot match CPU %s %s family." %(product, family))
+            return 25
+
+        cache_list = cpu.findall(".//size/..[@class='memory']")
+        if not cache_list:
+            print("Fail: Cannot find any CPU cache.")
+            return 30
+
+        for cache in cache_list:
+            if cache.find('size') is None or cache.find('slot') is None:
+                print("Fail: Cannot access Last Level Cache (LLC).")
+                return 40
+
+            cache_size = int(cache.find('size').text) / 1024
+            if cache_size > 1024:
+                cache_size = cache_size / 1024
+                print(('%s %d MB') %(cache.find('slot').text, cache_size))
+                if re.search('L3', cache.find('slot').text):
+                    if cache_size < 20:
+                        print('Fail: L3 cache size less than 20MB.')
+                        return 50
+            else:
+                print(('%s %d KB') %(cache.find('slot').text, cache_size))
+
+    return 0
+
+def main():
+    parser = ArgumentParser(formatter_class=RawTextHelpFormatter)
+
+    parser.add_argument('-p', '--product', type=str, required=True,
+                        help=("The CPU product name. [Example: Xeon]"))
+    parser.add_argument('-f', '--family', type=str, required=True,
+                        help=("Processor family. [Example: E5]"))
+
+    args = parser.parse_args()
+
+    product = args.product.title()
+    family = args.family.title()
+    return run(product, family)
+
+if __name__ == '__main__':
+    sys.exit(main())

=== added file 'scripts/memory_info'
--- scripts/memory_info	1970-01-01 00:00:00 +0000
+++ scripts/memory_info	2014-02-13 07:27:19 +0000
@@ -0,0 +1,77 @@
+#!/usr/bin/env python3
+"""
+Copyright (C) 2010-2013 by Cloud Computing Center for Mobile Applications
+Industrial Technology Research Institute
+
+memory_info
+  1. Use lshw command to gather memory information.
+  2. Testing prerequisites:
+     4 channels DDR3 registered memory interface on each processor 0 
+     and processor 1.
+     2 DDR3 slots per channel per processor. (total of 16 DIMMs 
+                                              on the motherboard)
+  3. The program will output memory module, vendor, size and slot.
+  4. Criteria: Total of 16 DIMMs on the motherboard.
+
+Authors
+  Nelson Chu <[e-mail]>
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU General Public License version 3,
+as published by the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""
+
+import sys
+import xml.etree.ElementTree as ET
+from subprocess import Popen, PIPE
+
+def main():
+    attribute = ['description', 'vendor', 'slot', 'size']
+    command = 'lshw -xml'
+    hwinfo_xml = Popen(command, stdout=PIPE, stderr=PIPE,
+                       shell=True).communicate()[0]
+    root = ET.fromstring(hwinfo_xml)
+
+    # Parse lshw XML for gathering memory information.
+    memory_list = root.findall(".//clock/..[@class='memory']")
+
+    if not memory_list:
+        print("Fail: Cannot parse any memory information.")
+        return 10
+
+    count = 0
+    for dimm in memory_list:
+        count = count +1
+        for attr in attribute:
+            if dimm.find(attr) is None:
+                print(("Fail: Cannot find memory %s") %attr)
+                return 20
+
+        memory_size = int(dimm.find('size').text) / (1024**3)
+        for attr in attribute:
+            if attr == 'description':
+                print('%s' %(dimm.find(attr).text), end=" ")
+                continue
+            elif attr == 'size':
+                print('%s=%dGB.' %(attr, memory_size))
+                continue
+            print('%s=%s' %(attr, dimm.find(attr).text), end=" ")
+
+    print("Total number of DIMMs is %s." %(count))
+    if count != 16:
+        print("Fail: Memory DIMM number is not meet the requirement.")
+        return 30
+
+    return 0
+
+if __name__ == '__main__':
+    sys.exit(main())

=== added file 'scripts/processor_topology'
--- scripts/processor_topology	1970-01-01 00:00:00 +0000
+++ scripts/processor_topology	2014-02-13 07:27:19 +0000
@@ -0,0 +1,130 @@
+#!/usr/bin/env python3
+'''
+cpu_topology
+Written by Jeffrey Lane <[e-mail]>
+'''
+import sys
+import os
+from subprocess import check_output
+
+class proc_cpuinfo():
+    '''
+    Class to get and handle information from /proc/cpuinfo
+    Creates a dictionary of data gleaned from that file.
+    '''
+    def __init__(self):
+        self.cpuinfo = {}
+        cpu_fh = open('/proc/cpuinfo', 'r')
+        try:
+            temp = cpu_fh.readlines()
+        finally:
+            cpu_fh.close()
+
+        for i in temp:
+            if i.startswith('processor'):
+                key = 'cpu' + (i.split(':')[1].strip())
+                self.cpuinfo[key] = {'core_id':'', 'physical_package_id':''}
+            elif i.startswith('core id'):
+                self.cpuinfo[key].update({'core_id': i.split(':')[1].strip()})
+            elif i.startswith('physical id'):
+                self.cpuinfo[key].update({'physical_package_id':
+                                          i.split(':')[1].strip()})
+            else:
+                continue
+
+
+class sysfs_cpu():
+    '''
+    Class to get and handle information from sysfs as relates to CPU topology
+    Creates an informational class to present information on various CPUs
+    '''
+
+    def __init__(self, proc):
+        self.syscpu = {}
+        self.path = '/sys/devices/system/cpu/' + proc + '/topology'
+        items = ['core_id', 'physical_package_id']
+        for i in items:
+            syscpu_fh = open(os.path.join(self.path, i), 'r')
+            try:
+                self.syscpu[i] = syscpu_fh.readline().strip()
+            finally:
+                syscpu_fh.close()
+
+
+def compare(proc_cpu, sys_cpu):
+    cpu_map = {}
+    '''
+    If there is only 1 CPU the test don't look for core_id
+    and physical_package_id because those information are absent in
+    /proc/cpuinfo on singlecore system
+    '''
+    for key in proc_cpu.keys():
+        if 'cpu1' not in proc_cpu:
+            cpu_map[key] = True
+        else:
+            for subkey in proc_cpu[key].keys():
+                if proc_cpu[key][subkey] == sys_cpu[key][subkey]:
+                    cpu_map[key] = True
+                else:
+                    cpu_map[key] = False
+    return cpu_map
+
+
+def main():
+    cpuinfo = proc_cpuinfo()
+    sys_cpu = {}
+    keys = cpuinfo.cpuinfo.keys()
+    for k in keys:
+        sys_cpu[k] = sysfs_cpu(k).syscpu
+    cpu_map = compare(cpuinfo.cpuinfo, sys_cpu)
+    if False in cpu_map.values() or len(cpu_map) < 1:
+        print("FAIL: CPU Topology is incorrect", file=sys.stderr)
+        print("-" * 52, file=sys.stderr)
+        print("{0}{1}".format("/proc/cpuinfo".center(30), "sysfs".center(25)),
+                file=sys.stderr)
+        print("{0}{1}{2}{3}{1}{2}".format(
+                "CPU".center(6),
+                "Physical ID".center(13),
+                "Core ID".center(9),
+                "|".center(3)), file=sys.stderr)
+        for key in sorted(sys_cpu.keys()):
+            print("{0}{1}{2}{3}{4}{5}".format(
+                  key.center(6),
+                  cpuinfo.cpuinfo[key]['physical_package_id'].center(13),
+                  cpuinfo.cpuinfo[key]['core_id'].center(9),
+                  "|".center(3),
+                  sys_cpu[key]['physical_package_id'].center(13),
+                  sys_cpu[key]['core_id'].center(9)), file=sys.stderr)
+        return 1
+    else:
+        # Use lscpu command to gather CPU information.
+        # Output the total number of CPUs, the number of threads per core, 
+        # the number of cores per socket, and the number of sockets.
+        # Criteria: It must be 8-12 cores per CPU and 2 threads per core.
+        # Revised by Nelson Chu <[e-mail]>
+        command = 'lscpu'
+        return_code = 0
+
+        with open(os.devnull, "w") as NULL:
+            cpu_info = check_output(command, stderr=NULL, shell=True)
+
+        cpu_info = cpu_info.decode('utf-8')
+
+        for cpu in cpu_info.split('\n'):
+            if cpu.startswith("CPU(s)"):
+                print(cpu)
+            if cpu.startswith("Thread(s) per core"):
+                print(cpu)
+            if cpu.startswith("Core(s) per socket"):
+                cores = int(cpu.split(":")[1])
+                print(cpu)
+                # It should be 8-12 cores per CPU.
+                if cores < 8 or cores > 12:
+                    return_code = 1
+            if cpu.startswith("Socket(s)"):
+                print(cpu)
+
+        return return_code
+
+if __name__ == '__main__':
+    sys.exit(main())