1173343 - upload partner builds with post_upload.py

Status: RESOLVED FIXED

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

Description

[[e-mail] (:bhearsum)]

These currently upload with an rsync that's not going to work in an S3 world. post_upload.py is getting some magic added to it to work in the S3 world, so we should convert the partner repacks to use it, and pick that up for free.

Comment 1

[[e-mail] (:bhearsum)]

From bug 1165405, Nick suggested this approach:
> Fix: Borrow code from the en-US builds, http://hg.mozilla.org/build/buildbotcustom/file/fd6476cc8f48/process/factory.py#l2625. Download upload.py & deps from gecko repo so we can use it. Possibly upload each partner repack separately, to avoid several GB in one go

That file is https://github.com/mozilla/gecko-dev/blob/master/build/upload.py, and depends on "redo", so we'd have to satisfy that dependency. Partner repacks already clone the tools repository, so if we fix bug 1170142 we'd have easy access redo. Alternatively, we might be able to use the existing "retry" module in build/tools and import it as redo. I lean towards the former since it's something we want to do anyways, and should be relatively simple.

Other thoughts after an initial dive:
* We need to support both current-style releases and release promotion style ones. As far as this bug goes, that means being able to upload to both a candidates directory or tinderbox-builds. The existing post_upload.py code should be able to take care of most of this, we might need to add support for a new style of tinderbox-builds directory though. The existing code uploads to "$branch-$platform-partner_repack" dirs: https://github.com/mozilla/build-buildbotcustom/blob/f41caf1f05aedd414f0da4483a9d3693d74deecd/process/factory.py#L4734
* We've already got a "--use-ci-builds" flag, so it should be easy to decide which upload mechanism to use.
* https://github.com/mozilla/build-partner-repacks/commit/e9b670a4ebf3f2ed566f60273d1519b32c027c47 added some code that flips between filenames depending on pretty vs. ugly. I _think_ we still need this because it focuses on input filenames, not output, but it's probably worth double checking.

Comment 2

[[e-mail] (:bhearsum)]

(In reply to Ben Hearsum [:bhearsum] from comment #1)
> Other thoughts after an initial dive:
> * We need to support both current-style releases and release promotion style
> ones. As far as this bug goes, that means being able to upload to both a
> candidates directory or tinderbox-builds. The existing post_upload.py code
> should be able to take care of most of this, we might need to add support
> for a new style of tinderbox-builds directory though. The existing code
> uploads to "$branch-$platform-partner_repack" dirs:
> https://github.com/mozilla/build-buildbotcustom/blob/
> f41caf1f05aedd414f0da4483a9d3693d74deecd/process/factory.py#L4734

Turns out we'll need to tweak it a bit to work with release-style partner dirs too - I can't see a non-hacky way of uploading to "partner-repacks" right now. I filed bug 1173384 for this.

Comment 3

[[e-mail] (:bhearsum)]

OK, so we should already be able to use a POST_UPLOAD_CMD like this for release-style uploads:
post_upload.py -p firefox -n 1 -v 38.0.6 --release-to-candidates-dir --signed /tmp/foo/38.0.6/build1/ /tmp/foo/38.0.6/build1/partner-repacks/yandex-ua/linux-i686/ru/firefox-38.0.6.tar.bz2 

And once bug 1173384 is fixed, we should be able to use something like this for release promotion style uploads:
post_upload.py -p firefox --tinderbox-builds-dir mozilla-release-linux-i686-partner_repack --release-to-tinderbox-builds /tmp/foo/38.0.6/build1/partner-repacks /tmp/foo/38.0.6/build1/partner-repacks/yandex-ua/linux-i686/ru/firefox-38.0.6.tar.bz2

Comment 4

[[e-mail] (:bhearsum)]

I've got a WIP patch for the partner-repacks script over at https://github.com/mozilla/build-partner-repacks/compare/master...bhearsum:partner-repack-upload. I should be able to give it a try tomorrow and hopefully fix up any issues.

One thing I realized while writing it was that the current release promotion style dirs that don't include "partner-repacks" inside of the tinderbox-builds directory probably won't work, because repacks such as EME-free upload to sister directories. Since we're not using release promotion yet I suspect that won't be a big deal to change.

Comment 5

[[e-mail] (:bhearsum)]

Attachment: add support for uploading to partner-repacks.py

I had to rework a bit of RepackBase to cleanly support both uploading and not while still keeping the files in the right places. We'll be uploading as part of automation, but I think we still need to support non-uploading for one-off repacks (eg, the kind done with https://intranet.mozilla.org/Build:Partners:Repacks:Creation#Running_the_repack_script). More specifically, I separated out the moving/chmoding into a separate action called "stage" because upload.py/post_upload.py need the files to be laid out in their final form before uploading. And when we're uploading as we go along now, the cleanup action can remove builds afterwards to save on runtime disk space requirements.

I followed the usual procedure with this script of requiring the caller to provide upload.py (like we do with pkg-dmg, unpack-diskimage.sh), and added a check to make sure we can import UploadFiles when uploading is enabled.

As noted in the comments, the CI build uploading probably won't work, but that's probably getting totally reworked during the release promotion project anyways...maybe it's better just to bail early if use_ci_builds is set for now, not sure.

I tested this by doing a few manual repacks as well as rerunning all of the 38.0.6. They uploaded to http://dev-stage01.srv.releng.scl3.mozilla.com/pub/mozilla.org/firefox/candidates/38.0.6-candidates/build1/ and AFAICT it's the same set of builds in the same locations as we got for the real 38.0.6, modulo the last few mac repacks that haven't finished yet.

Comment 6

[[e-mail] (:bhearsum)]

Attachment: support uploads via partner repack scripts in PartnerRepackFactory

This patch is pretty simple, just adds support for passing the new args to partner-repacks.py and rips out the existing uploading support.

Comment 7

[[e-mail] (:bhearsum)]

You can see my dev-master job results over at these links too, if you like:
http://dev-master2.bb.releng.use1.mozilla.com:8018/builders/release-mozilla-release-macosx64_partner_repack/builds/0
http://dev-master2.bb.releng.use1.mozilla.com:8018/builders/release-mozilla-release-win32_partner_repack/builds/0
http://dev-master2.bb.releng.use1.mozilla.com:8018/builders/release-mozilla-release-linux_partner_repack/builds/2

Comment 8

[Chris Cooper [:coop] (he/him)]

Comment on attachment 8621220 [details] [diff] [review]
add support for uploading to partner-repacks.py

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

r+ with post_upload_cmd decision addressed.

::: scripts/partner-repacks.py
@@ +967,5 @@
> +                        # by creating TC artifacts instead
> +                        tinderbox_builds_dir = "{}-{}-partner_repack".format(path.basename(options.repo), ftp_platform)
> +                        post_upload_cmd = \
> +                            "post_upload.py -p firefox --tinderbox-builds-dir {} ".format(tinderbox_builds_dir) \
> +                          + "--release-to-tinderbox-builds"

Should we bother setting something that probably doesn't work vs just setting post_upload_cmd=None and letting whoever is running the script in this mode (likely by hand anyway) deal with uploading the repacks to where they are needed?

Comment 9

[[e-mail] (:bhearsum)]

(In reply to Chris Cooper [:coop] from comment #8)
> Comment on attachment 8621220 [details] [diff] [review]
> add support for uploading to partner-repacks.py
> 
> Review of attachment 8621220 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with post_upload_cmd decision addressed.
> 
> ::: scripts/partner-repacks.py
> @@ +967,5 @@
> > +                        # by creating TC artifacts instead
> > +                        tinderbox_builds_dir = "{}-{}-partner_repack".format(path.basename(options.repo), ftp_platform)
> > +                        post_upload_cmd = \
> > +                            "post_upload.py -p firefox --tinderbox-builds-dir {} ".format(tinderbox_builds_dir) \
> > +                          + "--release-to-tinderbox-builds"
> 
> Should we bother setting something that probably doesn't work vs just
> setting post_upload_cmd=None and letting whoever is running the script in
> this mode (likely by hand anyway) deal with uploading the repacks to where
> they are needed?

I think setting post_upload_cmd to None when upload-host et. al are passed would be pretty confusing. How do you feel about failing early with an "unsupported" message if use_ci_builds and upload_host are both set?

Comment 10

[[e-mail] (:bhearsum)]

Attachment: bail early for CI builds + uploading

(In reply to Ben Hearsum [:bhearsum] from comment #9)
> (In reply to Chris Cooper [:coop] from comment #8)
> > Comment on attachment 8621220 [details] [diff] [review]
> > add support for uploading to partner-repacks.py
> > 
> > Review of attachment 8621220 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > r+ with post_upload_cmd decision addressed.
> > 
> > ::: scripts/partner-repacks.py
> > @@ +967,5 @@
> > > +                        # by creating TC artifacts instead
> > > +                        tinderbox_builds_dir = "{}-{}-partner_repack".format(path.basename(options.repo), ftp_platform)
> > > +                        post_upload_cmd = \
> > > +                            "post_upload.py -p firefox --tinderbox-builds-dir {} ".format(tinderbox_builds_dir) \
> > > +                          + "--release-to-tinderbox-builds"
> > 
> > Should we bother setting something that probably doesn't work vs just
> > setting post_upload_cmd=None and letting whoever is running the script in
> > this mode (likely by hand anyway) deal with uploading the repacks to where
> > they are needed?
> 
> I think setting post_upload_cmd to None when upload-host et. al are passed
> would be pretty confusing. How do you feel about failing early with an
> "unsupported" message if use_ci_builds and upload_host are both set?

I did this, which also made me realize that post_upload_cmd is set (although unused) even if upload_host isn't. If you want something else here still just let me know.

Comment 11

[Chris Cooper [:coop] (he/him)]

(In reply to Ben Hearsum [:bhearsum] from comment #10) 
> > I think setting post_upload_cmd to None when upload-host et. al are passed
> > would be pretty confusing. How do you feel about failing early with an
> > "unsupported" message if use_ci_builds and upload_host are both set?
> 
> I did this, which also made me realize that post_upload_cmd is set (although
> unused) even if upload_host isn't. If you want something else here still
> just let me know.

My main argument here is that I don't think this mode is actually used. I know I've run the script against beta builds before, but purely in a test capacity. Exiting early with 'unsupported" is fine.

Comment 12

[Nick Thomas [:nthomas] (UTC+12)]

Bustage in 39.0b6: 
Traceback (most recent call last):
  File "./partner-repacks.py", line 988, in <module>
    repackObj.doRepack()
  File "./partner-repacks.py", line 419, in doRepack
    self.stage()
  File "./partner-repacks.py", line 389, in stage
    os.chmod("{}.asc".format(self.final_build), self.mode)
AttributeError: 'RepackLinux' object has no attribute 'mode'

Fixup: http://hg.mozilla.org/build/partner-repacks/rev/d81e715d6c1c

Comment 13

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #12)
> Bustage in 39.0b6: 
> Traceback (most recent call last):
>   File "./partner-repacks.py", line 988, in <module>
>     repackObj.doRepack()
>   File "./partner-repacks.py", line 419, in doRepack
>     self.stage()
>   File "./partner-repacks.py", line 389, in stage
>     os.chmod("{}.asc".format(self.final_build), self.mode)
> AttributeError: 'RepackLinux' object has no attribute 'mode'
> 
> Fixup: http://hg.mozilla.org/build/partner-repacks/rev/d81e715d6c1c

Thanks for fixing this up. Looks like the issue crept in after a bad refactor.

I did some verification of this by comparing the 39.0b5 and 39.0b6 partner repacks on stage. All of the same ones look present and in the same place:
[[e-mail]3 candidates]$ find 39.0b5-candidates/ -wholename "*partner"* -or -wholename "*eme*" | grep -v log | sed -e 's/39.0b5/39.0b6/g' | sort > ~/a.txt
[[e-mail]3 candidates]$ find 39.0b6-candidates/ -wholename "*partner"* -or -wholename "*eme*" | grep -v log | sort > ~/b.txt
[[e-mail]3 candidatediff -Naur ~/[ab].txt
[[e-mail]3 candidates]$ 

And the check_permissions builder verified that all of the files are set to 644, as they should be.

Comment 14

[Chris Cooper [:coop] (he/him)]

In production: https://hg.mozilla.org/build/buildbotcustom/rev/e35e6e58cc82