Merge "Document executor/merger stats" into feature/zuulv3
diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py
index 8023ef6..a97e47e 100755
--- a/tests/unit/test_scheduler.py
+++ b/tests/unit/test_scheduler.py
@@ -1465,8 +1465,6 @@
 
     @simple_layout('layouts/autohold.yaml')
     def test_autohold(self):
-        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
-
         client = zuul.rpcclient.RPCClient('127.0.0.1',
                                           self.gearman_server.port)
         self.addCleanup(client.shutdown)
@@ -1474,15 +1472,36 @@
                             "reason text", 1)
         self.assertTrue(r)
 
-        self.executor_server.failJob('project-test2', A)
+        # First check that successful jobs do not autohold
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
         self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
 
         self.waitUntilSettled()
 
         self.assertEqual(A.data['status'], 'NEW')
         self.assertEqual(A.reported, 1)
-        self.assertEqual(self.getJobFromHistory('project-test2').result,
-                         'FAILURE')
+        # project-test2
+        self.assertEqual(self.history[0].result, 'SUCCESS')
+
+        # Check nodepool for a held node
+        held_node = None
+        for node in self.fake_nodepool.getNodes():
+            if node['state'] == zuul.model.STATE_HOLD:
+                held_node = node
+                break
+        self.assertIsNone(held_node)
+
+        # Now test that failed jobs are autoheld
+        B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
+        self.executor_server.failJob('project-test2', B)
+        self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
+
+        self.waitUntilSettled()
+
+        self.assertEqual(B.data['status'], 'NEW')
+        self.assertEqual(B.reported, 1)
+        # project-test2
+        self.assertEqual(self.history[1].result, 'FAILURE')
 
         # Check nodepool for a held node
         held_node = None
@@ -1502,14 +1521,14 @@
         self.assertEqual(held_node['comment'], "reason text")
 
         # Another failed change should not hold any more nodes
-        B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B')
-        self.executor_server.failJob('project-test2', B)
-        self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
+        C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C')
+        self.executor_server.failJob('project-test2', C)
+        self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
         self.waitUntilSettled()
-        self.assertEqual(B.data['status'], 'NEW')
-        self.assertEqual(B.reported, 1)
-        self.assertEqual(self.getJobFromHistory('project-test2').result,
-                         'FAILURE')
+        self.assertEqual(C.data['status'], 'NEW')
+        self.assertEqual(C.reported, 1)
+        # project-test2
+        self.assertEqual(self.history[2].result, 'FAILURE')
 
         held_nodes = 0
         for node in self.fake_nodepool.getNodes():
@@ -1518,6 +1537,49 @@
         self.assertEqual(held_nodes, 1)
 
     @simple_layout('layouts/autohold.yaml')
+    def test_autohold_ignores_aborted_jobs(self):
+        client = zuul.rpcclient.RPCClient('127.0.0.1',
+                                          self.gearman_server.port)
+        self.addCleanup(client.shutdown)
+        r = client.autohold('tenant-one', 'org/project', 'project-test2',
+                            "reason text", 1)
+        self.assertTrue(r)
+
+        self.executor_server.hold_jobs_in_build = True
+
+        # Create a change that will have its job aborted
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        # Creating new patchset on change A will abort A,1's job because
+        # a new patchset arrived replacing A,1 with A,2.
+        A.addPatchset()
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(2))
+
+        self.waitUntilSettled()
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
+        self.assertEqual(A.data['status'], 'NEW')
+        # Note only the successful job for A,2 will report as we don't
+        # report aborted builds for old patchsets.
+        self.assertEqual(A.reported, 1)
+        # A,1 project-test2
+        self.assertEqual(self.history[0].result, 'ABORTED')
+        # A,2 project-test2
+        self.assertEqual(self.history[1].result, 'SUCCESS')
+
+        # Check nodepool for a held node
+        held_node = None
+        for node in self.fake_nodepool.getNodes():
+            if node['state'] == zuul.model.STATE_HOLD:
+                held_node = node
+                break
+        self.assertIsNone(held_node)
+
+    @simple_layout('layouts/autohold.yaml')
     def test_autohold_list(self):
         client = zuul.rpcclient.RPCClient('127.0.0.1',
                                           self.gearman_server.port)
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index b4702f9..92353fb 100755
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -178,6 +178,36 @@
         self.executor_server.release()
         self.waitUntilSettled()
 
+    def test_branch_variants_reconfigure(self):
+        # Test branch variants of jobs with inheritance
+        self.executor_server.hold_jobs_in_build = True
+        # This creates a new branch with a copy of the config in master
+        self.create_branch('puppet-integration', 'stable')
+        self.fake_gerrit.addEvent(
+            self.fake_gerrit.getFakeBranchCreatedEvent(
+                'puppet-integration', 'stable'))
+        self.waitUntilSettled()
+
+        with open(os.path.join(FIXTURE_DIR,
+                               'config/branch-variants/git/',
+                               'puppet-integration/.zuul.yaml')) as f:
+            config = f.read()
+
+        # Push a change that triggers a dynamic reconfiguration
+        file_dict = {'.zuul.yaml': config}
+        A = self.fake_gerrit.addFakeChange('puppet-integration', 'master', 'A',
+                                           files=file_dict)
+        self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
+        self.waitUntilSettled()
+
+        ipath = self.builds[0].parameters['zuul']['_inheritance_path']
+        for i in ipath:
+            self.log.debug("inheritance path %s", i)
+        self.assertEqual(len(ipath), 5)
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.waitUntilSettled()
+
 
 class TestInRepoConfig(ZuulTestCase):
     # A temporary class to hold new tests while others are disabled
diff --git a/zuul/configloader.py b/zuul/configloader.py
index 426842b..2093c70 100644
--- a/zuul/configloader.py
+++ b/zuul/configloader.py
@@ -1635,7 +1635,10 @@
         else:
             # Use the cached branch list; since this is a dynamic
             # reconfiguration there should not be any branch changes.
-            branches = project.unparsed_branch_config.keys()
+            branches = sorted(project.unparsed_branch_config.keys())
+            if 'master' in branches:
+                branches.remove('master')
+                branches = ['master'] + branches
 
         for branch in branches:
             fns1 = []
diff --git a/zuul/nodepool.py b/zuul/nodepool.py
index 7dafca0..b96d1ca 100644
--- a/zuul/nodepool.py
+++ b/zuul/nodepool.py
@@ -87,9 +87,6 @@
         :param set autohold_key: A set with the tenant/project/job names
             associated with the given NodeSet.
         '''
-        if autohold_key not in self.sched.autohold_requests:
-            return
-
         (hold_iterations, reason) = self.sched.autohold_requests[autohold_key]
         nodes = nodeset.getNodes()
 
diff --git a/zuul/scheduler.py b/zuul/scheduler.py
index d402d51..8db9e46 100644
--- a/zuul/scheduler.py
+++ b/zuul/scheduler.py
@@ -922,16 +922,19 @@
             autohold_key = (build.pipeline.layout.tenant.name,
                             build.build_set.item.change.project.canonical_name,
                             build.job.name)
-
-            try:
-                self.nodepool.holdNodeSet(nodeset, autohold_key)
-            except Exception:
-                self.log.exception("Unable to process autohold for %s:",
-                                   autohold_key)
-                if autohold_key in self.autohold_requests:
-                    self.log.debug("Removing autohold %s due to exception",
-                                   autohold_key)
-                    del self.autohold_requests[autohold_key]
+            if (build.result == "FAILURE" and
+                autohold_key in self.autohold_requests):
+                # We explicitly only want to hold nodes for jobs if they have
+                # failed and have an autohold request.
+                try:
+                    self.nodepool.holdNodeSet(nodeset, autohold_key)
+                except Exception:
+                    self.log.exception("Unable to process autohold for %s:",
+                                       autohold_key)
+                    if autohold_key in self.autohold_requests:
+                        self.log.debug("Removing autohold %s due to exception",
+                                       autohold_key)
+                        del self.autohold_requests[autohold_key]
 
             self.nodepool.returnNodeSet(nodeset)
         except Exception: