Closed Bug 1442793 Opened 6 years ago Closed 6 years ago

support private artifacts for partner repacks

Categories

(Release Engineering :: Release Automation: Other, enhancement)

enhancement
Not set
normal

Tracking

(firefox61 fixed)

RESOLVED FIXED
Tracking Status
firefox61 --- fixed

People

(Reporter: mozilla, Assigned: mozilla)

References

Details

Attachments

(21 files)

14.51 KB, patch
Details | Diff | Splinter Review
55 bytes, text/x-github-pull-request
jlorenzo
: review+
Details | Review
678 bytes, text/plain
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
59 bytes, text/x-review-board-request
bhearsum
: review+
Details
      No description provided.
Blocks: 1398803
Hopefully we don't have to do all of this. But I do think we should make public/build a task attribute default, and allow for overriding.
Thanks Johan! I think this will become scriptworker 10.0.0.
Attachment #8955731 - Flags: review?(jlorenzo)
Attachment #8955731 - Flags: review?(jlorenzo) → review+
See Also: → 1230134
Rolled out with https://hg.mozilla.org/build/puppet/rev/b9dd0bf8f210 . Next: docker-worker private artifact downloads, which may involve taskcluster-proxy.
Fixed the graph-generation exceptions. Still need to make sure the generated graph is correct, and that setting a non-public artifact_prefix works.
(In reply to Aki Sasaki [:aki] from comment #17)
> Fixed the graph-generation exceptions. Still need to make sure the generated
> graph is correct, and that setting a non-public artifact_prefix works.

And finish writing the patch set.
Patch set is written to add the artifact_prefix attribute in the graph, and now the graph doesn't have a ton of other unexpected side effects... the only thing changed is toolchains, and not in their artifact_prefix.

I now need to test to make sure that a) this is passed down appropriately when we set a non-public/build artifact_prefix, and b) whether we need any build system changes.
The task definitions for the try push look good: aki-test is propagated in the build, build-signing, and xpcshell. Let's see how it goes.
setup-stub broke for some reason [!]. Also, the tests now need the queue:get-artifact:aki-test/* scope.
Neither of these are blockers for partners, I don't think - we can keep the build publishing to public/build, and change the partner repack task to publish to releng/partners/.... Landed on maple; I'll see if partner private artifacts work there.
https://hg.mozilla.org/projects/maple/rev/3da127c845b5694d8ccbe394d9feb3185de19d89 to allow repackage-partner to download private artifacts.
(In reply to Aki Sasaki [:aki] from comment #79)
> https://hg.mozilla.org/projects/maple/rev/
> ed81fa0029fa5a027b3d8caac1d19f204a31c225 to add taskcluster-proxy support to
> generic-worker task transforms. The docs say it's a thing:
> https://docs.taskcluster.net/reference/workers/generic-worker/docs/
> features#feature-taskclusterproxy

Pete - is taskcluster-proxy rolled out to generic workers? getting `[taskcluster:error] Could not start taskcluster proxy: exec: "taskcluster-proxy": executable file not found in %PATH%`
https://tools.taskcluster.net/groups/a2x2127ZQt6tZYgbRkr2sA/tasks/HyLv2HeIT6-cunfK46duPg/details
Flags: needinfo?(pmoore)
Depends on: 1449981
(In reply to Aki Sasaki [:aki] from comment #80)
> (In reply to Aki Sasaki [:aki] from comment #79)
> > https://hg.mozilla.org/projects/maple/rev/
> > ed81fa0029fa5a027b3d8caac1d19f204a31c225 to add taskcluster-proxy support to
> > generic-worker task transforms. The docs say it's a thing:
> > https://docs.taskcluster.net/reference/workers/generic-worker/docs/
> > features#feature-taskclusterproxy
> 
> Pete - is taskcluster-proxy rolled out to generic workers? getting
> `[taskcluster:error] Could not start taskcluster proxy: exec:
> "taskcluster-proxy": executable file not found in %PATH%`
> https://tools.taskcluster.net/groups/a2x2127ZQt6tZYgbRkr2sA/tasks/HyLv2HeIT6-
> cunfK46duPg/details

Indeed taskcluster-proxy will need to be installed on gecko-3-b-win2012. I've updated bug 1449981 with details.
Flags: needinfo?(pmoore)
Ah wait - if you just need to download private artifacts and mount them into the task directory, you can do that without taskcluster-proxy.

You just need to use the mounts feature of generic-worker. It will take care of downloading a private artifact for you (if the task has the required scopes) and will mount it into a path of your choosing inside the task directory.

See the "mounts" property of the task payload.
If it is an archive (zip, tar.gz, etc) you have the choice whether to mount it as a file (and unpack it yourself) or to mount it as a read only directory (i.e. let generic-worker do the extraction for you). Both ways should work.
Of course you can use taskcluster-proxy if you want to, e.g. if you already have scripts to do so that you use with docker-worker - but if this is greenfield development, I'd recommend mounting the artifact using the mounts feature, as it is more explicit than downloading at runtime.
Comment on attachment 8958297 [details]
bug 1442793 - add artifact_prefix attribute.

https://reviewboard.mozilla.org/r/227228/#review242570
Attachment #8958297 - Flags: review?(bhearsum) → review+
Comment on attachment 8958298 [details]
bug 1442793 - get_artifact_prefix and get_artifact_path.

https://reviewboard.mozilla.org/r/227230/#review242572
Attachment #8958298 - Flags: review?(bhearsum) → review+
Comment on attachment 8958299 [details]
bug 1442793 - repackage_signing artifact_prefix.

https://reviewboard.mozilla.org/r/227232/#review242574

::: taskcluster/taskgraph/transforms/repackage_signing.py:100
(Diff revision 5)
>  
>          upstream_artifacts = [{
>              "taskId": {"task-reference": "<repackage>"},
>              "taskType": "repackage",
>              "paths": [
> -                "public/build/{}target.complete.mar".format(locale_str),
> +                get_artifact_path(dep_job, "{}target.complete.mar".format(locale_str)),

This shouldn't block anything, but I see changes like this to practically every transform that deals with artifacts. It makes me wonder if this should be handled in taskgraph/transforms/task.py instead - what do you think?
Attachment #8958299 - Flags: review?(bhearsum) → review+
Comment on attachment 8958300 [details]
bug 1442793 - tests artifact_prefix.

https://reviewboard.mozilla.org/r/227234/#review242576
Attachment #8958300 - Flags: review?(bhearsum) → review+
Comment on attachment 8958301 [details]
bug 1442793 - diffoscope artifact_prefix.

https://reviewboard.mozilla.org/r/227236/#review242578
Attachment #8958301 - Flags: review?(bhearsum) → review+
Attachment #8958302 - Flags: review?(bhearsum) → review+
Comment on attachment 8958303 [details]
bug 1442793 - generated sources artifact_prefix.

https://reviewboard.mozilla.org/r/227240/#review242602
Attachment #8958303 - Flags: review?(bhearsum) → review+
Attachment #8958304 - Flags: review?(bhearsum) → review+
Comment on attachment 8958305 [details]
bug 1442793 - beetmover artifact_prefix.

https://reviewboard.mozilla.org/r/227244/#review242608
Attachment #8958305 - Flags: review?(bhearsum) → review+
Comment on attachment 8958306 [details]
bug 1442793 - mozharness test artifact_prefix.

https://reviewboard.mozilla.org/r/227246/#review242610
Attachment #8958306 - Flags: review?(bhearsum) → review+
Attachment #8961097 - Flags: review?(bhearsum) → review+
Comment on attachment 8961097 [details]
bug 1442793 - release_generate_checksums_beetmover artifact_prefix

https://reviewboard.mozilla.org/r/229912/#review242612
Comment on attachment 8961098 [details]
bug 1442793 - release_generate_checksums_signing artifact_prefix

https://reviewboard.mozilla.org/r/229914/#review242616
Attachment #8961098 - Flags: review?(bhearsum) → review+
Attachment #8961099 - Flags: review?(bhearsum) → review+
Comment on attachment 8961100 [details]
bug 1442793 - repackage artifact_prefix.

https://reviewboard.mozilla.org/r/229918/#review242620
Attachment #8961100 - Flags: review?(bhearsum) → review+
Comment on attachment 8961223 [details]
bug 1442793 - partials signing artifact_prefix.

https://reviewboard.mozilla.org/r/230000/#review242622
Attachment #8961223 - Flags: review?(bhearsum) → review+
Comment on attachment 8961224 [details]
bug 1442793 - partials artifact_prefix.

https://reviewboard.mozilla.org/r/230002/#review242624
Attachment #8961224 - Flags: review?(bhearsum) → review+
Attachment #8961225 - Flags: review?(bhearsum) → review+
Comment on attachment 8960852 [details]
bug 1442793 - nightly-l10n artifact_prefix.

https://reviewboard.mozilla.org/r/229612/#review242628
Attachment #8960852 - Flags: review?(bhearsum) → review+
I think all of these patches look good. I was grepping around maple and found a few places that I thought might also need similar changes:
taskgraph/transforms/upload_generated_sources.py
taskgraph/transforms/release_sign_and_push_langpacks.py
taskgraph/transforms/repackage_partner.py
taskgraph/transforms/release_snap_push.py
taskgraph/transforms/job/debian_package.py
taskgraph/transforms/job/toolchain.py
taskgraph/transforms/job/spidermonkey.py
taskgraph/transforms/source_test.py
taskgraph/transforms/release_beetmover_signed_addons.py

All still contain 'public/build' references. Quite a few of them are only on maple code, so changes might belong in a different patch or bug.
(In reply to Ben Hearsum (:bhearsum) from comment #122)
> I think all of these patches look good. I was grepping around maple and
> found a few places that I thought might also need similar changes:
> taskgraph/transforms/upload_generated_sources.py
> taskgraph/transforms/release_sign_and_push_langpacks.py
> taskgraph/transforms/repackage_partner.py
> taskgraph/transforms/release_snap_push.py
> taskgraph/transforms/job/debian_package.py
> taskgraph/transforms/job/toolchain.py
> taskgraph/transforms/job/spidermonkey.py
> taskgraph/transforms/source_test.py
> taskgraph/transforms/release_beetmover_signed_addons.py
> 
> All still contain 'public/build' references. Quite a few of them are only on
> maple code, so changes might belong in a different patch or bug.

Thanks! Yeah, I'm leaving some of those alone because they're

- maple only (e.g. partners hasn't landed on central yet), or a combination of
- difficult to change and
- unlikely to require private artifacts.

I knew going in that this patchset would lay the groundwork for private artifacts, but would only be supported for real in the partner repack critical path.
Comment on attachment 8958299 [details]
bug 1442793 - repackage_signing artifact_prefix.

https://reviewboard.mozilla.org/r/227232/#review242574

> This shouldn't block anything, but I see changes like this to practically every transform that deals with artifacts. It makes me wonder if this should be handled in taskgraph/transforms/task.py instead - what do you think?

That probably makes sense, if and when artifact_prefix is supported through the entire graph. I left things like toolchains alone because I didn't see a straightforward way to pass the prefix down to the task.
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9f8c088dce9f
add artifact_prefix attribute. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/0ffeb06f9541
get_artifact_prefix and get_artifact_path. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/d8d0e4665d0b
repackage_signing artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/4055c1ca987b
tests artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/0b45c230926f
diffoscope artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/e7821c5c236a
job public_artifacts -> artifacts. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/25ea434bd55e
generated sources artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/298f38a58ec7
signed_artifacts artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/1a93b230ff0d
beetmover artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/b1ce919d3f34
mozharness test artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/b0d243813a06
release_generate_checksums_beetmover artifact_prefix r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/d1c361a5ddc7
release_generate_checksums_signing artifact_prefix r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/58b09a0406d5
generate_taskcluster_artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/d552d6d9ac7f
repackage artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/dac61b3288af
partials signing artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/fcf2f5931ff8
partials artifact_prefix. r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/bd8ad86b57d5
pushapk artifact_prefix r=bhearsum
https://hg.mozilla.org/integration/autoland/rev/55ca7aeeeccf
nightly-l10n artifact_prefix. r=bhearsum
You need to log in before you can comment on or make changes to this bug.