New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 780876 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 780892
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 780494



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

Project Member Reported by akes...@chromium.org, Nov 2 2017

Issue description

balance_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.
 
Blocking: 780494
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.

SGTM, but I think the "--model foo" -> "label model:foo" translation should happen within balance_pool (again, much like run_suite).
Re #3. Ack.
Cc: shapiroc@chromium.org
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).
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.

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.
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)
Mergedinto: 780892
Status: Duplicate (was: Started)
Project Member

Comment 10 by bugdroid1@chromium.org, 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

Project Member

Comment 11 by bugdroid1@chromium.org, 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

Project Member

Comment 12 by bugdroid1@chromium.org, 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