Closed Bug 1218919 Opened 6 years ago Closed 6 years ago

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

Categories

(Taskcluster :: Services, defect)

defect
Not set
normal

Tracking

(b2g-v2.5 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
b2g-v2.5 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
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.
Bug 1218919 - Make try branch config inherit from base_flags.yml
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
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 ahalberstadt@mozilla.com --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?
Flags: needinfo?(garndt)
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?
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.
(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 ahalberstadt@mozilla.com --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
Flags: needinfo?(garndt)
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
Attachment #8680163 - Attachment description: MozReview Request: Bug 1218919 - Make try branch config inherit from base_flags.yml → MozReview Request: Bug 1218919 - Make try branch config inherit from base_flags.yml, r=garndt
Attachment #8680163 - Flags: review?(garndt)
Here's the update diff of new jobs running on try. I verified that b2g-inbound is the same after this patch.
Attachment #8680188 - Attachment is obsolete: true
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 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!
Attachment #8680163 - Flags: review?(garndt) → review+
Duplicate of this bug: 1213798
Blocks: 1219118
https://hg.mozilla.org/mozilla-central/rev/ef14325835f6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Component: Integration → Services
You need to log in before you can comment on or make changes to this bug.