From 78639f0217d4027bf2d5542473562a1634f92ba8 Mon Sep 17 00:00:00 2001 From: Michael Gugino Date: Tue, 3 Oct 2017 18:50:10 -0400 Subject: docker_image_availability: credentials to skopeo Currently, docker_image_availability health_check does not support authenticated registries. This commit adds the '--creds=' option to skopeo if needed to support authentication credentials. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1316341 Some other fixes to handle docker config better: Should now account properly for blocked registries, insecure registries, multiple additional registries, and oreg_url registry with or without credentials. Output on failure should be clearer about what was tried. Fixed a bug in the action_plugin_test exposed by these changes. --- .../openshift_checks/docker_image_availability.py | 109 +++++++++++++-------- .../test/action_plugin_test.py | 1 + .../test/docker_image_availability_test.py | 49 ++------- 3 files changed, 76 insertions(+), 83 deletions(-) (limited to 'roles') 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 63ccadcd1..7c8ac78fe 100644 --- a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py +++ b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py @@ -1,5 +1,6 @@ """Check that required Docker images are available.""" +from pipes import quote from ansible.module_utils import six from openshift_checks import OpenShiftCheck from openshift_checks.mixins import DockerHostMixin @@ -33,10 +34,39 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): # we use python-docker-py to check local docker for images, and skopeo # to look for images available remotely without waiting to pull them. dependencies = ["python-docker-py", "skopeo"] - skopeo_img_check_command = "timeout 10 skopeo inspect --tls-verify=false docker://{registry}/{image}" + # command for checking if remote registries have an image, without docker pull + skopeo_command = "timeout 10 skopeo inspect --tls-verify={tls} {creds} docker://{registry}/{image}" + skopeo_example_command = "skopeo inspect [--tls-verify=false] [--creds=:] docker:///" def __init__(self, *args, **kwargs): super(DockerImageAvailability, self).__init__(*args, **kwargs) + + self.registries = dict( + # set of registries that need to be checked insecurely (note: not accounting for CIDR entries) + insecure=set(self.ensure_list("openshift_docker_insecure_registries")), + # set of registries that should never be queried even if given in the image + blocked=set(self.ensure_list("openshift_docker_blocked_registries")), + ) + + # ordered list of registries (according to inventory vars) that docker will try for unscoped images + regs = self.ensure_list("openshift_docker_additional_registries") + # currently one of these registries is added whether the user wants it or not. + deployment_type = self.get_var("openshift_deployment_type") + if deployment_type == "origin" and "docker.io" not in regs: + regs.append("docker.io") + elif deployment_type == 'openshift-enterprise' and "registry.access.redhat.com" not in regs: + regs.append("registry.access.redhat.com") + self.registries["configured"] = regs + + # for the oreg_url registry there may be credentials specified + components = self.get_var("oreg_url", default="").split('/') + self.registries["oreg"] = "" if len(components) < 3 else components[0] + self.skopeo_command_creds = "" + oreg_auth_user = self.get_var('oreg_auth_user', default='') + oreg_auth_password = self.get_var('oreg_auth_password', default='') + if oreg_auth_user != '' and oreg_auth_password != '': + self.skopeo_command_creds = "--creds={}:{}".format(quote(oreg_auth_user), quote(oreg_auth_password)) + # record whether we could reach a registry or not (and remember results) self.reachable_registries = {} @@ -62,26 +92,25 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): if not missing_images: return {} - registries = self.known_docker_registries() - if not registries: - return {"failed": True, "msg": "Unable to retrieve any docker registries."} - - available_images = self.available_images(missing_images, registries) + available_images = self.available_images(missing_images) unavailable_images = set(missing_images) - set(available_images) if unavailable_images: - registries = [ - reg if self.reachable_registries.get(reg, True) else reg + " (unreachable)" - for reg in registries - ] + unreachable = [reg for reg, reachable in self.reachable_registries.items() if not reachable] + unreachable_msg = "Failed connecting to: {}\n".format(", ".join(unreachable)) + blocked_msg = "Blocked registries: {}\n".format(", ".join(self.registries["blocked"])) msg = ( - "One or more required Docker images are not available:\n {}\n" - "Configured registries: {}\n" - "Checked by: {}" + "One or more required container images are not available:\n {missing}\n" + "Checked with: {cmd}\n" + "Default registries searched: {registries}\n" + "{blocked}" + "{unreachable}" ).format( - ",\n ".join(sorted(unavailable_images)), - ", ".join(registries), - self.skopeo_img_check_command + missing=",\n ".join(sorted(unavailable_images)), + cmd=self.skopeo_example_command, + registries=", ".join(self.registries["configured"]), + blocked=blocked_msg if self.registries["blocked"] else "", + unreachable=unreachable_msg if unreachable else "", ) return dict(failed=True, msg=msg) @@ -138,11 +167,10 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): def local_images(self, images): """Filter a list of images and return those available locally.""" - registries = self.known_docker_registries() found_images = [] for image in images: # docker could have the image name as-is or prefixed with any registry - imglist = [image] + [reg + "/" + image for reg in registries] + imglist = [image] + [reg + "/" + image for reg in self.registries["configured"]] if self.is_image_local(imglist): found_images.append(image) return found_images @@ -152,37 +180,27 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): result = self.execute_module("docker_image_facts", {"name": image}) return bool(result.get("images")) and not result.get("failed") - def known_docker_registries(self): - """Build a list of docker registries available according to inventory vars.""" - regs = self.get_var("openshift_docker_additional_registries", default=[]) + def ensure_list(self, registry_param): + """Return the task var as a list.""" # https://bugzilla.redhat.com/show_bug.cgi?id=1497274 - # if the result was a string type, place it into a list. We must do this + # If the result was a string type, place it into a list. We must do this # as using list() on a string will split the string into its characters. - if isinstance(regs, six.string_types): - regs = [regs] - else: - # Otherwise cast to a list as was done previously - regs = list(regs) + # Otherwise cast to a list as was done previously. + registry = self.get_var(registry_param, default=[]) + if not isinstance(registry, six.string_types): + return list(registry) + return self.normalize(registry) - deployment_type = self.get_var("openshift_deployment_type") - if deployment_type == "origin" and "docker.io" not in regs: - regs.append("docker.io") - elif deployment_type == 'openshift-enterprise' and "registry.access.redhat.com" not in regs: - regs.append("registry.access.redhat.com") - - return regs - - def available_images(self, images, default_registries): + def available_images(self, images): """Search remotely for images. Returns: list of images found.""" return [ image for image in images - if self.is_available_skopeo_image(image, default_registries) + if self.is_available_skopeo_image(image) ] - def is_available_skopeo_image(self, image, default_registries): + def is_available_skopeo_image(self, image): """Use Skopeo to determine if required image exists in known registry(s).""" - registries = default_registries - + registries = self.registries["configured"] # If image already includes a registry, only use that. # NOTE: This logic would incorrectly identify images that do not use a namespace, e.g. # registry.access.redhat.com/rhel7 as if the registry were a namespace. @@ -193,13 +211,18 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): registries = [registry] for registry in registries: + if registry in self.registries["blocked"]: + continue # blocked will never be consulted if registry not in self.reachable_registries: self.reachable_registries[registry] = self.connect_to_registry(registry) if not self.reachable_registries[registry]: - continue + continue # do not keep trying unreachable registries + + args = dict(registry=registry, image=image) + args["tls"] = "false" if registry in self.registries["insecure"] else "true" + args["creds"] = self.skopeo_command_creds if registry == self.registries["oreg"] else "" - args = {"_raw_params": self.skopeo_img_check_command.format(registry=registry, image=image)} - result = self.execute_module_with_retries("command", args) + result = self.execute_module_with_retries("command", {"_raw_params": self.skopeo_command.format(**args)}) if result.get("rc", 0) == 0 and not result.get("failed"): return True if result.get("rc") == 124: # RC 124 == timed out; mark unreachable diff --git a/roles/openshift_health_checker/test/action_plugin_test.py b/roles/openshift_health_checker/test/action_plugin_test.py index f14887303..40ad27d5d 100644 --- a/roles/openshift_health_checker/test/action_plugin_test.py +++ b/roles/openshift_health_checker/test/action_plugin_test.py @@ -94,6 +94,7 @@ def skipped(result): {}, ]) def test_action_plugin_missing_openshift_facts(plugin, task_vars, monkeypatch): + monkeypatch.setattr(plugin, 'load_known_checks', lambda *_: {}) monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check']) result = plugin.run(tmp=None, task_vars=task_vars) 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 43dcf0c9a..dec99e5db 100644 --- a/roles/openshift_health_checker/test/docker_image_availability_test.py +++ b/roles/openshift_health_checker/test/docker_image_availability_test.py @@ -98,40 +98,7 @@ def test_all_images_unavailable(task_vars): actual = check.run() assert actual['failed'] - assert "required Docker images are not available" in actual['msg'] - - -def test_no_known_registries(): - def execute_module(module_name=None, *_): - if module_name == "command": - return { - 'failed': True, - } - - return { - 'changed': False, - } - - def mock_known_docker_registries(): - return [] - - dia = DockerImageAvailability(execute_module, task_vars=dict( - openshift=dict( - common=dict( - service_type='origin', - is_containerized=False, - is_atomic=False, - ) - ), - openshift_docker_additional_registries=["docker.io"], - openshift_deployment_type="openshift-enterprise", - openshift_image_tag='latest', - group_names=['oo_nodes_to_config', 'oo_masters_to_config'], - )) - dia.known_docker_registries = mock_known_docker_registries - actual = dia.run() - assert actual['failed'] - assert "Unable to retrieve any docker registries." in actual['msg'] + assert "required container images are not available" in actual['msg'] @pytest.mark.parametrize("message,extra_words", [ @@ -172,13 +139,13 @@ def test_skopeo_update_failure(task_vars, message, extra_words): "spam/eggs:v1", ["test.reg"], True, True, False, - {"test.reg": False}, + {"test.reg": False, "docker.io": False}, ), ( "spam/eggs:v1", ["test.reg"], False, True, False, - {"test.reg": True}, + {"test.reg": True, "docker.io": True}, ), ( "eggs.reg/spam/eggs:v1", ["test.reg"], @@ -195,17 +162,19 @@ def test_registry_availability(image, registries, connection_test_failed, skopeo elif module_name == "command": return dict(msg="msg", failed=skopeo_failed) - check = DockerImageAvailability(execute_module, task_vars()) + tv = task_vars() + tv.update({"openshift_docker_additional_registries": registries}) + check = DockerImageAvailability(execute_module, tv) check._module_retry_interval = 0 - available = check.is_available_skopeo_image(image, registries) + available = check.is_available_skopeo_image(image) assert available == expect_success assert expect_registries_reached == check.reachable_registries @pytest.mark.parametrize("deployment_type, is_containerized, groups, oreg_url, expected", [ ( # standard set of stuff required on nodes - "origin", False, ['oo_nodes_to_config'], None, + "origin", False, ['oo_nodes_to_config'], "", set([ 'openshift/origin-pod:vtest', 'openshift/origin-deployer:vtest', @@ -225,7 +194,7 @@ def test_registry_availability(image, registries, connection_test_failed, skopeo ]) ), ( - "origin", True, ['oo_nodes_to_config', 'oo_masters_to_config', 'oo_etcd_to_config'], None, + "origin", True, ['oo_nodes_to_config', 'oo_masters_to_config', 'oo_etcd_to_config'], "", set([ # images running on top of openshift 'openshift/origin-pod:vtest', -- cgit v1.2.3