1409575 - fennec: trigger release promotion action task from releaserunner

Status: RESOLVED FIXED

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

Description

[Rail Aliiev [:rail]]

This may require another fork of release runner \o/

Comment 1

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Rail Aliiev [:rail] ⌚️ET from comment #0)
> This may require another fork of release runner \o/

Music to my ears .... :P

Comment 2

[Rail Aliiev [:rail]]

Attachment: WIP: tools (releaserunner3)

Comment 3

[Rail Aliiev [:rail]]

Attachment: WIP: tools (release-runner3)

Comment 4

[Rail Aliiev [:rail]]

Attachment: WIP: puppetize it

Comment 5

[Rail Aliiev [:rail]]

Attachment: Bug 1409575 - [tools] fennec: trigger release promotion action task from releaserunner

Review commit: https://reviewboard.mozilla.org/r/190902/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190902/

Comment 6

[Rail Aliiev [:rail]]

Attachment: Bug 1409575 - [puppet] fennec: trigger release promotion action task from releaserunner

Review commit: https://reviewboard.mozilla.org/r/190906/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190906/

Comment 7

[Rail Aliiev [:rail]]

Feel free to redirect these to aki if you don't have enough time. I thought that it may be interesting to you ;)

Comment 8

[Rail Aliiev [:rail]]

Comment on attachment 8919963 [details]
Bug 1409575 - [puppet] fennec: trigger release promotion action task from releaserunner

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190906/diff/1-2/

Comment 9

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Rail Aliiev [:rail] ⌚️ET from comment #7)
> Feel free to redirect these to aki if you don't have enough time. I thought
> that it may be interesting to you ;)

I thought you never ask! I'm happy to review these, I was wondering how we do the action tasks anyway so looking at them now.

Comment 10

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8919962 [details]
Bug 1409575 - [tools] fennec: trigger release promotion action task from releaserunner

https://reviewboard.mozilla.org/r/190902/#review196290

This is among the most beautiful things I've reviewed so far! \o/ \o/ \o/
Now I want to work on tcmigration too :P

::: buildfarm/release/release-runner3.py:1
(Diff revision 1)
> +#!/usr/bin/env python

Nit: Might be useful to add a docstring to explain the purpose of this release-runner3. We should do the same for release-runner2.py and release-runner.py. Avoids confusion for the following months.

::: buildfarm/release/release-runner3.py:11
(Diff revision 1)
> +import re
> +import site
> +import sys
> +import taskcluster
> +import time
> +import yaml

Nice reordering of the imports. Bah, I should've done the same for release-runner2.py.

::: buildfarm/release/release-runner3.py:172
(Diff revision 1)
> +    log.debug('Sleeping for %s seconds before polling again', sleeptime)
> +    time.sleep(sleeptime)
> +
> +
> +if __name__ == '__main__':
> +    parser = argparse.ArgumentParser()

++ for s/OptParse/argparse!

::: buildfarm/release/release-runner3.sh:6
(Diff revision 1)
> +#!/bin/bash
> +
> +VENV=$1
> +LOGFILE=$2
> +CONFIG=$3
> +

Neat cleanup of old `if [-Z {VENV, LOGFILE, CONFIG}]`. We should maybe do the same for release-runner2.sh ;-)

::: buildfarm/release/trigger_action.py:1
(Diff revision 1)
> +import argparse

This is the *new* releasetasks_graph_gen.py. Holy smokes, this is brilliant!

::: buildfarm/release/trigger_action.py:16
(Diff revision 1)
> +
> +from kickoff.actions import generate_action_task, submit_action_task
> +
> +log = logging.getLogger(__name__)
> +AVAILABLE_ACTIONS = {
> +    "fennec": ["publish_fennec"]

If we don't need the `product_from_action_flavor`, there's no need to define this as a dict, we can simply go with a list full of flavors.

::: buildfarm/release/trigger_action.py:20
(Diff revision 1)
> +AVAILABLE_ACTIONS = {
> +    "fennec": ["publish_fennec"]
> +}
> +
> +
> +def product_from_action_flavor(flavor):

Unused function.

::: buildfarm/release/trigger_action.py:76
(Diff revision 1)
> +    log.info("Revision: %s", parameters["head_rev"])
> +    log.info("Next version: %s", action_input["next_version"])
> +    log.info("Build number: %s", action_input["build_number"])
> +    log.info("Task definition:\n%s", json.dumps(action_task, sort_keys=True, indent=2))
> +    if not args.force:
> +        yes_no = raw_input("Submit the task? [y/N]: ")

I don't see why we need this. I guess it's nicer to see that interactive command and control the submission but it's the same as we'd set the `--force` isn't it?

::: lib/python/kickoff/actions.py:2
(Diff revision 1)
> +import copy
> +import jsone

Unclear to me where does this come from. Is this the same thing with the Earlang Json package? I can't find it installed via puppet nor any reference on it in Pypi.

::: lib/python/kickoff/actions.py:15
(Diff revision 1)
> +
> +def find_action(name, actions):
> +    for action in actions["actions"]:
> +        if action["name"] == name:
> +            return copy.deepcopy(action)
> +    else:

Nit: useless else. can directly return None outside of the loop.

::: lib/python/kickoff/actions.py:28
(Diff revision 1)
> +    q.raise_for_status()
> +    return q.json()
> +
> +
> +def generate_action_task(project, revision, next_version, build_number, release_promotion_flavor):
> +    decsision_task_route = "gecko.v2.{project}.revision.{revision}.firefox.decision".format(

typo in variable name.

Comment 11

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8919962 [details]
Bug 1409575 - [tools] fennec: trigger release promotion action task from releaserunner

https://reviewboard.mozilla.org/r/190902/#review196332

Comment 12

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8919962 [details]
Bug 1409575 - [tools] fennec: trigger release promotion action task from releaserunner

https://reviewboard.mozilla.org/r/190902/#review196290

> Unclear to me where does this come from. Is this the same thing with the Earlang Json package? I can't find it installed via puppet nor any reference on it in Pypi.

Nevermind me, found https://pypi.python.org/pypi/json-e in the meantime + on puppet.

Comment 13

[Mihai Tabara [:mtabara]⌚️GMT]

Comment on attachment 8919963 [details]
Bug 1409575 - [puppet] fennec: trigger release promotion action task from releaserunner

https://reviewboard.mozilla.org/r/190906/#review196356

B-E-A-U-T-I-F-U-L!

::: manifests/moco-nodes.pp:674
(Diff revision 2)
>  node 'buildbot-master83.bb.releng.scl3.mozilla.com' {
>      $aspects           = [ 'high-security' ]
>      $only_user_ssh     = true
>      $releaserunner_env = 'dev'
>      $releaserunner2_env = 'dev-fennec'
> +    $releaserunner3_env = 'dev'

Neat! I should've done the same for release-runner2. Not sure why I used `dev-fennec`.

::: modules/releaserunner3/manifests/init.pp:10
(Diff revision 2)
>      include users::builder
> -    include releaserunner2::settings
> -    include releaserunner2::services
> -    include packages::mozilla::python27
>      include packages::gcc
> -    include packages::libffi
> +    include releaserunner3::settings

Nice cleanup of these!

::: modules/releaserunner3/manifests/init.pp
(Diff revision 2)
> -                'cffi==0.8.6',
> -                'chunkify==1.2',
> -                'cryptography==0.6',
> -                'decorator==3.4.0',
> -                'ecdsa==0.10',
> +                'json-e==2.3.1',
> +                'mohawk==0.3.4',
> +                'requests==2.18.4',
> +                'simplejson==3.11.1',
> +                'six==1.11.0',
> -                'enum34==1.0.4',

Nice to see so many packages go away!

::: modules/releaserunner3/manifests/init.pp
(Diff revision 2)
> -            mode      => '0600',
> -            owner     => $users::builder::username,
> -            group     => $users::builder::group,
> -            content   => template('releaserunner2/release-runner.yml.erb'),
> -            show_diff => false;
> -        "${releaserunner2::settings::root}/docker-worker-pub.pem":

So nice to see we no longer need this deployed nor used via releasetasks!

Comment 14

[Rail Aliiev [:rail]]

Comment on attachment 8919962 [details]
Bug 1409575 - [tools] fennec: trigger release promotion action task from releaserunner

https://reviewboard.mozilla.org/r/190902/#review196290

> I don't see why we need this. I guess it's nicer to see that interactive command and control the submission but it's the same as we'd set the `--force` isn't it?

I added this safeguard to let a releaseduty to verify at least the major things, like build number. You can say this is an interactive dry run. :) I added `--force` in case we call this from another script.

> Nit: useless else. can directly return None outside of the loop.

I prefer to use for/else (it's not a if/else ;) ). To me it looks like I have more control on the flow and it looks explicit. If you decide to edit this block, it's easier with this approach. Just a personal preference. :)

Comment 15

[Rail Aliiev [:rail]]

Comment on attachment 8919962 [details]
Bug 1409575 - [tools] fennec: trigger release promotion action task from releaserunner

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/190902/diff/1-2/

Comment 16

[Rail Aliiev [:rail]]

Aki, one thing that I wanted to clarify.

in the patch https://reviewboard.mozilla.org/r/190902/diff/2#index_header I use the following code:

  action_task = jsone.render(relpro["task"], context)
  # override ACTION_TASK_GROUP_ID, so we know the new ID in advance
  action_task["payload"]["env"]["ACTION_TASK_GROUP_ID"] = action_task_id

Normally ACTION_TASK_GROUP_ID is preset to the original decision task taskId. The action task (normally again) generates a bunch of tasks and uses a separate taskGroupId for the new graph (in our case), but we don't know it in advance. With the hack above I know the new taskGroupId in advance (so I can email it from release runner), but I'm worried that it may affect other things, especially CoT.

Do you think I should drop the hack and wait for the action task to finish before I can email?

Comment 17

[Rail Aliiev [:rail]]

I also wonder if this hack somehow affects the dependency chain. I vaguely remember seeing a lot of tasks in the publush_fennec graph...

Comment 18

[Aki Sasaki (not active)]

I think it's ok.

By default (outside of release runner), the action task will be part of the original on-push graph, but it will spawn a taskGroupId that is the same as the action task's taskId. However, I think as long as task.extra.parent is set to the on-push decision task, we're good; we can specify the action task's taskGroupId. We should be able to verify on maple. If it breaks something in cot, I think we can fix cot to allow for this.

Comment 19

[Rail Aliiev [:rail]]

Thank you! I'm deploying this now! :)

Comment 20

[Rail Aliiev [:rail]]

Comment on attachment 8919962 [details]
Bug 1409575 - [tools] fennec: trigger release promotion action task from releaserunner

https://hg.mozilla.org/build/tools/rev/6dcd06595cfb648561d30b6566556909b7b5daf9

Comment 21

[Rail Aliiev [:rail]]

Comment on attachment 8919963 [details]
Bug 1409575 - [puppet] fennec: trigger release promotion action task from releaserunner

https://hg.mozilla.org/build/puppet/rev/9b340e5acad9d57c89eb7152202a58ede2102b9d

Comment 22

[Rail Aliiev [:rail]]

I ended up adding this missing piece: https://hg.mozilla.org/build/puppet/diff/9b340e5acad9/modules/toplevel/manifests/mixin/releaserunner3.pp

Comment 23

[Mihai Tabara [:mtabara]⌚️GMT]

(In reply to Rail Aliiev [:rail] ⌚️ET from comment #14)
> Comment on attachment 8919962 [details]
> Bug 1409575 - [tools] fennec: trigger release promotion action task from
> releaserunner
> 
> https://reviewboard.mozilla.org/r/190902/#review196290
> 
> > I don't see why we need this. I guess it's nicer to see that interactive command and control the submission but it's the same as we'd set the `--force` isn't it?
> 
> I added this safeguard to let a releaseduty to verify at least the major
> things, like build number. You can say this is an interactive dry run. :) I
> added `--force` in case we call this from another script.

Sounds good, makes sense!
 
> > Nit: useless else. can directly return None outside of the loop.
> 
> I prefer to use for/else (it's not a if/else ;) ). To me it looks like I
> have more control on the flow and it looks explicit. If you decide to edit
> this block, it's easier with this approach. Just a personal preference. :)

Fine by me! :)

(In reply to Rail Aliiev [:rail] ⌚️ET from comment #22)
> I ended up adding this missing piece:
> https://hg.mozilla.org/build/puppet/diff/9b340e5acad9/modules/toplevel/
> manifests/mixin/releaserunner3.pp

Ah, I knew I'd miss something in the puppet patches, mea culpa!

Comment 24

[Rail Aliiev [:rail]]

Attachment: dump_json.diff

Pretty print the task definition in the logs

Comment 25

[Rail Aliiev [:rail]]

Comment on attachment 8920405 [details] [diff] [review]
dump_json.diff

https://hg.mozilla.org/build/tools/rev/53e724372a9e1b72c4afa32dde8673faa444b233

Comment 26

[Rail Aliiev [:rail]]

Attachment: WIP: previous_graph_ids.diff

I forgot that we need to pass previous_graph_ids (at least). This is a WIP patch. Probably it should be more flexible to be able to handle Firefox.

Comment 27

[Rail Aliiev [:rail]]

Attachment: tools-previous_graph_ids.diff

This should work fine for Fennec, but for Firefox we will something that would accept of find previous action_graph_ids. In Firefox case we'll have 1 decision and 3 action tasks (promote, push to cdns, publish) or 2 action tasks for betas (promote will include push to cdns).

Comment 28

[Rail Aliiev [:rail]]

Comment on attachment 8920689 [details] [diff] [review]
tools-previous_graph_ids.diff

https://hg.mozilla.org/build/tools/rev/dcfa5e051898682627c4278ac746819e9bd6ef10