1415557 - Update verify tests: pass OS versions to balrog URLs

Status: RESOLVED FIXED

(Release Engineering :: Release Automation, defect)

Description

[Johan Lorenzo [:jlorenzo]]

57.0 has a lot of rules[1] that depend on what OS Firefox runs on. At the moment, UV tests are failing, because the URL used defines "default" as the OS version[2]. "default" is not a supported keyword in our balrog rules. Then we need to change this URL[3] to be either Linux, Darwin, or Windows_NT (instead of default).

[1] https://docs.google.com/spreadsheets/d/17u3x092rGPB0rTwManlJ3nN2WdNwy2DgXifW8RwtpFA/edit#gid=0
[2] For instance:  https://aus5.mozilla.org/update/3/Firefox/55.0.3/20170824053838/Darwin_x86_64-gcc3-u-i386-x86_64/sr/release-localtest/default/default/default/update.xml?force=1 . The OS Version is the "default" right after "release-localtest"[2a].
[2a] https://github.com/mozilla/balrog/blob/814a66e17b57f522a1259da4052cc6bc5cb8311e/auslib/web/public/swagger/api.yml#L73
[3] https://github.com/mozilla/build-tools/blob/409cce5a0aa943b52dbfb4c67c824bc561806fc5/release/updates/verify.sh#L127

Comment 1

[[e-mail] (:bhearsum)]

Attachment: set OS_VERSION in update verify; update release configs to remove watersheds

This is a hack, but it seems to work. I haven't been able to test on OS X or Windows (because they run in buildbot, and I can't force them to use my tools repo).

Because OS_VERSION causes us to hit different rules in Balrog, I've had to alter the configs to remove all of the Linux & Windows previous versions locked behind a watershed (eg: everything below 56.0 on windows stops at 56.0). This patch is probably also going to cause other branches to fail similarly, so my suggestion is to land this, re-tag for the release you want to test (eg: FIREFOX_57_0_RELEASE), and backout immediately. This will need to be done for each RC, too.

Comment 2

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8926454 [details] [diff] [review]
set OS_VERSION in update verify; update release configs to remove watersheds

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

Good to see some old update paths cleaned up. r+ modulo the question on 55.0.3.

::: release/updates/release-firefox-win32.cfg
@@ -1,4 @@
>  release="56.0.2" product="Firefox" platform="WINNT_x86-msvc-x86" build_id="20171024165158" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/56.0.2/win32/%locale%/Firefox Setup 56.0.2.exe" aus_server="https://aus5.mozilla.org" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub" to="/firefox/candidates/57.0-candidates/build1/win32/%locale%/Firefox Setup 57.0.exe" to_build_id="20171106194249" to_display_version="57.0" to_app_version="57.0"
>  release="56.0.1" product="Firefox" platform="WINNT_x86-msvc-x86" build_id="20171002220106" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da dsb el en-GB en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete"
>  release="56.0" product="Firefox" platform="WINNT_x86-msvc-x86" build_id="20170926190823" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/56.0/win32/%locale%/Firefox Setup 56.0.exe" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub"
> -release="55.0.3" product="Firefox" platform="WINNT_x86-msvc-x86" build_id="20170824053622" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/55.0.3/win32/%locale%/Firefox Setup 55.0.3.exe" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub" to_build_id="20170926190823" to_display_version="56.0" to_app_version="56.0"

Should we keep this one and make it point to 56.0? We changed that balrog rule to apply to Windows only. It's a good sanity check to do, IMO.

::: release/updates/release-firefox-win64.cfg
@@ -1,4 @@
>  release="56.0.2" product="Firefox" platform="WINNT_x86_64-msvc-x64" build_id="20171024165158" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/56.0.2/win64/%locale%/Firefox Setup 56.0.2.exe" aus_server="https://aus5.mozilla.org" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub" to="/firefox/candidates/57.0-candidates/build1/win64/%locale%/Firefox Setup 57.0.exe" to_build_id="20171106194249" to_display_version="57.0" to_app_version="57.0"
>  release="56.0.1" product="Firefox" platform="WINNT_x86_64-msvc-x64" build_id="20171002220106" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da dsb el en-GB en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete"
>  release="56.0" product="Firefox" platform="WINNT_x86_64-msvc-x64" build_id="20170926190823" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/56.0/win64/%locale%/Firefox Setup 56.0.exe" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub"
> -release="55.0.3" product="Firefox" platform="WINNT_x86_64-msvc-x64" build_id="20170824053622" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/55.0.3/win64/%locale%/Firefox Setup 55.0.3.exe" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub" to_build_id="20170926190823" to_display_version="56.0" to_app_version="56.0"

