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 8746e692c1.  Subsequent
support for ARM64 in commit ce4ada93f9
used the 'processor' entry, resulting in double-counting of cores on
POWER systems.

When unit tests were later written for this code in
commit 55306906cf, the erroneous values
were just accepted in the test instead of being diagnosed.

Signed-off-by: Yaakov Selkowitz <yselkowi@redhat.com>
pull/60325/head
Yaakov Selkowitz 5 years ago committed by Sam Doran
parent 2a9964ede8
commit 93d9d64038

@ -242,8 +242,9 @@ class LinuxHardware(Hardware):
# The fields for ARM CPUs do not always include 'vendor_id' or 'model name', # The fields for ARM CPUs do not always include 'vendor_id' or 'model name',
# and sometimes includes both 'processor' and 'Processor'. # and sometimes includes both 'processor' and 'Processor'.
# Always use 'processor' count for ARM systems # The fields for Power CPUs include 'processor' and 'cpu'.
if collected_facts.get('ansible_architecture', '').startswith(('armv', 'aarch')): # Always use 'processor' count for ARM and Power systems
if collected_facts.get('ansible_architecture', '').startswith(('armv', 'aarch', 'ppc')):
i = processor_occurence i = processor_occurence
# FIXME # FIXME

@ -495,9 +495,9 @@ CPU_INFO_TEST_SCENARIOS = [
'7', 'POWER7 (architected), altivec supported' '7', 'POWER7 (architected), altivec supported'
], ],
'processor_cores': 1, 'processor_cores': 1,
'processor_count': 16, 'processor_count': 8,
'processor_threads_per_core': 1, 'processor_threads_per_core': 1,
'processor_vcpus': 16 'processor_vcpus': 8
}, },
}, },
{ {
@ -531,9 +531,9 @@ CPU_INFO_TEST_SCENARIOS = [
'23', 'POWER8 (architected), altivec supported', '23', 'POWER8 (architected), altivec supported',
], ],
'processor_cores': 1, 'processor_cores': 1,
'processor_count': 48, 'processor_count': 24,
'processor_threads_per_core': 1, 'processor_threads_per_core': 1,
'processor_vcpus': 48 'processor_vcpus': 24
}, },
}, },
{ {

@ -26,13 +26,13 @@ def test_get_cpu_info_missing_arch(mocker):
module = mocker.Mock() module = mocker.Mock()
inst = linux.LinuxHardware(module) 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.path.exists', return_value=False)
mocker.patch('os.access', return_value=True) mocker.patch('os.access', return_value=True)
for test in CPU_INFO_TEST_SCENARIOS: for test in CPU_INFO_TEST_SCENARIOS:
mocker.patch('ansible.module_utils.facts.hardware.linux.get_file_lines', side_effect=[[], test['cpuinfo']]) mocker.patch('ansible.module_utils.facts.hardware.linux.get_file_lines', side_effect=[[], test['cpuinfo']])
test_result = inst.get_cpu_facts() 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 assert test['expected_result'] != test_result
else: else:
assert test['expected_result'] == test_result assert test['expected_result'] == test_result

Loading…
Cancel
Save