From f946763d0a233506ab19a5ab91aabbd661533237 Mon Sep 17 00:00:00 2001 From: Ian Wienand <iwienand@redhat.com> Date: Fri, 5 Jun 2020 13:33:57 +1000 Subject: [PATCH] Revert "download-artifact : support recursive download" This reverts commit 7101fe7d1c13250415f5c6f6392c2a22720bbe43. This unfortunately has a number of problems. Firstly, the "Fail if no wget" fails when download_artifact_recurse isn't set, because we didn't check for wget. Also, the download doesn't work with some providers. wget asks for gzip downloads with it's accept headers (which can't be turned off) but the recursive download doesn't understand the gzipped index.html file and thus doesn't find anything to walk. The "--compression=auto" flag is available to overcome this, but is not widely supported (and not supported on the executor). https://review.opendev.org/733728 attempted to work-around this but the problems with this approach seem too much for now. Change-Id: I9bc55d771ec1828d374684d0ffe5ec1d1494773e --- roles/download-artifact/README.rst | 13 -- roles/download-artifact/defaults/main.yaml | 1 - roles/download-artifact/tasks/main.yaml | 36 ----- roles/download-artifact/tasks/recursive.yaml | 27 ---- test-playbooks/download-artifact.yaml | 35 ---- zuul-tests.d/artifact-jobs.yaml | 159 ------------------- 6 files changed, 271 deletions(-) delete mode 100644 roles/download-artifact/tasks/recursive.yaml delete mode 100644 test-playbooks/download-artifact.yaml delete mode 100644 zuul-tests.d/artifact-jobs.yaml diff --git a/roles/download-artifact/README.rst b/roles/download-artifact/README.rst index efdb6ad18..21b19cf8a 100644 --- a/roles/download-artifact/README.rst +++ b/roles/download-artifact/README.rst @@ -32,16 +32,3 @@ many artifacts as match the selection criteria. :default: {{ zuul.executor.work_root }} The directory in which to place the downloaded artifacts. - -.. zuul:rolevar:: download_artifact_recurse - :default: False - - If the ``type`` describes a directory, this can be used to download - it recursively. Note this requires ``wget`` on the host. The - *last* component of the artifact URL should be a directory, and - that will be the directory saved to - ``download_artifact_directory``. - - For example, the contents of - ``https://foo.com/abc/123/test/the_artifact/`` will be saved to - ``{{ download_artifact_directory }}/the_artifact/``. diff --git a/roles/download-artifact/defaults/main.yaml b/roles/download-artifact/defaults/main.yaml index 72d03e104..56abfcf00 100644 --- a/roles/download-artifact/defaults/main.yaml +++ b/roles/download-artifact/defaults/main.yaml @@ -1,4 +1,3 @@ --- download_artifact_query: "change={{ zuul.change }}&patchset={{ zuul.patchset }}&pipeline={{ download_artifact_pipeline }}&job_name={{ download_artifact_job }}" download_artifact_directory: "{{ zuul.executor.work_root }}" -download_artifact_recurse: False diff --git a/roles/download-artifact/tasks/main.yaml b/roles/download-artifact/tasks/main.yaml index ef4b46f2d..f2414887d 100644 --- a/roles/download-artifact/tasks/main.yaml +++ b/roles/download-artifact/tasks/main.yaml @@ -1,24 +1,3 @@ -- name: Look for wget - shell: | - command -v wget || exit 1 - args: - executable: /bin/bash - register: _wget_installed - failed_when: false - when: download_artifact_recurse - -- name: Fail if no wget - fail: - msg: 'wget not found, can not recurse' - when: - - _wget_installed.rc != 0 - - download_artifact_recurse - -- name: Ensure the download directory - file: - name: '{{ download_artifact_directory }}' - state: directory - - name: Query Zuul API for artifact information uri: url: "{{ download_artifact_api }}/builds?{{ download_artifact_query }}" @@ -26,21 +5,6 @@ - name: Parse build response set_fact: build: "{{ build.json[0] }}" - -- name: Recursive downloads - loop: "{{ build.artifacts }}" - include_tasks: recursive.yaml - loop_control: - loop_var: zj_artifact - when: - - "'metadata' in zj_artifact" - - "'type' in zj_artifact.metadata" - - download_artifact_recurse - - > - zj_artifact.metadata.type == download_artifact_type or - ((download_artifact_type | type_debug) == 'list' - and zj_artifact.metadata.type in download_artifact_type) - - name: Download archive by type uri: url: "{{ zj_artifact.url }}" diff --git a/roles/download-artifact/tasks/recursive.yaml b/roles/download-artifact/tasks/recursive.yaml deleted file mode 100644 index 0152fc8c1..000000000 --- a/roles/download-artifact/tasks/recursive.yaml +++ /dev/null @@ -1,27 +0,0 @@ -- name: Find directories to strip - set_fact: - # - take the url, strip of any trailing "/" - # - find everything on the // side - # - split it at /'s, then disregrad the host (ignored by parent - # flag) and the final directory (the one we want) i.e. -2 - cut_dirs: '{{ zj_artifact.url.rpartition("//")[-1].rstrip("/").split("/") | length - 2 }}' - -- name: Recursive download - # NOTE(ianw): something weird about wget recursive, you want to make - # sure the directory to download has a trailing "/" or you can get - # directory errors; something like - # http://savannah.gnu.org/bugs/?29647 - # Otherwise this - # - drops the host (-nH) - # - doesn't walk upwards (--no-parent) - # - doesn't save index.html files (maybe this should be optional for docs?) - # - cuts out the all the parent directories but the last one - # - # so "https://foo.com/logs/123/abc/dir/the_artifact" gets saved to - # {{ download_artifact_directory }}/the_artifact - command: | - wget -nH --no-parent --reject="index.html*" --cut-dirs={{ cut_dirs }} -P {{ download_artifact_directory }} -m {{ zj_artifact.url.rstrip("/") + "/" }} - args: - # We really want to use wget here, because the get_url type things - # don't do recursive. - warn: false diff --git a/test-playbooks/download-artifact.yaml b/test-playbooks/download-artifact.yaml deleted file mode 100644 index 2fd8e93f6..000000000 --- a/test-playbooks/download-artifact.yaml +++ /dev/null @@ -1,35 +0,0 @@ -- name: Run the download-artifacts role - hosts: all - roles: - - ensure-output-dirs - - tasks: - # NOTE(ianw) : This has probably expired and thus broken this - # test. You need to choose a new artifact to download to test - # this as part of updating the download-artifact role - - - name: Download centos 7 x86 artifact on host - include_role: - name: download-artifact - vars: - download_artifact_query: "change=733420&patchset=1&pipeline=gate&job_name=openafs-rpm-package-build-centos-7-x86" - download_artifact_api: "https://zuul.opendev.org/api/tenant/openstack" - download_artifact_type: rpm - download_artifact_pipeline: gate - download_artifact_job: openafs-rpm-package-build-centos-7-x86 - download_artifact_directory: '/tmp/' - download_artifact_recurse: yes - - - name: Download centos 7 x86 artifact on executor - include_role: - name: download-artifact - apply: - delegate_to: localhost - vars: - download_artifact_query: "change=733420&patchset=1&pipeline=gate&job_name=openafs-rpm-package-build-centos-7-x86" - download_artifact_api: "https://zuul.opendev.org/api/tenant/openstack" - download_artifact_type: rpm - download_artifact_pipeline: gate - download_artifact_job: openafs-rpm-package-build-centos-7-x86 - download_artifact_directory: '{{ zuul.executor.work_root }}/test-artifacts/' - download_artifact_recurse: yes diff --git a/zuul-tests.d/artifact-jobs.yaml b/zuul-tests.d/artifact-jobs.yaml deleted file mode 100644 index 324c67c16..000000000 --- a/zuul-tests.d/artifact-jobs.yaml +++ /dev/null @@ -1,159 +0,0 @@ -- job: - name: zuul-jobs-test-download-artifact - tags: all-platforms - description: | - Test artifact roles - parent: base-minimal - run: test-playbooks/download-artifact.yaml - files: - - ^roles/download-artifact/.* - -- job: - name: zuul-jobs-test-download-artifact-centos-7 - description: Test artifact roles on centos-7 - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: centos-7 - label: centos-7 - -- job: - name: zuul-jobs-test-download-artifact-centos-8 - description: Test artifact roles on centos-8 - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: centos-8 - label: centos-8 - -- job: - name: zuul-jobs-test-download-artifact-debian-stretch - description: Test artifact roles on debian-stretch - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: debian-stretch - label: debian-stretch - -- job: - name: zuul-jobs-test-download-artifact-fedora-31 - description: Test artifact roles on fedora-31 - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: fedora-31 - label: fedora-31 - -- job: - name: zuul-jobs-test-download-artifact-gentoo-17-0-systemd - description: Test artifact roles on gentoo-17-0-systemd - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: gentoo-17-0-systemd - label: gentoo-17-0-systemd - -- job: - name: zuul-jobs-test-download-artifact-opensuse-15 - description: Test artifact roles on opensuse-15 - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: opensuse-15 - label: opensuse-15 - -- job: - name: zuul-jobs-test-download-artifact-opensuse-tumbleweed-nv - voting: false - description: Test artifact roles on opensuse-tumbleweed - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: opensuse-tumbleweed - label: opensuse-tumbleweed - -- job: - name: zuul-jobs-test-download-artifact-ubuntu-bionic - description: Test artifact roles on ubuntu-bionic - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: ubuntu-bionic - label: ubuntu-bionic - -- job: - name: zuul-jobs-test-download-artifact-ubuntu-xenial - description: Test artifact roles on ubuntu-xenial - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: ubuntu-xenial - label: ubuntu-xenial - -- job: - name: zuul-jobs-test-download-artifact-ubuntu-bionic-plain - description: Test artifact roles on ubuntu-bionic-plain - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: ubuntu-bionic-plain - label: ubuntu-bionic-plain - -- job: - name: zuul-jobs-test-download-artifact-ubuntu-xenial-plain - description: Test artifact roles on ubuntu-xenial-plain - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: ubuntu-xenial-plain - label: ubuntu-xenial-plain - -- job: - name: zuul-jobs-test-download-artifact-centos-8-plain - description: Test artifact roles on centos-8-plain - parent: zuul-jobs-test-download-artifact - tags: auto-generated - nodeset: - nodes: - - name: centos-8-plain - label: centos-8-plain - -- project: - check: - jobs: - - zuul-jobs-test-download-artifact-centos-7 - - zuul-jobs-test-download-artifact-centos-8 - - zuul-jobs-test-download-artifact-debian-stretch - - zuul-jobs-test-download-artifact-fedora-31 - - zuul-jobs-test-download-artifact-gentoo-17-0-systemd - - zuul-jobs-test-download-artifact-opensuse-15 - - zuul-jobs-test-download-artifact-opensuse-tumbleweed-nv - - zuul-jobs-test-download-artifact-ubuntu-bionic - - zuul-jobs-test-download-artifact-ubuntu-xenial - - zuul-jobs-test-download-artifact-ubuntu-bionic-plain - - zuul-jobs-test-download-artifact-ubuntu-xenial-plain - - zuul-jobs-test-download-artifact-centos-8-plain - gate: - jobs: - - zuul-jobs-test-download-artifact-centos-7 - - zuul-jobs-test-download-artifact-centos-8 - - zuul-jobs-test-download-artifact-debian-stretch - - zuul-jobs-test-download-artifact-fedora-31 - - zuul-jobs-test-download-artifact-gentoo-17-0-systemd - - zuul-jobs-test-download-artifact-opensuse-15 - - zuul-jobs-test-download-artifact-ubuntu-bionic - - zuul-jobs-test-download-artifact-ubuntu-xenial - - zuul-jobs-test-download-artifact-ubuntu-bionic-plain - - zuul-jobs-test-download-artifact-ubuntu-xenial-plain - - zuul-jobs-test-download-artifact-centos-8-plain