Same comment as win32

::: release/updates/verify.sh
@@ +105,5 @@
>  
> +  os_version="default"
> +  case `echo $platform | cut -c-12` in
> +    Linux*)
> +      os_version="Linux%204.4.0-83-generic%20(GTK 3.18.9,libpulse 8.0.0)"

Already discussed on IRC: Should we make this os_version more generic? I'm afraid we may write a rule for GTK, and the test will fire back at us.

That's not a blocker for 57.0, though.

Comment 3

[[e-mail] (:bhearsum)]

(In reply to Johan Lorenzo [:jlorenzo] from comment #2)
> Comment on attachment 8926454 [details] [diff] [review]
> set OS_VERSION in update verify; update release configs to remove watersheds
> 
> Review of attachment 8926454 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Good to see some old update paths cleaned up. r+ modulo the question on
> 55.0.3.
> 
> ::: release/updates/release-firefox-win32.cfg
> @@ -1,4 @@
> >  release="56.0.2" product="Firefox" platform="WINNT_x86-msvc-x86" build_id="20171024165158" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/56.0.2/win32/%locale%/Firefox Setup 56.0.2.exe" aus_server="https://aus5.mozilla.org" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub" to="/firefox/candidates/57.0-candidates/build1/win32/%locale%/Firefox Setup 57.0.exe" to_build_id="20171106194249" to_display_version="57.0" to_app_version="57.0"
> >  release="56.0.1" product="Firefox" platform="WINNT_x86-msvc-x86" build_id="20171002220106" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da dsb el en-GB en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete"
> >  release="56.0" product="Firefox" platform="WINNT_x86-msvc-x86" build_id="20170926190823" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/56.0/win32/%locale%/Firefox Setup 56.0.exe" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub"
> > -release="55.0.3" product="Firefox" platform="WINNT_x86-msvc-x86" build_id="20170824053622" locales="ach af an ar as ast az be bg bn-BD bn-IN br bs ca cak cs cy da de dsb el en-GB en-US en-ZA eo es-AR es-CL es-ES es-MX et eu fa ff fi fr fy-NL ga-IE gd gl gn gu-IN he hi-IN hr hsb hu hy-AM id is it ja ka kab kk km kn ko lij lt lv mai mk ml mr ms my nb-NO nl nn-NO or pa-IN pl pt-BR pt-PT rm ro ru si sk sl son sq sr sv-SE ta te th tr uk ur uz vi xh zh-CN zh-TW" channel="release-localtest" patch_types="complete partial" from="/firefox/releases/55.0.3/win32/%locale%/Firefox Setup 55.0.3.exe" ftp_server_from="https://archive.mozilla.org/pub" ftp_server_to="https://archive.mozilla.org/pub" to_build_id="20170926190823" to_display_version="56.0" to_app_version="56.0"
> 
> Should we keep this one and make it point to 56.0? We changed that balrog
> rule to apply to Windows only. It's a good sanity check to do, IMO.

This won't work, unfortunately. "to" is a "global" key to the code that does the chunking, so you can't have different values per line (https://hg.mozilla.org/build/tools/file/tip/lib/python/release/updates/verify.py#l17).

The purpose of update verify is to ensure that the MARs apply correctly, so I don't think it's important that we tested updates *to* 56.0 in it. My update status checks should verify the <=55.0.3 -> 56.0 update paths (and we verified that those MARs apply correctly when we shipped 56.0).

> ::: release/updates/verify.sh
> @@ +105,5 @@
> >  
> > +  os_version="default"
> > +  case `echo $platform | cut -c-12` in
> > +    Linux*)
> > +      os_version="Linux%204.4.0-83-generic%20(GTK 3.18.9,libpulse 8.0.0)"
> 
> Already discussed on IRC: Should we make this os_version more generic? I'm
> afraid we may write a rule for GTK, and the test will fire back at us.
> 
> That's not a blocker for 57.0, though.

Let's postpone this discussion :). I think it is related to other discussions about what we want to do with overall update testing.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 8926454 [details] [diff] [review]
set OS_VERSION in update verify; update release configs to remove watersheds

➜  tools hg out -r upstream ssh://hg.mozilla.org/build/tools
comparing with ssh://hg.mozilla.org/build/tools
searching for changes
changeset:   8100:a35b0a76164b
tag:         FIREFOX_57_0_BUILD1
tag:         FIREFOX_57_0_RELEASE
parent:      8096:9a582d523507
user:        Ben Hearsum <[e-mail]>
date:        Wed Nov 08 12:52:54 2017 -0500
summary:     bug 1415557: set OS_VERSION in update verify; update release configs to remove watersheds. r=jlorenzo

changeset:   8101:60ffcfd939d9
user:        Ben Hearsum <[e-mail]>
date:        Wed Nov 08 12:53:08 2017 -0500
summary:     Added tag FIREFOX_57_0_BUILD1 for changeset a35b0a76164b

changeset:   8102:bb8713ddb389
user:        Ben Hearsum <[e-mail]>
date:        Wed Nov 08 12:53:11 2017 -0500
summary:     Added tag FIREFOX_57_0_RELEASE for changeset a35b0a76164b

changeset:   8103:932786cb9755
bookmark:    upstream
tag:         tip
user:        Ben Hearsum <[e-mail]>
date:        Wed Nov 08 12:54:11 2017 -0500
summary:     Backout bug 1415557.

➜  tools hg push -r upstream ssh://hg.mozilla.org/build/tools
pushing to ssh://hg.mozilla.org/build/tools
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 4 changesets with 12 changes to 6 files
remote: recorded push in pushlog
remote: 
remote: View your changes here:
remote:   https://hg.mozilla.org/build/tools/rev/a35b0a76164bd1b6f6cfef4049e3f3b06f87bc46
remote:   https://hg.mozilla.org/build/tools/rev/60ffcfd939d98be9b7c76024c2ebd6b7b1b59e96
remote:   https://hg.mozilla.org/build/tools/rev/bb8713ddb389b28a374629b4a9dfe827687a40f4
remote:   https://hg.mozilla.org/build/tools/rev/932786cb9755d72b92b130607902d8b4c01326e8
remote: recorded changegroup in replication log in 0.002s

Comment 5

[[e-mail] (:bhearsum)]

Attachment: set OS_VERSION & SYSTEM_CAPABILITIES

The previous patch mostly worked - except it ended up uncovering a subtle bug in Balrog (https://bugzilla.mozilla.org/show_bug.cgi?id=1415649). The problem occurs because update verify sends what is not really a valid update URL -- it uses /update/3 with 56.0.1, which means SYSTEM_CAPABILITIES is not in the update URL. Because some of the Rules we're using are looking for JAWS and memory fields, we end up with an internal server error.

We can workaround this for now by using /update/6 for update verify. As with the previous patch, my intent is to land, retag, and backout immediately.

Comment 6

[[e-mail] (:bhearsum)]

With my second patch we managed to get full green update verify for 57.0 RC1. We'll need to do another land/tag/backout cycle for any additional RCs. The update verify config diffs probably won't apply correctly - but the removals are easy enough to do by hand.

Comment 7

[Jordan Lund (:jlund)]

bhearsum, jlorenzo, do you think we can land this permanently? It's quite cumbersome for releaseduty and delays releases landing this during a dot release. Open to any suggestions.

Do you think there will be conflicts during release automation tools bump? Will ESR, beta, and devedition support it?

Comment 8

[[e-mail] (:bhearsum)]

(In reply to Jordan Lund (:jlund) from comment #7)
> bhearsum, jlorenzo, do you think we can land this permanently? It's quite
> cumbersome for releaseduty and delays releases landing this during a dot
> release. Open to any suggestions.
> 
> Do you think there will be conflicts during release automation tools bump?
> Will ESR, beta, and devedition support it?

The update verify config parts can't be landed permanently - we regenarate those files out of the patcher configs from scratch for each release, and they differ substantially after each version bump. The right way to fix this part is to adjust the patcher-configs. For example, if 45.0.1 and earlier shouldn't be tested on Linux, it should be removed from the platforms block for all of those releases (https://github.com/mozilla/build-tools/blob/34e32842f1a20a08a33fada77a530ce14272e4f2/release/patcher-configs/mozRelease-branch-patcher2.cfg#L167).

The verify.sh part could probably be landed - but keep in mind that it will apply to _all_ update verify tests, not just mozilla-release. It looks generic enough that it shouldn't cause issues on other branches, but I can't make any guarantees. The correct fix for this is to move os_version and system_capabilities into the update verify config (which would mean smartening up the creation script, and possibly adding some extra data into the patcher configs). I'm not sure it's worthwhile to do this when we're planning to kill patcher configs so soon in the future - that might be a more appropriate time to fix this more robustly.

Comment 9

[Jordan Lund (:jlund)]

Thanks for the context!

I think this is a good point about not diving too deep while patcher configs is dying.

We talked a lot about this today. For 58.0, there is three obvious solutions:

1) land tools patch after 58.0 gtb like 57.*. The hope here being that for the 59 cycle, we have an easier means to fix proper.
2) remove the localtest rules that confuse UV and instead
3) fix patcher configs the correct way via what's described in comment 8

At this point, for 58.0 at least POA is to go for (1). For (3), because patcher configs is being killed, it's likely not worth the effort. And with (2), it is likely still beneficial to validate that versions can apply to updates/watersheds that they are expected to get, even if we are verifying that the artifacts are still available.

Nick is going to quickly investigate that the reasons against (2) and (3) are valid by looking more into the patch and logic.

There are other improvements we can make to help take some burden off releaseduty. We are going to remove WNP blobs from localtest. UV doesn't test that anyway and if we include the wnp rules, it breaks UV as they point to previous release unless we update after artifacts complete. This will solve at least some of the hot fixing we need to do for UV to pass. For WNP, we will rely on setting cdntest up for ben's script, and QA.

Comment 10

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

tl;dr - I think we can get to 'green u.v. on the first try' using a mixture of approaches
 a) handling WNP differently
 b) adjusting the patcher config for the watersheds
 c) not use a manual tools patch landing

verbosity -
I revisited the update verify logs for 57.0.2 build2 and didn't find quite what I expected - all the failures on the Run 0's had the error
  FAIL: expected buildID 20171206182557 does not match actual 20171128222554
Spot checks of the update.xml showed a WNP and 57.0.1, instead of 57.0.2. Didn't look at every single log but it seems to be consistently WNP rules rather than JAWS/win64 migration etc. Why ?

Looking a bit deeper, in Balrog if the request does not send JAWS:0/1 and it will default to value of None, and rules which care about JAWS:1 will not match. eg [1] and [2] both yield 57.0.4 just now. Similar seems to hold for win64. That leaves the OS version changes in https://hg.mozilla.org/build/tools/diff/101b7de4f015/release/updates/verify.sh, but we have to go back to WinXP migration to ESR, websense rules, and OSX 10.6-10.8 deprecation before those come into play, back in the late 40s/early 50 versions. Those didn't interfere with earlier testing IIRC.

So we sidestep all the special rules except WNP. Possible solutions there are 
 A) removing WNP rules from release-localtest. If we do this it may be wise to add then back to localtest before we get as far as cdntest, so we can run benScript earlier and possibly for QE testing. Have to remove them again ahead of any build2+ though!
 B) bug 1400016 moves the WNP logic into main release blob, so we only have one rule which the automation already updates
 C) we add new aliases in balrog for the WNP rules, and put them in rules_to_update to update at [3]. The automation will point the WNP rules to Firefox-<VERSION>-buildN when the updates job runs. It won't have the WNP data yet so we'll still have to manually add that release and update the rules, but I don't think it invalidates u.v. tests.

