Issue metadata
Sign in to add a comment
|
balance_pool: support restriction to a subspace (i.e. an extra label) to support "board:foo model:bar" pools |
||||||||||||||||||||||||
Issue descriptionbalance_pool is only aware of board: and pool: labels. When trying to balance or create pools for a model, we need finer grain. See Issue 780494 for context.
,
Nov 2 2017
I think we should only add a --model flag, not a generic --label restriction. tl;dr: Public API should not be more generic than required. I've learned this from looking Android platform API revisions. Overly generic API leads to compatibility headache in the future. Internally, feel free to implement it as a full blown label restriction. -------------- This is a case of making something too generic so that we start accepting user input in a not-so-well-specified schema. This is the same reasoning I gave for adding --model to run_suite. Basically, - we will not expand this definition again for years (model is the first replacement / extension of board in years) - we do not have a well specified schema for labels So, I'd rather not sign us up to support arbitrary labels that users can then throw at us.
,
Nov 2 2017
SGTM, but I think the "--model foo" -> "label model:foo" translation should happen within balance_pool (again, much like run_suite).
,
Nov 2 2017
Re #3. Ack.
,
Nov 2 2017
Problem: even with this feature, balance_pool --all-boards will not do the right thing, and DUTs from different models will diffuse over time between managed pool of the same board (because of the autobalancing cronjob).
,
Nov 2 2017
Really, we need to drop --board from this script in favour of --model. See issue 780892 and friends. Recommendation: - For now, add a --model along with --board - Have --all-boards mean match both model and board. Even if issue 780891 is not yet fixed (i.e., I don't trust that the model labels are accurate), DUTs for the same board/model should still have the same label:model if it exists.
,
Nov 2 2017
I'll take a whack at 780891 today. If that is done, we can simply replace --board with --model in balance_pool directly. (leave --board behind to mean --model while we update users of this script) And --all-boards can just restrict by model.
,
Nov 3 2017
The expectation in #6 was entirely wrong. Non-coral DUTs have models all over the place. See https://bugs.chromium.org/p/chromium/issues/detail?id=780891#c9 This means that we can't have the auto-balance match on both --board and --model. (will do the wrong thing for non-coral DUTs)
,
Nov 3 2017
,
Nov 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/c2bbdb2e1ec8dcad85679117f4d240674c60b436 commit c2bbdb2e1ec8dcad85679117f4d240674c60b436 Author: Aviv Keshet <akeshet@chromium.org> Date: Wed Nov 08 09:11:20 2017 autotest: add extra_labels arg to get_multiple_histories BUG= chromium:780876 TEST=None Change-Id: I3dc8e95336b6060562da9a2f6b31aadfa0e72f67 Reviewed-on: https://chromium-review.googlesource.com/751683 Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> Commit-Ready: Aviv Keshet <akeshet@chromium.org> Tested-by: Aviv Keshet <akeshet@chromium.org> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org> [modify] https://crrev.com/c2bbdb2e1ec8dcad85679117f4d240674c60b436/server/lib/status_history.py
,
Nov 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/ead47d59d57ff717cfd6e7506f5dd43f02278eb2 commit ead47d59d57ff717cfd6e7506f5dd43f02278eb2 Author: Aviv Keshet <akeshet@chromium.org> Date: Wed Nov 08 23:10:09 2017 autotest: balance_pool: add extra_labels support to _DUTPool This adds but does not use a new extra_labels feature to _DUTPool. BUG= chromium:780876 TEST=None Change-Id: I26a9a2919fb129edca124aec32e4659f229a820a Reviewed-on: https://chromium-review.googlesource.com/751688 Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> Tested-by: Aviv Keshet <akeshet@chromium.org> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org> [modify] https://crrev.com/ead47d59d57ff717cfd6e7506f5dd43f02278eb2/site_utils/balance_pools.py
,
Nov 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/39c20dd036c0aafc59277e86bc925c9b77f713ef commit 39c20dd036c0aafc59277e86bc925c9b77f713ef Author: Aviv Keshet <akeshet@chromium.org> Date: Wed Nov 08 23:10:09 2017 autotest: balance_pools: support a --model argument BUG= chromium:780876 TEST=./balance_pools.py -n --model astronaut cq coral -t 6 Change-Id: Ic01dc244fc4b1955c2ec207decacbf920a5260f3 Reviewed-on: https://chromium-review.googlesource.com/752026 Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> Tested-by: Aviv Keshet <akeshet@chromium.org> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org> [modify] https://crrev.com/39c20dd036c0aafc59277e86bc925c9b77f713ef/site_utils/balance_pools.py |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by akes...@chromium.org
, Nov 2 2017