Wpt: TaskCluster is not running tests from branches other than master

[clopez]

Meanwhile checking if the issue with #19961 was resolved or not, I have notified that TaskCluster is not running anymore the tests from any of the epochs/* or triggers/* branches.

You can see this for example on the daily branch: if you click on the last commit you will see taskcluster run, but if you pay attention that run is not from that branch, but for master. You can see that in the taskcluster run it has ran with firefox nightly and chrome dev, which corresponds to master:

event.ref == "refs/heads/master": [{name: firefox, channel: nightly}, {name: chrome, channel: dev}],

But the run for epochs/daily (firefox/stable, chrome/stable, webkitgtk_minibrowser/nightly) is missing there

At the bottom, there is an error from taskcluster on the commit

To test if I could reproduce the issue, I pushed master (10d4cb65c0) to the branch triggers/webkitgtk_minibrowser_nightly and the same happened. But then I pushed an empty commit and it worked. (I mean that it worked that taskcluster did what was expected, not that the test itself inside TC passed)

I think for some reason taskcluster is failing to process the commits on branches where the commit has been already processed on master or something like that, because it fails when you push something from master but not when you push something new to the branch.

The only change on .taskcluster.yml I see since the last time I know this worked fine is 8aeb449009

//cc @jgraham @foolip

[foolip]

@stephenmcgruer I guess this is why stable builds are gone, and also a case where better monitoring might have identified the cause earlier. (Because the severity would have been more obvious if notes WebKitGTK runs also missing.)

@jgraham I've assigned to you, are you able to look into this?

[stephenmcgruer]

I think the monitoring that would have worked here would have had to be reacting to (or checking for) expected taskcluster events. We knew that stable was missing; @Hexcles just thought it was the github outage since that was going on at the same time. I don't think knowing that WebKitGTK was missing too would have caused us to assume anything else than the github outage took that out too.

That said, monitoring for expected/unexpected taskcluster events on all branches is also very reasonable. I will add this to my pile of monitoring-related issues, but currently tackling monitoring is below the metrics and wpt.fyi improvements work going on.

[clopez]

This seems to have worked now that i pushed master again to branch triggers/webkitgtk_minibrowser_nightly
Let's wait to see if it gets fixed also for the epochs/daily branch

[clopez]

Let's wait to see if it gets fixed also for the epochs/daily branch

Unfortunately this issue continues to happen on the epochs/daily branch:

On the last commit from yesterday on that branch there are 2 errors from taskcluster logged (data.metadata.owner should match format "email") and there are not runs registered for chrome/stable firefox/stable or webkitgtk_minibrowser/nightly

[jgraham]

I'm assuming the problem here is that the method of creating those branches changed and it actually doesn't provide a valid email address. So the question is, which process now pushes those branches?

[clopez]

On the last commit from yesterday on that branch there are 2 errors from taskcluster logged (data.metadata.owner should match format "email")

Trying to guess what can cause this error this may be because the one (the bot doing that) that pushes commits to the epochs branches has an invalid e-mail for its account?? The taskcluster definition sets data.metadata.owner = ${event.pusher.email}

[jgraham]

Yes, exactly. So either we fix the pusher (which seems to be a GH action) to have something like a valid email address or we handle the case where we don't have an email address in the taskcluster definition.

[clopez]

Right, this points to be an issue with GH actions.
The pusher for the action seems to be pushing as the special user x-access-token user

[foolip]

We could use a wpt-pr-bot token just to get an email address, but that would be unfortunate when GitHub Actions provides a way to avoid storing a long-term token even as a secret.

[foolip]

@jgraham can you help find where to file an issue for this? I've come up empty with a search for "should match format" in taskcluster...

[clopez]

I was trying to test something like this

diff --git a/.taskcluster.yml b/.taskcluster.yml
index 6de1e280ce..37158b897b 100644
--- a/.taskcluster.yml
+++ b/.taskcluster.yml
@@ -73,7 +73,10 @@ tasks:
                   A subset of WPT's "${chunk[0]}" tests (chunk number ${chunk[1]}
                   of ${chunk[2]}), run in the ${browser.channel} release of
                   ${browser.name}.
