Closed Bug 1160385 Opened 4 years ago Closed 3 years ago

Generate checksums for release promotion

Categories

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

defect
Not set

Tracking

(firefox47 fixed, firefox49 fixed)

RESOLVED FIXED
Tracking Status
firefox47 --- fixed
firefox49 --- fixed

People

(Reporter: nthomas, Assigned: jlund)

References

Details

Attachments

(13 files, 3 obsolete files)

3.09 KB, patch
rail
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
58 bytes, text/x-review-board-request
Details
695 bytes, patch
rail
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
601 bytes, patch
rail
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
58 bytes, text/x-review-board-request
jlund
: checked-in+
Details
58 bytes, text/x-review-board-request
rail
: review+
jlund
: checked-in+
Details
48 bytes, text/x-github-pull-request
rail
: review+
Details | Review
1.16 KB, patch
jlund
: review+
Details | Diff | Splinter Review
1.13 KB, patch
rail
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
1.13 KB, patch
rail
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
2.00 KB, patch
rail
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
1.34 KB, patch
rail
: review+
jlund
: checked-in+
Details | Diff | Splinter Review
58 bytes, text/x-review-board-request
jlund
: review+
Details
Create a job which combines the checksums files from the builds and/or antivirus scan to produce the consolidated files SHA512SUMS, SHA1SUMs etc.
See Also: → 1174145
Assignee: nthomas → nobody
Status: ASSIGNED → NEW
Assignee: nobody → jlund
first step to this is to tell the checksums builder about beetmover s3 creds so we can use that instead of product-delivery bucket.

note: I created the puppet secret files mentioned in this patch already
Attachment #8721074 - Flags: review?(rail)
Attachment #8721074 - Flags: review?(rail) → review+
I'm going to land comment 2 on date as well as puppet patch while I implement releasetasks
Comment on attachment 8721516 [details]
MozReview Request: Bug 1160385 - adds version, buildnum support from tc releasetasks

on date: https://hg.mozilla.org/projects/date/rev/e0a91fd6584f
Comment on attachment 8721074 [details] [diff] [review]
160218_bug_1160385_relpro_checksums-puppet.patch

on puppet prod: https://hg.mozilla.org/build/puppet/rev/308a4c385bad
Attachment #8721074 - Flags: checked-in+
Comment on attachment 8721520 [details] [diff] [review]
160219_bug_1160385_checksums-bbot-cfgs.patch

I guess I need a review for this before I test it..
Attachment #8721520 - Flags: review?(rail)
Attachment #8721522 - Flags: review?(rail)
Comment on attachment 8721520 [details] [diff] [review]
160219_bug_1160385_checksums-bbot-cfgs.patch

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

::: mozilla/project_branches.py
@@ +198,5 @@
> +        'beetmover_candidates_bucket': 'mozilla-releng-beet-mover-dev',
> +        'stage_product': {
> +            'firefox': 'firefox',
> +            'fennec': 'mobile',
> +        }

can you add a trailing coma here
Attachment #8721520 - Flags: review?(rail) → review+
Comment on attachment 8721522 [details] [diff] [review]
160219_bug_1160385_checksums-bbotcustom.patch

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

conditional r+

