From bdce6ed65a5c6a85caa79fa87d1c409db9682a53 Mon Sep 17 00:00:00 2001 From: Paul Belanger Date: Wed, 21 Mar 2018 13:27:18 -0400 Subject: [PATCH] Enable autohold for RETRY_LIMIT / POST_FAILURE We'd like to support autohold on RETRY_LIMIT / POST_FAILURE results, making it easier for operators to debug failing jobs. Change-Id: I367bd92b6ae24e097b3112dcbe876d14d9e4802e Signed-off-by: Paul Belanger --- .../common-config/playbooks/post-fail.yaml | 5 ++ .../pre-playbook/git/common-config/zuul.yaml | 19 ++++++ .../pre-playbook/git/org_project2/.zuul.yaml | 5 ++ .../pre-playbook/git/org_project2/README | 1 + .../pre-playbook/git/org_project3/.zuul.yaml | 5 ++ .../pre-playbook/git/org_project3/README | 1 + tests/fixtures/config/pre-playbook/main.yaml | 3 +- tests/unit/test_v3.py | 61 +++++++++++++++++++ zuul/scheduler.py | 5 +- 9 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 tests/fixtures/config/pre-playbook/git/common-config/playbooks/post-fail.yaml create mode 100644 tests/fixtures/config/pre-playbook/git/org_project2/.zuul.yaml create mode 100644 tests/fixtures/config/pre-playbook/git/org_project2/README create mode 100644 tests/fixtures/config/pre-playbook/git/org_project3/.zuul.yaml create mode 100644 tests/fixtures/config/pre-playbook/git/org_project3/README diff --git a/tests/fixtures/config/pre-playbook/git/common-config/playbooks/post-fail.yaml b/tests/fixtures/config/pre-playbook/git/common-config/playbooks/post-fail.yaml new file mode 100644 index 0000000000..f58533f10a --- /dev/null +++ b/tests/fixtures/config/pre-playbook/git/common-config/playbooks/post-fail.yaml @@ -0,0 +1,5 @@ +- hosts: all + tasks: + - invalid-task: + path: "{{zuul._test.test_root}}/{{zuul.build}}.post.flag" + state: touch diff --git a/tests/fixtures/config/pre-playbook/git/common-config/zuul.yaml b/tests/fixtures/config/pre-playbook/git/common-config/zuul.yaml index 16f48b1ccb..449fb5a644 100644 --- a/tests/fixtures/config/pre-playbook/git/common-config/zuul.yaml +++ b/tests/fixtures/config/pre-playbook/git/common-config/zuul.yaml @@ -21,3 +21,22 @@ pre-run: playbooks/pre.yaml post-run: playbooks/post.yaml run: playbooks/python27.yaml + +- job: + name: python27-node + pre-run: playbooks/pre.yaml + post-run: playbooks/post.yaml + run: playbooks/python27.yaml + nodeset: + nodes: + name: test + label: label1 + +- job: + name: python27-node-post + post-run: playbooks/post-fail.yaml + run: playbooks/python27.yaml + nodeset: + nodes: + name: test + label: label1 diff --git a/tests/fixtures/config/pre-playbook/git/org_project2/.zuul.yaml b/tests/fixtures/config/pre-playbook/git/org_project2/.zuul.yaml new file mode 100644 index 0000000000..57ce2a71cd --- /dev/null +++ b/tests/fixtures/config/pre-playbook/git/org_project2/.zuul.yaml @@ -0,0 +1,5 @@ +- project: + name: org/project2 + check: + jobs: + - python27-node diff --git a/tests/fixtures/config/pre-playbook/git/org_project2/README b/tests/fixtures/config/pre-playbook/git/org_project2/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/pre-playbook/git/org_project2/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/pre-playbook/git/org_project3/.zuul.yaml b/tests/fixtures/config/pre-playbook/git/org_project3/.zuul.yaml new file mode 100644 index 0000000000..06f8cc8751 --- /dev/null +++ b/tests/fixtures/config/pre-playbook/git/org_project3/.zuul.yaml @@ -0,0 +1,5 @@ +- project: + name: org/project3 + check: + jobs: + - python27-node-post diff --git a/tests/fixtures/config/pre-playbook/git/org_project3/README b/tests/fixtures/config/pre-playbook/git/org_project3/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/pre-playbook/git/org_project3/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/pre-playbook/main.yaml b/tests/fixtures/config/pre-playbook/main.yaml index 60338795d1..7eacee534d 100644 --- a/tests/fixtures/config/pre-playbook/main.yaml +++ b/tests/fixtures/config/pre-playbook/main.yaml @@ -6,4 +6,5 @@ - common-config untrusted-projects: - org/project - + - org/project2 + - org/project3 diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index aa86896f79..e72eefd380 100755 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -2189,6 +2189,67 @@ class TestPrePlaybooks(AnsibleZuulTestCase): self.assertTrue(os.path.exists(post_flag_path), "The file %s should exist" % post_flag_path) + def test_post_playbook_fail_autohold(self): + client = zuul.rpcclient.RPCClient('127.0.0.1', + self.gearman_server.port) + self.addCleanup(client.shutdown) + r = client.autohold('tenant-one', 'org/project3', 'python27-node-post', + "", "", "reason text", 1) + self.assertTrue(r) + + A = self.fake_gerrit.addFakeChange('org/project3', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + build = self.getJobFromHistory('python27-node-post') + self.assertEqual(build.result, 'POST_FAILURE') + + # 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.assertIsNotNone(held_node) + # Validate node has recorded the failed job + self.assertEqual( + held_node['hold_job'], + " ".join(['tenant-one', + 'review.example.com/org/project3', + 'python27-node-post', '.*']) + ) + self.assertEqual(held_node['comment'], "reason text") + + def test_pre_playbook_fail_autohold(self): + client = zuul.rpcclient.RPCClient('127.0.0.1', + self.gearman_server.port) + self.addCleanup(client.shutdown) + r = client.autohold('tenant-one', 'org/project2', 'python27-node', + "", "", "reason text", 1) + self.assertTrue(r) + + A = self.fake_gerrit.addFakeChange('org/project2', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + build = self.getJobFromHistory('python27-node') + self.assertIsNone(build.result) + self.assertIn('RETRY_LIMIT', A.messages[0]) + + # 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.assertIsNotNone(held_node) + # Validate node has recorded the failed job + self.assertEqual( + held_node['hold_job'], + " ".join(['tenant-one', + 'review.example.com/org/project2', + 'python27-node', '.*']) + ) + self.assertEqual(held_node['comment'], "reason text") + class TestPostPlaybooks(AnsibleZuulTestCase): tenant_config_file = 'config/post-playbook/main.yaml' diff --git a/zuul/scheduler.py b/zuul/scheduler.py index c7a3dc0c90..dbad1bb45a 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -1041,8 +1041,9 @@ class Scheduler(threading.Thread): def _processAutohold(self, build): # We explicitly only want to hold nodes for jobs if they have - # failed and have an autohold request. - if build.result != "FAILURE": + # failed / retry_limit / post_failure and have an autohold request. + hold_list = ["FAILURE", "RETRY_LIMIT", "POST_FAILURE"] + if build.result not in hold_list: return autohold_key = self._getAutoholdRequestKey(build)