-                owner: ${event.pusher.email}
+                owner:
+                  $if: '@' in ${event.pusher.email}
+                  then: ${event.pusher.email}
+                  else: ${event.pusher.name}@users.noreply.github.com
                 source: ${event.repository.url}
               payload:
                 image:

But I'm finding difficult to test it, the new community-tc is failing to work as expected on my wpt fork

[foolip]

@clopez that's a promising workaround! Can you try it on a branch in this repo instead?

I don't know what ${event.pusher.name} will resolve to, but it'd probably be best to use a fixed email address and not that of the person committing. A large fraction of commits will be the head of a triggers branch at some point, and notifications of failures there should go to wpt infra maintainers, not the wpt committer.

[jgraham]

The "should match format" is presumably generated by the json schema validation library. So that string isn't going to appear in the taskcluster source.

I think @clopez's fix is the easiest thing to do in the short term. But I don't know that in is valid json-e; I remember this stuff being hard to do.

[jgraham]

Oh, it's in the docs, so either that changed or I misremembered. So I think that patch is reasonable and will happily approve it if there's some evidence it actually works.

[clopez]

@clopez that's a promising workaround! Can you try it on a branch in this repo instead?

I can try, but I would need the GITHUB_TOKEN password to push as x-access-token. If you can share that with me, then I can try here in a branch.

[jgraham]

If you push to a trigger branch and things run I'm confident we won't break master and so we can merge and at the worst we don't fix the problem.

[foolip]

@clopez the token that's causing the problem is one from GitHub Actions. To know it works before landing a change you would need to arrange for GitHub Actions to push a change to a branch that would trigger a Taskcluster run. But as @jgraham says it's probably fine to experiment directly on master since things are already broken.

[Hexcles]

@clopez no one has access to that token. It's a black box; GitHub generates a new token for each job which expires in 60min. https://help.github.com/en/actions/automating-your-workflow-with-github-actions/authenticating-with-the-github_token

Another way to fix this: we could generate a personal access token for wpt-pr-bot and use that in our epoch update action.

[clopez]

I have opened PR #20199 with the above patch.

I tested it on the branch triggers/webkitgtk_minibrowser_nightly and it seems to work as expected.

I also tested that it works on the else condition (metadata.owner = [email protected])

But until the github action triggers I can't be 100% sure it will work in that case, in any case I don't think it will break things more than currently are.

[foolip]

Looks like it didn't work: https://github.com/web-platform-tests/wpt/commit/dfb9b28271#commitcomment-35904695

New error is "InterpreterError at template.tasks[0].metadata.owner: Infix: in expects Array, string, or object on right side"

[foolip]

@clopez I've reassigned to you since you've already begun working on this.

[clopez]

I opened PR #20211 with a second attempt at fix this

[stephenmcgruer]

Still... failing... I think? (Caveat: jetlagged)

https://github.com/web-platform-tests/wpt/commit/29675aba0993eb9543825568753a1b01385c7c28#commitcomment-35924646

"data.metadata.owner should match format "email""

Note: we're now at 10 days without a stable run. I am getting more concerned. @foolip

[clopez]

Let's try again.. I opened PR #20224 this time modiying the taskcluster config to trigger also on the branch epochs/three_hourly to speed up testing (this will be reverted once things work as expected)

[foolip]

I just did git push --force-with-lease origin origin/epochs/daily:triggers/firefox_stable and might do a few more. Mentioning this to not confuse anyone, it's not the epochs/* triggers that have suddenly begun working.

[foolip]

Triggered task groups: Chrome stable and Firefox stable

Chrome/Firefox beta didn't work however, 500 InternalServerError: https://github.com/web-platform-tests/wpt/commit/f291a09ba5#commitcomment-35928798

[clopez]

Triggered task groups: Chrome stable and Firefox stable

Chrome/Firefox beta didn't work however, 500 InternalServerError: f291a09#commitcomment-35928798

I see that branches triggers/chrome_beta and triggers/firefox_beta are currently on f291a09ba5 which is previous to 689b968cd1. Likely that explains it

