Closed
Bug 1355482
Opened 8 years ago
Closed 6 years ago
document taskcluster releng implementation and release promotion
Categories
(Release Engineering :: General, enhancement, P2)
Release Engineering
General
Tracking
(firefox-esr60 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | fixed |
People
(Reporter: kmoir, Assigned: mozilla)
References
Details
Attachments
(7 files, 3 obsolete files)
(deleted),
patch
|
bhearsum
:
review+
dustin
:
feedback+
|
Details | Diff | Splinter Review |
(deleted),
text/x-phabricator-request
|
bhearsum
:
review+
|
Details |
(deleted),
patch
|
mozilla
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
(deleted),
text/x-phabricator-request
|
bhearsum
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
bhearsum
:
review+
|
Details |
(deleted),
patch
|
catlee
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/x-review-board-request
|
mozilla
:
review+
|
Details |
Each time I've met with the buildduty folks for the past weeks they have expressed that the lack of documentation on the current state of the tascluster migration and release promotion is very frustrating for them. They can't find documentation to gain a better understanding of these systems in transition work.
We need to create some docs for them so they can understand how the tasks and jobs are transformed, the kind dependencies, and any other knowledge they need so they can address get up to speed to address operational issues when they arise.
Topics include
beetmover
balrog
beetmover
l10n
signing
repackaging
transforms
kinds
release promotion
various steps in the release process, how we go from a ci build to a final release
In addition to documentation, it would be great if various releng folks could record tech talks on their area of expertise in this area. We could use these to onboard new employees/interns.
Comment 1•7 years ago
|
||
Kim, who do you think is best suited to do this? I'm thinking some combination of you, Mihai, Callek and Rail.
Flags: needinfo?(kmoir)
Priority: -- → P2
Reporter | ||
Comment 2•7 years ago
|
||
Sure, I think if if we each tackle a couple of topics this would be great. Will bring it up at the next tcmigration meeting. As well not sure where it should reside in releng docs or wiki, but pointers to content from the builduty wiki in any case would be a good start.
Flags: needinfo?(kmoir)
Assignee | ||
Comment 3•7 years ago
|
||
https://trello.com/c/yJ8lOHfA/205-expand-taskcluster-relpro-in-tree-docs specifies adding release promotion documentation in-tree, so it'll show up in the gecko rtd. We could use that for general docs and have supplemental buildduty docs in a wiki, or include buildduty docs in the same in-tree location.
Assignee | ||
Updated•7 years ago
|
Summary: document taskcluster releng implementation and release promotion for operational teams → document taskcluster releng implementation and release promotion
Assignee | ||
Comment 4•7 years ago
|
||
I'm starting some in-tree sphinx docs for tc relpro; hopefully this serves as a good starting point to add more docs.
To generate the gecko Sphinx docs locally,
./mach doc --outdir docs-out
and then you can browse to file:///.../docs-out/html/main/index.html
I had to install jsdoc before this would work... for mac, `brew install jsdoc3` . The error message from mach suggests using npm.
`./mach doc` failed initially because I had some old trees cloned inside of `testing/mozharness/build/mozilla-{beta,central}` (probably for merge day testing); evidently it scours the entire tree recursively for files to build. To fix, I moved the temp workdir `testing/mozharness/build` outside of my tree.
Keywords: leave-open
Assignee | ||
Comment 5•7 years ago
|
||
Using a branch off gecko-dev is probably even easier, since the rst is easily viewable as html on github.
Assignee | ||
Comment 6•7 years ago
|
||
First pass. This lays down structure that we can add to (e.g. https://trello.com/c/yJ8lOHfA/205-expand-taskcluster-relpro-in-tree-docs).
Attachment #8960792 -
Flags: review?(bhearsum)
Attachment #8960792 -
Flags: feedback?(dustin)
Assignee | ||
Comment 7•7 years ago
|
||
Added some more ascii art + clarifications.
Attachment #8960792 -
Attachment is obsolete: true
Attachment #8960792 -
Flags: review?(bhearsum)
Attachment #8960792 -
Flags: feedback?(dustin)
Attachment #8961170 -
Flags: review?(bhearsum)
Attachment #8961170 -
Flags: feedback?(dustin)
Comment 8•7 years ago
|
||
Comment on attachment 8961170 [details] [diff] [review]
initial relpro in-tree documentation
Review of attachment 8961170 [details] [diff] [review]:
-----------------------------------------------------------------
This looks good, I really like the structure. There's a few notes below, mostly requests for expanding things a bit.
::: taskcluster/docs/release-promotion-action.rst
@@ +2,5 @@
> +========================
> +
> +The `release promotion action`_ allows us to chain multiple graphs together.
> +Essentially, we're using :ref:`optimization` logic to replace task labels in the
> +current graph with task IDs from the previous graph(s).
I know we in RelEng use the word "graph" a lot, but I wonder if we should start trying to switch to "group" (since that's the Taskcluster vernacular these days).
@@ +57,5 @@
> +| l10n-repack
> +| |
> +| l10n-signing
> +
> +(This is the ``snowman`` model: If you request the body of the snowman and
I think it would be good to make the snowman model explanation a bit more prominent (eg: not behind paranthesis, maybe also above the examples?) -- this metaphor seems to be working well when explaining it to new people.
@@ +72,5 @@
> +
> +Release promotion action mechanics
> +----------------------------------
> +
> +The action downloads the ``parameters.yml`` from the initial ``previous_graph_id``.
Can you add something that describes previous_graph_id (and any other action inputs) in more detail? When I read this paragraph I feel like I'm supposed to know something about it already - in particular, what the difference between an "initial" previous graph id and other ones is.
@@ +99,5 @@
> +------------------------------------------------------
> +
> +Currently, we're able to trigger this action via `Treeherder`_; we sometimes use this method for testing purposes. This requires being signed in with the right scopes. On `Release Promotion Projects`_, here's a dropdown in the top right of a given revision. Choose ``Custom Push Action``, then ``Release Promotion``. The inputs are specifiable as raw yaml on the left hand column.
> +
> +Triggering the release promotion action via releaserunner3
Since we have multiple ways to trigger these actions, it would be helpful to explain why someone would choose one over the other. (As things read now, I don't know why I would want to use releaserunner3 or trigger_action.py when I could just click some buttons on Treeherder.)
::: taskcluster/docs/release-promotion.rst
@@ +1,4 @@
> +Release Promotion
> +=================
> +
> +Release promotion allows us to ship the same compiled binaries that we've
This is an excellent intro/history lesson.
Attachment #8961170 -
Flags: review?(bhearsum) → review+
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c72d3c87aeda
add release promotion docs. r=bhearsum
Assignee | ||
Comment 10•7 years ago
|
||
Awesome. Now we have todos for the following additional docs:
* best practices in taskgraph kinds/transforms
* explain our kinds for people not in releng
* beetmover/balrog/partners/partials/signing/pushapk in depth
All up for grabs!
Assignee | ||
Comment 11•7 years ago
|
||
Hm, I thought I responded or had a draft of responses, but that tab seems to be missing.
(In reply to Ben Hearsum (:bhearsum) from comment #8)
> Comment on attachment 8961170 [details] [diff] [review]
> initial relpro in-tree documentation
>
> Review of attachment 8961170 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> This looks good, I really like the structure. There's a few notes below,
> mostly requests for expanding things a bit.
Thanks! I addressed your review comments before landing. We can land followups if/as needed.
> ::: taskcluster/docs/release-promotion-action.rst
> @@ +2,5 @@
> > +========================
> > +
> > +The `release promotion action`_ allows us to chain multiple graphs together.
> > +Essentially, we're using :ref:`optimization` logic to replace task labels in the
> > +current graph with task IDs from the previous graph(s).
>
> I know we in RelEng use the word "graph" a lot, but I wonder if we should
> start trying to switch to "group" (since that's the Taskcluster vernacular
> these days).
Done. We do have vestiges still, e.g. `previous_graph_ids` and `taskcluster/taskgraph`
> @@ +57,5 @@
> > +| l10n-repack
> > +| |
> > +| l10n-signing
> > +
> > +(This is the ``snowman`` model: If you request the body of the snowman and
>
> I think it would be good to make the snowman model explanation a bit more
> prominent (eg: not behind paranthesis, maybe also above the examples?) --
> this metaphor seems to be working well when explaining it to new people.
Done. Glad to hear the analogy is helpful.
> @@ +72,5 @@
> > +
> > +Release promotion action mechanics
> > +----------------------------------
> > +
> > +The action downloads the ``parameters.yml`` from the initial ``previous_graph_id``.
>
> Can you add something that describes previous_graph_id (and any other action
> inputs) in more detail? When I read this paragraph I feel like I'm supposed
> to know something about it already - in particular, what the difference
> between an "initial" previous graph id and other ones is.
Hm, I might have missed this comment in my followups. I can attach another patch.
> @@ +99,5 @@
> > +------------------------------------------------------
> > +
> > +Currently, we're able to trigger this action via `Treeherder`_; we sometimes use this method for testing purposes. This requires being signed in with the right scopes. On `Release Promotion Projects`_, here's a dropdown in the top right of a given revision. Choose ``Custom Push Action``, then ``Release Promotion``. The inputs are specifiable as raw yaml on the left hand column.
> > +
> > +Triggering the release promotion action via releaserunner3
>
> Since we have multiple ways to trigger these actions, it would be helpful to
> explain why someone would choose one over the other. (As things read now, I
> don't know why I would want to use releaserunner3 or trigger_action.py when
> I could just click some buttons on Treeherder.)
Done.
> ::: taskcluster/docs/release-promotion.rst
> @@ +1,4 @@
> > +Release Promotion
> > +=================
> > +
> > +Release promotion allows us to ship the same compiled binaries that we've
>
> This is an excellent intro/history lesson.
Thanks!
Comment 12•7 years ago
|
||
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dead3908b2cc
release promotion doc review followup. r=bhearsum
Comment 13•7 years ago
|
||
bugherder |
Comment 14•7 years ago
|
||
Comment on attachment 8961170 [details] [diff] [review]
initial relpro in-tree documentation
Review of attachment 8961170 [details] [diff] [review]:
-----------------------------------------------------------------
::: taskcluster/docs/release-promotion-action.rst
@@ +1,4 @@
> +Release Promotion Action
> +========================
> +
> +The `release promotion action`_ allows us to chain multiple graphs together.
The content of this section seems almost entirely focused on the mechanics of optimizing these promotion taskgraphs. Maybe this would make a good subsection? There's a need here for more general information: what does the release promotion action do? Who runs it? Is there a different action for each phase of release promotion? What are the parameters? Much of that is answered below, so perhaps this just means re-ordering the content in this file.
@@ +2,5 @@
> +========================
> +
> +The `release promotion action`_ allows us to chain multiple graphs together.
> +Essentially, we're using :ref:`optimization` logic to replace task labels in the
> +current graph with task IDs from the previous graph(s).
Within the in-tree configuration, taskgraph is the right term.
@@ +10,5 @@
> + G
> + |
> + t1
> + |
> + t2
This looks like G (a graph) depends on t1 -- is G meant to denote the decision task here? Or is that intended as a label?
G
t1
|
t2
@@ +27,5 @@
> + or
> +
> + G1 G2 G
> + | | |
> + t1 t0 |
Why is this t0 here?
@@ +97,5 @@
> +
> +Triggering the release promotion action via Treeherder
> +------------------------------------------------------
> +
> +Currently, we're able to trigger this action via `Treeherder`_; we sometimes use this method for testing purposes. This requires being signed in with the right scopes. On `Release Promotion Projects`_, here's a dropdown in the top right of a given revision. Choose ``Custom Push Action``, then ``Release Promotion``. The inputs are specifiable as raw yaml on the left hand column.
*there's
@@ +104,5 @@
> +----------------------------------------------------------
> +
> +`Releaserunner3`_ is our current method of triggering the release promotion action in production. Examples of how to run this are in the `releasewarrior docs`_.
> +
> +To deal with the above ``previous_graph_ids`` logic, we allow for a ``decision_task_id`` in `trigger_action.py`_. As of 2018-03-14, this script assumes we want to download ``parameters.yml`` from the same decision task that we get ``actions.json`` from.
Is there a need to include the date here? Are older versions of releaserunner3 still used?
::: taskcluster/docs/release-promotion.rst
@@ +6,5 @@
> +
> +In the olden days, we used to re-compile our release builds with separate
> +configs, which led to release-specific bugs which weren't caught by continuous
> +integration tests. This also meant we required new builds at release time, which
> + also increased the end-to-end time for a given release significantly. Release
too many "also"
leading ' ' in this line
Attachment #8961170 -
Flags: feedback?(dustin) → feedback+
Comment 15•7 years ago
|
||
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/af32770a782b
relpro docs: address dustin's review comments. r=dustin
Assignee | ||
Comment 16•7 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] pronoun: he from comment #14)
> Comment on attachment 8961170 [details] [diff] [review]
> initial relpro in-tree documentation
Thanks!
> ::: taskcluster/docs/release-promotion-action.rst
> @@ +1,4 @@
> > +Release Promotion Action
> > +========================
> > +
> > +The `release promotion action`_ allows us to chain multiple graphs together.
>
> The content of this section seems almost entirely focused on the mechanics
> of optimizing these promotion taskgraphs. Maybe this would make a good
> subsection? There's a need here for more general information: what does the
> release promotion action do? Who runs it? Is there a different action for
> each phase of release promotion? What are the parameters? Much of that is
> answered below, so perhaps this just means re-ordering the content in this
> file.
I put some overview in the other page, but it makes sense to put an
action-specific overview here. Done.
> @@ +2,5 @@
> > +========================
> > +
> > +The `release promotion action`_ allows us to chain multiple graphs together.
> > +Essentially, we're using :ref:`optimization` logic to replace task labels in the
> > +current graph with task IDs from the previous graph(s).
>
> Within the in-tree configuration, taskgraph is the right term.
Ok. After Catlee's comment, I replaced "graph" with "task group" in most places, noting that "graph" was an alternate term. I think I'll go back over and rename to "taskgraph" and note that "task group" and "graph" are alternate terms.
> @@ +10,5 @@
> > + G
> > + |
> > + t1
> > + |
> > + t2
>
> This looks like G (a graph) depends on t1 -- is G meant to denote the
> decision task here? Or is that intended as a label?
It's both the decision/action task as well as the graph label. From the above line:
`Let's call our new graph ``G``::`
I'm certain we can make this better, but I wasn't sure how best to do that.
> @@ +27,5 @@
> > + or
> > +
> > + G1 G2 G
> > + | | |
> > + t1 t0 |
>
> Why is this t0 here?
To show that tasks outside of graph G aren't used in the optimization.
I could have filled in a few more screens full of different `previous_graph_ids`
examples; the ones I have were the minimum I thought could represent multiple
`previous_graph_ids` behavior.
> @@ +97,5 @@
> > +
> > +Triggering the release promotion action via Treeherder
> > +------------------------------------------------------
> > +
> > +Currently, we're able to trigger this action via `Treeherder`_; we sometimes use this method for testing purposes. This requires being signed in with the right scopes. On `Release Promotion Projects`_, here's a dropdown in the top right of a given revision. Choose ``Custom Push Action``, then ``Release Promotion``. The inputs are specifiable as raw yaml on the left hand column.
>
> *there's
Fixed.
> @@ +104,5 @@
> > +----------------------------------------------------------
> > +
> > +`Releaserunner3`_ is our current method of triggering the release promotion action in production. Examples of how to run this are in the `releasewarrior docs`_.
> > +
> > +To deal with the above ``previous_graph_ids`` logic, we allow for a ``decision_task_id`` in `trigger_action.py`_. As of 2018-03-14, this script assumes we want to download ``parameters.yml`` from the same decision task that we get ``actions.json`` from.
>
> Is there a need to include the date here? Are older versions of
> releaserunner3 still used?
Future-proofing against rr3 changes that aren't reflected in the in-tree docs, since rr3 lives out-of-tree with no current plans to move it. Ideally we replace it all with ship-it v2, that removes the need to run `trigger_action.py` manually at all, and we can revamp this section.
> ::: taskcluster/docs/release-promotion.rst
> @@ +6,5 @@
> > +
> > +In the olden days, we used to re-compile our release builds with separate
> > +configs, which led to release-specific bugs which weren't caught by continuous
> > +integration tests. This also meant we required new builds at release time, which
> > + also increased the end-to-end time for a given release significantly. Release
>
> too many "also"
> leading ' ' in this line
Fixed.
https://hg.mozilla.org/integration/mozilla-inbound/rev/af32770a782bc9a12fa4aa2e8b04379d84627e05
Comment 17•7 years ago
|
||
bugherder |
Comment 18•7 years ago
|
||
I think this covers enough? I couldn't think of anything else to add, or other places that would benefit from Balrog mentions.
I also removed some dead kinds (balrog-l10n, release-secondary-balrog-publishing).
Attachment #8966321 -
Flags: review?(aki)
Assignee | ||
Comment 19•7 years ago
|
||
Comment on attachment 8966321 [details] [diff] [review]
add balrog kind documentation
Much better! Do you think it's worth linking to the official balrog docs + the balrog and balrogscript repos from http://gecko-docs.mozilla.org.s3.amazonaws.com/taskcluster/taskcluster/release-promotion.html somewhere?
Attachment #8966321 -
Flags: review?(aki) → review+
Comment 20•7 years ago
|
||
https://firefox-source-docs.mozilla.org/taskcluster/taskcluster/release-promotion.html (I don't know if that S3 bucket is still updated..)
Comment 21•7 years ago
|
||
Comment 22•7 years ago
|
||
I added the links you suggested, and was inspired by Simon's patch to make a higher level section, too.
Attachment #8966321 -
Attachment is obsolete: true
Attachment #8966640 -
Flags: review?(aki)
Assignee | ||
Comment 23•7 years ago
|
||
Comment on attachment 8966640 [details] [diff] [review]
updated balrog docs
Thank you!
>diff --git a/taskcluster/docs/release-promotion.rst b/taskcluster/docs/release-promotion.rst
>index e7f5470ccb3d..214a639ba7ce 100644
>--- a/taskcluster/docs/release-promotion.rst
>+++ b/taskcluster/docs/release-promotion.rst
>@@ -41,8 +41,12 @@ limited rollout percentage or are dependent on multiple downstream signoffs to
> fully ship.
>
> In-depth relpro guide
> ---------------------
>
> .. toctree::
>
> release-promotion-action
>+
>+Release Promotion Tasks
>+-----------------------
>+* :doc:`balrog`
I was thinking we'd add these subpages to the toctree. I'm ok with it here if you think it works better.
Attachment #8966640 -
Flags: review?(aki) → review+
Comment 24•7 years ago
|
||
(In reply to Aki Sasaki [:aki] from comment #23)
> Comment on attachment 8966640 [details] [diff] [review]
> updated balrog docs
>
> Thank you!
>
> >diff --git a/taskcluster/docs/release-promotion.rst b/taskcluster/docs/release-promotion.rst
> >index e7f5470ccb3d..214a639ba7ce 100644
> >--- a/taskcluster/docs/release-promotion.rst
> >+++ b/taskcluster/docs/release-promotion.rst
> >@@ -41,8 +41,12 @@ limited rollout percentage or are dependent on multiple downstream signoffs to
> > fully ship.
> >
> > In-depth relpro guide
> > ---------------------
> >
> > .. toctree::
> >
> > release-promotion-action
> >+
> >+Release Promotion Tasks
> >+-----------------------
> >+* :doc:`balrog`
>
> I was thinking we'd add these subpages to the toctree. I'm ok with it here
> if you think it works better.
Fine with me, I was just following the trend that Simon set in his partials documentation.
Updated•7 years ago
|
Attachment #8966640 -
Flags: checked-in+
Comment 25•7 years ago
|
||
Pushed by bhearsum@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ff3f4b6ce41
document taskcluster releng implementation and release promotion - add in depth balrog docs. r=aki
Comment 26•7 years ago
|
||
bugherder |
Comment 27•7 years ago
|
||
bugherder |
Comment 28•7 years ago
|
||
Comment on attachment 8966535 [details]
Bug 1355482 Partials documentation r=bhearsum
Ben Hearsum (:bhearsum) has approved the revision.
https://phabricator.services.mozilla.com/D898
Attachment #8966535 -
Flags: review+
Comment 29•7 years ago
|
||
Pushed by sfraser@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1dfc2207a62f
Partials documentation r=bhearsum
Comment 30•7 years ago
|
||
bugherder |
Comment 31•7 years ago
|
||
(In reply to Pulsebot from comment #25)
> Pushed by bhearsum@mozilla.com:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/1ff3f4b6ce41
> document taskcluster releng implementation and release promotion - add in
> depth balrog docs. r=aki
I forgot to add the new file in this. I'm pushing that now.
Comment 32•7 years ago
|
||
Pushed by bhearsum@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a3030ca3e161
document taskcluster releng implementation and release promotion - add missing file. r=aki
Comment 33•7 years ago
|
||
bugherder |
Comment 34•7 years ago
|
||
A space was needed to make the code block show up
Comment 35•7 years ago
|
||
Comment on attachment 8968436 [details]
Bug 1355482 Fix partials documentation code block r=bhearsum
Ben Hearsum (:bhearsum) has approved the revision.
https://phabricator.services.mozilla.com/D957
Attachment #8968436 -
Flags: review+
Comment 36•7 years ago
|
||
Pushed by sfraser@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9d00dc13a563
Fix partials documentation code block r=bhearsum
Comment 37•7 years ago
|
||
bugherder |
Updated•6 years ago
|
Component: General → Documentation
Comment hidden (mozreview-request) |
Assignee | ||
Comment 39•6 years ago
|
||
Attachment #8980387 -
Flags: review?(catlee)
Comment 40•6 years ago
|
||
Comment on attachment 8980387 [details] [diff] [review]
signing-docs.patch
Review of attachment 8980387 [details] [diff] [review]:
-----------------------------------------------------------------
Looks great, thanks for writing this up!
Just a few minor nits.
::: taskcluster/docs/signing.rst
@@ +66,5 @@
> +
> +**Build internal signing**: Certain package types require the internals to be signed.
> +Certain formats extract the internal binaries and sign them, e.g. exe or dmg.
> +These kinds include `build-signing`, `nightly-l10n-signing`,
> +`release-eme-free-repack-signing`, and `release-partner-repack-signing`.
I don't really understand this description. Is this for e.g. signing xul.dll on Windows?
@@ +110,5 @@
> +is a ``tar.gz``.
> +
> +``signcode`` signing takes individual binaries or a zipfile. We sign the
> +individual file or internals of the zipfile, skipping any already-signed files
> +and a select few blocklisted files (using the `_should_sign_windows`_ function).
s/blocklisted/blacklisted/
@@ +119,5 @@
> +``mar`` signing signs our update files (Mozilla ARchive). ``mar_sha384`` is
> +the same, but with a different hashing algorithm.
> +
> +``emevoucher`` - Firefox releases support `Encrypted Media Extensions`_ by default
> +(EME-free builds are available, however). The voucher is part of that.
I don't think this is used any more. Probably don't need to include it.
Attachment #8980387 -
Flags: review?(catlee) → review+
Assignee | ||
Comment 41•6 years ago
|
||
(In reply to Chris AtLee [:catlee] from comment #40)
> Comment on attachment 8980387 [details] [diff] [review]
> signing-docs.patch
>
> Review of attachment 8980387 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Looks great, thanks for writing this up!
>
> Just a few minor nits.
>
> ::: taskcluster/docs/signing.rst
> @@ +66,5 @@
> > +
> > +**Build internal signing**: Certain package types require the internals to be signed.
> > +Certain formats extract the internal binaries and sign them, e.g. exe or dmg.
> > +These kinds include `build-signing`, `nightly-l10n-signing`,
> > +`release-eme-free-repack-signing`, and `release-partner-repack-signing`.
>
> I don't really understand this description. Is this for e.g. signing xul.dll
> on Windows?
Correct. I'll try to word this better.
> @@ +110,5 @@
> > +is a ``tar.gz``.
> > +
> > +``signcode`` signing takes individual binaries or a zipfile. We sign the
> > +individual file or internals of the zipfile, skipping any already-signed files
> > +and a select few blocklisted files (using the `_should_sign_windows`_ function).
>
> s/blocklisted/blacklisted/
I'm explicitly using `blocklisted` and `allowlisted` rather than `blacklisted` and `whitelisted`.
> @@ +119,5 @@
> > +``mar`` signing signs our update files (Mozilla ARchive). ``mar_sha384`` is
> > +the same, but with a different hashing algorithm.
> > +
> > +``emevoucher`` - Firefox releases support `Encrypted Media Extensions`_ by default
> > +(EME-free builds are available, however). The voucher is part of that.
>
> I don't think this is used any more. Probably don't need to include it.
Ok!
Comment 42•6 years ago
|
||
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffbf2064e3f0
add in-tree signing docs. r=catlee
Comment 43•6 years ago
|
||
bugherder |
Comment 44•6 years ago
|
||
mozreview-review |
Comment on attachment 8980360 [details]
Bug 1355482 - Add pushapk documentation
https://reviewboard.mozilla.org/r/246536/#review252836
Looks good overall! A few suggestions below:
::: taskcluster/docs/pushapk.rst:18
(Diff revision 1)
> +The google-play-strings downloads every string for every locale from the `l10n website`_. This happens at the very beginning of the promotion, in order to spot any potential issue (like infrastructure-related), early. This prevents holding the release at the last minute .
> +
> +Push it all to Google Play
> +--------------------------
> +
> +Before any request is made to Google Play, the push-apk task sanity check the data passed over:
Grammar fix: "sanity checks"
::: taskcluster/docs/pushapk.rst:25
(Diff revision 1)
> +1. APKs are signed with the correct certificates. This may not sound extremely important because Google Play is vigilant about APK signatures and will refuse any APK for which the signature is not valid. However, it is safer to bail out before any outbound traffic is done to Google Play. Besides, with this check, Google acts as a second factor instead of being the only actor accountable for signatures.
> +2. All given APKs have matching data (version numbers, package names, etc.)
> +3. No required processor architecture is missing, in order to upload them all in the same request. We have to publish them at the same time because some Android devices support several architectures. We have already had one big crash on these devices because an x86 APK was overseeded by its “brother in ARM”.
> +4. APKs are multilocales
> +
> +Then it uploads the APKs and the strings fetched in the previous task. Like said above, Google Play performs some additional checks on their end. These errors are reported back.
It might be helpful here to give a few examples about the types of checks that Google Play does (if we even know).
::: taskcluster/docs/pushapk.rst:29
(Diff revision 1)
> +
> +Then it uploads the APKs and the strings fetched in the previous task. Like said above, Google Play performs some additional checks on their end. These errors are reported back.
> +
> +These checks and actions are done in several components. Here's how they interact:
> +
> +.. image:: https://johanlorenzo.github.io/blog/images/posts/push-apk/architecture.svg
This graph is now part of the docs! You should check it in.
::: taskcluster/docs/pushapk.rst:29
(Diff revision 1)
> +
> +Then it uploads the APKs and the strings fetched in the previous task. Like said above, Google Play performs some additional checks on their end. These errors are reported back.
> +
> +These checks and actions are done in several components. Here's how they interact:
> +
> +.. image:: https://johanlorenzo.github.io/blog/images/posts/push-apk/architecture.svg
This graph looks good overall, I just have one question: What system is "stores_l10n" part of (I'm guessing that's the task that has previously downloaded and stored the l10n strings)? I think it could use an encompassing box, a more descriptive name, or both.
Comment 45•6 years ago
|
||
mozreview-review |
Comment on attachment 8980360 [details]
Bug 1355482 - Add pushapk documentation
https://reviewboard.mozilla.org/r/246536/#review253570
Attachment #8980360 -
Flags: review?(bhearsum) → review-
Comment hidden (mozreview-request) |
Comment 47•6 years ago
|
||
mozreview-review |
Comment on attachment 8980360 [details]
Bug 1355482 - Add pushapk documentation
https://reviewboard.mozilla.org/r/246536/#review254726
::: taskcluster/docs/pushapk.rst:25
(Diff revision 1)
> +1. APKs are signed with the correct certificates. This may not sound extremely important because Google Play is vigilant about APK signatures and will refuse any APK for which the signature is not valid. However, it is safer to bail out before any outbound traffic is done to Google Play. Besides, with this check, Google acts as a second factor instead of being the only actor accountable for signatures.
> +2. All given APKs have matching data (version numbers, package names, etc.)
> +3. No required processor architecture is missing, in order to upload them all in the same request. We have to publish them at the same time because some Android devices support several architectures. We have already had one big crash on these devices because an x86 APK was overseeded by its “brother in ARM”.
> +4. APKs are multilocales
> +
> +Then it uploads the APKs and the strings fetched in the previous task. Like said above, Google Play performs some additional checks on their end. These errors are reported back.
Good idea. I added 2 checks I remember we hit.
::: taskcluster/docs/pushapk.rst:29
(Diff revision 1)
> +
> +Then it uploads the APKs and the strings fetched in the previous task. Like said above, Google Play performs some additional checks on their end. These errors are reported back.
> +
> +These checks and actions are done in several components. Here's how they interact:
> +
> +.. image:: https://johanlorenzo.github.io/blog/images/posts/push-apk/architecture.svg
Good point. I added the graph in tree.
I changed the schema to be less confusing about "stores_l10n" That was the way we did, before strings are fetched in a separate task. What do you think?
Comment 48•6 years ago
|
||
mozreview-review |
Comment on attachment 8980360 [details]
Bug 1355482 - Add pushapk documentation
https://reviewboard.mozilla.org/r/246536/#review254750
Attachment #8980360 -
Flags: review?(bhearsum) → review+
Comment 49•6 years ago
|
||
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/739ed6830af9
Add pushapk documentation r=bhearsum
Comment 50•6 years ago
|
||
bugherder |
Blocks: 1468751
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 54•6 years ago
|
||
mozreview-review |
Comment on attachment 8988123 [details]
Bug 1355482 - documentation cleanups for release promotion,
https://reviewboard.mozilla.org/r/253372/#review259992
Code analysis found 4 defects in this patch:
- 4 defects found by mozlint
You can run this analysis locally with:
- `./mach lint path/to/file` (JS/Python)
If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx
::: taskcluster/taskgraph/util/partners.py:63
(Diff revision 1)
> +# {
> +# "data": {
> +# "repository": {
> +# "object": {
> +# "text": "<?xml version=\"1.0\" ?>\n<manifest>\n " +
> +# "<remote fetch=\"git@github.com:mozilla-partners/\" name=\"mozilla-partners\"/>\n " +
Error: Line too long (104 > 99 characters) [flake8: E501]
::: taskcluster/taskgraph/util/partners.py:65
(Diff revision 1)
> +# "repository": {
> +# "object": {
> +# "text": "<?xml version=\"1.0\" ?>\n<manifest>\n " +
> +# "<remote fetch=\"git@github.com:mozilla-partners/\" name=\"mozilla-partners\"/>\n " +
> +# "<remote fetch=\"git@github.com:mozilla/\" name=\"mozilla\"/>\n\n " +
> +# "<project name=\"repack-scripts\" path=\"scripts\" remote=\"mozilla-partners\" " +
Error: Line too long (100 > 99 characters) [flake8: E501]
::: taskcluster/taskgraph/util/partners.py:66
(Diff revision 1)
> +# "object": {
> +# "text": "<?xml version=\"1.0\" ?>\n<manifest>\n " +
> +# "<remote fetch=\"git@github.com:mozilla-partners/\" name=\"mozilla-partners\"/>\n " +
> +# "<remote fetch=\"git@github.com:mozilla/\" name=\"mozilla\"/>\n\n " +
> +# "<project name=\"repack-scripts\" path=\"scripts\" remote=\"mozilla-partners\" " +
> +# "revision=\"master\"/>\n <project name=\"build-tools\" path=\"scripts/tools\" " +
Error: Line too long (100 > 99 characters) [flake8: E501]
::: taskcluster/taskgraph/util/partners.py:67
(Diff revision 1)
> +# "text": "<?xml version=\"1.0\" ?>\n<manifest>\n " +
> +# "<remote fetch=\"git@github.com:mozilla-partners/\" name=\"mozilla-partners\"/>\n " +
> +# "<remote fetch=\"git@github.com:mozilla/\" name=\"mozilla\"/>\n\n " +
> +# "<project name=\"repack-scripts\" path=\"scripts\" remote=\"mozilla-partners\" " +
> +# "revision=\"master\"/>\n <project name=\"build-tools\" path=\"scripts/tools\" " +
> +# "remote=\"mozilla\" revision=\"master\"/>\n <project name=\"mozilla-EME-free\" " +
Error: Line too long (101 > 99 characters) [flake8: E501]
Assignee | ||
Comment 55•6 years ago
|
||
mozreview-review |
Comment on attachment 8988122 [details]
Bug 1355482 - documentation for partner repacks,
https://reviewboard.mozilla.org/r/253370/#review260122
::: taskcluster/docs/partner-repacks.rst:30
(Diff revision 1)
> +* ``release_enable_partners``
> +* ``release_partner_config``
> +* ``release_partner_build_number``
> +* ``release_partners``
> +
> +We split the repacks into two 'branches', EME-free and everything else, to retain some
I wonder if the term `branches` might confuse people into thinking they're on separate vcs branches. I'm not sure how best to improve on it: two sets of kinds? Explicitly note that these are branches of the graph, rather than repo branches?
::: taskcluster/docs/partner-repacks.rst:34
(Diff revision 1)
> +
> +We split the repacks into two 'branches', EME-free and everything else, to retain some
> +flexibility over enabling/disabling them separately. This costs us some duplication of the kinds
> +in the repacking stack. The two enable parameters are booleans to turn these two branches
> +on/off. We set them in release-runner3's `is_partner_enabled() <https://dxr.mozilla
> +.org/build-central/source/tools/buildfarm/release/release-runner3.py#144>`_ when starting a release.
We'll need to update this link when we switch to ship-it v2.
An aside on these links: because they're not pinned to a revision, the line numbers may become inaccurate over time. If we link to a specific revision, the logic may become outdated over time. I'm not sure how best to resolve this.
Attachment #8988122 -
Flags: review?(aki) → review+
Assignee | ||
Comment 56•6 years ago
|
||
mozreview-review |
Comment on attachment 8988123 [details]
Bug 1355482 - documentation cleanups for release promotion,
https://reviewboard.mozilla.org/r/253372/#review260134
Thanks!
Attachment #8988123 -
Flags: review?(aki) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Attachment #8988123 -
Attachment is obsolete: true
Comment 60•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8988122 [details]
Bug 1355482 - documentation for partner repacks,
https://reviewboard.mozilla.org/r/253370/#review260122
> I wonder if the term `branches` might confuse people into thinking they're on separate vcs branches. I'm not sure how best to improve on it: two sets of kinds? Explicitly note that these are branches of the graph, rather than repo branches?
I improved this by switching to dxr search links which redirect to the result.
Comment 61•6 years ago
|
||
Pushed by nthomas@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8ebfd75f501
documentation for partner repacks, r=aki
Comment 62•6 years ago
|
||
Pushed by nthomas@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/347196a3c2c0
documentation cleanups for release promotion, r=aki
Comment 63•6 years ago
|
||
(In reply to Nick Thomas [:nthomas] (UTC+12) from comment #58)
> Comment on attachment 8988123 [details]
> Bug 1355482 - documentation cleanups for release promotion,
>
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/253372/diff/1-2/
I somehow managed to mark this is as discarded in mozreview, but landed it at https://hg.mozilla.org/integration/mozilla-inbound/rev/347196a3c2c004364d07bd1d6a8e1a94dbcb3fb2
Comment 64•6 years ago
|
||
bugherder |
Comment 65•6 years ago
|
||
bugherder |
Assignee | ||
Comment 66•6 years ago
|
||
I think this is good. We can continue to add more, e.g. bouncer.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment 67•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr60/rev/10d71f634565
https://hg.mozilla.org/releases/mozilla-esr60/rev/6a513fe07dfa
status-firefox-esr60:
--- → fixed
Updated•6 years ago
|
Keywords: leave-open
Updated•6 years ago
|
Assignee: nobody → nthomas
Comment 68•6 years ago
|
||
This was a bit of a group effort but aki did the most.
Assignee: nthomas → aki
Updated•3 years ago
|
Component: Documentation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•