Closed Bug 1433459 Opened 3 years ago Closed 3 years ago

[tracking] add bouncer scriptworker to drive all operations regarding bouncer in TC world

Categories

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

enhancement
Not set
normal

Tracking

(firefox60 fixed)

RESOLVED FIXED
Tracking Status
firefox60 --- fixed

People

(Reporter: mtabara, Assigned: jlorenzo)

References

(Blocks 1 open bug)

Details

Attachments

(7 files, 1 obsolete file)

This includes:
* bouncer submission
* bouncer aliases (post release)
* potentially uptake monitoring too
No longer depends on: 1398796
Summary: [meta] add bouncer scriptworker to drive all operations regarding bouncer in TC world → [tracking] add bouncer scriptworker to drive all operations regarding bouncer in TC world
Attachment #8952336 - Flags: review?(aki)
Attachment #8952336 - Flags: review?(aki) → review+
Comment on attachment 8953010 [details]
Bug 1433459 - Add bouncer_scriptworker instances

https://reviewboard.mozilla.org/r/222274/#review228178

Looks good so far with some tiny nits. Might need to wait until we finalize the bouncerscript side to confirm the configs generated by puppet but we can always iterate on this later on this week.

::: modules/bouncer_scriptworker/manifests/init.pp:31
(Diff revision 1)
> +            packages => [
> +                'PyYAML==3.12',
> +                'aiohttp==2.3.9',
> +                'arrow==0.12.1',
> +                'async_timeout==1.4.0',
> +                'bouncerscript==0.1.0',

This is still in the future but sure :}

::: modules/bouncer_scriptworker/manifests/init.pp:49
(Diff revision 1)
> +                'python-dateutil==2.6.1',
> +                'python-gnupg==0.4.1',
> +                'redo==1.6',
> +                'requests==2.18.4',
> +                'scriptworker==8.2.0',
> +                'bouncerapi==0.1.0',

Applies to shipitapi only, not needed on bouncerscript.

::: modules/bouncer_scriptworker/manifests/settings.pp:11
(Diff revision 1)
> +    include ::config
> +    include users::builder
> +
> +    $root                     = $config::scriptworker_root
> +
> +    $bouncer_stage_instance_scope = 'project:releng:bouncer:server:staging'

would s/staging/dep be better and consistent with what we currently have in others?
Attachment #8953010 - Flags: review?(mtabara) → review+
I added scopes to https://tools.taskcluster.net/auth/roles/moz-tree%3Alevel%3A3%3Agecko . Some of them (staging, dev, actions:*) could move to moz-tree:level:1:gecko, but this unblocks birch for now.
Today we received this alert on #buildduty channel. ([sns alert] Feb 22 09:35:09 aws-manager2.srv.releng.scl3.mozilla.com b-2008-ec2-golden: 2018-02-22 01:35:09,124 - ERROR - IP 10.134.48.9 reserved for b-2008-ec2-golden, but not available). 
j.lorenzo could you please change the IP of bouncerworker-dev-1?
Sorry about this. I wonder how that could happen. I used invtool to get this free IP earlier this week. Anyway, I terminated bouncerworker-dev-1 to create a new one. The new one is now mapped to 10.134.48.58.
Attachment #8953081 - Attachment is obsolete: true
Attachment #8953081 - Flags: review?(rail)
Attachment #8953081 - Flags: review?(mtabara)
Comment on attachment 8953080 [details]
Bug 1433459 - part 1: Move bouncer submission tasks to scriptworker

https://reviewboard.mozilla.org/r/222364/#review228336

::: taskcluster/taskgraph/transforms/bouncer_submission.py:36
(Diff revision 4)
> +CANDIDATES_PATH_TEMPLATE = '/{product}/candidates/{version}-candidates/build{build_number}/\
> +{update_folder}{ftp_platform}/:lang/{file}'
> +RELEASES_PATH_TEMPLATE = '/{product}/releases/{version}/{update_folder}{ftp_platform}/:lang/{file}'
> +
> +
> +CONFIG_PER_BOUNCER_PRODUCT = {

This looks similar to our bouncer mozharness configs. Can you file a bug so we consolidate them?

::: taskcluster/taskgraph/transforms/bouncer_submission.py:160
(Diff revision 4)
> +        for bouncer_product in bouncer_products
> +        for previous_version in previous_versions
> +    }
> +
> +
> +def craft_paths_per_bouncer_platform(product, bouncer_product, bouncer_platforms, current_version,

Something tells me that if you have the paths explicitly specified in the config, the logic would be much simpler. ;) Something similar to what we have now. I found it was easy to implement bouncer check using those configs. See https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/release/bouncer_check.py#93-124