I hope that if they are updated to current master, then taskcluster run will work.
Note: I have merged beff32767b on master.

[clopez]

It triggered now for branch epochs/three_hourly but it failed again another time with error data.metadata.owner should match format "email"

So, I don't know what to do... since I can't see the json that github sends to taskcluster I can't debug this further

Will be setting the owner e-mail always to something static like [email protected] (without any variable that can be broken) acceptable?

[Hexcles]

Here's another idea. Can we hard code in .taskcluster.yml an email address (e.g. a bot account) as the owner of the epochs/* pushes?

[clopez]

Here's another idea. Can we hard code in .taskcluster.yml an email address (e.g. a bot account) as the owner of the epochs/* pushes?

Good idea. I have opened PR #20232 with that

[clopez]

So, I don't know what to do... since I can't see the json that github sends to taskcluster I can't debug this further

Hopefully once we get the first runs from github actions this can be improved, as taskcluster shows you the json from github when you click on "_View task definition_" for a given task (at the end of the json it shows it includes an "extra": { "github_event": with all the details)

[clopez]

It seems it worked! \o/
And now I know why it was failing before:

• event.sender.login is github-actions[bot] which likely is not valid for as uid of an e-mail (because of the [] characters)
• event.pusher.email is null

[Hexcles]

event.sender.login is github-actions[bot] which likely is not valid for as uid of an e-mail (because of the [] characters)

Darn!

So I'm in favour of keeping #20232 as the permanent solution.

[foolip]

https://github.com/web-platform-tests/wpt/pull/20232 seems fine to keep permanently, but could we check event.pusher.email directly? Or was that one of the attempts that didn’t work out?

Let’s wait until we have the next daily runs triggered before closing this.

[jgraham]

Once we land the decision task we can write more complex logic here in Python rather than in json-e, and so fix this in a better way.

[clopez]

but could we check event.pusher.email directly? Or was that one of the attempts that didn’t work out?

It was the first attempt: 689b968cd1f8848dc2c62928ffd1b20598be3537

And it failed with:

InterpreterError at template.tasks[0].metadata.owner: Infix: in expects Array, string, or object on right side

Maybe I should have null-checked it? 'event.pusher.mail && "@" in event.pusher.email'

[clopez]

Once we land the decision task we can write more complex logic here in Python rather than in json-e, and so fix this in a better way.

Other useful thing (IMHO) would be that the errors that taskcluster writes as comments on the commits to be more verbose and to include more helpful debugging information like the whole json it has received that caused the error.

[clopez]

Maybe I should have null-checked it? 'event.pusher.mail && "@" in event.pusher.email'

I have opened another PR doing this check #20249
I tested it with another variable that is null when i push (event.base_ref) and it worked on my tests.

[clopez]

Nice, it worked as expected on the branch epochs/three_hourly

I think we can remove now the temporal trigger for this branch that was added to speed up testing and then close this issue.

[stephenmcgruer]

That's awesome! Thank you for your work resolving this, it has been a hard issue but so important to get fixed. Here's to getting our stable runs back!

[clopez]

That's awesome! Thank you for your work resolving this, it has been a hard issue but so important to get fixed. Here's to getting our stable runs back!

Thanks! It has been time consuming fix this because of the lack of data that I had about what was happening between github and taskcluster and the difficulty to test changes. But in the end things are working back, so that's nice. stable runs back and webkitgtk nightlies also! ;)

I have opened PR #20255 for removing the temporal trigger on epochs/three_hourly

[clopez]

And the final check worked: the trigger on epochs/daily went as expected. !
So I'm closing this.

However, I want to comment also that one of the runs for WebKitGTK failed for an issue with pip that looks like related to unreliable network connection on the container runing the test.

ProtocolError: ("Connection broken: error(104, 'Connection reset by peer')", error(104, 'Connection reset by peer'))

Its not the first time I see network reliability issues inside taskcluster. See #19725

I don't know we can do about this.

[foolip]

Yep, network errors frequently cause Taskcluster runs to fail. What I think we should do is monitor runs and figure out which causes of failure are most common, then fix those until the rate of failure is acceptable. @stephenmcgruer FYI.