From d7730980c9a45e93b314e5971e876e11b2ce12a9 Mon Sep 17 00:00:00 2001 From: Jose Luis Franco Arza Date: Wed, 20 Jan 2021 15:55:29 +0100 Subject: [PATCH] [tripleo_transfer] Do transfers directly from src to dst When running fetch with become, the slurp module will also be used to fetch the contents of the file for determining the remote checksum. This effectively doubles the transfer size [0] and shows up as a MemoryError when the file size is large enough. In TripleO this is problematic in large & old deployments when transferring the /var/lib/mysql folder. This patch switches to using rsync directly between the src and dst hosts to transfer the data. This is advantageous not only for solving the above-mentioned bug, but is also faster. A simpler implementation using synchronize was attempted [1], but there were issues with the mistral container which prevented that approach from being successful. [0] https://docs.ansible.com/ansible/latest/collections/ansible/builtin/fetch_module.html#notes [1] https://review.opendev.org/c/openstack/tripleo-ansible/+/776565/11 Closes-Bug: #1908425 Closes-Bug: rhbz#1904681 Closes-Bug: rhbz#1916162 Depends-On: https://review.opendev.org/778456 Change-Id: Ifc03f9eb1cb4ca3faec194569f4cb2dace93323f --- .../roles/tripleo_transfer/defaults/main.yml | 9 +- .../molecule/default/converge.yml | 9 +- .../roles/tripleo_transfer/tasks/flag.yml | 7 - .../roles/tripleo_transfer/tasks/main.yml | 158 +++++++++--------- .../{tasks/cleanup.yml => vars/main.yml} | 14 +- 5 files changed, 100 insertions(+), 97 deletions(-) rename tripleo_ansible/roles/tripleo_transfer/{tasks/cleanup.yml => vars/main.yml} (60%) diff --git a/tripleo_ansible/roles/tripleo_transfer/defaults/main.yml b/tripleo_ansible/roles/tripleo_transfer/defaults/main.yml index 48bde8918..04ee047d9 100644 --- a/tripleo_ansible/roles/tripleo_transfer/defaults/main.yml +++ b/tripleo_ansible/roles/tripleo_transfer/defaults/main.yml @@ -24,9 +24,12 @@ # * `tripleo_transfer_dest_host` -- the inventory name of the destination host # * `tripleo_transfer_dest_dir` -- directory on the destination host to transfer to tripleo_transfer_debug: "{{ ((ansible_verbosity | int) >= 2) | bool }}" -tripleo_transfer_storage_root_dir: /var/lib/mistral/tripleo-transfer -tripleo_transfer_storage_root_become: false tripleo_transfer_src_become: true tripleo_transfer_dest_become: true -tripleo_transfer_dest_wipe: true tripleo_transfer_flag_file: ~ +# tripleo_transfer_key_location: location of the private key used to connect +# from src host to dest host. +tripleo_transfer_key_location: "~/transfer_key" +# tripleo_transfer_cleanup_keys: clean up the keypair from the source host +# and remove public key from destination host when true. +tripleo_transfer_cleanup_keys: true diff --git a/tripleo_ansible/roles/tripleo_transfer/molecule/default/converge.yml b/tripleo_ansible/roles/tripleo_transfer/molecule/default/converge.yml index de74d12f8..758e9aff3 100644 --- a/tripleo_ansible/roles/tripleo_transfer/molecule/default/converge.yml +++ b/tripleo_ansible/roles/tripleo_transfer/molecule/default/converge.yml @@ -14,6 +14,14 @@ # License for the specific language governing permissions and limitations # under the License. +- name: Collect facts + hosts: all + gather_facts: false + any_errors_fatal: true + tasks: + - name: Gather a minimal set of facts + setup: + gather_subset: '!all,min' - name: Converge hosts: localhost @@ -25,4 +33,3 @@ tripleo_transfer_src_dir: /etc tripleo_transfer_dest_host: controller1 tripleo_transfer_dest_dir: /opt/etc-target - tripleo_transfer_storage_root_dir: /tmp/transfer-staging diff --git a/tripleo_ansible/roles/tripleo_transfer/tasks/flag.yml b/tripleo_ansible/roles/tripleo_transfer/tasks/flag.yml index c1ce261f3..71dbaf325 100644 --- a/tripleo_ansible/roles/tripleo_transfer/tasks/flag.yml +++ b/tripleo_ansible/roles/tripleo_transfer/tasks/flag.yml @@ -19,8 +19,6 @@ stat: path: "{{ tripleo_transfer_flag_file }}" register: tripleo_transfer_flag_stat - become: "{{ tripleo_transfer_dest_become }}" - delegate_to: "{{ tripleo_transfer_dest_host }}" - name: fail if flag file exists fail: @@ -32,18 +30,13 @@ and re-run the data transfer. when: - tripleo_transfer_flag_stat.stat.exists - delegate_to: "{{ tripleo_transfer_dest_host }}" - name: ensure directory for flag file exists file: path: "{{ tripleo_transfer_flag_file|dirname }}" state: directory - become: "{{ tripleo_transfer_dest_become }}" - delegate_to: "{{ tripleo_transfer_dest_host }}" - name: create the flag file file: path: "{{ tripleo_transfer_flag_file }}" state: touch - become: "{{ tripleo_transfer_dest_become }}" - delegate_to: "{{ tripleo_transfer_dest_host }}" diff --git a/tripleo_ansible/roles/tripleo_transfer/tasks/main.yml b/tripleo_ansible/roles/tripleo_transfer/tasks/main.yml index 7a2248577..0a9da1676 100644 --- a/tripleo_ansible/roles/tripleo_transfer/tasks/main.yml +++ b/tripleo_ansible/roles/tripleo_transfer/tasks/main.yml @@ -15,89 +15,87 @@ # under the License. -- name: make sure we don't have a trailing forward slash in the src - set_fact: - tripleo_transfer_src_dir_safe: "{{ tripleo_transfer_src_dir | regex_replace('\\/$', '') }}" - cacheable: false +# Note: +# This role is used in a playbook that typically targets the undercloud +# and may target other hosts, so to ensure that it executes against the +# right src and dest it uses delegation throughout. -- name: make sure we don't have a trailing forward slash in the dst - set_fact: - tripleo_transfer_dest_dir_safe: "{{ tripleo_transfer_dest_dir | regex_replace('\\/$', '') }}" - cacheable: false +- name: tripleo_transfer tasks + run_once: true + block: + - name: install requirements in src and dest hosts + become: true + package: + name: + - rsync + - openssh-clients + state: present + delegate_to: "{{ item }}" + loop: + - "{{ tripleo_transfer_src_host }}" + - "{{ tripleo_transfer_dest_host }}" -- name: ensure local storage directory exists and has correct permissions - file: - path: "{{ tripleo_transfer_storage_root_dir }}" - # Attempting to set an owner fails with "chown failed: failed to - # look up user" so we at least ensure the permissions. - mode: 0700 - state: directory - delegate_to: localhost - become: "{{ tripleo_transfer_storage_root_become }}" + - name: generate ssh key-pair in source host + openssh_keypair: + path: "{{ tripleo_transfer_key_location }}" + delegate_to: "{{ tripleo_transfer_src_host }}" + become: "{{ tripleo_transfer_src_become }}" + register: keypair_gen -- name: create tempfile for the archive - tempfile: - prefix: ansible.tripleo-transfer. - register: tripleo_transfer_tempfile - become: "{{ tripleo_transfer_src_become }}" - delegate_to: "{{ tripleo_transfer_src_host }}" + - name: set authorized-keys in destination host + authorized_key: + comment: "Added by tripleo-transfer" + user: "{{ ansible_user|default(ansible_ssh_user|default(hostvars[tripleo_transfer_dest_host].ansible_user_id)) }}" + state: present + key: "{{ keypair_gen.public_key }}" + delegate_to: "{{ tripleo_transfer_dest_host }}" + when: keypair_gen is succeeded -# Using the "archive" module lists lists all tarred files in module -# output, if there's too many files, it can crash ansible even with -# "no_log: "{{ not tripleo_transfer_debug | bool }}"". -- name: create the archive - shell: |- - set -euo pipefail - tar --transform "s|^{{ tripleo_transfer_src_dir_safe | basename }}|{{ tripleo_transfer_dest_dir_safe | basename }}|" \ - -czf "{{ tripleo_transfer_tempfile.path }}" \ - -C "{{ tripleo_transfer_src_dir_safe | dirname }}" \ - "{{ tripleo_transfer_src_dir_safe | basename }}" - become: "{{ tripleo_transfer_src_become }}" - delegate_to: "{{ tripleo_transfer_src_host }}" + - import_tasks: flag.yml + when: + - tripleo_transfer_flag_file != None + - tripleo_transfer_flag_file|length > 0 + become: "{{ tripleo_transfer_dest_become }}" + delegate_to: "{{ tripleo_transfer_dest_host }}" -- name: fetch the archive - fetch: - src: "{{ tripleo_transfer_tempfile.path }}" - dest: "{{ tripleo_transfer_storage_root_dir }}/{{ tripleo_transfer_dest_host }}{{ tripleo_transfer_dest_dir_safe }}.tar.gz" - flat: true - become: "{{ tripleo_transfer_src_become }}" - delegate_to: "{{ tripleo_transfer_src_host }}" + - name: synchronize both directories + vars: + hostvars_dest_host_ip: >- + {{ hostvars[tripleo_transfer_dest_host].ansible_host | + default(hostvars[tripleo_transfer_dest_host].inventory_hostname) }} + tripleo_transfer_dest_user: >- + {{ hostvars[tripleo_transfer_dest_host].ansible_user | + default(hostvars[tripleo_transfer_dest_host].ansible_ssh_user | + default(hostvars[tripleo_transfer_dest_host].ansible_user_id)) }} + shell: >- + /usr/bin/rsync + -v + --delay-updates + -F + --compress + --archive + --delete + --rsync-path='sudo rsync' + --rsh='ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i {{ tripleo_transfer_key_location }}' + {{ tripleo_transfer_src_dir_safe }} + {{ tripleo_transfer_dest_user }}@{{ hostvars_dest_host_ip }}:{{ tripleo_transfer_dest_dir_safe }} + become: "{{ tripleo_transfer_src_become }}" + delegate_to: "{{ tripleo_transfer_src_host }}" + always: + - name: clean up keys in source host + file: + path: "{{ item }}" + state: absent + delegate_to: "{{ tripleo_transfer_src_host }}" + become: "{{ tripleo_transfer_src_become }}" + loop: + - "{{ tripleo_transfer_key_location }}" + - "{{ tripleo_transfer_key_location }}.pub" -- name: remove tempfile - file: - name: "{{ tripleo_transfer_tempfile.path }}" - state: absent - become: "{{ tripleo_transfer_src_become }}" - delegate_to: "{{ tripleo_transfer_src_host }}" - -- include_tasks: flag.yml - when: - - tripleo_transfer_flag_file != None - - tripleo_transfer_flag_file|length > 0 - -- name: wipe the destination directory - file: - path: "{{ tripleo_transfer_dest_dir_safe }}" - state: absent - become: "{{ tripleo_transfer_dest_become }}" - delegate_to: "{{ tripleo_transfer_dest_host }}" - when: tripleo_transfer_dest_wipe|bool - -- name: make sure the destination parent directory is present - file: - path: "{{ tripleo_transfer_dest_dir_safe|dirname }}" - state: directory - become: "{{ tripleo_transfer_dest_become }}" - delegate_to: "{{ tripleo_transfer_dest_host }}" - -- name: push and extract the archive - unarchive: - src: "{{ tripleo_transfer_storage_root_dir }}/{{ tripleo_transfer_dest_host }}{{ tripleo_transfer_dest_dir_safe }}.tar.gz" - dest: "{{ tripleo_transfer_dest_dir_safe|dirname }}" - become: "{{ tripleo_transfer_dest_become }}" - delegate_to: "{{ tripleo_transfer_dest_host }}" - -- name: remove the local archive - file: - path: "{{ tripleo_transfer_storage_root_dir }}/{{ tripleo_transfer_dest_host }}{{ tripleo_transfer_dest_dir_safe }}.tar.gz" - state: absent + - name: remove public key from authorized keys in destination host + lineinfile: + path: "~/.ssh/authorized_keys" + state: absent + regexp: '.*Added by tripleo-transfer.*$' + delegate_to: "{{ tripleo_transfer_dest_host }}" + when: tripleo_transfer_cleanup_keys | bool diff --git a/tripleo_ansible/roles/tripleo_transfer/tasks/cleanup.yml b/tripleo_ansible/roles/tripleo_transfer/vars/main.yml similarity index 60% rename from tripleo_ansible/roles/tripleo_transfer/tasks/cleanup.yml rename to tripleo_ansible/roles/tripleo_transfer/vars/main.yml index efc23d3ce..dd498bc6d 100644 --- a/tripleo_ansible/roles/tripleo_transfer/tasks/cleanup.yml +++ b/tripleo_ansible/roles/tripleo_transfer/vars/main.yml @@ -15,9 +15,11 @@ # under the License. -- name: ensure tripleo_transfer storage directory is removed - file: - path: "{{ tripleo_transfer_storage_root_dir }}" - state: absent - delegate_to: localhost - become: "{{ tripleo_transfer_storage_root_become }}" +# All variables intended for internal use should be placed in this file. + + +# make sure we have a trailing forward slash in the src, otherwise rsync creates extra dir +tripleo_transfer_src_dir_safe: "{{ tripleo_transfer_src_dir }}/" + +# make sure we do not have a trailing forward slash in the dest +tripleo_transfer_dest_dir_safe: "{{ tripleo_transfer_dest_dir | regex_replace('\\/$', '') }}"