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

Issue 631237 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jul 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Config-updater requests manual change in chrome_config to satisfy chrome_config_unittest

Project Member Reported by nxia@chromium.org, Jul 25 2016

Issue description

https://uberchromegw.corp.google.com/i/chromeos.infra/builders/config-updater/builds/170

This is failing as one board was changed from important:False to important:True. When important is True, the release board will be assigned a waterfall automatically. So the unit test was broke as the manual _waterfall_config_map wasn't adjusted accordingly. 

So the question here is how to allow changing "important" attributes without changing _waterfall_config_map. 
1) make active_waterfall an attribute in GE template file.
2) remove important release configs from _waterfall_config_map
3) do not run the unittest on *-release configs.

 

Comment 1 by nxia@chromium.org, Jul 26 2016

Cc: akes...@chromium.org
I'm going to solve this with option 2).
Parse ge_config_template and remove important configs from _waterfall_config_map. does this sound good to you? akeshet@
Cc: dgarr...@chromium.org
sgtm -- dgarrett opinion?
DNJ has been hoping to start using waterfall_config_map to decide which builders should exist on a given waterfall (and so to stop parsing config_dump.json in buildbot). This conflicts with removing important builds from the config map.

Is it true that all builds in the GE data file should get built, important or not?

Comment 4 by nxia@chromium.org, Jul 26 2016

All important configs and all experimental configs in waterfall_config_map should be built. 

Now the problem is people may set important flag for a config from False to True in GE, but the config still exists in waterfall_config_map, then one unit test will fail in this case.

Comment 5 by nxia@chromium.org, Jul 26 2016

If we don't remove those important configs from waterfall_config_map in our code. People still need to manually make the change and check-in the change before they change attributes in GE. so the builds need to be removed anyway. 

Comment 6 by nxia@chromium.org, Jul 26 2016

Status: Started (was: Untriaged)
Sent out the workaround cl: https://chromium-review.googlesource.com/#/c/363581/

I would say the best way to solve this issue is option 1).
Make active_waterfall an attribute in GE template file that people can change through GE. 

we could get in the workaround CL to unblock people from changing 'important' flags through GE and coordinate with GE team to work on option 1) ?


Comment 7 by nxia@chromium.org, Jul 28 2016

After talked to dgarreet@, the best solution is to remove release configs from the waterfall_cofig_map. GE should make sure only active configs will show in the ge_config_template file.
Sent out the CL already.

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 29 2016

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

commit 963456b34a5ee4ad82b35501193d45cb1fc46303
Author: Ningning Xia <nxia@chromium.org>
Date: Wed Jul 27 01:47:23 2016

Remove release configs from _waterfall_config_map.

All build configs provided in ge_build_config.json should be assigned a
valid waterfall.
Remove release configs from _waterfall_config_map.

BUG= chromium:631237 
TEST=manually dump configs

Change-Id: Ie1bb1930393609db6eceecb35a62b2ef25f98bd6
Reviewed-on: https://chromium-review.googlesource.com/363631
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Ningning Xia <nxia@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Reviewed-by: Ningning Xia <nxia@chromium.org>

[modify] https://crrev.com/963456b34a5ee4ad82b35501193d45cb1fc46303/cbuildbot/chromeos_config.py

Comment 9 by nxia@chromium.org, Jul 29 2016

Status: Fixed (was: Started)

Sign in to add a comment