1213011 - Add a route for simulator post-build task

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Alexandre Poirot [:ochameau]]

That, to easily refer to lastest nighly build of it.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: patch v1

I matched the route name of mulet task.
I wish I could use a variable to describe the platform,
like {{platform}} or something alike that would be linux32,linux64,macos,win32,...
As this post-build task, may, in future be applied to more than just linux64 mulet build.

Also, is there anything I can do locally to test such change?
Like ./mach graph or run some test suite?

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Use stable file name for simulator xpi in TC.

I imagine I also need that to offer a link to "latest" xpi via index.taskcluster.net.
Before that patch, the filename contains the build date and so is hard to predict.
With this patch the filename will be always the same.

Greg, Is that fine, if from now on, I ask review to :jryans for the build-simulator.sh
who help me maintaining this simulator?

Comment 3

[Greg Arndt [:garndt]]

Comment on attachment 8671551 [details] [diff] [review]
patch v1

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

So, routes could be whatever you want them to be, but I think the routes that are within mulet are old style routes....they have been since replaced by those found in tasks/build.yml .

You can run the `mach taskcluster-graph` command (need to pass in some arguments to it) and that will spit out some JSON that you can verify.  Or you could just push to try and have it kick off and see how it looks for you in the index browser.  I would recommend starting off with index routes with like "index.public.garbage.ochameu..." so you don't pollute any other namespaces.

If this is the route you actually want, then r+

Comment 4

[Greg Arndt [:garndt]]

Comment on attachment 8671556 [details] [diff] [review]
Use stable file name for simulator xpi in TC.

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

Definitely nice to have a artifact name that remains consistent rather than one with version numbers.

Comment 5

[J. Ryan Stinnett [:jryans]]

Comment on attachment 8671551 [details] [diff] [review]
patch v1

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

::: testing/taskcluster/tasks/post-builds/mulet_simulator.yml
@@ +17,5 @@
>    provisionerId: aws-provisioner-v1
>    schedulerId: task-graph-scheduler
>  
> +  routes:
> +    - 'index.buildbot.branches.{{project}}.linux64-simulator'

Why do we want this route when we're not using buildbot here?

The Mulet builds are under index.gecko.v2.*, shouldn't this be under the same tree?

Comment 6

[Alexandre Poirot [:ochameau]]

index.gecko.v2.* is handled automagically and doesn't support post-build tasks from what I understood.
Whereas route specified here are manual thing, this is kind of workaround.
TBH I don't really care about the name as soon as we can have a link to latest build.
I just copy pasted the route of mulet build, which as you highlight looks wrong by itself.
I think taskcluster team is aware of this and just use v2 and automatic routes instead.

Comment 7

[Greg Arndt [:garndt]]

For now it seems that at least conforming to the v1 format would be good.

https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/tasks/build.yml#21

Comment 8

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7586c3fd4f48

Comment 9

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d61a7a954c06

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: patch v2

Now uses `index.gecko.v1.{{project}}.latest.simulator.{{build_type}}` as route.
But I need a tweak to mach_command.py to expose build_type to post-build yml files!

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Allow post-build tasks to use build_{name,type,product} variables

This is the additional patch I would need for this route, using build_type.
Today, we are exposing build_{name,type,product} only to `routes` field.
That is misleading as you could easily thing it is also exposed in `command`
or other YML fields!
With this patch, these variables are now exposed to the whole YML,
as well as the post-build task.
But I see some differences in the routes with this patch,
that I can't explain.
For example, without this patch we get this route:
  queue:route:index.gecko.v1.mozilla-central.latest.linux.linux32.opt
whereas we get this with the new patch:
  queue:route:index.gecko.v1.mozilla-central.latest.linux.linux.opt

Comment 12

[Alexandre Poirot [:ochameau]]

With all these patches, I can see the latest task easily via url like this:
  https://tools.taskcluster.net/index/#gecko.v1.try.latest.simulator/gecko.v1.try.latest.simulator.opt
And even get the xpi like this \o/
  https://index.taskcluster.net/v1/task/gecko.v1.try.latest.simulator.opt/artifacts/public/build/fxos-simulator-2.5.20151012130140-linux64.xpi
(I forgot to push the build-simulator.sh patch on try to get a unique xpi name)

Comment 13

[Greg Arndt [:garndt]]

Comment on attachment 8672607 [details] [diff] [review]
Allow post-build tasks to use build_{name,type,product} variables

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

Definitely need to troubleshoot why the routes changed before and after patch.  We can't have any mismatch in the routes that were being generated as these are heavily relied upon to be stable.

r- for now until we can verify what is causing the routes to be changed.  Routes before and after should be compared and resolved, and also probably should just link to a try push running everything just to make sure nothing is obviously broken.

