Merge pull request #5502 from mgugino-upstream-stage/fix-undefined-reg-auth

Automatic merge from submit-queue

Fix registry_auth logic for upgrades

Currently, the logic for registry authentication is
not implemented correctly to account for upgrades of
containerized hosts.

Additionally, the logic to account for multiple runs
of openshift-ansible might cause registry authentication
credentials to not be mounted inside of containerized hosts.

This commit adds the necessary logic to ensure containerized
hosts retain registry credentials.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1494470

10 files changed, 80 insertions, 26 deletions

diff --git a/roles/openshift_master/tasks/main.yml b/roles/openshift_master/tasks/main.yml
index 94b7df1fc..82b4b420c 100644
--- a/roles/openshift_master/tasks/main.yml
+++ b/roles/openshift_master/tasks/main.yml
@@ -177,8 +177,6 @@
     local_facts:
       no_proxy_etcd_host_ips: "{{ openshift_no_proxy_etcd_host_ips }}"
 
-- include: registry_auth.yml
-
 - name: Install the systemd units
   include: systemd_units.yml
 
diff --git a/roles/openshift_master/tasks/registry_auth.yml b/roles/openshift_master/tasks/registry_auth.yml
index 96b6c614e..2644f235e 100644
--- a/roles/openshift_master/tasks/registry_auth.yml
+++ b/roles/openshift_master/tasks/registry_auth.yml
@@ -1,27 +1,35 @@
 ---
+# We need to setup some variables as this play might be called directly
+# from outside of the role.
+- set_fact:
+    oreg_auth_credentials_path: "{{ r_openshift_master_data_dir }}/.docker"
+  when: oreg_auth_credentials_path is not defined
+
+- set_fact:
+    oreg_host: "{{ oreg_url.split('/')[0] if (oreg_url is defined and '.' in oreg_url.split('/')[0]) else '' }}"
+  when: oreg_host is not defined
+
 - name: Check for credentials file for registry auth
   stat:
     path: "{{ oreg_auth_credentials_path }}"
   when: oreg_auth_user is defined
   register: master_oreg_auth_credentials_stat
 
-# Container images may need the registry credentials
-- name: Setup ro mount of /root/.docker for containerized hosts
-  set_fact:
-    l_bind_docker_reg_auth: True
+- name: Create credentials for registry auth
+  command: "docker --config={{ oreg_auth_credentials_path }} login -u {{ oreg_auth_user }} -p {{ oreg_auth_password }} {{ oreg_host }}"
   when:
-  - openshift.common.is_containerized | bool
   - oreg_auth_user is defined
   - (not master_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
+  register: master_oreg_auth_credentials_create
   notify:
   - restart master api
   - restart master controllers
 
-- name: Create credentials for registry auth
-  command: "docker --config={{ oreg_auth_credentials_path }} login -u {{ oreg_auth_user }} -p {{ oreg_auth_password }} {{ oreg_host }}"
+# Container images may need the registry credentials
+- name: Setup ro mount of /root/.docker for containerized hosts
+  set_fact:
+    l_bind_docker_reg_auth: True
   when:
+  - openshift.common.is_containerized | bool
   - oreg_auth_user is defined
-  - (not master_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
-  notify:
-  - restart master api
-  - restart master controllers
+  - (master_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace or master_oreg_auth_credentials_create.changed) | bool
diff --git a/roles/openshift_master/tasks/systemd_units.yml b/roles/openshift_master/tasks/systemd_units.yml
index 7a918c57e..8de62c59a 100644
--- a/roles/openshift_master/tasks/systemd_units.yml
+++ b/roles/openshift_master/tasks/systemd_units.yml
@@ -17,6 +17,8 @@
     r_openshift_master_data_dir: "{{ openshift_data_dir | default('/var/lib/origin') }}"
   when: r_openshift_master_data_dir is not defined
 
+- include: registry_auth.yml
+
 - name: Remove the legacy master service if it exists
   include: clean_systemd_units.yml
 
diff --git a/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-api.service.j2 b/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-api.service.j2
index a184a59f6..5d4a99c97 100644
--- a/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-api.service.j2
+++ b/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-api.service.j2
@@ -20,7 +20,7 @@ ExecStart=/usr/bin/docker run --rm --privileged --net=host \
   -v {{ openshift.common.config_base }}:{{ openshift.common.config_base }} \
   {% if openshift_cloudprovider_kind | default('') != '' -%} -v {{ openshift.common.config_base }}/cloudprovider:{{ openshift.common.config_base}}/cloudprovider {% endif -%} \
   -v /etc/pki:/etc/pki:ro \
-  {% if l_bind_docker_reg_auth %} -v {{ oreg_auth_credentials_path }}:/root/.docker:ro{% endif %}\
+  {% if l_bind_docker_reg_auth | default(False) %} -v {{ oreg_auth_credentials_path }}:/root/.docker:ro{% endif %}\
   {{ openshift.master.master_image }}:${IMAGE_VERSION} start master api \
   --config=${CONFIG_FILE} $OPTIONS
 ExecStartPost=/usr/bin/sleep 10
diff --git a/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2 b/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2
index 2ded05f53..f93f3b565 100644
--- a/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2
+++ b/roles/openshift_master/templates/docker-cluster/atomic-openshift-master-controllers.service.j2
@@ -19,7 +19,7 @@ ExecStart=/usr/bin/docker run --rm --privileged --net=host \
   -v {{ openshift.common.config_base }}:{{ openshift.common.config_base }} \
   {% if openshift_cloudprovider_kind | default('') != '' -%} -v {{ openshift.common.config_base }}/cloudprovider:{{ openshift.common.config_base}}/cloudprovider {% endif -%} \
   -v /etc/pki:/etc/pki:ro \
-  {% if l_bind_docker_reg_auth %} -v {{ oreg_auth_credentials_path }}:/root/.docker:ro{% endif %}\
+  {% if l_bind_docker_reg_auth | default(False) %} -v {{ oreg_auth_credentials_path }}:/root/.docker:ro{% endif %}\
   {{ openshift.master.master_image }}:${IMAGE_VERSION} start master controllers \
   --config=${CONFIG_FILE} $OPTIONS
 ExecStartPost=/usr/bin/sleep 10
diff --git a/roles/openshift_node/tasks/registry_auth.yml b/roles/openshift_node/tasks/registry_auth.yml
index f370bb260..3d2831742 100644
--- a/roles/openshift_node/tasks/registry_auth.yml
+++ b/roles/openshift_node/tasks/registry_auth.yml
@@ -5,21 +5,20 @@
   when: oreg_auth_user is defined
   register: node_oreg_auth_credentials_stat
 
-# Container images may need the registry credentials
-- name: Setup ro mount of /root/.docker for containerized hosts
-  set_fact:
-    l_bind_docker_reg_auth: True
+- name: Create credentials for registry auth
+  command: "docker --config={{ oreg_auth_credentials_path }} login -u {{ oreg_auth_user }} -p {{ oreg_auth_password }} {{ oreg_host }}"
   when:
-    - openshift.common.is_containerized | bool
     - oreg_auth_user is defined
     - (not node_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
+  register: node_oreg_auth_credentials_create
   notify:
     - restart node
 
-- name: Create credentials for registry auth
-  command: "docker --config={{ oreg_auth_credentials_path }} login -u {{ oreg_auth_user }} -p {{ oreg_auth_password }} {{ oreg_host }}"
+# Container images may need the registry credentials
+- name: Setup ro mount of /root/.docker for containerized hosts
+  set_fact:
+    l_bind_docker_reg_auth: True
   when:
+    - openshift.common.is_containerized | bool
     - oreg_auth_user is defined
-    - (not node_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
-  notify:
-    - restart node
+    - (node_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace or oreg_auth_credentials_replace.changed) | bool
diff --git a/roles/openshift_node_upgrade/defaults/main.yml b/roles/openshift_node_upgrade/defaults/main.yml
index 3d8704308..6507b015d 100644
--- a/roles/openshift_node_upgrade/defaults/main.yml
+++ b/roles/openshift_node_upgrade/defaults/main.yml
@@ -4,3 +4,9 @@ os_sdn_network_plugin_name: "redhat/openshift-ovs-subnet"
 
 openshift_node_data_dir_default: "{{ openshift_data_dir | default('/var/lib/origin') }}"
 openshift_node_data_dir: "{{ openshift_node_data_dir_default }}"
+
+# oreg_url is defined by user input
+oreg_host: "{{ oreg_url.split('/')[0] if (oreg_url is defined and '.' in oreg_url.split('/')[0]) else '' }}"
+oreg_auth_credentials_path: "{{ openshift_node_data_dir }}/.docker"
+oreg_auth_credentials_replace: False
+l_bind_docker_reg_auth: False
diff --git a/roles/openshift_node_upgrade/tasks/main.yml b/roles/openshift_node_upgrade/tasks/main.yml
index e34319186..6bcf3072d 100644
--- a/roles/openshift_node_upgrade/tasks/main.yml
+++ b/roles/openshift_node_upgrade/tasks/main.yml
@@ -10,6 +10,8 @@
 
 # tasks file for openshift_node_upgrade
 
+- include: registry_auth.yml
+
 - name: Stop node and openvswitch services
   service:
     name: "{{ item }}"
diff --git a/roles/openshift_node_upgrade/tasks/registry_auth.yml b/roles/openshift_node_upgrade/tasks/registry_auth.yml
new file mode 100644
index 000000000..3d2831742
--- /dev/null
+++ b/roles/openshift_node_upgrade/tasks/registry_auth.yml
@@ -0,0 +1,24 @@
+---
+- name: Check for credentials file for registry auth
+  stat:
+    path: "{{ oreg_auth_credentials_path }}"
+  when: oreg_auth_user is defined
+  register: node_oreg_auth_credentials_stat
+
+- name: Create credentials for registry auth
+  command: "docker --config={{ oreg_auth_credentials_path }} login -u {{ oreg_auth_user }} -p {{ oreg_auth_password }} {{ oreg_host }}"
+  when:
+    - oreg_auth_user is defined
+    - (not node_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace) | bool
+  register: node_oreg_auth_credentials_create
+  notify:
+    - restart node
+
+# Container images may need the registry credentials
+- name: Setup ro mount of /root/.docker for containerized hosts
+  set_fact:
+    l_bind_docker_reg_auth: True
+  when:
+    - openshift.common.is_containerized | bool
+    - oreg_auth_user is defined
+    - (node_oreg_auth_credentials_stat.stat.exists or oreg_auth_credentials_replace or oreg_auth_credentials_replace.changed) | bool
diff --git a/roles/openshift_node_upgrade/templates/openshift.docker.node.service b/roles/openshift_node_upgrade/templates/openshift.docker.node.service
index 451412ab0..864e4b5d6 100644
--- a/roles/openshift_node_upgrade/templates/openshift.docker.node.service
+++ b/roles/openshift_node_upgrade/templates/openshift.docker.node.service
@@ -21,7 +21,22 @@ EnvironmentFile=/etc/sysconfig/{{ openshift.common.service_type }}-node-dep
 ExecStartPre=-/usr/bin/docker rm -f {{ openshift.common.service_type }}-node
 ExecStartPre=/usr/bin/cp /etc/origin/node/node-dnsmasq.conf /etc/dnsmasq.d/
 ExecStartPre=/usr/bin/dbus-send --system --dest=uk.org.thekelleys.dnsmasq /uk/org/thekelleys/dnsmasq uk.org.thekelleys.SetDomainServers array:string:/in-addr.arpa/127.0.0.1,/{{ openshift.common.dns_domain }}/127.0.0.1
-ExecStart=/usr/bin/docker run --name {{ openshift.common.service_type }}-node --rm --privileged --net=host --pid=host --env-file=/etc/sysconfig/{{ openshift.common.service_type }}-node -v /:/rootfs:ro,rslave -e CONFIG_FILE=${CONFIG_FILE} -e OPTIONS=${OPTIONS} -e HOST=/rootfs -e HOST_ETC=/host-etc -v {{ openshift_node_data_dir }}:{{ openshift_node_data_dir }}{{ ':rslave' if openshift.docker.gte_1_10 | default(False) | bool else '' }} -v {{ openshift.common.config_base }}/node:{{ openshift.common.config_base }}/node {% if openshift_cloudprovider_kind | default('') != '' -%} -v {{ openshift.common.config_base }}/cloudprovider:{{ openshift.common.config_base}}/cloudprovider {% endif -%} -v /etc/localtime:/etc/localtime:ro -v /etc/machine-id:/etc/machine-id:ro -v /run:/run -v /sys:/sys:rw -v /sys/fs/cgroup:/sys/fs/cgroup:rw -v /usr/bin/docker:/usr/bin/docker:ro -v /var/lib/docker:/var/lib/docker -v /lib/modules:/lib/modules -v /etc/origin/openvswitch:/etc/openvswitch -v /etc/origin/sdn:/etc/openshift-sdn -v /var/lib/cni:/var/lib/cni -v /etc/systemd/system:/host-etc/systemd/system -v /var/log:/var/log -v /dev:/dev $DOCKER_ADDTL_BIND_MOUNTS -v /etc/pki:/etc/pki:ro {{ openshift.node.node_image }}:${IMAGE_VERSION}
+ExecStart=/usr/bin/docker run --name {{ openshift.common.service_type }}-node \
+  --rm --privileged --net=host --pid=host --env-file=/etc/sysconfig/{{ openshift.common.service_type }}-node \
+  -v /:/rootfs:ro,rslave -e CONFIG_FILE=${CONFIG_FILE} -e OPTIONS=${OPTIONS} \
+  -e HOST=/rootfs -e HOST_ETC=/host-etc \
+  -v {{ openshift_node_data_dir }}:{{ openshift_node_data_dir }}{{ ':rslave' if openshift.docker.gte_1_10 | default(False) | bool else '' }} \
+  -v {{ openshift.common.config_base }}/node:{{ openshift.common.config_base }}/node \
+  {% if openshift_cloudprovider_kind | default('') != '' -%} -v {{ openshift.common.config_base }}/cloudprovider:{{ openshift.common.config_base}}/cloudprovider {% endif -%} \
+  -v /etc/localtime:/etc/localtime:ro -v /etc/machine-id:/etc/machine-id:ro \
+  -v /run:/run -v /sys:/sys:rw -v /sys/fs/cgroup:/sys/fs/cgroup:rw \
+  -v /usr/bin/docker:/usr/bin/docker:ro -v /var/lib/docker:/var/lib/docker \
+  -v /lib/modules:/lib/modules -v /etc/origin/openvswitch:/etc/openvswitch \
+  -v /etc/origin/sdn:/etc/openshift-sdn -v /var/lib/cni:/var/lib/cni \
+  -v /etc/systemd/system:/host-etc/systemd/system -v /var/log:/var/log \
+  -v /dev:/dev $DOCKER_ADDTL_BIND_MOUNTS -v /etc/pki:/etc/pki:ro \
+  {% if l_bind_docker_reg_auth %} -v {{ oreg_auth_credentials_path }}:/root/.docker:ro{% endif %}\
+  {{ openshift.node.node_image }}:${IMAGE_VERSION}
 ExecStartPost=/usr/bin/sleep 10
 ExecStop=/usr/bin/docker stop {{ openshift.common.service_type }}-node
 ExecStopPost=/usr/bin/rm /etc/dnsmasq.d/node-dnsmasq.conf