Closed
Bug 1461490
Opened 5 years ago
Closed 5 years ago
Add an allowlist of expected update verify differences
Categories
(Release Engineering :: Release Automation: Updates, defect, P1)
Release Engineering
Release Automation: Updates
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: nthomas, Assigned: nthomas)
References
Details
(Whiteboard: [releaseduty])
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
bhearsum
:
review+
nthomas
:
checked-in+
|
Details |
There are a some cases where we end up with differences after doing old+update vs new build: 1, up until 59.0b3 we had a preprocessed line in channel-prefs.js, which could change between builds. That was removed by bug 1431342. Example diff: Found diffs for complete update from https://aus5.mozilla.org/update/3/Firefox/58.0/20171127135700/Linux_x86-gcc3/en-US/beta-localtest/default/default/default/update.xml?force=1 diff -r source/firefox/defaults/pref/channel-prefs.js target/firefox/defaults/pref/channel-prefs.js 5d4 < //@line 6 "/builds/worker/workspace/build/src/browser/app/profile/channel-prefs.js" 2, when we push RC builds to the beta channel we expect the RC build to specify the release channel and to only accept release mar files. eg: Found diffs for complete update from https://aus5.mozilla.org/update/3/Firefox/60.0/20180426170554/Linux_x86-gcc3/en-US/release-localtest/default/default/default/update.xml?force=1 diff -r source/firefox/defaults/pref/channel-prefs.js target/firefox/defaults/pref/channel-prefs.js 5c5 < pref("app.update.channel", "beta"); --- > pref("app.update.channel", "release"); diff -r source/firefox/update-settings.ini target/firefox/update-settings.ini 5c5 < ACCEPTED_MAR_CHANNEL_IDS=firefox-mozilla-beta,firefox-mozilla-release --- > ACCEPTED_MAR_CHANNEL_IDS=firefox-mozilla-release 3, related to 2, we now test RC -> latest beta, and hit a channel-prefs.js difference but not one for update-settings.ini: Found diffs for complete update from https://aus5.mozilla.org/update/3/Firefox/60.0/20180503143129/Linux_x86-gcc3/th/beta-localtest/default/default/default/update.xml?force=1 diff -r source/firefox/defaults/pref/channel-prefs.js target/firefox/defaults/pref/channel-prefs.js 5c5 < pref("app.update.channel", "release"); --- > pref("app.update.channel", "beta"); Since bug 1446160 we fail on any differences, so we need to explicitly whitelist these to avoid releaseduty inspecting logs manually.
Assignee | ||
Comment 1•5 years ago
|
||
Note to self: also documented at https://github.com/mozilla-releng/releasewarrior-2.0/blob/86aa37b5fe64b01f66b37eb453bd1d4f4208aab0/docs/misc-operations/analyze-update-verify-logs.md#known-differences. Should update that when done here.
Assignee | ||
Comment 2•5 years ago
|
||
WIP: working 12/12 update verify that previously got hung up on the javascript comments in channel-prefs.js: https://tools.taskcluster.net/groups/cWh9ZYLtTHuYmVYRizzhYw/tasks/cWh9ZYLtTHuYmVYRizzhYw/details Code is in https://hg.mozilla.org/users/nthomas_mozilla.com/tools/, mostly 2e9fab62aee1. Possibly an issue with diff-summary.log not being populated.
Updated•5 years ago
|
Whiteboard: [releaseduty]
Assignee | ||
Updated•5 years ago
|
Summary: Add a whitelist for expected update verify differences → Add an allowlist of expected update verify differences
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•5 years ago
|
||
Retesting some recent releases: Firefox 61.0b6: https://tools.taskcluster.net/groups/GRAMDnbMT6K_zsACzqgRnw DevEdition 61.0b6: https://tools.taskcluster.net/groups/IMoe6W_rSHS0QjbGolBVsg Firefox 60.0 build2, release channel: https://tools.taskcluster.net/groups/AsEYwgWRQBSkXjlVd7jN0w Firefox 60.0 build2, beta channel: https://tools.taskcluster.net/groups/Po7Tmi92SzGfD7dPE0JjHA Firefox 52.8.0esr: https://tools.taskcluster.net/groups/IaNP62N5Qk2i4akWtUFbww Firefox 60.0esr : Not possible due to configuration generation issue, see bug 1462120
Assignee | ||
Comment 5•5 years ago
|
||
Doc update - https://github.com/mozilla-releng/releasewarrior-2.0/pull/147
Comment 6•5 years ago
|
||
mozreview-review |
Comment on attachment 8979135 [details] Bug 1461490 - update verify allowlist followup for windows ESR52, https://reviewboard.mozilla.org/r/245376/#review251452 ::: release/common/unpack.sh:107 (Diff revision 1) > fi > > if [ ! -z $update_settings_string ]; then > echo "Modifying update-settings.ini" > cat "${update_settings_file}" | sed -e "s/^ACCEPTED_MAR_CHANNEL_IDS.*/ACCEPTED_MAR_CHANNEL_IDS=${update_settings_string}/" > "${update_settings_file}.new" > - diff -u "{$update_settings_file}" "${update_settings_file}.new" > + diff -u "${update_settings_file}" "${update_settings_file}.new" weird, how did this work before?
Attachment #8979135 -
Flags: review?(aki) → review+
Comment 7•5 years ago
|
||
Thanks!
Assignee | ||
Comment 8•5 years ago
|
||
(In reply to Aki Sasaki [:aki] from comment #6) > weird, how did this work before? The call to diff wasn't working, but no 'set -e' so it was just a log line that didn't fail the job, eg Modifying update-settings.ini diff: {firefox/update-settings.ini}: No such file or directory in https://taskcluster-artifacts.net/O4sRM_ALTAqf5JyHHYs6SQ/0/public/logs/live_backing.log from 61.0b4.
Assignee | ||
Comment 9•5 years ago
|
||
Comment on attachment 8979135 [details] Bug 1461490 - update verify allowlist followup for windows ESR52, https://hg.mozilla.org/build/tools/rev/e94302d4b6061799e5a7fff698d482e672aceeda https://github.com/mozilla-releng/releasewarrior-2.0/commit/f0200e61feaffc1fcfea3787735e6bd26f4ccc9c (but I forgot to update the changelog)
Attachment #8979135 -
Flags: checked-in+
Comment 10•5 years ago
|
||
I'm guessing this is done; is that correct? If not, we should probably move the trello card back :)
Assignee | ||
Comment 11•5 years ago
|
||
All done!
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment 12•5 years ago
|
||
I _think_ this broke on Windows (which we still run it on for esr52). We ended up with output like: Comparing source/bin with target/bin... Directories only in source/bin: source/bin\gmp-clearkey\0.1 source/bin\fonts source/bin\browser\VisualElements source/bin\browser source/bin\browser\extensions source/bin\uninstall source/bin\browser\features source/bin\defaults\pref source/bin\gmp-clearkey source/bin\defaults Directories only in target/bin: target/bin\browser\VisualElements target/bin\defaults target/bin\browser\extensions target/bin\browser\features target/bin\gmp-clearkey target/bin\browser target/bin\uninstall target/bin\fonts target/bin\defaults\pref target/bin\gmp-clearkey\0.1 ...which is just a difference in ordering. I used https://gist.github.com/mozbhearsum/f827a919707445bbb0ced6884f761d7f to verify that only the ordering was different for all of the 52.8.1esr windows update verify logs. Might need a sorted() somewhere in the script for a quick workaround?
Assignee | ||
Comment 13•5 years ago
|
||
Thanks for the headsup. The source/bin prefix shouldn't be in output, which indicates this code is faulty in the presence of a forward slash: # trim off directory prefix for easier comparison all_dirs = [d.replace(path + '/', '') for d in all_dirs] I'll figure out a patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (mozreview-request) |
Assignee | ||
Comment 15•5 years ago
|
||
The new patch trims off the path prefix and one more character from the full path. I verified it fixes Win32 ESR52, and doesn't regress mac ESR52 or 60.0.2 mac/windows. The use of sorted() is just a pretty printing change.
Comment 16•5 years ago
|
||
mozreview-review |
Comment on attachment 8979135 [details] Bug 1461490 - update verify allowlist followup for windows ESR52, https://reviewboard.mozilla.org/r/245376/#review256180
Attachment #8979135 -
Flags: review?(bhearsum) → review+
Assignee | ||
Comment 17•5 years ago
|
||
Landed: https://hg.mozilla.org/build/tools/rev/88b450a68f2444542486da1421f85f3c56225d37 Spot verification: * 52.8.1esr fixed (rerun on default instead of tag) https://tools.taskcluster.net/groups/ZtQzYPSjSL2O2OzrZnrKDw/tasks/ZtQzYPSjSL2O2OzrZnrKDw/details * 61.0b12 still green (rerun) https://tools.taskcluster.net/groups/BFGv0jLNQGakqg872NYa5g/tasks/Cv425sO_TOeXlz22W8fKAQ/runs/1
Status: REOPENED → RESOLVED
Closed: 5 years ago → 5 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•