785661 - Update verify failed for TB/Firefox 15 versions < 12.0

Status: RESOLVED FIXED

(Release Engineering :: Release Automation: Other, defect)

Description

[Justin Wood (:Callek)]

So, across all platforms Firefox and Thunderbird Update Verify runs failed for versions < 12.0

For Firefox this was update_verify 6/6 only, for Thunderbird this was update_verify 5/6 and 6/6

My skim [1] seems to indicate we flipped the verify configs [2] to do a full update test for releases that did not have it before (essentially re-writing every line in update verify) I suspect its a regression from the multiple partials support and the newer script to update these manifests. -- and because of that we fail since we only offer updates to 12 first and it results in us comparing a Firefox 12 install with a Firefox 15 install.

Since the code is foreign to me, I'm not going to attempt a patch, and I'd like either rail or ben to give it a peek, to see if they agree with my assessment.

[1] - http://buildbot-master12.build.scl1.mozilla.com:8001/builders/release-mozilla-release-linux_update_verify_6%2F6/builds/0
[2] - http://hg.mozilla.org/build/tools/rev/e2edfe42d9ca

Comment 1

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

Looks like the right analysis to me, with the addition that Thunderbird 10.0 is failing for en-US/de/ru because we ended up using thunderbird/releases/10.0-real/. 

Really we should just be testing what we've just generated, but at the moment we add all the versions in the <release> block in the patcher config
 http://hg.mozilla.org/build/tools/file/default/scripts/updates/create-update-verify-configs.py#l109
We could just take the versions in the past-update lines, or trim out the versions we no longer care about from <release>, depending on if you want somewhere to look up historical data without grovelling in <vcs> history.

Comment 2

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

(In reply to Nick Thomas [:nthomas] from comment #1)
> Looks like the right analysis to me, with the addition that Thunderbird 10.0
> is failing for en-US/de/ru because we ended up using
> thunderbird/releases/10.0-real/. 

This is actually non-trival to solve, because release.paths.makeReleasesDir() from tools/lib/python/paths.py has no way to know about '-real'. Maybe we'd update the completemarurl in the patcher config to point to .../thunderbird/releases/10.0-real/... and use that instead ? Would be great if we could not end up *not* pointing at candidates directories as a result of switching over, which would mean updating completemarurl for the previous release when bumping the patcher config.

Comment 3

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

Attachment: [tools] workaround fix for FF & TB 15

Ditch anything older than 10.0, and fix up the TB 10.0 path. We'd rerun all the update verify builders after landing this because this will affect the chunking.

Comment 4

[Justin Wood (:Callek)]

Comment on attachment 655477 [details] [diff] [review]
[tools] workaround fix for FF & TB 15

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

Looks good as a temp solution.

Comment 5

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

Comment on attachment 655477 [details] [diff] [review]
[tools] workaround fix for FF & TB 15

Landed
 http://hg.mozilla.org/build/tools/rev/bdbd32407653
and moved {FIREFOX,THUNDERBIRD}_15_0_{BUILD1,RELEASE}_RUNTIME.

Comment 6

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

http://hg.mozilla.org/build/tools/rev/086309643a86 to use 10.0-real all the platforms, not just linux32.

Comment 7

[[e-mail] (:bhearsum)]

Attachment: fix the update verify dumper

OK, so the main issue here (testing versions that aren't actually updating to the latest one) seems like it's a bug in the update verify dumper, to me. PatcherConfig.getUpdatePaths thinks that only versions mentioned in a past-update line are "from" versions: https://github.com/mozilla/build-tools/blob/master/lib/python/release/updates/patcher.py#L104

The update verify dumper uses everything in releases: https://github.com/mozilla/build-tools/blob/master/scripts/updates/create-update-verify-configs.py#L109

I think the fix for this part is to factor out getUpdatePaths' calculation of "from" versions and use it in the update verify dumper.

Attached is a patch that should do that.

Comment 8

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #2)
> (In reply to Nick Thomas [:nthomas] from comment #1)
> > Looks like the right analysis to me, with the addition that Thunderbird 10.0
> > is failing for en-US/de/ru because we ended up using
> > thunderbird/releases/10.0-real/. 
> 
> This is actually non-trival to solve, because
> release.paths.makeReleasesDir() from tools/lib/python/paths.py has no way to
> know about '-real'. Maybe we'd update the completemarurl in the patcher
> config to point to .../thunderbird/releases/10.0-real/... and use that
> instead ? Would be great if we could not end up *not* pointing at candidates
> directories as a result of switching over, which would mean updating
> completemarurl for the previous release when bumping the patcher config.

This seems reasonable at face value...I'll give it a try.

Comment 9

[Rail Aliiev [:rail]]

As  a possible sollution for $version-real directories:
1) delete releases/$version directories (they are not official nor should  be synced)
2) create symlinks: $version -> $version-real to make update verify script happier
3) PROFIT (no hidden steps!)

Comment 10

[[e-mail] (:bhearsum)]

Attachment: don't exclude partials from completes

