1463749 - Missing checksums of language packs for ESR60 (60.0esr-candidates)

Status: RESOLVED FIXED

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

Description

[Sukhbir Singh]

The checksum files for 60.0esr are missing the hashes of the language packs (the XPI files). For example,

https://ftp.mozilla.org/pub/firefox/candidates/60.0esr-candidates/build6/SHA512SUMS

has no language pack files. 

Compare this with 60.0b10, which has the hashes:

https://ftp.mozilla.org/pub/firefox/candidates/60.0b10-candidates/build1/SHA512SUMS

cf6d43bce31c5ddeddaba7c2fa39f3130fd6a20bcfceba45e86f5bb04ccee0b7f9e0b3a63eed193d9c20749b43b04057a68dd2364679716d36fa87d1e05285f8  linux-i686/xpi/ach.xpi

This is true for both the SHA256SUMS and SHA512SUMS files of 60.0esr.

Comment 1

[Mihai Tabara [:mtabara]⌚️GMT]

Hm, at first glance this seems to be accurate. We need to dive in and see if there's actual tasks moving the checksums in the /beet files.

Comment 2

[Jordan Lund (:jlund)]

@callek - do you think this is something you can look into. At the very least, this should be tracked in our tcmigration board. You mentioned that you fear it is missing from recent beta and releases too.

while it seems to be there for 60.0b10 as pointed out in commment 0, you seem to be right and that it is missing from most recent beta: https://ftp.mozilla.org/pub/firefox/candidates/61.0b9-candidates/build1/SHA512SUMS

I imagine this is likely fallout from recent changes to the way we configure releases via taskgraph.

Comment 3

[Justin Wood (:Callek)]

Thank You Sukhbir Singh,

This is indeed a valid bug, I'll be investigating fixing this but it likely won't be fixed until at least 60.2esr possibly 60.3.

(There are some complications in the way some of the backend code is implemented to work through).


For my own memory:
From chatting with mihai yesterday, to have a permanent notes on how to approach fixing...

10:57 AM <mtabara> how about we do this
10:57 AM <mtabara> we add langpacks kinds here https://github.com/mozilla-releng/beetmoverscript/blob/master/beetmoverscript/constants.py#L152
10:57 AM <mtabara> which is used here https://github.com/mozilla-releng/beetmoverscript/blob/master/beetmoverscript/task.py#L88
10:58 AM <mtabara> which is used here https://github.com/mozilla-releng/beetmoverscript/blob/master/beetmoverscript/task.py#L93
10:58 AM <mtabara> instead of target.checksums you're giving it a custom name
10:58 AM <mtabara> that you can later on append in the templates 
10:59 AM <mtabara> similarly to what target-source.checksums is generated and uploaded https://github.com/mozilla-releng/beetmoverscript/blob/master/beetmoverscript/templates/firefox_candidates.yml#L99
10:59 AM <mtabara> so you'd have
10:59 AM <mtabara> a) two new kinds to add: checksums signing + beetmover-checksums
10:59 AM <mtabara> b) tweak beetmoverscript so that langpacks checksums are custom named to target-langpack{whatever}.checksums
11:00 AM <mtabara> add in it in the templates

Comment 4

[Justin Wood (:Callek)]

Attachment: Bug 1463749 - Langpack Checksums r=mtabara

Comment 5

[Mihai Tabara [:mtabara]⌚️GMT]

Not approving this yet. Let's see how it goes on some staging-release maple-testing.
I landed it - https://hg.mozilla.org/projects/maple/rev/e9e8173102e526673d81396d0853fa80b928bc8a
Will let the Decision task built for a bit before starting a staging release to see how things go.

Comment 6

[Mihai Tabara [:mtabara]⌚️GMT]

I pinned all beetmoverworkers to my environment which is based on your beetmoverscript PR[0].
Triggered a staging release here[1] with a sample task here[2].
It currently fails for CoT errors, haven't looked deep there yet:

===
2018-07-27T10:47:27 CRITICAL - Chain of Trust verification error!
Traceback (most recent call last):
  File "/builds/scriptworker/lib/python3.6/site-packages/scriptworker/cot/verify.py", line 1813, in verify_chain_of_trust
    await download_cot_artifacts(chain)
  File "/builds/scriptworker/lib/python3.6/site-packages/scriptworker/cot/verify.py", line 705, in download_cot_artifacts
    mandatory_artifacts_paths = await raise_future_exceptions(mandatory_artifact_tasks)
  File "/builds/scriptworker/lib/python3.6/site-packages/scriptworker/utils.py", line 326, in raise_future_exceptions
    succeeded_results, _ = await _process_future_exceptions(tasks, raise_at_first_error=True)
  File "/builds/scriptworker/lib/python3.6/site-packages/scriptworker/utils.py", line 360, in _process_future_exceptions
    raise exc
  File "/builds/scriptworker/lib/python3.6/site-packages/scriptworker/cot/verify.py", line 655, in download_cot_artifact
    raise CoTError("path {} not in {} {} chain of trust artifacts!".format(path, link.name, link.task_id))
scriptworker.exceptions.CoTError: 'path public/target-langpack.checksums not in beetmover:signing Ttm8VLvBTmODku1dYlzqqg chain of trust artifacts!'
2018-07-27T10:47:27    ERROR - Hit ScriptWorkerException: 'path public/target-langpack.checksums not in beetmover:signing Ttm8VLvBTmODku1dYlzqqg chain of trust artifacts!'
===

If the fix doesn't imply an in-tree follow-up, we can adddress this by rerunning the current tasks to save some time.

[0]: https://github.com/mozilla-releng/beetmoverscript/pull/174/files
[1]: https://tools.taskcluster.net/groups/A6mBskKvRuKAxXLkcl5OKg  
[2]: https://tools.taskcluster.net/groups/A6mBskKvRuKAxXLkcl5OKg/tasks/OHe7PdNxQvC8Jyh46B3fOA/details

Comment 7

[Mihai Tabara [:mtabara]⌚️GMT]

Judging by https://tools.taskcluster.net/groups/A6mBskKvRuKAxXLkcl5OKg/tasks/Ttm8VLvBTmODku1dYlzqqg/runs/0/artifacts sounds like the PR logic didn't work, for some reason the checksums file is still called "target.checksums" instead of "target-langpack.checksums"

Comment 8

[Mihai Tabara [:mtabara]⌚️GMT]

Found the culprit[1]. Pushed a fix in the PR[2] and rolled-that out to workers. 
I rerun all the initial `beetmover-signed-langpacks-nightly-l10n` to regenerate that checksums file and have it properly renamed to target-langpack.checksums. Afterwards I rerun all the failing with CoT-exception `release-beetmover-signed-langpacks-checksums` jobs which are now all green.

Results can be seen in the checksums beets folder[3]. For example [4][5][6]

[1]: https://github.com/mozilla-releng/beetmoverscript/pull/174#discussion_r205740916
[2]: https://github.com/mozilla-releng/beetmoverscript/pull/174/commits/385fbb97d9bf54a0ffb00a58a3dcaac8c6977803 rolled-out to beetmoverscript workers. 
[3]: http://ftp.stage.mozaws.net/pub/firefox/candidates/62.0b12-candidates/build1/beetmover-checksums/
[4]: http://ftp.stage.mozaws.net/pub/firefox/candidates/62.0b12-candidates/build1/beetmover-checksums/linux-i686/xpi/en-US.checksums.beet
[5]: http://ftp.stage.mozaws.net/pub/firefox/candidates/62.0b12-candidates/build1/beetmover-checksums/mac/xpi/en-US.checksums.beet
[6]: http://ftp.stage.mozaws.net/pub/firefox/candidates/62.0b12-candidates/build1/beetmover-checksums/win64/xpi/en-US.checksums.beet

Comment 9

[Mihai Tabara [:mtabara]⌚️GMT]

Only concern there is if that tag for `beetmover-signed-langpacks-nightly-l10n` is indeed the one we now have in constants.py or is there a mistake in the in-tree transforms that we need to address, and rollback to your initial PR approach.