The other part of the tools patch has been truncating the versions checked. That'll be different for 58.0 because we are adding a watershed at 57.0.4 for Linux & Mac (bug 1429283). The windows watershed at 56.0 is unchanged. Ben's suggestion of modifying the platforms in the patcher configs would result in automated, shorter u.v. configs, no intervention required during a release. Firmly in favour of this approach. 

We lose a checks that the watershed updates are available, hashes agree, etc but that's been SOP. We could try some final verification runs using older revs of u.v configs - 57.0.4 for mac and linux, 56.0 for win32 & win64. That'd check urls aren't 404s and file sizes match, but not check hashes. I'd wondered if we could just rerun jobs in the old graphs but different versions per platform prevents that, but the script is portable and fairly quick to run. We could automate that easily too, with a config which specifies a tag and configs for each watershed.


[1] https://aus5.mozilla.org/update/6/Firefox/56.0/20170926190823/WINNT_x86-msvc-x64/de/release-localtest/Windows_NT%2010.0.0.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:3072,JAWS:0/default/default/update.xml?force=1&mig64=1
[2] https://aus5.mozilla.org/update/3/Firefox/56.0/20170926190823/WINNT_x86-msvc-x64/de/release-localtest/Windows_NT%2010.0.0.0%20(x64)(noBug1296630v1)(nowebsense)/default/default/update.xml?force=1
[3] https://hg.mozilla.org/releases/mozilla-release/file/default/testing/mozharness/configs/releases/updates_firefox_release.py#l43

Comment 11

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

URLs that actually make my point (64bit native build instead of 64-on-32)

[1] https://aus5.mozilla.org/update/6/Firefox/56.0/20170926190823/WINNT_x86_64-msvc-x64/de/release/Windows_NT%2010.0.0.0%20(x64)(noBug1296630v1)(nowebsense)/ISET:SSE3,MEM:3072,JAWS:0/default/default/update.xml?force=1&mig64=1
[2] https://aus5.mozilla.org/update/3/Firefox/56.0/20170926190823/WINNT_x86_64-msvc-x64/de/release/Windows_NT%2010.0.0.0%20(x64)(noBug1296630v1)(nowebsense)/default/default/update.xml?force=1

Comment 12

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

Or even sending default for OS Version, which is what this bug was originally filed for
https://aus5.mozilla.org/update/6/Firefox/56.0/20170926190823/WINNT_x86_64-msvc-x64/de/release/default/ISET:SSE3,MEM:3072,JAWS:0/default/default/update.xml?force=1
https://aus5.mozilla.org/update/3/Firefox/56.0/20170926190823/WINNT_x86_64-msvc-x64/de/release/default/default/default/update.xml?force=1

Confused again now :/

Comment 13

[[e-mail] (:bhearsum)]

(In reply to Nick Thomas [:nthomas] (UTC+13) from comment #12)
> Or even sending default for OS Version, which is what this bug was
> originally filed for
> https://aus5.mozilla.org/update/6/Firefox/56.0/20170926190823/WINNT_x86_64-
> msvc-x64/de/release/default/ISET:SSE3,MEM:3072,JAWS:0/default/default/update.
> xml?force=1
> https://aus5.mozilla.org/update/3/Firefox/56.0/20170926190823/WINNT_x86_64-
> msvc-x64/de/release/default/default/default/update.xml?force=1
> 
> Confused again now :/

If I've understood correctly, the confusion is around why we've been appending different OS_VERSIONs since both of the above URLs are getting 57.0.4 already? If so, I'm confused too. 

Without a way to rewind the entire Rules table (or an improved bhearsum-memory), it's hard to be certain the state we were in when we first did this with 57.0, but it appears that we were also failing because of WNP rules in 57.0build1, with failures such as:
Got this response:
<?xml version="1.0"?>
<updates>
    <update type="minor" displayVersion="56.0.2" appVersion="56.0.2" platformVersion="56.0.2" buildID="20171024165158" detailsURL="https://www.mozilla.org/ach/firefox/56.0.2/releasenotes/" actions="showURL" openURL="https://www.mozilla.org/ach/firefox/56.0.2/whatsnew/?oldversion=%OLD_VERSION%">
        <patch type="complete" URL="http://archive.mozilla.org/pub/firefox/candidates/56.0.2-candidates/build1/update/linux-i686/ach/firefox-56.0.2.complete.mar" hashFunction="sha512" hashValue="490ec66c1c1db82311e457c913669a06e730b193f571a4d122e6829686f7f20f44e147552fc03f9a352b5bba5b202f7c635a27ad62815595dadddd293511354e" size="43696649"/>
        <patch type="partial" URL="http://archive.mozilla.org/pub/firefox/candidates/56.0.2-candidates/build1/update/linux-i686/ach/firefox-56.0-56.0.2.partial.mar" hashFunction="sha512" hashValue="8371668630c77a35b496f8a24107792ec106a5cd20a7dc22ca283db3013db16b463e1a5b91570845e002378548af240212340402268d909faff39a4c97fe56fc" size="4751472"/>
    </update>
</updates>

FAIL: expected buildID 20171106194249 does not match actual 20171024165158


I see lots of rule changes around the time (and shortly after) the failures, which makes me wonder if my patch didn't actually fix anything (but rather, the changes to update rules that happened between the failures & the patch landing). Without evidence to the contrary, that's my best guess.

Comment 14

[Jordan Lund (:jlund)]

(In reply to Nick Thomas [:nthomas] (UTC+13) from comment #10)

Thanks for poking!

> So we sidestep all the special rules except WNP. Possible solutions there
> are 
>  A) removing WNP rules from release-localtest. If we do this it may be wise
> to add then back to localtest before we get as far as cdntest, so we can run
> benScript earlier and possibly for QE testing. Have to remove them again
> ahead of any build2+ though!

For simplicity, I did this. However, I just lowered the priority, range form 5-8, so that we can easily move them back in the event we want to test with benscript before cdn push.

> The other part of the tools patch has been truncating the versions checked.
> That'll be different for 58.0 because we are adding a watershed at 57.0.4
> for Linux & Mac (bug 1429283). The windows watershed at 56.0 is unchanged.
> Ben's suggestion of modifying the platforms in the patcher configs would
> result in automated, shorter u.v. configs, no intervention required during a
> release. Firmly in favour of this approach. 
> 

I'm a little unclear what we need to do for 58.0. Do we need a patcher config patch post gtb that truncates certain versions? Do we expect linux/mac <56.0 clients to fail if they are watershed at 57.0.4-bz2?

Comment 15

[Jordan Lund (:jlund)]

(In reply to Jordan Lund (:jlund) from comment #14)
> (In reply to Nick Thomas [:nthomas] (UTC+13) from comment #10)
> 
> > The other part of the tools patch has been truncating the versions checked.
> > That'll be different for 58.0 because we are adding a watershed at 57.0.4
> > for Linux & Mac (bug 1429283). The windows watershed at 56.0 is unchanged.
> > Ben's suggestion of modifying the platforms in the patcher configs would
> > result in automated, shorter u.v. configs, no intervention required during a
> > release. Firmly in favour of this approach. 
> > 
> 
> I'm a little unclear what we need to do for 58.0. Do we need a patcher
> config patch post gtb that truncates certain versions? Do we expect
> linux/mac <56.0 clients to fail if they are watershed at 57.0.4-bz2?

bhearsum: do you have thoughts on my question at the end of https://bugzilla.mozilla.org/show_bug.cgi?id=1415557#c14
09:20:20 
<bhearsum> jlund: if we want automated update-verify configs that exclude versions that would fail, that means modifying the patcher-configs ahead of the updates builder running

I'll start looking into this.

Comment 16

[Jordan Lund (:jlund)]

Attachment: 1415557_update_verify_57_56_watersheds.patch

This is a stop gap solution for 58.0. I'll make a follow up patch to fix this through automation.

Essentially:

* removes all version <57.0.4 for linux/mac
* removes all versions <56.0 for win

Both 57.0.4 and 56.0 are watersheds for those platforms.

Feels weird deleting most lines! On the other hand, UV test times should improve :)

Comment 17

[Jordan Lund (:jlund)]

https://hg.mozilla.org/build/tools/rev/00b1e92b259cfa096697b313589f62c08f9450db
Bug 1415557 -removes <57.0.4 versions from linux/mac, <56.0 from windows to reflect current watersheds, r=nthomas

Comment 18

[Jordan Lund (:jlund)]

https://hg.mozilla.org/build/tools/rev/c39b3fa25721ee76d8e9825c58bece9eefb3e162
Bug 1415557 -removes <57.0.4 versions from linux/mac, <56.0 from windows to reflect current watersheds, relanding, r=bustage

Comment 19

[[e-mail] (:bhearsum)]

(In reply to Jordan Lund (:jlund) from comment #15)
> (In reply to Jordan Lund (:jlund) from comment #14)
> > (In reply to Nick Thomas [:nthomas] (UTC+13) from comment #10)
> > 
> > > The other part of the tools patch has been truncating the versions checked.
> > > That'll be different for 58.0 because we are adding a watershed at 57.0.4
> > > for Linux & Mac (bug 1429283). The windows watershed at 56.0 is unchanged.
> > > Ben's suggestion of modifying the platforms in the patcher configs would
> > > result in automated, shorter u.v. configs, no intervention required during a
> > > release. Firmly in favour of this approach. 
> > > 
> > 
> > I'm a little unclear what we need to do for 58.0. Do we need a patcher
> > config patch post gtb that truncates certain versions? Do we expect
> > linux/mac <56.0 clients to fail if they are watershed at 57.0.4-bz2?
> 
> bhearsum: do you have thoughts on my question at the end of
> https://bugzilla.mozilla.org/show_bug.cgi?id=1415557#c14
> 09:20:20 
> <bhearsum> jlund: if we want automated update-verify configs that exclude
> versions that would fail, that means modifying the patcher-configs ahead of
> the updates builder running
> 
> I'll start looking into this.

I'm not sure how far you got - but I just filed https://bugzilla.mozilla.org/show_bug.cgi?id=1432802 with a patch. (Sorry if I stepped on your toes - I'd forgotten you were going to look into it until I stumbled on this bug again.)