408453 - update verify should do some checks from all specified releases (rather than just all the previous one)

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P5)

Description

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

The release automation handles creating updates from previous versions, but it only tests *some* of the update paths. This came up again just today, so this bug is to track fixing this.

When doing release "n", we compare release "n-1" with release "n", and generate both complete updates and partial updates, so we can update users from "n-1" to "n". We also generate update mar files for "n-2" to "n", "n-3" to "n", ...etc. We call this "generating updates".

After we generate all these updates, we then need to test them. We do this by taking release "n-1", applying the updates giving us, in theory, release "n"... and we then compare this to a clean release "n". We call this "verifying updates". 

So far so good.

The problem is that we only "verify updates" for n-1 to n. We do not verify updates for n-2 to n, n-3 to n, ...etc.

These older update combinations need to be verified. Because verification is slow, it might help to do this in parallel.

Comment 1

[Robert Helmer [:rhelmer]]

bug 408443 (if it turns out to be valid) really puts a fine point on this.

update verify currently takes about 30 minutes on the slowest platform (win32) to verify that "n-1" plus update equals "n". This tests both partial and complete MARs; since all previous releases  get complete MARs only it might be slightly faster per-release for "n-2" and above but probably not very much.

update verify happens at the end of the release process and since it doesn't produce anything, no steps directly depend on it. It'd be pretty easy to make it do full verification for all previous releases and then worry about optimizing it afterwards; QA could choose to wait for full results or they could just wait for "n-1" results and we would be better off than we are today.

Comment 3

[juan becerra [:juanb]]

What are some of the types of updates failures we have found despite verification? How is our current process including manual spot checks addressing these failures, or not? For example, if updates work this cycle, say n-1, do we need to spot check these again next cycle? Can we trust that if the verification process passes it would be OK if QA doesn't do any manual spot checks at all?

Perhaps we could go over what the verification script does now, again, and what it could do in the near future in order to maintain a good level of assurance while avoiding manual spot checks as much as possible.

Comment 4

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

Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.

Comment 5

[[e-mail] (:bhearsum)]

This isn't going to be practical machine-time wise until we parallelize update_verify IMHO.

Comment 6

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

The only situation that springs to mind where n-2 -> n might fail is where we removed a file but didn't update removed-files.in. The partial for n-2 > to n-1 will have a remove instruction, but anyone who doesn't take that update and later gets a n-2 -> n complete might have issues.

Point releases tend to be  additive, if anything, so this doesn't come up very often. I'll see if I can figure out any other error cases, but this feels like a lot of computation for little gain.

Bug 527670 could help dealing with the output saner.

Comment 7

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

Need parallel update verify for this, bug 589251. Also kinda depends on how AUS3, it may make sense to ask AUS3 to provide a list of known releases in order to track less data in our config files.

Comment 8

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

Some of bug 527670 wouldn't hurt either.

Comment 9

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

joduinn, given this is an old bug and is effectively blocked by adding parallelism to update verify (a non-old bug, 589251), would it be acceptable to reduce the scope to test the top N locales for each old release ? For small value of N like 3.

Comment 10

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to comment #9)
> joduinn, given this is an old bug and is effectively blocked by adding
> parallelism to update verify (a non-old bug, 589251), would it be acceptable to
> reduce the scope to test the top N locales for each old release ? For small
> value of N like 3.

WFM. Once we get bug#589251 resolved, we can revisit this later in another bug for the other less-popular locales.

Comment 11

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

OK, morphing. 

The fun bit will be figuring out how to create and update the verify configs. We will now need 
* an extra line that applies the mars and diffs for the top N locales, like we do for the previous release
* still have the existing line that does the mar check for old releases
* the existing line that does the full check for the previous release
update-verify-bump.pl
Might make sense to have two configs for this purpose, and possibly do a poor-mans parallelisation by running them on different builders. update-verify-bump.pl will need changes. Also need to determine the top N locales; the transition from en-US only to en-US+locales might be tricky.

Comment 12

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

Top 5 locales for each branch:
Fx4.0   en-US, de, pt-BR, ru, es-ES
Fx3.6   en-US, de, es-ES, ru, fr
Fx3.5   en-US, de,    fr, ru, es-ES

So if we were to pick a top 3 to cover all branches it'd be en-US, de, ru (or maybe es-ES).

Comment 13

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

Attachment: [tools] Rough sketch

Here's a very rough sketch on how this could go if someone wants to take a stab at it.

*) we modify the existing update verify jobs to do
  *) applying mar for partial & complete for the previous release (unchanged0
  *) applying mar for (fake) partial & complete for older releases for 3 locales (new)
  *) downloading mar and checking hash and size for older releases for other locales (locale list changes)
  See the 3.6.11 example in moz192-firefox-win32.cfg

*) in verify.sh let the 'to' definition persist instead of wiping it for every config line. This avoids having to have a repeated definition in the config

*) update-verify-bump.pl (this is very rough, untested) - when bumping for a new version create the two sets of lines for the previous one. It'll regress bug 514040, might have to move patch_types after from in the list of parameters, or something.

Comment 14

[[e-mail] (:bhearsum)]

I volunteered to take this from Nick, I should be able to finish it up this quarter.

Comment 15

[[e-mail] (:bhearsum)]

Found a small bug in testing today: sometimes newlines are errantly inserted, like in this diff:
http://hg.mozilla.org/users/bhearsum_mozilla.com/tools/rev/d7395ab966af

Other than that, it seems to be working fine. So, next steps are:
- Debug/fix above bug
- Edit existing update verify configs to start testing older releases
- Full staging run
- Review/Landing

Comment 16

[[e-mail] (:bhearsum)]

Attachment: same as nick's patch, without verify config changse

This is the same as Nick's original patch, without the update verify changes. I'm r+'ing this based on my testing. I'll be attaching a separate patch with the config changes.

Comment 17

[[e-mail] (:bhearsum)]

Attachment: enable full tests for de, en-US, ru, for most releases

This patch enables full update verify tests for all old releases on 1.9.1, 1.9.2, and 2.0 except:
- Alphas
- 3.1a1 Mac (because the EULA prevents it from being installed with the current DMG unpack script)

I didn't touch the mozBeta configs because Nick is emptying them out in https://bugzilla.mozilla.org/show_bug.cgi?id=654507

Comment 18

[[e-mail] (:bhearsum)]

Attachment: script used to modify verify configs

Here's the script I used to bootstrap these configs. I had to modify a few things afterwards (expanding beta filenames for Mac and Windows, in particular).

Comment 19

[[e-mail] (:bhearsum)]

Attachment: don't modify mozBeta configs (for real)

Same as before, except this version actually *does* leave the mozBeta files untouched.

Comment 20

[[e-mail] (:bhearsum)]

Comment on attachment 531729 [details] [diff] [review]
same as nick's patch, without verify config changse

Landed: changeset:   1445:afe21a61d888

Comment 21

[[e-mail] (:bhearsum)]

Comment on attachment 533350 [details] [diff] [review]
don't modify mozBeta configs (for real)

Landed: changeset:   1446:be5e928209a3

Comment 22

[[e-mail] (:bhearsum)]

All done here.

Comment 23

[[e-mail] (:bhearsum)]

Somehow I managed to not test the update-verify-bump.pl patch, despite claiming I did. It's riddled with issues. Patch to fix is incoming soon.

Comment 24

[[e-mail] (:bhearsum)]

Attachment: fix update verify bumper to work with additional tests

This patch fixes the update verify script to properly add full and quick checks for the past releases. The regexes got a bit more complicated in order to cope with the corner cases of the locales either not existing, being at the start, the end, or being the only locale in a list.

Comment 25

[[e-mail] (:bhearsum)]

Comment on attachment 535507 [details] [diff] [review]
fix update verify bumper to work with additional tests

This patch actually has issues, will repost when I get them sorted out.

Comment 26

[[e-mail] (:bhearsum)]

Attachment: v2, create full & quick tests properly

I re-run the update verify part of updates with this patch, the results of which are here: http://hg.mozilla.org/users/bhearsum_mozilla.com/tools/rev/18d14ee021b3. I've got update verify about halfway finished at this point, you can view the results on my staging master if you'd like (http://staging-master.build.mozilla.org:8018/builders). Note that Mac and Windwos partials fail due to my staging run using the old UPDATE_PACKAGING_R14 that didn't have bugs 658094 or 386760 in it.

I also ran the scripts locally, simulating 4.0.1. Here's the diff against http://hg.mozilla.org/build/tools/raw-file/38d1f7ef1591/release/updates/moz20-firefox-linux.cfg
index 480340b..be8c424 100644
--- a/release/updates/moz20-firefox-linux.cfg
+++ b/release/updates/moz20-firefox-linux.cfg
@@ -1,5 +1,9 @@
-# 4.0rc1 linux
-release="4.0" product="Firefox" platform="Linux_x86-gcc3" build_id="20110303140816" locales="af ak ar ast be bg bn-BD bn-IN br bs ca cs cy da de el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa fi fr fy-NL ga-IE gd gl gu-IN he hi-IN hr hu hy-AM id is it ja kk kn ko ku lg lt lv mai mk ml mr nb-NO nl nn-NO nso or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta ta-LK te th tr uk zh-CN zh-TW zu" channel="betatest" patch_types="partial complete" from="/firefox/releases/4.0rc1/linux-i686/%locale%/firefox-4.0rc1.tar.bz2" aus_server="https://aus2.mozilla.org" ftp_server="stage-old.mozilla.org/pub/mozilla.org" to="/firefox/nightly/4.0rc2-candidates/build3/linux-i686/%locale%/firefox-4.0rc2.tar.bz2"
+# 4.0rc2 linux
+release="4.0" product="Firefox" platform="Linux_x86-gcc3" build_id="20110318052756" locales="af ak ar ast be bg bn-BD bn-IN br bs ca cs cy da de el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa fi fr fy-NL ga-IE gd gl gu-IN he hi-IN hr hu hy-AM id is it ja kk kn ko ku lg lt lv mai mk ml mr nb-NO nl nn-NO nso or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta ta-LK te th tr uk vi zh-CN zh-TW zu" channel="betatest" patch_types="partial complete" from="/firefox/releases/4.0rc2/linux-i686/%locale%/firefox-4.0rc2.tar.bz2" aus_server="https://aus2.mozilla.org" ftp_server="stage-old.mozilla.org/pub/mozilla.org" to="/firefox/nightly/4.0.1-candidates/build1/linux-i686/%locale%/firefox-4.0.1.tar.bz2"
+# 4.0rc1 linux - full check
+release="4.0" product="Firefox" platform="Linux_x86-gcc3" build_id="20110303140816" locales="de en-US ru" channel="betatest" patch_types="partial complete" from="/firefox/releases/4.0rc1/linux-i686/%locale%/firefox-4.0rc1.tar.bz2" 
+# 4.0rc1 linux - quick check
+release="4.0" product="Firefox" platform="Linux_x86-gcc3" build_id="20110303140816" locales="af ak ar ast be bg bn-BD bn-IN br bs ca cs cy da el en-GB en-ZA eo es-AR es-CL es-ES es-MX et eu fa fi fr fy-NL ga-IE gd gl gu-IN he hi-IN hr hu hy-AM id is it ja kk kn ko ku lg lt lv mai mk ml mr nb-NO nl nn-NO nso or pa-IN pl pt-BR pt-PT rm ro si sk sl son sq sr sv-SE ta ta-LK te th tr uk zh-CN zh-TW zu" channel="betatest" patch_types="partial complete" 
 # 4.0b12 linux
 release="4.0b12" product="Firefox" platform="Linux_x86-gcc3" build_id="20110222205441" locales="af ak ar ast be bg bn-BD bn-IN br bs ca cs cy da de el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fi fr fy-NL ga-IE gd gl gu-IN he hi-IN hr hu hy-AM id is it ja kk kn ko ku lg lt lv mai mk ml mr nb-NO nl nn-NO nso or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta ta-LK te th tr uk zh-CN zh-TW zu" channel="betatest" 
 # 4.0b11 linux

I will be adding tests, but due to hardcoding of locales in ExecuteTests it'll need some refactoring first -- and I don't have time to address that tonight. Very sorry for the amount of churn I went through on this.

Comment 27

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

Comment on attachment 535545 [details] [diff] [review]
v2, create full & quick tests properly

Looks fine to me. For clarity you could add some sort of comment prior to
 $line =~ s/aus_server.*$//;
since it's leaving 'from' behind, while the later 
 $line =~ s/from.*$//;
removes all traces of from (and to).

Comment 28

[[e-mail] (:bhearsum)]

Comment on attachment 535545 [details] [diff] [review]
v2, create full & quick tests properly

Landed this. Bug stays open because I've got to deal with the unit test situation still.

Comment 29

[[e-mail] (:bhearsum)]

Attachment: fix tests, and --test-older-partials

This patch fixes both the unit tests, and bug 660625.

I spent a bit of time trying to make the unit tests better, but the only way I could figure to is to split up the bumper into a bunch of smaller parts (which would be more easily testable). Doing that is likely to be as much work as rewriting the bumper based on the UpdateVerifyConfig class in the python library, so I decided not to.

I'll be attaching the reference configs from the first 3 tests, to hopefully give you some degree of confidence in the tests.

Comment 30

[[e-mail] (:bhearsum)]

Attachment: reference configs for non-major mode tests

Comment 31

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

Comment on attachment 536375 [details] [diff] [review]
fix tests, and --test-older-partials

lgtm!

Comment 32

[[e-mail] (:bhearsum)]

Attachment: fix newlines

Turns out the tests don't catch everything. They're perfectly valid, but don't catch bugs that only occur when we've got more than one old release in the config. Sadly, adding tests with different data is more work than it's worth.

Comment 33

[[e-mail] (:bhearsum)]

The bumps done for 5.0b4 look good: http://hg.mozilla.org/build/tools/rev/3d0dcd594391

Hesitantly, I'm calling this FIXED again.