Comment 10

[Mihai Tabara [:mtabara]⌚️GMT]

Worth waiting for the `release-generate-checksums-firefox`[1] task to run to make sure it incorporates the newly added checksums files in the final aggregated SHA{256,512}SUMS files.

[1]: https://tools.taskcluster.net/groups/A6mBskKvRuKAxXLkcl5OKg/tasks/ahSym8C1R46pyWyU37ldOQ/details

Comment 11

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #10)
> Worth waiting for the `release-generate-checksums-firefox`[1] task to run to
> make sure it incorporates the newly added checksums files in the final
> aggregated SHA{256,512}SUMS files.
> 
> [1]:
> https://tools.taskcluster.net/groups/A6mBskKvRuKAxXLkcl5OKg/tasks/
> ahSym8C1R46pyWyU37ldOQ/details

http://ftp.stage.mozaws.net/pub/firefox/candidates/62.0b12-candidates/build1/SHA256SUMS

Comment 12

[Justin Wood (:Callek)]

Ok, so this staging run had some issues.

Namely it didn't write checksum .beet's for l10n, this has a few minor issues.

- I need to add it to _repack templates in the github PR.
- The in-tree changes don't have individual locales (we're not passing any.../en-us)

There is a few minor issues/differences here so I want advice from mihai..

- Language Pack beetmover is the only l10n where we do more than one locale at a time of beetmoving...
- Language Pack beetmovers are "chunk_locales" so have multiple locales at once.
- Doing the checksums we are also splitting by chunk right now.

I can see a few ideas to fix this.

- Login in beetmoverscript to support multi locales at once.
- Lie for {locale} in the templates/payload (such as "ar..cs" or "chunk1" etc. for the .beet files)
- Split the checksums by locale, so we have N checksums (where N is number of locales) but the .beet files will have duplicate lines compared to each other (e.g. if a chunk has es-ES and en-GB, the each of their .beet files would have both) [Is this supported?!]
- Split all the langpack beetmover jobs up by locale instead of chunk [this has implication on setup/teardown time, because instead of 5-10 beetmover tasks we would have 100-200, just for the langpacks]

Mihai, Thoughts?

Comment 13

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Justin Wood (:Callek) from comment #12)
> Ok, so this staging run had some issues.
> 
> Namely it didn't write checksum .beet's for l10n, this has a few minor
> issues.
> 
> - I need to add it to _repack templates in the github PR.

True. But must be an error in in-tree transforms as well as if they were correct, we would've had the tasks scheuled and jobs failing with missing entry in the templates. So we have a bug somewhere in-tree. But I agree we need this in the _repack templates too.

> - The in-tree changes don't have individual locales (we're not passing
> any.../en-us)

In conjunction with the above to make it work, indeed!

> There is a few minor issues/differences here so I want advice from mihai..
> 
> - Language Pack beetmover is the only l10n where we do more than one locale
> at a time of beetmoving...
> - Language Pack beetmovers are "chunk_locales" so have multiple locales at
> once.
> - Doing the checksums we are also splitting by chunk right now.
> 
> I can see a few ideas to fix this.
> 
> - Login in beetmoverscript to support multi locales at once.

Not sure I follow here. We already support this for Fennec multi/en-US. e.g. https://tools.taskcluster.net/groups/TITlwqRURM-i2jOZ8dhSBA/tasks/Y3yUU359TTyi0MzSKo0vlA/details

> - Lie for {locale} in the templates/payload (such as "ar..cs" or "chunk1"
> etc. for the .beet files)
> - Split the checksums by locale, so we have N checksums (where N is number
> of locales) but the .beet files will have duplicate lines compared to each
> other (e.g. if a chunk has es-ES and en-GB, the each of their .beet files
> would have both) [Is this supported?!]
> - Split all the langpack beetmover jobs up by locale instead of chunk [this
> has implication on setup/teardown time, because instead of 5-10 beetmover
> tasks we would have 100-200, just for the langpacks]
> 
> Mihai, Thoughts?

