Merge pull request #3785 from juanvallejo/jvallejo/update-docker-image-availability-check

Merged by openshift-bot

roles/openshift_health_checker/meta/main.yml

@@ -2,3 +2,4 @@
 dependencies:
   - role: openshift_facts
   - role: openshift_repos
+  - role: openshift_version

roles/openshift_health_checker/openshift_checks/docker_image_availability.py

@@ -13,41 +13,55 @@ class DockerImageAvailability(OpenShiftCheck):
     name = "docker_image_availability"
     tags = ["preflight"]
 
-    skopeo_image = "openshift/openshift-ansible"
+    dependencies = ["skopeo", "python-docker-py"]
 
-    # FIXME(juanvallejo): we should consider other possible values of
-    # `deployment_type` (the key here). See
-    # https://github.com/openshift/openshift-ansible/blob/8e26f8c/roles/openshift_repos/vars/main.yml#L7
-    docker_image_base = {
+    deployment_image_info = {
         "origin": {
-            "repo": "openshift",
-            "image": "origin",
+            "namespace": "openshift",
+            "name": "origin",
         },
         "openshift-enterprise": {
-            "repo": "openshift3",
-            "image": "ose",
+            "namespace": "openshift3",
+            "name": "ose",
         },
     }
 
-    def run(self, tmp, task_vars):
-        required_images = self.required_images(task_vars)
-        missing_images = set(required_images) - set(self.local_images(required_images, task_vars))
+    @classmethod
+    def is_active(cls, task_vars):
+        """Skip hosts with unsupported deployment types."""
+        deployment_type = get_var(task_vars, "openshift_deployment_type")
+        has_valid_deployment_type = deployment_type in cls.deployment_image_info
 
-        # exit early if all images were found locally
-        if not missing_images:
-            return {"changed": False}
+        return super(DockerImageAvailability, cls).is_active(task_vars) and has_valid_deployment_type
 
-        msg, failed, changed = self.update_skopeo_image(task_vars)
+    def run(self, tmp, task_vars):
+        msg, failed, changed = self.ensure_dependencies(task_vars)
 
         # exit early if Skopeo update fails
         if failed:
+            if "No package matching" in msg:
+                msg = "Ensure that all required dependencies can be installed via `yum`.\n"
             return {
                 "failed": True,
                 "changed": changed,
-                "msg": "Failed to update Skopeo image ({img_name}). {msg}".format(img_name=self.skopeo_image, msg=msg),
+                "msg": (
+                    "Unable to update or install required dependency packages on this host;\n"
+                    "These are required in order to check Docker image availability:"
+                    "\n    {deps}\n{msg}"
+                ).format(deps=',\n    '.join(self.dependencies), msg=msg),
             }
 
+        required_images = self.required_images(task_vars)
+        missing_images = set(required_images) - set(self.local_images(required_images, task_vars))
+
+        # exit early if all images were found locally
+        if not missing_images:
+            return {"changed": changed}
+
         registries = self.known_docker_registries(task_vars)
+        if not registries:
+            return {"failed": True, "msg": "Unable to retrieve any docker registries.", "changed": changed}
+
         available_images = self.available_images(missing_images, registries, task_vars)
         unavailable_images = set(missing_images) - set(available_images)
 
@@ -55,44 +69,60 @@ class DockerImageAvailability(OpenShiftCheck):
             return {
                 "failed": True,
                 "msg": (
-                    "One or more required images are not available: {}.\n"
+                    "One or more required Docker images are not available:\n    {}\n"
                     "Configured registries: {}"
-                ).format(", ".join(sorted(unavailable_images)), ", ".join(registries)),
+                ).format(",\n    ".join(sorted(unavailable_images)), ", ".join(registries)),
                 "changed": changed,
             }
 
         return {"changed": changed}
 
     def required_images(self, task_vars):
-        deployment_type = get_var(task_vars, "deployment_type")
-        # FIXME(juanvallejo): we should handle gracefully with a proper error
-        # message when given an unexpected value for `deployment_type`.
-        image_base_name = self.docker_image_base[deployment_type]
-
-        openshift_release = get_var(task_vars, "openshift_release")
-        # FIXME(juanvallejo): this variable is not required when the
-        # installation is non-containerized. The example inventories have it
-        # commented out. We should handle gracefully and with a proper error
-        # message when this variable is required and not set.
-        openshift_image_tag = get_var(task_vars, "openshift_image_tag")
+        deployment_type = get_var(task_vars, "openshift_deployment_type")
+        image_info = self.deployment_image_info[deployment_type]
 
