1218919 - Try branch config re-defines everything instead of inheriting from the base job_flags.yml

Status: RESOLVED FIXED

(Taskcluster :: Services, defect)

Description

[Andrew Halberstadt [:ahal]]

There are a few taskcluster jobs (i.e flame-kk-spark builds) that run on every branch *except* for try. Not only is this a bad idea in that you get backed out despite a green try run, but as a result, the try branch config has to redefine everything.

This is bad, because it leaves an opportunity for what's running on try to get out of sync with what's running everywhere else.

Comment 1

[Andrew Halberstadt [:ahal]]

In case it wasn't clear, I'm proposing that anything running in the base_flags should also be running on try. If there is a legitimately good reason for those builds to be disabled on try, then I think we should manually add them to each branch they should be running on, instead of using the base_flags.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: MozReview Request: Bug 1218919 - Make try branch config inherit from base_flags.yml, r=garndt

Bug 1218919 - Make try branch config inherit from base_flags.yml

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Diff of extra jobs running on try after this change

Here's a diff of the extra jobs that will run on try due to this change. I obtained it by running:
> mach taskcluster-graph --project=try --message='try: -b do -p all -u all' --owner [e-mail] --extend-graph --print-names-only

both with and without this patch applied. Don't confuse the dashes as minuses.. they aren't part of the diff, no jobs are removed.

Greg, do you think it's ok that these extra jobs are scheduled on try? Or, who should I ask about it?

Comment 4

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

I suspect that at some point we *will* want to be able to schedule things on integration branches that aren't on try, so it'd be a shame to eliminate this possibility entirely.  There are a number of thingies flagged with try_by_default: false in buildbot-configs - https://dxr.mozilla.org/build_buildbot-configs/search?case=true&q=try_by_default

Maybe we should define "all" as an alias?

Comment 5

[Andrew Halberstadt [:ahal]]

It's not eliminating the possibility, you can still add those jobs to each of the various integration branches individually. If that's too much of a pain in the future, we could create a "everything_but_try.yml" (though I'm not fond of that idea). But if the jobs listed in that diff actually *shouldn't* be on try, just say the word and I'll remove them from base_flags.yml and add them directly to the branch configs they're supposed to be enabled on.

As for the try_by_default=False, I would love for that to exist in taskcluster. I was planning to file a bug about it. That way we could test out experimental jobs more easily on try without having them wasting machine resources on everyone's try: all pushes.

Also +1 to an 'all' alias. With buildbot jobs, I use "try: -a" to get everything.

Comment 6

[Greg Arndt [:garndt]]

(In reply to Andrew Halberstadt [:ahal] from comment #3)
> Created attachment 8680188 [details]
> Diff of extra jobs running on try after this change
> 
> Here's a diff of the extra jobs that will run on try due to this change. I
> obtained it by running:
> > mach taskcluster-graph --project=try --message='try: -b do -p all -u all' --owner [e-mail] --extend-graph --print-names-only
> 
> both with and without this patch applied. Don't confuse the dashes as
> minuses.. they aren't part of the diff, no jobs are removed.
> 
> Greg, do you think it's ok that these extra jobs are scheduled on try? Or,
> who should I ask about it?

re: the nexus builds, those are probably ok to be on Try.  The one build I know we try to not have on Try is the OTA builds, which I don't see here anyways.

Also, these jobs should not be run on Try as they currently require access to our remote device lab. We're not ready to push those out to try.  To make things easier this could just be added to the b2g-inbound config.  They are not required for all branches.
 - [TC] B2G Flame KK Eng
+  - [TC] Gaia Python Integration Sanity Tests
+  - [TC] Gaia Python Integration Functional DSDS Tests
+  - [TC] Gaia Python Functional Integration Tests
+  - [TC] Gaia Python Functional Integration Tests
+  - [TC] Gaia Python Functional Integration Tests
+  - [TC] Gaia Python Integration Unit Tests

Comment 7

[Andrew Halberstadt [:ahal]]

Comment on attachment 8680163 [details]
MozReview Request: Bug 1218919 - Make try branch config inherit from base_flags.yml, r=garndt

Bug 1218919 - Make try branch config inherit from base_flags.yml, r=garndt

Comment 8

[Andrew Halberstadt [:ahal]]

Attachment: Diff of extra jobs running on try after this change

Here's the update diff of new jobs running on try. I verified that b2g-inbound is the same after this patch.

Comment 9

[Greg Arndt [:garndt]]

Just be aware that depending on timing, bug 1219118 might affect landing of this patch because they are both touching the base jobs file. (touching different parts but should be ok).

Comment 10

[Greg Arndt [:garndt]]

Comment on attachment 8680163 [details]
MozReview Request: Bug 1218919 - Make try branch config inherit from base_flags.yml, r=garndt

https://reviewboard.mozilla.org/r/23581/#review21169

Looking at the diff between graphs and this patch, looks like things should work out ok.  Thanks for doing this!

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef14325835f6

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ef14325835f6

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/ef14325835f6