Rail pointed out on IRC that we shouldn't be excluding partials from the completes list, because there's a condition for handling that in the loop.

Comment 11

[[e-mail] (:bhearsum)]

OK, so the problem with testing the wrong versions should be fixed now. We still have to figure out what to do about -real situations. Comments 2 and 9 present two options for doing that.

Comment 12

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #2)
> (In reply to Nick Thomas [:nthomas] from comment #1)
> > Looks like the right analysis to me, with the addition that Thunderbird 10.0
> > is failing for en-US/de/ru because we ended up using
> > thunderbird/releases/10.0-real/. 
> 
> This is actually non-trival to solve, because
> release.paths.makeReleasesDir() from tools/lib/python/paths.py has no way to
> know about '-real'. Maybe we'd update the completemarurl in the patcher
> config to point to .../thunderbird/releases/10.0-real/... and use that
> instead ? Would be great if we could not end up *not* pointing at candidates
> directories as a result of switching over, which would mean updating
> completemarurl for the previous release when bumping the patcher config.

(In reply to Rail Aliiev [:rail] (off Sep 8-16) from comment #9)
> As  a possible sollution for $version-real directories:
> 1) delete releases/$version directories (they are not official nor should 
> be synced)
> 2) create symlinks: $version -> $version-real to make update verify script
> happier
> 3) PROFIT (no hidden steps!)

After a bit more thought, it seems like a combination of these two things can be our path to victory here. Nick's idea will let us be able to manually change paths to whatever they need to be (-real, -real-real, -real-real-real-fail), which requires us to:
* Make a one-off change to the patcher configs, replacing all completemarurl's with their releases/ equivalents.
* Adjust patcher-config-bump.pl to change the completemarurl of the "from" version to the releases/ equivalent.
* Adjust create-update-verify-configs.py to use completemarurl from the PatcherConfig instead of generating it on its own.

And Rail's idea will _hopefully_ ensure we don't need to make manual changes in the future. No code changes necessary for it, just documentation updates.

Nick, how does that sound to you?

Comment 13

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #2)
> (In reply to Nick Thomas [:nthomas] from comment #1)
> > Looks like the right analysis to me, with the addition that Thunderbird 10.0
> > is failing for en-US/de/ru because we ended up using
> > thunderbird/releases/10.0-real/. 
> 
> This is actually non-trival to solve, because
> release.paths.makeReleasesDir() from tools/lib/python/paths.py has no way to
> know about '-real'. Maybe we'd update the completemarurl in the patcher
> config to point to .../thunderbird/releases/10.0-real/... and use that
> instead ? Would be great if we could not end up *not* pointing at candidates
> directories as a result of switching over, which would mean updating
> completemarurl for the previous release when bumping the patcher config.

So, I was looking at doing this and I realized that completemarurl doesn't actually matter to us for update verify, because we get all MAR locations from AUS. That seems like a pretty ugly hack, though.

This makes me lean more strongly to simply doing what Rail suggests, and make sure that 10.0 symlinks to 10.0-real. I think that should be enough to fix us up here, assuming the CDN doesn't cache things forever and ever.

Another possibility would be to explicitly store the base releases directory path in the patcher config. Eg, add a 'releasesdir' directive to each <release> node. Then we'd be able to modify it by hand later if necessary.

Comment 14

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

re symlinks from 10.0 to 10.0-real, the CDN doesn't come into play for generating updates because we hit stage.m.o and that only caches for an hour. We would have to be careful with bouncer because the CDN will cache for 4 days, and without a CDN flush (or in the face of a flaky CDN flush) we'd have old content served as new. That'd result in update hash failures & people getting the wrong installers. So I think we'd still have to modify the locations in bouncer to explicitly have the -real suffix in the path. That's not such a big deal, we just have to remember to not get fooled that a symlink solves anything.

Comment 15

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] from comment #14)
> re symlinks from 10.0 to 10.0-real, the CDN doesn't come into play for
> generating updates because we hit stage.m.o and that only caches for an
> hour. We would have to be careful with bouncer because the CDN will cache
> for 4 days, and without a CDN flush (or in the face of a flaky CDN flush)
> we'd have old content served as new. That'd result in update hash failures &
> people getting the wrong installers. So I think we'd still have to modify
> the locations in bouncer to explicitly have the -real suffix in the path.
> That's not such a big deal, we just have to remember to not get fooled that
> a symlink solves anything.

Yeah, absolutely. I considering symlinking to be a fix merely for update verify. Maybe eventually we could switch the locations back, but I don't think that really matters much.

Comment 16

[[e-mail] (:bhearsum)]

So, I created the symlink to 10.0-real so update verify will work next time:
lrwxrwxrwx  1 tbirdbld thunderbird    9 Sep 14 06:12 10.0 -> 10.0-real
drwxr-xr-x 10 tbirdbld thunderbird 4096 Jan 31  2012 10.0-real

I also added docs on how to recover from this: https://wiki.mozilla.org/Release:Release_Automation_on_Mercurial:Troubleshooting#I_pushed_the_wrong_files_to_releases.2F.2C_help.21

I think that means we're all done here?