+        openshift_release = get_var(task_vars, "openshift_release", default="latest")
+        openshift_image_tag = get_var(task_vars, "openshift_image_tag")
         is_containerized = get_var(task_vars, "openshift", "common", "is_containerized")
 
-        if is_containerized:
-            images = set(self.containerized_docker_images(image_base_name, openshift_release))
-        else:
-            images = set(self.rpm_docker_images(image_base_name, openshift_release))
+        images = set(self.required_docker_images(
+            image_info["namespace"],
+            image_info["name"],
+            ["registry-console"] if "enterprise" in deployment_type else [],  # include enterprise-only image names
+            openshift_release,
+            is_containerized,
+        ))
 
         # append images with qualified image tags to our list of required images.
         # these are images with a (v0.0.0.0) tag, rather than a standard release
         # format tag (v0.0). We want to check this set in both containerized and
         # non-containerized installations.
         images.update(
-            self.qualified_docker_images(self.image_from_base_name(image_base_name), "v" + openshift_image_tag)
+            self.required_qualified_docker_images(
+                image_info["namespace"],
+                image_info["name"],
+                openshift_image_tag,
+            ),
         )
 
         return images
 
+    @staticmethod
+    def required_docker_images(namespace, name, additional_image_names, version, is_containerized):
+        if is_containerized:
+            return ["{}/{}:{}".format(namespace, name, version)] if name else []
+
+        # include additional non-containerized images specific to the current deployment type
+        return ["{}/{}:{}".format(namespace, img_name, version) for img_name in additional_image_names]
+
+    @staticmethod
+    def required_qualified_docker_images(namespace, name, version):
+        # pylint: disable=invalid-name
+        return [
+            "{}/{}-{}:{}".format(namespace, name, suffix, version)
+            for suffix in ["haproxy-router", "docker-registry", "deployer", "pod"]
+        ]
+
     def local_images(self, images, task_vars):
         """Filter a list of images and return those available locally."""
         return [
@@ -107,31 +137,26 @@ class DockerImageAvailability(OpenShiftCheck):
 
         return bool(result.get("images", []))
 
-    def known_docker_registries(self, task_vars):
-        result = self.module_executor("docker_info", {}, task_vars)
+    @staticmethod
+    def known_docker_registries(task_vars):
+        docker_facts = get_var(task_vars, "openshift", "docker")
+        regs = set(docker_facts["additional_registries"])
 
-        if result.get("failed", False):
-            return []
+        deployment_type = get_var(task_vars, "openshift_deployment_type")
+        if deployment_type == "origin":
+            regs.update(["docker.io"])
+        elif "enterprise" in deployment_type:
+            regs.update(["registry.access.redhat.com"])
 
-        # FIXME(juanvallejo): wrong default type, result["info"] is expected to
-        # contain a dictionary (see how we call `docker_info.get` below).
-        docker_info = result.get("info", "")
-        return [registry.get("Name", "") for registry in docker_info.get("Registries", {})]
+        return list(regs)
 
     def available_images(self, images, registries, task_vars):
         """Inspect existing images using Skopeo and return all images successfully inspected."""
         return [
             image for image in images
-            if self.is_image_available(image, registries, task_vars)
+            if any(self.is_available_skopeo_image(image, registry, task_vars) for registry in registries)
         ]
 
-    def is_image_available(self, image, registries, task_vars):
-        for registry in registries:
-            if self.is_available_skopeo_image(image, registry, task_vars):
-                return True
-
-        return False
-
     def is_available_skopeo_image(self, image, registry, task_vars):
         """Uses Skopeo to determine if required image exists in a given registry."""
 
@@ -140,40 +165,15 @@ class DockerImageAvailability(OpenShiftCheck):
             image=image,
         )
 
-        args = {
-            "name": "skopeo_inspect",
-            "image": self.skopeo_image,
-            "command": cmd_str,
-            "detach": False,
-            "cleanup": True,
-        }
-        result = self.module_executor("docker_container", args, task_vars)
-        return result.get("failed", False)
-
-    def containerized_docker_images(self, base_name, version):
-        return [
-            "{image}:{version}".format(image=self.image_from_base_name(base_name), version=version)
-        ]
+        args = {"_raw_params": cmd_str}
+        result = self.module_executor("command", args, task_vars)
+        return not result.get("failed", False) and result.get("rc", 0) == 0
 
