371305 - make update verification run on each platform

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Robert Helmer [:rhelmer]]

Attachment: require that config is set before verify is invoked, and copy the proper updater binary

We need to run update verification on the proper platform (it all runs on one Mac right now), and we should set it up so this can run automatically from Buildbot (both for nightlies and on-demand).

Comment 1

[Robert Helmer [:rhelmer]]

I've made this work from Buildbot by setting the environment:

updateEnvironment = {
    'release' : '2.0.0.1',
    'product' : 'Firefox',
    'platform': 'Linux_x86-gcc3',
    'build_id' : '2006120814',
    'locales' : 'en-US',
    'channel' : 'betatest',
    'from' : '/firefox/releases/2.0.0.1/linux-i686/%locale%/firefox-2.0.0.1.tar.gz',
    'to' : '/firefox/nightly/2.0.0.2-candidates/rc5/firefox-2.0.0.2.%locale%.linux-i686.tar.gz'}


Note that "from" and "to" are optional, if are specified then it means that they should be downloaded, updates applied, and compared. If both "from" and "to" are not specified, then the verification scripts simply check to make sure that MARs are available and match the desired checksum and size specified in the update.xml

All of the other variables are required.

Once the above it set, verify.sh (checked out from mozilla/testing/release/) can be invoked like so:

f1.addStep(ShellCommand, command=["./verify.sh", "-c"],
                         workdir="build/updates",
                         env=updateEnvironment)


Any ideas on what the best way to get the data in the environment into Buildbot is? We could have Buildbot just pick up an OS-specific config file out of CVS, or we could allow sendchange to specify all of this data (which means that for a release we'd submit ~120 changes, which is probably the most scalable way to do this but also pretty annoying).

For this round, I would be happy to have these running on three machines, one for each OS being tested (we should be testing these on the proper OS anyway, e.g. updater.exe should be used for win32).

Comment 2

[Robert Helmer [:rhelmer]]

Attachment: return >0 exit code on failure, remove "tee /dev/stderr"

Same as last time, plus:

* remove the "| tee /dev/stderr" stuff, don't need that anymore
* set exit code to be the same as the number of failures

This patch makes it work nicely in buildbot out-of-the-box, and should work without complaint on Windows. I'll likely have at least one more patch to make it unpack and use the right updater binary on win32 and mac (this patch tested on linux via buildbot)

Comment 3

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 256108 [details] [diff] [review]
return >0 exit code on failure, remove "tee /dev/stderr"

Need to call the right updater per platform in check_updates.sh (win/mac). 

Otherwise, this looks like it should work. It'll be easier to read in a non-diffed state.

Comment 4

[Robert Helmer [:rhelmer]]

Attachment: corrected updater path, tested on win32/mac

Comment 5

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 256846 [details] [diff] [review]
corrected updater path, tested on win32/mac

double-A-plus-goldstar

Comment 6

[Robert Helmer [:rhelmer]]

Attachment: revert verify.sh changes, make config file configurable instead

I went ahead and landed the bits in the common/ dir from the last patch, thanks!

I haven't checked in any changes to verify.sh; I think that the short-term path to happiness is going to be doing 3 runs of this script, one per OS, each with it's own config file from CVS. The attached patch just makes it possible to optionally specify a config file, e.g.:

./verify.sh -t moz18-branch-updates.cfg

If the config file is not specified:

./verify.sh -t 

Then update.cfg is used (the previous behavior).

All buildbot needs to do is specify the config file (it'll be OS specific, probably something like moz18-branch-osx.cfg); the environment variable trick above isn't needed yet, although I would like to do something more like that in the future.

Comment 7

[Robert Helmer [:rhelmer]]

Attachment: remove "|tee /dev/stderr" and set diff FAIL to WARN

Same as last time, but:

1) remove "| tee /dev/stderr", won't work on cygwin and this was only useful when triggered by a human; might as well just grep for FAIL now
2) just because there are differences it isn't necessarily a FAIL - it should definitely be a WARN, this needs to be decided by a human

I've been meaning to take care of #2 for a while, even though it's not strictly related to this bug. It'd be ideal to make this part of the test smarter so we can confidently pass/fail; some of the issues we found are not the kind of thing you'd want to ignore a file for (e.g. "diff -x filename").

Comment 8

[Robert Helmer [:rhelmer]]

Attachment: use wget instead of curl for complete update case

Use wget for the normal "complete update" test. This still uses curl for the "quick" headers-only tests, because that has no platform dependencies, and isn't a feature wget has afaict (that is, follow a redirect and return the results of a HEAD request).

This change allows us to run this script on the standard build machines, which include wget but not curl.

Comment 9

[J. Paul Reed [:preed]]

Comment on attachment 258601 [details] [diff] [review]
use wget instead of curl for complete update case

The error was:

../common/download_mars.sh: line 7: curl: command not found

This patch doesn't patch download_marsh.sh

Also, I'd use -O instead of >.

Comment 10

[Robert Helmer [:rhelmer]]

Attachment: sorry, this misses the common dir (ran diff too high up)

Comment 11

[Robert Helmer [:rhelmer]]

Attachment: use wget, dispense with stdout/redirect

Comment 12

[J. Paul Reed [:preed]]

Comment on attachment 258606 [details] [diff] [review]
use wget, dispense with stdout/redirect

This is OK, except for the -qO [argument to O].

I'm actually surprised that wget allows this, since it's very confusing; it makes it look like the O (which could be a 0/zero) is an argument to -q.

Can we do -q -O [foo] ?

Comment 13

[Robert Helmer [:rhelmer]]

Attachment: separate -q and -O for readability

Comment 14

[Robert Helmer [:rhelmer]]

Landed:

Checking in common/download_mars.sh;
/cvsroot/mozilla/testing/release/common/download_mars.sh,v  <--  download_mars.sh
new revision: 1.15; previous revision: 1.14
done
Checking in updates/verify.sh;
/cvsroot/mozilla/testing/release/updates/verify.sh,v  <--  verify.sh
new revision: 1.8; previous revision: 1.7
done

Comment 15

[Robert Helmer [:rhelmer]]

Attachment: use wget --no-check-certificate

Some versions of wget balk at * certificates, and need --no-check-certificate (a *.mozilla.org cert in this case).

The one in common/download_mars.sh that talks to AUS is the important one for the "complete" case, but we might as well make it consistent across-the-board (including the one remaining instance of "curl", the equiv is "-k").

Comment 16

[J. Paul Reed [:preed]]

Comment on attachment 258613 [details] [diff] [review]
use wget --no-check-certificate

r=preed.

I'll land this for you.

Comment 17

[Robert Helmer [:rhelmer]]

I think to make the whole thing cross-platform (including the "-t" mode) will require more than we can do with wget and bash, and implies bug 373995.