From 0c3a273805a53a42ccc79ffd26a8b41ac13a860d Mon Sep 17 00:00:00 2001 From: Patrick McConnell Date: Thu, 29 Jan 2015 15:00:01 +0100 Subject: [PATCH 1/3] Fix for memory fact gathering I have a host which started to fail while gathering facts after the addition of expanded memory facts in PR #9839: Traceback (most recent call last): File "/home/ansible/.ansible/tmp/ansible-tmp-1422536976.05-133253824703289/setup", line 4278, in main() File "/home/ansible/.ansible/tmp/ansible-tmp-1422536976.05-133253824703289/setup", line 137, in main data = run_setup(module) File "/home/ansible/.ansible/tmp/ansible-tmp-1422536976.05-133253824703289/setup", line 81, in run_setup facts = ansible_facts(module) File "/home/ansible/.ansible/tmp/ansible-tmp-1422536976.05-133253824703289/setup", line 4217, in ansible_facts facts.update(Hardware().populate()) File "/home/ansible/.ansible/tmp/ansible-tmp-1422536976.05-133253824703289/setup", line 2339, in populate self.get_memory_facts() File "/home/ansible/.ansible/tmp/ansible-tmp-1422536976.05-133253824703289/setup", line 2375, in get_memory_facts 'cached': memstats['swapcached'] KeyError: 'swapcached' My problem host doesn't have SwapCached in /proc/meminfo. It may be better to set defaults for these keys, since the values provided by /proc/meminfo can change from version to version. --- lib/ansible/module_utils/facts.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/ansible/module_utils/facts.py b/lib/ansible/module_utils/facts.py index 0ce3ced507..423a603be8 100644 --- a/lib/ansible/module_utils/facts.py +++ b/lib/ansible/module_utils/facts.py @@ -635,19 +635,19 @@ class LinuxHardware(Hardware): memstats[key.lower()] = long(val) / 1024 self.facts['memory_mb'] = { 'real' : { - 'total': memstats['memtotal'], - 'used': (memstats['memtotal'] - memstats['memfree']), - 'free': memstats['memfree'] + 'total': memstats.get('memtotal', 0), + 'used': (memstats.get('memtotal', 0) - memstats.get('memfree', 0)), + 'free': memstats.get('memfree', 0) }, 'nocache' : { - 'free': memstats['cached'] + memstats['memfree'] + memstats['buffers'], - 'used': memstats['memtotal'] - (memstats['cached'] + memstats['memfree'] + memstats['buffers']) + 'free': memstats.get('cached', 0) + memstats.get('memfree', 0) + memstats.get('buffers', 0), + 'used': memstats.get('memtotal', 0) - (memstats.get('cached', 0) + memstats.get('memfree', 0) + memstats.get('buffers', 0)) }, 'swap' : { - 'total': memstats['swaptotal'], - 'free': memstats['swapfree'], - 'used': memstats['swaptotal'] - memstats['swapfree'], - 'cached': memstats['swapcached'] + 'total': memstats.get('swaptotal', 0), + 'free': memstats.get('swapfree', 0), + 'used': memstats.get('swaptotal', 0) - memstats.get('swapfree', 0), + 'cached': memstats.get('swapcached', 0) } } From f20967078ebaa5c5bcb88e6ad12d50da28f761b5 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 29 Jan 2015 12:09:19 -0800 Subject: [PATCH 2/3] Fixes to @RadishTheHut's memory facts as discussed in https://github.com/ansible/ansible/pull/10129#issuecomment-72077500 * Switch default value from 0 to None. * Prefill keys with default value so that determining calculated values is easier --- lib/ansible/module_utils/facts.py | 47 +++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/lib/ansible/module_utils/facts.py b/lib/ansible/module_utils/facts.py index 423a603be8..7340e665c2 100644 --- a/lib/ansible/module_utils/facts.py +++ b/lib/ansible/module_utils/facts.py @@ -602,9 +602,11 @@ class LinuxHardware(Hardware): """ platform = 'Linux' - MEMORY_FACTS = ['MemTotal', 'SwapTotal', 'MemFree', 'SwapFree'] - EXTRA_MEMORY_FACTS = ['Buffers', 'Cached', 'SwapCached'] + # Originally only had these four as toplevelfacts + ORIGINAL_MEMORY_FACTS = frozenset(('MemTotal', 'SwapTotal', 'MemFree', 'SwapFree')) + # Now we have all of these in a dict structure + MEMORY_FACTS = ORIGINAL_MEMORY_FACTS.union(('Buffers', 'Cached', 'SwapCached')) def __init__(self): Hardware.__init__(self) @@ -621,34 +623,49 @@ class LinuxHardware(Hardware): return self.facts def get_memory_facts(self): - memstats = {} if not os.access("/proc/meminfo", os.R_OK): return + + # No defaultdict in py2.4 + memstat_keys = self.MEMORY_FACTS.union( + ('real:used', 'nocache:free', 'nocache:used', 'swap:used')) + memstats = dict(zip(memstat_keys, (None,) * len(memstat_keys))) for line in open("/proc/meminfo").readlines(): data = line.split(":", 1) key = data[0] - if key in LinuxHardware.MEMORY_FACTS: + if key in self.ORIGINAL_MEMORY_FACTS: val = data[1].strip().split(' ')[0] self.facts["%s_mb" % key.lower()] = long(val) / 1024 - if key in LinuxHardware.MEMORY_FACTS or key in LinuxHardware.EXTRA_MEMORY_FACTS: + + if key in self.MEMORY_FACTS: val = data[1].strip().split(' ')[0] memstats[key.lower()] = long(val) / 1024 + + if None not in (memstats['memtotal'], memstats['memfree']): + memstats['real:used'] = memstats['memtotal'] - memstats['memfree'] + if None not in (memstats['cached'], memstats['memfree'], memstats['buffers']): + memstats['nocache:free'] = memstats['cached'] + memstats['memfree'] + memstats['buffers'] + if None not in (memstats['memtotal'], memstats['nocache:free']): + memstats['nocache:used'] = memstats['memtotal'] - memstats['nocache:free'] + if None not in (memstats['swaptotal'], memstats['swapfree']): + memstats['swap:used'] = memstats['swaptotal'] - memstats['swapfree'] + self.facts['memory_mb'] = { 'real' : { - 'total': memstats.get('memtotal', 0), - 'used': (memstats.get('memtotal', 0) - memstats.get('memfree', 0)), - 'free': memstats.get('memfree', 0) + 'total': memstats['memtotal'], + 'used': memstats['real:used'], + 'free': memstats['memfree'], }, 'nocache' : { - 'free': memstats.get('cached', 0) + memstats.get('memfree', 0) + memstats.get('buffers', 0), - 'used': memstats.get('memtotal', 0) - (memstats.get('cached', 0) + memstats.get('memfree', 0) + memstats.get('buffers', 0)) + 'free': memstats['nocache:free'], + 'used': memstats['nocache:used'], }, 'swap' : { - 'total': memstats.get('swaptotal', 0), - 'free': memstats.get('swapfree', 0), - 'used': memstats.get('swaptotal', 0) - memstats.get('swapfree', 0), - 'cached': memstats.get('swapcached', 0) - } + 'total': memstats['swaptotal'], + 'free': memstats['swapfree'], + 'used': memstats['swap:used'], + 'cached': memstats['swapcached'], + }, } def get_cpu_facts(self): From 04b2c698baa0024cbac79ce079af0a12239368ae Mon Sep 17 00:00:00 2001 From: Patrick McConnell Date: Fri, 30 Jan 2015 05:56:41 +0100 Subject: [PATCH 3/3] Updated memory facts fix using dict.get() to avoid KeyError --- lib/ansible/module_utils/facts.py | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/ansible/module_utils/facts.py b/lib/ansible/module_utils/facts.py index 7340e665c2..1995d21ed8 100644 --- a/lib/ansible/module_utils/facts.py +++ b/lib/ansible/module_utils/facts.py @@ -626,10 +626,7 @@ class LinuxHardware(Hardware): if not os.access("/proc/meminfo", os.R_OK): return - # No defaultdict in py2.4 - memstat_keys = self.MEMORY_FACTS.union( - ('real:used', 'nocache:free', 'nocache:used', 'swap:used')) - memstats = dict(zip(memstat_keys, (None,) * len(memstat_keys))) + memstats = {} for line in open("/proc/meminfo").readlines(): data = line.split(":", 1) key = data[0] @@ -641,30 +638,30 @@ class LinuxHardware(Hardware): val = data[1].strip().split(' ')[0] memstats[key.lower()] = long(val) / 1024 - if None not in (memstats['memtotal'], memstats['memfree']): + if None not in (memstats.get('memtotal'), memstats.get('memfree')): memstats['real:used'] = memstats['memtotal'] - memstats['memfree'] - if None not in (memstats['cached'], memstats['memfree'], memstats['buffers']): + if None not in (memstats.get('cached'), memstats.get('memfree'), memstats.get('buffers')): memstats['nocache:free'] = memstats['cached'] + memstats['memfree'] + memstats['buffers'] - if None not in (memstats['memtotal'], memstats['nocache:free']): + if None not in (memstats.get('memtotal'), memstats.get('nocache:free')): memstats['nocache:used'] = memstats['memtotal'] - memstats['nocache:free'] - if None not in (memstats['swaptotal'], memstats['swapfree']): + if None not in (memstats.get('swaptotal'), memstats.get('swapfree')): memstats['swap:used'] = memstats['swaptotal'] - memstats['swapfree'] self.facts['memory_mb'] = { 'real' : { - 'total': memstats['memtotal'], - 'used': memstats['real:used'], - 'free': memstats['memfree'], + 'total': memstats.get('memtotal'), + 'used': memstats.get('real:used'), + 'free': memstats.get('memfree'), }, 'nocache' : { - 'free': memstats['nocache:free'], - 'used': memstats['nocache:used'], + 'free': memstats.get('nocache:free'), + 'used': memstats.get('nocache:used'), }, 'swap' : { - 'total': memstats['swaptotal'], - 'free': memstats['swapfree'], - 'used': memstats['swap:used'], - 'cached': memstats['swapcached'], + 'total': memstats.get('swaptotal'), + 'free': memstats.get('swapfree'), + 'used': memstats.get('swap:used'), + 'cached': memstats.get('swapcached'), }, }