Merge pull request #6597 from mgugino-upstream-stage/etc-remove-become-no

Automatic merge from submit-queue.

Remove become=no from etcd cert tasks

etcd runs some actions locally to copy certs from the
    CA cert host.
    
    We shouldn't hard-code become behavior as it can be
    unexpected for the end user.

7 files changed, 43 insertions, 54 deletions

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_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))
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: