From 6e424878ae0669adb2b2950c4e0e06c9c46daaec Mon Sep 17 00:00:00 2001
From: Simon Westphahl <simon.westphahl@bmw.de>
Date: Wed, 6 Feb 2019 08:57:42 +0100
Subject: [PATCH] Fix error reporting for special task failures

For some tasks the Ansible log will not contain enough information to
debug failures (e.g. missing role with include_role).

Ansible treats those issues not like an error (exit code 1) but like a
failed task, leading to an exit code of 2.

Change-Id: Iea754814e3d55be6be1c2de7f2d45ceda757f480
---
 .../job-output-missing-role-include.yaml      |  4 +++
 .../job-output/git/common-config/zuul.yaml    |  5 ++++
 tests/unit/test_v3.py                         | 10 +++++--
 zuul/executor/server.py                       | 29 ++++++++++++++-----
 4 files changed, 37 insertions(+), 11 deletions(-)
 create mode 100644 tests/fixtures/config/job-output/git/common-config/playbooks/job-output-missing-role-include.yaml

diff --git a/tests/fixtures/config/job-output/git/common-config/playbooks/job-output-missing-role-include.yaml b/tests/fixtures/config/job-output/git/common-config/playbooks/job-output-missing-role-include.yaml
new file mode 100644
index 0000000000..e8e7f2860f
--- /dev/null
+++ b/tests/fixtures/config/job-output/git/common-config/playbooks/job-output-missing-role-include.yaml
@@ -0,0 +1,4 @@
+- hosts: all
+  tasks:
+    - include_role:
+        name: not_existing
diff --git a/tests/fixtures/config/job-output/git/common-config/zuul.yaml b/tests/fixtures/config/job-output/git/common-config/zuul.yaml
index 5b44347b2e..a0122a657a 100644
--- a/tests/fixtures/config/job-output/git/common-config/zuul.yaml
+++ b/tests/fixtures/config/job-output/git/common-config/zuul.yaml
@@ -30,6 +30,10 @@
     name: job-output-missing-role
     run: playbooks/job-output-missing-role.yaml
 
+- job:
+    name: job-output-missing-role-include
+    run: playbooks/job-output-missing-role-include.yaml
+
 - project:
     name: org/project
     check:
@@ -47,3 +51,4 @@
     check:
       jobs:
         - job-output-missing-role
+        - job-output-missing-role-include
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index b9955d2c2b..00875eb005 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -4360,11 +4360,15 @@ class TestJobOutput(AnsibleZuulTestCase):
         self.assertHistory([
             dict(name='job-output-missing-role', result='FAILURE',
                  changes='1,1'),
+            dict(name='job-output-missing-role-include', result='FAILURE',
+                 changes='1,1'),
         ], ordered=False)
 
-        job_output = self._get_file(self.history[0],
-                                    'work/logs/job-output.txt')
-        self.assertIn('the role \'not_existing\' was not found', job_output)
+        for history in self.history:
+            job_output = self._get_file(history,
+                                        'work/logs/job-output.txt')
+            self.assertIn('the role \'not_existing\' was not found',
+                          job_output)
 
     def test_job_output_failure_log(self):
         logger = logging.getLogger('zuul.AnsibleJob')
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index 05c8729b4a..678919c945 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -1948,14 +1948,27 @@ class AnsibleJob(object):
                         now=datetime.datetime.now(),
                         line=line.decode('utf-8').rstrip()))
         elif ret == 2:
-            # This is a workaround to detect winrm connection failures that are
-            # not detected by ansible. These can be detected if the string
-            # 'FATAL ERROR DURING FILE TRANSFER' is in the ansible output.
-            # In this case we should treat the host as unreachable and retry
-            # the job.
-            for line in syntax_buffer:
-                if b'FATAL ERROR DURING FILE TRANSFER' in line:
-                    return self.RESULT_UNREACHABLE, None
+            with open(self.jobdir.job_output_file, 'a') as job_output:
+                found_marker = False
+                for line in syntax_buffer:
+                    # This is a workaround to detect winrm connection failures
+                    # that are not detected by ansible. These can be detected
+                    # if the string 'FATAL ERROR DURING FILE TRANSFER' is in
+                    # the ansible output. In this case we should treat the
+                    # host as unreachable and retry the job.
+                    if b'FATAL ERROR DURING FILE TRANSFER' in line:
+                        return self.RESULT_UNREACHABLE, None
+
+                    # Extract errors for special cases that are treated like
+                    # task errors by Ansible (e.g. missing role when using
+                    # 'include_role').
+                    if line.startswith(b'ERROR!'):
+                        found_marker = True
+                    if not found_marker:
+                        continue
+                    job_output.write("{now} | {line}\n".format(
+                        now=datetime.datetime.now(),
+                        line=line.decode('utf-8').rstrip()))
 
         return (self.RESULT_NORMAL, ret)