896015 - Fix return codes and tbpl coloring for talos mozharness

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Armen [:armenzg]]

Attachment: only consider return codes for talos mozharness

If we look at this log:
https://tbpl.mozilla.org/php/getParsedLog.php?id=25494235&tree=Try&full=1
we see that the return code is 1 rather than 2, yet, tbpl marked the job as "red".

mozharness unittests only listen to return codes and we want talos mozharness to do the same.

log_eval_func=rc_eval_func({
              0: SUCCESS,
              1: WARNINGS,
              2: FAILURE,
              3: EXCEPTION,
              4: RETRY,
            }),

The patch attached has already been reviewed positively.

(In reply to Aki Sasaki [:aki] from comment #22)
> Comment on attachment 778586 [details] [diff] [review]
> only consider return codes for talos mozharness
> 
> Before we land this and reconfig, we should verify the talos script actually
> exits with the appropriate self.return_code.
> 
> To do that we may have to add more self.buildbot_status() calls in
> mozharness.mozilla.testing.talos.  However, iirc, talos never goes orange,
> only green/red/retry.

(In reply to Ed Morley [:edmorley UTC+1] from comment #23)
> (In reply to Aki Sasaki [:aki] from comment #22)
> > However, iirc, talos never goes orange,
> > only green/red/retry.
> 
> Talos also goes orange for crashes and test-unexpected-fails as of bug
> 829728, via:
> https://hg.mozilla.org/build/buildbotcustom/file/f24d9219c221/steps/talos.
> py#l115

Comment 1

[Jason Yeo[:jyeo]]

Attachment: Call buildbot_status

I haven't tested this yet. I will do a try push next week. Let me know if this is what you are referring to.

Comment 2

[Aki Sasaki (not active)]

Comment on attachment 778639 [details] [diff] [review]
Call buildbot_status

Hm, I missed the fact that we already set self.return_code.

Do the talos return codes match what we're expecting here? (i.e., 0 success, 1 warning, 2 failure, 4 retry ?)  If so, we may not need this patch at all  If not, we may need to map the talos return codes to this set, or patch talos to return the codes we're expecting.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

talos returns 1 for orange, 2 for red, 0 for success.  With these and mozharness talos as it is on try server, I am unable to get orange:
https://tbpl.mozilla.org/php/getParsedLog.php?id=25494235&tree=Try&full=1

Comment 4

[Armen [:armenzg]]

(In reply to Aki Sasaki [:aki] from comment #2)
> Comment on attachment 778639 [details] [diff] [review]
> Call buildbot_status
> 
> Hm, I missed the fact that we already set self.return_code.
> 
> Do the talos return codes match what we're expecting here? (i.e., 0 success,
> 1 warning, 2 failure, 4 retry ?)  If so, we may not need this patch at all 
> If not, we may need to map the talos return codes to this set, or patch
> talos to return the codes we're expecting.

(In reply to Joel Maher (:jmaher) from comment #3)
> talos returns 1 for orange, 2 for red, 0 for success.  With these and
> mozharness talos as it is on try server, I am unable to get orange:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=25494235&tree=Try&full=1

aki, does this mean that we're good to go with just the first patch?

I will land it for now since it only affect talos on try.

Comment 5

[Armen [:armenzg]]

Comment on attachment 778618 [details] [diff] [review]
only consider return codes for talos mozharness

https://hg.mozilla.org/build/buildbotcustom/rev/a23709fffc46

Comment 6

[Jason Yeo[:jyeo]]

Attachment: Call buildbot_status. Fix minor bug.

Fixing a minor bug. I left out the underscore when calling determine_status.

I'm not sure how we are doing the tbpl log parsing. I applied the patch, forced return codes 0, 1 and 2 in talos and I got # TBPL SUCCESS #, # TBPL WARNING # and # TBPL FAILURE # respectively, printed in the logs. Is this the desired output?

I checked the logs for the other tests (mochitest, reftest, etc) and they color the jobs as orange when # TBPL WARNING # is present. I believe tbpl would color it orange when it sees # TBPL WARNING # in the logs right?

Comment 7

[Armen [:armenzg]]

Sheriffs, anyone know about comment 6?

Comment 8

[Ed Morley [:emorley]]

TBPL just displays the result given to it by buildbot, so this is a buildbot/releng question :-)

Comment 9

[Aki Sasaki (not active)]

(In reply to Armen Zambrano G. [:armenzg] (Release Enginerring) (EDT/UTC-4) from comment #4)
> (In reply to Aki Sasaki [:aki] from comment #2)
> > Comment on attachment 778639 [details] [diff] [review]
> > Call buildbot_status
> > 
> > Hm, I missed the fact that we already set self.return_code.
> > 
> > Do the talos return codes match what we're expecting here? (i.e., 0 success,
> > 1 warning, 2 failure, 4 retry ?)  If so, we may not need this patch at all 
> > If not, we may need to map the talos return codes to this set, or patch
> > talos to return the codes we're expecting.
> 
> (In reply to Joel Maher (:jmaher) from comment #3)
> > talos returns 1 for orange, 2 for red, 0 for success.  With these and
> > mozharness talos as it is on try server, I am unable to get orange:
> > https://tbpl.mozilla.org/php/getParsedLog.php?id=25494235&tree=Try&full=1
> 
> aki, does this mean that we're good to go with just the first patch?
> 
> I will land it for now since it only affect talos on try.

Yes, we're good to go with just the first patch.
If we want to declare any Exception or Retry statuses, we'll need to add those, but for green/orange/red we're good.

Comment 10

[Aki Sasaki (not active)]

(In reply to Jason Yeo [:jyeo] from comment #6)
> Created attachment 779220 [details] [diff] [review]
> Call buildbot_status. Fix minor bug.
> 
> Fixing a minor bug. I left out the underscore when calling determine_status.
> 
> I'm not sure how we are doing the tbpl log parsing. I applied the patch,
> forced return codes 0, 1 and 2 in talos and I got # TBPL SUCCESS #, # TBPL
> WARNING # and # TBPL FAILURE # respectively, printed in the logs. Is this
> the desired output?
> 
> I checked the logs for the other tests (mochitest, reftest, etc) and they
> color the jobs as orange when # TBPL WARNING # is present. I believe tbpl
> would color it orange when it sees # TBPL WARNING # in the logs right?

The TBPL strings are only needed until patch 1 is put into production; afterwards we're going to go by exit status only.

Comment 11

[Rail Aliiev [:rail]]

in production

Comment 12

[Armen [:armenzg]]

jmaher, can you please re-trigger your job and see if it got fixed?

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

all fixed, thanks guys!