taskgraph: Make update tasks support esr60

RESOLVED FIXED

Status

enhancement
RESOLVED FIXED
Last year
8 months ago

People

(Reporter: jlorenzo, Assigned: Callek)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 fixed, firefox61 fixed)

Details

Attachments

(5 attachments, 1 obsolete attachment)

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
Hello Ben! 60.0esr goes to build on April 30th. Would you feel comfortable looking into this bug by then?
Flags: needinfo?(bhearsum)
(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)
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)
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: Last year
Flags: needinfo?(jlorenzo)
Resolution: --- → INVALID
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 → ---
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)
(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 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 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 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+
Attachment #8969579 - Attachment is obsolete: true
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: nobody → bugspam.Callek
https://hg.mozilla.org/mozilla-central/rev/0b24ec75e753
https://hg.mozilla.org/mozilla-central/rev/f12de32d3468
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → FIXED
: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)
(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 → ---
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?
Attachment #8970785 - Flags: review?(rail)
Attachment #8970785 - Flags: review?(bhearsum)
:rail I forgot ben is on PTO this week, can you help with c#20?
Flags: needinfo?(rail)
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)
Attachment #8970785 - Flags: review?(rail) → review+
Attachment #8970786 - Flags: review?(rail) → review+
Flags: needinfo?(bhearsum)
Attachment #8970785 - Flags: review?(bhearsum)
Attachment #8970786 - Flags: review?(bhearsum)
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
https://hg.mozilla.org/mozilla-central/rev/4226da9821ec
Status: REOPENED → RESOLVED
Closed: Last yearLast year
Resolution: --- → FIXED
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+
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f89e9dbb8a1e
Add esr watersheds in update configs r=bhearsum
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+
Pushed by mozilla@hocat.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a06fb7991bde
Fix Update verify tests p=Callek r=rail
You need to log in before you can comment on or make changes to this bug.