It would be great to mirror some production data to dev balrog instance. This patch changes the config to support multiple servers and gets rid of balrog_username in favour of balrog_usernames = {None: "default_username"}. It also makes sure that some of the servers are mandatory (ignore_failures: False) and some not. I haven't tested this yet.

Comment on attachment 8578314 [details] [diff] [review] mirror_to_dev_balrog-mozharness.diff Rail told me he has a newer version of this patch incoming....

I tested this patch on Android nightly and L10N, Firefox Nightly. You can see the results here: http://dev-master2.bb.releng.use1.mozilla.com:8033/builders No more balrog vars in build_pool_specifics.py. From what I found the only consumer was fx_desktop_build.py. Other scripts use balrog/*.py files. I added --config ... in the next patch.

Comment on attachment 8578751 [details] [diff] [review] mirror_to_dev_balrog-mozharness.diff Review of attachment 8578751 [details] [diff] [review]: ----------------------------------------------------------------- Have well have you tested this? It looks fine to me, and I'm extremely happy to get rid of the duplicate balrog server information, but it looks very risky to land. Maybe pinned Mozharness makes it safer because you can test it out on a twig first? ::: mozharness/mozilla/updates/balrog.py @@ +118,5 @@ > > + server_args = [ > + "--api-root", server["balrog_api_root"], > + "--username", self._query_balrog_username(server) > + ] Seeing this repeated 3 times has a smell. Do you think it's worth refactoring?

Comment on attachment 8578752 [details] [diff] [review] balrog_configs-buildbot-configs.diff Review of attachment 8578752 [details] [diff] [review]: ----------------------------------------------------------------- Is this safe to land _without_ the Mozharness patch being present? Pinned Mozharness menas this will be active for some branches before the Mozharness changes take effect.

(In reply to Ben Hearsum [:bhearsum] from comment #4) > Comment on attachment 8578751 [details] [diff] [review] > mirror_to_dev_balrog-mozharness.diff > > Review of attachment 8578751 [details] [diff] [review]: > ----------------------------------------------------------------- > > Have well have you tested this? It looks fine to me, and I'm extremely happy > to get rid of the duplicate balrog server information, but it looks very > risky to land. Maybe pinned Mozharness makes it safer because you can test > it out on a twig first? > > ::: mozharness/mozilla/updates/balrog.py > @@ +118,5 @@ > > > > + server_args = [ > > + "--api-root", server["balrog_api_root"], > > + "--username", self._query_balrog_username(server) > > + ] > > Seeing this repeated 3 times has a smell. Do you think it's worth > refactoring? I thought about that, but then decided to leave it like this, because different scripts may have different arguments. It's a bit verbose, but explicit and easy to find/change the arguments.

(In reply to Ben Hearsum [:bhearsum] from comment #5) > Comment on attachment 8578752 [details] [diff] [review] > balrog_configs-buildbot-configs.diff > > Review of attachment 8578752 [details] [diff] [review]: > ----------------------------------------------------------------- > > Is this safe to land _without_ the Mozharness patch being present? Pinned > Mozharness menas this will be active for some branches before the Mozharness > changes take effect. Yes, I think they are safe to land before mozharness changes. The plan is: 1) land the configs+mozharness, reconfig. IIRC, it will affect only Android builds. 2) Land an in-tree patch (TBD) to activate the change for desktop builds.

Typoed

Comment on attachment 8578751 [details] [diff] [review] mirror_to_dev_balrog-mozharness.diff Review of attachment 8578751 [details] [diff] [review]: ----------------------------------------------------------------- looks good! the `all(rc == 0 for rc in return_codes)` parts worries me as return_codes in mh when 0 (False) are expected to signify success while > 0 (True) signifies a failure. see in line comments for more details r- for now. I'll r+ this patch if you explain that I am reading incorrectly ::: mozharness/mozilla/building/buildbase.py @@ -1621,3 @@ > return > > - if c['balrog_api_root']: wow, did I really check for balrog_api_root twice? :) ::: mozharness/mozilla/updates/balrog.py @@ +62,5 @@ > + if server["ignore_failures"]: > + self.info("Ignoring result, ignore_failures set to True") > + else: > + return_codes.append(return_code) > + return all(rc == 0 for rc in return_codes) so, iiuc, before this used to return an int and 0 (which equates to False) meant success. Now this returns a boolean and True equates to a success. this will likely be a problem here: http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/building/buildbase.py#1624

Good catch! This should fix it. Interdiff: https://gist.github.com/rail/3de6b7987821852b5f10

Comment on attachment 8579697 [details] [diff] [review] mirror_to_dev_balrog-mozharness-1.diff Review of attachment 8579697 [details] [diff] [review]: ----------------------------------------------------------------- nice. those built-in functions sure come in handy!

Comment on attachment 8578752 [details] [diff] [review] balrog_configs-buildbot-configs.diff Review of attachment 8578752 [details] [diff] [review]: ----------------------------------------------------------------- looks good I think. If this lands and reconfigs before mozharness updates across all trees, I'm guessing this will just be a no-op for the desktop builds because build_pool_specifics.py will still have the balrog keys that configs/balrog/{staging,production}.py have. the order that does matter is landing + reconfig buildbot patch before pushing out the mozharness patch.

Yeah, once the configs are landed we can proceed with other patches, including TBD in-tree pinning.

Comment on attachment 8578752 [details] [diff] [review] balrog_configs-buildbot-configs.diff https://hg.mozilla.org/build/buildbot-configs/rev/1d12367da5cb

Comment on attachment 8579287 [details] [diff] [review] puppet.diff remote: View your changes here: remote: https://hg.mozilla.org/build/puppet/rev/9ab40dc6cd57 remote: https://hg.mozilla.org/build/puppet/rev/0bf85d2de5ee

Comment on attachment 8579697 [details] [diff] [review] mirror_to_dev_balrog-mozharness-1.diff https://hg.mozilla.org/build/mozharness/rev/ff37fa579a65

In production: https://hg.mozilla.org/build/buildbot-configs/rev/1d12367da5cb

mozharness production tag moved to: https://hg.mozilla.org/build/mozharness/rev/production

It worked fine for m-c en-US nighlty.

Not sure if I want to bother myself with nightly l10n repacks, because they will be switching to mozharness soon.