From edde1f2bf0fa180fc69d905ce2aa27e68f7295dd Mon Sep 17 00:00:00 2001 From: Michael Gugino Date: Wed, 3 Jan 2018 13:55:29 -0500 Subject: Fix docker_image_availability checks This commit ensures that oreg_url is properly templated by ansible before being consumed in the logic. This commit also adds a method to the base health check class to detect if self._templar is none, and return the appropriate templated/untemplated version of the variable. This is mostly for unit tests. --- roles/openshift_health_checker/openshift_checks/__init__.py | 7 +++++++ .../openshift_checks/docker_image_availability.py | 10 ++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py index b7b16e0ea..83e551b5d 100644 --- a/roles/openshift_health_checker/openshift_checks/__init__.py +++ b/roles/openshift_health_checker/openshift_checks/__init__.py @@ -95,6 +95,13 @@ class OpenShiftCheck(object): # These are intended to be a sequential record of what the check observed and determined. self.logs = [] + def template_var(self, var_to_template): + """Return a templated variable if self._templar is not None, else + just return the variable as-is""" + if self._templar is not None: + return self._templar.template(var_to_template) + return var_to_template + @abstractproperty def name(self): """The name of this check, usually derived from the class name.""" 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 744b79c1a..7afb8f730 100644 --- a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py +++ b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py @@ -64,7 +64,9 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): self.registries["configured"] = regs # for the oreg_url registry there may be credentials specified - components = self.get_var("oreg_url", default="").split('/') + oreg_url = self.get_var("oreg_url", default="") + oreg_url = self.template_var(oreg_url) + components = oreg_url.split('/') self.registries["oreg"] = "" if len(components) < 3 else components[0] # Retrieve and template registry credentials, if provided @@ -72,9 +74,8 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): 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 != '': - if self._templar is not None: - oreg_auth_user = self._templar.template(oreg_auth_user) - oreg_auth_password = self._templar.template(oreg_auth_password) + oreg_auth_user = self.template_var(oreg_auth_user) + oreg_auth_password = self.template_var(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) @@ -153,6 +154,7 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): # template for images that run on top of OpenShift image_url = "{}/{}-{}:{}".format(image_info["namespace"], image_info["name"], "${component}", "${version}") image_url = self.get_var("oreg_url", default="") or image_url + image_url = self.template_var(image_url) if 'oo_nodes_to_config' in host_groups: for suffix in NODE_IMAGE_SUFFIXES: required.add(image_url.replace("${component}", suffix).replace("${version}", image_tag)) -- cgit v1.2.3 From 7923eb92c86a128504436ba0708c96b655de5269 Mon Sep 17 00:00:00 2001 From: Michael Gugino Date: Wed, 3 Jan 2018 15:10:26 -0500 Subject: Remove become=no from various roles and tasks etcd runs some actions locally to copy certs from the CA cert host. This commit ensures that we respect the end user's intended behavior with become when using 'anisble_become' in the inventory. Other roles with similar tasks have been modified in the same manner. We shouldn't hard-code become behavior as it can be unexpected for the end user. This only currently works in the CI because the CI passes the '-b' argument on the command line, which will override the task behavior. --- .../fetch_client_certificates_from_ca.yml | 23 +++++++--------------- .../fetch_server_certificates_from_ca.yml | 18 ++++------------- roles/openshift_examples/tasks/main.yml | 12 +++++++---- roles/openshift_hosted_templates/tasks/main.yml | 12 +++++++---- roles/openshift_node_certificates/tasks/main.yml | 15 +++----------- 5 files changed, 30 insertions(+), 50 deletions(-) (limited to 'roles') diff --git a/roles/etcd/tasks/certificates/fetch_client_certificates_from_ca.yml b/roles/etcd/tasks/certificates/fetch_client_certificates_from_ca.yml index d4518554c..78578a055 100644 --- a/roles/etcd/tasks/certificates/fetch_client_certificates_from_ca.yml +++ b/roles/etcd/tasks/certificates/fetch_client_certificates_from_ca.yml @@ -79,13 +79,6 @@ when: etcd_client_certs_missing | bool delegate_to: "{{ etcd_ca_host }}" -- name: Create local temp directory for syncing certs - local_action: command mktemp -d /tmp/etcd_certificates-XXXXXXX - register: g_etcd_client_mktemp - changed_when: False - when: etcd_client_certs_missing | bool - become: no - - name: Create a tarball of the etcd certs command: > tar -czvf {{ etcd_generated_certs_dir }}/{{ etcd_cert_subdir }}.tgz @@ -101,8 +94,7 @@ - name: Retrieve the etcd cert tarballs fetch: src: "{{ etcd_generated_certs_dir }}/{{ etcd_cert_subdir }}.tgz" - dest: "{{ g_etcd_client_mktemp.stdout }}/" - flat: yes + dest: "/tmp" fail_on_missing: yes validate_checksum: yes when: etcd_client_certs_missing | bool @@ -116,10 +108,15 @@ - name: Unarchive etcd cert tarballs unarchive: - src: "{{ g_etcd_client_mktemp.stdout }}/{{ etcd_cert_subdir }}.tgz" + src: "/tmp/{{ inventory_hostname }}/{{ etcd_generated_certs_dir }}/{{ etcd_cert_subdir }}.tgz" dest: "{{ etcd_cert_config_dir }}" when: etcd_client_certs_missing | bool +- name: Delete temporary directory + local_action: file path="/tmp/{{ inventory_hostname }}" state=absent + changed_when: False + when: etcd_client_certs_missing | bool + - file: path: "{{ etcd_cert_config_dir }}/{{ item }}" owner: root @@ -130,9 +127,3 @@ - "{{ etcd_cert_prefix }}client.key" - "{{ etcd_cert_prefix }}ca.crt" when: etcd_client_certs_missing | bool - -- name: Delete temporary directory - local_action: file path="{{ g_etcd_client_mktemp.stdout }}" state=absent - changed_when: False - when: etcd_client_certs_missing | bool - become: no diff --git a/roles/etcd/tasks/certificates/fetch_server_certificates_from_ca.yml b/roles/etcd/tasks/certificates/fetch_server_certificates_from_ca.yml index 59a6b6590..987380d0c 100644 --- a/roles/etcd/tasks/certificates/fetch_server_certificates_from_ca.yml +++ b/roles/etcd/tasks/certificates/fetch_server_certificates_from_ca.yml @@ -105,13 +105,6 @@ when: etcd_server_certs_missing | bool delegate_to: "{{ etcd_ca_host }}" -- name: Create local temp directory for syncing certs - local_action: command mktemp -d /tmp/etcd_certificates-XXXXXXX - become: no - register: g_etcd_server_mktemp - changed_when: False - when: etcd_server_certs_missing | bool - - name: Create a tarball of the etcd certs command: > tar -czvf {{ etcd_generated_certs_dir }}/{{ etcd_cert_subdir }}.tgz @@ -127,8 +120,7 @@ - name: Retrieve etcd cert tarball fetch: src: "{{ etcd_generated_certs_dir }}/{{ etcd_cert_subdir }}.tgz" - dest: "{{ g_etcd_server_mktemp.stdout }}/" - flat: yes + dest: "/tmp" fail_on_missing: yes validate_checksum: yes when: etcd_server_certs_missing | bool @@ -144,7 +136,7 @@ - name: Unarchive cert tarball unarchive: - src: "{{ g_etcd_server_mktemp.stdout }}/{{ etcd_cert_subdir }}.tgz" + src: "/tmp/{{ inventory_hostname }}/{{ etcd_generated_certs_dir }}/{{ etcd_cert_subdir }}.tgz" dest: "{{ etcd_cert_config_dir }}" when: etcd_server_certs_missing | bool @@ -161,8 +153,7 @@ - name: Retrieve etcd ca cert tarball fetch: src: "{{ etcd_generated_certs_dir }}/{{ etcd_ca_name }}.tgz" - dest: "{{ g_etcd_server_mktemp.stdout }}/" - flat: yes + dest: "/tmp" fail_on_missing: yes validate_checksum: yes when: etcd_server_certs_missing | bool @@ -177,8 +168,7 @@ when: etcd_server_certs_missing | bool - name: Delete temporary directory - local_action: file path="{{ g_etcd_server_mktemp.stdout }}" state=absent - become: no + local_action: file path="/tmp/{{ inventory_hostname }}" state=absent changed_when: False when: etcd_server_certs_missing | bool diff --git a/roles/openshift_examples/tasks/main.yml b/roles/openshift_examples/tasks/main.yml index a09a598bd..ff04cdf9c 100644 --- a/roles/openshift_examples/tasks/main.yml +++ b/roles/openshift_examples/tasks/main.yml @@ -13,18 +13,23 @@ # use it either due to changes introduced in Ansible 2.x. - name: Create local temp dir for OpenShift examples copy local_action: command mktemp -d /tmp/openshift-ansible-XXXXXXX - become: False register: copy_examples_mktemp run_once: True +- name: Create local temp dir for OpenShift examples copy + local_action: command chmod 755 "{{ copy_examples_mktemp.stdout }}" + run_once: True + - name: Create tar of OpenShift examples local_action: command tar -C "{{ role_path }}/files/examples/{{ content_version }}/" -cvf "{{ copy_examples_mktemp.stdout }}/openshift-examples.tar" . args: # Disables the following warning: # Consider using unarchive module rather than running tar warn: no - become: False - register: copy_examples_tar + +- name: Create local temp dir for OpenShift examples copy + local_action: command chmod 744 "{{ copy_examples_mktemp.stdout }}/openshift-examples.tar" + run_once: True - name: Create the remote OpenShift examples directory file: @@ -38,7 +43,6 @@ dest: "{{ examples_base }}/" - name: Cleanup the OpenShift Examples temp dir - become: False local_action: file dest="{{ copy_examples_mktemp.stdout }}" state=absent # Done copying examples diff --git a/roles/openshift_hosted_templates/tasks/main.yml b/roles/openshift_hosted_templates/tasks/main.yml index b2313c297..672d25b4d 100644 --- a/roles/openshift_hosted_templates/tasks/main.yml +++ b/roles/openshift_hosted_templates/tasks/main.yml @@ -1,20 +1,25 @@ --- - name: Create local temp dir for OpenShift hosted templates copy local_action: command mktemp -d /tmp/openshift-ansible-XXXXXXX - become: False register: copy_hosted_templates_mktemp run_once: True # AUDIT:changed_when: not set here because this task actually # creates something +- name: Create local temp dir for OpenShift examples copy + local_action: command chmod 755 "{{ copy_hosted_templates_mktemp.stdout }}" + run_once: True + - name: Create tar of OpenShift examples local_action: command tar -C "{{ role_path }}/files/{{ content_version }}/{{ hosted_deployment_type }}" -cvf "{{ copy_hosted_templates_mktemp.stdout }}/openshift-hosted-templates.tar" . args: # Disables the following warning: # Consider using unarchive module rather than running tar warn: no - become: False - register: copy_hosted_templates_tar + +- name: Create local temp dir for OpenShift examples copy + local_action: command chmod 744 "{{ copy_hosted_templates_mktemp.stdout }}/openshift-hosted-templates.tar" + run_once: True - name: Create remote OpenShift hosted templates directory file: @@ -28,7 +33,6 @@ dest: "{{ hosted_base }}/" - name: Cleanup the OpenShift hosted templates temp dir - become: False local_action: file dest="{{ copy_hosted_templates_mktemp.stdout }}" state=absent - name: Modify registry paths if registry_url is not registry.access.redhat.com diff --git a/roles/openshift_node_certificates/tasks/main.yml b/roles/openshift_node_certificates/tasks/main.yml index e95e38fdf..5f73f3bdc 100644 --- a/roles/openshift_node_certificates/tasks/main.yml +++ b/roles/openshift_node_certificates/tasks/main.yml @@ -94,13 +94,6 @@ delegate_to: "{{ openshift_ca_host }}" run_once: true -- name: Create local temp directory for syncing certs - local_action: command mktemp -d /tmp/openshift-ansible-XXXXXXX - register: node_cert_mktemp - changed_when: False - when: node_certs_missing | bool - become: no - - name: Create a tarball of the node config directories command: > tar -czvf {{ openshift_node_generated_config_dir }}.tgz @@ -117,8 +110,7 @@ - name: Retrieve the node config tarballs from the master fetch: src: "{{ openshift_node_generated_config_dir }}.tgz" - dest: "{{ node_cert_mktemp.stdout }}/" - flat: yes + dest: "/tmp" fail_on_missing: yes validate_checksum: yes when: node_certs_missing | bool @@ -132,15 +124,14 @@ - name: Unarchive the tarball on the node unarchive: - src: "{{ node_cert_mktemp.stdout }}/{{ openshift_node_cert_subdir }}.tgz" + src: "/tmp/{{ inventory_hostname }}/{{ openshift_node_generated_config_dir }}.tgz" dest: "{{ openshift_node_cert_dir }}" when: node_certs_missing | bool - name: Delete local temp directory - local_action: file path="{{ node_cert_mktemp.stdout }}" state=absent + local_action: file path="/tmp/{{ inventory_hostname }}" state=absent changed_when: False when: node_certs_missing | bool - become: no - name: Copy OpenShift CA to system CA trust copy: -- cgit v1.2.3