summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLuke Meyer <lmeyer@redhat.com>2017-07-28 17:06:54 -0400
committerLuke Meyer <lmeyer@redhat.com>2017-08-08 15:42:28 -0400
commit3c71d009c034c4a0f795ae0fb939746aea80fbca (patch)
tree86a3b227f2322300ba23999843ac6d8bead27a61
parent566731dc4dd1aebbbf0244dc8a31eefb8cd42de5 (diff)
downloadopenshift-3c71d009c034c4a0f795ae0fb939746aea80fbca.tar.gz
openshift-3c71d009c034c4a0f795ae0fb939746aea80fbca.tar.bz2
openshift-3c71d009c034c4a0f795ae0fb939746aea80fbca.tar.xz
openshift-3c71d009c034c4a0f795ae0fb939746aea80fbca.zip
openshift_checks: refactor find_ansible_mount
Reuse the code for finding the ansible_mounts mount for a path.
-rw-r--r--roles/openshift_health_checker/openshift_checks/__init__.py25
-rw-r--r--roles/openshift_health_checker/openshift_checks/disk_availability.py33
-rw-r--r--roles/openshift_health_checker/openshift_checks/docker_storage.py22
-rw-r--r--roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py17
-rw-r--r--roles/openshift_health_checker/openshift_checks/etcd_volume.py20
-rw-r--r--roles/openshift_health_checker/test/disk_availability_test.py34
-rw-r--r--roles/openshift_health_checker/test/etcd_imagedata_size_test.py5
-rw-r--r--roles/openshift_health_checker/test/etcd_volume_test.py5
8 files changed, 73 insertions, 88 deletions
diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py
index f26008c9f..0390aa80f 100644
--- a/roles/openshift_health_checker/openshift_checks/__init__.py
+++ b/roles/openshift_health_checker/openshift_checks/__init__.py
@@ -153,6 +153,31 @@ class OpenShiftCheck(object):
components = tuple(int(x) for x in components[:2])
return components
+ def find_ansible_mount(self, path):
+ """Return the mount point for path from ansible_mounts."""
+
+ # reorganize list of mounts into dict by path
+ mount_for_path = {
+ mount['mount']: mount
+ for mount
+ in self.get_var('ansible_mounts')
+ }
+
+ # NOTE: including base cases '/' and '' to ensure the loop ends
+ mount_targets = set(mount_for_path.keys()) | {'/', ''}
+ mount_point = path
+ while mount_point not in mount_targets:
+ mount_point = os.path.dirname(mount_point)
+
+ try:
+ return mount_for_path[mount_point]
+ except KeyError:
+ known_mounts = ', '.join('"{}"'.format(mount) for mount in sorted(mount_for_path))
+ raise OpenShiftCheckException(
+ 'Unable to determine mount point for path "{}".\n'
+ 'Known mount points: {}.'.format(path, known_mounts or 'none')
+ )
+
LOADER_EXCLUDES = (
"__init__.py",
diff --git a/roles/openshift_health_checker/openshift_checks/disk_availability.py b/roles/openshift_health_checker/openshift_checks/disk_availability.py
index 39ac0e4ec..6d1dea9ce 100644
--- a/roles/openshift_health_checker/openshift_checks/disk_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/disk_availability.py
@@ -1,6 +1,5 @@
"""Check that there is enough disk space in predefined paths."""
-import os.path
import tempfile
from openshift_checks import OpenShiftCheck, OpenShiftCheckException
@@ -55,9 +54,6 @@ class DiskAvailability(OpenShiftCheck):
def run(self):
group_names = self.get_var("group_names")
- ansible_mounts = self.get_var("ansible_mounts")
- ansible_mounts = {mount['mount']: mount for mount in ansible_mounts}
-
user_config = self.get_var("openshift_check_min_host_disk_gb", default={})
try:
# For backwards-compatibility, if openshift_check_min_host_disk_gb
@@ -80,7 +76,7 @@ class DiskAvailability(OpenShiftCheck):
# not part of the official recommendation but present in the user
# configuration.
for path, recommendation in self.recommended_disk_space_bytes.items():
- free_bytes = self.free_bytes(path, ansible_mounts)
+ free_bytes = self.free_bytes(path)
recommended_bytes = max(recommendation.get(name, 0) for name in group_names)
config = user_config.get(path, {})
@@ -127,22 +123,17 @@ class DiskAvailability(OpenShiftCheck):
return {}
- @staticmethod
- def free_bytes(path, ansible_mounts):
+ def free_bytes(self, path):
"""Return the size available in path based on ansible_mounts."""
- mount_point = path
- # arbitry value to prevent an infinite loop, in the unlike case that '/'
- # is not in ansible_mounts.
- max_depth = 32
- while mount_point not in ansible_mounts and max_depth > 0:
- mount_point = os.path.dirname(mount_point)
- max_depth -= 1
-
+ mount = self.find_ansible_mount(path)
try:
- free_bytes = ansible_mounts[mount_point]['size_available']
+ return mount['size_available']
except KeyError:
- known_mounts = ', '.join('"{}"'.format(mount) for mount in sorted(ansible_mounts)) or 'none'
- msg = 'Unable to determine disk availability for "{}". Known mount points: {}.'
- raise OpenShiftCheckException(msg.format(path, known_mounts))
-
- return free_bytes
+ raise OpenShiftCheckException(
+ 'Unable to retrieve disk availability for "{path}".\n'
+ 'Ansible facts included a matching mount point for this path:\n'
+ ' {mount}\n'
+ 'however it is missing the size_available field.\n'
+ 'To investigate, you can inspect the output of `ansible -m setup <host>`'
+ ''.format(path=path, mount=mount)
+ )
diff --git a/roles/openshift_health_checker/openshift_checks/docker_storage.py b/roles/openshift_health_checker/openshift_checks/docker_storage.py
index 7ae384bd7..0558ddf14 100644
--- a/roles/openshift_health_checker/openshift_checks/docker_storage.py
+++ b/roles/openshift_health_checker/openshift_checks/docker_storage.py
@@ -1,6 +1,5 @@
"""Check Docker storage driver and usage."""
import json
-import os.path
import re
from openshift_checks import OpenShiftCheck, OpenShiftCheckException
from openshift_checks.mixins import DockerHostMixin
@@ -252,7 +251,7 @@ class DockerStorage(DockerHostMixin, OpenShiftCheck):
"msg": "Specified 'max_overlay_usage_percent' is not a percentage: {}".format(threshold),
}
- mount = self.find_ansible_mount(path, self.get_var("ansible_mounts"))
+ mount = self.find_ansible_mount(path)
try:
free_bytes = mount['size_available']
total_bytes = mount['size_total']
@@ -275,22 +274,3 @@ class DockerStorage(DockerHostMixin, OpenShiftCheck):
}
return {}
-
- # TODO(lmeyer): migrate to base class
- @staticmethod
- def find_ansible_mount(path, ansible_mounts):
- """Return the mount point for path from ansible_mounts."""
-
- mount_for_path = {mount['mount']: mount for mount in ansible_mounts}
- mount_point = path
- while mount_point not in mount_for_path:
- if mount_point in ["/", ""]: # "/" not in ansible_mounts???
- break
- mount_point = os.path.dirname(mount_point)
-
- try:
- return mount_for_path[mount_point]
- except KeyError:
- known_mounts = ', '.join('"{}"'.format(mount) for mount in sorted(mount_for_path)) or 'none'
- msg = 'Unable to determine mount point for path "{}". Known mount points: {}.'
- raise OpenShiftCheckException(msg.format(path, known_mounts))
diff --git a/roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py b/roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py
index ae8460b7e..f4296753a 100644
--- a/roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py
+++ b/roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py
@@ -2,7 +2,7 @@
Ansible module for determining if the size of OpenShift image data exceeds a specified limit in an etcd cluster.
"""
-from openshift_checks import OpenShiftCheck, OpenShiftCheckException
+from openshift_checks import OpenShiftCheck
class EtcdImageDataSize(OpenShiftCheck):
@@ -12,7 +12,7 @@ class EtcdImageDataSize(OpenShiftCheck):
tags = ["etcd"]
def run(self):
- etcd_mountpath = self._get_etcd_mountpath(self.get_var("ansible_mounts"))
+ etcd_mountpath = self.find_ansible_mount("/var/lib/etcd")
etcd_avail_diskspace = etcd_mountpath["size_available"]
etcd_total_diskspace = etcd_mountpath["size_total"]
@@ -68,18 +68,5 @@ class EtcdImageDataSize(OpenShiftCheck):
return {}
@staticmethod
- def _get_etcd_mountpath(ansible_mounts):
- valid_etcd_mount_paths = ["/var/lib/etcd", "/var/lib", "/var", "/"]
-
- mount_for_path = {mnt.get("mount"): mnt for mnt in ansible_mounts}
- for path in valid_etcd_mount_paths:
- if path in mount_for_path:
- return mount_for_path[path]
-
- paths = ', '.join(sorted(mount_for_path)) or 'none'
- msg = "Unable to determine a valid etcd mountpath. Paths mounted: {}.".format(paths)
- raise OpenShiftCheckException(msg)
-
- @staticmethod
def _to_gigabytes(byte_size):
return float(byte_size) / 10.0**9
diff --git a/roles/openshift_health_checker/openshift_checks/etcd_volume.py b/roles/openshift_health_checker/openshift_checks/etcd_volume.py
index e55d55e91..e5d93ff3f 100644
--- a/roles/openshift_health_checker/openshift_checks/etcd_volume.py
+++ b/roles/openshift_health_checker/openshift_checks/etcd_volume.py
@@ -1,6 +1,6 @@
"""A health check for OpenShift clusters."""
-from openshift_checks import OpenShiftCheck, OpenShiftCheckException
+from openshift_checks import OpenShiftCheck
class EtcdVolume(OpenShiftCheck):
@@ -11,8 +11,8 @@ class EtcdVolume(OpenShiftCheck):
# Default device usage threshold. Value should be in the range [0, 100].
default_threshold_percent = 90
- # Where to find ectd data, higher priority first.
- supported_mount_paths = ["/var/lib/etcd", "/var/lib", "/var", "/"]
+ # Where to find etcd data
+ etcd_mount_path = "/var/lib/etcd"
def is_active(self):
etcd_hosts = self.get_var("groups", "etcd", default=[]) or self.get_var("groups", "masters", default=[]) or []
@@ -20,7 +20,7 @@ class EtcdVolume(OpenShiftCheck):
return super(EtcdVolume, self).is_active() and is_etcd_host
def run(self):
- mount_info = self._etcd_mount_info()
+ mount_info = self.find_ansible_mount(self.etcd_mount_path)
available = mount_info["size_available"]
total = mount_info["size_total"]
used = total - available
@@ -41,15 +41,3 @@ class EtcdVolume(OpenShiftCheck):
return {"failed": True, "msg": msg}
return {}
-
- def _etcd_mount_info(self):
- ansible_mounts = self.get_var("ansible_mounts")
- mounts = {mnt.get("mount"): mnt for mnt in ansible_mounts}
-
- for path in self.supported_mount_paths:
- if path in mounts:
- return mounts[path]
-
- paths = ', '.join(sorted(mounts)) or 'none'
- msg = "Unable to find etcd storage mount point. Paths mounted: {}.".format(paths)
- raise OpenShiftCheckException(msg)
diff --git a/roles/openshift_health_checker/test/disk_availability_test.py b/roles/openshift_health_checker/test/disk_availability_test.py
index 5720eeacf..f4fd2dfed 100644
--- a/roles/openshift_health_checker/test/disk_availability_test.py
+++ b/roles/openshift_health_checker/test/disk_availability_test.py
@@ -20,12 +20,24 @@ def test_is_active(group_names, is_active):
assert DiskAvailability(None, task_vars).is_active() == is_active
-@pytest.mark.parametrize('ansible_mounts,extra_words', [
- ([], ['none']), # empty ansible_mounts
- ([{'mount': '/mnt'}], ['/mnt']), # missing relevant mount paths
- ([{'mount': '/var'}], ['/var']), # missing size_available
+@pytest.mark.parametrize('desc, ansible_mounts, expect_chunks', [
+ (
+ 'empty ansible_mounts',
+ [],
+ ['determine mount point', 'none'],
+ ),
+ (
+ 'missing relevant mount paths',
+ [{'mount': '/mnt'}],
+ ['determine mount point', '/mnt'],
+ ),
+ (
+ 'missing size_available',
+ [{'mount': '/var'}, {'mount': '/usr'}, {'mount': '/tmp'}],
+ ['missing', 'size_available'],
+ ),
])
-def test_cannot_determine_available_disk(ansible_mounts, extra_words):
+def test_cannot_determine_available_disk(desc, ansible_mounts, expect_chunks):
task_vars = dict(
group_names=['masters'],
ansible_mounts=ansible_mounts,
@@ -34,8 +46,8 @@ def test_cannot_determine_available_disk(ansible_mounts, extra_words):
with pytest.raises(OpenShiftCheckException) as excinfo:
DiskAvailability(fake_execute_module, task_vars).run()
- for word in 'determine disk availability'.split() + extra_words:
- assert word in str(excinfo.value)
+ for chunk in expect_chunks:
+ assert chunk in str(excinfo.value)
@pytest.mark.parametrize('group_names,configured_min,ansible_mounts', [
@@ -97,7 +109,7 @@ def test_succeeds_with_recommended_disk_space(group_names, configured_min, ansib
assert not result.get('failed', False)
-@pytest.mark.parametrize('name,group_names,configured_min,ansible_mounts,extra_words', [
+@pytest.mark.parametrize('name,group_names,configured_min,ansible_mounts,expect_chunks', [
(
'test with no space available',
['masters'],
@@ -164,7 +176,7 @@ def test_succeeds_with_recommended_disk_space(group_names, configured_min, ansib
['0.0 GB'],
),
], ids=lambda argval: argval[0])
-def test_fails_with_insufficient_disk_space(name, group_names, configured_min, ansible_mounts, extra_words):
+def test_fails_with_insufficient_disk_space(name, group_names, configured_min, ansible_mounts, expect_chunks):
task_vars = dict(
group_names=group_names,
openshift_check_min_host_disk_gb=configured_min,
@@ -174,8 +186,8 @@ def test_fails_with_insufficient_disk_space(name, group_names, configured_min, a
result = DiskAvailability(fake_execute_module, task_vars).run()
assert result['failed']
- for word in 'below recommended'.split() + extra_words:
- assert word in result.get('msg', '')
+ for chunk in 'below recommended'.split() + expect_chunks:
+ assert chunk in result.get('msg', '')
@pytest.mark.parametrize('name,group_names,context,ansible_mounts,failed,extra_words', [
diff --git a/roles/openshift_health_checker/test/etcd_imagedata_size_test.py b/roles/openshift_health_checker/test/etcd_imagedata_size_test.py
index e3d6706fa..d3aae98f2 100644
--- a/roles/openshift_health_checker/test/etcd_imagedata_size_test.py
+++ b/roles/openshift_health_checker/test/etcd_imagedata_size_test.py
@@ -1,7 +1,8 @@
import pytest
from collections import namedtuple
-from openshift_checks.etcd_imagedata_size import EtcdImageDataSize, OpenShiftCheckException
+from openshift_checks.etcd_imagedata_size import EtcdImageDataSize
+from openshift_checks import OpenShiftCheckException
from etcdkeysize import check_etcd_key_size
@@ -56,7 +57,7 @@ def test_cannot_determine_available_mountpath(ansible_mounts, extra_words):
with pytest.raises(OpenShiftCheckException) as excinfo:
check.run()
- for word in 'determine valid etcd mountpath'.split() + extra_words:
+ for word in ['Unable to determine mount point'] + extra_words:
assert word in str(excinfo.value)
diff --git a/roles/openshift_health_checker/test/etcd_volume_test.py b/roles/openshift_health_checker/test/etcd_volume_test.py
index 0b255136e..077cea3ea 100644
--- a/roles/openshift_health_checker/test/etcd_volume_test.py
+++ b/roles/openshift_health_checker/test/etcd_volume_test.py
@@ -1,6 +1,7 @@
import pytest
-from openshift_checks.etcd_volume import EtcdVolume, OpenShiftCheckException
+from openshift_checks.etcd_volume import EtcdVolume
+from openshift_checks import OpenShiftCheckException
@pytest.mark.parametrize('ansible_mounts,extra_words', [
@@ -15,7 +16,7 @@ def test_cannot_determine_available_disk(ansible_mounts, extra_words):
with pytest.raises(OpenShiftCheckException) as excinfo:
EtcdVolume(fake_execute_module, task_vars).run()
- for word in 'Unable to find etcd storage mount point'.split() + extra_words:
+ for word in ['Unable to determine mount point'] + extra_words:
assert word in str(excinfo.value)