adjust post_upload.py to use new candidates mount

RESOLVED FIXED

Status

Release Engineering
General
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: catlee, Assigned: jhopkins)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

6 years ago
post_upload.py needs to be taught how to use the new /pub/mozilla.org/firefox/candidates directory.

I think the best way forward here is to have post_upload.py copy files into the new candidates directory (e.g. /pub/mozilla.org/firefox/candidates/11.0b2-candidates/build1/...) and make sure a symlink exists from the old candidates directory to the new one (/pub/mozilla.org/firefox/nightly/11.0b2-candidates -> /pub/mozilla.org/firefox/candidates/11.0b2-candidates)

Comment 1

6 years ago
Mobile already does this by --nightly-dir=candidates in release uploads, I believe (sans softlink).
No longer depends on: 725838
Assignee: nobody → nrthomas
Priority: -- → P2
Created attachment 596843 [details] [diff] [review]
[puppet-manifests] create {firefox,thunderbird}/candidates on our file servers
Attachment #596843 - Flags: review?(rail)
Hmm, might be a bunch of buildbot factory work for this so I've pre-created a symlink for 11.0b3 in case I don't get that in time. Did a quick verify that post_upload follows that properly too.
Attachment #596843 - Flags: review?(rail) → review+
Comment on attachment 596843 [details] [diff] [review]
[puppet-manifests] create {firefox,thunderbird}/candidates on our file servers

http://hg.mozilla.org/build/puppet-manifests/rev/fdda5d64c8d0

master-puppet1 updated from f74e9363affa to fdda5d64c8d0.
Attachment #596843 - Flags: checked-in+
Is this bug done?

I run the following steps because the documentation [1] indicates to do so:
 # ffxbld@stage
 [ffxbld@surf ~]$ cd /pub/mozilla.org/firefox/nightly
 [ffxbld@surf nightly]$ mkdir ../candidates/12.0b5-candidates
 [ffxbld@surf nightly]$ ln -s ../candidates/12.0b5-candidates 12.0b5-candidates

[1]
https://wiki.mozilla.org/Releases/BuildNotesTemplate#Preparing_to_start_Automation
It's not, because of suckage. Those steps are the workaround.
We need this for rapid betas.
Blocks: 747168
(Reporter)

Updated

6 years ago
Assignee: nrthomas → jhopkins
(Assignee)

Comment 8

6 years ago
Created attachment 631103 [details] [diff] [review]
create nightly->candidates symlink if not already present (needs testing)
(Assignee)

Updated

6 years ago
Attachment #631103 - Flags: feedback?(catlee)
(Reporter)

Comment 9

6 years ago
Comment on attachment 631103 [details] [diff] [review]
create nightly->candidates symlink if not already present (needs testing)

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

::: stage/post_upload.py
@@ +213,5 @@
>  def ReleaseToTinderboxBuildsOverwrite(options, upload_dir, files):
>      ReleaseToTinderboxBuilds(options, upload_dir, files, dated=False)
>  
> +def symlink_nightly_to_candidates(nightly_path, candidates_base_path, version):
> +    _from = "%s/%s-candidates" % (nightly_path, version)

Can you use CANDIDATES_BASE_DIR here?

@@ +219,5 @@
> +    if not os.path.exists(_from):
> +        print("ln -s %s %s" % (_to, _from))
> +        os.symlink(_to, _from)
> +    else:
> +        print("symlink already exists: %s <- %s" % (_to, _from))

we don't need this else: clause in production.
Attachment #631103 - Flags: feedback?(catlee) → feedback+
(Assignee)

Comment 10

6 years ago
If someone is doing test builds on dev-stage01, it'd be nice to piggyback some testing of this post_upload.py patch.
(Assignee)

Comment 11

6 years ago
Created attachment 634974 [details] [diff] [review]
create nightly->candidates symlink if not already present (needs testing)

updated patch addressing catlee's feedback
(Assignee)

Comment 12

6 years ago
Created attachment 635811 [details] [diff] [review]
updated post_upload.py patch and .ini changes

Bugfix + change to accommodate ini-file-based configuration.

Test-build logs (note that the second build detects that directory/symlink are present and does nothing):
 http://dev-master01.build.scl1.mozilla.com:8900/builders/release-comm-beta-win32_build/builds/1/steps/make_upload/logs/stdio
 http://dev-master01.build.scl1.mozilla.com:8900/builders/release-comm-beta-linux_build/builds/4/steps/make_upload/logs/stdio
Attachment #631103 - Attachment is obsolete: true
Attachment #634974 - Attachment is obsolete: true
Attachment #635811 - Flags: review?(catlee)
(Reporter)

Comment 13

6 years ago
Comment on attachment 635811 [details] [diff] [review]
updated post_upload.py patch and .ini changes

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

::: stage/post_upload.py
@@ +228,5 @@
> +        print >> sys.stderr, "mkdir %s" % _to
> +        os.mkdir(_to)
> +    if not os.path.exists(_from):
> +        print >> sys.stderr, "ln -s %s %s" % (_to, _from)
> +        os.symlink(_to, _from)

sorry, I didn't catch this before, but I believe there's a possible race condition here if two release builds are trying to upload at the same time, they could both get False for os.path.exists() and then fail when trying to create either the directory or symlink.

Can you add some code to handle the case where the directory/symlink has been created between the test for existence and their creation?
Attachment #635811 - Flags: review?(catlee) → review-
(Assignee)

Comment 14

6 years ago
Created attachment 637620 [details] [diff] [review]
updated post_upload.py patch and .ini changes

I've added try/except blocks to address your comments.  I've also made the directory/symlink checks specifically check the existence of a directory or symlink instead of just "something by that name exists".
Attachment #635811 - Attachment is obsolete: true
Attachment #637620 - Flags: review?(catlee)
(Reporter)

Comment 15

6 years ago
Comment on attachment 637620 [details] [diff] [review]
updated post_upload.py patch and .ini changes

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

looks good!

::: stage/post_upload.py
@@ +239,5 @@
> +            os.symlink(_to, _from)
> +        except OSError, e:
> +            if e.errno == errno.EEXIST:
> +                print "%s already exists, continuing anyways" % _from
> +                pass

nit: pass is unnecessary here
Attachment #637620 - Flags: review?(catlee) → review+
(Assignee)

Comment 16

6 years ago
Comment on attachment 637620 [details] [diff] [review]
updated post_upload.py patch and .ini changes

Landed in https://hg.mozilla.org/build/tools/rev/9473d51af6cb
(addressed nit)
Attachment #637620 - Flags: checked-in+
(Assignee)

Comment 17

6 years ago
Following instructions at https://wiki.mozilla.org/ReleaseEngineering/How_To/Modify_scripts_on_stage

$ svn commit
Sending        files/bin/post_upload.py
Sending        files/etc/post_upload.ini
Transmitting file data ..
Committed revision 42139.
Comment on attachment 637620 [details] [diff] [review]
updated post_upload.py patch and .ini changes

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

So this change broke SeaMonkey, and with the bustage I noticed an (unrelated) error.

I solved this by just creating /home/ftp/pub/seamonkey/candidates  but would have been nice if this code did not require that (the code and ini absolutely require me to have candidates somewhere other than nightly/ so it can symlink, which is really unnecessary for SeaMonkey) that said, if the symlink ends up working through our beta without an issue, I won't argue for more than the actual error to be fixed.

::: stage/post_upload.py
@@ +228,5 @@
> +        print >> sys.stderr, "mkdir %s" % _to
> +        try:
> +            os.mkdir(_to)
> +        except OSError, e:
> +            if e.errno == errno.EEXIST:

So, here (and below) yield errors, due to errno being undefined (I suspect you wanted to import errno above for this)

