Closed Bug 1083326 Opened 10 years ago Closed 10 years ago

Script aurora (un)throttling

Categories

(Release Engineering :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rail, Assigned: bhearsum)

References

Details

(Whiteboard: [merge])

Attachments

(2 files, 2 obsolete files)

I think we have all of the necessary pieces in the balrog client to do this....should be easy!
Assignee: nobody → bhearsum
I think the most difficult part of this is figuring out what releases to lock the rules to without the user needing to enter it. I can't think of a case where we wouldn't want to use the most recent nightlies at the time of the script runs, but figuring that out programatically isn't possible yet. We'll need to:
* Add a /releases API endpoint to Balrog (which is needed for the 2.0 UI anyways, I think).
** Need to be able to query by product name, partial release name at minimum, ascending+descending order, maximum number of results (for performance reasons)
* Add support to the balrog submission tools to do work with these endpoints
* Write a new script that takes a rule id, product name, and branch name. It will query Balrog for the latest release name that matches product name + branch name and update the given rule id's mapping to it.
* Add a new action to the merge day script that calls the new balrog script.

Rail, how does the above sound to you?
Flags: needinfo?(rail)
All sounds reasonable to me. You can also add an ability to override the mapping name by the client, so can pass it to the server, but for this task it's an overkill. Using the latest build ID (or whatever we have in the latest blob) is a perfect default.
Flags: needinfo?(rail)
(In reply to Rail Aliiev [:rail] from comment #3)
> All sounds reasonable to me. You can also add an ability to override the
> mapping name by the client, so can pass it to the server, but for this task
> it's an overkill. Using the latest build ID (or whatever we have in the
> latest blob) is a perfect default.

Hm, I hadn't considered using the buildid in the blob. If we did that, we wouldn't need any new endpoints - but we'd be guessing at the blob name in the client. Eg, read latest blob to grab buildid, construct new blob name of $product-$branch-nightly-$buildid. That sounds a little hacky, but not terrible...

I do like the idea of being able to override though - that would be useful for reusing the script for other things. That should be trivial to build in, so I'll plan to do it.
This needs testing, and probably some more safeguards around it, but I wanted to get high level feedback first. Should I be adding a new class in submitter/cli.py? Maybe one for locking and one for unlocking?

It depends on a patch to Balrog that will let it return JSON for GETs to /rules/:id.
Attachment #8508125 - Flags: feedback?(rail)
Comment on attachment 8508125 [details] [diff] [review]
untested script to lock/unlock nightly rules

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

::: scripts/updates/balrog-nightly-locker.py
@@ +5,5 @@
> +
> +# Use explicit version of python-requests
> +sys.path.insert(0, path.join(path.dirname(__file__),
> +                             "../../lib/python/vendor/requests-0.10.8"))
> +sys.path.insert(0, path.join(path.dirname(__file__), "../../lib/python"))

any reason tho not use site.addsitedir?

@@ +31,5 @@
> +
> +    for opt in ('api_root', 'credentials_file', 'username'):
> +        if not getattr(options, opt):
> +            print >>sys.stderr, "Required option %s not present" % opt
> +            sys.exit(1)

y u no argparse with required=True? :)

@@ +75,5 @@
> +
> +        for rule_id in options.rule_ids:
> +            rule_data, _ = rule_api.get_data(rule_id)
> +            root_name = rule_data["mapping"].split("-")[:-1]
> +            release_name = "%s-latest" % root_name

