summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLuke Meyer <lmeyer@redhat.com>2017-08-23 17:44:19 -0400
committerLuke Meyer <lmeyer@redhat.com>2017-09-12 10:31:45 -0400
commita202f1647a28e4d246d0341e37501df44cb1a914 (patch)
tree2dfcfd0dd5f475bff402ea6c8a4e9cc0bda7ac79
parentf4a328ad2dc1d1ba93efa15ac0bf4502bad40079 (diff)
downloadopenshift-a202f1647a28e4d246d0341e37501df44cb1a914.tar.gz
openshift-a202f1647a28e4d246d0341e37501df44cb1a914.tar.bz2
openshift-a202f1647a28e4d246d0341e37501df44cb1a914.tar.xz
openshift-a202f1647a28e4d246d0341e37501df44cb1a914.zip
openshift_checks: add retries in python
-rw-r--r--roles/openshift_health_checker/openshift_checks/__init__.py17
-rw-r--r--roles/openshift_health_checker/openshift_checks/docker_image_availability.py6
-rw-r--r--roles/openshift_health_checker/openshift_checks/mixins.py2
-rw-r--r--roles/openshift_health_checker/openshift_checks/package_availability.py2
-rw-r--r--roles/openshift_health_checker/openshift_checks/package_update.py2
-rw-r--r--roles/openshift_health_checker/openshift_checks/package_version.py2
-rw-r--r--roles/openshift_health_checker/test/docker_image_availability_test.py18
-rw-r--r--roles/openshift_health_checker/test/package_availability_test.py4
-rw-r--r--roles/openshift_health_checker/test/package_update_test.py4
-rw-r--r--roles/openshift_health_checker/test/package_version_test.py8
10 files changed, 43 insertions, 22 deletions
diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py
index 02ee1d0f9..987c955b6 100644
--- a/roles/openshift_health_checker/openshift_checks/__init__.py
+++ b/roles/openshift_health_checker/openshift_checks/__init__.py
@@ -4,6 +4,7 @@ Health checks for OpenShift clusters.
import operator
import os
+import time
from abc import ABCMeta, abstractmethod, abstractproperty
from importlib import import_module
@@ -57,6 +58,9 @@ class OpenShiftCheck(object):
self._execute_module = execute_module
self.task_vars = task_vars or {}
self.tmp = tmp
+ # mainly for testing purposes; see execute_module_with_retries
+ self._module_retries = 3
+ self._module_retry_interval = 5 # seconds
# set to True when the check changes the host, for accurate total "changed" count
self.changed = False
@@ -115,6 +119,19 @@ class OpenShiftCheck(object):
)
return self._execute_module(module_name, module_args, self.tmp, self.task_vars)
+ def execute_module_with_retries(self, module_name, module_args):
+ """Run execute_module and retry on failure."""
+ result = {}
+ tries = 0
+ while True:
+ res = self.execute_module(module_name, module_args)
+ if tries > self._module_retries or not res.get("failed"):
+ result.update(res)
+ return result
+ result["last_failed"] = res
+ tries += 1
+ time.sleep(self._module_retry_interval)
+
def get_var(self, *keys, **kwargs):
"""Get deeply nested values from task_vars.
diff --git a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
index 866c74d7c..458e51d87 100644
--- a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
@@ -171,10 +171,8 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
registries = [registry]
for registry in registries:
- args = {
- "_raw_params": self.skopeo_img_check_command + " docker://{}/{}".format(registry, image)
- }
- result = self.execute_module("command", args)
+ args = {"_raw_params": self.skopeo_img_check_command + " docker://{}/{}".format(registry, image)}
+ result = self.execute_module_with_retries("command", args)
if result.get("rc", 0) == 0 and not result.get("failed"):
return True
diff --git a/roles/openshift_health_checker/openshift_checks/mixins.py b/roles/openshift_health_checker/openshift_checks/mixins.py
index e9bae60a3..24f1d938a 100644
--- a/roles/openshift_health_checker/openshift_checks/mixins.py
+++ b/roles/openshift_health_checker/openshift_checks/mixins.py
@@ -36,7 +36,7 @@ class DockerHostMixin(object):
# NOTE: we would use the "package" module but it's actually an action plugin
# and it's not clear how to invoke one of those. This is about the same anyway:
- result = self.execute_module(
+ result = self.execute_module_with_retries(
self.get_var("ansible_pkg_mgr", default="yum"),
{"name": self.dependencies, "state": "present"},
)
diff --git a/roles/openshift_health_checker/openshift_checks/package_availability.py b/roles/openshift_health_checker/openshift_checks/package_availability.py
index a86180b00..21355c2f0 100644
--- a/roles/openshift_health_checker/openshift_checks/package_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/package_availability.py
@@ -26,7 +26,7 @@ class PackageAvailability(NotContainerizedMixin, OpenShiftCheck):
packages.update(self.node_packages(rpm_prefix))
args = {"packages": sorted(set(packages))}
- return self.execute_module("check_yum_update", args)
+ return self.execute_module_with_retries("check_yum_update", args)
@staticmethod
def master_packages(rpm_prefix):
diff --git a/roles/openshift_health_checker/openshift_checks/package_update.py b/roles/openshift_health_checker/openshift_checks/package_update.py
index 1e9aecbe0..8464e8a5e 100644
--- a/roles/openshift_health_checker/openshift_checks/package_update.py
+++ b/roles/openshift_health_checker/openshift_checks/package_update.py
@@ -11,4 +11,4 @@ class PackageUpdate(NotContainerizedMixin, OpenShiftCheck):
def run(self):
args = {"packages": []}
- return self.execute_module("check_yum_update", args)
+ return self.execute_module_with_retries("check_yum_update", args)
diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py
index 0b795b6c4..d4aec3ed8 100644
--- a/roles/openshift_health_checker/openshift_checks/package_version.py
+++ b/roles/openshift_health_checker/openshift_checks/package_version.py
@@ -76,7 +76,7 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck):
],
}
- return self.execute_module("aos_version", args)
+ return self.execute_module_with_retries("aos_version", args)
def get_required_ovs_version(self):
"""Return the correct Open vSwitch version(s) for the current OpenShift version."""
diff --git a/roles/openshift_health_checker/test/docker_image_availability_test.py b/roles/openshift_health_checker/test/docker_image_availability_test.py
index 8d0a53df9..b51fdeac4 100644
--- a/roles/openshift_health_checker/test/docker_image_availability_test.py
+++ b/roles/openshift_health_checker/test/docker_image_availability_test.py
@@ -69,7 +69,7 @@ def test_all_images_available_remotely(available_locally):
return {'images': [], 'failed': available_locally}
return {'changed': False}
- result = DockerImageAvailability(execute_module, task_vars=dict(
+ check = DockerImageAvailability(execute_module, task_vars=dict(
openshift=dict(
common=dict(
service_type='origin',
@@ -81,7 +81,9 @@ def test_all_images_available_remotely(available_locally):
openshift_deployment_type='origin',
openshift_image_tag='v3.4',
group_names=['nodes', 'masters'],
- )).run()
+ ))
+ check._module_retry_interval = 0
+ result = check.run()
assert not result.get('failed', False)
@@ -97,7 +99,7 @@ def test_all_images_unavailable():
'changed': False,
}
- actual = DockerImageAvailability(execute_module, task_vars=dict(
+ check = DockerImageAvailability(execute_module, task_vars=dict(
openshift=dict(
common=dict(
service_type='origin',
@@ -109,7 +111,9 @@ def test_all_images_unavailable():
openshift_deployment_type="openshift-enterprise",
openshift_image_tag='latest',
group_names=['nodes', 'masters'],
- )).run()
+ ))
+ check._module_retry_interval = 0
+ actual = check.run()
assert actual['failed']
assert "required Docker images are not available" in actual['msg']
@@ -136,7 +140,7 @@ def test_skopeo_update_failure(message, extra_words):
return {'changed': False}
- actual = DockerImageAvailability(execute_module, task_vars=dict(
+ check = DockerImageAvailability(execute_module, task_vars=dict(
openshift=dict(
common=dict(
service_type='origin',
@@ -148,7 +152,9 @@ def test_skopeo_update_failure(message, extra_words):
openshift_deployment_type="openshift-enterprise",
openshift_image_tag='',
group_names=['nodes', 'masters'],
- )).run()
+ ))
+ check._module_retry_interval = 0
+ actual = check.run()
assert actual["failed"]
for word in extra_words:
diff --git a/roles/openshift_health_checker/test/package_availability_test.py b/roles/openshift_health_checker/test/package_availability_test.py
index 1fe648b75..8aa87ca59 100644
--- a/roles/openshift_health_checker/test/package_availability_test.py
+++ b/roles/openshift_health_checker/test/package_availability_test.py
@@ -56,7 +56,7 @@ def test_package_availability(task_vars, must_have_packages, must_not_have_packa
assert 'packages' in module_args
assert set(module_args['packages']).issuperset(must_have_packages)
assert not set(module_args['packages']).intersection(must_not_have_packages)
- return return_value
+ return {'foo': return_value}
result = PackageAvailability(execute_module, task_vars).run()
- assert result is return_value
+ assert result['foo'] is return_value
diff --git a/roles/openshift_health_checker/test/package_update_test.py b/roles/openshift_health_checker/test/package_update_test.py
index 06489b0d7..7d9035a36 100644
--- a/roles/openshift_health_checker/test/package_update_test.py
+++ b/roles/openshift_health_checker/test/package_update_test.py
@@ -9,7 +9,7 @@ def test_package_update():
assert 'packages' in module_args
# empty list of packages means "generic check if 'yum update' will work"
assert module_args['packages'] == []
- return return_value
+ return {'foo': return_value}
result = PackageUpdate(execute_module).run()
- assert result is return_value
+ assert result['foo'] is return_value
diff --git a/roles/openshift_health_checker/test/package_version_test.py b/roles/openshift_health_checker/test/package_version_test.py
index e871f39f0..8564cd4db 100644
--- a/roles/openshift_health_checker/test/package_version_test.py
+++ b/roles/openshift_health_checker/test/package_version_test.py
@@ -52,7 +52,7 @@ def test_invalid_openshift_release_format():
])
def test_package_version(openshift_release):
- return_value = object()
+ return_value = {"foo": object()}
def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None, *_):
assert module_name == 'aos_version'
@@ -66,7 +66,7 @@ def test_package_version(openshift_release):
check = PackageVersion(execute_module, task_vars_for(openshift_release, 'origin'))
result = check.run()
- assert result is return_value
+ assert result == return_value
@pytest.mark.parametrize('deployment_type,openshift_release,expected_docker_version', [
@@ -79,7 +79,7 @@ def test_package_version(openshift_release):
])
def test_docker_package_version(deployment_type, openshift_release, expected_docker_version):
- return_value = object()
+ return_value = {"foo": object()}
def execute_module(module_name=None, module_args=None, *_):
assert module_name == 'aos_version'
@@ -93,7 +93,7 @@ def test_docker_package_version(deployment_type, openshift_release, expected_doc
check = PackageVersion(execute_module, task_vars_for(openshift_release, deployment_type))
result = check.run()
- assert result is return_value
+ assert result == return_value
@pytest.mark.parametrize('group_names,is_containerized,is_active', [