Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Facts: Use vm_stat instead of sysctl for free memory #52917

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
5 participants
@ssbarnea
Copy link
Contributor

ssbarnea commented Feb 25, 2019

SUMMARY

Fixes #52896
Signed-off-by: Abhijeet Kasurde akasurde@redhat.com

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/module_utils/facts/hardware/darwin.py

@ansibot

This comment has been minimized.

@Akasurde

This comment has been minimized.

Copy link
Member

Akasurde commented Mar 10, 2019

cc @samdoran Can we merge this if you are OK with the changes ? Thanks.

@ssbarnea

This comment has been minimized.

Copy link
Contributor Author

ssbarnea commented Mar 11, 2019

ready_for_review
@samdoran @bcoca can you help?

@ssbarnea

This comment has been minimized.

Copy link
Contributor Author

ssbarnea commented Mar 11, 2019

@Akasurde I have being told that we also need a changelog fragment for this in order to be merged and we need to move quickly to get into Thursday release.

I think is likely to be looking like:

bugfixes:
  - setup - fixed free memory detection on MacOS

More info at https://docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html#changelogs-how-to

I would have tryied to add the fragment file myself but I have no push rights to your branch, the one used to create the PR.

@samdoran
Copy link
Member

samdoran left a comment

Rather than use a regexp for parsing the command output, we can make the output into a dict and get the keys we want:

diff --git a/lib/ansible/module_utils/facts/hardware/darwin.py b/lib/ansible/module_utils/facts/hardware/darwin.py
index 065a4d5cf7..280d640410 100644
--- a/lib/ansible/module_utils/facts/hardware/darwin.py
+++ b/lib/ansible/module_utils/facts/hardware/darwin.py
@@ -18,7 +18,7 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 import re
-
+from ansible.module_utils.common.process import get_bin_path
 from ansible.module_utils.facts.hardware.base import Hardware, HardwareCollector
 from ansible.module_utils.facts.sysctl import get_sysctl
 
@@ -90,18 +90,30 @@ class DarwinHardware(Hardware):
         }
 
         total_used = 0
-        rc, out, err = self.module.run_command("/usr/bin/vm_stat")
+        vm_stat_command = get_bin_path('vm_stat', True)
+        rc, out, err = self.module.run_command(vm_stat_command)
         if rc == 0:
-            # Free = Total - (Wired + active + inactive)
-            wired_mem_mb = re.search(r'Pages wired down:\s+(\d+)\.', out)
-            active_mem_mb = re.search(r'Pages active:\s+(\d+)\.', out)
-            inactive_mem_mb = re.search(r'Pages inactive:\s+(\d+)\.', out)
-            if wired_mem_mb:
-                total_used += int(wired_mem_mb.group(1)) * 4096
-            if active_mem_mb:
-                total_used += int(active_mem_mb.group(1)) * 4096
-            if inactive_mem_mb:
-                total_used += int(inactive_mem_mb.group(1)) * 4096
+            # Get a generator of tuples from the command output so we can later
+            # turn it into a dictionary
+            memory_stats = (line.rstrip('.').split(':', 1) for line in out.splitlines())
+
+            # Strip extra left spaces from the value
+            memory_stats = dict((k, v.lstrip()) for k, v in memory_stats)
+
+            for k, v in memory_stats.items():
+                try:
+                    memory_stats[k] = int(v)
+                except ValueError as ve:
+                    # Most values convert cleanly to integors but if the field does
+                    # not convert to an integer, just leave it alone.
+                    pass
+
+            if memory_stats.get('Pages wired down'):
+                total_used += memory_stats['Pages wired down'] * 4096
+            if memory_stats.get('Pages active'):
+                total_used += memory_stats['Pages active'] * 4096
+            if memory_stats.get('Pages inactive'):
+                total_used += memory_stats['Pages inactive'] * 4096
 
         memory_facts['memfree_mb'] = memory_facts['memtotal_mb'] - (total_used // 1024 // 1024)
 
Show resolved Hide resolved lib/ansible/module_utils/facts/hardware/darwin.py Outdated

@Akasurde Akasurde force-pushed the Akasurde:i52896 branch from 0cf2eca to 66200f5 Mar 13, 2019

@Akasurde Akasurde requested a review from samdoran Mar 18, 2019

@ansibot ansibot added core_review and removed needs_revision labels Mar 18, 2019

Facts: Use vm_stat instead of sysctl for free memory
Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

@Akasurde Akasurde force-pushed the Akasurde:i52896 branch from 66200f5 to f35b720 Mar 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.