1469803 - Add ability to run bouncer check on demand

Status: RESOLVED FIXED

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

Description

[Simon Fraser [:sfraser] ⌚️GMT]

The release-bouncer-check kind uses https://hg.mozilla.org/mozilla-central/file/tip/testing/mozharness/scripts/release/bouncer_check.py to test the state of the bouncer aliases for a given release. It's currently run in-tree, and it would be useful to be able to run this on demand. We should add a mach target for this kind, so it can run in isolation, and then we can add a hook, and possible cron entry for it.

Comment 1

[Simon Fraser [:sfraser] ⌚️GMT]

release-bouncer-check depends on release-beetmover-push-to-release, which brings in the entire build process as a dependency.

The payload itself works fine, as long as it knows the version and config to use:

./mach python testing/mozharness/scripts/release/bouncer_check.py --version=60.0.2 --config releases/bouncer_firefox_release.py

Could call `--version=$(cat browser/config/version.txt)` so it doesn't depend on release config, and separate it out to its own kind with no dependencies.

Comment 2

[Phabricator Automation]

Attachment: Bug 1469803 Separate bouncer-check for cron tests r=mtabara

We shouldn't run this on central, as it falls back to the dev configs, and fails.
It should be fine on beta/release/esr60. I had to move this version of the check to its own
kind to avoid the dependency tree bringing in the entire build process. Perhaps we can
refactor later to avoid duplication

Comment 3

[Phabricator Automation]

Comment on attachment 8987022 [details]
Bug 1469803 Separate bouncer-check for cron tests r=mtabara

Mihai Tabara [:mtabara]⌚️GMT has approved the revision.

https://phabricator.services.mozilla.com/D1765

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a37e6ed0f4e7
Separate bouncer-check for cron tests r=mtabara

Comment 5

[Simon Fraser [:sfraser] ⌚️GMT]

From the review:

> This looks good to me. I'm slightly worried we're not testing it anywhere before enabling it on release trees but in the end, 
> it doesn't break nor block releases.
> That being said, I think it's maybe worth landing this to beta only first, to iterate in case things are broken and only then 
> cherry-pick to release/esr60 to avoid debugging on multiple branches all-at-once.

I agree - I was going to let it be naturally merged into beta, along with the cron entry (PR for which is coming)

Comment 6

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #5)
> From the review:
> 
> > This looks good to me. I'm slightly worried we're not testing it anywhere before enabling it on release trees but in the end, 
> > it doesn't break nor block releases.
> > That being said, I think it's maybe worth landing this to beta only first, to iterate in case things are broken and only then 
> > cherry-pick to release/esr60 to avoid debugging on multiple branches all-at-once.
> 
> I agree - I was going to let it be naturally merged into beta, along with
> the cron entry (PR for which is coming)

Sounds good, awesome!

Comment 7

[Mihai Tabara [:mtabara]⌚️GMT]

I'm curious, this just occurred to me. How come the bouncer check will work after we version bump? 
I mean, once we ship a beta release, say, we bump the version to X+1 while bouncer entries correspond to X. During a release, release-bouncer-check runs in the push phase which means it's still using that in-tree version (via https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/transforms/bouncer_check.py#l25), following which in the shipping phase it's being bumped.

Does this make sense or am I missing something? It just occurred to me. I guess we could munge that version passing somehow.
What say you?

Comment 8

[Simon Fraser [:sfraser] ⌚️GMT]

Good point, and well spotted. I'd run tests on mozilla-release and mozilla-beta parameters.yml, but it must have caught the versions at the right time to work. Trying it with new parameters files now, it doesn't work. 

Trying to decrease the version number seems wrong. What's a safe way of working out the previous version?

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/a37e6ed0f4e7

Comment 10

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #8)
> Good point, and well spotted. I'd run tests on mozilla-release and
> mozilla-beta parameters.yml, but it must have caught the versions at the
> right time to work. Trying it with new parameters files now, it doesn't
> work. 
> 
> Trying to decrease the version number seems wrong. What's a safe way of
> working out the previous version?

Hm, not sure. Let's ask Rail, he's done some work both in bouncer and in managing those previous versions thing.
@rail: in this bug we've taken basically your bouncer check and we run it on a daily basis to check the status of bouncer products. However, normall that checks the version after it's been bumped in-tree. Any thoughts in how we can solve this?

Comment 11

[Rail Aliiev [:rail]]

Maybe we should use the version that we published in https://product-details.mozilla.org/1.0/firefox_versions.json? Also running this multiple times a day would be beneficial. I know that the QE team checks them every 5m or so ;)

Comment 12

[Simon Fraser [:sfraser] ⌚️GMT]

Hmm, we wouldn't want to check that url in the task graph generation, to avoid requiring a network connection when doing so locally. But it doesn't matter if the value changes: in fact, we want it to always check the current value retrieved.

How about this for a plan?

1. Modify testing/mozharness/scripts/release/bouncer_check.py
   a. Add --version-field option, which conflicts with --version. This specifies which field to use from firefox_versions.json
   b. Add --versions-url option, with a default of product-details, from above.
   bouncer_check.py can now either be told a specific version to check, or 'whatever is latest for this label'
2. transforms/bouncer_check.py has changes to `add_command` https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/bouncer_check.py#28-32 such that it adds --version or --version-field depending on the task kind.
   Precedent for transforms behaving different depending on kind:
   i. https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/transforms/beetmover_repackage_partner.py#l119
   ii. https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/transforms/chunk_partners.py#l30
   iii. https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/transforms/partner_repack.py#l28
   and others.

Comment 13

[Simon Fraser [:sfraser] ⌚️GMT]

Attachment: Bug 1469803 Update cron bouncer check to use product-details r=mtabara

