Closed Bug 408453 Opened 12 years ago Closed 8 years ago

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

Categories

(Release Engineering :: General, defect, P5)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: joduinn, Assigned: bhearsum)

References

Details

(Whiteboard: [automation][updates][releases])

Attachments

(7 files, 3 obsolete files)

2.22 KB, patch
bhearsum
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
2.66 KB, text/plain
Details
202.87 KB, patch
catlee
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
2.28 KB, patch
nthomas
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
3.73 KB, patch
nthomas
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
2.54 KB, text/plain
Details
1.27 KB, patch
nthomas
: review+
bhearsum
: checked-in+
Details | Diff | Splinter Review
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.
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.
Priority: -- → P3
Component: Release Engineering → Release Engineering: Future
QA Contact: build → release
Duplicate of this bug: 498451
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.
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
This isn't going to be practical machine-time wise until we parallelize update_verify IMHO.
Whiteboard: [automation][updates]
Priority: P3 → P5
Summary: update verify should verify updating from all specified versions → update verify should do full check from all specified releases (rather than just the previous one)
Whiteboard: [automation][updates] → [automation][updates][releases]
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.
Assignee: nobody → nrthomas
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.
Some of bug 527670 wouldn't hurt either.
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.
(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.
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.
Summary: update verify should do full check from all specified releases (rather than just the previous one) → update verify should do some checks from all specified releases (rather than just all the previous one)
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).
Attached patch [tools] Rough sketch (obsolete) — Splinter Review
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.
I volunteered to take this from Nick, I should be able to finish it up this quarter.
Assignee: nrthomas → bhearsum
Depends on: 608011
Depends on: 589251
No longer depends on: 608011
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
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.
Attachment #497756 - Attachment is obsolete: true
Attachment #531729 - Flags: review+
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
Attachment #532952 - Flags: review?(catlee)
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).
Same as before, except this version actually *does* leave the mozBeta files untouched.
Attachment #532952 - Attachment is obsolete: true
Attachment #533350 - Flags: review?(catlee)
Attachment #532952 - Flags: review?(catlee)
Attachment #533350 - Flags: review?(catlee) → review+
Comment on attachment 531729 [details] [diff] [review]
same as nick's patch, without verify config changse

Landed: changeset:   1445:afe21a61d888
Attachment #531729 - Flags: checked-in+
Comment on attachment 533350 [details] [diff] [review]
don't modify mozBeta configs (for real)

Landed: changeset:   1446:be5e928209a3
Attachment #533350 - Flags: checked-in+
All done here.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Attachment #535507 - Flags: review?(nrthomas)
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.
Attachment #535507 - Attachment is obsolete: true
Attachment #535507 - Flags: review?(nrthomas)
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.
Attachment #535545 - Flags: review?(nrthomas)
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).
Attachment #535545 - Flags: review?(nrthomas) → review+
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.
Attachment #535545 - Flags: checked-in+
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.
Attachment #536375 - Flags: review?(nrthomas)
Comment on attachment 536375 [details] [diff] [review]
fix tests, and --test-older-partials

lgtm!
Attachment #536375 - Flags: review?(nrthomas) → review+
Attachment #536375 - Flags: checked-in+
Attached patch fix newlinesSplinter Review
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.
Attachment #536938 - Flags: review?(nrthomas)
Attachment #536938 - Flags: review?(nrthomas) → review+
Attachment #536938 - Flags: checked-in+
The bumps done for 5.0b4 look good: http://hg.mozilla.org/build/tools/rev/3d0dcd594391

Hesitantly, I'm calling this FIXED again.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 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.