From 5d3f0fe02c601a9e23522f8bb3702760fcfffa02 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 (cherry picked from commit 93d9d640380252084855885ad27873b4377898ec) --- changelogs/fragments/58360-fix-cpu-count-ppc.yaml | 2 ++ 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 ++-- 4 files changed, 11 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/58360-fix-cpu-count-ppc.yaml diff --git a/changelogs/fragments/58360-fix-cpu-count-ppc.yaml b/changelogs/fragments/58360-fix-cpu-count-ppc.yaml new file mode 100644 index 00000000000..490e412ad6e --- /dev/null +++ b/changelogs/fragments/58360-fix-cpu-count-ppc.yaml @@ -0,0 +1,2 @@ +bugfixes: + - facts - fixed double-counting of CPUs on POWER systems diff --git a/lib/ansible/module_utils/facts/hardware/linux.py b/lib/ansible/module_utils/facts/hardware/linux.py index 19ca6e47999..e7626dc0cc5 100644 --- a/lib/ansible/module_utils/facts/hardware/linux.py +++ b/lib/ansible/module_utils/facts/hardware/linux.py @@ -239,8 +239,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 7455b26bd4f..175fb403d88 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