Closed Bug 1293730 Opened 3 years ago Closed 3 years ago

Fennec x86 builds as tier 1

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla51

People

(Reporter: coop, Assigned: kmoir)

References

Details

Attachments

(5 files, 3 obsolete files)

No description provided.
Assignee: nobody → kmoir
Attached patch bug1293730tc.patch (obsolete) — Splinter Review
depends on patches in bug 1282468 to land first
Attached patch bug1283730bb.patch (obsolete) — Splinter Review
patch to remove bb android x86 builds and tests

builder diff looks like this
38d37
< Android 4.2 x86 autoland build SigningScriptFactory
67d65
< Android 4.2 x86 date build SigningScriptFactory
83d80
< Android 4.2 x86 mozilla-inbound build SigningScriptFactory
104,105d100
< Android 4.2 x86 oak build SigningScriptFactory
< Android 4.2 x86 oak nightly SigningScriptFactory
132d126
< Android 4.2 x86 fx-team build SigningScriptFactory
158,159d151
< Android 4.2 x86 mozilla-central build SigningScriptFactory
< Android 4.2 x86 mozilla-central nightly SigningScriptFactory

test builder diff is similar
:gbrown do you have any concerns regarding disabling buildbot android x86 builds/tests and moving to tc ones to tier 1?
Flags: needinfo?(gbrown)
That's okay with me. TC Android x86 tests are running well.
Flags: needinfo?(gbrown)
Attachment #8779484 - Flags: review?(jlund)
Comment on attachment 8779470 [details] [diff] [review]
bug1293730tc.patch

Review of attachment 8779470 [details] [diff] [review]:
-----------------------------------------------------------------

::: taskcluster/taskgraph/transforms/tests/all_kinds.py
@@ +35,5 @@
>      specify a tier otherwise."""
>      for test in tests:
>          # only override if not set for the test
>          if 'tier' not in test:
> +            if test['test-platform'] in ('linux64/debug', 'linux64-asan/opt', 'android-x86/opt'):

bumping asan opt at the same time?
(In reply to Kim Moir [:kmoir] from comment #1)
> Created attachment 8779470 [details] [diff] [review]
> bug1293730tc.patch
> 
> depends on patches in bug 1282468 to land first

ah, this answers my own question :)
Comment on attachment 8779484 [details] [diff] [review]
bug1283730bb.patch

Review of attachment 8779484 [details] [diff] [review]:
-----------------------------------------------------------------

hm, so does this mean we are okay with stopping x86 nightlies? should we remove it from https://nightly.mozilla.org/ and update anything else, e.g. balrog?
(In reply to Jordan Lund (:jlund) from comment #8)
> Comment on attachment 8779484 [details] [diff] [review]
> bug1283730bb.patch
> 
> Review of attachment 8779484 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> hm, so does this mean we are okay with stopping x86 nightlies? should we
> remove it from https://nightly.mozilla.org/ and update anything else, e.g.
> balrog?
Flags: needinfo?(kmoir)
Comment on attachment 8779470 [details] [diff] [review]
bug1293730tc.patch

Review of attachment 8779470 [details] [diff] [review]:
-----------------------------------------------------------------

::: taskcluster/taskgraph/transforms/tests/all_kinds.py
@@ +35,5 @@
>      specify a tier otherwise."""
>      for test in tests:
>          # only override if not set for the test
>          if 'tier' not in test:
> +            if test['test-platform'] in ('linux64/debug', 'linux64-asan/opt', 'android-x86/opt'):

Also, please spread these out one-per-line
gbrown: are we okay with stopping android x86 nightlies? We don't have nightlies working in TC yet
Flags: needinfo?(kmoir) → needinfo?(gbrown)
Oops. I think we need Android x86 nightlies.
Flags: needinfo?(gbrown)
Comment on attachment 8779470 [details] [diff] [review]
bug1293730tc.patch

Review of attachment 8779470 [details] [diff] [review]:
-----------------------------------------------------------------