root_name is a list, probably want to join() or rule_data["mapping"].rsplit("-", 1)[0]
Attachment #8508125 - Flags: feedback?(rail) → feedback+
Depends on: 1085588
Attached patch tested scriptSplinter Review
I think this is feature complete now. I switched to argparse as you requested (which really was a lot nicer, I have to say). I had to keep with the old requests version, because the Balrog submitter requires it :(.

I improved logging, added a dry run mode, and tested this pretty well against dev. I also made sure that repeatedly locking or unlocking rules doesn't cause any harm (it just does the same thing over and over again).

One thing that maybe should be improved is handling of rules that point to non-nightly releases. For example, when I try to lock a release rule:
➜  updates git:(aurora-throttle) python balrog-nightly-locker.py -a https://aus4-admin-dev.allizom.org -c creds.py -u bhearsum@mozilla.com -r 40 lock
Starting new HTTPS connection (1): aus4-admin-dev.allizom.org
Starting new HTTPS connection (1): aus4-admin-dev.allizom.org
Locking rule 40 to Firefox-34.0b2-20141020184313
Starting new HTTPS connection (2): aus4-admin-dev.allizom.org
Caught HTTPError: mapping
Traceback (most recent call last):
  File "balrog-nightly-locker.py", line 72, in <module>
    rule_api.update_rule(rule_id, mapping=release_name)
  File "/home/bhearsum/repos/working/tools/lib/python/balrog/submitter/api.py", line 172, in update_rule
    url_template_vars=url_template_vars)
  File "/home/bhearsum/repos/working/tools/lib/python/balrog/submitter/api.py", line 103, in request
    return self.do_request(url, data, method, url_template_vars)
  File "/home/bhearsum/repos/working/tools/lib/python/balrog/submitter/api.py", line 118, in do_request
    headers=headers)
  File "../../lib/python/vendor/requests-0.10.8/requests/sessions.py", line 203, in request
    r.send(prefetch=prefetch)
  File "../../lib/python/vendor/requests-0.10.8/requests/models.py", line 585, in send
    self.response.raise_for_status()
  File "../../lib/python/vendor/requests-0.10.8/requests/models.py", line 810, in raise_for_status
    raise http_error
requests.exceptions.HTTPError: 400 Client Error


So it's not necessarily dangerous (not now, at least), but it's not exactly user friendly. What do you think?


Even with this landed we may need more work in this bug to integrate it with something. The uplift script, maybe?
Attachment #8508125 - Attachment is obsolete: true
Attachment #8508985 - Flags: review?(rail)
Comment on attachment 8508985 [details] [diff] [review]
tested script

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

Ship it!

::: scripts/updates/balrog-nightly-locker.py
@@ +22,5 @@
> +    parser.add_argument("-v", "--verbose", dest="verbose", action="store_true", default=False)
> +    parser.add_argument("-r", "--rule-id", dest="rule_ids", action="append", required=True)
> +    parser.add_argument("-n", "--dry-run", dest="dry_run", action="store_true", default=False)
> +    parser.add_argument("action", nargs=1, choices=["lock", "unlock"])
> +    parser.add_argument("action_args", nargs=REMAINDER)

Oh, I didn't know about REMAINDER! It turnes out it equals to "..." :)
Attachment #8508985 - Flags: review?(rail) → review+
Attachment #8508985 - Flags: checked-in+
So, this script is landed but it still needs to be integrated into....something.
I only did rudimentary testing on my laptop so far, but it worked! I'd kind of like to do a full run of gecko_migration.py, but I'm not exactly sure how -- is it just a matter of changing a bunch of repo paths?
Attachment #8512061 - Flags: feedback?(rail)
Comment on attachment 8512061 [details] [diff] [review]
first crack at integration with gecko migration script

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

(In reply to Ben Hearsum [:bhearsum] from comment #10)
> Created attachment 8512061 [details] [diff] [review]
> first crack at integration with gecko migration script
> 
> I only did rudimentary testing on my laptop so far, but it worked! I'd kind
> of like to do a full run of gecko_migration.py, but I'm not exactly sure how
> -- is it just a matter of changing a bunch of repo paths?

You can run the script as following:

python mozharness/scripts/merge_day/gecko_migration.py -c mozharness/configs/merge_day/aurora_to_beta.py

It won't push anything by default. It won't even commit the last config bump (unless you tell it with --commit-changes --push). I think you don't need to test pushing with the changes you made.

::: mozharness/mozilla/updates/balrog.py
@@ +83,5 @@
> +        return_code = self.retry(
> +            self.run_command, attempts=5, args=(cmd,),
> +        )
> +        if return_code not in [0]:
> +            self.return_code = 1

Do you need to set self.return_code for 0?
Attachment #8512061 - Flags: feedback?(rail) → feedback+
(In reply to Rail Aliiev [:rail] from comment #11)
> Comment on attachment 8512061 [details] [diff] [review]
> first crack at integration with gecko migration script
> 
> Review of attachment 8512061 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> (In reply to Ben Hearsum [:bhearsum] from comment #10)
> > Created attachment 8512061 [details] [diff] [review]
> > first crack at integration with gecko migration script
> > 
> > I only did rudimentary testing on my laptop so far, but it worked! I'd kind
> > of like to do a full run of gecko_migration.py, but I'm not exactly sure how
> > -- is it just a matter of changing a bunch of repo paths?
> 
> You can run the script as following:
> 
> python mozharness/scripts/merge_day/gecko_migration.py -c
> mozharness/configs/merge_day/aurora_to_beta.py
> 
> It won't push anything by default. It won't even commit the last config bump
> (unless you tell it with --commit-changes --push). I think you don't need to
> test pushing with the changes you made.
> 
> ::: mozharness/mozilla/updates/balrog.py
> @@ +83,5 @@
> > +        return_code = self.retry(
> > +            self.run_command, attempts=5, args=(cmd,),
> > +        )
> > +        if return_code not in [0]:
> > +            self.return_code = 1
> 
> Do you need to set self.return_code for 0?

I don't think so...it defaults to 0: http://mxr.mozilla.org/build-central/source/mozharness/mozharness/base/script.py#1041
The only change in this patch is setting the actions for the central -> aurora config.

I think this is well tested enough. Here's a gecko migration run:
(tuxedo)➜  merge_day git:(aurora-lock-merge-day) ✗ python gecko_migration.py --config-file ~/repos/working/mozharness/configs/balrog/staging.py --config ~/repos/working/mozharness/configs/merge_day/central_to_aurora.py --balrog-credentials-file creds.txt --balrog-username stage-ffxbld
<snip>
17:05:14     INFO - #####
17:05:14     INFO - ##### Running lock-update-paths step.
17:05:14     INFO - #####
17:05:14     INFO - Running main action method: lock_update_paths
17:05:14     INFO - Calling Balrog rule locking script.
17:05:14     INFO - retry: Calling <bound method GeckoMigration.run_command of <__main__.GeckoMigration object at 0x7f5690b476d0>> with args: (['python', '/home/bhearsum/repos/working/mozharness/scripts/merge_day/build/tools/scripts/updates/balrog-nightly-locker.py', '--api-root', 'https://aus4-admin-dev.allizom.org', '--username', 'stage-ffxbld', '--credentials-file', '/home/bhearsum/repos/working/mozharness/scripts/merge_day/creds.txt', '-n', '-r', '1', '--verbose', 'lock'],), kwargs: {}, attempt #1
17:05:14     INFO - Running command: ['python', '/home/bhearsum/repos/working/mozharness/scripts/merge_day/build/tools/scripts/updates/balrog-nightly-locker.py', '--api-root', 'https://aus4-admin-dev.allizom.org', '--username', 'stage-ffxbld', '--credentials-file', '/home/bhearsum/repos/working/mozharness/scripts/merge_day/creds.txt', '-n', '-r', '1', '--verbose', 'lock']
17:05:14     INFO - Copy/paste: python /home/bhearsum/repos/working/mozharness/scripts/merge_day/build/tools/scripts/updates/balrog-nightly-locker.py --api-root https://aus4-admin-dev.allizom.org --username stage-ffxbld --credentials-file /home/bhearsum/repos/working/mozharness/scripts/merge_day/creds.txt -n -r 1 --verbose lock
17:05:14     INFO -  Balrog request to https://aus4-admin-dev.allizom.org/rules/1
17:05:14     INFO -  Data sent: None
17:05:14     INFO -  Starting new HTTPS connection (1): aus4-admin-dev.allizom.org
17:05:16     INFO -  "GET /rules/1 HTTP/1.1" 200 372
17:05:16     INFO -  Balrog request to https://aus4-admin-dev.allizom.org/releases/Firefox-mozilla-central-nightly-latest
17:05:16     INFO -  Data sent: None
17:05:16     INFO -  Starting new HTTPS connection (1): aus4-admin-dev.allizom.org
17:05:18     INFO -  "GET /releases/Firefox-mozilla-central-nightly-latest HTTP/1.1" 200 74740
17:05:18     INFO -  Would've locked rule 1 to Firefox-mozilla-central-nightly-00000000000000
17:05:18     INFO - Return code: 0


And I did a rerun of b2g nightly to make sure I didn't break the BalrogMixin:
13:40:08     INFO - Calling Balrog submission script
13:40:08     INFO - retry: Calling <bound method B2GBuild.run_command of <__main__.B2GBuild object at 0x1b003d0>> with args: (['python', '/builds/slave/b2g_m-cen_ham_ntly-00000000000/build/build-tools/scripts/updates/balrog-submitter.py', '--build-properties', '/builds/slave/b2g_m-cen_ham_ntly-00000000000/balrog_props.json', '--api-root', 'https://aus4-admin-dev.allizom.org', '--username', 'stage-b2gbld', '-t', 'nightly', '--credentials-file', '/builds/slave/b2g_m-cen_ham_ntly-00000000000/oauth.txt', '--verbose'],), kwargs: {}, attempt #1
13:40:08     INFO - Running command: ['python', '/builds/slave/b2g_m-cen_ham_ntly-00000000000/build/build-tools/scripts/updates/balrog-submitter.py', '--build-properties', '/builds/slave/b2g_m-cen_ham_ntly-00000000000/balrog_props.json', '--api-root', 'https://aus4-admin-dev.allizom.org', '--username', 'stage-b2gbld', '-t', 'nightly', '--credentials-file', '/builds/slave/b2g_m-cen_ham_ntly-00000000000/oauth.txt', '--verbose']
13:40:08     INFO - Copy/paste: python /builds/slave/b2g_m-cen_ham_ntly-00000000000/build/build-tools/scripts/updates/balrog-submitter.py --build-properties /builds/slave/b2g_m-cen_ham_ntly-00000000000/balrog_props.json --api-root https://aus4-admin-dev.allizom.org --username stage-b2gbld -t nightly --credentials-file /builds/slave/b2g_m-cen_ham_ntly-00000000000/oauth.txt --verbose
13:40:08     INFO -  Balrog request to https://aus4-admin-dev.allizom.org/releases/B2G-mozilla-central-nightly-00000000000000
13:40:08     INFO -  Data sent: None
13:40:08     INFO -  Starting new HTTPS connection (1): aus4-admin-dev.allizom.org
13:40:10     INFO -  "HEAD /releases/B2G-mozilla-central-nightly-00000000000000 HTTP/1.1" 404 0
13:40:21     INFO -  Caught HTTPError:
13:40:21     INFO -  Balrog request to https://aus4-admin-dev.allizom.org/csrf_token
13:40:21     INFO -  Data sent: None
13:40:21     INFO -  Starting new HTTPS connection (2): aus4-admin-dev.allizom.org
13:40:22     INFO -  "HEAD /csrf_token HTTP/1.1" 200 0
13:40:22     INFO -  Balrog request to https://aus4-admin-dev.allizom.org/releases/B2G-mozilla-central-nightly-00000000000000/builds/hamachi/en-US
13:40:22     INFO -  Data sent: {'product': u'B2G', 'hashFunction': u'sha512', 'schema_version': 4, 'alias': 'null', 'copyTo': '["B2G-mozilla-central-nightly-latest"]', 'version': u'36.0a1', 'data': '{"platformVersion": "36.0a1", "isOSUpdate": true, "completes": [{"fileUrl": "http://ftp.mozilla.org/pub/mozilla.org/b2g/nightly/latest-mozilla-central/fota-hamachi-update.mar", "hashValue": "d92016b9106c733d2a320cd0fe7f8e2bbadd1b1ff07a8a9598bcd915fd2e63f1d7ccfd5e55c5c452f11ba2c137ff863350b7847770464a77521d9e1ea5420a7b", "from": "*", "filesize": 61250817}], "buildID": "00000000000000", "displayVersion": "36.0a1", "appVersion": "36.0a1"}'}
13:40:22     INFO -  Starting new HTTPS connection (3): aus4-admin-dev.allizom.org
13:40:23     INFO -  "PUT /releases/B2G-mozilla-central-nightly-00000000000000/builds/hamachi/en-US HTTP/1.1" 201 23
Attachment #8512061 - Attachment is obsolete: true
Attachment #8512629 - Flags: review?(rail)
Attachment #8512629 - Flags: review?(rail) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Depends on: 1100474
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: