1215204 - Add bouncer submitter to release promotion graph

Status: RESOLVED FIXED

(Release Engineering :: Release Automation, defect)

Description

[Rail Aliiev [:rail]]

The builder definition may need some changes to not require config time variables, like version number, build number etc.

Comment 1

[Kim Moir [:kmoir] ET]

A few questions

So we are just going to replace the existing builders here
http://hg.mozilla.org/build/buildbotcustom/file/664d99eb01d6/process/release.py#l1709
with jobs that can go through the buildbot bridge

The changes to not require require config time variables, like version number, build number etc. are because these are provided in releasetasks?

Comment 2

[Rail Aliiev [:rail]]

(In reply to Kim Moir [:kmoir] from comment #1)
> A few questions
> 
> So we are just going to replace the existing builders here
> http://hg.mozilla.org/build/buildbotcustom/file/664d99eb01d6/process/release.
> py#l1709
> with jobs that can go through the buildbot bridge

At this stage we are not replacing the builder yet. You need to add a new builder with a different name (including the branch name in the builder name should solve the problem) to generateReleasePromotionBuilders(), see http://hg.mozilla.org/build/buildbotcustom/file/664d99eb01d6/process/release.py#l2079.
 
> The changes to not require require config time variables, like version
> number, build number etc. are because these are provided in releasetasks?

Correct. These variables will be set in the TC task generated by releasetasks. The only way to pass some variables via BBB runtime is using properties, similar to https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/l10n.yml.tmpl#L28-L36

You may need to change the script to read those properties, similar to https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/desktop_l10n.py#335 and later access them similar to https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/scripts/desktop_l10n.py#336

Comment 3

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bb.patch

Comment 4

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bbcustom.patch

Comment 5

[Kim Moir [:kmoir] ET]

Attachment: bug1215204mh.patch

Comment 6

[Kim Moir [:kmoir] ET]

Attachment: bug1215204templates.patch

This is a draft of the templates file, there is still some content to be added

Comment 7

[Kim Moir [:kmoir] ET]

Comment on attachment 8689047 [details] [diff] [review]
bug1215204bbcustom.patch

Rail, I forgot to mention that we discussed yesterday that the properties that are defined in release tasks could be removed but I tried that and then the checkconfig failed.  So I left them in so they could be overwritten

Comment 8

[Kim Moir [:kmoir] ET]

Comment on attachment 8689047 [details] [diff] [review]
bug1215204bbcustom.patch

Not sure that this has everything but I need to test it and it is difficult to do so without landing and testing

Comment 9

[Kim Moir [:kmoir] ET]

Attachment: bug1215204templates.patch

Comment 10

[Rail Aliiev [:rail]]

Comment on attachment 8689047 [details] [diff] [review]
bug1215204bbcustom.patch

Review of attachment 8689047 [details] [diff] [review]:
-----------------------------------------------------------------

In overall you are in the proper direction, just need to address 2 major things:

1) don't iterate over platforms, the script has to be run once.
2) clean up unused properties

::: process/release.py
@@ +1853,5 @@
>              },
>          }
>          builders.append(l10n_builder)
>  
> +    for platform in config["release_platforms"]:

I don't think you need to iterate over all platforms. The script should handle all of them in a single run. Actually, it won't add new platforms after the first run, because ti tries to be idempotent and checks if the product already exists and if it does, the script skips it.

So there should be a single call of this script, not per platform.

@@ +1857,5 @@
> +    for platform in config["release_platforms"]:
> +        bouncer_mh_cfg = {
> +            "script_name": "scripts/bouncer_submitter.py",
> +            "extra_args": [
> +                 "-c",  "releases/bouncer_%s_release.py" % pf['product_name'],

this won't work for beta, for example. 
I'd move bouncer_submitter_config (https://hg.mozilla.org/build/buildbot-configs/file/tip/mozilla/release-fennec-mozilla-release.py.template#l178) to branch config, like you did in https://hg.mozilla.org/build/buildbot-configs/rev/93d843ea9c08

@@ +1876,5 @@
> +                                                  )
> +      
> +        bouncer_builder = {
> +            "name": bouncer_buildername, 
> +            "slavenames": pf["slaves"],

this would run windows submission on windows slave, but there is no reason to do that. Once you get rid of the "for platform ..." loop, you can use linux+linux64 slaves for this job.

@@ +1877,5 @@
> +      
> +        bouncer_builder = {
> +            "name": bouncer_buildername, 
> +            "slavenames": pf["slaves"],
> +            'category': bouncer_buildername, 

this should be builderPrefix(''), it's the same for all builder, so we can group them

@@ +1883,5 @@
> +            'slavebuilddir': normalizeName(bouncer_buildername),
> +            'factory': bouncer_submitter_factory,
> +            #'env': builder_env
> +            'properties': {
> +                'slavebuilddir': normalizeName(bouncer_buildername), 

I don't think you need "slavebuilddir" in properties.

@@ +1884,5 @@
> +            'factory': bouncer_submitter_factory,
> +            #'env': builder_env
> +            'properties': {
> +                'slavebuilddir': normalizeName(bouncer_buildername), 
> +                'balrog_api_root': config['balrog_api_root'],

I don't think this script interacts with balrog, does it?

@@ +1886,5 @@
> +            'properties': {
> +                'slavebuilddir': normalizeName(bouncer_buildername), 
> +                'balrog_api_root': config['balrog_api_root'],
> +                'bouncerServer': config['bouncerServer'],
> +                'tuxedoServerUrl':  config['tuxedoServerUrl'],

it's already passed in extra_args, no need to pass it again.

@@ +1887,5 @@
> +                'slavebuilddir': normalizeName(bouncer_buildername), 
> +                'balrog_api_root': config['balrog_api_root'],
> +                'bouncerServer': config['bouncerServer'],
> +                'tuxedoServerUrl':  config['tuxedoServerUrl'],
> +                'bouncerServer': config['bouncerServer'], 

This is passed twice. I don't think you need this.

@@ +1892,5 @@
> +                 #restructure it or use json.dumps (see l10n script)
> +                 'bouncer_aliases': {
> +                     'Firefox-%(version)s': 'firefox-latest',
> +                     'Firefox-%(version)s-stub': 'firefox-stub',
> +                 },

This won't work, because properties have to be strings. I don't think it is used by the script, so you probably don't need this.

@@ +1894,5 @@
> +                     'Firefox-%(version)s': 'firefox-latest',
> +                     'Firefox-%(version)s-stub': 'firefox-stub',
> +                 },
> +                 #this should be in releasetasks
> +                 'platform': platform,

should be platform=None to make buildbot happy.

@@ +1896,5 @@
> +                 },
> +                 #this should be in releasetasks
> +                 'platform': platform,
> +                 'product': pf['product_name'],
> +                 'branch': 'releases/date',

shouldn't be hardcoded.

@@ +1897,5 @@
> +                 #this should be in releasetasks
> +                 'platform': platform,
> +                 'product': pf['product_name'],
> +                 'branch': 'releases/date',
> +                 'bouncer_submitter_config': "releases/bouncer_%s_release.py" % pf['product_name'],                  

this is already in extra_args.

Comment 11

[Rail Aliiev [:rail]]

Comment on attachment 8689050 [details] [diff] [review]
bug1215204mh.patch

Review of attachment 8689050 [details] [diff] [review]:
-----------------------------------------------------------------

You also need to override --previous-version with multiple values. The values should be passed from shipt-it via releasetasks. something similar to https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/funsize.yml.tmpl#L1

Something like

...
  task:
...
    payload:
...
     properties:
...
       - partial_versions: {% for partial_version, partial_info in partial_updates.iteritems() %}{{ partial_version }}{% endfor %}
...

and then something like

self.config["prev_versions"] = self.buildbot_config["properties"].get("partial_versions").split()

::: testing/mozharness/scripts/bouncer_submitter.py
@@ +72,5 @@
> +        #override properties from buildbot properties here as defined by taskcluster properties
> +        self.read_buildbot_config()
> +
> +        #check if release promotion is true first before overwriting these properties
> +        #? do I need to define buildbot_json_path

I don't think you need to define it.

@@ +76,5 @@
> +        #? do I need to define buildbot_json_path
> +        if self.buildbot_config["properties"].get("release_promotion"):
> +            for prop in ['product', 'version', 'build_number', 'revision', 'bouncer_submitter_config']:
> +                if self.buildbot_config["properties"].get(prop):
> +                    self.info("Overriding en_us_binary_url with %s" %

the log message is wrong.

Comment 12

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bbcustom2.patch

I couldn't get 'category': builderPrefix('') to be in scope and work

Comment 13

[Kim Moir [:kmoir] ET]

Attachment: bug1215204mh2.patch

Comment 14

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bbconfigs2.patch

Comment 15

[Kim Moir [:kmoir] ET]

Attachment: bug1215204mh3.patch

Comment 16

[Kim Moir [:kmoir] ET]

Attachment: bug1215204templates2.patch

Comment 17

[Kim Moir [:kmoir] ET]

Comment on attachment 8689200 [details] [diff] [review]
bug1215204bbcustom2.patch

I couldn't get 'category': builderPrefix('') to be in scope and work, not sure how to do this part (re your last review comments)

Comment 18

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/93d843ea9c08

Comment 19

[Rail Aliiev [:rail]]

Comment on attachment 8689200 [details] [diff] [review]
bug1215204bbcustom2.patch

Review of attachment 8689200 [details] [diff] [review]:
-----------------------------------------------------------------

Almost there! :)

::: process/release.py
@@ +1875,5 @@
> +      
> +    bouncer_builder = {
> +                      "name": bouncer_buildername, 
> +                      "slavenames": config["platforms"]["linux"]["slaves"] + config["platforms"]["linux64"]["slaves"],
> +                      'category': name,

category has to be builderPrefix('')

@@ +1883,5 @@
> +                      #'env': builder_env,
> +                      'properties': {
> +                                     'bouncerServer': config['bouncerServer'],
> +                                     'tuxedoServerUrl':  config['tuxedoServerUrl'],
> +                                     'bouncerServer': config['bouncerServer'], 

I don't think you need these 3 (actually 2, one duplicate) properties. The only valuable one (tuxedoServerUrl) is already passed via extra_args as --bouncer-api-prefix

@@ +1884,5 @@
> +                      'properties': {
> +                                     'bouncerServer': config['bouncerServer'],
> +                                     'tuxedoServerUrl':  config['tuxedoServerUrl'],
> +                                     'bouncerServer': config['bouncerServer'], 
> +                      #these should be in releasetasks

You can remove this comment, the following 3 properties are required by log_upload.py

Comment 20

[Rail Aliiev [:rail]]

Comment on attachment 8689549 [details] [diff] [review]
bug1215204bbconfigs2.patch

Review of attachment 8689549 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozilla/project_branches.py
@@ +197,3 @@
>          'bouncerServer': 'download.mozilla.org',
>          'tuxedoServerUrl':  'https://bounceradmin.mozilla.com/api',
>          'bouncerServer': 'download.mozilla.org',

Can you remove bouncerServer entries when you land this? I don't think we use them yet.

Comment 21

[Rail Aliiev [:rail]]

Comment on attachment 8689555 [details] [diff] [review]
bug1215204mh3.patch

Review of attachment 8689555 [details] [diff] [review]:
-----------------------------------------------------------------

Feel free to land it on date to test.

::: testing/mozharness/scripts/bouncer_submitter.py
@@ +78,5 @@
> +                if self.buildbot_config["properties"].get(prop):
> +                    self.info("Overriding %s with %s" %
> +                              prop, self.buildbot_config["properties"][prop])
> +                    self.config[prop] = self.buildbot_config["properties"][prop]
> +            if self.buildbot_config["properties".get("partial_versions")]:

should be
 if self.buildbot_config["properties"].get("partial_versions"):

Comment 22

[Rail Aliiev [:rail]]

Comment on attachment 8689564 [details] [diff] [review]
bug1215204templates2.patch

Review of attachment 8689564 [details] [diff] [review]:
-----------------------------------------------------------------

BTW, it would be great to add unit tests tests and you can use github PRs for these patches so you get tests running automatically on travis.

::: releasetasks/templates/bouncer.yml.tmpl
@@ +1,1 @@
> +{% for platform, platform_info in bouncer_config["platforms"].iteritems() %}

It doesn't match the buildername(s) in buildbot. Don't need to iterate over platforms.

@@ +1,5 @@
> +{% for platform, platform_info in bouncer_config["platforms"].iteritems() %}
> +# TODO: make a helper function to generate consistent builder names?
> +{% set buildername = "release-{}_{}_{}__bncr_sub".format(branch, product, platform) %}
> +-
> +    taskId: "{{ stableSlugId('{}'.format(buildername)) }}"

taskId: "{{ stableSlugId(buildername) }}"

looks simpler ;)

Comment 23

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bbconfigs3.patch

fixed project branches patch

Comment 24

[Kim Moir [:kmoir] ET]

Attachment: bug1215204templates3.patch

Comment 25

[Kim Moir [:kmoir] ET]

Attachment: bug1215204mh4.patch

Comment 26

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bbcustom3.patch

In comment 19, rail suggested that category has to be builderPrefix('').  However, builderPrefix is not in scope for 
generateReleasePromotionBuilders since it is defined within generateReleaseBranchObjects.  So I just used the bit that applied to generateReleasePromotionBuilders for the category

Comment 27

[Jordan Lund (:jlund)]

Comment on attachment 8691534 [details] [diff] [review]
bug1215204bbcustom3.patch

Review of attachment 8691534 [details] [diff] [review]:
-----------------------------------------------------------------

looks good to me. I didn't cross examine line by line against the 'old way' but I suspect we can iterate on this if we need to :)

tiny nit: there are about 5 spots where bugzilla's splinter view picked up whitespaces.

::: process/release.py
@@ +1854,5 @@
>          builders.append(l10n_builder)
> +  
> +    bouncer_mh_cfg = {
> +        "script_name": "scripts/bouncer_submitter.py",
> +        "extra_args": [

so args like --build-num, --version, and --revision no longer required?

@@ +1873,5 @@
> +                                             )
> +      
> +    bouncer_builder = {
> +                      "name": bouncer_buildername, 
> +                      "slavenames": config["platforms"]["linux"]["slaves"] + config["platforms"]["linux64"]["slaves"],

oh neat, I didn't realize we could use linux32 or 64 for this.

@@ +1877,5 @@
> +                      "slavenames": config["platforms"]["linux"]["slaves"] + config["platforms"]["linux64"]["slaves"],
> +                      "builddir": bouncer_buildername,
> +                      "slavebuilddir": normalizeName(bouncer_buildername),
> +                      "factory": bouncer_submitter_factory,
> +                      "category": "release-%s-%s" % (config["bouncer_branch"], ''),

category ends with '-'?

Comment 28

[Jordan Lund (:jlund)]

Comment on attachment 8690093 [details] [diff] [review]
bug1215204templates3.patch

Review of attachment 8690093 [details] [diff] [review]:
-----------------------------------------------------------------

awesome. please bear with me as I get up to speed with you.

setting to r- for now. as always, it can be persuaded to an r+ if I am wrong :)

::: releasetasks/templates/bouncer.yml.tmpl
@@ +1,2 @@
> +# TODO: make a helper function to generate consistent builder names?
> +{% set buildername = "release-{}_{}__bncr_sub".format(branch, product) %}

there is two underscores btween product and bncr. iiuc, this has to match the buildername you defined in bbot-cfg's release.py

@@ +23,5 @@
> +                product: "{{ product }}"
> +                # Quotes cannot be used around this string because the loop causes it to have trailing whitespace
> +                # (which gets stripped by the yaml parser when unquoted). Kindof hacky.
> +                version: {{ version }}
> +                build_number: {{ buildNumber }}

ah right, we moved build number, revision, partials, and version here because they are release specific. ignore my comment in other attachment

@@ +26,5 @@
> +                version: {{ version }}
> +                build_number: {{ buildNumber }}
> +                release_promotion: true
> +                revision: "{{ revision }}"
> +                partial_versions: {% for partial_version, partial_info in partial_updates.iteritems() %}{{ partial_version }}{% endfor %}

I'm a jinja noob. It looks like the output of this will be a string of partials concatenated together without a delim. Am I reading that right?

@@ +33,5 @@
> +            description: "Release Promotion bouncer submission job"
> +            owner: "[e-mail]"
> +            source: https://github.com/mozilla/releasetasks
> +
> +        {% if running_tests is defined %}

do we have tests for this?

@@ +39,5 @@
> +            task_name: "{{ buildername }}"
> +        {% endif %}
> +
> +{% endfor %}
> +{% endfor %}

hrm, iiuc, these two endfors are not closing any loops

Comment 29

[Kim Moir [:kmoir] ET]

Attachment: bug1215204templates4.patch

Re review comments from comment 28

I don't have tests yet, I have to add them


I'm a jinja noob too.  
For this part
 partial_versions: {% for partial_version, partial_info in partial_updates.iteritems() %}{{ partial_version }}{

I just copied it from the funsize template

Comment 30

[Jordan Lund (:jlund)]

Comment on attachment 8692017 [details] [diff] [review]
bug1215204templates4.patch

Review of attachment 8692017 [details] [diff] [review]:
-----------------------------------------------------------------

again, I'm learning as I go so please bear with me if I end up wrong here...

::: releasetasks/templates/bouncer.yml.tmpl
@@ +1,3 @@
> +# TODO: make a helper function to generate consistent builder names?
> +{% set buildername = "release-{}_{}_bncr_sub".format(branch, product) %}
> +{% for partial_version, partial_info in partial_updates.iteritems() %}

hm, so what are we trying to do here? this will create a task for each partial iiuc and each task would trigger the same buildbot builder with the same properties no? Don't we just want to create one bouncer submitter task?

@@ +27,5 @@
> +                version: {{ version }}
> +                build_number: {{ buildNumber }}
> +                release_promotion: true
> +                revision: "{{ revision }}"
> +                partial_versions: {% for partial_version, partial_info in partial_updates.iteritems() %}{{ partial_version }}{% endfor %}

this is the same look as the one at the top.

iiuc you are trying to account for: https://dxr.mozilla.org/build-central/source/buildbotcustom/process/release.py#1444 by passing the previous partial versions via buildbot properties.

But what I still think we need to do is have separator between partial versions so we don't end up with something like: partial_versions: "44.0b144.0b341.b3'

maybe we should try something like:
code: https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/l10n.yml.tmpl#L33
output: https://queue.taskcluster.net/v1/task/Fyw6nQxMSNGQxSSfddXqAQ

Comment 31

[Kim Moir [:kmoir] ET]

Attachment: bug1215204templates5.patch

Sorry for the many review requests. I find this hard to write because I can't test it first, which I usually do so I'm kind of flying blind.  I added the formatting for the partial_versions

Comment 32

[Jordan Lund (:jlund)]

Comment on attachment 8692578 [details] [diff] [review]
bug1215204templates5.patch

Review of attachment 8692578 [details] [diff] [review]:
-----------------------------------------------------------------

I hear yeah. This looks great. We can iterate on this once we see what it's like on date :)

Comment 33

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bbcustom3nowhitespace.patch

Comment 34

[Kim Moir [:kmoir] ET]

Comment on attachment 8692997 [details] [diff] [review]
bug1215204bbcustom3nowhitespace.patch

in production

Comment 35

[Kim Moir [:kmoir] ET]

Have a pull request that passes
https://github.com/mozilla/releasetasks/pull/56

Not sure if I should merge as it seems I can


kmoir	I issued a pull request for my patches for the bouncer submitter but it failed https://github.com/mozilla/releasetasks/pull/54/files
	kmoir	https://tools.taskcluster.net/task-graph-inspector/#VYZ_aV80Suqlh6ERzQIQFg/HoaNq5VESVuxh1w64jfijA
	kmoir	not sure how to debug what went wrong
	jlund	kmoir: so I think everytime we call https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/release_graph.yml.tmpl
	jlund	via https://github.com/mozilla/releasetasks/blob/77dc076c4c8868eb0ca159f3b5b0bba4d4d9cd81/releasetasks/__init__.py#L17
	jlund	in all our tests (and of course in releaserunner), we need to add a 'bouncer_enabled' arg
	jlund	e.g. https://github.com/mozilla/releasetasks/blob/master/releasetasks/test/test_releasetasks.py#L62
	kmoir	jlund: thanks!
	jlund	kmoir: can you see the log and the failures in the taskcluster task url you pasted above?
	jlund	it's in the 'Run 0' tab

Comment 36

[Kim Moir [:kmoir] ET]

So I'm trying to run this in release runner and it is failing to create the task graph

https://index.taskcluster.net/v1/task/buildbot.revisions.c5283bf0194859ba1f0a3e3b0c2d889456b625e4.date.linux

Not sure why, going to ask rail about it next week at our all hands

Comment 37

[Rail Aliiev [:rail]]

Looks like you need to add "bouncer_enabled": branchConfig[something_that_enables_bouncer] to kwargs in 
https://hg.mozilla.org/build/tools/file/tip/buildfarm/release/release-runner.py#l267 which is passed to make_task_graph() in https://hg.mozilla.org/build/tools/file/tip/buildfarm/release/release-runner.py#l301

Comment 38

[Rail Aliiev [:rail]]

Kim, can you also add releasetasks unittests for this feature.

Comment 39

[Kim Moir [:kmoir] ET]

Attachment: bug1215204tools.patch

Comment 40

[Rail Aliiev [:rail]]

Comment on attachment 8697357 [details] [diff] [review]
bug1215204tools.patch

Hmm, I don't see this variable name anywhere in the configs. Maybe you forgot to add it to project_branches.py?

Comment 41

[Kim Moir [:kmoir] ET]

Attachment: bug1215204date.patch

Comment 42

[Kim Moir [:kmoir] ET]

Attachment: bug1215204date.patch

Comment 43

[Rail Aliiev [:rail]]

Comment on attachment 8697382 [details] [diff] [review]
bug1215204date.patch

conditional r+: please remove surrounding quotes, it should be True.

Comment 44

[Kim Moir [:kmoir] ET]

Comment on attachment 8697382 [details] [diff] [review]
bug1215204date.patch

http://hg.mozilla.org/build/buildbot-configs/rev/9cbdc3b456f1

removed quotes

Comment 45

[Kim Moir [:kmoir] ET]

Comment on attachment 8697357 [details] [diff] [review]
bug1215204tools.patch

asking for review again since I updated project_branches.py

Comment 46

[Kim Moir [:kmoir] ET]

So releaserunner is failing like this

Traceback (most recent call last):
  File "release-runner.py", line 302, in main
    graph = make_task_graph(**kwargs)
  File "/home/cltbld/releasetasks/releasetasks/__init__.py", line 50, in make_task_graph
    return yaml.safe_load(template.render(**template_vars))
  File "/builds/releaserunner/lib/python2.7/site-packages/jinja2/environment.py", line 894, in render
    return self.environment.handle_exception(exc_info, True)
  File "/home/cltbld/releasetasks/releasetasks/templates/release_graph.yml.tmpl", line 69, in top-level template code
    {{ bouncer_tasks()|indent(4) }}
  File "/home/cltbld/releasetasks/releasetasks/templates/release_graph.yml.tmpl", line 68, in template
    {% macro bouncer_tasks() %}{% include "bouncer.yml.tmpl" %}{% endmacro %}
  File "/home/cltbld/releasetasks/releasetasks/templates/bouncer.yml.tmpl", line 30, in top-level template code
    partial_versions: {% for p in partial_version %}{{ "{}, ".format(p) }}{% endfor %}
UndefinedError: 'partial_version' is undefined

because partial_versions is not defined properly in the template

I changed it to 
partial_versions: {% for p in partial_updates.iteritems() %}{{ "{}, ".format(p) }}{% endfor %}

https://github.com/kmoir/releasetasks/commit/20e8472b63447a4c6051ecb2d273ae1875b44bde

but the task graph is failing by travis, not sure if this is because there are previous commits from other people that are causing it to fail. If you have suggestions on how to fix this it would be appreciated

Comment 47

[Kim Moir [:kmoir] ET]

so problem is that product is undefined for task

Comment 48

[Rail Aliiev [:rail]]

I believe you need to pass more variables to the call, something like in https://github.com/rail/releasetasks/blob/master/releasetasks/test/test_releasetasks.py#L174

Comment 49

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bouncerenabled.patch

missing bouncer_enabled in process/release.py

Comment 50

[Rail Aliiev [:rail]]

Comment on attachment 8700741 [details] [diff] [review]
bug1215204bouncerenabled.patch

Review of attachment 8700741 [details] [diff] [review]:
-----------------------------------------------------------------

::: process/release.py
@@ +1882,5 @@
>                        "properties": {
>                            "platform": None,
>                            "product": pf["product_name"],
> +                          "branch": config["bouncer_branch"],
> +                          "branch": config["bouncer_enabled"]

Hmm, forgot to fix the first part? "branch" is set twice here. Probably should be "bouncer_enabled"?

Comment 51

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bouncerenabled2.patch

Comment 52

[Kim Moir [:kmoir] ET]

current status

job is being scheduled but has error
kmoir> now the error is that it  "Failed to infer task payload format"
4:53 PM <kmoir> https://tools.taskcluster.net/task-graph-inspector/#Q02OdIodR8CtxK9--UX4gQ/oY1Aif18RWmroandG-OFnQ/
4:54 PM <kmoir> also noticing the scope is buildbot-bridge:builder-name:release-date_firefox_bncr_sub
4:54 PM <kmoir> not sure if I have to change something there

Comment 53

[Rail Aliiev [:rail]]

(In reply to Kim Moir [:kmoir] from comment #52)
> current status
> 
> job is being scheduled but has error
> kmoir> now the error is that it  "Failed to infer task payload format"
> 4:53 PM <kmoir>
> https://tools.taskcluster.net/task-graph-inspector/#Q02OdIodR8CtxK9--UX4gQ/
> oY1Aif18RWmroandG-OFnQ/

It looks like buildbot-bridge is broken now, this not the only task with exception. I see l10n repacks orange, even though they worked fine in the past.

I tried to grep logs, but they are too brief:

[[e-mail] bbb]# grep -C3  oY1Aif18RWmroandG-OFnQ *.log                                                                
tclistener.log-2015-12-22 13:37:21,713 - bbb.servicebase - Fetching task -Qk6iqXcRPODENVayEKlqg
tclistener.log-2015-12-22 13:37:21,720 - requests.packages.urllib3.connectionpool - Starting new HTTPS connection (1): queue.taskcluster.net
tclistener.log-2015-12-22 13:37:22,253 - bbb.servicebase - Fetching task 4B71MPC4TBqjGyqWi5thNQ
tclistener.log:2015-12-22 13:38:29,822 - bbb.services - Handling Taskcluster exception (canceled) for oY1Aif18RWmroandG-OFnQ
tclistener.log:2015-12-22 13:38:29,822 - bbb.servicebase - Fetching task oY1Aif18RWmroandG-OFnQ
tclistener.log-2015-12-22 13:38:29,836 - bbb.services - No task found in our database, nothing to do.

> 4:54 PM <kmoir> also noticing the scope is
> buildbot-bridge:builder-name:release-date_firefox_bncr_sub

The scope LGTM, compared with other jobs.

Comment 54

[Rail Aliiev [:rail]]

It turns out that the job fails in buildbot, see http://buildbot-master72.bb.releng.usw2.mozilla.com:8001/builders/release-date_firefox_bncr_sub/builds/0

Traceback (most recent call last):
  File "/builds/buildbot/build1/lib/python2.7/site-packages/buildbot-0.8.2_hg_41f0fbee10f4_production_0.8-py2.7.egg/buildbot/process/buildstep.py", line 728, in startStep
    d.addCallback(self._startStep_2)
  File "/builds/buildbot/build1/lib/python2.7/site-packages/twisted/internet/defer.py", line 260, in addCallback
    callbackKeywords=kw)
  File "/builds/buildbot/build1/lib/python2.7/site-packages/twisted/internet/defer.py", line 249, in addCallbacks
    self._runCallbacks()
  File "/builds/buildbot/build1/lib/python2.7/site-packages/twisted/internet/defer.py", line 441, in _runCallbacks
    self.result = callback(self.result, *args, **kw)
--- <exception caught here> ---
  File "/builds/buildbot/build1/lib/python2.7/site-packages/buildbot-0.8.2_hg_41f0fbee10f4_production_0.8-py2.7.egg/buildbot/process/buildstep.py", line 769, in _startStep_2
    skip = self.start()
  File "/builds/buildbot/build1/lib/python2.7/site-packages/buildbot-0.8.2_hg_41f0fbee10f4_production_0.8-py2.7.egg/buildbot/steps/shell.py", line 212, in start
    command = properties.render(self.command)
  File "/builds/buildbot/build1/lib/python2.7/site-packages/buildbot-0.8.2_hg_41f0fbee10f4_production_0.8-py2.7.egg/buildbot/process/properties.py", line 108, in render
    return [ self.render(e) for e in value ]
  File "/builds/buildbot/build1/lib/python2.7/site-packages/buildbot-0.8.2_hg_41f0fbee10f4_production_0.8-py2.7.egg/buildbot/process/properties.py", line 106, in render
    return value.render(self.pmap)
  File "/builds/buildbot/build1/lib/python2.7/site-packages/buildbot-0.8.2_hg_41f0fbee10f4_production_0.8-py2.7.egg/buildbot/process/properties.py", line 194, in render
    s = self.fmtstring % pmap
  File "/builds/buildbot/build1/lib/python2.7/site-packages/buildbot-0.8.2_hg_41f0fbee10f4_production_0.8-py2.7.egg/buildbot/process/properties.py", line 169, in __getitem__
    rv = properties[key]
  File "/builds/buildbot/build1/lib/python2.7/site-packages/buildbot-0.8.2_hg_41f0fbee10f4_production_0.8-py2.7.egg/buildbot/process/properties.py", line 50, in __getitem__
    rv = self.properties[name][0]
exceptions.KeyError: 'repo_path'

Comment 55

[Kim Moir [:kmoir] ET]

Attachment: bug1215204fixproperties.patch

patch to add missing repo_path as shown in the error here
http://buildbot-master70.bb.releng.use1.mozilla.com:8001/builders/release-date_firefox_bncr_sub/builds/2/steps/download_and_extract_scripts_archive/logs/err.html

also, add script_repo_revision and change the order of the properties to be the same as the l10n builder

Comment 56

[Rail Aliiev [:rail]]

Comment on attachment 8703700 [details] [diff] [review]
bug1215204fixproperties.patch

lgtm!

Comment 57

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bouncer.patch

to fix
http://buildbot-master91.bb.releng.usw2.mozilla.com:8001/builders/release-date_firefox_bncr_sub/builds/0/steps/run_script/logs/stdio

Comment 58

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bouncer.patch

Comment 59

[Rail Aliiev [:rail]]

Comment on attachment 8703794 [details] [diff] [review]
bug1215204bouncer.patch

I think we wanted to have a separate config for date to avoid collisions with production submission or (better) to use staging/dev bouncer like in https://dxr.mozilla.org/build-central/source/buildbot-configs/mozilla/staging_release-fennec-mozilla-release.py.template#191

Comment 60

[Kim Moir [:kmoir] ET]

Attachment: bug1215204staging.patch

Comment 61

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bouncerconfig.patch

so this is to add the releases/bouncer_firefox_date.py to date, (just copied the beta version) and the previous patch is submit to the bouncer staging server. Hope this is the correct way forward

Comment 62

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bncr.patch

Comment 63

[Kim Moir [:kmoir] ET]

Attachment: bug1215204bouncersub.patch

I landed this on date yesterday

Comment 64

[Kim Moir [:kmoir] ET]

Attachment: bug1215204properties.patch

set buildbot_json_path because it's missing

Comment 65

[Kim Moir [:kmoir] ET]

Attachment: bug1215204mhargs.patch

missing mh args

Comment 66

[Rail Aliiev [:rail]]

Comment on attachment 8704707 [details] [diff] [review]
bug1215204mhargs.patch

a nit, I'd add a trailing coma after the last line, so the next patch looks cleaner

Comment 67

[Rail Aliiev [:rail]]

Comment on attachment 8704706 [details] [diff] [review]
bug1215204properties.patch

302 jlund

I see buildbot_json_path is mostly used in configs, not in code.

Comment 68

[Kim Moir [:kmoir] ET]

Comment on attachment 8704707 [details] [diff] [review]
bug1215204mhargs.patch

landed and added trailing comma

Comment 69

[Jordan Lund (:jlund)]

Comment on attachment 8704706 [details] [diff] [review]
bug1215204properties.patch

Review of attachment 8704706 [details] [diff] [review]:
-----------------------------------------------------------------

this would absolutely work fine but I would ask to tweak it a bit:

::: testing/mozharness/scripts/bouncer_submitter.py
@@ +69,5 @@
>      def _pre_config_lock(self, rw_config):
>          super(BouncerSubmitter, self)._pre_config_lock(rw_config)
>  
>          #override properties from buildbot properties here as defined by taskcluster properties
> +        self.config['buildbot_json_path'] = "buildprops.json"

_pre_config_lock is generally reserved for dynamic config changes that are not known pre run time.

since this value buildbot_json_path is pretty standard and static, we should put it into one of the config files for bouncer

e.g. https://dxr.mozilla.org/build-central/source/mozharness/configs/releases/bouncer_firefox_beta.py?case=true&from=bouncer_firefox_beta.py

saying that, since we will likely end up with more than one config file and you may not wish to put this item in each, you could define this in this class's BaseScript.__init__ under `config`: e.g. like https://dxr.mozilla.org/build-central/source/mozharness/scripts/desktop_l10n.py#168

mh looks for config items in 3 places and merges them by order:

from highest precedence..
1) CLI options
2) 'config' attr within BaseScript init.
3) config files

Comment 70

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbotcustom/rev/93196910e3df

Comment 71

[Release Engineering SlaveAPI Service]

In production: https://hg.mozilla.org/build/buildbotcustom/rev/4cda855c1404

Comment 72

[Kim Moir [:kmoir] ET]

Attachment: bug1215204mhargs2.patch

Comment 73

[Kim Moir [:kmoir] ET]

Attachment: bug1215204date.patch

this worked on when I reran the the job on a spot instance and modified the scripts locally
[e-mail] rel-date_fx_bncr_sub-000000000]$ /tools/buildbot/bin/python scripts/scripts/bouncer_submitter.py -c releases/bouncer_firefox_beta.py --credentials-file oauth.txt --bouncer-api-prefix https://bounceradmin.allizom.org/api --repo projects/date
18:17:23     INFO - MultiFileLogger online at 20160107 18:17:23 in /builds/slave/rel-date_fx_bncr_sub-000000000
18:17:23     INFO - Using buildbot properties:
18:17:23     INFO - {
18:17:23     INFO -     "project": "", 
18:17:23     INFO -     "product": "firefox", 
18:17:23     INFO -     "build_number": 2, 
18:17:23     INFO -     "taskId": "BczAgDHfQdueE5rocPgjnQ", 
18:17:23     INFO -     "repository": "", 
18:17:23     INFO -     "buildername": "release-date_firefox_bncr_sub", 
18:17:23     INFO -     "basedir": "/builds/slave/rel-date_fx_bncr_sub-000000000", 
18:17:23     INFO -     "buildnumber": 7, 
18:17:23     INFO -     "slavename": "bld-linux64-spot-535", 
18:17:23     INFO -     "version": "44.0b31", 
18:17:23     INFO -     "release_promotion": true, 
18:17:23     INFO -     "platform": null, 
18:17:23     INFO -     "branch": "releases/date", 
18:17:23     INFO -     "bouncer_enabled": true, 
18:17:23     INFO -     "script_repo_revision": "production", 
18:17:23     INFO -     "master": "http://buildbot-master73.bb.releng.usw2.mozilla.com:8001/", 
18:17:23     INFO -     "revision": "53f6e3a60916b82de924c8878249930453af9312", 
18:17:23     INFO -     "partial_versions": "44.0b2,", 
18:17:23     INFO -     "repo_path": "projects/date"
18:17:23     INFO - }
18:17:23     INFO - Overriding product with firefox
18:17:23     INFO - Overriding version with 44.0b31
18:17:23     INFO - Overriding build_number with 2
18:17:23     INFO - Overriding revision with 53f6e3a60916b82de924c8878249930453af9312
18:17:23     INFO - Run as scripts/scripts/bouncer_submitter.py -c releases/bouncer_firefox_beta.py --credentials-file oauth.txt --bouncer-api-prefix https://bounceradmin.allizom.org/api --repo projects/date

Comment 74

[Kim Moir [:kmoir] ET]

Attachment: bug1215204mhargs2.patch

Comment 75

[Kim Moir [:kmoir] ET]

So the job is being scheduled and running but failing on the buildbot side because it can't access 
https://bounceradmin.allizom.org

<kmoir> my job from last night failed because it couldn't access the bounceradmin dev instance
10:02 AM <kmoir> http://buildbot-master72.bb.releng.usw2.mozilla.com:8001/builders/release-date_firefox_bncr_sub/builds/3/steps/run_script/logs/stdio
10:02 AM <kmoir> how do you access this instance to ensure it is up?
10:02 AM <kmoir> https://bounceradmin.allizom.org/api/product_show?product=Firefox-44.0b33-Complete
10:02 AM <kmoir> I can't access it via a browser either
10:03 AM <kmoir> I looked in the wiki and mana but couldn't find anything on it

Comment 76

[Rail Aliiev [:rail]]

Wooohoo, great progress! I think it "Should Just Work (TM)" :), especially if the command line is the same with existing one.

Comment 77

[Kim Moir [:kmoir] ET]

Attachment: bug1215204tuxedourl.patch

new staging url as per discussion this morning

Comment 78

[Kim Moir [:kmoir] ET]

I ran a build with the last patch manually and I got this error message so will fix this
09:53:58     INFO - <?xml version="1.0" encoding="utf-8"?><locations><product id="5160" name="Firefox-44.0b33-stub"><location id="26241" os="win64">/firefox/releases/44.0b33/win64/:lang/Firefox%20Setup%20Stub%2044.0b33.exe</location></product></locations>
09:53:58    FATAL - Uncaught exception: Traceback (most recent call last):
09:53:58    FATAL -   File "/builds/slave/rel-date_fx_bncr_sub-000000000/scripts/mozharness/base/script.py", line 1721, in run
09:53:58    FATAL -     self.run_action(action)
09:53:58    FATAL -   File "/builds/slave/rel-date_fx_bncr_sub-000000000/scripts/mozharness/base/script.py", line 1663, in run_action
09:53:58    FATAL -     self._possibly_run_method(method_name, error_if_missing=True)
09:53:58    FATAL -   File "/builds/slave/rel-date_fx_bncr_sub-000000000/scripts/mozharness/base/script.py", line 1604, in _possibly_run_method
09:53:58    FATAL -     return getattr(self, method_name)()
09:53:58    FATAL -   File "scripts/scripts/bouncer_submitter.py", line 158, in submit
09:53:58    FATAL -     self.submit_partials()
09:53:58    FATAL -   File "scripts/scripts/bouncer_submitter.py", line 170, in submit_partials
09:53:58    FATAL -     prev_version, prev_build_number = prev_version.split("build")
09:53:58    FATAL - ValueError: need more than 1 value to unpack
09:53:58    FATAL - Running post_fatal callback...
09:53:58    FATAL - Exiting -1

Comment 79

[Justin Wood (:Callek)]

Comment on attachment 8706459 [details] [diff] [review]
bug1215204tuxedourl.patch

Review of attachment 8706459 [details] [diff] [review]:
-----------------------------------------------------------------

Would be nice to also touch the rest of the files as well... (but not required by this review):

https://dxr.mozilla.org/build-central/search?q=bounceradmin.allizom&redirect=false&case=false

Comment 80

[Kim Moir [:kmoir] ET]

Attachment: bug1215204updateall.patch

updates to other pointers to staging bouncer server

Comment 81

[Kim Moir [:kmoir] ET]

My bouncer job is failing now because
http://buildbot-master74.bb.releng.usw2.mozilla.com:8001/builders/release-date_firefox_bncr_sub/builds/10/steps/run_script/logs/stdio

the versions specified in taskcluster for previous versions look like this

"partial_versions": "44.0b2, 44.0b4, 44.0b6,", 
which gets split and added into prev_versions

while the ones with the old jobs look like this
'prev_versions': ('43.0.1build1', '43.0.3build1', '43.0.2build1'),

Yet the code wants to split them both like this
File "scripts/scripts/bouncer_submitter.py", line 170, in submit_partials
06:25:21    FATAL -     prev_version, prev_build_number = prev_version.split("build")

Should I just add some code to bouncer_submitter.py so that they are split differently for release promotion builds or should the original value be different?

Comment 82

[Rail Aliiev [:rail]]

I'd probably change https://github.com/rail/releasetasks/blob/master/releasetasks/templates/bouncer.yml.tmpl#L30

to something like 

partial_versions: "{{ ",".join(sorted(partial_updates.keys()) }}"

and adjust the test accordingly.

Comment 83

[Kim Moir [:kmoir] ET]

I have a pull request to address the previous comment

https://github.com/mozilla/releasetasks/pulls

However, I'm stuck on another issue
in the templates for releasetasks we don't input the build number for partials for bouncer (However the partial builder is included in the l10n and funsize tasks as a point of reference)
the existing code splits the partials based on "build" in 
http://hg.mozilla.org/build/tools/file/tip/buildfarm/release/release-runner.py#l72
and https://hg.mozilla.org/projects/date/file/tip/testing/mozharness/scripts/bouncer_submitter.py#l170

So I think the https://github.com/mozilla/releasetasks/blob/master/releasetasks/templates/bouncer.yml.tmpl
should include  previousBuildNumber: {{ partial_info["buildNumber"] }} appended to this values in here 
 partial_versions: "{{ ", ".join(sorted(partial_updates.keys())) }}"

so that when the job runs it has values 
like
'prev_versions': ('44.0b2build1', '44.0b4build1', '44.0b6build1'),
instead of the current values in the release promotion jobs on date

that look like this
"44.0b2, 44.0b4, 44.0b6"

is this on the right track?

Comment 84

[Rail Aliiev [:rail]]

Yeah, looks like we need to pass the build numbers as well. probably something like https://gist.github.com/rail/e182c4cdd38fab03c9fa may resolve the issue. Not sure how to kill the trailing coma though. Maybe the coma is not that important.

Comment 85

[Justin Wood (:Callek)]

(In reply to Rail Aliiev [:rail] from comment #84)
> Yeah, looks like we need to pass the build numbers as well. probably
> something like https://gist.github.com/rail/e182c4cdd38fab03c9fa may resolve
> the issue. Not sure how to kill the trailing coma though. Maybe the coma is
> not that important.

Just found this:

http://jinja.pocoo.org/docs/dev/templates/#joiner

via:

http://stackoverflow.com/a/33940873

Comment 86

[Kim Moir [:kmoir] ET]

Attachment: pull request

Pull request to fix issue with missing build info in bouncer data + remove last comma in list

Comment 87

[Kim Moir [:kmoir] ET]

bouncer submitter worked when I updated the dev instance to the latest version of releasetasks and ran another release


http://buildbot-master72.bb.releng.usw2.mozilla.com:8001/builders/release-date_firefox_bncr_sub/builds/6

Comment 88

[Kim Moir [:kmoir] ET]

Attachment: bug1215204mhmi.patch

mh patches from date to land on m-i

Comment 89

[Rail Aliiev [:rail]]

Comment on attachment 8708020 [details] [diff] [review]
bug1215204mhmi.patch

lgtm

Comment 90

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/85224fb4d75b

Comment 91

[Kim Moir [:kmoir] ET]

Comment on attachment 8708020 [details] [diff] [review]
bug1215204mhmi.patch

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=85224fb4d75b

Comment 92

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/85224fb4d75b

Comment 93

[Rail Aliiev [:rail]]

synced to esr45: https://hg.mozilla.org/releases/mozilla-esr45/pushloghtml?changeset=c6c98bb9b707

Comment 94

[Rail Aliiev [:rail]]

one more: https://hg.mozilla.org/releases/mozilla-esr45/rev/d3b53881c3f4