Closed Bug 1291070 Opened 3 years ago Closed 3 years ago
run-task doesn't preserve exit code
https://tools.taskcluster.net/task-inspector/#JwegKrBNR02fojVRKDAhAA/0 demonstrates a task failure that resulted in exit code 0. I believe the bug is that we're not calling poll() on the process object.
Before, we were returning None, which gets converted to 0. Derp. Also fix a flake8 failure introduced by 9f5fbb3066c9. We'll also need to generate a new decision image. But that will require someone with TC privileges to be around. That can be done in a separate commit to unblock this from landing and fixing consumers of run-task that aren't the decision image. Review commit: https://reviewboard.mozilla.org/r/68440/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68440/
Attachment #8776771 - Flags: review?(mh+mozilla)
https://reviewboard.mozilla.org/r/68440/#review65562 ::: testing/docker/recipes/run-task:64 (Diff revision 1) > if data == '': > break > > print_line(prefix, data) > > - return p.returncode > + return p.poll() This seems about as racy as returning p.returncode. You want p.wait().
Comment on attachment 8776771 [details] Bug 1291070 - Return process exit code properly; Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68440/diff/1-2/
Comment on attachment 8776771 [details] Bug 1291070 - Return process exit code properly; https://reviewboard.mozilla.org/r/68440/#review65568
Attachment #8776771 - Flags: review?(mh+mozilla) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/51f838971d62 Return process exit code properly; r=glandium
Sorry had to back out your push due to Taskgraph Test failure. e.g., https://treeherder.mozilla.org/logviewer.html#?job_id=1278585&repo=autoland#L287
Backout by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/ea6e87bbd03e Backed out changeset 51f838971d62 for Taskgraph test failure
(In reply to Iris Hsiao [:ihsiao] from comment #6) > Sorry had to back out your push due to Taskgraph Test failure. e.g., > https://treeherder.mozilla.org/logviewer. > html#?job_id=1278585&repo=autoland#L287 This is one of the rare case where backing out was the wrong thing to do, because you just end up hiding a real problem, and possible future problems. The reason being, what this bug fixes is errors being ignored. So what happened is that what you backed out was just revealing an error that was ignored on preceding pushes. For instance, you'll find the same error in the log for the taskgraph test log on the previous push, except the job is not marked as failed because of this bug. https://treeherder.mozilla.org/logviewer.html#?job_id=1276396&repo=autoland
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=1c2fac92d6e4c8b7dee278bd22b09b96b4889db7 had a different error https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=df0e932cac3a403d8660997d701f8edc49a99c9e is the first one with the exact same error message, but might as well not be the source of it. And https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b18ad418cba80522c888d329ae4eb37ab07fb436 hid the failure, while this bug restored it.
The error is: AssertionError: 'e61e675ce05e8c11424437db3f1004079374c1a5fe6ad6800346cebe137b0797' != u'f13cde7fef96593671a41f81f9f0871c929a7dcba41d5a2dde1e8e1576b2bca0' https://hg.mozilla.org/integration/autoland/rev/aac97e9f9addbc974882a05253149ee646494415 changed that test.
Glandium's right on all points. The hash failure is from bug 1290611. This bug can be re-landed, and let's roll the decision image version bump (bug 1291148) in here while doing so. Greg, I'll wait for your go-ahead and a sha256 to push to docker hub.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/78e29c684909 Return process exit code properly; r=glandium
To pull in run-task exit code fix. Review commit: https://reviewboard.mozilla.org/r/68576/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/68576/
Attachment #8776962 - Flags: review?(dustin)
Comment on attachment 8776962 [details] Bug 1291070 - Tag and use decision image 0.1.4; https://reviewboard.mozilla.org/r/68576/#review65660
Attachment #8776962 - Flags: review?(dustin) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/4509bcf4ba36 Tag and use decision image 0.1.4; r=dustin
Fixes landed. Autoland repo is green. Nothing left to do here.
I think this is going to cause flake8 bustage on central when it gets merged in as an infraction slipped in from inbound in the meantime. TEST-UNEXPECTED-ERROR | taskcluster/taskgraph/task/docker_image.py:77:100 | line too long (100 > 99 characters) (E501) From: https://public-artifacts.taskcluster.net/G3KJ131PSJSzmK5Z1rBxVA/0/public/logs/live_backing.log
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.