::: process/release.py
@@ +1896,5 @@
> +        'slavebuilddir': normalizeName(checksums_buildername),
> +        'factory': checksums_factory,
> +        'properties': {
> +            'branch': branch_name,
> +            'platform': None,

Can you add "product" here to make log_uploader.py work?
Attachment #8721522 - Flags: review?(rail) → review+
because because..
Attachment #8722189 - Flags: review?(rail)
Attachment #8722189 - Flags: review?(rail) → review+
Comment on attachment 8722226 [details]
MozReview Request: Bug 1160385 - Generate checksums for release promotion, r?rail

https://reviewboard.mozilla.org/r/35919/#review32573
Attachment #8722226 - Flags: review?(rail) → review+
Attachment #8721522 - Attachment is obsolete: true
Attachment #8721520 - Flags: checked-in+
Attachment #8722189 - Flags: checked-in+
Attachment #8722225 - Flags: checked-in+
Attachment #8722226 - Flags: checked-in+
Attachment #8722734 - Flags: review?(rail)
Attachment #8722734 - Flags: review?(rail) → review+
Almost there!

http://buildbot-master72.bb.releng.usw2.mozilla.com:8001/builders/release-date-firefox_chcksms/builds/0/steps/run_script/logs/stdio

23:15:55     INFO - Run as scripts/scripts/release/generate-checksums.py --stage-product firefox --bucket-name-prefix mozilla-releng-beet-mover-dev --credentials /builds/dev-beetmover-s3.credentials --tools-repo /tools/checkouts/build-tools
Traceback (most recent call last):
  File "scripts/scripts/release/generate-checksums.py", line 262, in <module>
    myScript = ChecksumsGenerator()
  File "scripts/scripts/release/generate-checksums.py", line 93, in __init__
    self.file_prefix = self._get_file_prefix()
  File "scripts/scripts/release/generate-checksums.py", line 140, in _get_file_prefix
    self.config["stage_product"], self.config["version"], self.config["build_number"]
KeyError: 'version'
bah, wrong task
actually it failed: http://buildbot-master74.bb.releng.usw2.mozilla.com:8001/builders/release-date-firefox_chcksms/builds/0/steps/run_script/logs/stdio

01:51:41     INFO - #####
01:51:41     INFO - ##### Running collect-individual-checksums step.
01:51:41     INFO - #####
01:51:41     INFO - Running main action method: collect_individual_checksums
01:51:41     INFO - Connecting to S3
01:51:41     INFO - Connecting to bucket mozilla-releng-beet-mover-dev-firefox
01:51:42    FATAL - Uncaught exception: Traceback (most recent call last):
01:51:42    FATAL -   File "/builds/slave/rel-date-fx_chcksms-0000000000/scripts/mozharness/base/script.py", line 1765, in run
01:51:42    FATAL -     self.run_action(action)
01:51:42    FATAL -   File "/builds/slave/rel-date-fx_chcksms-0000000000/scripts/mozharness/base/script.py", line 1707, in run_action
01:51:42    FATAL -     self._possibly_run_method(method_name, error_if_missing=True)
01:51:42    FATAL -   File "/builds/slave/rel-date-fx_chcksms-0000000000/scripts/mozharness/base/script.py", line 1647, in _possibly_run_method
01:51:42    FATAL -     return getattr(self, method_name)()
01:51:42    FATAL -   File "scripts/scripts/release/generate-checksums.py", line 163, in collect_individual_checksums
01:51:42    FATAL -     bucket = self._get_bucket()
01:51:42    FATAL -   File "scripts/scripts/release/generate-checksums.py", line 156, in _get_bucket
01:51:42    FATAL -     self.bucket = conn.get_bucket(self.bucket_name)
01:51:42    FATAL -   File "/builds/slave/rel-date-fx_chcksms-0000000000/build/venv/lib/python2.7/site-packages/boto/s3/connection.py", line 502, in get_bucket
01:51:42    FATAL -     return self.head_bucket(bucket_name, headers=headers)
01:51:42    FATAL -   File "/builds/slave/rel-date-fx_chcksms-0000000000/build/venv/lib/python2.7/site-packages/boto/s3/connection.py", line 546, in head_bucket
01:51:42    FATAL -     raise err
01:51:42    FATAL - S3ResponseError: S3ResponseError: 404 Not Found
01:51:42    FATAL - 
01:51:42    FATAL - Running post_fatal callback...
01:51:42    FATAL - Exiting -1
01:51:42     INFO - Running post-run listener: copy_logs_to_upload_dir
01:51:42     INFO - Copying logs to upload dir...
01:51:42     INFO - mkdir: /builds/slave/rel-date-fx_chcksms-0000000000/build/upload/logs
Attachment #8724372 - Flags: review?(jlund) → review+
(In reply to Rail Aliiev [:rail] from comment #20)
> http://hg.mozilla.org/projects/date/file/tip/testing/mozharness/scripts/
> release/generate-checksums.py#l131 adds "-firefox" to the bucket name.
> Probably need to move it to configs.

I can patch something up
Attached patch checksums_use_full_bucket.patch (obsolete) — Splinter Review
v1
Attachment #8724913 - Flags: feedback?(rail)
v2

over irc there was some concern for fennec. I believe v1 patch addressed both desktop/fennec in the current bbot process case. but v2 addresses the fennec relpro case.

There are X amount of changes we need to make in order to support multiple products in relpro but since we are close to starting work on fennec, here is beetmover bucket plumbing..
Attachment #8724917 - Flags: feedback?(rail)
Comment on attachment 8724917 [details] [diff] [review]
checksums_use_full_bucket_support_relpro_fennec.patch

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

::: process/release.py
@@ +954,5 @@
>              extra_args=[
>                  "--stage-product", releaseConfig["stage_product"],
>                  "--version", releaseConfig["version"],
>                  "--build-number", releaseConfig["buildNumber"],
> +                "--bucket-name-full", branchConfig["S3Bucket"],

I bet this should be left as is, unless you uplift your patch everywhere, including esr38.
Attachment #8724917 - Flags: feedback?(rail) → feedback+
Comment on attachment 8724913 [details] [diff] [review]
checksums_use_full_bucket.patch

v2 is "more better" :P
Attachment #8724913 - Flags: feedback?(rail)
Attachment #8724913 - Attachment is obsolete: true
Attachment #8724917 - Attachment is obsolete: true
Attachment #8725117 - Flags: review?(rail)
https://hg.mozilla.org/mozilla-central/rev/aae846e4552b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Attachment #8725117 - Flags: review?(rail) → review+
Attachment #8725118 - Flags: review?(rail) → review+
Attachment #8725119 - Flags: review?(rail) → review+
Attachment #8725120 - Flags: review?(rail) → review+
Attachment #8725117 - Flags: checked-in+
Attachment #8725118 - Flags: checked-in+
Attachment #8725119 - Flags: checked-in+
Attachment #8725120 - Flags: checked-in+
Reopening to make sure we landed everything we need to m-c
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/1ed6ec687c61
https://hg.mozilla.org/mozilla-central/rev/b326f5187df6
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Blocks: 1260892
Blocks: 1260893
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8746185 [details]
MozReview Request: Bug 1160385 - Generate beetmover checksums r=jlund a=release DONTBUILD

https://reviewboard.mozilla.org/r/49291/#review46213

wow, this is neato. I like this approach :) some in line comments before stamping

::: testing/mozharness/scripts/release/beet_mover.py:254
(Diff revision 1)
> +    def _strip_prefix(self, s3_key):
> +        """Return file name relative to prefix"""
> +        # "abc/def/hfg".split("abc/de")[-1] == "f/hfg"
> +        return s3_key.split(self._get_template_vars()["s3_prefix"])[-1]
> +
>      def upload_bits(self):

it took a bit to understand changes made in this method

we have many variables that are variations of `s3_key` and `downloaded_file`.

I wonder if we can use less variables or change a name. `downloaded_file_info` and `beet_info_dest` stick out.

could just be me :)

::: testing/mozharness/scripts/release/beet_mover.py:276
(Diff revision 1)
> +                s3_key = self.manifest['mapping'][locale][deliverable]['s3_key']
>                  downloaded_file = os.path.join(dirs['abs_work_dir'], self.get_filename_from_url(source))
> -                self.upload_bit(
> -                    source=downloaded_file,
> -                    s3_key=self.manifest['mapping'][locale][deliverable]['s3_key'],
> -                    bucket=bucket,
> +                hash_ = self.file_sha512sum(downloaded_file)
> +                size = os.path.getsize(downloaded_file)
> +                file_ = self._strip_prefix(s3_key)
> +                downloaded_file_info = '{}.beet'.format(downloaded_file)

lol

::: testing/mozharness/scripts/release/generate-checksums.py:147
(Diff revision 1)
>              suffix = "firefox"
>  
>          return "{}-{}".format(self.config["bucket_name_prefix"], suffix)
>  
>      def _get_file_prefix(self):
> -        return "pub/{}/candidates/{}-candidates/build{}".format(
> +        return "pub/{}/candidates/{}-candidates/build{}/".format(

hmm, self.file_prefix seems to be used all over this script. why does it need a trailing / now? I'm surprised it doesn't break anything else.

::: testing/mozharness/scripts/release/generate-checksums.py:186
(Diff revision 1)
>  
>          def find_checksums_files():
>              self.info("Getting key names from bucket")
> +            checksum_files = {"beets": [], "checksums": []}
>              for key in bucket.list(prefix=self.file_prefix):
>                  if key.key.endswith(".checksums"):

hm, if we kept the '.checksums' files in the beet_mover yml template configs, won't they take precedence to the .beet equivalent you are generating that has proper file names?
Attachment #8746185 - Flags: review?(jlund)
Duplicate of this bug: 1268135
(In reply to Jordan Lund (:jlund) from comment #39)
> it took a bit to understand changes made in this method
> 
> we have many variables that are variations of `s3_key` and `downloaded_file`.
> 
> I wonder if we can use less variables or change a name.
> `downloaded_file_info` and `beet_info_dest` stick out.
> 
> could just be me :)

I see what you mean. I can rearrange things to be more clear.
 
> >      def _get_file_prefix(self):
> > -        return "pub/{}/candidates/{}-candidates/build{}".format(
> > +        return "pub/{}/candidates/{}-candidates/build{}/".format(
> 
> hmm, self.file_prefix seems to be used all over this script. why does it
> need a trailing / now? I'm surprised it doesn't break anything else.

I made this change just for the consistency with some other method which lists files under releases/$version/. Trailing slash was important there to prevent matching all betas when you try to use a prefix like releases/46.0 (which would match releases/46.0b*). In the current situation it is not a big deal, because we never go from build10  to build1 (the latter would catch the former without this fix). But as I said, this is not a big deal, I can revert thi hunk.

> ::: testing/mozharness/scripts/release/generate-checksums.py:186
> (Diff revision 1)
> >  
> >          def find_checksums_files():
> >              self.info("Getting key names from bucket")
> > +            checksum_files = {"beets": [], "checksums": []}
> >              for key in bucket.list(prefix=self.file_prefix):
> >                  if key.key.endswith(".checksums"):
> 
> hm, if we kept the '.checksums' files in the beet_mover yml template
> configs, won't they take precedence to the .beet equivalent you are
> generating that has proper file names?

Nope, they shouldn't. You evaluate this condition for every file in the bucket separately.
Comment on attachment 8746185 [details]
MozReview Request: Bug 1160385 - Generate beetmover checksums r=jlund a=release DONTBUILD

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/49291/diff/1-2/
Attachment #8746185 - Flags: review?(jlund)
Comment on attachment 8746185 [details]
MozReview Request: Bug 1160385 - Generate beetmover checksums r=jlund a=release DONTBUILD

https://reviewboard.mozilla.org/r/49291/#review46389

++ ty, I could grep this in seconds.. :)
Attachment #8746185 - Flags: review?(jlund) → review+
Duplicate of this bug: 1260892
https://hg.mozilla.org/mozilla-central/rev/eaecda9c7ac6
Status: REOPENED → RESOLVED
Closed: 4 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.