1467555 - Make Android single-locale nightlies tier-2

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, enhancement)

Description

[Axel Hecht [:Pike]]

I've checked with Delphine, and we think that single-locale Android nightlies should be running and getting fixes, but they don't warrant back-outs if broken.

Thus I recommend that we classify them as Tier-2, unless that isn't what I think it is ;-)

Comment 1

[Justin Wood (:Callek)]

This is easy, but what value does this added complexity earn us if we're only shipping on nightly and not shipping via any app stores? Vs removing then entirely

Comment 2

[Axel Hecht [:Pike]]

We still want to have the opportunity to test new locales before adding them to the multilocale builds.

That's basically an all-or-nothing step, and in particular on mobile, being able to test on-device is important.

Clarifying question, does making this build tier-2 make things more complex, or is the complexity about android having both single and multi-locale builds?

Comment 3

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

(In reply to Axel Hecht [:Pike] from comment #2)
> We still want to have the opportunity to test new locales before adding them
> to the multilocale builds.

We can change the list of locales for try builds, so we can still test new locales before shipping them as part of Nightly.

> That's basically an all-or-nothing step, and in particular on mobile, being
> able to test on-device is important.
> 
> Clarifying question, does making this build tier-2 make things more complex,
> or is the complexity about android having both single and multi-locale
> builds?

From the build system perspective, it's really the similar-but-subtly-different single- and multi-locale builds.  I'd like to see only one thing -- a build -- where we ship

{'default': 'en-US', 'ui-locales': ['en-US, 'de', 'es-ES', ...]}

and then, if we care to, we also build lots of

{'default': 'de', 'ui-locales': ['de']}

single-locale builds.  The key point is there's only one build process and the sets of locales change.

Comment 4

[Jordan Lund (:jlund)]

@ciduty - this should be an easy patch. Some context is needed though.

To recap here for lack of better place at the moment,

CI, Nightlies, and Releases are all taskcluster graphs generated from `mach taskgraph`.

Official taskgraph docs: https://firefox-source-docs.mozilla.org/taskcluster/docs/taskcluster/index.html
A helpful blog post: http://code.v.igoro.us/posts/2018/05/design-of-task-graph-generation.html

All graphs are initiated by Decision tasks (bootstrap taskcluster tasks that submit groups of more taskcluster tasks).

For this bug, we are modifying the Nightly taskgraph. That decision task for nightlies is initiated twice a day off of mozilla-central by the cron and hook feature within Taskcluster (time-based). To contrast, CI is initiated when commits are pushed to mozilla-central (per-checkin).

Saying that, both CI and Nightly graphs are *generated and submitted* similarly. That is, they both invoke `mach taskgraph`. Just with different options and parameters.

Most recent Android Nightly: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=199a085199815cc99daa658956a7c9436e1d436b&filter-searchStr=nightly

Example single-locale task we want to convert to tier 2 (from tier 1):
  * task in Treeherder web ui view: https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=199a085199815cc99daa658956a7c9436e1d436b&filter-searchStr=nightly&selectedJob=182282553
 * task in Taskcluster web ui view: https://tools.taskcluster.net/groups/RB_L1FPZSVuwIPcX7AEeHQ/tasks/NpV-zY_ORDSMNc4c5nFyHw/details

How that nightly graph was generated and submitted (decision task): https://tools.taskcluster.net/groups/RB_L1FPZSVuwIPcX7AEeHQ/tasks/RB_L1FPZSVuwIPcX7AEeHQ/runs/0/artifacts
   * taskgraph cmd: "['bash', '-cx', 'cd /builds/worker/checkouts/gecko && ln -s /builds/worker/artifacts artifacts && ./mach --log-no-times taskgraph decision --pushlog-id=\'-1\' --pushdate=\'0\' --project=\'mozilla-central\' --message="$GECKO_COMMIT_MSG" --owner=\'[e-mail]\' --level=\'3\' --base-repository="$GECKO_BASE_REPOSITORY" --head-repository="$GECKO_HEAD_REPOSITORY" --head-ref="$GECKO_HEAD_REF" --head-rev="$GECKO_HEAD_REV" --target-tasks-method=nightly_fennec\n']
   * full log: https://taskcluster-artifacts.net/RB_L1FPZSVuwIPcX7AEeHQ/0/public/logs/live_backing.log
   * parameters file used by above taskgraph cmd: https://queue.taskcluster.net/v1/task/RB_L1FPZSVuwIPcX7AEeHQ/runs/0/artifacts/public/parameters.yml
   * full graph in json format: https://taskcluster-artifacts.net/RB_L1FPZSVuwIPcX7AEeHQ/0/public/task-graph.json
   * full graph in web UI format: https://tools.taskcluster.net/groups/RB_L1FPZSVuwIPcX7AEeHQ


code in question we need to modify:
   * taskgraph root dir: https://dxr.mozilla.org/mozilla-central/source/taskcluster
   * where the single-locale nightly tasks get their tier: https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/l10n/kind.yml#88
 

Now, android single locale tasks, afaict and I'm not familiar with the code, seems to be bundled with desktop platforms. Since we only want to make *android* single locale tasks tier 2, we will need to condition off of that:
   * https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/l10n/kind.yml#96


How to patch:

You will need to run `mach taskgraph` locally, using similar options and parameters.yml as described above from production. To help debug, you can run different taskgraph subcommands and it will generated different output formats: https://firefox-source-docs.mozilla.org/taskcluster/docs/taskcluster/mach.html

callek can help with how to run locally and then for review.

@ciduty - who is up to the challenge?

Comment 5

[Roland Mutter Michael (:rmutter)]

I would like to go for it. I'll pm :callek for the next steps.

Comment 6

[Justin Wood (:Callek)]

(In reply to Nick Alexander :nalexander from comment #3)
> (In reply to Axel Hecht [:Pike] from comment #2)
> > We still want to have the opportunity to test new locales before adding them
> > to the multilocale builds.
> 
> We can change the list of locales for try builds, so we can still test new
> locales before shipping them as part of Nightly.

In particular yes we can change the list of locales in try and create builds off them. I'm also open to creating special once-a-week or once-in-a-while nightlies with locales we want to provide for testing before being willing to incorporate it in an official shipping product.

> 
> > Clarifying question, does making this build tier-2 make things more complex,
> > or is the complexity about android having both single and multi-locale
> > builds?
> 
> From the build system perspective, it's really the
> similar-but-subtly-different single- and multi-locale builds.  I'd like to
> see only one thing -- a build -- where we ship
> at least 
> {'default': 'en-US', 'ui-locales': ['en-US, 'de', 'es-ES', ...]}
> 
> and then, if we care to, we also build lots of
> 
> {'default': 'de', 'ui-locales': ['de']}
> 
> single-locale builds.  The key point is there's only one build process and
> the sets of locales change.

I'll expand on this. Switching them to tier 2 shouldn't really be an issue code complexity as it stands.

Being able to remove the code complexity around single-locale android builds however could be a great win. Would enable us to move faster on multi-locale changes in product(s) at least on mobile.

I'd be open to ~*today* doing an english build + non-multi-locale-locales as a multi-locale build if it means we satisfy all of your and Delphine's concerns here. And specifically if we can reduce the cadence on these locales we build for testing reasons then that could additionally be a win for CI load and costs.

If we however cannot get rid of 'single locale' as it exists today, at least in the ways we can work around those concerns/needs I'd like to keep them happening on nightly and most checkins to ensure code changes don't break them.

We can discuss this briefly this week, and spin out a new bug since this one can serve strictly as the "make them tier 2" which is a relatively simple change.

Comment 7

[Justin Wood (:Callek)]

Thanks Jordan for laying out the basics... I have a few slight corrections but nothing too onerous.

(In reply to Jordan Lund (:jlund) from comment #4)
> For this bug, we are modifying the Nightly taskgraph. That decision task for
> nightlies is initiated twice a day off of mozilla-central by the cron and
> hook feature within Taskcluster (time-based). To contrast, CI is initiated
> when commits are pushed to mozilla-central (per-checkin).

We'll also be modifying the on-change l10n for android I just created, because if Nightly is tier 2 here, so should the same job in normal CI.

> Saying that, both CI and Nightly graphs are *generated and submitted*
> similarly. That is, they both invoke `mach taskgraph`. Just with different
> options and parameters.

> code in question we need to modify:
>    * taskgraph root dir:
> https://dxr.mozilla.org/mozilla-central/source/taskcluster
>    * where the single-locale nightly tasks get their tier:
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/l10n/kind.
> yml#88

This file is specifically for the on-push l10n jobs.
The one for nightlies is at https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/nightly-l10n/kind.yml#107

You'll need to modify both.

> Now, android single locale tasks, afaict and I'm not familiar with the code,
> seems to be bundled with desktop platforms. Since we only want to make
> *android* single locale tasks tier 2, we will need to condition off of that:
>    *
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/l10n/kind.
> yml#96

This is correct in both cases.

> How to patch:
> 
> You will need to run `mach taskgraph` locally, using similar options and
> parameters.yml as described above from production. To help debug, you can
> run different taskgraph subcommands and it will generated different output
> formats:
> https://firefox-source-docs.mozilla.org/taskcluster/docs/taskcluster/mach.
> html

The patch will be basically adding by-build-platform as used elsewhere in the file, android.*: will be tier 2, and default: will be tier 1.

When running the taskgraph commands you may get errors about unrelated task kinds and their tiers, if that happens we can work it out together in this bug or IRC, but should be relatively easy patches to fix. [you can't have a tier 1 task depend on a tier 2 or our assertions break]

> @ciduty - who is up to the challenge?

Thanks Roland!

Comment 8

[Roland Mutter Michael (:rmutter)]

:Callek 

+++ b/taskcluster/ci/nightly-l10n/kind.yml
@@ -104,7 +104,7 @@ job-template:
          win.*: aws-provisioner-v1/gecko-{level}-b-win2012
    treeherder:
       symbol: L10n(N)
-      tier: 1
+      tier: 2
       platform:
          by-build-platform:
             linux64-nightly: linux64/opt
diff --git a/taskcluster/taskgraph/transforms/repackage_signing.py b/taskcluster/taskgraph/transforms/repackage_signing.py
--- a/taskcluster/taskgraph/transforms/repackage_signing.py
+++ b/taskcluster/taskgraph/transforms/repackage_signing.py
@@ -61,7 +61,8 @@ def make_repackage_signing_description(c
             'treeherder', {}).get('machine', {}).get('platform', '')
         treeherder.setdefault('platform',
                               "{}/opt".format(dep_th_platform))
-        treeherder.setdefault('tier', 1)
+
+        treeherder.setdefault('tier', dep_job.task.get('extra', {}).get('treeherder', {}).get('tier', 1))
         treeherder.setdefault('kind', 'build')

Comment 9

[Justin Wood (:Callek)]

(In reply to Roland Mutter Michael (:rmutter) from comment #8)
> :Callek 
> 
> +++ b/taskcluster/ci/nightly-l10n/kind.yml
> @@ -104,7 +104,7 @@ job-template:
>           win.*: aws-provisioner-v1/gecko-{level}-b-win2012
>     treeherder:
>        symbol: L10n(N)
> -      tier: 1
> +      tier: 2

This should use the by-build-platform syntax from below it, specifically marking android as tier 1 and `default: 1` for everything else. `We should also do it in taskcluster/ci/l10n/kind.yml`

> diff --git a/taskcluster/taskgraph/transforms/repackage_signing.py
> b/taskcluster/taskgraph/transforms/repackage_signing.py
> --- a/taskcluster/taskgraph/transforms/repackage_signing.py
> +++ b/taskcluster/taskgraph/transforms/repackage_signing.py
> @@ -61,7 +61,8 @@ def make_repackage_signing_description(c
>              'treeherder', {}).get('machine', {}).get('platform', '')
>          treeherder.setdefault('platform',
>                                "{}/opt".format(dep_th_platform))
> -        treeherder.setdefault('tier', 1)
> +
> +        treeherder.setdefault('tier', dep_job.task.get('extra',
> {}).get('treeherder', {}).get('tier', 1))
>          treeherder.setdefault('kind', 'build')

You'll also have to make this sort of change elsewhere, depending on where ends up failing as you go. For example with your patch (and my fix above) I'm still hitting:
`Exception: beetmover-l10n-an-android-api-16-nightly/opt (tier 1) cannot depend on nightly-l10n-android-api-16-nightly-1/opt (tier 2)`

Which would be https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/beetmover.py#344

Note after patching that I still get the error about beetmover, so I used pythons pdb [1] and did:

> /home/callek/mozilla/hg/mozilla-central/taskcluster/taskgraph/transforms/beetmover.py(346)make_task_description()
-> treeherder.setdefault(
(Pdb) treeherder
{u'platform': u'android-4-2-x86/opt', u'symbol': u'BM-S'}
(Pdb) dep_job.label
u'build-signing-android-x86-nightly/opt'
(Pdb) 

Which then points me at signing which is then https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/signing.py#123

I'd like to see how far you can take it from here though...

[1] - Python's pdb is a debugger and it lets you drop into the debugger at specific points in executing code. To achieve this I did

diff --git a/taskcluster/taskgraph/transforms/beetmover.py b/taskcluster/taskgraph/transforms/beetmover.py
--- a/taskcluster/taskgraph/transforms/beetmover.py
+++ b/taskcluster/taskgraph/transforms/beetmover.py
@@ -336,17 +336,22 @@ def make_task_description(config, jobs):
         attributes = dep_job.attributes
 
         treeherder = job.get('treeherder', {})
         treeherder.setdefault('symbol', 'BM-S')
         dep_th_platform = dep_job.task.get('extra', {}).get(
             'treeherder', {}).get('machine', {}).get('platform', '')
         treeherder.setdefault('platform',
                               "{}/opt".format(dep_th_platform))
-        treeherder.setdefault('tier', 1)
+        if 'android' in dep_th_platform:
+            import pdb;pdb.set_trace()
+        treeherder.setdefault(
+            'tier',
+            dep_job.task.get('extra', {}).get('treeherder', {}).get('tier', 1)
+        )

Comment 10

[Jordan Lund (:jlund)]

hey rmutter, how's this going? Did you get a chance to look at this last shift?

Comment 11

[Roland Mutter Michael (:rmutter)]

Hey :jlund, I've been in PTO for 2 shifts before the last shift. Last shift I didn't had a chance to look at it. Looking into it right now . Will update the bug after this one.

Comment 12

[Roland Mutter Michael (:rmutter)]

Have been talking with Callek and we'll have a meeting next week around this issue.

Comment 13

[Roland Mutter Michael (:rmutter)]

Attachment: Patch for making Android single-locale nightlies tier-2

That took a while but finally we had the chance to meet and make the required modifications. Thanks :callek once again for everything.

Comment 14

[Justin Wood (:Callek)]

Comment on attachment 8993387 [details] [diff] [review]
Patch for making Android single-locale nightlies tier-2

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

This is pretty good, thanks again!

Just a few small fixes to make, once re-attached I'll land for you.

::: taskcluster/ci/l10n/kind.yml
@@ +97,4 @@
>              win32: windows2012-32/opt
>              win64: windows2012-64/opt
>              android-api-16: android-4-0-armv7-api16/opt
> +

We don't need this blank line...

::: taskcluster/taskgraph/transforms/repackage_signing.py
@@ +61,4 @@
>              'treeherder', {}).get('machine', {}).get('platform', '')
>          treeherder.setdefault('platform',
>                                "{}/opt".format(dep_th_platform))
> +        #treeherder.setdefault('tier', 1)

Please remove this commented out line.

@@ +61,5 @@
>              'treeherder', {}).get('machine', {}).get('platform', '')
>          treeherder.setdefault('platform',
>                                "{}/opt".format(dep_th_platform))
> +        #treeherder.setdefault('tier', 1)
> +        treeherder.setdefault('tier', dep_job.task.get('extra', {}).get('treeherder', {}).get('tier', 1))

We're getting "Line Too Long" here, lets reformat to 
```
treeherder.setdefault(
    'tier',
    dep_job.task.get('extra', {}).get('treeherder', {}).get('tier', 1)
    )
```

(That is same indentation level for the treeherder.setdefault line, and then four spaces in more for each following line.)

Comment 15

[Roland Mutter Michael (:rmutter)]

Attachment: Patch for making Android single-locale nightlies tier-2

There we go!

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/025c35ad797f
Make Android single-locale nightlies tier-2. r=Callek

Comment 17

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/025c35ad797f