From d6fd0d2acaec9f53e75d82db30411f96a5bf2cc9 Mon Sep 17 00:00:00 2001
From: Timoses <timosesu@gmail.com>
Date: Wed, 1 May 2019 10:10:56 +0200
Subject: [PATCH] Enable delegating all downloads (binaries, images, kubeadm
 images) (#4420)

* Download to delegate and sync files when download_run_once

* Fail on error after saving container image

* Do not set changed status when downloaded container was up to date

* Only sync containers when they are actually required

Previously, non-required images (pull_required=false as
image existed on target host) were synced to the target
hosts. This failed as the image was not downloaded to
the download_delegate and hence was not available for
syncing.

* Sync containers when only missing on some hosts

* Consider images with multiple repo tags

* Enable kubeadm images pull/syncing with download_delegate

* Use kubeadm images list to pull/sync

'kubeadm config images pull' is replaced by collecting the images
list with 'kubeadm config images list' and using the commonly
used method of pull/syncing the images.

* Ensure containers are downloaded and synced for all hosts

* Fix download/syncing when download_delegate is a kubernetes host
---
 docs/downloads.md                             | 10 ++--
 roles/download/defaults/main.yml              |  4 +-
 roles/download/tasks/download_container.yml   |  7 ++-
 roles/download/tasks/download_file.yml        | 29 +++++++++-
 roles/download/tasks/kubeadm_images.yml       | 50 ++++++++++++++++-
 roles/download/tasks/main.yml                 | 51 ++++++++++--------
 .../download/tasks/set_docker_image_facts.yml | 11 +++-
 roles/download/tasks/sync_container.yml       | 15 +++---
 roles/download/tasks/sync_file.yml            | 53 +++++++++++++++++++
 roles/kubernetes/master/tasks/main.yml        |  2 +-
 roles/kubernetes/node/tasks/install.yml       |  6 +--
 11 files changed, 188 insertions(+), 50 deletions(-)
 create mode 100644 roles/download/tasks/sync_file.yml

diff --git a/docs/downloads.md b/docs/downloads.md
index dc6f5cf30..21f91fb68 100644
--- a/docs/downloads.md
+++ b/docs/downloads.md
@@ -12,14 +12,14 @@ Kubespray supports several download/upload modes. The default is:
 
 There is also a "pull once, push many" mode as well:
 
-* Override the ``download_run_once: True`` to download container images only once
+* Override the ``download_run_once: True`` to download container images and binaries only once
   then push to cluster nodes in batches. The default delegate node
-  for pushing images is the first `kube-master`.
+  for pushing is the first `kube-master`.
 * If your ansible runner node (aka the admin node) have password-less sudo and
   docker enabled, you may want to define the ``download_localhost: True``, which
-  makes that node a delegate for pushing images while running the deployment with
-  ansible. This maybe the case if cluster nodes cannot access each over via ssh
-  or you want to use local docker images as a cache for multiple clusters.
+  makes that node a delegate for pushing while running the deployment with
+  ansible. This may be the case if cluster nodes cannot access each other via ssh
+  or you want to use local docker images and binaries as a cache for multiple clusters.
 
 Container images and binary files are described by the vars like ``foo_version``,
 ``foo_download_url``, ``foo_checksum`` for binaries and ``foo_image_repo``,
diff --git a/roles/download/defaults/main.yml b/roles/download/defaults/main.yml
index ae897c6e5..a661c0d00 100644
--- a/roles/download/defaults/main.yml
+++ b/roles/download/defaults/main.yml
@@ -321,7 +321,7 @@ downloads:
     enabled: true
     file: true
     version: "{{ kubeadm_version }}"
-    dest: "{{local_release_dir}}/kubeadm"
+    dest: "{{ local_release_dir }}/kubeadm-{{ kubeadm_version }}-{{ image_arch }}"
     sha256: "{{ kubeadm_binary_checksum }}"
     url: "{{ kubeadm_download_url }}"
     unarchive: false
@@ -334,7 +334,7 @@ downloads:
     enabled: true
     file: true
     version: "{{ kube_version }}"
-    dest: "{{ local_release_dir }}/hyperkube"
+    dest: "{{ local_release_dir }}/hyperkube-{{ kube_version }}-{{ image_arch }}"
     sha256: "{{ hyperkube_binary_checksum }}"
     url: "{{ hyperkube_download_url }}"
     unarchive: false
diff --git a/roles/download/tasks/download_container.yml b/roles/download/tasks/download_container.yml
index bf0c9b1cf..12cc67c3a 100644
--- a/roles/download/tasks/download_container.yml
+++ b/roles/download/tasks/download_container.yml
@@ -1,9 +1,6 @@
 ---
 - name: container_download | Make download decision if pull is required by tag or sha256
   include_tasks: set_docker_image_facts.yml
-  delegate_to: "{{ download_delegate if download_run_once or omit }}"
-  delegate_facts: yes
-  run_once: "{{ download_run_once }}"
   when:
     - download.enabled
     - download.container
@@ -18,11 +15,12 @@
   until: pull_task_result is succeeded
   retries: 4
   delay: "{{ retry_stagger | random + 3 }}"
+  changed_when: not 'up to date' in pull_task_result.stdout
   when:
     - download_run_once
     - download.enabled
     - download.container
-    - pull_required|default(download_always_pull)
+    - any_pull_required | default(download_always_pull)
   delegate_to: "{{ download_delegate }}"
   delegate_facts: yes
   run_once: yes
@@ -33,6 +31,7 @@
   until: pull_task_result is succeeded
   retries: 4
   delay: "{{ retry_stagger | random + 3 }}"
+  changed_when: not 'up to date' in pull_task_result.stdout
   when:
     - not download_run_once
     - download.enabled
diff --git a/roles/download/tasks/download_file.yml b/roles/download/tasks/download_file.yml
index b55639ffd..3993ef6b6 100644
--- a/roles/download/tasks/download_file.yml
+++ b/roles/download/tasks/download_file.yml
@@ -15,7 +15,33 @@
     - download.file
     - group_names | intersect(download.groups) | length
 
-- name: file_download | Download item
+# As in 'download_container.yml':
+#   In Ansible 2.4 omitting download delegate is broken. Move back
+#   to one task in the future.
+- name: file_download | Download item (delegate)
+  get_url:
+    url: "{{download.url}}"
+    dest: "{{download.dest}}"
+    sha256sum: "{{download.sha256 | default(omit)}}"
+    owner: "{{ download.owner|default(omit) }}"
+    mode: "{{ download.mode|default(omit) }}"
+    validate_certs: "{{ download_validate_certs }}"
+    url_username: "{{ download.username|default(omit) }}"
+    url_password: "{{ download.password|default(omit) }}"
+    force_basic_auth: "{{ download.force_basic_auth|default(omit) }}"
+  register: get_url_result
+  until: "'OK' in get_url_result.msg or 'file already exists' in get_url_result.msg"
+  retries: 4
+  delay: "{{ retry_stagger | default(5) }}"
+  delegate_to: "{{ download_delegate }}"
+  when:
+    - download_run_once
+    - download.enabled
+    - download.file
+    - group_names | intersect(download.groups) | length
+  run_once: yes
+
+- name: file_download | Download item (all)
   get_url:
     url: "{{download.url}}"
     dest: "{{download.dest}}"
@@ -31,6 +57,7 @@
   retries: 4
   delay: "{{ retry_stagger | default(5) }}"
   when:
+    - not download_run_once
     - download.enabled
     - download.file
     - group_names | intersect(download.groups) | length
diff --git a/roles/download/tasks/kubeadm_images.yml b/roles/download/tasks/kubeadm_images.yml
index a166a35ab..8257bccc1 100644
--- a/roles/download/tasks/kubeadm_images.yml
+++ b/roles/download/tasks/kubeadm_images.yml
@@ -1,4 +1,22 @@
 ---
+- name: kubeadm | Download kubeadm
+  include_tasks: "download_file.yml"
+  vars:
+    download: "{{ download_defaults | combine(downloads.kubeadm) }}"
+  when:
+    - not skip_downloads|default(false)
+    - downloads.kubeadm.enabled
+
+- name: kubeadm | Sync kubeadm
+  include_tasks: "sync_file.yml"
+  vars:
+    download: "{{ download_defaults | combine(downloads.kubeadm) }}"
+  when:
+    - not skip_downloads|default(false)
+    - downloads.kubeadm.enabled
+    - download_run_once
+    - group_names | intersect(download.groups) | length
+
 - name: kubeadm | Create kubeadm config
   template:
     src: "kubeadm-images.yaml.j2"
@@ -6,7 +24,7 @@
 
 - name: kubeadm | Copy kubeadm binary from download dir
   synchronize:
-    src: "{{ local_release_dir }}/kubeadm"
+    src: "{{ local_release_dir }}/kubeadm-{{ kubeadm_version }}-{{ image_arch }}"
     dest: "{{ bin_dir }}/kubeadm"
     compress: no
     perms: yes
@@ -22,3 +40,33 @@
 
 - name: container_download | download images for kubeadm config images
   command: "{{ bin_dir }}/kubeadm config images pull --config={{ kube_config_dir }}/kubeadm-images.yaml"
+  when: not download_run_once
+
+- name: container_download | fetch list of kubeadm config images
+  command: "{{ bin_dir }}/kubeadm config images list --config={{ kube_config_dir }}/kubeadm-images.yaml"
+  register: result
+  run_once: true
+  when: download_run_once
+  changed_when: false
+
+- vars:
+    kubeadm_images_list: "{{ result.stdout_lines }}"
+  set_fact:
+    kubeadm_image:
+      key: "kubeadm_{{ (item | regex_replace('^(?:.*\\/)*','')).split(':')[0] }}"
+      value:
+        enabled: true
+        container: true
+        repo: "{{ item.split(':')[0] }}"
+        tag: "{{ item.split(':')[1] }}"
+        groups:
+          - k8s-cluster
+  loop: "{{ kubeadm_images_list | flatten(levels=1) }}"
+  run_once: true
+  when: download_run_once
+  register: result_images
+
+- set_fact:
+    kubeadm_images: "{{ result_images.results | map(attribute='ansible_facts.kubeadm_image') | list | items2dict }}"
+  run_once: true
+  when: download_run_once
diff --git a/roles/download/tasks/main.yml b/roles/download/tasks/main.yml
index 06ad9505a..f9764ae6a 100644
--- a/roles/download/tasks/main.yml
+++ b/roles/download/tasks/main.yml
@@ -3,31 +3,36 @@
   when:
     - not skip_downloads|default(false)
 
-- name: "Download items"
-  include_tasks: "{{ include_file }}"
-  vars:
-    download: "{{ download_defaults | combine(item.value) }}"
-    include_file: "download_{% if download.container %}container{% else %}file{% endif %}.yml"
-  with_dict: "{{ downloads }}"
-  when:
-    - not skip_downloads|default(false)
-    - item.value.enabled
-    - (not (item.value.container|default(False))) or (item.value.container and download_container)
-
-- name: "Sync container"
-  include_tasks: sync_container.yml
-  vars:
-    download: "{{ download_defaults | combine(item.value) }}"
-  with_dict: "{{ downloads }}"
-  when:
-    - not skip_downloads|default(false)
-    - item.value.enabled
-    - item.value.container | default(false)
-    - download_run_once
-    - group_names | intersect(download.groups) | length
-
 - include_tasks: kubeadm_images.yml
   when:
     - kube_version is version('v1.11.0', '>=')
     - not skip_downloads|default(false)
     - inventory_hostname in groups['kube-master']
+
+- set_fact:
+    kubeadm_images: {}
+  when:
+    - kubeadm_images is not defined
+
+- name: "Download items"
+  include_tasks: "{{ include_file }}"
+  vars:
+    download: "{{ download_defaults | combine(item.value) }}"
+    include_file: "download_{% if download.container %}container{% else %}file{% endif %}.yml"
+  with_dict: "{{ downloads | combine(kubeadm_images) }}"
+  when:
+    - not skip_downloads|default(false)
+    - item.value.enabled
+    - (not (item.value.container|default(False))) or (item.value.container and download_container)
+
+- name: "Sync items"
+  include_tasks: "{{ include_file }}"
+  vars:
+    download: "{{ download_defaults | combine(item.value) }}"
+    include_file: "sync_{% if download.container %}container{% else %}file{% endif %}.yml"
+  with_dict: "{{ downloads | combine(kubeadm_images) }}"
+  when:
+    - not skip_downloads|default(false)
+    - item.value.enabled
+    - download_run_once
+    - group_names | intersect(download.groups) | length
diff --git a/roles/download/tasks/set_docker_image_facts.yml b/roles/download/tasks/set_docker_image_facts.yml
index 935f1bf3f..84ed88760 100644
--- a/roles/download/tasks/set_docker_image_facts.yml
+++ b/roles/download/tasks/set_docker_image_facts.yml
@@ -9,7 +9,7 @@
 
 - name: Register docker images info
   shell: >-
-    {{ docker_bin_dir }}/docker images -q | xargs {{ docker_bin_dir }}/docker inspect -f "{{ '{{' }} if .RepoTags {{ '}}' }}{{ '{{' }} (index .RepoTags 0) {{ '}}' }}{{ '{{' }} end {{ '}}' }}{{ '{{' }} if .RepoDigests {{ '}}' }},{{ '{{' }} (index .RepoDigests 0) {{ '}}' }}{{ '{{' }} end {{ '}}' }}" | tr '\n' ','
+    {{ docker_bin_dir }}/docker images -q | xargs -r {{ docker_bin_dir }}/docker inspect -f "{{ '{{' }} if .RepoTags {{ '}}' }}{{ '{{' }} (index .RepoTags) {{ '}}' }}{{ '{{' }} end {{ '}}' }}{{ '{{' }} if .RepoDigests {{ '}}' }},{{ '{{' }} (index .RepoDigests) {{ '}}' }}{{ '{{' }} end {{ '}}' }}" | sed -e 's/^ *\[//g' -e 's/\] *$//g' -e 's/ /\n/g' | tr '\n' ','
   no_log: true
   register: docker_images
   failed_when: false
@@ -22,6 +22,15 @@
       {%- if pull_args in docker_images.stdout.split(',') %}false{%- else -%}true{%- endif -%}
   when: not download_always_pull
 
+- name: Does any host require container pull?
+  vars:
+    hosts_pull_required: "{{ hostvars.values() | map(attribute='pull_required') | select('defined') | list }}"
+  set_fact:
+    any_pull_required: "{{ True in hosts_pull_required }}"
+  run_once: true
+  changed_when: false
+  when: not download_always_pull
+
 - name: Check the local digest sha256 corresponds to the given image tag
   assert:
     that: "{{download.repo}}:{{download.tag}} in docker_images.stdout.split(',')"
diff --git a/roles/download/tasks/sync_container.yml b/roles/download/tasks/sync_container.yml
index 4af3bdcb6..aeac6fd15 100644
--- a/roles/download/tasks/sync_container.yml
+++ b/roles/download/tasks/sync_container.yml
@@ -1,9 +1,6 @@
 ---
 - name: container_download | Make download decision if pull is required by tag or sha256
   include: set_docker_image_facts.yml
-  delegate_to: "{{ download_delegate if download_run_once or omit }}"
-  delegate_facts: no
-  run_once: "{{ download_run_once }}"
   when:
     - download.enabled
     - download.container
@@ -54,6 +51,7 @@
     - download.enabled
     - download.container
     - download_run_once
+    - any_pull_required | default(download_always_pull)
   tags:
     - facts
 
@@ -62,11 +60,12 @@
   delegate_to: "{{ download_delegate }}"
   delegate_facts: no
   register: saved
-  run_once: true
+  failed_when: saved.stderr != ""
   when:
     - download.enabled
     - download.container
     - download_run_once
+    - any_pull_required | default(download_always_pull)
     - (ansible_os_family not in ["CoreOS", "Container Linux by CoreOS"] or download_delegate == "localhost")
     - (container_changed or not img.stat.exists)
 
@@ -82,6 +81,7 @@
     - download.enabled
     - download.container
     - download_run_once
+    - any_pull_required | default(download_always_pull)
     - ansible_os_family not in ["CoreOS", "Container Linux by CoreOS"]
     - inventory_hostname == download_delegate
     - download_delegate != "localhost"
@@ -94,9 +94,6 @@
     use_ssh_args: "{{ has_bastion | default(false) }}"
     mode: pull
     private_key: "{{ ansible_ssh_private_key_file }}"
-  delegate_to: localhost
-  delegate_facts: no
-  run_once: true
   become: false
   when:
     - download.enabled
@@ -113,8 +110,6 @@
     dest: "{{ fname }}"
     use_ssh_args: "{{ has_bastion | default(false) }}"
     mode: push
-  delegate_to: localhost
-  delegate_facts: no
   become: true
   register: get_task
   until: get_task is succeeded
@@ -124,6 +119,7 @@
     - download.enabled
     - download.container
     - download_run_once
+    - pull_required|default(download_always_pull)
     - (ansible_os_family not in ["CoreOS", "Container Linux by CoreOS"] and
       inventory_hostname != download_delegate or
       download_delegate == "localhost")
@@ -137,6 +133,7 @@
     - download.enabled
     - download.container
     - download_run_once
+    - pull_required|default(download_always_pull)
     - (ansible_os_family not in ["CoreOS", "Container Linux by CoreOS"] and
       inventory_hostname != download_delegate or download_delegate == "localhost")
   tags:
diff --git a/roles/download/tasks/sync_file.yml b/roles/download/tasks/sync_file.yml
new file mode 100644
index 000000000..530a8237d
--- /dev/null
+++ b/roles/download/tasks/sync_file.yml
@@ -0,0 +1,53 @@
+---
+- name: file_download | create local download destination directory
+  file:
+    path: "{{download.dest|dirname}}"
+    state: directory
+    recurse: yes
+    mode: 0755
+  delegate_to: localhost
+  become: false
+  run_once: true
+  when:
+    - download_delegate != "localhost"
+    - download_run_once
+    - download.enabled
+    - download.file
+
+- name: file_download | copy file to ansible host
+  synchronize:
+    src: "{{ download.dest }}"
+    dest: "{{ download.dest }}"
+    use_ssh_args: "{{ has_bastion | default(false) }}"
+    mode: pull
+  run_once: true
+  become: false
+  when:
+    - download.enabled
+    - download.file
+    - download_run_once
+    - ansible_os_family not in ["CoreOS", "Container Linux by CoreOS"]
+    - inventory_hostname == download_delegate
+    - download_delegate != "localhost"
+
+- name: file_download | upload file to nodes
+  synchronize:
+    src: "{{ download.dest }}"
+    dest: "{{ download.dest }}"
+    use_ssh_args: "{{ has_bastion | default(false) }}"
+    mode: push
+  become: true
+  register: get_task
+  until: get_task is succeeded
+  retries: 4
+  delay: "{{ retry_stagger | random + 3 }}"
+  when:
+    - download.enabled
+    - download.file
+    - download_run_once
+    - (ansible_os_family not in ["CoreOS", "Container Linux by CoreOS"] and
+      inventory_hostname != download_delegate or
+      download_delegate == "localhost")
+  tags:
+    - upload
+    - upgrade
diff --git a/roles/kubernetes/master/tasks/main.yml b/roles/kubernetes/master/tasks/main.yml
index 31627fb33..9094ab6cb 100644
--- a/roles/kubernetes/master/tasks/main.yml
+++ b/roles/kubernetes/master/tasks/main.yml
@@ -19,7 +19,7 @@
 
 - name: Install | Copy kubectl binary from download dir
   synchronize:
-    src: "{{ local_release_dir }}/hyperkube"
+    src: "{{ local_release_dir }}/hyperkube-{{ kube_version }}-{{ image_arch }}"
     dest: "{{ bin_dir }}/kubectl"
     compress: no
     perms: yes
diff --git a/roles/kubernetes/node/tasks/install.yml b/roles/kubernetes/node/tasks/install.yml
index d5ef4279a..52ad10b6a 100644
--- a/roles/kubernetes/node/tasks/install.yml
+++ b/roles/kubernetes/node/tasks/install.yml
@@ -1,7 +1,7 @@
 ---
 - name: install | Copy kubeadm binary from download dir
   synchronize:
-    src: "{{ local_release_dir }}/kubeadm"
+    src: "{{ local_release_dir }}/kubeadm-{{ kubeadm_version }}-{{ image_arch }}"
     dest: "{{ bin_dir }}/kubeadm"
     compress: no
     perms: yes
@@ -25,7 +25,7 @@
 
 - name: install | Copy kubelet binary from download dir
   synchronize:
-    src: "{{ local_release_dir }}/hyperkube"
+    src: "{{ local_release_dir }}/hyperkube-{{ kube_version }}-{{ image_arch }}"
     dest: "{{ bin_dir }}/kubelet"
     compress: no
     perms: yes
@@ -48,7 +48,7 @@
 
 - name: install | Copy hyperkube binary from download dir
   synchronize:
-    src: "{{ local_release_dir }}/hyperkube"
+    src: "{{ local_release_dir }}/hyperkube-{{ kube_version }}-{{ image_arch }}"
     dest: "{{ bin_dir }}/hyperkube"
     compress: no
     perms: yes