(Here and below)
Attachment #637620 - Flags: feedback-
(In reply to Justin Wood (:Callek) from comment #18)
> So this change broke SeaMonkey, ...

example of said bustage:

python comm-beta/mozilla/build/upload.py --base-path . source/seamonkey-2.11b4.source.tar.bz2
 in dir /builds/slave/rel-comm-beta-source/. (timeout 1200 secs)
...
 using PTY: False
sys.argv: ['/usr/local/bin/post_upload.py', '-p', 'seamonkey', '-v', '2.11b4', '-n', '1', '-c', '/tmp/tmp.5EQ41L0Njy/', '/tmp/tmp.5EQ41L0Njy/source/seamonkey-2.11b4.source.tar.bz2']
mkdir /home/ftp/pub/seamonkey/candidates/2.11b4-candidates
Traceback (most recent call last):
  File "/usr/local/bin/post_upload.py", line 500, in <module>
    func(options, upload_dir, files)
  File "/usr/local/bin/post_upload.py", line 259, in ReleaseToCandidatesDir
    symlink_nightly_to_candidates(NIGHTLY_PATH, candidatesFullPath, options.version)
  File "/usr/local/bin/post_upload.py", line 232, in symlink_nightly_to_candidates
    if e.errno == errno.EEXIST:
NameError: global name 'errno' is not defined
Uploading /builds/slave/rel-comm-beta-source/source/seamonkey-2.11b4.source.tar.bz2
Running post-upload command: post_upload.py -p seamonkey -v 2.11b4 -n 1 -c
Command ['ssh', '-o', 'IdentityFile=~/.ssh/seabld_dsa', 'seabld@stage.mozilla.org', 'post_upload.py -p seamonkey -v 2.11b4 -n 1 -c "/tmp/tmp.5EQ41L0Njy/" "/tmp/tmp.5EQ41L0Njy/source/seamonkey-2.11b4.source.tar.bz2"'] returned non-zero exit code: 1
program finished with exit code 2
elapsedTime=2.262517
Of course, now, after the build itself finished fine, I'm hitting (on repacks, trying to download the binaries):

--02:34:29--  http://stage.mozilla.org/pub/mozilla.org/seamonkey/nightly/2.11b4-candidates/build1/linux-i686/en-US/seamonkey-2.11b4.tar.bz2
Resolving stage.mozilla.org... 63.245.215.47
Connecting to stage.mozilla.org|63.245.215.47|:80... connected.
HTTP request sent, awaiting response... 403 Forbidden
02:34:29 ERROR 403: Forbidden.

Comment 21

6 years ago
(In reply to Justin Wood (:Callek) from comment #19)
> NameError: global name 'errno' is not defined


That's the actual problem in the patch jhopkins did there. If it would find errno.EEXIST correctly, it should just continue fine.

Comment 22

6 years ago
Also, another problem we did run into is that if we put thinks into candidates/ and symlink from nightly/, then repacks choke because http access on stage forbids access to symlinked files/directories and the steps cannot download the builds for repacks. This might be SeaMonkey-only due to using some old release stuff, but not 100% sure. Better check FF/TB for this as well.

Updated

6 years ago
Blocks: 769931
$ svn ci -m "Bug 725839 Backout r42139, due to bustage"
Sending        files\bin\post_upload.py
Sending        files\etc\post_upload.ini
Transmitting file data ..
Committed revision 42383.

#...

$ hg out
comparing with ssh://hg.mozilla.org/build/tools
searching for changes
changeset:   2736:cbe3e3905ccf
parent:      2733:9473d51af6cb
user:        Justin Wood <Callek@gmail.com>
date:        Tue Jul 03 11:33:55 2012 -0400
summary:     Bug 725839 - Backed out changeset 9473d51af6cb due to bustage

changeset:   2737:b5beba297c82
tag:         tip
parent:      2735:979ae81a376c
parent:      2736:cbe3e3905ccf
user:        Justin Wood <Callek@gmail.com>
date:        Tue Jul 03 11:34:25 2012 -0400
summary:     Merge Bug 725839 backout

$ hg push
pushing to ssh://hg.mozilla.org/build/tools
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 2 changesets with 4 changes to 4 files
remote: Trying to insert into pushlog.
remote: Please do not interrupt...
remote: Inserted into the pushlog db successfully.

Updated

6 years ago
Attachment #637620 - Flags: checked-in+ → checked-in-

Updated

6 years ago
Attachment #596843 - Flags: checked-in+ → checked-in-

Updated

6 years ago
Attachment #596843 - Flags: checked-in- → checked-in+
(Assignee)

Comment 24

6 years ago
Created attachment 640254 [details] [diff] [review]
fix EEXIST usage, create relative (not absolute) symlink
Attachment #637620 - Attachment is obsolete: true
Attachment #640254 - Flags: review?(kairo)
Attachment #640254 - Flags: review?(bugspam.Callek)
Comment on attachment 640254 [details] [diff] [review]
fix EEXIST usage, create relative (not absolute) symlink

This looks alright to me, I got confused at first on why you needed to do the path.split there, and I'm still not certain on why it is required, but it looks like it will work anyway.

I'd like to allow catlee to take a stab at the review though.
Attachment #640254 - Flags: review?(kairo)
Attachment #640254 - Flags: review?(catlee)
Attachment #640254 - Flags: review?(bugspam.Callek)
Attachment #640254 - Flags: feedback+
(Reporter)

Updated

6 years ago
Attachment #640254 - Flags: review?(catlee) → review+
(Assignee)

Comment 26

6 years ago
Comment on attachment 640254 [details] [diff] [review]
fix EEXIST usage, create relative (not absolute) symlink

Landed in Mercurial at https://hg.mozilla.org/build/tools/rev/5931465e0903

Subversion productiondelivery commit:

$ svn commit
Sending        files/bin/post_upload.py
Sending        files/etc/post_upload.ini
Transmitting file data ..
Committed revision 43187.
Attachment #640254 - Flags: checked-in+
Depends on: 774589
(Assignee)

Comment 27

6 years ago
Things are looking good.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.