This may require another fork of release runner \o/

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

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

(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 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 on attachment 8919962 [details] Bug 1409575 - [tools] fennec: trigger release promotion action task from releaserunner https://reviewboard.mozilla.org/r/190902/#review196332

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 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 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. :)

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?

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

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.

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

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 on attachment 8919963 [details] Bug 1409575 - [puppet] fennec: trigger release promotion action task from releaserunner https://hg.mozilla.org/build/puppet/rev/9b340e5acad9d57c89eb7152202a58ede2102b9d

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

(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!

Pretty print the task definition in the logs

Comment on attachment 8920405 [details] [diff] [review] dump_json.diff https://hg.mozilla.org/build/tools/rev/53e724372a9e1b72c4afa32dde8673faa444b233

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.

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 on attachment 8920689 [details] [diff] [review] tools-previous_graph_ids.diff https://hg.mozilla.org/build/tools/rev/dcfa5e051898682627c4278ac746819e9bd6ef10