I think if you use the config-driven logic, then you don't need to address all the edge cases, such as fennec without locales.

::: taskcluster/taskgraph/transforms/bouncer_submission.py:195
(Diff revision 4)
> +
> +    return paths_per_bouncer_platform
> +
> +
> +def craft_bouncer_product_name(product, bouncer_product, current_version,
> +                               current_build_number=None, previous_version=None):

This function looks a bit fragile to me. CAn we explicitly add bouncer product names to the config?

::: taskcluster/taskgraph/transforms/bouncer_submission.py:234
(Diff revision 4)
> +
> +def craft_check_uptake(bouncer_product):
> +    return bouncer_product != 'complete-mar-candidates'
> +
> +
> +def craft_ssl_only(bouncer_product):

Hmm, this won't be valid for ESR... I remember that we use SSL forthe esr installers...
Attachment #8953080 - Flags: review?(rail)
Comment on attachment 8953080 [details]
Bug 1433459 - part 1: Move bouncer submission tasks to scriptworker

https://reviewboard.mozilla.org/r/222364/#review229130

LGTM, thanks again for the help with this! 
There are some hacks and if/clauses in transforms that I think we should address but since we don't yet have a better way of doing things in a universally accepted way, I guess we can go on with this and iterate later on during a potential clea-all-tasks.

::: taskcluster/ci/release-bouncer-sub/kind.yml:42
(Diff revision 4)
> +
>     fennec:
> -      name: fennec_release_bouncer_sub
> +      bouncer-platforms: ['android', 'android-x86']
> +      bouncer-products: ['apk']
>        shipping-product: fennec
> -      run:
> +      locales-file: mobile/locales/l10n-changesets.json

I think we can remove this, it's in `job-defaults` anyway.

::: taskcluster/taskgraph/transforms/bouncer_submission.py:95
(Diff revision 4)
> +        )
> +        resolve_keyed_by(
> +            job, 'scopes', item_name=job['name'], project=config.params['project']
> +        )
> +
> +        job['scopes'].append('project:releng:bouncer:action:submission')

I'm wondering if we shouldn't move this outside of this? Either constants.py or scriptworker/utils?

::: taskcluster/taskgraph/transforms/bouncer_submission.py:194
(Diff revision 4)
> +        paths_per_bouncer_platform[bouncer_platform] = file_relative_location
> +
> +    return paths_per_bouncer_platform
> +
> +
> +def craft_bouncer_product_name(product, bouncer_product, current_version,

I was almost tempted to better suggest a mapping here but seeing there's so many exceptions I'm actually leaning towards keeping this like this.
Attachment #8953080 - Flags: review?(mtabara) → review+
Comment on attachment 8953996 [details]
Bug 1433459 - part 2: Move aliases tasks to scriptworker

https://reviewboard.mozilla.org/r/223168/#review229134

::: taskcluster/ci/release-bouncer-aliases/kind.yml:9
(Diff revision 1)
>  
>  loader: taskgraph.loader.transform:loader
>  
>  transforms:
>     - taskgraph.transforms.release_deps:transforms
> -   - taskgraph.transforms.job:transforms
> +   - taskgraph.transforms.bouncer_aliases:transforms

is this tranform missing or it's a part of some other patch?
Attachment #8953996 - Flags: review?(rail)
Comment on attachment 8953996 [details]
Bug 1433459 - part 2: Move aliases tasks to scriptworker

https://reviewboard.mozilla.org/r/223168/#review229136
Attachment #8953996 - Flags: review?(mtabara) → review+
Blocks: 1441203
Comment on attachment 8953080 [details]
Bug 1433459 - part 1: Move bouncer submission tasks to scriptworker

https://reviewboard.mozilla.org/r/222364/#review228336

> This looks similar to our bouncer mozharness configs. Can you file a bug so we consolidate them?

We shouldn't need these mozhaness config anymore. I filed bug 1441203 to track the deletion.

> Something tells me that if you have the paths explicitly specified in the config, the logic would be much simpler. ;) Something similar to what we have now. I found it was easy to implement bouncer check using those configs. See https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/release/bouncer_check.py#93-124
> 
> I think if you use the config-driven logic, then you don't need to address all the edge cases, such as fennec without locales.

Discussed over IRC. Using Mozharness configs shouldn't be the final goals. The main reasons are: 
* Configs should be at a standard place across the taskgraph. This means probably in kind.yml
* These configs shouldn't duplicate each line like the mozharness configs. Most of the behaviors are standard among bouncer products.
* Implementing them in kind.yml is indeed doable, but cumbersome and eventually results in as much duplication as in mozharness.


I agree these function are hairy. We should ensure they're not broken by adding unit tests to transforms. This is a bigger project, though.

> This function looks a bit fragile to me. CAn we explicitly add bouncer product names to the config?

See previous comment.

> Hmm, this won't be valid for ESR... I remember that we use SSL forthe esr installers...

Good catch. Fixed!
Comment on attachment 8953080 [details]
Bug 1433459 - part 1: Move bouncer submission tasks to scriptworker

https://reviewboard.mozilla.org/r/222364/#review229130

> I think we can remove this, it's in `job-defaults` anyway.

Nope ;) We do need to override the default value for Fennec.

> I'm wondering if we shouldn't move this outside of this? Either constants.py or scriptworker/utils?

Good point. We want to move from scriptworker.utils, in favor of by-project in kinds. Then, I moved it there.
Comment on attachment 8953996 [details]
Bug 1433459 - part 2: Move aliases tasks to scriptworker

https://reviewboard.mozilla.org/r/223168/#review229134

> is this tranform missing or it's a part of some other patch?

Good catch, that was a bad merge on me! Fixed.
Comment on attachment 8953996 [details]
Bug 1433459 - part 2: Move aliases tasks to scriptworker

https://reviewboard.mozilla.org/r/223168/#review229246
Attachment #8953996 - Flags: review?(rail) → review+
Comment on attachment 8953080 [details]
Bug 1433459 - part 1: Move bouncer submission tasks to scriptworker

https://reviewboard.mozilla.org/r/222364/#review229244
Attachment #8953080 - Flags: review?(rail) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 3b38bac41d14585764bf208b98f0ceb822c4815b -d 61165dc3f54a: rebasing 450018:3b38bac41d14 "Bug 1433459 - part 1: Move bouncer submission tasks to scriptworker r=mtabara,rail"
merging taskcluster/taskgraph/transforms/task.py
warning: conflicts while merging taskcluster/taskgraph/transforms/task.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 3b38bac41d14585764bf208b98f0ceb822c4815b -d 61165dc3f54a: rebasing 450018:3b38bac41d14 "Bug 1433459 - part 1: Move bouncer submission tasks to scriptworker r=mtabara,rail"
merging taskcluster/taskgraph/transforms/task.py
warning: conflicts while merging taskcluster/taskgraph/transforms/task.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e91e79d2232e
part 1: Move bouncer submission tasks to scriptworker r=mtabara,rail
https://hg.mozilla.org/integration/mozilla-inbound/rev/5396cec9cde6
part 2: Move aliases tasks to scriptworker r=mtabara,rail
https://hg.mozilla.org/mozilla-central/rev/e91e79d2232e
https://hg.mozilla.org/mozilla-central/rev/5396cec9cde6
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Landed on beta at:
* https://hg.mozilla.org/releases/mozilla-beta/rev/ff884199e8b1dfe369c3989422dba57467b50221
* https://hg.mozilla.org/releases/mozilla-beta/rev/75bf6a6a5f8850bf9bb2c71e652b148338b1b683

Context:
I just chatted with jcristau. He would like to kick off a devedition build with what's currently on beta (i.e.: no extra patch since yesterday's mergeday). Julien doesn't want more patches in the product (but doesn't care too much about the TC changes). Therefore, I uplifted the bouncer and version bump to beta.
Comment on attachment 8953080 [details]
Bug 1433459 - part 1: Move bouncer submission tasks to scriptworker

See comment 35 and comment 36.
Attachment #8953080 - Flags: checked-in+
Comment on attachment 8953996 [details]
Bug 1433459 - part 2: Move aliases tasks to scriptworker

See comment 35 and comment 36.
Attachment #8953996 - Flags: checked-in+
I unpinned both bouncerworker{-dev,}-1 from my puppet environment and made sure puppet still applied on the machine => It worked. I also checked the production instance is still able to run a task[1]. We're good to let this roll in production!

[1] I reran https://tools.taskcluster.net/groups/ZNS-m-xcRAWJ3FWzL4byMQ/tasks/BADAoqJuRSysHYp-yLcy-g
See Also: → 1443104
Blocks: 1443305
Blocks: 1443549
Attachment #8956501 - Flags: review?(mtabara) → review+
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f51ec69adf23
part 3: Fix bad scopes for aliases task r=mtabara
Depends on: 1458479
Blocks: 1408868
Blocks: 1488686
Blocks: 1488518
Assignee: nobody → jlorenzo
You need to log in before you can comment on or make changes to this bug.