Closed Bug 1432219 Opened 7 years ago Closed 7 years ago

[tracking] migrate release checksums builder off of BBB to TC

Categories

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

Product:
Release Engineering
Component:
Release Automation: Other
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox60 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox60 --- fixed

People

(Reporter: mtabara, Assigned: mtabara)

References

Details

Attachments

(3 files, 1 obsolete file)

add TC checksums generation and kill the obsolete BB counterpart
35.19 KB, patch
mozilla
: review+
rail
: review+
Details | Diff | Splinter Review
Refactored version of the patch.
35.49 KB, patch
bhearsum
: checked-in+
Details | Diff | Splinter Review
Bug 1432219 - part 3: Fix bad beetmover worker type in production r=bhearsum a=release
2.68 KB, patch
bhearsum
: review+
jlorenzo
: checked-in+
Details | Diff | Splinter Review
Roght now we run generate_checksums[1] under BBB. We want to run this in TC, as one of the following actions: a) scriptworker b) docker-worker + tcsecrets for readonly AWS S3 account [1]: https://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/generate-checksums.py#l202
Summary: [tracking] migrate release checksums builder off of BBB to TC → [meta] migrate release checksums builder off of BBB to TC
Summary: [meta] migrate release checksums builder off of BBB to TC → [tracking] migrate release checksums builder off of BBB to TC
* tested locally and the script works as expected * added the in-tree stuff for the task Pushed https://hg.mozilla.org/projects/maple/rev/233ea27e332b Will follow-up with a staging release to see how the task in the graph behaves.
Some beetmoverscript related changes lie within https://github.com/mozilla-releng/beetmoverscript/pull/101/files
Remember to back https://hg.mozilla.org/projects/maple/rev/474bd3ac18ff out when work in bug 1424482 is done. At least the "Devedition" instead of "Firefox" in the release props.
(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #15) > Remember to back https://hg.mozilla.org/projects/maple/rev/474bd3ac18ff out > when work in bug 1424482 is done. At least the "Devedition" instead of > "Firefox" in the release props. Backed-it-out, will patch beetmoverscript instead. If we don't have `build_platform` set correctly, the `release-generate-checksum` family of task are getting filtered out of the in-tree generation.
Grafted and squeezed the changes on top of inbound for final review. Things work as expected across all platforms on maple. Am thinking about landing to central and let it ride the trains.
Attachment #8952677 - Flags: review?(rail)
Attachment #8952677 - Flags: review?(aki)
Comment on attachment 8952677 [details] [diff] [review] add TC checksums generation and kill the obsolete BB counterpart Grrr, some errors in the graft! ditching this, will follow-up
Attachment #8952677 - Attachment is obsolete: true
Attachment #8952677 - Flags: review?(rail)
Attachment #8952677 - Flags: review?(aki)
Grafted and squeezed the changes on top of inbound for final review. Things work as expected across all platforms on maple. Am thinking about landing to central and let it ride the trains.
Attachment #8952691 - Flags: review?(rail)
Attachment #8952691 - Flags: review?(aki)
Bug 1424482 currently blocks landing this on central because we don't have the craft_release_properties method.
Depends on: 1424482
Comment on attachment 8952691 [details] [diff] [review] add TC checksums generation and kill the obsolete BB counterpart Review of attachment 8952691 [details] [diff] [review]: ----------------------------------------------------------------- LGTM with some nits: ::: taskcluster/ci/release-generate-checksums-signing/kind.yml @@ +8,5 @@ > + - release-generate-checksums > + > +transforms: > + - taskgraph.transforms.release_generate_checksums_signing:transforms > + - taskgraph.transforms.release_notifications:transforms do you need to add release_deps to these? ::: taskcluster/taskgraph/transforms/release_generate_checksums_beetmover.py @@ +37,5 @@ > + > +transforms = TransformSequence() > + > +# shortcut for a string where task references are allowed > +taskref_or_string = Any( I see this boilerplate all over the place, but this variable doesn't seem to be used anywhere. Do we really need it? @@ +72,5 @@ > + yield job > + > + > +@transforms.add > +def make_task_description(config, jobs): This function looks so familiar. We should probably revisit these at some point.
Attachment #8952691 - Flags: review?(rail) → review+
Comment on attachment 8952691 [details] [diff] [review] add TC checksums generation and kill the obsolete BB counterpart Awesome! Thank you! >diff --git a/taskcluster/ci/release-generate-checksums/kind.yml b/taskcluster/ci/release-generate-checksums/kind.yml >--- a/taskcluster/ci/release-generate-checksums/kind.yml >+++ b/taskcluster/ci/release-generate-checksums/kind.yml > run: >- using: buildbot >- release-promotion: true >+ using: mozharness >+ actions: [create-virtualenv collect-individual-checksums create-big-checksums create-summary] >+ options: >+ - "version={version}" >+ - "build-number={build_number}" >+ script: "mozharness/scripts/release/generate-checksums.py" >+ index: >+ type: release >+ routes: >+ - index.releases.v1.{branch}.latest.{product}.latest.checksums >+ - index.releases.v1.{branch}.{revision}.{product}.{underscore_version}.build{build_number}.checksums I don't think we need index or routes anymore with the new notification scheme. > jobs: > firefox: >- name: release-firefox_chcksms > shipping-product: firefox >+ attributes: >+ build_platform: linux64 >+ build_type: opt >+ index: >+ product: firefox Here too. > fennec: >- name: release-fennec_chcksms > shipping-product: fennec >+ attributes: >+ build_platform: android-nightly >+ build_type: opt >+ index: >+ product: fennec here too. > devedition: >- name: devedition_release_chcksms > shipping-product: devedition >+ attributes: >+ build_platform: linux64-devedition >+ build_type: opt >+ index: >+ product: devedition Here too. >diff --git a/taskcluster/taskgraph/transforms/release_generate_checksums_beetmover.py b/taskcluster/taskgraph/transforms/release_generate_checksums_beetmover.py <snip> >+taskref_or_string = Any( >+ basestring, >+ {Required('task-reference'): basestring}) +1 to Rail's comment. We use `taskref_or_string` in schema definitions; I don't see it in use here. >+@transforms.add >+def make_task_worker(config, jobs): >+ for job in jobs: >+ valid_beetmover_job = (len(job["dependencies"]) == 2 and >+ any(['signing' in j for j in job['dependencies']])) >+ if not valid_beetmover_job: >+ raise NotImplementedError("Beetmover must have two dependencies.") >+ >+ build_task = None >+ signing_task = None >+ for dependency in job["dependencies"].keys(): >+ if 'signing' in dependency: >+ signing_task = dependency >+ else: >+ build_task = dependency >+ >+ signing_task_ref = "<" + str(signing_task) + ">" >+ build_task_ref = "<" + str(build_task) + ">" using .format() is probably preferable, e.g. signing_task_ref = "<{}>".format(str(signing_task)) >diff --git a/taskcluster/taskgraph/transforms/release_generate_checksums_signing.py b/taskcluster/taskgraph/transforms/release_generate_checksums_signing.py <snip> >+taskref_or_string = Any( >+ basestring, >+ {Required('task-reference'): basestring}) Same here. >+@transforms.add >+def make_release_generate_checksums_signing_description(config, jobs): >+ for job in jobs: >+ dep_job = job['dependent-task'] >+ attributes = copy_attributes_from_dependent_job(dep_job) >+ >+ treeherder = job.get('treeherder', {}) >+ treeherder.setdefault('symbol', 'SGenChcks') >+ 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) >+ treeherder.setdefault('kind', 'build') +1 to Rail's comment about common code. These treeherder things are common enough that we could potentially write and import a helper function that does them. >diff --git a/testing/mozharness/configs/releases/checksums_devedition.py b/testing/mozharness/configs/releases/checksums_devedition.py >new file mode 100644 >--- /dev/null >+++ b/testing/mozharness/configs/releases/checksums_devedition.py >@@ -0,0 +1,5 @@ >+# lint_ignore=E501 >+config = { >+ "stage_product": "devedition", >+ "bucket_name": "net-mozaws-prod-delivery-archive", >+} These are so small that we could potentially add them to the kind.yml, but these work too.
Attachment #8952691 - Flags: review?(aki) → review+
Adding myself a NI to prevent deleting the old mozharness piece because it's being used by TB
(In reply to Rail Aliiev [:rail] ⌚️ET from comment #23) > ::: taskcluster/ci/release-generate-checksums-signing/kind.yml > @@ +8,5 @@ > > + - release-generate-checksums > > + > > +transforms: > > + - taskgraph.transforms.release_generate_checksums_signing:transforms > > + - taskgraph.transforms.release_notifications:transforms > > do you need to add release_deps to these? Nope, I think I only need to amend the main task with that, not the following signing + beetmover tasks. It's the same in release-source. > ::: taskcluster/taskgraph/transforms/release_generate_checksums_beetmover.py > @@ +37,5 @@ > > + > > +transforms = TransformSequence() > > + > > +# shortcut for a string where task references are allowed > > +taskref_or_string = Any( > > I see this boilerplate all over the place, but this variable doesn't seem to > be used anywhere. Do we really need it? You're right, slippery copy-paste, I removed it from both {beetmover,signing} corresponding jobs. > @@ +72,5 @@ > > + yield job > > + > > + > > +@transforms.add > > +def make_task_description(config, jobs): > > This function looks so familiar. We should probably revisit these at some > point. It is. It's used in all beetmover transforms. But since we haven't done it too generic from the beginning, over time it became hard to amend it, hance I rewrote it in the beetmover jobs for the release-generate-checksums. It's not ideal, but afaik, we want to address the beetmover transforms optimizations anyway soon in the cleanup March month. So I'll leave it for then to improve in the future.
(In reply to Aki Sasaki [:aki] from comment #24) > >diff --git a/taskcluster/ci/release-generate-checksums/kind.yml b/taskcluster/ci/release-generate-checksums/kind.yml > >--- a/taskcluster/ci/release-generate-checksums/kind.yml > >+++ b/taskcluster/ci/release-generate-checksums/kind.yml > > run: > >- using: buildbot > >- release-promotion: true > >+ using: mozharness > >+ actions: [create-virtualenv collect-individual-checksums create-big-checksums create-summary] > >+ options: > >+ - "version={version}" > >+ - "build-number={build_number}" > >+ script: "mozharness/scripts/release/generate-checksums.py" > >+ index: > >+ type: release > >+ routes: > >+ - index.releases.v1.{branch}.latest.{product}.latest.checksums > >+ - index.releases.v1.{branch}.{revision}.{product}.{underscore_version}.build{build_number}.checksums > > I don't think we need index or routes anymore with the new notification > scheme. > > > jobs: > > firefox: > >- name: release-firefox_chcksms > > shipping-product: firefox > >+ attributes: > >+ build_platform: linux64 > >+ build_type: opt > >+ index: > >+ product: firefox > > Here too. > > > fennec: > >- name: release-fennec_chcksms > > shipping-product: fennec > >+ attributes: > >+ build_platform: android-nightly > >+ build_type: opt > >+ index: > >+ product: fennec > > here too. > > > devedition: > >- name: devedition_release_chcksms > > shipping-product: devedition > >+ attributes: > >+ build_platform: linux64-devedition > >+ build_type: opt > >+ index: > >+ product: devedition > > Here too. I think there must have been some bitrotten, I don't see those in my patches. I think when I rebased against inbound it was already up-to-date. I'll double check before I land to inbound. > >diff --git a/taskcluster/taskgraph/transforms/release_generate_checksums_beetmover.py b/taskcluster/taskgraph/transforms/release_generate_checksums_beetmover.py > <snip> > >+taskref_or_string = Any( > >+ basestring, > >+ {Required('task-reference'): basestring}) > > +1 to Rail's comment. We use `taskref_or_string` in schema definitions; I > don't see it in use here. Done. > >+@transforms.add > >+def make_task_worker(config, jobs): > >+ for job in jobs: > >+ valid_beetmover_job = (len(job["dependencies"]) == 2 and > >+ any(['signing' in j for j in job['dependencies']])) > >+ if not valid_beetmover_job: > >+ raise NotImplementedError("Beetmover must have two dependencies.") > >+ > >+ build_task = None > >+ signing_task = None > >+ for dependency in job["dependencies"].keys(): > >+ if 'signing' in dependency: > >+ signing_task = dependency > >+ else: > >+ build_task = dependency > >+ > >+ signing_task_ref = "<" + str(signing_task) + ">" > >+ build_task_ref = "<" + str(build_task) + ">" > > using .format() is probably preferable, e.g. > > signing_task_ref = "<{}>".format(str(signing_task)) Done. Thank you.
Refactored on top of inbound + addressing aki,rail's comments from 8952691: add TC checksums generation and kill the obsolete BB counterpart. This is ready to land on mozilla-inbound as soon as trees open.
Attachment #8954756 - Attachment is patch: true
Pushed by bhearsum@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/07922a7fba83 migrate release checksums builder off of BBB to TC. r=aki, rail
Attachment #8955489 - Flags: review?(bhearsum) → review+
Comment on attachment 8955489 [details] [diff] [review] Bug 1432219 - part 3: Fix bad beetmover worker type in production r=bhearsum a=release Landed on beta at: https://hg.mozilla.org/releases/mozilla-beta/rev/dbe9214d56355abb026839a145a0580068f75e33
Attachment #8955489 - Flags: checked-in+
Pushed by jlorenzo@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d29a3444f2bd part 3: Fix bad beetmover worker type in production r=bhearsum
Pushed by mtabara@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d131285f10c6 fix release generate checksums signing scopes in-tree. r=me a=release CLOSED TREE
See Also: → 1446816
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: