Remove gerrit-buildbucket-config subcommand from mb |
||||||||||||||||||
Issue descriptionThe preferred way to update buildbucket.config on the meta/config branch (which controls what builders are available in Gerrit for try jobs) is to generate the file from the mb_config.pyl file. However, that file doesn't know about LUCI buckets (since the recipes still use mastername and buildername), and so if we move a builder from buildbot to LUCI, mb will try to move it back. For now, we can hand-edit the resulting file to work around this, but we'll need to fix this ASAP.
,
Feb 21 2018
resulting config file is trivial [1], biggest knowledge requirement is for mb, with which i'm not familiar and I doubt anyone on foundation team is. My guess is the only thing to do is adding another field to annotate each builder which mb knows about to specify that it's on LUCI. Then, for each trybot on LUCI, mb should emit 'luci.chromium.try' bucket. [1] https://chromium.googlesource.com/chromium/src/+/refs/meta/config/buildbucket.config
,
Feb 21 2018
I'm a good owner for this, since it's my code ;) I think tandrii@ is right and we'll either need to do something like that or figure out how to parse //infra/config/cq.cfg and extract the information out of that. Do we have the generated python stubs for that proto somewhere that I can check into src (or is it already checked in somewhere)?
,
Feb 21 2018
> Do we have the generated python stubs for that proto somewhere that I can check into src (or is it already checked in somewhere)? turns out we do, but it's outdated and probably should be removed from there anyway: https://chromium.googlesource.com/chromium/tools/depot_tools.git/+/master/third_party/cq_client/ maintain it is a pain, because it requires manual file editing to work around python protobuf imports in google namespace :(
,
Feb 21 2018
Mkay, do we have a vpython package for this? Or, what is the recommended way for me to read this proto from python?
,
Feb 21 2018
No, there is no vpython package. But, I don't see how parsing cq.cfg is sufficient to cover all try builders, not all which are in cq.cfg, but all of which should be in gerrit UI.
,
Feb 21 2018
The LUCI builders should all be in cr-buildbucket.cfg - https://chromium.googlesource.com/chromium/src/+/infra/config/cr-buildbucket.cfg which is also a protobuf, though in a separate ref, and is served by luci-config. If the overall goal here is to keep the list of tryjobs in Gerrit in sync, can buildbucket plugin read these builders directly from luci-config? Perhaps, in addition to the current config for buildbot builders (which then wouldn't need the mb change)?
,
Feb 21 2018
> I don't see how parsing cq.cfg is sufficient to cover all try builders, > not all which are in cq.cfg, but all of which should be in gerrit UI. Ah, good point. > The LUCI builders should all be in cr-buildbucket.cfg - > https://chromium.googlesource.com/chromium/src/+/infra/config/cr-buildbucket.cfg > > which is also a protobuf, though in a separate ref, and is served by luci-config. Being on a separate ref makes that more annoying in this case, all right (in every case, really), though I think tandrii was planning to move this to master? Also, cr-buildbucket.cfg doesn't list which builders are present in buildbot, so it's not perfect. I'm not sure if that's enough to tell us whether to link to the LUCI builder, or the Buildbot builder, or both. Modifying the plugin to read from luci config sounds like an interesting idea, but (a) isn't something I'm going to do, that's probably more of a tandrii thing, and (b) is probably substantially more work.
,
Feb 21 2018
I think maybe the right thing to do is: 1) If the bot hasn't been migrated at all, only show the buildbot version 2) If the bot has been 100% migrated, only show the LUCI version 3) Otherwise, show both And I think in order to do that I probably need to query the migration app for the current state, since the needed information is scattered across too many places. @nodir - is there some way I can do that?
,
Feb 21 2018
Re: #c5: I'd say, the recommended way to read protos in Python is through "import protobuf". We don't have special packages for reading specifically buildbucket or CQ protos, but I suppose once you know the schema, reading generically is just as easy. There is a generic CIPD package for protobuf: infra/python/wheels/protobuf Example usage: https://chromium.googlesource.com/chromium/src/tools/franky/+/master/test.py.vpython#67
,
Feb 22 2018
1. Just wrote proposal for moving configs to master branch. I'm ready to execute on it on Friday if nobody finds blockers. (internal doc http://shortn/_d7DritEU3w) 2. luci-config isn't useful as it doesn't answer question which builder has migrated. Bu drapnke@ #c9 is correct, the right way is to ask luci-migration app. Luckily it's simple urlfetch away: https://luci-migration.appspot.com/masters/tryserver.chromium.linux/builders/chromium_presubmit/?format=json (see https://cs.chromium.org/chromium/infra/go/src/infra/appengine/luci-migration/app/handlers.go?l=260)
,
Feb 22 2018
the plan in c#9 is what i had in mind, except there is https://apis-explorer.appspot.com/apis-explorer/?base=https://cr-buildbucket.appspot.com/_ah/api#p/swarmbucket/v1/swarmbucket.get_builders to return swarmbucket builds in JSON. migration app currently returns state of individual builders. That would be slow. We will need to update it to return state of all builders of a master in one RPC.
,
Feb 22 2018
,
Feb 22 2018
,
Feb 22 2018
Today I will work on BQ export in Buildbucket, not this.
,
Feb 22 2018
please update buildbucket.config manually in the meantime, or take this bug.
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/4099f00969d65e9863dd534e92a8ca92208e93a2 commit 4099f00969d65e9863dd534e92a8ca92208e93a2 Author: Nodir Turakulov <nodir@google.com> Date: Thu Feb 22 22:29:11 2018 [luci-migration] include builder states in master json response Bug: 813196 Change-Id: I5758b54f8e8d52fa78698f75104685322a0b6f5b Reviewed-on: https://chromium-review.googlesource.com/931684 Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org> Commit-Queue: Nodir Turakulov <nodir@chromium.org> [modify] https://crrev.com/4099f00969d65e9863dd534e92a8ca92208e93a2/go/src/infra/appengine/luci-migration/app/master.go
,
Feb 28 2018
,
Mar 1 2018
@nodir - does the change in #17 mean that there's an entry point I should call now? Alternatively, this is not a performance-sensitive thing, so I don't think we care too much if it's slow, as long as you wouldn't be worried that we'd DOS buildbucket (and I'd assume we wouldn't).
,
Mar 1 2018
the change in #17 is for buildbucket-gerrit plugin to call. Overall, I think this bug should be fixed on the gerrit-buildbucket plugin side.
,
Mar 1 2018
If you're suggesting that the gerrit-buildbucket plugin should just call buildbucket to determine the list, I'm all for that, but I have no idea when that might be done. In the meantime, should I make it possible to edit things by hand, or should I modify mb to call something? +agable for gerrit plugin as well.
,
Mar 1 2018
i think doing it manually occasionally is better than you spending time writing code will be deleted in a week or two.
,
Mar 1 2018
okay, if it'll be that soon, I agree. Thanks! I'll leave this open until we have a proper fix where this either works correctly or (better) can be removed altogether.
,
Mar 1 2018
,
Mar 26 2018
Issue 825315 has been merged into this issue.
,
Mar 28 2018
,
Mar 28 2018
,
Apr 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/232e8e9fc710de19f7a0e81114e4deaf8b297c4d commit 232e8e9fc710de19f7a0e81114e4deaf8b297c4d Author: Nodir Turakulov <nodir@google.com> Date: Mon Apr 23 20:22:00 2018 [luci-migration] add CORS support Buildbucket-Gerrit plugin needs to query luci_is_prod of builders to decide whether to show Buildbot or LUCI variant of a builder in "Choose trybots" dialog. Currently the browser rejects such requests due to CORS. Add CORS support. R=jchinlee@chromium.org Bug: 813196 Change-Id: Id0ef74a9a853f550c5e89b63ccb8b3cc1e712446 Reviewed-on: https://chromium-review.googlesource.com/1024510 Commit-Queue: Nodir Turakulov <nodir@chromium.org> Reviewed-by: Jao-ke Chin-Lee <jchinlee@chromium.org> [modify] https://crrev.com/232e8e9fc710de19f7a0e81114e4deaf8b297c4d/go/src/infra/appengine/luci-migration/app/handlers.go [modify] https://crrev.com/232e8e9fc710de19f7a0e81114e4deaf8b297c4d/go/src/infra/appengine/luci-migration/app/master.go [modify] https://crrev.com/232e8e9fc710de19f7a0e81114e4deaf8b297c4d/go/src/infra/appengine/luci-migration/app/builder.go
,
Apr 24 2018
,
Apr 24 2018
,
Apr 25 2018
The following revision refers to this bug: https://chromium.googlesource.com/infra/gerrit-plugins/buildbucket/+/45f02b90b32b45d97d596688dbc0e422d8fe86c0 commit 45f02b90b32b45d97d596688dbc0e422d8fe86c0 Author: Nodir Turakulov <nodir@google.com> Date: Wed Apr 25 03:17:25 2018 [buildbucket-gerrit] load master migration states For a given Buildbot bucket, some builders might already be migrated to LUCI, and some might not yet be. Whether a Buildbot builder has migrated to LUCI changes dynamically and is available in luci-migration.appspot.com app. Currently a given builder is shown twice, as Buildbot and LUCI variants. For all configured Buildbot buckets (those that start with "master.") use luci-migration API to load migration states of all of its builders. If a builder has already migrated to LUCI, hide its Buildbot variant, otherwise hide the LUCI variant. Bug: 813196 Change-Id: Ia274f5fd70d526244950c06cf1778b6ff7cb3b4e Reviewed-on: https://chromium-review.googlesource.com/1024630 Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> [modify] https://crrev.com/45f02b90b32b45d97d596688dbc0e422d8fe86c0/test/cr-buildbucket-client_test.html [modify] https://crrev.com/45f02b90b32b45d97d596688dbc0e422d8fe86c0/src/main/resources/static/cr-buildbucket-client.js [modify] https://crrev.com/45f02b90b32b45d97d596688dbc0e422d8fe86c0/test/cr-tryjob-picker_test.html [modify] https://crrev.com/45f02b90b32b45d97d596688dbc0e422d8fe86c0/src/main/resources/static/cr-tryjob-picker.js
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/infra/gerrit-plugins/buildbucket/+/c39cdbca1168e9bac33913f0b2f3d470acd52e1f commit c39cdbca1168e9bac33913f0b2f3d470acd52e1f Author: Nodir Turakulov <nodir@google.com> Date: Thu Apr 26 07:13:18 2018 [buildbucket-gerrit] simplify _updateBuckets _updateBuckets combines the logic of computing the final list of builders and updating the element state. Simplify _updateBuckets by limiting its responsibility to computing and making it pure. This simplifies its tests too. Move the responsibility of updating element state and "canceling" previous operation to _pluginConfigChanged. Also a minor improvements to comments and filtering out buckets without builders. Bug: 813196 Change-Id: I0f466de59a3d5af4940184ecc8da29131999afcf Reviewed-on: https://chromium-review.googlesource.com/1027207 Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> [modify] https://crrev.com/c39cdbca1168e9bac33913f0b2f3d470acd52e1f/test/cr-tryjob-picker_test.html [modify] https://crrev.com/c39cdbca1168e9bac33913f0b2f3d470acd52e1f/src/main/resources/static/cr-tryjob-picker.js
,
May 7 2018
the buildbucket.config file no longer needs to list LUCI builders. Dirk, I looked at mb.py trying to remove the subcommand, but I am not sure how deep should we go, e.g. I don't know if we still need luci_tryservers. Could you do this cleanup?
,
May 8 2018
> the buildbucket.config file no longer needs to list LUCI builders +kbr@ who recently documented the (now deprecated?) manual process
,
May 10 2018
,
May 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f8266167ee87cfa2b24c9c8b3379e2d603a45375 commit f8266167ee87cfa2b24c9c8b3379e2d603a45375 Author: Dirk Pranke <dpranke@chromium.org> Date: Thu May 10 01:47:45 2018 Remove gerrit-buildbucket-config command from MB. The Gerrit plugin will now automatically populate the list of tryservers from LUCI, meaning that we no longer need to manually configure things and don't need to generate a config file from the MB entries. Accordingly, this CL removes the `gerrit-buildbucket-config` code from MB. Bug: 813196 Change-Id: I1c4ba1099a11822c2c741eb21dc61e4f5a163369 Reviewed-on: https://chromium-review.googlesource.com/1053377 Reviewed-by: Nodir Turakulov <nodir@chromium.org> Reviewed-by: John Budorick <jbudorick@chromium.org> Commit-Queue: Dirk Pranke <dpranke@chromium.org> Cr-Commit-Position: refs/heads/master@{#557416} [modify] https://crrev.com/f8266167ee87cfa2b24c9c8b3379e2d603a45375/tools/mb/docs/user_guide.md [modify] https://crrev.com/f8266167ee87cfa2b24c9c8b3379e2d603a45375/tools/mb/mb.py [modify] https://crrev.com/f8266167ee87cfa2b24c9c8b3379e2d603a45375/tools/mb/mb_config.pyl [modify] https://crrev.com/f8266167ee87cfa2b24c9c8b3379e2d603a45375/tools/mb/mb_unittest.py
,
May 10 2018
The associated docs about updating refs/meta/config are step 5 in this section: https://chromium.googlesource.com/chromium/src/+/master/docs/gpu/gpu_testing_bot_details.md#How-to-add-a-new-manually_triggered-trybot Could you tell me what needs to be updated there?
,
May 10 2018
See https://groups.google.com/a/google.com/forum/m/#!topic/luci-eng/WxTclk4R1eA Step 5 in https://chromium.googlesource.com/chromium/src/+/4d1bb448ed20272310bf769ee1b25dca3af3dff4/docs/gpu/gpu_testing_bot_details.md can be deleted
,
May 11 2018
Deleted in https://chromium-review.googlesource.com/1054357 . Thanks. |
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by sergeybe...@chromium.org
, Feb 21 2018