Handle existing broken config in job updates

If Zuul was unable to freeze the job graph of the previous layout,
then it would throw an exception when calculating whether jobs have
changed.  This is a problem if it does so on a change which fixes
an error in the current tenant config, as it means the fix can not
merge.

To handle this case, we need to catch that exception, and then provide
a default value indicating whether the job config has been updated
or not.  Because returning True could cause every job with a file
matcher to run unecessarily, the safer default is False (so the job
relies on the file matcher alone).

Change-Id: I8a72e57e068b073e37274cdabeacffc9daee2e94

tests/fixtures/config/job-update-broken/git/common-config/playbooks/run.yaml

@@ -0,0 +1 @@
+---

tests/fixtures/config/job-update-broken/git/common-config/zuul.yaml

@@ -0,0 +1,17 @@
+- pipeline:
+    name: check
+    manager: independent
+    trigger:
+      gerrit:
+        - event: patchset-created
+    success:
+      gerrit:
+        Verified: 1
+    failure:
+      gerrit:
+        Verified: -1
+
+- job:
+    name: base
+    parent: null
+    run: playbooks/run.yaml

tests/fixtures/config/job-update-broken/git/org_project/README

@@ -0,0 +1 @@
+test

tests/fixtures/config/job-update-broken/git/org_project/zuul.d/existing.yaml

@@ -0,0 +1,7 @@
+# Every fake change in the unit tests modifies "README"
+
+- job:
+    name: existing-files
+    foo: broken
+    files:
+      - README.txt

tests/fixtures/config/job-update-broken/git/org_project/zuul.d/project.yaml

@@ -0,0 +1,6 @@
+- project:
+    name: org/project
+    templates:
+      - files-template
+    check:
+      jobs: []

tests/fixtures/config/job-update-broken/main.yaml

@@ -0,0 +1,8 @@
+- tenant:
+    name: tenant-one
+    source:
+      gerrit:
+        config-projects:
+          - common-config
+        untrusted-projects:
+          - org/project

tests/unit/test_scheduler.py

@@ -5712,6 +5712,68 @@ For CI problems and help debugging, contact ci@example.org"""
         ], ordered=False)
 
 
+class TestJobUpdateBrokenConfig(ZuulTestCase):
+    tenant_config_file = 'config/job-update-broken/main.yaml'
+
+    def test_fix_check_without_running(self):
+        "Test that we can fix a broken check pipeline (don't run the job)"
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: existing-files
+                files:
+                  - README.txt
+
+            - project-template:
+                name: files-template
+                check:
+                  jobs:
+                    - existing-files
+                    - noop
+            """)
+
+        # When the config is broken, we don't override any files
+        # matchers since we don't have a valid basis.  Since this
+        # doesn't update README.txt, nothing should run.
+        file_dict = {'zuul.d/existing.yaml': in_repo_conf}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertHistory([])
+        self.assertEqual(A.reported, 1)
+
+    def test_fix_check_with_running(self):
+        "Test that we can fix a broken check pipeline (do run the job)"
+        in_repo_conf = textwrap.dedent(
+            """
+            - job:
+                name: existing-files
+                files:
+                  - README.txt
+
+            - project-template:
+                name: files-template
+                check:
+                  jobs:
+                    - existing-files
+            """)
+
+        # When the config is broken, we don't override any files
+        # matchers since we don't have a valid basis.  Since this
+        # does update README.txt, the job should run.
+        file_dict = {'zuul.d/template.yaml': in_repo_conf,
+                     'README.txt': ''}
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+        self.assertHistory([
+            dict(name='existing-files', result='SUCCESS', changes='1,1'),
+        ])
+        self.assertEqual(A.reported, 1)
+
+
 class TestJobUpdateFileMatcher(ZuulTestCase):
     tenant_config_file = 'config/job-update/main.yaml'
 

zuul/model.py

@@ -2878,12 +2878,21 @@ class QueueItem(object):
             # This change updates the layout.  Calculate the job as it
             # would be if the layout had not changed.
             if self._old_job_graph is None:
-                ppc = layout_ahead.getProjectPipelineConfig(self)
-                log.debug("Creating job graph for config change detection")
-                self._old_job_graph = layout_ahead.createJobGraph(
-                    self, ppc, skip_file_matcher=True)
-                log.debug("Done creating job graph for "
-                          "config change detection")
+                try:
+                    ppc = layout_ahead.getProjectPipelineConfig(self)
+                    log.debug("Creating job graph for config change detection")
+                    self._old_job_graph = layout_ahead.createJobGraph(
+                        self, ppc, skip_file_matcher=True)
+                    log.debug("Done creating job graph for "
+                              "config change detection")
+                except Exception:
+                    self.log.debug(
+                        "Error freezing job graph in job update check:",
+                        exc_info=True)
+                    # The config was broken before, we have no idea
+                    # which jobs have changed, so rather than run them
+                    # all, just rely on the file matchers as-is.
+                    return False
             old_job = self._old_job_graph.jobs.get(job.name)
             if old_job is None:
                 log.debug("Found a newly created job")