From f6a92ecb6cfd232d088421b58f0b8d302b7923f9 Mon Sep 17 00:00:00 2001
From: Monty Taylor <mordred@inaugust.com>
Date: Tue, 16 Jan 2018 12:39:09 -0600
Subject: [PATCH] Make subunit processing more resilient

Use subunit2html from tox envs instead of os-testr-env

We have an openstack-infra hard-coded location in this role. Rework it
to try looking in the tox env if one exists or to look on the system.

Leave /usr/os-testr-env in as a fallback but with a debug statement so
that we can track instances of the fallback being used.

tox_envlist can be a comma separated list. If it is, the "find
subunit2html in a tox env" logic isn't going to work.

Use a shell script so that we can do a first-found logic similar to what
we use in ensure-sphinx for finding requirements files.

It's a big bash blob. Put it in a file so it syntax highlights correctly
and execute it with the script module.

Change-Id: I0b468840fd04a379007fe5aca07feb2eaf7081ac
---
 roles/fetch-subunit-output/defaults/main.yaml |  1 +
 .../files/find-subunit2html.sh                | 45 ++++++++++++++
 .../fetch-subunit-output/files/find-testr.sh  | 59 +++++++++++++++++++
 .../tasks/find-subunit-html.yaml              | 16 +++++
 roles/fetch-subunit-output/tasks/main.yaml    | 59 +++++--------------
 roles/fetch-subunit-output/tasks/process.yaml | 30 ++++++----
 6 files changed, 156 insertions(+), 54 deletions(-)
 create mode 100644 roles/fetch-subunit-output/files/find-subunit2html.sh
 create mode 100644 roles/fetch-subunit-output/files/find-testr.sh
 create mode 100644 roles/fetch-subunit-output/tasks/find-subunit-html.yaml

diff --git a/roles/fetch-subunit-output/defaults/main.yaml b/roles/fetch-subunit-output/defaults/main.yaml
index 2cdab6ceb..95a9e3b77 100644
--- a/roles/fetch-subunit-output/defaults/main.yaml
+++ b/roles/fetch-subunit-output/defaults/main.yaml
@@ -1,2 +1,3 @@
 ---
+tox_envlist: ""
 zuul_work_dir: "{{ zuul.project.src_dir }}"
diff --git a/roles/fetch-subunit-output/files/find-subunit2html.sh b/roles/fetch-subunit-output/files/find-subunit2html.sh
new file mode 100644
index 000000000..98a1d23d1
--- /dev/null
+++ b/roles/fetch-subunit-output/files/find-subunit2html.sh
@@ -0,0 +1,45 @@
+#!/bin/bash
+
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -eu
+
+zuul_work_dir=$1
+# Use cd in the script rather than chdir from the script module. The chdir
+# has some source-code comments that indicate for script that it needs to be
+# an absolute path. Since our zuul_work_dir defaults to
+# "src/{{ zuul.project.src_dir }}" in many places, that could be rather bad.
+# The script can ensure it starts from the home dir and then change into the
+# directory, which should work for both relative and absolute paths.
+cd $HOME
+cd $zuul_work_dir
+
+# Add all the tox envs to the path so that type will work. Prefer tox
+# envs to system path. If there is more than one tox env, it doesn't
+# matter which one we use, PATH will find the first command.
+if [[ -d .tox ]] ; then
+    for tox_bindir in $(find .tox -mindepth 2 -maxdepth 2 -name 'bin') ; do
+        PATH=$(pwd)/$tox_bindir:$PATH
+    done
+fi
+
+# NOTE(mordred): Fallback default to /usr/os-testr-env/bin/subunit2html
+# to provide a fallback case for OpenStack while we make sure this
+# logic works. Once we're satisfied that nobody is using
+# /usr/os-testr-env/bin/subunit2html, we can remove this.
+PATH="$PATH:/usr/os-testr-env/bin"
+
+type -p $command
diff --git a/roles/fetch-subunit-output/files/find-testr.sh b/roles/fetch-subunit-output/files/find-testr.sh
new file mode 100644
index 000000000..9ba14fb81
--- /dev/null
+++ b/roles/fetch-subunit-output/files/find-testr.sh
@@ -0,0 +1,59 @@
+#!/bin/bash
+
+# Copyright (c) 2018 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -eu
+
+zuul_work_dir=$1
+
+# Use cd in the script rather than chdir from the script module. The chdir
+# has some source-code comments that indicate for script that it needs to be
+# an absolute path. Since our zuul_work_dir defaults to
+# "{{ zuul.project.src_dir }}" in many places, that could be rather bad.
+# The script can ensure it starts from the home dir and then change into the
+# directory, which should work for both relative and absolute paths.
+cd $HOME
+cd $zuul_work_dir
+
+commands=""
+if [[ -d .testrepository ]] ; then
+    commands="testr ${commands}"
+fi
+
+# NOTE(mordred) Check for the failing file in the .stestr directory
+# nstead of just the directory. A stestr run that fails due to python
+# parsing errors will leave a directory but with no test results, which
+# will result in an error in the subunit generation phase.
+if [[ -f .stestr/failing ]] ; then
+    commands="stestr ${commands}"
+fi
+
+# Add all the tox envs to the path so that type will work. Prefer tox
+# envs to system path. If there is more than one tox env, it doesn't
+# matter which one we use, PATH will find the first command.
+if [[ -d .tox ]] ; then
+    for tox_bindir in $(find .tox -mindepth 2 -maxdepth 2 -name 'bin') ; do
+        PATH=$(pwd)/$tox_bindir:$PATH
+    done
+fi
+
+for command in $commands; do
+    found=$(type -p $command)
+    if [[ -n $found ]] ; then
+        echo $found
+        break
+    fi
+done
diff --git a/roles/fetch-subunit-output/tasks/find-subunit-html.yaml b/roles/fetch-subunit-output/tasks/find-subunit-html.yaml
new file mode 100644
index 000000000..8fd36df2c
--- /dev/null
+++ b/roles/fetch-subunit-output/tasks/find-subunit-html.yaml
@@ -0,0 +1,16 @@
+- name: Look for subunit2html command
+  script: "find-subunit2html.sh {{ zuul_work_dir }}"
+  failed_when: false
+  register: find_subunit2html_output
+
+- name: Define subunit_html_command fact
+  when:
+    - find_subunit2html_output.rc == 0
+    - find_subunit2html_output.stdout_lines
+  set_fact:
+   subunit_html_command: "{{ find_subunit2html_output.stdout_lines[0] }}"
+
+- name: Emit a debug line so we can search logstash for fallback hits
+  debug:
+    msg: "subunit2html found in: {{ subunit_html_command }}"
+  when: subunit_html_command is defined
diff --git a/roles/fetch-subunit-output/tasks/main.yaml b/roles/fetch-subunit-output/tasks/main.yaml
index 4d0e76e2e..496ed7d43 100644
--- a/roles/fetch-subunit-output/tasks/main.yaml
+++ b/roles/fetch-subunit-output/tasks/main.yaml
@@ -1,46 +1,19 @@
-# TODO(mordred) tox_envlist can be a list. If it is, we should look for each
-# command in each envlist dir until we find one that has it.
-- name: Set paths to testr and stestr commands from tox
-  set_fact:
-    testr_command: .tox/{{ tox_envlist }}/bin/testr
-    stestr_command: .tox/{{ tox_envlist }}/bin/stestr
-  when: tox_envlist is defined
+# We're not using with_first_found because the files are remote, not local.
+# We want to use stestr if it exists or fallback to testr - and we want to
+# prefer files found in tox envs.
+- name: Find stestr or testr executable
+  script: "find-testr.sh {{ zuul_work_dir }}"
+  register: testr_command
 
