1205931 - try builds created by buildbot bridge fail to upload logs

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Nick Thomas [:nthomas] (UTC+12)]

eg:
Running [u'/builds/buildbot/try1/bin/python', u'/builds/buildbot/try1/lib/python2.7/site-packages/buildbotcustom/bin/try_mailer.py', u'--log-url', u'http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-88ba41c28dce/try-linux64/try-linux64-bm78-try1-build2849.txt.gz', u'-f', u'[e-mail]', u'--to-author', u'/builds/buildbot/try1/master/try-linux64', u'2849']

Traceback (most recent call last):
  File "/builds/buildbot/try1/lib/python2.7/site-packages/buildbotcustom/bin/try_mailer.py", line 201, in <module>
    match = re.search("try: ", build.source.changes[-1].comments)
IndexError: tuple index out of range

There doesn't appear to be any change associated with the build (eg http://buildbot-master78.build.mozilla.org:8101/builders/Linux%20x86-64%20try%20build/builds/2849).

The error results in alerts like
<nagios-releng> Thu 19:18:58 PDT [4677] buildbot-master114.bb.releng.use1.mozilla.com:Command Queue is CRITICAL: 2 dead items

Comment 1

[[e-mail] (:bhearsum)]

Sounds like we need bug 1197204 to fix this. I had put it off because Armen found a workaround for the original reason it was filed. Supporting Changes enough to make this not fail should be easy enough, though I'm not sure w'll be able to get at the commit message.

Comment 2

[[e-mail] (:bhearsum)]

Reading try_mailer.py a bit I _think_ comments is the only thing needed, and even then it's only usued to determine e-mail preferences. As a short term workaround, just creating an empty change and setting comments to _anything_ should fix the log upload issue.

https://github.com/mozilla/build-buildbotcustom/blob/master/bin/try_mailer.py#L56 tries to pull the author out of properties first, which is set even for the buildbot bridge submitted try job.

Comment 3

[Armen [:armenzg]]

nthomas: I assumethere are no changes because I'm scheduling the tasks off-band. Not triggered by a spuh.

Can I set changes from the task definition?
Or should I set something in the properties? [1]

Should we add this requirement to the json validation?

[1] https://github.com/armenzg/mozilla_ci_tools/blob/mozci_bbb/mozci/sources/buildbot_bridge.py#L53

Comment 4

[[e-mail] (:bhearsum)]

(In reply to Armen Zambrano Gasparnian [:armenzg] from comment #3)
> nthomas: I assumethere are no changes because I'm scheduling the tasks
> off-band. Not triggered by a spuh.
> 
> Can I set changes from the task definition?
> Or should I set something in the properties? [1]
> 
> Should we add this requirement to the json validation?
> 
> [1]
> https://github.com/armenzg/mozilla_ci_tools/blob/mozci_bbb/mozci/sources/
> buildbot_bridge.py#L53

Changes are not supported by the Bridge yet, per bug 1197204.

Comment 5

[Armen [:armenzg]]

bhearsum: I believe that if we taught the try_mailer.py to also check in the properties it would work, however, fixing changes would also work.


nthomas: does the log not get uploaded? or the email is not sent out?

I see both logs in here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-88ba41c28dce/try-linux64/

These are logs for this push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=88ba41c28dce

Comment 6

[[e-mail] (:bhearsum)]

(In reply to Armen Zambrano Gasparnian [:armenzg] from comment #5)
> bhearsum: I believe that if we taught the try_mailer.py to also check in the
> properties it would work, however, fixing changes would also work.

There's really no point in having try mailer look in properties. It's about the same amount of difficulty adding Change support as it is making "comments" a required property -- the tricky part is _finding_ the comments, not communicating them to Buildbot. If we want a short term workaround here, I suggest making try_mailer.py not die if it can't find comments.

> 
> nthomas: does the log not get uploaded? or the email is not sent out?

I can see your confusion. This is failing to send the "log uploaded" pulse message, but it doesn't mean that the log wasn't uploaded, just that the pulse message couldn't be sent, which ends up showing up as a dead item in the command queue, causing a nagios alert to go off and human intervention required.

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

It would be really really nice if we could keep try syntax away from TC (and lure it into a trap and put it out of its misery). Since using mozci+bbb to trigger jobs is quite different from pushing with a try syntax to turn on emails, I'd be in favour of adjusting try_mailer.py to not die, and just default to no email in this scenario.

Comment 8

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #7)
> It would be really really nice if we could keep try syntax away from TC (and
> lure it into a trap and put it out of its misery). Since using mozci+bbb to
> trigger jobs is quite different from pushing with a try syntax to turn on
> emails, I'd be in favour of adjusting try_mailer.py to not die, and just
> default to no email in this scenario.

+1 to moving away from try syntax. I'll see what I can do about a patch to try_mailer.py.

Comment 9

[[e-mail] (:bhearsum)]

Attachment: make try mailer a noop for jobs without comments or who

After a full reading of it, it looks like guarding against changes being an empty tuple is only part of try_mailer.py that needs fixing. However, I also saw that an Exception might be raised if "who" cannot be found. I don't _think_ that code will get hit if changes is empty, but I changed that logic a bit to be sure.

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8663679 [details] [diff] [review]
make try mailer a noop for jobs without comments or who

lgtm

Comment 11

[[e-mail] (:bhearsum)]

Armen did a try job through the Bridge today after this went to production and it worked! https://tools.taskcluster.net/task-graph-inspector/#7KDymp-MSHW5ysmRt6CZJQ/