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

Issue 901871 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 898576



Sign in to add a comment

Make all unibuild release builder hwtest stages async and ignore test results for builder success

Project Member Reported by shapiroc@google.com, Nov 5

Issue description

For context, see:  crbug.com/898576 

Proposal is to kick off all hwtest test stages async and ignore the results for unibuild release builders.

 
bhthompson@ ... let me know if there are any TPM objections to this.

I assume since it fails all the time already, it's basically a net-zero impact to TPM release workflows.
I have long been for defining build status as 'did the build generate proper binaries which can be tested and released', making it distinct from 'does it work', SGTM. I have not seen value in build status in a long time.

I would advocate for keeping this behavior consistent, and removing HWTest from the path to builder status on traditional release builders also. 

From a release perspective, as long as the results show up eventually (and as reliably as possible) in the Goldeneye dashboard we should be ok. 

I would be cautious about this for the purposes of other build time decisions though, such as starting additional test suites based on builder status alone. The additional out of band suites only run when the build is green because they presume that the image can pass tests at that point, if this change causes out of band suites to run even if the image is toxic to the lab, this could cause a particularly bad image to bring down the lab worse than it would have otherwise.
Cc: akes...@chromium.org
akeshet@ ... can you answer the out of band suite question?

how are the out of band suites ever working for coral (and others) if we are constantly failing anyway?

let's start with making this change for unified builds, then spread to non-uni if it makes sense (pending the out of band answer)
This is the problem Ben was pointing out in https://bugs.chromium.org/p/chromium/issues/detail?id=898576#c7 when it fails all the time, out of band suites never run, and we become starved of results unless someone starts them manually.

What we really want is such tests to be initiated on a particular model, by that particular model passing some set of prerequisite tests, however that is more complex to implement. I imagine this is part of the end game vision for how testing should work (e.g. some form of test recipes, not builders directly initiating tests). 
Cc: yshaul@chromium.org
yes ... end game will target this case.

cbuildbot makes this a bit difficult right now.  either sync and we care or async and we don't care.  anything more complex would take a bit of work to plumb in.

seems like the real requirement here is we should be tying the out-of-band suites to test results and not build results

can we adjust suite_scheduler to query tko instead and make decisions based on that?
Discussed this today...

General decision is that we're going to make the following changes in cbuildbot:
- only run bvt-sanity synchronously, then run everything else async
- for unified builds, only run bvt-sanity on a single model

This should resolve the immediate issue and buy us time until the lab is more stable.  Even then, we'll still be okay with this behavior.
Cc: johndhong@chromium.org ihf@chromium.org stagenut@chromium.org dgarr...@chromium.org derat@chromium.org djmm@chromium.org haoweiw@chromium.org dgagnon@chromium.org kbleicher@chromium.org hidehiko@chromium.org
 Issue 891467  has been merged into this issue.
Blocking: 898576
I've argued for this for a while. There was push back originally, related to how GE imported test results, but I'm not sure if it still applies.

go/cros-split-release-build-test

Cl is out for review

On Wed, Nov 7, 2018, 6:04 PM dgarrett via monorail <
monorail+v2.1654237156@chromium.org wrote:
https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1324773 going through CQ.

In order to only run 1 bvt-sanity and bvt-installer test, I'm going to make the changes in GE and let the existing config system work.  This is so we don't mess up reporting and don't have to write circumventing special case code for this in cbuildbot.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/7f1ac04ec34ee3dc2b949fae2fe989fe520af3da

commit 7f1ac04ec34ee3dc2b949fae2fe989fe520af3da
Author: C Shapiro <shapiroc@chromium.org>
Date: Mon Nov 12 18:55:57 2018

chromite: Make most release hwtests async

See related bug.  We don't need to tie hw testing on release builders to
the success of the build itself.  We're just going to do minimal testing
and then make everything else async from there.

Since the unified builds and non-unibuilds test config is different
right now, this just starts with the unified builds impl.

BUG= chromium:901871 
TEST=unit tests and config check

Change-Id: I6756ee5b9a9adfb995433fcb37e7ec5e9bff07cc
Reviewed-on: https://chromium-review.googlesource.com/c/1324773
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Reviewed-by: Aviv Keshet <akeshet@chromium.org>
Tested-by: C Shapiro <shapiroc@chromium.org>
Commit-Queue: C Shapiro <shapiroc@chromium.org>

[modify] https://crrev.com/7f1ac04ec34ee3dc2b949fae2fe989fe520af3da/config/config_dump.json
[modify] https://crrev.com/7f1ac04ec34ee3dc2b949fae2fe989fe520af3da/lib/config_lib.py
[modify] https://crrev.com/7f1ac04ec34ee3dc2b949fae2fe989fe520af3da/config/chromeos_config_test.py

All done right? Okay to close?
Status: Verified (was: Assigned)
Looking at coral's recent behavior on GE it is working as intended. Mostly green builds with two legitimate failures.

https://cros-goldeneye.corp.google.com/chromeos/console/listBuild?boards=coral&milestone=&chromeOsVersion=&chromeVersion=&startTimeFrom=&startTimeTo=&token=AFuIdJBtZHF02J59V5xJOdiPYR8U%3A1542322321173#%2F

Sign in to add a comment