From 090e5ea774608b99dab7a1567d44c0eb972dada4 Mon Sep 17 00:00:00 2001
From: Luke Meyer <lmeyer@redhat.com>
Date: Fri, 26 May 2017 15:48:10 -0400
Subject: memory check: use GiB/MiB and adjust memtotal

fixes https://bugzilla.redhat.com/show_bug.cgi?id=1455884

Various things reserve memory such that memtotal is quite lower than the
actual physical RAM of the system. It's larger as RAM increases but it's
not really proportional so I just added a flat 1GiB adjustment in the
comparison. This ought to "pass when it's close enough."
---
 .../openshift_checks/memory_availability.py        | 25 ++++++++++-------
 .../test/memory_availability_test.py               | 31 +++++++++++++++-------
 2 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/roles/openshift_health_checker/openshift_checks/memory_availability.py b/roles/openshift_health_checker/openshift_checks/memory_availability.py
index 8b1a58ef4..f4e31065f 100644
--- a/roles/openshift_health_checker/openshift_checks/memory_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/memory_availability.py
@@ -1,6 +1,9 @@
 # pylint: disable=missing-docstring
 from openshift_checks import OpenShiftCheck, get_var
 
+MIB = 2**20
+GIB = 2**30
+
 
 class MemoryAvailability(OpenShiftCheck):
     """Check that recommended memory is available."""
@@ -11,10 +14,12 @@ class MemoryAvailability(OpenShiftCheck):
     # Values taken from the official installation documentation:
     # https://docs.openshift.org/latest/install_config/install/prerequisites.html#system-requirements
     recommended_memory_bytes = {
-        "masters": 16 * 10**9,
-        "nodes": 8 * 10**9,
-        "etcd": 8 * 10**9,
+        "masters": 16 * GIB,
+        "nodes": 8 * GIB,
+        "etcd": 8 * GIB,
     }
+    # https://access.redhat.com/solutions/3006511 physical RAM is partly reserved from memtotal
+    memtotal_adjustment = 1 * GIB
 
     @classmethod
     def is_active(cls, task_vars):
@@ -25,21 +30,21 @@ class MemoryAvailability(OpenShiftCheck):
 
     def run(self, tmp, task_vars):
         group_names = get_var(task_vars, "group_names")
-        total_memory_bytes = get_var(task_vars, "ansible_memtotal_mb") * 10**6
+        total_memory_bytes = get_var(task_vars, "ansible_memtotal_mb") * MIB
 
         recommended_min = max(self.recommended_memory_bytes.get(name, 0) for name in group_names)
-        configured_min = int(get_var(task_vars, "openshift_check_min_host_memory_gb", default=0)) * 10**9
+        configured_min = float(get_var(task_vars, "openshift_check_min_host_memory_gb", default=0)) * GIB
         min_memory_bytes = configured_min or recommended_min
 
-        if total_memory_bytes < min_memory_bytes:
+        if total_memory_bytes + self.memtotal_adjustment < min_memory_bytes:
             return {
                 'failed': True,
                 'msg': (
-                    'Available memory ({available:.1f} GB) '
-                    'below recommended value ({recommended:.1f} GB)'
+                    'Available memory ({available:.1f} GiB) is too far '
+                    'below recommended value ({recommended:.1f} GiB)'
                 ).format(
-                    available=float(total_memory_bytes) / 10**9,
-                    recommended=float(min_memory_bytes) / 10**9,
+                    available=float(total_memory_bytes) / GIB,
+                    recommended=float(min_memory_bytes) / GIB,
                 ),
             }
 
diff --git a/roles/openshift_health_checker/test/memory_availability_test.py b/roles/openshift_health_checker/test/memory_availability_test.py
index 1db203854..4fbaea0a9 100644
--- a/roles/openshift_health_checker/test/memory_availability_test.py
+++ b/roles/openshift_health_checker/test/memory_availability_test.py
@@ -36,6 +36,11 @@ def test_is_active(group_names, is_active):
         1,  # configure lower threshold
         2000,  # too low for recommended but not for configured
     ),
+    (
+        ['nodes'],
+        2,  # configure threshold where adjustment pushes it over
+        1900,
+    ),
     (
         ['etcd'],
         0,
@@ -65,38 +70,44 @@ def test_succeeds_with_recommended_memory(group_names, configured_min, ansible_m
         ['masters'],
         0,
         0,
-        ['0.0 GB'],
+        ['0.0 GiB'],
     ),
     (
         ['nodes'],
         0,
         100,
-        ['0.1 GB'],
+        ['0.1 GiB'],
+    ),
+    (
+        ['nodes'],
+        24,  # configure higher threshold
+        20 * 1024,  # enough to meet recommended but not configured
+        ['20.0 GiB'],
     ),
     (
         ['nodes'],
         24,  # configure higher threshold
-        20000,  # enough to meet recommended but not configured
-        ['20.0 GB'],
+        22 * 1024,  # not enough for adjustment to push over threshold
+        ['22.0 GiB'],
     ),
     (
         ['etcd'],
         0,
-        7000,
-        ['7.0 GB'],
+        6 * 1024,
+        ['6.0 GiB'],
     ),
     (
         ['etcd', 'masters'],
         0,
-        9000,  # enough memory for etcd, not enough for a master
-        ['9.0 GB'],
+        9 * 1024,  # enough memory for etcd, not enough for a master
+        ['9.0 GiB'],
     ),
     (
         ['nodes', 'masters'],
         0,
         # enough memory for a node, not enough for a master
-        11000,
-        ['11.0 GB'],
+        11 * 1024,
+        ['11.0 GiB'],
     ),
 ])
 def test_fails_with_insufficient_memory(group_names, configured_min, ansible_memtotal_mb, extra_words):
-- 
cgit v1.2.3