-    @staticmethod
-    def rpm_docker_images(base, version):
-        return [
-            "{image_repo}/registry-console:{version}".format(image_repo=base["repo"], version=version)
-        ]
+    # ensures that the skopeo and python-docker-py packages exist
+    # check is skipped on atomic installations
+    def ensure_dependencies(self, task_vars):
+        if get_var(task_vars, "openshift", "common", "is_atomic"):
+            return "", False, False
 
-    @staticmethod
-    def qualified_docker_images(image_name, version):
-        return [
-            "{}-{}:{}".format(image_name, component, version)
-            for component in "haproxy-router docker-registry deployer pod".split()
-        ]
-
-    @staticmethod
-    def image_from_base_name(base):
-        return "".join([base["repo"], "/", base["image"]])
-
-    # ensures that the skopeo docker image exists, and updates it
-    # with latest if image was already present locally.
-    def update_skopeo_image(self, task_vars):
-        result = self.module_executor("docker_image", {"name": self.skopeo_image}, task_vars)
-        return result.get("msg", ""), result.get("failed", False), result.get("changed", False)
+        result = self.module_executor("yum", {"name": self.dependencies, "state": "latest"}, task_vars)
+        return result.get("msg", ""), result.get("failed", False) or result.get("rc", 0) != 0, result.get("changed")

roles/openshift_health_checker/test/docker_image_availability_test.py

@@ -3,26 +3,176 @@ import pytest
 from openshift_checks.docker_image_availability import DockerImageAvailability
 
 