::: testing/taskcluster/mach_commands.py
@@ +114,5 @@
>      :param dict task: task definition.
>      :param json_routes: the list of routes to use from routes.json
>      :param parameters: dictionary of parameters to use in route templates
>      """
>      fmt = parameters.copy()

Do we need to copy the parameters anymore if we are not updating them?

Comment 14

[Alexandre Poirot [:ochameau]]

Attachment: Return variables defined from template inheritance v1

So I figured out why the routes were different.
I think there was something wrong with decorate_task_json_routes.
It used to pull build_name/build_type from `build` (itself comes from job_flags.yml).
From the jobs flags 32bits linux is labelled "linux" (see testing/taskcluster/tasks/branches/base_jobs.yml),
whereas the build_name used in yml files is "linux32" (see testing/taskcluster/tasks/builds/linux32_clobber.yml).

I think we should have only one source of truth and use the variables set in build ymls.
(And may be ensure that job flags yml uses the same names?)

In order to do that, we have to somehow pull these build_name/build_type attributes
out of build ymls. This patch allows to do that.
The only side effect I see is that build_name/build_type/build_product attributes now appear
in tasckcluster-graph. On build objects that now have taskId, task, build_name, build_product and build_type attributes.

Comment 15

[Alexandre Poirot [:ochameau]]

Attachment: Allow post-build tasks to use build_{name,type,product} variables v2

With the previous patch, this one now doesn't introduce any different behavior in tasckcluster-graph,
except for post-build tasks, that now have access to all these variables.

Comment 16

[Greg Arndt [:garndt]]

so with these two patches I am unable to generate a graph like I would have done before (and like how the decision task does it).

Also, the tests for test_templates.py are now failing.

$ mach taskcluster-graph --project try  --message "try: -b do -p all -u all" --head-repository http://hg.mozilla.org/integration/b2g-inbound --head-rev tip --owner [e-mail]
Error running mach:

    ['taskcluster-graph', '--project', 'try', '--message', 'try: -b do -p all -u all', '--head-repository', 'http://hg.mozilla.org/integration/b2g-inbound', '--head-rev', 'tip', '--owner', '[e-mail]']

The error occurred in the implementation of the invoked mach command.

This should never occur and is likely a bug in the implementation of that
command. Consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

KeyError: 'build_name'

  File "/Users/mozilla/work/test_gecko/testing/taskcluster/mach_commands.py", line 371, in create_graph
    build_parameters['build_name'] = build_task['build_name']


--------------------------------------------

$ mach python-test tests/test_templates.py
 0:00.37 /Users/mozilla/work/test_gecko/obj-x86_64-apple-darwin13.4.0/_virtualenv/bin/python ./tests/test_templates.py
 0:00.61 TEST-PASS | ./tests/test_templates.py | test_deep_inheritance
 0:00.62 TEST-UNEXPECTED-FAIL | ./tests/test_templates.py | line 44, test_inheritance: {'content': 'content', 'variable': 'inherit', 'woot': 'inherit'} != {'content': 'content', 'variable': 'inherit'}
 0:00.62 - {'content': 'content', 'variable': 'inherit', 'woot': 'inherit'}
 0:00.62 ?                                    -------------------
 0:00.62
 0:00.62 + {'content': 'content', 'variable': 'inherit'}
 0:00.62 TEST-PASS | ./tests/test_templates.py | test_inheritance_circular
 0:00.63 TEST-UNEXPECTED-FAIL | ./tests/test_templates.py | line 55, test_inheritance_implicat_pass: {'a': 'a', 'c': 'c', 'b': 'b', 'values': ['overriden', 'b', 'c']} != {'values': ['overriden', 'b', 'c']}
 0:00.63 - {'a': 'a', 'b': 'b', 'c': 'c', 'values': ['overriden', 'b', 'c']}
 0:00.63 + {'values': ['overriden', 'b', 'c']}
 0:00.63 TEST-PASS | ./tests/test_templates.py | test_inheritance_with_simple_extensions
 0:00.63 TEST-PASS | ./tests/test_templates.py | test_invalid_path
 0:00.63 TEST-PASS | ./tests/test_templates.py | test_no_templates
 0:00.63 TEST-PASS | ./tests/test_templates.py | test_with_templates

Comment 17

[Greg Arndt [:garndt]]

Comment on attachment 8673106 [details] [diff] [review]
Allow post-build tasks to use build_{name,type,product} variables v2

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

seems that a graph can't be generated now and tests fail

Comment 18

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a5dccbf2f50

Comment 19

[Alexandre Poirot [:ochameau]]

New patch, still good regarding just the simulators:
  https://tools.taskcluster.net/task-graph-inspector/#NgVnjyusSnGshHOopkVrlQ/W-UL5_bLRimtrCkGIiNg6g/0

Is there any specific try run I should spawn or do I need to do all/all ?!

Comment 20

[Alexandre Poirot [:ochameau]]

Attachment: Return variables defined from template inheritance v2

I haven't been able to run testing/taskcluster/tests/test_templates.py.
mach python-test fails with python exception, not being able to find TC modules...

I don't know if that's the right behavior...
One alternative could be to set build_name/build_type from the inherited variables
in build.yml and build_phone.yml.

Comment 21

[Alexandre Poirot [:ochameau]]

Attachment: Allow post-build tasks to use build_{name,type,product} variables v3

Dolphin builds ends up with undefined build_name/build_type,
so the routes set by phone_build.yml don't work.
Isn't that an issue?

In any case I made it so that we would accept a build stack without these attributes.

Comment 22

[Greg Arndt [:garndt]]

I was looking over this and it seems that now we have things in the graph we didn't have before and are not valid properties for the graph... build_name, build_product, and build_type now show up in the final graph when these are just variables used for rendering the templates.

Comment 23

[Greg Arndt [:garndt]]

> Dolphin builds ends up with undefined build_name/build_type,
> so the routes set by phone_build.yml don't work.
> Isn't that an issue?

That does seem like an issue, perhaps :wcosta knows more.  

Wander, it appears in the graph as:
"index.gecko.v1.try.revision.linux.tip.."

Comment 24

[Wander Lairson Costa [:wcosta]]

(In reply to Greg Arndt [:garndt] from comment #23)
> > Dolphin builds ends up with undefined build_name/build_type,
> > so the routes set by phone_build.yml don't work.
> > Isn't that an issue?
> 
> That does seem like an issue, perhaps :wcosta knows more.  
> 
> Wander, it appears in the graph as:
> "index.gecko.v1.try.revision.linux.tip.."

dolphin tasks don't setup these variables, because of that they are undefined. This is problem a mistake made by the original task author that passed by the reviewer's eyes.

Comment 25

[Alexandre Poirot [:ochameau]]

https://hg.mozilla.org/integration/fx-team/rev/b842ef4420f1b247ab9706ce8d5028c8bca52ecd
Bug 1213011 - Use stable file name for simulator xpi in TC. r=garndt

Comment 26

[Alexandre Poirot [:ochameau]]

Attachment: Allow post-build tasks to use build_{name,type,product} variables v4

Ok, then here is another approach, already suggested in comment 20.
Instead of hacking into template behavior, 
I define build_type/build_name like build_product, from build.yml and phone_build.yml.
Then I can easily fetch that from mach_commands.py.

I keep throwing patches at you, but it could be more efficient
if you provide more feedback and sometimes highlights what
I should or could be implementing instead of only highlighting what artifacts
cause my patches.

Comment 27

[Carsten Book [:Tomcat]]

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

Comment 28

[Greg Arndt [:garndt]]

(In reply to Alexandre Poirot [:ochameau] from comment #26)
> Created attachment 8679003 [details] [diff] [review]
> Allow post-build tasks to use build_{name,type,product} variables v4
> 
> Ok, then here is another approach, already suggested in comment 20.
> Instead of hacking into template behavior, 
> I define build_type/build_name like build_product, from build.yml and
> phone_build.yml.
> Then I can easily fetch that from mach_commands.py.
> 
> I keep throwing patches at you, but it could be more efficient
> if you provide more feedback and sometimes highlights what
> I should or could be implementing instead of only highlighting what artifacts
> cause my patches.

That's definitely fair feedback, and I certainly didn't mean to cause an issue so I apologize.  I will be taking a look at this patch in the morning.

Comment 29

[Greg Arndt [:garndt]]

Comment on attachment 8679003 [details] [diff] [review]
Allow post-build tasks to use build_{name,type,product} variables v4

Reviewed the before and after output and verified only the route for simulator was changed (based on the previous patch that added the route to it).  Route now contains the 'opt' build_type

Comment 30

[Alexandre Poirot [:ochameau]]

https://hg.mozilla.org/integration/fx-team/rev/778030b1b88b7857d296eff8d63850c701c35139
Bug 1213011 - Allow post-build tasks to use build_{name,type,product} variables. r=garndt

https://hg.mozilla.org/integration/fx-team/rev/d41522b5aed48941893f47d57c54e9cab6e5b5d1
Bug 1213011 - Add a route to post-build simulator task to easily download it. r=garndt

Comment 31

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/778030b1b88b
https://hg.mozilla.org/mozilla-central/rev/d41522b5aed4