From 1338f813ec8e0c0bd0c5167e2ba209ca44986a7e Mon Sep 17 00:00:00 2001
From: "James E. Blair" <jim@acmegating.com>
Date: Mon, 7 Jun 2021 15:53:40 -0700
Subject: [PATCH] Fix reporting merge failure behind change adding ppc

Change I7a9a2005eb57c4e2fd398e616c31ba5035e8d965 corrected an edge case
where a reconfiguration would clear a cached layout, and, on already
enqueued changes where a project was being added to a pipeline, the
absence of that layout would cause Zuul not to report since it could
not determine whether the project was part of the pipeline.

The fix for that suggested we should examine the case where a change
with a merge conflict is behind a successful change which adds a project
to a pipeline.  In that case, we should fall back to the layout of the
item ahead, but the item ahead is no longer in the queue.  We should,
however, have the item ahead's layout in the cache and referenced by
UUID, but we need to set our failing item's layout uuid in that case.

To correct this, we call getLayout for the side effect of saving the
layout uuid on the item in both of the cases where we otherwise
short-circuit layout generation on merge or config errors.

This case doesn't need a reconfiguration to trigger it -- an item
with no jobs behind an item with a dynamic layout will cause the item
ahead to regenate its layout, so it's not subject to the bug fixed in
I7a9a200.  Instead, this case requires the item ahead to succeed so
that it's not present in the queue when the item behind reports.

Change-Id: I7b4d1596262b8b1b5c300f44401aafe923e718b3
---
 tests/unit/test_v3.py    | 56 ++++++++++++++++++++++++++++++++++++++++
 zuul/manager/__init__.py | 10 ++++---
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index d6834c8c53..3e275f7a25 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -3057,6 +3057,62 @@ class TestInRepoJoin(ZuulTestCase):
             dict(name='project-test1', result='FAILURE', changes='1,1'),
         ], ordered=False)
 
+    def test_dynamic_dependent_pipeline_merge_failure(self):
+        # Test that a merge failure behind a change adding a project
+        # to a dependent pipeline is correctly reported.
+        self.executor_server.hold_jobs_in_build = True
+
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: project-test1
+                run: playbooks/project-test1.yaml
+
+            - project:
+                name: org/project
+                gate:
+                  jobs:
+                    - project-test1
+            """)
+
+        file_dict = {'.zuul.yaml': in_repo_conf}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=file_dict)
+        A.addApproval('Code-Review', 2)
+        self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+        self.waitUntilSettled()
+
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: project-test2
+                run: playbooks/project-test1.yaml
+
+            - project:
+                name: org/project
+                gate:
+                  jobs:
+                    - project-test2
+            """)
+
+        file_dict = {'.zuul.yaml': in_repo_conf}
+        B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B',
+                                           files=file_dict)
+        B.addApproval('Code-Review', 2)
+        self.fake_gerrit.addEvent(B.addApproval('Approved', 1))
+        self.waitUntilSettled()
+
+        self.orderedRelease()
+        self.waitUntilSettled()
+        self.assertEqual(A.reported, 2,
+                         "A should report start and success")
+        self.assertEqual(A.data['status'], 'MERGED')
+        self.assertEqual(B.reported, 1,
+                         "B should report merge failure")
+        self.assertHistory([
+            dict(name='project-test1', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
+
     def test_dynamic_dependent_pipeline_absent(self):
         # Test that a series of dependent changes don't report merge
         # failures to a pipeline they aren't in.
diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py
index 3eddcaae9e..8c6492fddf 100644
--- a/zuul/manager/__init__.py
+++ b/zuul/manager/__init__.py
@@ -1038,6 +1038,9 @@ class PipelineManager(metaclass=ABCMeta):
             msg = "This change depends on a change "\
                   "with an invalid configuration.\n"
             item.setConfigError(msg)
+            # Find our layout since the reporter will need it to
+            # determine if the project is in the pipeline.
+            self.getLayout(item)
             return False
 
         # The next section starts between 0 and 2 remote merger
@@ -1076,9 +1079,10 @@ class PipelineManager(metaclass=ABCMeta):
 
         # If the change can not be merged or has config errors, don't
         # run jobs.
-        if build_set.unable_to_merge:
-            return False
-        if build_set.config_errors:
+        if build_set.unable_to_merge or build_set.config_errors:
+            # Find our layout since the reporter will need it to
+            # determine if the project is in the pipeline.
+            self.getLayout(item)
             return False
 
         # With the merges done, we have the info needed to get a