-- name: Set paths to testr and stestr commands from system
-  set_fact:
-    testr_command: testr
-    stestr_command: stestr
-  when: tox_envlist is not defined
+- when:
+    - testr_command.rc == 0
+    - testr_command.stdout_lines
+  block:
 
-# NOTE(mordred) Check for the failing file in the .stestr directory instead of
-# just the directory. An stestr run that fails due to python parsing errors
-# will leave a directory but with no test results, which will result in an
-# error in the subunit generation phase.
-- name: Check for stestr directory
-  stat:
-    path: "{{ zuul_work_dir }}/.stestr/failing"
-  register: stestr_stat
+    - name: Generate subunit file
+      shell:
+        cmd: "{{ testr_command.stdout_lines[0] }} last --subunit > ./testrepository.subunit"
+        chdir: "{{ zuul_work_dir }}"
 
-- name: Generate stestr subunit file
-  shell:
-    cmd: "{{ stestr_command }} last --subunit > ./testrepository.subunit"
-    chdir: "{{ zuul_work_dir }}"
-  when: stestr_stat.stat.exists
-
-- name: Check for testr directory
-  stat:
-    path: "{{ zuul_work_dir }}/.testrepository"
-  register: testr_stat
-  when: not stestr_stat.stat.exists
-
-- name: Generate testrepository.subunit file
-  shell:
-    cmd: "{{ testr_command }} last --subunit > ./testrepository.subunit"
-    chdir: "{{ zuul_work_dir }}"
-  when:
-    - not stestr_stat.stat.exists
-    - testr_stat.stat.exists
-
-- name: Process and fetch subunit results
-  include: process.yaml
-  when: stestr_stat.stat.exists or testr_stat.stat.exists
+    - name: Process and fetch subunit results
+      include: process.yaml
diff --git a/roles/fetch-subunit-output/tasks/process.yaml b/roles/fetch-subunit-output/tasks/process.yaml
index b2b6aa013..e8eda69cc 100644
--- a/roles/fetch-subunit-output/tasks/process.yaml
+++ b/roles/fetch-subunit-output/tasks/process.yaml
@@ -1,22 +1,30 @@
+- name: Find subunit2html
+  include: find-subunit-html.yaml
+
 - name: Generate testr_results.html file
-  # TODO(pabelanger): We cannot depend on /usr/os-testr-env here!!!
-  command: "/usr/os-testr-env/bin/subunit2html ./testrepository.subunit testr_results.html"
+  command: "{{ subunit_html_command }} ./testrepository.subunit testr_results.html"
   args:
     chdir: "{{ zuul_work_dir }}"
+  when: subunit_html_command is defined
 
-- name: Compress testrepository files
+- name: Find subunit files
+  find:
+    paths:
+      - "{{ ansible_user_dir }}/{{ zuul_work_dir }}"
+    patterns:
+      - testr_results.html
+      - testrepository.subunit
+  register: subunit_files
+
+- name: Compress subunit files
   archive:
-    path: "{{ zuul_work_dir }}/{{ item }}"
-  with_items:
-    - testrepository.subunit
-    - testr_results.html
+    path: "{{ item.path }}"
+  with_items: "{{ subunit_files.files }}"
 
 - name: Collect test-results
   synchronize:
     dest: "{{ zuul.executor.log_root }}"
     mode: pull
-    src: "{{ zuul_work_dir }}/{{ item }}"
+    src: "{{ item.path }}.gz"
     verify_host: true
-  with_items:
-    - "*testr_results.html.gz"
-    - "*testrepository.subunit.gz"
+  with_items: "{{ subunit_files.files }}"