Closed
Bug 1453274
Opened 6 years ago
Closed 6 years ago
taskgraph: Make update tasks support esr60
Categories
(Release Engineering :: Release Automation: Other, enhancement)
Release Engineering
Release Automation: Other
Tracking
(firefox-esr60 fixed, firefox61 fixed)
RESOLVED
FIXED
People
(Reporter: jlorenzo, Assigned: Callek)
References
Details
Attachments
(5 files, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
rail
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
rail
:
review+
|
Details |
1.21 KB,
text/plain
|
rail
:
review+
|
Details |
685 bytes,
patch
|
rail
:
review+
|
Details | Diff | Splinter Review |
59 bytes,
text/x-review-board-request
|
bhearsum
:
review+
jlorenzo
:
checked-in+
|
Details |
Generating a taskgraph with esr60 configs[1] leads to some discrepancies. The things I've noticed are: * release-final-verify-firefox, release-update-verify-config-firefox-linux, release-update-verify-firefoxlinux-1/12: * esr60 configs are still missing in https://hg.mozilla.org/build/tools/file/5641e7cf9ea4/release/updates There are probably other issues I didn't see. [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1434889#c4
Reporter | ||
Comment 1•6 years ago
|
||
Hello Ben! 60.0esr goes to build on April 30th. Would you feel comfortable looking into this bug by then?
Flags: needinfo?(bhearsum)
Comment 2•6 years ago
|
||
(In reply to Johan Lorenzo [:jlorenzo] - On PTO - Back on April 16th from comment #1) > Hello Ben! 60.0esr goes to build on April 30th. Would you feel comfortable > looking into this bug by then? I'm not sure what there is to look into, exactly. With the most recent work, we no longer have update verify configs that are checked into the tools repo - we build them on the fly in the update-verify-config tasks, and use those when running update verify. I'm happy to help if I can, though.
Flags: needinfo?(bhearsum)
Comment 3•6 years ago
|
||
I looked over the update and final verify tasks and see blocks like: "TASKCLUSTER_VERIFY_CONFIG": { "task-reference": "https://queue.taskcluster.net/v1/task/<release-update-verify-config-firefox-linux>/artifacts/public/build/update-verify.cfg" } and "task-reference": "hg clone $BUILD_TOOLS_REPO tools && cd tools/release && ./final-verification.sh https://queue.taskcluster.net/v1/task/<release-update-verify-config-firefox-macosx64>/artifacts/public/build/update-verify.cfg https://queue.taskcluster.net/v1/task/<release-update-verify-config-firefox-win64>/artifacts/public/build/update-verify.cfg https://queue.taskcluster.net/v1/task/<release-update-verify-config-firefox-linux>/artifacts/public/build/update-verify.cfg https://queue.taskcluster.net/v1/task/<release-update-verify-config-firefox-win32>/artifacts/public/build/update-verify.cfg https://queue.taskcluster.net/v1/task/<release-update-verify-config-firefox-linux64>/artifacts/public/build/update-verify.cfg" ...which means they are not using configs from the build/tools repo. I'll this bug open in case I've missed something, but I'm pretty sure there's no prep we need to do here.
Flags: needinfo?(jlorenzo)
Reporter | ||
Comment 4•6 years ago
|
||
You're right! I'm sorry, I was confused by the command line which clones the tools repo beforehand. Thank you for checking this out! Closing bug.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(jlorenzo)
Resolution: --- → INVALID
Reporter | ||
Comment 5•6 years ago
|
||
Crap, the update config tasks failed because it considers "60.0esr" to no be a valid version number[1]. Reopening bug. [1] https://tools.taskcluster.net/groups/H0mR0zVhT66ohtOLFl-OZw/tasks/auQhcGj5SbuGhBhCynqAMw/runs/0/logs/public%2Flogs%2Flive_backing.log#L304
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Reporter | ||
Comment 6•6 years ago
|
||
I think I can fix it, but I don't know where the update configs are generated. Would you have a pointer, Ben?
Flags: needinfo?(bhearsum)
Comment 7•6 years ago
|
||
(In reply to Johan Lorenzo [:jlorenzo] from comment #6) > I think I can fix it, but I don't know where the update configs are > generated. Would you have a pointer, Ben? https://hg.mozilla.org/mozilla-central/file/default/python/mozrelease/mozrelease/versions.py is the code that's throwing the exception. The calling code is in https://hg.mozilla.org/mozilla-central/file/default/testing/mozharness/scripts/release/update-verify-config-creator.py#l206
Flags: needinfo?(bhearsum)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•6 years ago
|
||
mozreview-review |
Comment on attachment 8969579 [details] Bug 1453274 - Make ModernMozillaVersion support ESR https://reviewboard.mozilla.org/r/238340/#review244192 I don't think this works. I got this error from 52.0esr: >>> mmv("52.0esr") Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python2.7/distutils/version.py", line 40, in __init__ self.parse(vstring) File "/usr/lib/python2.7/distutils/version.py", line 113, in parse self.version = tuple(map(string.atoi, [major, minor, patch])) File "/usr/lib/python2.7/string.py", line 405, in atoi return _int(s, base) ValueError: invalid literal for int() with base 10: '0esr' >>> I believe the problem is that the StrictVersion class expects a digit at the end of the "tag" (like "esr"). We can't switch away from it entirely, because we need its version comparison logic, so I'm not sure exactly what to do...if it was possible to provide a "default" for optional values that might work, or maybe we have to go so far as to modify the class a bit. As a final sanity check, I think it would be good to run all of the version numbers from 3.0 and up on https://archive.mozilla.org/pub/firefox/releases/ through the new class (you can ignore things like "-funnelcake" and "-real" which obviously are not real versions). ::: python/mozrelease/mozrelease/versions.py:19 (Diff revision 2) > + * a beta has 2 digits only > + * an ESR can have either 2 or 3 > + * a build number can exist at the end of the version. > + """ > + # Inspired by https://github.com/mozilla-releng/ship-it/blob/\ > + # 18dc35c511c2d38a1f7472c34c435176f3807212/kickoff/views/forms.py#L221 Nit: please put this on one line. You can probably use # noqa or something like it to get flake8 to be quiet? ::: python/mozrelease/mozrelease/versions.py:25 (Diff revision 2) > + version_re = re.compile(r"""^(\d+)\.( # Major version number > + (0)(a1|a2|b(\d+)|esr)? # 2-digit-versions (like 46.0, 46.0b1, 46.0esr) > + |( # Here begins the 3-digit-versions. > + ([1-9]\d*)\.(\d+)|(\d+)\.([1-9]\d*) # 46.0.0 is not correct > + )(esr|([a-zA-Z]+(\d+)))? # Neither is 46.2.0b1, but we allow 3.6.3plugin1 > + )(build(\d+))?$""", re.VERBOSE) I don't think we should support "buildN" here -- this is meant to be version numbers only, not version+build number.
Attachment #8969579 -
Flags: review?(bhearsum) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 13•6 years ago
|
||
mozreview-review |
Comment on attachment 8970275 [details] Bug 1453274 - Add testing for python/mozrelease/versions.py https://reviewboard.mozilla.org/r/239070/#review244734
Attachment #8970275 -
Flags: review?(rail) → review+
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8970276 [details] Bug 1453274 - Support esr version strings. https://reviewboard.mozilla.org/r/239072/#review244736 Wow, a lot of ESR fun :)
Attachment #8970276 -
Flags: review?(rail) → review+
Assignee | ||
Updated•6 years ago
|
Attachment #8969579 -
Attachment is obsolete: true
Comment 15•6 years ago
|
||
Pushed by Callek@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0b24ec75e753 Add testing for python/mozrelease/versions.py r=rail https://hg.mozilla.org/integration/autoland/rev/f12de32d3468 Support esr version strings. r=rail
Assignee | ||
Comment 16•6 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/46a0d50672cea5246c8244ba68c114e62786850e https://hg.mozilla.org/releases/mozilla-beta/rev/cb3a85e429f635ca0d77a264e53dd317fedf89c4
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → bugspam.Callek
Comment 17•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0b24ec75e753 https://hg.mozilla.org/mozilla-central/rev/f12de32d3468
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Assignee | ||
Comment 18•6 years ago
|
||
:ben, I notice now that despite mozrelease being in tree, mozharness for UV tries to pull it from our pypi.. and thus is failing because it doesn't have the patch I landed... Is there a better fix we can do here, if not can you cut a new package using whatever means necessary for this? https://taskcluster-artifacts.net/YY7-xjSFREaMuGXZB6OnIw/0/public/logs/live_backing.log
Flags: needinfo?(bhearsum)
Assignee | ||
Comment 19•6 years ago
|
||
(In reply to Justin Wood (:Callek) from comment #18) > :ben, I notice now that despite mozrelease being in tree, mozharness for UV > tries to pull it from our pypi.. and thus is failing because it doesn't have > the patch I landed... Is there a better fix we can do here, if not can you > cut a new package using whatever means necessary for this? > > https://taskcluster-artifacts.net/YY7-xjSFREaMuGXZB6OnIw/0/public/logs/ > live_backing.log Ok I think I solved this piece, patches coming. This bug is still ToDo though...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 20•6 years ago
|
||
Ben, I still have some outstanding questions/confusion here that could use your help: * include-version, what should we use? - I see https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-update-verify-config/kind.yml#54 and https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/update_verify_config.py#24 where I'm not sure if we need to add an esr specific regex. * last-watershed - https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-update-verify-config/kind.yml#62 - could use some help understanding whats in balrog to fill in for this one, especially in a way that I can test this codepath on jamun if possible * channel - I'm assuming we want to s/esr52/esr60/ here https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-update-verify-config/kind.yml#102 * mar-channel-id-override - https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-update-verify-config/kind.yml#111 - Do we need this for esr? * secondary-update-verify-config - https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-secondary-update-verify-config/kind.yml#63 - Does this need changes for esr60?
Assignee | ||
Comment 21•6 years ago
|
||
Attachment #8970785 -
Flags: review?(rail)
Attachment #8970785 -
Flags: review?(bhearsum)
Assignee | ||
Comment 22•6 years ago
|
||
Attachment #8970786 -
Flags: review?(rail)
Attachment #8970786 -
Flags: review?(bhearsum)
Assignee | ||
Comment 23•6 years ago
|
||
:rail I forgot ben is on PTO this week, can you help with c#20?
Flags: needinfo?(rail)
Comment 24•6 years ago
|
||
Just a disclaimer, I know not enough about the new code we use for update verify, but will try my best to guess :) (In reply to Justin Wood (:Callek) from comment #20) > Ben, I still have some outstanding questions/confusion here that could use > your help: > > * include-version, what should we use? > - I see > https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-update- > verify-config/kind.yml#54 and > https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/ > transforms/update_verify_config.py#24 where I'm not sure if we need to add > an esr specific regex. I would probably add another regexp and make it ESR only, so we don't mix up releases and ESR versions. > > * last-watershed > - > https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-update- > verify-config/kind.yml#62 > - could use some help understanding whats in balrog to fill in for this > one, especially in a way that I can test this codepath on jamun if possible I think this indicates the minimum version we should be testing against. In other words, there is no reason to test Firefox 33 -> latest updates, if there is a watershed in between. Not sure how this will fork for esr 60.0, where we don't have updates enabled. In the past we solved this issue by changing the test balrog rules, so we can test both versions of esr (52 and 60 in this case) separately. Something like this: 1) adjust the rules so they match the final ones (52 doesn't update to 60) 2) test 52 3) adjust the rules so 52 updates to 60 4) test 60 updates Depending on the timing, the order may change, but the idea is the same. > * channel > - I'm assuming we want to s/esr52/esr60/ here > https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-update- > verify-config/kind.yml#102 > Yup. It will overlap with esr52, but see the point above on how to test them. > * mar-channel-id-override > - > https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release-update- > verify-config/kind.yml#111 > - Do we need this for esr? Nope, it's beta-only, where we can accept "release" MARs (RC updates). > > * secondary-update-verify-config > - > https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/release- > secondary-update-verify-config/kind.yml#63 > - Does this need changes for esr60? I don't think so. From the name and the config I guess it is related to the case where we test RC updates on the beta channel. No such thing for ESR.
Flags: needinfo?(rail)
Updated•6 years ago
|
Attachment #8970785 -
Flags: review?(rail) → review+
Updated•6 years ago
|
Attachment #8970786 -
Flags: review?(rail) → review+
Updated•6 years ago
|
Flags: needinfo?(bhearsum)
Updated•6 years ago
|
Attachment #8970785 -
Flags: review?(bhearsum)
Updated•6 years ago
|
Attachment #8970786 -
Flags: review?(bhearsum)
Comment 25•6 years ago
|
||
Pushed by Callek@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4226da9821ec Bump mozrelease in tree version, to distinguish it from the pypi mirror version. r=rail
Comment 26•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/4226da9821ec
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 29•6 years ago
|
||
mozreview-review |
Comment on attachment 8972614 [details] Bug 1453274 - Add esr watersheds in update configs https://reviewboard.mozilla.org/r/241168/#review247038
Attachment #8972614 -
Flags: review?(bhearsum) → review+
Comment 30•6 years ago
|
||
Pushed by jlorenzo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f89e9dbb8a1e Add esr watersheds in update configs r=bhearsum
Reporter | ||
Comment 31•6 years ago
|
||
Comment on attachment 8972614 [details] Bug 1453274 - Add esr watersheds in update configs Landed on release at https://hg.mozilla.org/releases/mozilla-release/rev/fb499da47550f9abfa20a93c3830fbf971962e99. This will be picked up by sheriffs for esr60 and beta.
Attachment #8972614 -
Flags: checked-in+
Comment 32•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr60/rev/593627bdd350 https://hg.mozilla.org/releases/mozilla-esr60/rev/fb499da47550
status-firefox-esr60:
--- → fixed
Comment 33•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f89e9dbb8a1e
Comment 34•6 years ago
|
||
Pushed by mozilla@hocat.ca: https://hg.mozilla.org/integration/mozilla-inbound/rev/a06fb7991bde Fix Update verify tests p=Callek r=rail
Comment 35•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/a06fb7991bde
Comment 36•5 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr60/rev/2ccd653fce85
You need to log in
before you can comment on or make changes to this bug.
Description
•