Comment 14

[Phabricator Automation]

Comment on attachment 8988806 [details]
Bug 1469803 Update cron bouncer check to use product-details r=mtabara

Rail Aliiev [:rail] ⌚️ET has approved the revision.

https://phabricator.services.mozilla.com/D1883

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/068e65e94cff
Update cron bouncer check to use product-details r=rail

Comment 16

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #12)
> Hmm, we wouldn't want to check that url in the task graph generation, to
> avoid requiring a network connection when doing so locally. But it doesn't
> matter if the value changes: in fact, we want it to always check the current
> value retrieved.
> 
> How about this for a plan?
> 
> 1. Modify testing/mozharness/scripts/release/bouncer_check.py
>    a. Add --version-field option, which conflicts with --version. This
> specifies which field to use from firefox_versions.json
>    b. Add --versions-url option, with a default of product-details, from
> above.
>    bouncer_check.py can now either be told a specific version to check, or
> 'whatever is latest for this label'
> 2. transforms/bouncer_check.py has changes to `add_command`
> https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/
> transforms/bouncer_check.py#28-32 such that it adds --version or
> --version-field depending on the task kind.
>    Precedent for transforms behaving different depending on kind:
>    i.
> https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/
> transforms/beetmover_repackage_partner.py#l119
>    ii.
> https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/
> transforms/chunk_partners.py#l30
>    iii.
> https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/
> transforms/partner_repack.py#l28
>    and others.

This looks really good to me, nice tweak around it.
I only have two questions/concerns but we can address them later on:

1. Looks like we've removed esr60 from checkings, any reason we've done that? Is it because we don't want to deal with FIREFOX_ESR vs FIREFOX_ESR_NEXT since next cycle we'll EOL-ing 52 anyway?

2. I may be wrong but we might get bitten with this during the next set of b1-b2 (I suppose 62.b1 and 62.b3). Unclear to me which of these versions are beta and which are devedition - LATEST_FIREFOX_DEVEL_VERSION vs FIREFOX_DEVEDITION vs LATEST_FIREFOX_RELEASED_DEVEL_VERSION.

During first week after mergeduty is completed we only build Devedition VERSION.0b1 and VERSION.0b2 hence the beta-version and devedition-version should be different there. However, it's still unclear to me which values those are since they have such a conflicting naming in Ship-it. I think it's safe to ignore that for now and address this in the future whenever we will hit this, if any.

Comment 17

[Simon Fraser [:sfraser] ⌚️GMT]

(In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #16)

> 1. Looks like we've removed esr60 from checkings, any reason we've done
> that? Is it because we don't want to deal with FIREFOX_ESR vs
> FIREFOX_ESR_NEXT since next cycle we'll EOL-ing 52 anyway?

Essentially, yes. It's something we can add once the easier cases are working.
 
> 2. I may be wrong but we might get bitten with this during the next set of
> b1-b2 (I suppose 62.b1 and 62.b3). Unclear to me which of these versions are
> beta and which are devedition - LATEST_FIREFOX_DEVEL_VERSION vs
> FIREFOX_DEVEDITION vs LATEST_FIREFOX_RELEASED_DEVEL_VERSION.
> 
> During first week after mergeduty is completed we only build Devedition
> VERSION.0b1 and VERSION.0b2 hence the beta-version and devedition-version
> should be different there. However, it's still unclear to me which values
> those are since they have such a conflicting naming in Ship-it. I think it's
> safe to ignore that for now and address this in the future whenever we will
> hit this, if any.

Yeah, I'm not clear on what to do there. I hope that, as you say, LATEST_FIREFOX_RELEASED_DEVEL_VERSION is not updated for b1/b2 but FIREFOX_DEVEDITION is.

Comment 18

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #17)
> (In reply to Mihai Tabara [:mtabara]⌚️GMT from comment #16)
> 
> > 1. Looks like we've removed esr60 from checkings, any reason we've done
> > that? Is it because we don't want to deal with FIREFOX_ESR vs
> > FIREFOX_ESR_NEXT since next cycle we'll EOL-ing 52 anyway?
> 
> Essentially, yes. It's something we can add once the easier cases are
> working.

Makes perfect sense! We should add a note somewhere to make sure we don't forget to add this check but other than that, sounds good to me.
  
> > 2. I may be wrong but we might get bitten with this during the next set of
> > b1-b2 (I suppose 62.b1 and 62.b3). Unclear to me which of these versions are
> > beta and which are devedition - LATEST_FIREFOX_DEVEL_VERSION vs
> > FIREFOX_DEVEDITION vs LATEST_FIREFOX_RELEASED_DEVEL_VERSION.
> > 
> > During first week after mergeduty is completed we only build Devedition
> > VERSION.0b1 and VERSION.0b2 hence the beta-version and devedition-version
> > should be different there. However, it's still unclear to me which values
> > those are since they have such a conflicting naming in Ship-it. I think it's
> > safe to ignore that for now and address this in the future whenever we will
> > hit this, if any.
> 
> Yeah, I'm not clear on what to do there. I hope that, as you say,
> LATEST_FIREFOX_RELEASED_DEVEL_VERSION is not updated for b1/b2 but
> FIREFOX_DEVEDITION is.

Sounds good. Let's ignore then for now and see if all this works for most common cases beta/release.

Comment 19

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/068e65e94cff

Comment 20

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Tiberius Oros[:tiberius_oros] from comment #19)
> https://hg.mozilla.org/mozilla-central/rev/068e65e94cff

Just to reiterate, this is riding the trains.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/5bb9d23f91c4
https://hg.mozilla.org/releases/mozilla-esr60/rev/0f7218ac2538