It doesn't actually matter if they are chunked or not in my opnion. Beetmover jobs will generate a target.checksums on all the files that are transferred in one beetmover at a time. So if you have multiple locales baked within a beetmover job, all those artifacts across all the locales/taskIds will be aggregated together (here[1]) and joint checksums generated at the end that is to be transferred into .beet files later on in downstream tasks. 

If we end up having duplicate checksums across different .beet files, that's going to definitely break the release-generate-checksums job because of this check[2]. We could probably workaround that by amending the script if we really wanted to.


[1]: https://github.com/mozilla-releng/beetmoverscript/blob/master/beetmoverscript/script.py#L315
[2]: https://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/generate-checksums.py#l169

Comment 14

[Justin Wood (:Callek)]

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #13)
> (In reply to Justin Wood (:Callek) from comment #12)
> > I can see a few ideas to fix this.
> > 
> > - Login in beetmoverscript to support multi locales at once.
> 
> Not sure I follow here. We already support this for Fennec multi/en-US. e.g.
> https://tools.taskcluster.net/groups/TITlwqRURM-i2jOZ8dhSBA/tasks/
> Y3yUU359TTyi0MzSKo0vlA/details

While for android this is "multi" which is "all not en-us" which is its own locale code, in this bugs case we have locales of "en-GB, en-CA, ru" etc together in one beetmoved job, and others with other sets of locales. So doing multiple payload attempts could work but wouldn't solve the issues

> 
> > - Lie for {locale} in the templates/payload (such as "ar..cs" or "chunk1"
> > etc. for the .beet files)

This would mean naming "locale" "chunkN" fwiw. and then in beetmover templates {locale} which really gets expanded to "chunkN"

> > - Split the checksums by locale, so we have N checksums (where N is number
> > of locales) but the .beet files will have duplicate lines compared to each
> > other (e.g. if a chunk has es-ES and en-GB, the each of their .beet files
> > would have both) [Is this supported?!]

You mention below that this would break beetmover checksums right now.

> > - Split all the langpack beetmover jobs up by locale instead of chunk [this
> > has implication on setup/teardown time, because instead of 5-10 beetmover
> > tasks we would have 100-200, just for the langpacks]

This feels like the best option right now, but that means we'd have MANY more beetmover jobs than currently, do you forsee any problems with doing that.

> It doesn't actually matter if they are chunked or not in my opnion.
> Beetmover jobs will generate a target.checksums on all the files that are
> transferred in one beetmover at a time. So if you have multiple locales
> baked within a beetmover job, all those artifacts across all the
> locales/taskIds will be aggregated together (here[1]) and joint checksums
> generated at the end that is to be transferred into .beet files later on in
> downstream tasks. 
> 
> If we end up having duplicate checksums across different .beet files, that's
> going to definitely break the release-generate-checksums job because of this
> check[2]. We could probably workaround that by amending the script if we
> really wanted to.

Yea the issue is more about the fact that all sums are in the beetmover job that handles multiple locales (but not all locales).

> 
> [1]:
> https://github.com/mozilla-releng/beetmoverscript/blob/master/
> beetmoverscript/script.py#L315
> [2]:
> https://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/
> release/generate-checksums.py#l169

Comment 15

[Mihai Tabara [:mtabara]⌚️GMT]

Discussed over IRC:

mtabara> Callek: I wonder if it's worth tweaking the release-generate-checksums job instead if that's the main issue, hope I'm reading this right
17:50:11 <Callek> mtabara: its more an issue of where/how to store the .beet's in the folders and under what name
17:50:11 <Callek> at least to me
17:50:26 <mtabara> we could amend code here https://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/generate-checksums.py#l169 to say: 'could be duplicate but must match' at least
17:50:51 <mtabara> what I'm trying to say is we could be quite flexible imo
17:51:07 <Callek> mtabara: which means how do we expose the checksum files to begin with (when we push the .xpi's) and then how do we validate them
17:51:18 <mtabara> the aggregation checksums job runs recursively on whatever it finds under beetmover-checksums
17:51:31 <Callek> mtabara: ahhh ok, so you're suggesting duplicating all the .beet's across locales and then having gen-checksums not care if they are duplicated as long as sha's match
17:51:37 <mtabara> exactly
17:51:48 <Callek> mtabara: ok, I think that makes sense to me, I can handle it
17:52:37 <mtabara> I agree with you that it's a waste to throw another 100 beetmover jobs in the game 
17:52:42 <mtabara> graphs are already too big
17:54:41 — mtabara files https://github.com/mozilla-releng/beetmoverscript/issues/177 to tackle it at some point after maven + declarative artifacts are done

Comment 16

[Justin Wood (:Callek)]

Attachment: Bug 1463749 - Support checksum .beet files having duplicate content, as long as sha's match. r=mtabara

Comment 17

[Phabricator Automation]

Comment on attachment 8995326 [details]
Bug 1463749 - Langpack Checksums r=mtabara

Mihai Tabara [:mtabara]⌚️GMT has approved the revision.

https://phabricator.services.mozilla.com/D2446

Comment 18

[Phabricator Automation]

Comment on attachment 8997694 [details]
Bug 1463749 - Support checksum .beet files having duplicate content, as long as sha's match. r=mtabara

Mihai Tabara [:mtabara]⌚️GMT has approved the revision.

https://phabricator.services.mozilla.com/D2756

Comment 19

[Mihai Tabara [:mtabara]⌚️GMT]

Landed all these patches + rolled-out latest changes from beetmoverscript.
Kicked-off https://tools.taskcluster.net/groups/dEqXPEX5TueDxsnJ50mj4Q to track latest staging release for these.

Comment 20

[Justin Wood (:Callek)]

Mihai, I found an issue in my taskgraph patch, can you please land the interdiff to maple and kick off a new run?

Comment 21

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Justin Wood (:Callek) from comment #20)
> Mihai, I found an issue in my taskgraph patch, can you please land the
> interdiff to maple and kick off a new run?

Done:
* https://hg.mozilla.org/projects/maple/rev/831501044347
* https://tools.taskcluster.net/groups/Ja0bBG5CS6ie8irPbI4r4g

Comment 22

[Mihai Tabara [:mtabara]⌚️GMT]

Deployed beetmoverscript to production:
* https://github.com/mozilla-releng/beetmoverscript/pull/174
* https://github.com/mozilla-releng/build-puppet/pull/161

7.7.0 encompasses these changes.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/185d367f1916
Support checksum .beet files having duplicate content, as long as sha's match. r=mtabara
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a174db276a3
Langpack Checksums r=mtabara

Comment 24

[Justin Wood (:Callek)]

Comment on attachment 8995326 [details]
Bug 1463749 - Langpack Checksums r=mtabara

Approval Request Comment
[Feature/Bug causing the regression]: Language Pack Signing
[User impact if declined]: Users unable to verify that language packs on releases.mozilla.org are of the proper shasum
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: No (unable to verify here, but I did test on a releng staging release)
[Needs manual test from QE? If yes, steps to reproduce]:  No
[List of other uplifts needed for the feature/fix]: Just the other patch in this bug
[Is the change risky?]: 1 out of 5 risk (5 being very risky)
[Why is the change risky/not risky?]: If it breaks could cause some slight delay in shipping the release, but is not a product-level user facing issue.
[String changes made/needed]: None

Comment 25

[Justin Wood (:Callek)]

Comment on attachment 8997694 [details]
Bug 1463749 - Support checksum .beet files having duplicate content, as long as sha's match. r=mtabara

Same considerations as the previous approval request in this bug.

Comment 26

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/185d367f1916
https://hg.mozilla.org/mozilla-central/rev/9a174db276a3

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8995326 [details]
Bug 1463749 - Langpack Checksums r=mtabara

Fixes missing checksums for langpacks. Approved for 62.0b17 and ESR 60.2.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b93cd3e23a6c
https://hg.mozilla.org/releases/mozilla-beta/rev/af8e3fb57cb7

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/9acf7f25890b
https://hg.mozilla.org/releases/mozilla-esr60/rev/2177be7a65c0