486992 - post_upload.py errors aren't caught

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

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

Hit an issue today where I ran out of space on staging-stage and post_upload.py errored out:
/tools/python/bin/python ../../build/upload.py --base-path ../../dist "../../dist/linux-i686/fr/firefox-3.5rc1.tar.bz2" ../../dist/linux-i686/xpi/fr.xpi
Uploading /builds/moz2_debug_slave/linux_repack/build/mozilla-1.9.1/dist/linux-i686/fr/firefox-3.5rc1.tar.bz2

firefox-3.5rc1.tar.bz2                          0%    0     0.0KB/s   --:-- ETA
firefox-3.5rc1.tar.bz2                        100% 9619KB   9.4MB/s   00:01    
Uploading /builds/moz2_debug_slave/linux_repack/build/mozilla-1.9.1/dist/linux-i686/xpi/fr.xpi

fr.xpi                                          0%    0     0.0KB/s   --:-- ETA
fr.xpi                                        100%  125KB 125.2KB/s   00:00    
Running post-upload command: post_upload.py -p firefox -v 3.5rc1 -n 1 --release-to-candidates-dir
Traceback (most recent call last):
  File "/usr/bin/post_upload.py", line 243, in ?
    func(options, upload_dir, files)
  File "/usr/bin/post_upload.py", line 129, in ReleaseToCandidatesDir
    CopyFileToDir(f, upload_dir, realCandidatesPath, preserve_dirs=True)
  File "/usr/bin/post_upload.py", line 34, in CopyFileToDir
    os.makedirs(full_dest_dir, 0755)
  File "/usr/lib/python2.3/os.py", line 154, in makedirs
    mkdir(name, mode)
OSError: [Errno 28] No space left on device: '/home/ftp/pub/firefox/nightly/3.5rc1-candidates/build1/linux-i686/fr'
Encountered error while uploading

The exit code isn't caught, it goes on to try to upload a second file, and the buildbot step has exit status 0. At least for releases we need to handle this better. I think the culprit is no error handling in hg:build/tools/stage/post_upload.py.

Comment 1

[Aki Sasaki (not active)]

Hm. Correct me if I'm wrong, but it looks like shutil.copyfile() doesn't do any sort of checking.  We could potentially do a checksum of each file pair (old vs new) to CopyFileToDir(), but this will add time, especially if we checksum large files.  A filesize comparison would be faster though less thorough.

Comment 2

[Chris AtLee [:catlee]]

I think the problem is in mozilla's build/upload.py, where it doesn't check the return from DoSSHCommand.

Comment 3

[(not currently active) Ted Mielczarek]

It does check the return (DoSSHCommand throws on non-zero exit):
http://mxr.mozilla.org/mozilla-central/source/build/upload.py#122
but upload.py doesn't exit with an error on exceptions:
http://mxr.mozilla.org/mozilla-central/source/build/upload.py#229

Comment 4

[[e-mail] (:bhearsum)]

(In reply to comment #3)
> It does check the return (DoSSHCommand throws on non-zero exit):
> http://mxr.mozilla.org/mozilla-central/source/build/upload.py#122
> but upload.py doesn't exit with an error on exceptions:
> http://mxr.mozilla.org/mozilla-central/source/build/upload.py#229

So we just need to sys.exit(1) on the except's in the if __name__ == '__main__' block?

Comment 5

[(not currently active) Ted Mielczarek]

Looks like it, yes, assuming post_upload.py also exits with an error.

Comment 6

[[e-mail] (:bhearsum)]

Judging from comment#0 where a Traceback is being printed, and post_upload.py not doing try/except when it calls the functions it looks like it exits non-zero. I just tested locally and indeed, when CopyFileToDir() raises an exception the exit code is non-zero. I'll get a patch for upload.py ready.

Comment 7

[[e-mail] (:bhearsum)]

Attachment: use sys.exit(non-zero) when exceptions are caught in upload.py

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py

changeset:   27534:5ed2092ca3a4

Landed on mozilla-central.

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py

The codepaths here aren't reached unless there's a failure to upload (eg. network problem). These are pretty rare, so they haven't been hit in a real build yet. It's a pretty trivial patch which will only improve error reporting, so I think this is ready to go on mozilla-1.9.1.

Technically, this is POTB, so I'm requesting approval.

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py

a191=beltzner

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 373340 [details] [diff] [review]
use sys.exit(non-zero) when exceptions are caught in upload.py

mozilla-1.9.1: changeset:   24948:376ea3c5e508

Comment 12

[[e-mail] (:bhearsum)]

I just landed this on mozilla-1.9.1. Tracemonkey will pick this up next time they merge, so I'm going to consider this bug FIXED.