-@pytest.mark.xfail(strict=True)  # TODO: remove this once this test is fully implemented.
-@pytest.mark.parametrize('task_vars,expected_result', [
-    (
-        dict(
-            openshift=dict(common=dict(
+@pytest.mark.parametrize('deployment_type,is_active', [
+    ("origin", True),
+    ("openshift-enterprise", True),
+    ("enterprise", False),
+    ("online", False),
+    ("invalid", False),
+    ("", False),
+])
+def test_is_active(deployment_type, is_active):
+    task_vars = dict(
+        openshift_deployment_type=deployment_type,
+    )
+    assert DockerImageAvailability.is_active(task_vars=task_vars) == is_active
+
+
+@pytest.mark.parametrize("is_containerized,is_atomic", [
+    (True, True),
+    (False, False),
+    (True, False),
+    (False, True),
+])
+def test_all_images_available_locally(is_containerized, is_atomic):
+    def execute_module(module_name, args, task_vars):
+        if module_name == "yum":
+            return {"changed": True}
+
+        assert module_name == "docker_image_facts"
+        assert 'name' in args
+        assert args['name']
+        return {
+            'images': [args['name']],
+        }
+
+    result = DockerImageAvailability(execute_module=execute_module).run(tmp=None, task_vars=dict(
+        openshift=dict(
+            common=dict(
+                service_type='origin',
+                is_containerized=is_containerized,
+                is_atomic=is_atomic,
+            ),
+            docker=dict(additional_registries=["docker.io"]),
+        ),
+        openshift_deployment_type='origin',
+        openshift_release='v3.4',
+        openshift_image_tag='3.4',
+    ))
+
+    assert not result.get('failed', False)
+
+
+@pytest.mark.parametrize("available_locally", [
+    False,
+    True,
+])
+def test_all_images_available_remotely(available_locally):
+    def execute_module(module_name, args, task_vars):
+        if module_name == 'docker_image_facts':
+            return {'images': [], 'failed': available_locally}
+        return {'changed': False}
+
+    result = DockerImageAvailability(execute_module=execute_module).run(tmp=None, task_vars=dict(
+        openshift=dict(
+            common=dict(
                 service_type='origin',
                 is_containerized=False,
-            )),
-            openshift_release='v3.5',
-            deployment_type='origin',
-            openshift_image_tag='',  # FIXME: should not be required
+                is_atomic=False,
+            ),
+            docker=dict(additional_registries=["docker.io", "registry.access.redhat.com"]),
         ),
-        {'changed': False},
+        openshift_deployment_type='origin',
+        openshift_release='3.4',
+        openshift_image_tag='v3.4',
+    ))
+
+    assert not result.get('failed', False)
+
+
+def test_all_images_unavailable():
+    def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None):
+        if module_name == "command":
+            return {
+                'failed': True,
+            }
+
+        return {
+            'changed': False,
+        }
+
+    check = DockerImageAvailability(execute_module=execute_module)
+    actual = check.run(tmp=None, task_vars=dict(
+        openshift=dict(
+            common=dict(
+                service_type='origin',
+                is_containerized=False,
+                is_atomic=False,
+            ),
+            docker=dict(additional_registries=["docker.io"]),
+        ),
+        openshift_deployment_type="openshift-enterprise",
+        openshift_release=None,
+        openshift_image_tag='latest'
+    ))
+
+    assert actual['failed']
+    assert "required Docker images are not available" in actual['msg']
+
+
+@pytest.mark.parametrize("message,extra_words", [
+    (
+        "docker image update failure",
+        ["docker image update failure"],
+    ),
+    (
+        "No package matching 'skopeo' found available, installed or updated",
+        ["dependencies can be installed via `yum`"]
     ),
-    # TODO: add more parameters here to test the multiple possible inputs that affect behavior.
 ])
-def test_docker_image_availability(task_vars, expected_result):
+def test_skopeo_update_failure(message, extra_words):
     def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None):
-        return {'info': {}}  # TODO: this will vary depending on input parameters.
+        if module_name == "yum":
+            return {
+                "failed": True,
+                "msg": message,
+                "changed": False,
+            }
 
-    check = DockerImageAvailability(execute_module=execute_module)
-    result = check.run(tmp=None, task_vars=task_vars)
-    assert result == expected_result
+        return {'changed': False}
+
+    actual = DockerImageAvailability(execute_module=execute_module).run(tmp=None, task_vars=dict(
+        openshift=dict(
+            common=dict(
+                service_type='origin',
+                is_containerized=False,
+                is_atomic=False,
+            ),
+            docker=dict(additional_registries=["unknown.io"]),
+        ),
+        openshift_deployment_type="openshift-enterprise",
+        openshift_release='',
+        openshift_image_tag='',
+    ))
+
+    assert actual["failed"]
+    for word in extra_words:
+        assert word in actual["msg"]
+
+
+@pytest.mark.parametrize("deployment_type,registries", [
+    ("origin", ["unknown.io"]),
+    ("openshift-enterprise", ["registry.access.redhat.com"]),
+    ("openshift-enterprise", []),
+])
+def test_registry_availability(deployment_type, registries):
+    def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None):
+        return {
+            'changed': False,
+        }
+
+    actual = DockerImageAvailability(execute_module=execute_module).run(tmp=None, task_vars=dict(
+        openshift=dict(
+            common=dict(
+                service_type='origin',
+                is_containerized=False,
+                is_atomic=False,
+            ),
+            docker=dict(additional_registries=registries),
+        ),
+        openshift_deployment_type=deployment_type,
+        openshift_release='',
+        openshift_image_tag='',
+    ))
+
+    assert not actual.get("failed", False)

test/integration/openshift_health_checker/preflight/playbooks/package_availability_missing_required.yml

@@ -1,4 +1,5 @@
 ---
+# NOTE: this test is probably superfluous since openshift_version already does it
 - include: ../../setup_container.yml
   vars:
     image: preflight-aos-package-checks
@@ -7,11 +8,22 @@
 
 - name: Fail as required packages cannot be installed
   hosts: all
+  pre_tasks:
+
+    # run before roles to prevent openshift_version breaking
+    - include: tasks/enable_repo.yml
+      vars: { repo_name: "ose-3.2" }
+
   roles:
     - openshift_health_checker
-  tasks:
+
+  post_tasks:
     - block:
 
+        # put the repo back to disabled
+        - include: tasks/enable_repo.yml
+          vars: { repo_name: "ose-3.2", repo_enabled: 0 }
+
         - action: openshift_health_check
           args:
             checks: [ 'package_availability' ]

test/integration/openshift_health_checker/preflight/playbooks/package_update_dep_missing.yml

@@ -8,9 +8,16 @@
 
 - name: Fails when a dependency required for update is missing
   hosts: all
+  pre_tasks:
+
+    # run before roles to prevent openshift_version breaking
+    - include: tasks/enable_repo.yml
+      vars: { repo_name: "ose-3.2" }
+
   roles:
     - openshift_health_checker
-  tasks:
+
+  post_tasks:
     - block:
 
         - include: tasks/enable_repo.yml

test/integration/openshift_health_checker/preflight/playbooks/package_update_repo_broken.yml

@@ -8,9 +8,16 @@
 
 - name: Fails when a repo definition is completely broken
   hosts: all
+  pre_tasks:
+
+    # run before roles to prevent openshift_version breaking
+    - include: tasks/enable_repo.yml
+      vars: { repo_name: "ose-3.2" }
+
   roles:
     - openshift_health_checker
-  tasks:
+
+  post_tasks:
     - block:
 
         - include: tasks/enable_repo.yml

test/integration/openshift_health_checker/preflight/playbooks/package_update_repo_disabled.yml

@@ -8,9 +8,16 @@
 
 - name: Succeeds when nothing blocks a yum update
   hosts: all
+  pre_tasks:
+
+    # run before roles to prevent openshift_version breaking
+    - include: tasks/enable_repo.yml
+      vars: { repo_name: "ose-3.2" }
+
   roles:
     - openshift_health_checker
-  tasks:
+
+  post_tasks:
     - block:
 
         - action: openshift_health_check

test/integration/openshift_health_checker/preflight/playbooks/package_update_repo_unreachable.yml

@@ -8,9 +8,16 @@
 
 - name: Fails when repo content is not available
   hosts: all
+  pre_tasks:
+
+    # run before roles to prevent openshift_version breaking
+    - include: tasks/enable_repo.yml
+      vars: { repo_name: "ose-3.2" }
+
   roles:
     - openshift_health_checker
-  tasks:
+
+  post_tasks:
     - block:
 
         - include: tasks/enable_repo.yml

test/integration/openshift_health_checker/preflight/playbooks/package_version_matches.yml

@@ -8,13 +8,17 @@
 
 - name: Success when AOS version matches openshift_release
   hosts: all
+  pre_tasks:
+
+    # run before roles to prevent openshift_version breaking
+    - include: tasks/enable_repo.yml
+      vars: { repo_name: "ose-3.2" }
+
   roles:
     - openshift_health_checker
-  tasks:
-    - block:
 
-        - include: tasks/enable_repo.yml
-          vars: { repo_name: "ose-3.2" }
+  post_tasks:
+    - block:
 
         - action: openshift_health_check
           args:

test/integration/openshift_health_checker/preflight/playbooks/package_version_mismatches.yml

@@ -1,4 +1,5 @@
 ---
+# NOTE: this test is probably superfluous since openshift_version already does it
 - include: ../../setup_container.yml
   vars:
     image: preflight-aos-package-checks
@@ -8,14 +9,24 @@
 
 - name: Failure when AOS version doesn't match openshift_release
   hosts: all
+  pre_tasks:
+
+    # run before roles to prevent openshift_version breaking
+    - include: tasks/enable_repo.yml
+      vars: { repo_name: "ose-3.3" }
+
   roles:
     - openshift_health_checker
-  tasks:
+
+  post_tasks:
     - block:
 
+        # put the repo back to disabled
+        - include: tasks/enable_repo.yml
+          vars: { repo_name: "ose-3.3", repo_enabled: 0 }
+        # test with wrong repo enabled
         - include: tasks/enable_repo.yml
           vars: { repo_name: "ose-3.2" }
-
         - action: openshift_health_check
           args:
             checks: [ 'package_version' ]

test/integration/openshift_health_checker/preflight/playbooks/package_version_multiple.yml

@@ -7,14 +7,19 @@
 
 - name: Fails when multiple AOS versions are available
   hosts: all
+  pre_tasks:
+
+    # run before roles to prevent openshift_version breaking
+    - include: tasks/enable_repo.yml
+      vars: { repo_name: "ose-3.2" }
+
   roles:
     - openshift_health_checker
-  tasks:
-    - block:
 
-        - include: tasks/enable_repo.yml
-          vars: { repo_name: "ose-3.2" }
+  post_tasks:
+    - block:
 
+        # enable repo with extra minor version available
         - include: tasks/enable_repo.yml
           vars: { repo_name: "ose-3.3" }
 

test/integration/openshift_health_checker/preflight/playbooks/tasks/enable_repo.yml

@@ -6,4 +6,4 @@
     dest: /etc/yum.repos.d/{{ repo_name }}.repo
     section: "{{ repo_name }}"
     option: enabled
-    value: 1
+    value: "{{ repo_enabled | default(1) }}"