::: taskcluster/taskgraph/transforms/tests/all_kinds.py
@@ +35,5 @@
>      specify a tier otherwise."""
>      for test in tests:
>          # only override if not set for the test
>          if 'tier' not in test:
> +            if test['test-platform'] in ('linux64/debug', 'linux64-asan/opt', 'android-x86/opt'):

I notice there is also tier-setting code at https://hg.mozilla.org/mozilla-central/annotate/0502bd9e025edde29777ba1de4280f9b52af4663/taskcluster/taskgraph/transforms/tests/android_test.py#l29 (which may not be working -- bug 1294297). It would be nice to have this all in one place, or, have symmetric android_test/desktop_test transforms.
Ugh, yes, please synthesize those two set_tier transforms :(
Comment on attachment 8779470 [details] [diff] [review]
bug1293730tc.patch

clearing reviews for now because of comment 12
Attachment #8779470 - Flags: review?(jlund)
Attachment #8779484 - Flags: review?(jlund)
See Also: → 1295186
(In reply to Jordan Lund (:jlund) from comment #15) 
> clearing reviews for now because of comment 12

We need to accept some amount of risk to make this TC transition in finite time. Can we push forward her, please? 

Builds generated in buildbot and TC are meant to be identical, modulo the timing and machine info inserted at build time. This is one of the steps that we're supposed to verify with each migration. 

Given that buildbot and TC builds *should* be interchangeable, can we disable everything except the Fennec x86 nightly build in buildbot and move all other builds/tests to tier 1 in TC?
coop: yes, I am refactoring the patches now
Attached patch bug1283730bb-2.patch (obsolete) — Splinter Review
disable buildbot builds for Android x86 but not nightly builds.  The test config branches don't all have a enable_nightly flag, this is why they have an explicit list of branches to skip.
Attachment #8779484 - Attachment is obsolete: true
Attachment #8779470 - Attachment is obsolete: true
Attachment #8781558 - Flags: review?(jlund)
Comment on attachment 8781571 [details] [diff] [review]
bug1293730tc-2.patch

The multiple tier references mentioned in comment 2 are no longer there, must have been fixed as part of another bug
Attachment #8781571 - Flags: review?(jlund)
Comment on attachment 8781558 [details] [diff] [review]
bug1283730bb-2.patch

Review of attachment 8781558 [details] [diff] [review]:
-----------------------------------------------------------------

sorry if I'm mistaken but I think this will keep all the m-c and oak x86 builds and tests. my interpretation from comment 16 was that we want to have just the x86 nightly build job and stop all x86 CI builds and tests on trunk.

::: mozilla-tests/mobile_config.py
@@ +3010,5 @@
> +        # nightly builds have not yet been migrated to TC
> +        # thus the bb ones stay enabled
> +        if name in ['mozilla-central', 'oak']:
> +            continue
> +        if 'android-x86' in platform:

I don't think this line is needed as the first condition in this `for` block achieves the same thing.
Attachment #8781558 - Flags: review?(jlund) → review-
Attachment #8781571 - Flags: review?(jlund) → review+
Comment on attachment 8781558 [details] [diff] [review]
bug1283730bb-2.patch

Jordan, yes I realize this. You can enable nightlies via setting enable_nightly to True but I couldn't find a way to enable nightlies but not on commit builds.  Do you have any suggestions on how to do this?
Flags: needinfo?(jlund)
(In reply to Kim Moir [:kmoir] from comment #22)
> Comment on attachment 8781558 [details] [diff] [review]
> bug1283730bb-2.patch
> 
> Jordan, yes I realize this. You can enable nightlies via setting
> enable_nightly to True but I couldn't find a way to enable nightlies but not
> on commit builds.  Do you have any suggestions on how to do this?

hm, how about something like:

        if 'android-x86' in platform:
            if branch['enable_nightly']:
                # keep the nightly but remove the CI equivalent
                branch['platforms'][platform]["enable_dep"] = False
            else:
                # remove all the android-x86 build jobs on this branch
                del branch['platforms'][platform]

context: https://dxr.mozilla.org/build-central/source/buildbotcustom/misc.py#750
Flags: needinfo?(jlund)
Attachment #8781558 - Attachment is obsolete: true
Attached file builder diff
Attachment #8782456 - Attachment is patch: false
Attached file tests builder diff
Comment on attachment 8782455 [details] [diff] [review]
bug1293730bb-3.patch

Thanks I didn't know about enable_dep
Attachment #8782455 - Flags: review?(jlund)
Comment on attachment 8782455 [details] [diff] [review]
bug1293730bb-3.patch

Review of attachment 8782455 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm :)
Attachment #8782455 - Flags: review?(jlund) → review+
Comment on attachment 8782455 [details] [diff] [review]
bug1293730bb-3.patch

landed and merged to production
https://hg.mozilla.org/mozilla-central/rev/8569eaf4b4e7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
looks like this broke release branches, where nighties are not enabled, but we still need android-x86
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8810320 [details]
Bug 1293730 - Do not remove nor change android-x86 platform properties on release branches.

https://reviewboard.mozilla.org/r/92688/#review92662
Attachment #8810320 - Flags: review?(jlund) → review+
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.