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

Issue 771699 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature



Sign in to add a comment

EXPERIMENTAL=foo-chrome-pfq should not be respected by master-chromium-pfq (due to risk to binhost test)

Project Member Reported by akes...@chromium.org, Oct 4 2017

Issue description

We use the EXPERIMENTAL='foo-paladin' feature to easily disable paladins.

However, this feature (which is currently also respected by the master-chromium-pfq) should not be allowed on chrome-pfq, because it shortcuts binhost-test.

Binhost test relies on the in-source definition of what is and isn't important to determine which prebuilts will be uploaded. That is used to calculate whether the current pfq run will provide sufficient prebuilts for the varies use flag sets needed by the paladins. With tree-status experimental for chrome pfq we are vulnerable to the following breakage:

1) foo-chrome-pfq is the only pfq builder that provides chrome prebuilts for some useflag (+foo) and it is marked as important=True in chromeos_config.

2) the tree status is set EXPERIMENTAL=foo-chrome-pfq

3) binhost test on master-chromium-pfq passes, because it relies on chromeos_config to see that foo-chrome-pfq is marked as important

4) foo-chrome-pfq fails to build, other pfq builds pass.

5) master-chromium-pfq commits the chrome uprev. Now the -paladins are broken because there is no prebuilt for chrome with (+foo)
 
I'm judging that master-chromium-pfq respects this based on the logging in https://uberchromegw.corp.google.com/i/chromeos/builders/master-chromium-pfq/builds/4962/steps/MasterSlaveSyncCompletion/logs/stdio

02:16:40: INFO: Got experimental build configs [u'guado_moblab-paladin', u'betty-arc64-paladin', u'wizpig-paladin', u'betty-chrome-pfq'] from tree status.



It's possible that it merely fetches the experimental configs from tree status, but doesn't actually respect them.
That log comes from build_status.UpdateSlaveStatus:
https://cs.corp.google.com/chromeos_public/chromite/cbuildbot/build_status.py?rcl=221bd82440159b189d1fab1f28586794f6a3d9ae&l=148

The list of experimental builders gets polled here while waiting for slaves to complete and stored in metadata with key METADATA_EXPERIMENTAL_BUILDERS, and is used as an input to determine whether to keep waiting for slaves or not.

The list of experimental builders ends up getting used in BinhostConfWriter, where if a slave is experimental it won't upload a prebuilt for it. I suppose this is the case that you are concerned about?
> The list of experimental builders ends up getting used in BinhostConfWriter, where if a slave is experimental it won't upload a prebuilt for it. I suppose this is the case that you are concerned about?

I think technically this will affect whether prebuilts are published, not uploaded (uploading happens on the slaves).

But yes, that's the gist of the problem. 
Components: Infra>Client>ChromeOS>CI
Components: -Infra>Client>ChromeOS
Cc: -davidjames@chromium.org

Comment 8 by nxia@chromium.org, Jun 8 2018

Cc: -nxia@chromium.org
Labels: -Pri-1 -Type-Bug Pri-2 Type-Feature
Note that EXPERIMENTAL= is about to be removed so we may just implement this in the new UI with the relevant restrictions on particular types of builders.

Sign in to add a comment