From 93d9d640380252084855885ad27873b4377898ec Mon Sep 17 00:00:00 2001 From: Yaakov Selkowitz Date: Fri, 9 Aug 2019 13:01:29 -0400 Subject: [PATCH] facts: fix double-counting of CPUs on POWER systems (#58360) On POWER systems, /proc/cpuinfo provides a 'processor' entry as a counter, and a 'cpu' entry with a description (similar to 'model name' on x86). Support for POWER in get_cpu_facts was added via the 'cpu' entry in commit 8746e692c121d7081e5c545de8e98908a95b510b. Subsequent support for ARM64 in commit ce4ada93f9daae4e1806c612b5c2f96727dc3de5 used the 'processor' entry, resulting in double-counting of cores on POWER systems. When unit tests were later written for this code in commit 55306906cf7577cc5a666bf6f3d0486dcf937c03, the erroneous values were just accepted in the test instead of being diagnosed. Signed-off-by: Yaakov Selkowitz --- lib/ansible/module_utils/facts/hardware/linux.py | 5 +++-- test/units/module_utils/facts/hardware/linux_data.py | 8 ++++---- .../facts/hardware/test_linux_get_cpu_info.py | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/ansible/module_utils/facts/hardware/linux.py b/lib/ansible/module_utils/facts/hardware/linux.py index befc2fb5e70..503a6e3b73c 100644 --- a/lib/ansible/module_utils/facts/hardware/linux.py +++ b/lib/ansible/module_utils/facts/hardware/linux.py @@ -242,8 +242,9 @@ class LinuxHardware(Hardware): # The fields for ARM CPUs do not always include 'vendor_id' or 'model name', # and sometimes includes both 'processor' and 'Processor'. - # Always use 'processor' count for ARM systems - if collected_facts.get('ansible_architecture', '').startswith(('armv', 'aarch')): + # The fields for Power CPUs include 'processor' and 'cpu'. + # Always use 'processor' count for ARM and Power systems + if collected_facts.get('ansible_architecture', '').startswith(('armv', 'aarch', 'ppc')): i = processor_occurence # FIXME diff --git a/test/units/module_utils/facts/hardware/linux_data.py b/test/units/module_utils/facts/hardware/linux_data.py index ba2e528d7ac..05dc0e6513c 100644 --- a/test/units/module_utils/facts/hardware/linux_data.py +++ b/test/units/module_utils/facts/hardware/linux_data.py @@ -495,9 +495,9 @@ CPU_INFO_TEST_SCENARIOS = [ '7', 'POWER7 (architected), altivec supported' ], 'processor_cores': 1, - 'processor_count': 16, + 'processor_count': 8, 'processor_threads_per_core': 1, - 'processor_vcpus': 16 + 'processor_vcpus': 8 }, }, { @@ -531,9 +531,9 @@ CPU_INFO_TEST_SCENARIOS = [ '23', 'POWER8 (architected), altivec supported', ], 'processor_cores': 1, - 'processor_count': 48, + 'processor_count': 24, 'processor_threads_per_core': 1, - 'processor_vcpus': 48 + 'processor_vcpus': 24 }, }, { diff --git a/test/units/module_utils/facts/hardware/test_linux_get_cpu_info.py b/test/units/module_utils/facts/hardware/test_linux_get_cpu_info.py index 542ed4bce2d..cda251e4ea9 100644 --- a/test/units/module_utils/facts/hardware/test_linux_get_cpu_info.py +++ b/test/units/module_utils/facts/hardware/test_linux_get_cpu_info.py @@ -26,13 +26,13 @@ def test_get_cpu_info_missing_arch(mocker): module = mocker.Mock() inst = linux.LinuxHardware(module) - # ARM will report incorrect processor count if architecture is not available + # ARM and Power will report incorrect processor count if architecture is not available mocker.patch('os.path.exists', return_value=False) mocker.patch('os.access', return_value=True) for test in CPU_INFO_TEST_SCENARIOS: mocker.patch('ansible.module_utils.facts.hardware.linux.get_file_lines', side_effect=[[], test['cpuinfo']]) test_result = inst.get_cpu_facts() - if test['architecture'].startswith(('armv', 'aarch')): + if test['architecture'].startswith(('armv', 'aarch', 'ppc')): assert test['expected_result'] != test_result else: assert test['expected_result'] == test_result