1548781 - Report github status as "failure" if any task in group has failed

Status: NEW

(Taskcluster :: Services, defect)

Description

[Mitchell Hentges [:mhentges] 🦀]

Attachment: pending-github.png

While working on Fenix, I've noticed that the commit builds and PR builds are frequently shown with the yellow circle ("still processing") yet, when I follow the links to Taskcluster, I can see that >= 1 of the tasks have already failed.

I'm guessing that this is because taskcluster-github sees that there's still pending builds, so it reports that the overall build is pending (even though we already know that there's failures).

For example:

1. (See attachment pending-github.png)
2. (See attachment on-taskcluster.png)

Preferred behaviour:

• If all tasks in group are green, report build as "succeeded"
• If all tasks in group are either green or pending, report build as "pending"
• If any task is failed/exception'd/etc, report build as "failed"

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

Attachment: on-taskcluster.png

Comment 2

[[:owlish] 🦉 PST]

Dustin, I believe it's the same problem with the binding: queueEvents.taskFailed('route.${this.context.cfg.app.statusTaskRoute}')
because they have a decision task.

To solve the immediate need, I would roll back Checks altogether (we are the only people using them anyways, and I am in a different project at the moment so no work is being done), use only Statuses everywhere and return to the old schedulerId bindings.

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

This issue might be minor enough to just wait until we more fully support checks, rather than rolling back. Is that OK with you, Mitchell?

Comment 4

[[:owlish] 🦉 PST]

Actually, another thing to do on Fenix's end would be to edit the decision task to the route field to the task definitions, with value statuses. It's not how it's supposed to be (editing things manually I mean), but should do for now

Comment 5

[Mitchell Hentges [:mhentges] 🦀]

Yeah, I'm sure we'll be okay without rolling back.
Just to make sure I understand correctly: owlish, you're referring to each task in the group having:

route:
  - statuses
  ...

Is that correct?

Comment 6

[[:owlish] 🦉 PST]

Correct!

Comment 7

[Simon Sapin (:SimonSapin)]

I’m also seeing this behavior in Servo (for example in https://community-tc.services.mozilla.com/tasks/groups/dBc6k0i9S_65B08EOsNXLw), and would also prefer the behavior proposed in the bug’s description.

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

I think checks sort of "automatically" behave this way. Maybe the answer here is to just move everyone to checks?

Comment 9

[Simon Sapin (:SimonSapin)]

Checks is not viable yet for Servo, because of bug 1533235.

Homu tests PRs serially, so the time until the Status is resolved matters a lot. This bug is really hurting that cycle time. Worse, manually cancelling tasks seems to make the Status never be resolved.

Comment 10

[Simon Sapin (:SimonSapin)]

When reading the code it seems it already behave as expected:

https://github.com/taskcluster/taskcluster/blob/5e6c2b635b916f495873e9b64fd07a69fb090946/services/github/src/handlers.js#L387-L389

Then I read comment 2 again and realize this is what happens: tc-gh is not getting task-specific messages for tasks created by Servo’s decision task. Owlish, are there downsides to the work-around you suggested in comment 4?

Comment 11

[[:owlish] 🦉 PST]

Then I read comment 2 again and realize this is what happens: tc-gh is not getting task-specific messages for tasks created by Servo’s decision task. Owlish, are there downsides to the work-around you suggested in comment 4?

You have to manually edit the files..... and also when the work on checks is resumed and any changes made, you'd have to manually edit them again (likely)..... Like, your decision task code gets coupled a bit with our checks implementation.... Other than that, I don't think I see any other downsides.

Comment 12

[Simon Sapin (:SimonSapin)]

Note that this issue is about Statuses, but yeah the work-around is the same as with Checks.

The situation with Checks + decision tasks is tracked in bug 1533235. It’s is arguably worse since we don’t get the benefit of taskGroupResolved messages: other tasks are ignored entirely.

when the work on checks is resumed and any changes made,

When the time comes, could you please comment in bug 1533235 about those changes?